Develop #96
Develop #96
Conversation
- Changed several type definitions from `type` to `interface` for better extensibility in components such as `PieSlice`, `ConfirmationContextValue`, and `QueueMessagePart`. - Updated function parameters and return types to use more consistent array syntax (e.g., `Array<Type>` instead of `Type[]`) in components like `DataTableProps` and `AttachmentsContext`. - Refactored various components to use `void` for fire-and-forget function calls, ensuring no promises are returned where void is expected, particularly in `ChatProvider` and `WorkflowActions`. - Removed unnecessary ESLint disable comments to clean up the code and improve readability. - Adjusted default parameter values to use shorthand syntax in hooks like `useDebounce` and `useToggle`. - Improved conditional return statements for clarity and consistency across multiple components. - Removed a test template file that was no longer needed. - Updated ESLint configuration to enhance type checking and ensure better adherence to coding standards.
- Refactor `AgentConfirmationProps` to remove unnecessary line breaks. - Add new icons (`ActivityIcon`, `NetworkIcon`) to `chat-messages.tsx`. - Implement type guards for various data parts in `chat-messages.tsx`. - Create `AgentDataSection`, `WorkflowDataSection`, and `NetworkDataSection` components for rendering nested execution results. - Update `NestedAgentChat` to handle form submission asynchronously and log errors. - Modify `network-chat.tsx` to handle clipboard actions asynchronously. - Improve error handling in `NetworkProvider` and `WorkflowProvider` for message sending and stopping execution. - Remove deprecated `maskSensitiveMessageData` function from `pg-storage.ts`. - Update `formatStorageMessages` to directly push messages to `messageData.parts` based on operation status.
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Reviewer's GuideThis PR strengthens type safety, improves async/Promise handling, and enriches nested tool/network/workflow rendering in chat and network UIs while tightening ESLint/TypeScript configurations and cleaning up unused‑var suppressions across the codebase. Sequence diagram for assistant message rendering with nested tool, workflow, and network partssequenceDiagram
participant UI as ChatUI
participant Msg as MessageItem
participant TG as TypeGuards
participant ADS as AgentDataSection
participant WDS as WorkflowDataSection
participant NDS as NetworkDataSection
participant AT as AgentTool
UI->>Msg: render(message)
Msg->>Msg: compute dataParts via useMemo
loop for each part in dataParts
Msg->>TG: isAgentDataPart(part)
alt part is AgentDataPart
Msg->>ADS: render part
ADS-->>Msg: Agent execution collapsible
else not AgentDataPart
Msg->>TG: isWorkflowDataPart(part)
alt part is WorkflowDataPart
Msg->>WDS: render part
WDS-->>Msg: Workflow execution collapsible
else not WorkflowDataPart
Msg->>TG: isNetworkDataPart(part)
alt part is NetworkDataPart
Msg->>NDS: render part
NDS-->>Msg: Network execution collapsible
else other data-tool-* part
alt part.type startsWith data-tool-
Msg->>AT: render custom tool UI
AT-->>Msg: Tool-specific panel
else generic data-* part
Msg-->>Msg: render generic Collapsible with JSON
end
end
end
end
end
Msg-->>UI: message body with nested sections
Class diagram for updated nested data part rendering in chat messagesclassDiagram
direction LR
class UIMessage {
+string id
+parts MastraDataPart[]
}
class MastraDataPart {
+string type
+string id
+unknown data
}
class AgentDataPart {
+string type
+string id
+AgentExecutionData data
}
class WorkflowDataPart {
+string type
+string id
+WorkflowExecutionData data
}
class NetworkDataPart {
+string type
+string id
+NetworkExecutionData data
}
class AgentExecutionData {
+string text
+unknown usage
+toolResults unknown[]
}
class WorkflowExecutionData {
+string name
+string status
+WorkflowStepMap steps
+WorkflowOutput output
}
class NetworkExecutionData {
+string name
+string status
+NetworkStep[] steps
+NetworkUsage usage
+unknown output
}
class WorkflowStepMap {
<<map>>
+string key
+WorkflowStep value
}
class WorkflowStep {
+string status
+unknown input
+unknown output
+unknown suspendPayload
}
class NetworkStep {
+string name
+string status
+unknown input
+unknown output
}
class NetworkUsage {
+number inputTokens
+number outputTokens
+number totalTokens
}
class MessageItem {
+UIMessage message
-MastraDataPart[] dataParts
+render()
}
class AgentDataSection {
+AgentDataPart part
+render()
}
class WorkflowDataSection {
+WorkflowDataPart part
+render()
}
class NetworkDataSection {
+NetworkDataPart part
+render()
}
class AgentTool {
+string id
+string type
+unknown data
+render()
}
class TypeGuards {
+bool hasStringType(unknown part)
+bool isAgentDataPart(unknown part)
+bool isWorkflowDataPart(unknown part)
+bool isNetworkDataPart(unknown part)
}
class KeyHelpers {
+string getToolCallId(unknown tool, number fallbackIndex)
}
UIMessage "1" o-- "*" MastraDataPart
MastraDataPart <|-- AgentDataPart
MastraDataPart <|-- WorkflowDataPart
MastraDataPart <|-- NetworkDataPart
MessageItem ..> MastraDataPart : filters dataParts
MessageItem ..> AgentDataPart : uses when isAgentDataPart
MessageItem ..> WorkflowDataPart : uses when isWorkflowDataPart
MessageItem ..> NetworkDataPart : uses when isNetworkDataPart
MessageItem --> AgentDataSection : renders nested agent
MessageItem --> WorkflowDataSection : renders nested workflow
MessageItem --> NetworkDataSection : renders nested network
MessageItem --> AgentTool : renders other data-tool-* parts
MessageItem ..> TypeGuards
MessageItem ..> KeyHelpers
AgentDataSection --> AgentExecutionData
WorkflowDataSection --> WorkflowExecutionData
NetworkDataSection --> NetworkExecutionData
WorkflowExecutionData o-- WorkflowStepMap
WorkflowStepMap o-- WorkflowStep
NetworkExecutionData o-- NetworkStep
NetworkExecutionData o-- NetworkUsage
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR applies widespread refactoring across 50+ files, converting type aliases to interfaces throughout the codebase, updating import statements to use type-only imports where appropriate, and adding promise/error handling improvements. Key functional changes include expanded Mastra data part rendering in chat messages and ESLint configuration enhancements. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
NetworkRoutingPanelsome Tailwind class names look malformed (e.g."bento-grid grid-cols-2! gap-2!","bento-item p-2! ..."); the!modifier should precede the utility (e.g.!grid-cols-2,!p-2) or be removed, otherwise styles may not apply as intended. - The refactor of
formatStorageMessagesinsrc/mastra/config/pg-storage.tsremovedmaskSensitiveMessageDataand now logs fulldetailsviaJSON.stringify, which risks exposing secrets (passwords, tokens, etc.); consider reintroducing masking or otherwise filtering sensitive fields before building the message text. - Some of the new layout class changes may not correspond to valid Tailwind utilities (e.g.
"h-100"and"w-50"innetwork-chat.tsx), which could break the intended sizing; double-check these against your Tailwind config or revert to the priorh-[400px]/w-[200px]style.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `NetworkRoutingPanel` some Tailwind class names look malformed (e.g. `"bento-grid grid-cols-2! gap-2!"`, `"bento-item p-2! ..."`); the `!` modifier should precede the utility (e.g. `!grid-cols-2`, `!p-2`) or be removed, otherwise styles may not apply as intended.
- The refactor of `formatStorageMessages` in `src/mastra/config/pg-storage.ts` removed `maskSensitiveMessageData` and now logs full `details` via `JSON.stringify`, which risks exposing secrets (passwords, tokens, etc.); consider reintroducing masking or otherwise filtering sensitive fields before building the message text.
- Some of the new layout class changes may not correspond to valid Tailwind utilities (e.g. `"h-100"` and `"w-50"` in `network-chat.tsx`), which could break the intended sizing; double-check these against your Tailwind config or revert to the prior `h-[400px]`/`w-[200px]` style.
## Individual Comments
### Comment 1
<location> `app/networks/components/network-chat.tsx:298` </location>
<code_context>
return (
- <div className="my-4 h-[400px] w-full">
+ <div className="my-4 h-100 w-full">
{title && <div className="text-sm font-medium mb-2">{title}</div>}
<WebPreview defaultUrl={url}>
</code_context>
<issue_to_address>
**issue (bug_risk):** The `h-100` class is not a standard Tailwind utility and will likely not have any effect.
The previous `h-[400px]` was valid; `h-100` isn’t in Tailwind’s default scale and will be ignored unless explicitly added to your theme. If you need 100px use `h-[100px]`, or for 100% height use `h-full`.
</issue_to_address>
### Comment 2
<location> `app/networks/components/network-chat.tsx:1486` </location>
<code_context>
onValueChange={(value) => selectNetwork(value)}
>
- <PromptInputSelectTrigger className="w-[200px]">
+ <PromptInputSelectTrigger className="w-50">
<NetworkIcon className="mr-2 size-4" />
<PromptInputSelectValue />
</code_context>
<issue_to_address>
**issue (bug_risk):** The `w-50` class is not a valid Tailwind width utility and may break layout.
Unless you’ve added a custom `w-50` in your Tailwind config, this will not compile to a width class. For a fixed 200px width, use `w-[200px]`; for a 50% width, use `w-1/2`.
</issue_to_address>
### Comment 3
<location> `app/networks/components/network-routing-panel.tsx:196-200` </location>
<code_context>
Available Agents
</p>
- <div className="bento-grid !grid-cols-2 !gap-2">
+ <div className="bento-grid grid-cols-2! gap-2!">
{networkConfig.agents.map((agent) => (
<div
key={agent.id}
- className="bento-item !p-2 flex items-center gap-2"
+ className="bento-item p-2! flex items-center gap-2"
>
<BotIcon className="size-3 text-primary" />
</code_context>
<issue_to_address>
**issue (bug_risk):** The `grid-cols-2!`, `gap-2!`, and `p-2!` class names are not valid Tailwind utilities.
To apply Tailwind’s `!important` variant, the `!` must come before the utility: `!grid-cols-2`, `!gap-2`, `!p-2`. With `grid-cols-2!`, `gap-2!`, and `p-2!`, Tailwind won’t generate styles, so the layout/spacing overrides won’t take effect.
</issue_to_address>
### Comment 4
<location> `src/mastra/config/pg-storage.ts:28-37` </location>
<code_context>
// return maskedStream;
//}
-// Utility function to mask sensitive data in message content for logging
-export function maskSensitiveMessageData(
- content: string,
</code_context>
<issue_to_address>
**🚨 issue (security):** Removing masking of sensitive message data while still logging raw `details` may expose secrets in logs.
Previously, `maskSensitiveMessageData` scrubbed fields like `password`, `secret`, and `token`. The new behavior logs `JSON.stringify(details)` for error/non-success states with no redaction, so any secrets in `details` (e.g., credentials in error payloads or API responses) will be written to logs. Please either restore a masking step or guarantee `details` is sanitized before logging.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
🤖 Hi @ssdeanx, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
|
🤖 I'm sorry @ssdeanx, but I was unable to process your request. Please see the logs for more details. |
Summary of ChangesHello @ssdeanx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the overall code quality, stability, and performance of the application. It includes updates to type definitions, removal of unnecessary comments, better handling of asynchronous operations, and enhancements to data handling and UI components. These changes collectively contribute to a more robust and maintainable codebase. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (20)
app/components/charts/PieWidget.tsx (1)
5-12: Consider adding TSDoc comments for exported public API.The exported
PieSliceinterface andPieWidgetPropsinterface would benefit from TSDoc documentation explaining their purpose and field descriptions, per the coding guidelines for documenting public APIs.📝 Suggested TSDoc documentation
+/** + * Represents a single slice of data in the pie chart. + */ export interface PieSlice { name: string; value: number; fill?: string } +/** + * Props for the PieWidget component. + */ interface PieWidgetProps { + /** Array of pie slices to render */ data: PieSlice[] + /** Chart height in pixels */ height?: number + /** Inner radius for donut effect */ innerRadius?: number + /** Outer radius of the pie */ outerRadius?: number }hooks/use-utils.ts (1)
71-77: Unconventional formatting for block statements.The early return statements at lines 72 and 87 use compressed formatting
{return}without spaces. Per coding guidelines recommending Prettier for consistent formatting, these should follow standard conventions:Suggested formatting
useEffect(() => { - if (delay === null) {return} + if (delay === null) { + return + } const id = setInterval(() => savedCallback.current(), delay) return () => clearInterval(id) }, [delay])Apply the same pattern to
useTimeoutat line 87.Also applies to: 86-91
src/mastra/config/pg-storage.ts (4)
11-11: Remove unused importSpanType.
SpanTypeis imported but never used in this file. According to the AI summary, tracing integration was removed, making this import dead code.Suggested fix
-import { SpanType } from '@mastra/core/observability'
33-35: Hardcoded credentials in fallback connection string violate security guidelines.The fallback
'postgresql://user:password@localhost:5432/mydb'contains hardcoded credentials. Per coding guidelines, sensitive values must be stored in environment variables and never included in code. This pattern is repeated at lines 55-57.Consider throwing an error if the required environment variable is not set, rather than falling back to insecure defaults.
Suggested fix
export const pgStore = new PostgresStore({ id: 'pg:store', connectionString: - process.env.SUPABASE ?? - 'postgresql://user:password@localhost:5432/mydb', + process.env.SUPABASE ?? (() => { throw new Error('SUPABASE environment variable is required') })(),
227-227: Non-functionalAbortControllerpattern.Creating
new AbortController().signalinline discards the controller reference, making it impossible to abort the operation. Either expose the controller to callers or remove this parameter if abort capability isn't required.Suggested fix (if abort not needed)
- abortSignal: new AbortController().signal,
182-192: Inconsistent indentation in reranker configuration.The
vectorweight at line 187 has inconsistent indentation compared to sibling properties. Consider running Prettier to auto-format.Suggested fix
reranker: { - model: new ModelRouterLanguageModel('google/gemini-2.5-flash-lite-preview-09-2025'), - options: { - weights: { - semantic: 0.5, // Semantic relevance weight - vector: 0.3, // Vector similarity weight - position: 0.2, // Original position weight - }, - topK: 5, - }, - }, + model: new ModelRouterLanguageModel('google/gemini-2.5-flash-lite-preview-09-2025'), + options: { + weights: { + semantic: 0.5, // Semantic relevance weight + vector: 0.3, // Vector similarity weight + position: 0.2, // Original position weight + }, + topK: 5, + }, + },app/workflows/providers/workflow-context.tsx (1)
790-796: Missing.catch()handler onsendMessage— inconsistent withrunWorkflow.The
runWorkflowfunction (line 756) usesvoid sendMessage(...).catch(...)butresumeWorkflowomits the catch block. This inconsistency can lead to unhandled promise rejections and makes error handling unpredictable.Proposed fix
void sendMessage({ text: resumeData ? `resume ${JSON.stringify(resumeData)}` : 'resume', // eslint-disable-next-line `@typescript-eslint/no-explicit-any` metadata: (hasRunId ? { runId } : {}) as any, - }) + }).catch((err) => { + console.error('Failed to resume workflow', err) + setWorkflowStatus('error') + setCurrentRun((prev) => + prev ? { ...prev, status: 'error', error: String(err) } : null + ) + })src/mastra/tools/tests/csv-to-json.tool.test.ts (1)
95-141: Inconsistent indentation in test block.This test uses 8-space indentation while other tests in the file use 4-space indentation. Consider aligning the indentation for consistency.
🔧 Suggested fix
it('should emit progress events', async () => { - // Arrange - const csvData = 'name,value\nTest,100' - const mockWriter: any = { - custom: vi.fn(), - write: vi.fn(), - } - const mockSpan: any = { - update: vi.fn(), - end: vi.fn(), - error: vi.fn(), - } - const mockTracingContext: any = { - currentSpan: { - createChildSpan: vi.fn().mockReturnValue(mockSpan), - }, - } - - // Act - await csvToJsonTool.execute!( - { - csvData, - options: { - delimiter: ',', - columns: true, - trim: true, - skip_empty_lines: true, - }, - }, - { - writer: mockWriter, - tracingContext: mockTracingContext, - abortSignal: new AbortController().signal, - } - ) - - // Assert - expect(mockWriter.custom).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'data-tool-progress', - data: expect.objectContaining({ - status: 'in-progress', - }), - id: 'csv-to-json', - }) - ) - }) + // Arrange + const csvData = 'name,value\nTest,100' + const mockWriter: any = { + custom: vi.fn(), + write: vi.fn(), + } + const mockSpan: any = { + update: vi.fn(), + end: vi.fn(), + error: vi.fn(), + } + const mockTracingContext: any = { + currentSpan: { + createChildSpan: vi.fn().mockReturnValue(mockSpan), + }, + } + + // Act + await csvToJsonTool.execute!( + { + csvData, + options: { + delimiter: ',', + columns: true, + trim: true, + skip_empty_lines: true, + }, + }, + { + writer: mockWriter, + tracingContext: mockTracingContext, + abortSignal: new AbortController().signal, + } + ) + + // Assert + expect(mockWriter.custom).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'data-tool-progress', + data: expect.objectContaining({ + status: 'in-progress', + }), + id: 'csv-to-json', + }) + ) + })src/components/ai-elements/loader.tsx (1)
82-84: Consider converting to interface withextends.While
LoaderPropsis currently a type alias using intersection, per coding guidelines preferring interfaces, this could be refactored to use interface extension for consistency.♻️ Optional refactor to interface
-export type LoaderProps = HTMLAttributes<HTMLDivElement> & { - size?: number -} +export interface LoaderProps extends HTMLAttributes<HTMLDivElement> { + size?: number +}src/mastra/tools/tests/cytoscape.tool.test.ts (1)
14-35: Consider typing the mock context more explicitly.The
createMockContextfunction returnsany, which reduces type safety. Consider using a more specific return type or the actual context interface to catch type errors in tests.♻️ Optional type improvement
const createMockContext = ( overrides: Partial<CytoscapeContext> = {} - ): any => ({ + ) => ({ writer: { custom: vi.fn().mockResolvedValue(undefined), - } as any, + }, abortSignal: { aborted: false } as AbortSignal, requestContext: { userId: 'test-user', workspaceId: 'test-workspace', ...overrides, - } as CytoscapeContext, + }, tracingContext: { currentSpan: { createChildSpan: vi.fn().mockReturnValue({ update: vi.fn(), end: vi.fn(), error: vi.fn(), }), }, - } as any, + }, })This removes explicit
anycasts while allowing TypeScript to infer the mock structure. Theas anycasts oncytoscapeToolin test calls would still be needed due to tool type inference limitations.app/components/api-components.tsx (2)
32-36: Add error handling for clipboard operations.
navigator.clipboard.writeTextcan throw if clipboard access is denied or unavailable. Consider wrapping in try-catch to prevent unhandled promise rejections.♻️ Proposed fix
- const copyPath = () => { - navigator.clipboard.writeText(path) - setCopied(true) - setTimeout(() => setCopied(false), 2000) + const copyPath = async () => { + try { + await navigator.clipboard.writeText(path) + setCopied(true) + setTimeout(() => setCopied(false), 2000) + } catch { + // Clipboard access denied or unavailable + } }
116-120: Add error handling for clipboard operations.The async
copyCodefunction awaitsnavigator.clipboard.writeTextbut doesn't handle potential errors. If clipboard access is denied, this will cause an unhandled rejection.♻️ Proposed fix
const copyCode = async () => { - await navigator.clipboard.writeText(children) - setCopied(true) - setTimeout(() => setCopied(false), 2000) + try { + await navigator.clipboard.writeText(children) + setCopied(true) + setTimeout(() => setCopied(false), 2000) + } catch { + // Clipboard access denied or unavailable + } }app/api/contact/route.ts (1)
41-49: Avoid logging user email addresses (PII).The console log includes
email: data.email, which is personally identifiable information. Per coding guidelines, sensitive data like emails should not be logged. Consider masking or omitting the email from logs.Proposed fix
console.log('Contact form submission:', { name: `${data.firstName} ${data.lastName}`, - email: data.email, + email: data.email ? '***@***.***' : 'N/A', company: data.company || 'N/A', inquiryType: data.inquiryType || 'general', subject: data.subject, - message: data.message, + message: data.message.slice(0, 50) + (data.message.length > 50 ? '...' : ''), timestamp: new Date().toISOString(), })app/networks/components/network-routing-panel.tsx (1)
73-77: UnnecessaryBoolean()wrapper.The
Boolean(event.agentId)is redundant for a simple truthiness check on a string. The originalevent.agentId &&pattern is idiomatic and clearer.Suggested simplification
- {(Boolean(event.agentId)) && ( + {event.agentId && ( <Badge variant="outline" className="text-xs px-1 py-0"> {event.agentId} </Badge> )}app/chat/providers/chat-context-types.ts (1)
41-46: Consolidate duplicate interface definitions to avoid type inconsistencies.Two separate
QueuedTaskandWebPreviewDatainterface definitions exist with different field sets. Inapp/chat/providers/chat-context-types.ts,QueuedTasklackscreatedAt,completedAt, anderrorfields, whileWebPreviewDatais missinghtml,editable,showConsole, andheightfields—both present inapp/chat/components/chat.types.ts. Duplicate type definitions violate the DRY principle and create maintenance burden. Define these interfaces in a single shared location and import them across both files, or explicitly document and justify why the context versions intentionally differ.src/components/ai-elements/code-block.tsx (1)
87-99: Add error handling for thehighlightCodepromise.The
.then()call lacks a.catch()handler. IfcodeToHtmlfails (e.g., invalid language), the error is silently swallowed, which violates the guideline to avoid silent failures.Proposed fix
useEffect(() => { - highlightCode(code, language, showLineNumbers).then(([light, dark]) => { - if (!mounted.current) { - setHtml(light) - setDarkHtml(dark) - mounted.current = true - } - }) + highlightCode(code, language, showLineNumbers) + .then(([light, dark]) => { + if (!mounted.current) { + setHtml(light) + setDarkHtml(dark) + mounted.current = true + } + }) + .catch((error) => { + console.error('Failed to highlight code:', error) + }) return () => { mounted.current = false } }, [code, language, showLineNumbers])app/chat/components/nested-agent-chat.tsx (1)
45-59: Consider using stable keys for message parts.Using array index as a React key can cause rendering issues if parts are reordered or modified. For
data-tool-agentparts, theidfromAgentDataPartcould be used as a stable key.💡 Optional: Use stable key for data-tool-agent
{message.parts.map((part, index) => { if (part.type === 'text') { - return <p key={index}>{part.text}</p> + return <p key={`text-${index}`}>{part.text}</p> } if (part.type === 'data-tool-agent') { const { id, data } = part as AgentDataPart return ( - <div key={index} className="nested-agent"> + <div key={id} className="nested-agent"> <strong>Nested Agent: {id}</strong> {data.text && <p>{data.text}</p>} </div> ) } return null })}app/chat/components/agent-confirmation.tsx (1)
154-171: Consider more explicit null/undefined check for approval ID.The pattern
approval?.id && onReject(approval.id)will skip invoking the callback ifapproval.idis any falsy value, including an empty string. While the type system definesid: stringas required, using an explicit null/undefined check would be more precise:Suggested change
<ConfirmationAction variant="outline" - onClick={() => approval?.id && onReject(approval.id)} + onClick={() => approval?.id != null && onReject(approval.id)} >This ensures the callback only skips for
nullorundefined, not other falsy values like empty strings.src/components/ai-elements/tool.tsx (1)
32-37: Type mismatch:ToolHeaderPropsdoes not include CollapsibleTrigger props for rest spread.The
ToolHeadercomponent spreads...propsontoCollapsibleTrigger(line 86), butToolHeaderPropsonly declarestitle,type,state, andclassName. This means any additional props passed toToolHeaderwon't be type-checked, and TypeScript may warn or allow invalid props to slip through.Consider extending the appropriate base props:
♻️ Suggested fix
-export interface ToolHeaderProps { +export interface ToolHeaderProps extends Omit<ComponentProps<typeof CollapsibleTrigger>, 'children'> { title?: string type: ToolUIPart['type'] state: ToolUIPart['state'] - className?: string }Also applies to: 74-86
src/components/ai-elements/confirmation.tsx (1)
14-36: Redundant union member inToolUIPartApproval.The union type contains overlapping members. The member at lines 25-29 (
{ id: string; approved: true; reason?: string }) is already covered by the member at lines 21-24 ({ id: string; approved: boolean; reason?: string }) sincebooleanincludestrue. This creates redundancy.♻️ Suggested simplification
type ToolUIPartApproval = | { id: string approved?: never reason?: never } | { id: string approved: boolean reason?: string } - | { - id: string - approved: true - reason?: string - } - | { id: string approved: false reason?: string } | undefinedNote: If the intent is to create a discriminated union for specific handling of
truevsfalsevsboolean, keep the structure but add a clarifying comment.
🤖 Fix all issues with AI agents
In `@app/chat/components/agent-artifact.tsx`:
- Around line 62-73: The clipboard error is being swallowed in handleCopy's
doCopy catch block; change the catch to log the error (e.g., console.error) and
optionally surface user feedback (e.g., set an error state or trigger a toast)
so failures are visible; update the catch in the doCopy async function inside
handleCopy to log err and handle user-visible error behavior while keeping the
fire-and-forget pattern and existing setCopied logic.
In `@app/chat/components/chat-messages.tsx`:
- Around line 167-174: The inline mapping over agentData.toolResults is doing
complex type-narrowing; extract a small helper (e.g., getToolName(tr: unknown):
string) that checks typeof tr === 'object' && tr !== null and then safely reads
tr.toolName, returning the string if valid or 'unknown' otherwise, and replace
the inline map with agentData.toolResults.map(getToolName) to improve
readability and maintainability.
- Around line 1047-1055: The code currently treats any part whose partType
startsWith('data-tool-') as an AgentDataPart and spreads it into the AgentTool
component, which will break if the object lacks expected fields; update the
branch around the partType check to validate the shape before rendering: check
that the part has the AgentDataPart properties (e.g., id, type, data) and only
then render <AgentTool ...>; if validation fails, render the existing
JSON-collapsible/fallback UI instead; use the partType variable and
AgentTool/AgentDataPart identifiers to locate and modify the logic.
In `@app/chat/components/nested-agent-chat.tsx`:
- Around line 14-28: The handler currently only logs sendMessage failures to
console; update handleSubmit to surface errors to users by adding a
component-level error state (e.g., error / setError) and set it inside the catch
block when sendMessage fails, keep clearing input in finally via setInput(''),
and ensure the component renders the error state (a dismissible inline message
or toast) so users see the failure instead of only console output; reference
handleSubmit, sendMessage, setInput and the component render to add the UI for
error display.
In `@app/dashboard/_components/data-table.tsx`:
- Around line 86-87: The comparisons using loose equality (aVal == null and bVal
== null) violate the eqeqeq rule; update the null checks in the sorter (where
aVal and bVal are compared) to use strict equality by explicitly checking both
null and undefined (e.g. test aVal === null || aVal === undefined and likewise
for bVal) or replace with a small helper like isNil(aVal) that uses strict
checks, then return 1/-1 as before.
In `@app/networks/components/network-chat.tsx`:
- Around line 504-522: MastraData and MastraPart are duplicated in this file;
extract a single shared definition at module scope and remove the second copy to
avoid drift. Create one top-level interface block for MastraData and MastraPart
(export them if other modules import them), update any local references in
functions/components to use these single definitions (e.g., usages around
network-chat.tsx regions where MastraData and MastraPart are referenced), and
delete the duplicate definitions at lines ~813-831 so only the module-level
interfaces remain.
- Around line 968-970: The clipboard write in network-chat.tsx currently calls
void navigator.clipboard.writeText(textPart.text) and silently swallows errors;
update this call to mirror the ArtifactDisplay handler's behavior by adding
error handling around navigator.clipboard.writeText (either await in an async
handler with try/catch or add .catch) and in the catch call the same error
reporting used by ArtifactDisplay (e.g., log the error and/or surface a user
toast) so failures are not silently ignored.
In `@app/workflows/components/workflow-actions.tsx`:
- Around line 48-58: The try/catch in handleFitViewAsync currently swallows all
errors; update it to log the caught error for debugging (e.g., console.debug or
console.warn) while keeping the fire-and-forget usage in handleFitView
unchanged. Specifically, in handleFitViewAsync (which calls
reactFlowInstance?.fitView), catch the error as e and log a brief contextual
message including e so failures to fitView are visible during debugging without
changing the async invocation pattern in handleFitView.
In `@eslint.config.js`:
- Around line 2-9: You are importing both the direct plugin alias tseslint (from
'@typescript-eslint/eslint-plugin') and the meta-package typescriptEslint (from
'typescript-eslint'), which can cause duplicate plugin registration; remove the
direct plugin import (tseslint) and consolidate to use the meta-package export
(typescriptEslint) for both plugin and parser references: update the plugins
block to use typescriptEslint.plugin (e.g., '@typescript-eslint') and set
languageOptions.parser to typescriptEslint.parser, removing any other references
to tseslint to avoid redundancy.
In `@hooks/use-utils.ts`:
- Around line 56-58: The public function useToggle currently relies on type
inference for its parameter; update the signature of useToggle to explicitly
annotate the parameter as a boolean (i.e., change initialValue = false to
initialValue: boolean = false) so the public API has an explicit parameter type
while keeping the existing return type and implementation unchanged.
In `@src/mastra/config/pg-storage.ts`:
- Around line 295-314: The function formatStorageMessages now serializes details
with JSON.stringify() and thus can leak sensitive fields; before pushing to
messageData.parts you must sanitize details using the existing
maskSensitiveMessageData() helper (or implement equivalent masking) so that
credentials/PII are removed or redacted; locate where formatStorageMessages
constructs messageData.parts and replace JSON.stringify(details) with
JSON.stringify(maskSensitiveMessageData(details)) (or call your new masker) for
all branches (status === 'success', 'error', and default) to ensure consistent
masking.
In `@src/mastra/lib/http-client.ts`:
- Around line 16-17: The retryDelay signature currently uses the explicit any
type for its second parameter; change it to a safer type (prefer unknown) or a
specific error type if known (e.g., AxiosError) by updating the retryDelay?:
(retryCount: number, error: unknown) => number (or the specific error type) so
callers must narrow the error before use and the file no longer uses any; update
any affected call sites to perform appropriate type checks or casts.
| const handleCopy = useCallback(() => { | ||
| const doCopy = async () => { | ||
| try { | ||
| await navigator.clipboard.writeText(editedCode) | ||
| setCopied(true) | ||
| setTimeout(() => setCopied(false), 2000) | ||
| } catch (err) { | ||
| void err | ||
| } | ||
| } | ||
| void doCopy() | ||
| }, [editedCode]) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fire-and-forget pattern is correct, but error is silently swallowed.
The async IIFE pattern correctly handles the clipboard operation without returning a Promise. However, void err silently discards any clipboard errors without logging or user feedback, making debugging difficult if copy operations fail.
♻️ Suggested: Log clipboard errors for debugging
const handleCopy = useCallback(() => {
const doCopy = async () => {
try {
await navigator.clipboard.writeText(editedCode)
setCopied(true)
setTimeout(() => setCopied(false), 2000)
} catch (err) {
- void err
+ console.error('Failed to copy to clipboard:', err)
}
}
void doCopy()
}, [editedCode])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleCopy = useCallback(() => { | |
| const doCopy = async () => { | |
| try { | |
| await navigator.clipboard.writeText(editedCode) | |
| setCopied(true) | |
| setTimeout(() => setCopied(false), 2000) | |
| } catch (err) { | |
| void err | |
| } | |
| } | |
| void doCopy() | |
| }, [editedCode]) | |
| const handleCopy = useCallback(() => { | |
| const doCopy = async () => { | |
| try { | |
| await navigator.clipboard.writeText(editedCode) | |
| setCopied(true) | |
| setTimeout(() => setCopied(false), 2000) | |
| } catch (err) { | |
| console.error('Failed to copy to clipboard:', err) | |
| } | |
| } | |
| void doCopy() | |
| }, [editedCode]) |
🤖 Prompt for AI Agents
In `@app/chat/components/agent-artifact.tsx` around lines 62 - 73, The clipboard
error is being swallowed in handleCopy's doCopy catch block; change the catch to
log the error (e.g., console.error) and optionally surface user feedback (e.g.,
set an error state or trigger a toast) so failures are visible; update the catch
in the doCopy async function inside handleCopy to log err and handle
user-visible error behavior while keeping the fire-and-forget pattern and
existing setCopied logic.
| ...(Array.isArray(agentData.toolResults) && | ||
| agentData.toolResults.length > 0 && { | ||
| toolsUsed: agentData.toolResults.map((tr: unknown) => | ||
| typeof tr === 'object' && tr !== null && typeof (tr as Record<string, unknown>).toolName === 'string' | ||
| ? (tr as Record<string, unknown>).toolName | ||
| : 'unknown' | ||
| ), | ||
| }), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Complex inline type narrowing in toolResults mapping.
The nested type assertions in the map callback are difficult to read and maintain. Consider extracting a helper function for cleaner type narrowing.
♻️ Suggested improvement
// Helper to safely extract tool name
function getToolName(tr: unknown): string {
if (typeof tr === 'object' && tr !== null && 'toolName' in tr) {
const name = (tr as { toolName: unknown }).toolName
return typeof name === 'string' ? name : 'unknown'
}
return 'unknown'
}
// Then in the component:
toolsUsed: agentData.toolResults.map(getToolName),🤖 Prompt for AI Agents
In `@app/chat/components/chat-messages.tsx` around lines 167 - 174, The inline
mapping over agentData.toolResults is doing complex type-narrowing; extract a
small helper (e.g., getToolName(tr: unknown): string) that checks typeof tr ===
'object' && tr !== null and then safely reads tr.toolName, returning the string
if valid or 'unknown' otherwise, and replace the inline map with
agentData.toolResults.map(getToolName) to improve readability and
maintainability.
| // Render tool-specific data parts using the AgentTool UI when available so custom tool events are displayed nicely. | ||
| if (typeof partType === 'string' && partType.startsWith('data-tool-')) { | ||
| return ( | ||
| <AgentTool | ||
| key={`${message.id}-${partType}-${index}`} | ||
| id={nestedPart.id ?? partType} | ||
| type={partType as any} | ||
| data={nestedPart.data} | ||
| key={key} | ||
| {...(part as AgentDataPart)} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find AgentTool component definition
rg -t ts -t tsx "export.*AgentTool\s*=" --max-count=5 -A 10Repository: ssdeanx/AgentStack
Length of output: 89
🏁 Script executed:
# Find AgentDataPart type definition
rg -t ts -t tsx "type\s+AgentDataPart|interface\s+AgentDataPart" --max-count=5 -A 5Repository: ssdeanx/AgentStack
Length of output: 89
🏁 Script executed:
# Check the context around line 1047-1055 in chat-messages.tsx
sed -n '1040,1060p' app/chat/components/chat-messages.tsxRepository: ssdeanx/AgentStack
Length of output: 1310
🏁 Script executed:
# Find where partType and part are defined before line 1047
sed -n '1000,1050p' app/chat/components/chat-messages.tsx | head -60Repository: ssdeanx/AgentStack
Length of output: 2815
🏁 Script executed:
# Search for type guard definitions and AgentDataPart type
rg "isAgentDataPart|isWorkflowDataPart|isNetworkDataPart|type AgentDataPart" --max-count=20Repository: ssdeanx/AgentStack
Length of output: 694
🏁 Script executed:
# Find AgentTool component
fd "AgentTool" --type f -e ts -e tsx | head -20Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Look for where AgentTool is imported or defined in chat-messages.tsx
head -100 app/chat/components/chat-messages.tsx | grep -E "import|AgentTool"Repository: ssdeanx/AgentStack
Length of output: 1479
🏁 Script executed:
# Search for AgentDataPart definition and check its structure
rg "AgentDataPart" --max-count=20 -A 3Repository: ssdeanx/AgentStack
Length of output: 7256
🏁 Script executed:
# Find the exact line where AgentTool is being used with the type assertion
sed -n '1045,1065p' app/chat/components/chat-messages.tsxRepository: ssdeanx/AgentStack
Length of output: 1495
🏁 Script executed:
# Check what other data-tool-* variants might exist
rg "data-tool-" --max-count=30 | grep -E "data-tool-[a-z-]+" -o | sort | uniqRepository: ssdeanx/AgentStack
Length of output: 133
🏁 Script executed:
# Verify the AgentDataPart structure from mastra/ai-sdk type definition
rg "export.*type.*AgentDataPart|export.*interface.*AgentDataPart" -A 5Repository: ssdeanx/AgentStack
Length of output: 400
🏁 Script executed:
# Get the full type guard definitions to understand the pattern
sed -n '900,1000p' app/chat/components/chat-messages.tsx | grep -A 10 "function is"Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Check if there are any type definitions for WorkflowDataPart and NetworkDataPart
sed -n '1,150p' app/chat/components/chat-messages.tsx | grep -E "AgentDataPart|WorkflowDataPart|NetworkDataPart"Repository: ssdeanx/AgentStack
Length of output: 618
🏁 Script executed:
# Check if AgentDataPart properties are optional
rg -A 15 "export.*type AgentDataPart"Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Verify the union type definition for dataParts in chat-messages.tsx
sed -n '90,130p' app/chat/components/chat-messages.tsx | grep -A 10 "type.*DataPart"Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Check the DataPartUnion type definition more carefully
sed -n '50,150p' app/chat/components/chat-messages.tsxRepository: ssdeanx/AgentStack
Length of output: 3212
🏁 Script executed:
# Look at the entire dataParts.map() block to understand the flow
sed -n '1000,1070p' app/chat/components/chat-messages.tsxRepository: ssdeanx/AgentStack
Length of output: 4216
🏁 Script executed:
# Check if AgentDataPart is fully defined or if it has optional properties in mastra/ai-sdk
rg "export" app/chat/components/chat-messages.tsx | grep -E "AgentDataPart|WorkflowDataPart|NetworkDataPart" | head -10Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Look for any other usages of data-tool-* to understand the intent
rg "data-tool-" app/chat/components/chat-messages.tsx -B 2 -A 2Repository: ssdeanx/AgentStack
Length of output: 1914
Do not spread unknown data-tool-* parts as AgentDataPart without validation.
At line 1052, the catch-all condition partType.startsWith('data-tool-') captures any unhandled tool-specific parts (e.g., custom data-tool-* variants). These may not have the AgentDataPart structure (with id, type, data properties), but the code spreads them directly into AgentTool, which destructures these expected properties. This will cause runtime errors if a custom tool part is missing required fields. Either validate the structure before spreading or use the fallback JSON collapsible for unknown tool types.
🤖 Prompt for AI Agents
In `@app/chat/components/chat-messages.tsx` around lines 1047 - 1055, The code
currently treats any part whose partType startsWith('data-tool-') as an
AgentDataPart and spreads it into the AgentTool component, which will break if
the object lacks expected fields; update the branch around the partType check to
validate the shape before rendering: check that the part has the AgentDataPart
properties (e.g., id, type, data) and only then render <AgentTool ...>; if
validation fails, render the existing JSON-collapsible/fallback UI instead; use
the partType variable and AgentTool/AgentDataPart identifiers to locate and
modify the logic.
| const handleSubmit = (e: FormEvent<HTMLFormElement>) => { | ||
| e.preventDefault() | ||
| // Use a self-invoking async function to avoid returning a Promise from the event handler | ||
| void (async () => { | ||
| try { | ||
| await sendMessage({ text: input }) | ||
| } catch (err) { | ||
| // Log the error and allow the UI to continue | ||
| // eslint-disable-next-line no-console | ||
| console.error('Failed to send message', err) | ||
| } finally { | ||
| setInput('') | ||
| } | ||
| })() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good async handling pattern.
The self-invoking async function correctly prevents returning a Promise from the event handler. The finally block ensures input clearing regardless of success/failure.
Consider providing user-facing error feedback instead of only logging to console, as users won't see the error otherwise.
💡 Optional: Add user-facing error state
export function NestedAgentChat() {
const [input, setInput] = useState('')
+ const [error, setError] = useState<string | null>(null)
const { messages, sendMessage, status } = useChat({
...
})
const handleSubmit = (e: FormEvent<HTMLFormElement>) => {
e.preventDefault()
+ setError(null)
void (async () => {
try {
await sendMessage({ text: input })
} catch (err) {
- // eslint-disable-next-line no-console
- console.error('Failed to send message', err)
+ const message = err instanceof Error ? err.message : 'Failed to send message'
+ setError(message)
} finally {
setInput('')
}
})()
}🤖 Prompt for AI Agents
In `@app/chat/components/nested-agent-chat.tsx` around lines 14 - 28, The handler
currently only logs sendMessage failures to console; update handleSubmit to
surface errors to users by adding a component-level error state (e.g., error /
setError) and set it inside the catch block when sendMessage fails, keep
clearing input in finally via setInput(''), and ensure the component renders the
error state (a dismissible inline message or toast) so users see the failure
instead of only console output; reference handleSubmit, sendMessage, setInput
and the component render to add the UI for error display.
| if (aVal == null) {return 1} | ||
| if (bVal == null) {return -1} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using strict equality for null checks.
The coding guidelines specify using strict equality (===/!==) always. While == null is an idiomatic shorthand for checking both null and undefined, it technically violates the eqeqeq: ['error', 'always'] rule.
Proposed fix for strict equality
- if (aVal == null) {return 1}
- if (bVal == null) {return -1}
+ if (aVal === null || aVal === undefined) {return 1}
+ if (bVal === null || bVal === undefined) {return -1}🤖 Prompt for AI Agents
In `@app/dashboard/_components/data-table.tsx` around lines 86 - 87, The
comparisons using loose equality (aVal == null and bVal == null) violate the
eqeqeq rule; update the null checks in the sorter (where aVal and bVal are
compared) to use strict equality by explicitly checking both null and undefined
(e.g. test aVal === null || aVal === undefined and likewise for bVal) or replace
with a small helper like isNil(aVal) that uses strict checks, then return 1/-1
as before.
| import tseslint from '@typescript-eslint/eslint-plugin' | ||
| import tsparser from '@typescript-eslint/parser' | ||
| import prettierConfig from 'eslint-config-prettier' | ||
| import reactHooks from 'eslint-plugin-react-hooks' | ||
| import reactRefresh from 'eslint-plugin-react-refresh' | ||
| import { globalIgnores } from 'eslint/config' | ||
| import globals from 'globals' | ||
| import typescriptEslint from 'typescript-eslint' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential duplicate TypeScript ESLint plugin registration.
You're importing both @typescript-eslint/eslint-plugin (line 2, as tseslint) and typescript-eslint (line 9, as typescriptEslint). The typescript-eslint package is a meta-package that re-exports the plugin. Using both may cause conflicts or redundant plugin registration.
Consider using only the typescript-eslint meta-package for both configs and plugin access:
♻️ Suggested consolidation
import js from '@eslint/js'
-import tseslint from '@typescript-eslint/eslint-plugin'
-import tsparser from '@typescript-eslint/parser'
import prettierConfig from 'eslint-config-prettier'
import reactHooks from 'eslint-plugin-react-hooks'
import reactRefresh from 'eslint-plugin-react-refresh'
import { globalIgnores } from 'eslint/config'
import globals from 'globals'
import typescriptEslint from 'typescript-eslint'Then in the config block (around line 68), use the plugin from the meta-package:
plugins: {
'@typescript-eslint': typescriptEslint.plugin,
},
languageOptions: {
parser: typescriptEslint.parser,
// ...
},🤖 Prompt for AI Agents
In `@eslint.config.js` around lines 2 - 9, You are importing both the direct
plugin alias tseslint (from '@typescript-eslint/eslint-plugin') and the
meta-package typescriptEslint (from 'typescript-eslint'), which can cause
duplicate plugin registration; remove the direct plugin import (tseslint) and
consolidate to use the meta-package export (typescriptEslint) for both plugin
and parser references: update the plugins block to use typescriptEslint.plugin
(e.g., '@typescript-eslint') and set languageOptions.parser to
typescriptEslint.parser, removing any other references to tseslint to avoid
redundancy.
| export function useToggle( | ||
| initialValue: boolean = false | ||
| initialValue = false | ||
| ): [boolean, () => void, (value: boolean) => void] { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider restoring explicit type annotation for public API clarity.
Per coding guidelines requiring explicit typing in public functions, the parameter type should be explicit rather than inferred:
export function useToggle(
- initialValue = false
+ initialValue: boolean = false
): [boolean, () => void, (value: boolean) => void] {While TypeScript correctly infers boolean from the default value, explicit annotations improve readability and maintain consistency with the rest of the codebase's typing conventions.
🤖 Prompt for AI Agents
In `@hooks/use-utils.ts` around lines 56 - 58, The public function useToggle
currently relies on type inference for its parameter; update the signature of
useToggle to explicitly annotate the parameter as a boolean (i.e., change
initialValue = false to initialValue: boolean = false) so the public API has an
explicit parameter type while keeping the existing return type and
implementation unchanged.
| if (status === 'success') { | ||
| messageData.parts.push({ | ||
| type: 'text', | ||
| text: `Storage operation "${operation}" completed successfully at ${timestamp}.`, | ||
| }) | ||
| } else if (status === 'error') { | ||
| messageData.parts.push({ | ||
| type: 'text', | ||
| text: `Storage operation "${operation}" failed at ${timestamp}. Details: ${JSON.stringify( | ||
| details | ||
| )}`, | ||
| }) | ||
| } else { | ||
| messageData.parts.push({ | ||
| type: 'text', | ||
| text: `Storage operation "${operation}" status at ${timestamp}: ${JSON.stringify( | ||
| details | ||
| )}`, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Sensitive data masking removed from storage messages.
The coding guidelines specify: "Mask sensitive data in logs using maskSensitiveMessageData() helper from src/mastra/config/pg-storage.ts". However, the AI summary indicates this function was removed, and formatStorageMessages now directly serializes details via JSON.stringify() without any sanitization.
This could expose sensitive information (connection strings, credentials, PII) in error and info messages. Please either:
- Restore and use
maskSensitiveMessageData()to sanitizedetailsbefore serialization - Implement an alternative masking approach for sensitive fields
🤖 Prompt for AI Agents
In `@src/mastra/config/pg-storage.ts` around lines 295 - 314, The function
formatStorageMessages now serializes details with JSON.stringify() and thus can
leak sensitive fields; before pushing to messageData.parts you must sanitize
details using the existing maskSensitiveMessageData() helper (or implement
equivalent masking) so that credentials/PII are removed or redacted; locate
where formatStorageMessages constructs messageData.parts and replace
JSON.stringify(details) with JSON.stringify(maskSensitiveMessageData(details))
(or call your new masker) for all branches (status === 'success', 'error', and
default) to ensure consistent masking.
|
|
||
| retryDelay?: (retryCount: number, error: any) => number |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Approve ESLint cleanup, but address the any type.
Good removal of the ESLint disable comment. However, the error parameter on line 17 uses the any type, which violates the coding guideline to avoid explicit any types.
♻️ Replace `any` with `unknown` or specific error type
- retryDelay?: (retryCount: number, error: any) => number
+ retryDelay?: (retryCount: number, error: unknown) => numberAlternatively, use a more specific type if the error type from axios-retry is known:
- retryDelay?: (retryCount: number, error: any) => number
+ retryDelay?: (retryCount: number, error: Error) => numberAs per coding guidelines, prefer unknown or specific types over any.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| retryDelay?: (retryCount: number, error: any) => number | |
| retryDelay?: (retryCount: number, error: unknown) => number |
🤖 Prompt for AI Agents
In `@src/mastra/lib/http-client.ts` around lines 16 - 17, The retryDelay signature
currently uses the explicit any type for its second parameter; change it to a
safer type (prefer unknown) or a specific error type if known (e.g., AxiosError)
by updating the retryDelay?: (retryCount: number, error: unknown) => number (or
the specific error type) so callers must narrow the error before use and the
file no longer uses any; update any affected call sites to perform appropriate
type checks or casts.
| // eslint-disable-next-line object-shorthand | ||
| tracingContext: tracingContext, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the file around lines 128-129
head -140 src/mastra/tools/weather-tool.ts | tail -20Repository: ssdeanx/AgentStack
Length of output: 876
🏁 Script executed:
#!/bin/bash
# Search for other instances of tracingContext in the codebase to see how they're handled
rg -n "tracingContext" src/mastra/tools/ --type=ts -B2 -A2 | head -100Repository: ssdeanx/AgentStack
Length of output: 6023
🏁 Script executed:
#!/bin/bash
# Check ESLint configuration
find . -name ".eslintrc*" -o -name "eslint.config.*" | head -5Repository: ssdeanx/AgentStack
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Check the ESLint configuration for object-shorthand rule
cat ./eslint.config.js | grep -A5 -B5 "object-shorthand"Repository: ssdeanx/AgentStack
Length of output: 393
🏁 Script executed:
#!/bin/bash
# Let's also check if there are other instances of disabled object-shorthand in the codebase
rg "eslint-disable.*object-shorthand" --type=ts -B1 -A1Repository: ssdeanx/AgentStack
Length of output: 314
🏁 Script executed:
#!/bin/bash
# Let's see the actual weather-tool.ts file to understand the full context
cat -n src/mastra/tools/weather-tool.ts | sed -n '110,145p'Repository: ssdeanx/AgentStack
Length of output: 1683
Remove the eslint-disable comment and use object shorthand syntax.
The tracingContext: tracingContext should be shortened to { tracingContext }. This is the only eslint-disable for object-shorthand in the codebase, and it conflicts with the strict enforcement of the rule ('object-shorthand': 'error' in eslint.config.js).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly enhances type safety, error handling, and linting across various components, aligning with the summary's goals. Key improvements include stricter ESLint configurations, explicit handling of asynchronous operations in event handlers to prevent unhandled promise rejections, and better type guarding for complex data structures. The introduction of dedicated UI sections for nested agent, workflow, and network execution data greatly improves the observability of these processes. Overall, these changes contribute to a more robust and maintainable codebase.
| /** | ||
| * Type guard to check for type property | ||
| */ | ||
| function hasStringType(part: unknown): part is { type: string } { | ||
| return ( | ||
| typeof part === 'object' && | ||
| part !== null && | ||
| 'type' in part && | ||
| typeof (part as { type: unknown }).type === 'string' | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Type guard to check if a part is an AgentDataPart | ||
| */ | ||
| function isAgentDataPart(part: unknown): part is AgentDataPart { | ||
| return hasStringType(part) && part.type === 'data-tool-agent' | ||
| } | ||
|
|
||
| /** | ||
| * Type guard to check if a part is a WorkflowDataPart | ||
| */ | ||
| function isWorkflowDataPart(part: unknown): part is WorkflowDataPart { | ||
| return ( | ||
| hasStringType(part) && | ||
| (part.type === 'data-workflow' || part.type === 'data-tool-workflow') | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Type guard to check if a part is a NetworkDataPart | ||
| */ | ||
| function isNetworkDataPart(part: unknown): part is NetworkDataPart { | ||
| return ( | ||
| hasStringType(part) && | ||
| (part.type === 'data-network' || part.type === 'data-tool-network') | ||
| ) | ||
| } |
There was a problem hiding this comment.
The addition of type guards (hasStringType, isAgentDataPart, isWorkflowDataPart, isNetworkDataPart) significantly improves type safety when dealing with polymorphic MastraDataParts. This reduces reliance on any and makes the code more robust and easier to reason about.
| /** | ||
| * Safely resolve a tool call id for use in React keys without using `any`. | ||
| * Falls back to the provided index if no string id is available. | ||
| */ | ||
| function getToolCallId(tool: unknown, fallbackIndex: number): string { | ||
| // Explicitly check for null and non-object values to satisfy strict boolean checks | ||
| if (tool === null || typeof tool !== 'object') { | ||
| return `idx-${fallbackIndex}` | ||
| } | ||
|
|
||
| const maybeTool = tool as { toolCallId?: unknown; id?: unknown } | ||
|
|
||
| if (typeof maybeTool.toolCallId === 'string' && maybeTool.toolCallId.trim().length > 0) { | ||
| return maybeTool.toolCallId | ||
| } | ||
|
|
||
| if (typeof maybeTool.id === 'string' && maybeTool.id.trim().length > 0) { | ||
| return maybeTool.id | ||
| } | ||
|
|
||
| return `idx-${fallbackIndex}` | ||
| } |
There was a problem hiding this comment.
The getToolCallId utility function is a great addition for safely resolving React keys. It provides a robust way to generate unique and stable keys for tool invocations, falling back to an index if no string ID is available, which is crucial for list rendering performance and avoiding any casts.
| const normalizedRaw = (() => { | ||
| if (raw === null || raw === undefined) { | ||
| return '' | ||
| } | ||
| if (typeof raw === 'string') { | ||
| return raw | ||
| } | ||
| if (typeof raw === 'object') { | ||
| try { | ||
| return JSON.stringify(raw) | ||
| } catch { | ||
| return '[object]' | ||
| } | ||
| } | ||
| if (typeof raw === 'number' || typeof raw === 'boolean' || typeof raw === 'bigint') { | ||
| return String(raw) | ||
| } | ||
| if (typeof raw === 'symbol') { | ||
| return raw.toString() | ||
| } | ||
| // Narrow to a callable function type so we can safely access .name and .toString() | ||
| const isFunction = (v: unknown): v is (...args: unknown[]) => unknown => | ||
| typeof v === 'function' | ||
|
|
||
| if (isFunction(raw)) { | ||
| try { | ||
| return (raw as { name?: string }).name ?? raw.toString() | ||
| } catch { | ||
| return '[function]' | ||
| } | ||
| } | ||
| return '' | ||
| })() |
There was a problem hiding this comment.
The mapRawStateToStatus function has been significantly improved by adding robust type checks for the raw input. Handling null, undefined, string, object, number, boolean, bigint, and symbol explicitly, along with a type guard for functions, makes this utility much more resilient and type-safe. This is a critical improvement for data validation and preventing unexpected runtime behavior.
const normalizedRaw = (() => {
if (raw === null || raw === undefined) {
return ''
}
if (typeof raw === 'string') {
return raw
}
if (typeof raw === 'object') {
try {
return JSON.stringify(raw)
} catch {
return '[object]'
}
}
if (typeof raw === 'number' || typeof raw === 'boolean' || typeof raw === 'bigint') {
return String(raw)
}
// Narrow to a callable function type so we can safely access .name and .toString()
const isFunction = (v: unknown): v is (...args: unknown[]) => unknown =>
typeof v === 'function'
if (isFunction(raw)) {
try {
return (raw as { name?: string }).name ?? raw.toString()
} catch {
return '[function]'
}
}
return ''
})()
| const tsTypeCheckedRecommended = typescriptEslint.plugin.configs['flat/recommended-type-checked'].map((cfg) => | ||
| cfg.files ? cfg : { ...cfg, files: ['**/*.ts', '**/*.tsx', '**/*.mts', '**/*.cts'] } | ||
| ) |
There was a problem hiding this comment.
The addition of tsTypeCheckedRecommended from typescript-eslint is a significant improvement. This enables stricter type-checking rules across the codebase, which helps catch type-related errors early and enforces better type discipline. This is a high-impact change for code quality and reliability.
| '@typescript-eslint/strict-boolean-expressions': 'warn', | ||
| '@typescript-eslint/switch-exhaustiveness-check': 'error', |
There was a problem hiding this comment.
Enabling strict-boolean-expressions is a valuable rule for enforcing explicit boolean checks. It helps prevent common pitfalls where non-boolean values are implicitly coerced to booleans, leading to more predictable and safer code. This is a strong improvement for correctness.
| '@typescript-eslint/strict-boolean-expressions': 'warn', | |
| '@typescript-eslint/switch-exhaustiveness-check': 'error', | |
| '@typescript-eslint/prefer-readonly': 'off', | |
| '@typescript-eslint/strict-boolean-expressions': 'warn', |
| if (typeof partType === 'string' && partType.startsWith('data-tool-')) { | ||
| return ( | ||
| <AgentTool | ||
| key={`${message.id}-${partType}-${index}`} | ||
| id={nestedPart.id ?? partType} | ||
| type={partType as any} | ||
| data={nestedPart.data} | ||
| key={key} | ||
| {...(part as AgentDataPart)} | ||
| /> |
There was a problem hiding this comment.
Casting part as AgentDataPart directly in the AgentTool props is less ideal than using a more specific type if available, or ensuring the AgentTool component can handle a broader MastraDataPart type. While AgentDataPart might be compatible, it's worth verifying if AgentTool expects a more generic MastraDataPart or if AgentDataPart is indeed the most accurate type here.
<AgentTool
key={key}
{...(part as AgentDataPart)}
/>
| Available Agents | ||
| </p> | ||
| <div className="bento-grid !grid-cols-2 !gap-2"> | ||
| <div className="bento-grid grid-cols-2! gap-2!"> |
There was a problem hiding this comment.
Removing the ! from the Tailwind CSS classes grid-cols-2! and gap-2! is a good cleanup. The ! modifier in Tailwind is typically used for !important, and its removal suggests that the styling is now correctly applied without needing to override other styles, leading to cleaner and more predictable CSS.
| <div className="bento-grid grid-cols-2! gap-2!"> | |
| <div className="bento-grid grid-cols-2 gap-2"> |
| <div | ||
| key={agent.id} | ||
| className="bento-item !p-2 flex items-center gap-2" | ||
| className="bento-item p-2! flex items-center gap-2" |
There was a problem hiding this comment.
Removing the ! from the Tailwind CSS class p-2! is a good cleanup. The ! modifier in Tailwind is typically used for !important, and its removal suggests that the styling is now correctly applied without needing to override other styles, leading to cleaner and more predictable CSS.
| className="bento-item p-2! flex items-center gap-2" | |
| className="bento-item p-2 flex items-center gap-2" |
| // Ensure we don't leave a floating promise. Handle any rejection explicitly. | ||
| void stop().catch(() => { | ||
| /* ignore errors from stop */ | ||
| }) |
There was a problem hiding this comment.
Explicitly voiding the stop() call and adding an empty .catch() block is a good practice when you intend to "fire and forget" a promise and ignore its potential rejection. This prevents unhandled promise rejections and clearly communicates the intent to suppress errors from the stop operation.
void stop().catch(() => {
/* ignore errors from stop */
})
| '@typescript-eslint/no-unnecessary-boolean-literal-compare': | ||
| 'error', |
There was a problem hiding this comment.
Enabling no-unnecessary-boolean-literal-compare is a good rule for code clarity and conciseness. It flags redundant comparisons like x === true when x is already a boolean, encouraging more idiomatic boolean expressions.
| '@typescript-eslint/no-unnecessary-boolean-literal-compare': | |
| 'error', | |
| '@typescript-eslint/prefer-regexp-exec': 'error', | |
| '@typescript-eslint/no-unnecessary-boolean-literal-compare': | |
| 'error', |
There was a problem hiding this comment.
Pull request overview
This pull request focuses on improving code quality and consistency by addressing ESLint violations, enhancing type safety, and refactoring asynchronous operations. The changes align the codebase with stricter TypeScript and ESLint configurations, including the addition of type-checked rules and consistent use of type imports.
Changes:
- Upgraded ESLint configuration to include TypeScript type-checked rules and stricter checking
- Removed unnecessary eslint-disable comments throughout the codebase and fixed underlying issues
- Converted type aliases to interfaces per the consistent-type-definitions rule
- Refactored floating promise patterns using the void operator for better async handling
- Enhanced type safety by replacing 'any' with proper type guards and interface definitions
- Cleaned up unused code including imports, template files, and dead functions
Reviewed changes
Copilot reviewed 55 out of 57 changed files in this pull request and generated 37 comments.
Show a summary per file
| File | Description |
|---|---|
| eslint.config.js | Enhanced ESLint configuration with TypeScript type-checked rules and Node.js globals |
| ui/*.tsx | Removed eslint-disable comments for unused callback parameters |
| src/mastra/tools/weather-tool.ts | Fixed toolCallId logging and removed object-shorthand violations |
| src/mastra/config/pg-storage.ts | Removed unused maskSensitiveMessageData function and simplified formatStorageMessages |
| src/mastra/config/*.ts | Cleaned up unused google SDK imports |
| app/chat/components/chat-messages.tsx | Added type guards, rendering components for nested data structures, and improved type safety |
| app/chat/components/*.tsx | Refactored async patterns and improved form submission handling |
| app/networks/components/*.tsx | Enhanced type safety with proper type assertions and fixed async operations |
| app/workflows/providers/workflow-context.tsx | Added proper error handling for async sendMessage and stop operations |
| src/components/ai-elements/*.tsx | Converted type aliases to interfaces for consistency |
| hooks/*.ts | Applied consistent curly brace patterns and removed default parameter types |
| convex/_generated/*.js | Removed eslint-disable comments from generated files |
| <div | ||
| key={agent.id} | ||
| className="bento-item !p-2 flex items-center gap-2" | ||
| className="bento-item p-2! flex items-center gap-2" |
There was a problem hiding this comment.
The "!" suffix is not a valid Tailwind CSS utility class modifier. Using "!p-2" will not work. Instead, use the proper Tailwind syntax "!p-2" (without spaces between classes), or simply "p-2" with inline styles if overriding is needed.
| @@ -1,4 +1,4 @@ | |||
| import { type GoogleGenerativeAIProviderOptions } from '@ai-sdk/google' | |||
| import type { GoogleGenerativeAIProviderOptions } from '@ai-sdk/google' | |||
There was a problem hiding this comment.
Inconsistent type-import usage. The import of 'GoogleGenerativeAIProviderOptions' is changed to use 'type' import, but this is inconsistent with other imports in the same file. According to the ESLint rule '@typescript-eslint/consistent-type-imports' configured as 'error', all type-only imports should consistently use the 'type' keyword. Verify that all other type-only imports in this file follow the same pattern.
| const handleCopy = useCallback(() => { | ||
| navigator.clipboard | ||
| .writeText(text) | ||
| .then(() => { | ||
| setCopied(true) | ||
| setTimeout(() => setCopied(false), 2000) | ||
| }) | ||
| .catch((err) => { | ||
| console.error('Failed to copy:', err) | ||
| }) |
There was a problem hiding this comment.
The wrapped inline function pattern adds unnecessary complexity. The handleCopy callback is being converted from async/await to a promise chain unnecessarily. The original async/await pattern was clearer. If the goal is to avoid returning a Promise from the callback, use 'void handleCopy()' at the call site instead of wrapping the entire function implementation.
| }, | ||
| languageOptions: { | ||
| parser: tsparser, | ||
| globals: { ...globals.browser, ...globals.node }, |
There was a problem hiding this comment.
Adding globals.node to the globals configuration is important for Node.js environment compatibility. This change ensures that Node.js global variables are recognized by ESLint, preventing false positives for undefined variables in Node.js code.
| approved: true | ||
| reason?: string | ||
| } | ||
|
|
There was a problem hiding this comment.
The removed duplicate type in the ToolUIPartApproval union should have removed lines 20-24 instead of lines 30-34. Lines 20-24 contain the exact duplicate that has the same structure as lines 27-29 (approved: true), while lines 31-35 represent the rejected case (approved: false) which is semantically distinct and necessary.
| // import { maskStreamTags } from '@mastra/core'; | ||
|
|
||
| import { ModelRouterEmbeddingModel, ModelRouterLanguageModel } from '@mastra/core/llm' | ||
| import { SpanType } from '@mastra/core/observability' |
There was a problem hiding this comment.
'SpanType' is defined but never used.
| retries?: number | ||
| // eslint-disable-next-line no-unused-vars | ||
|
|
||
| retryDelay?: (retryCount: number, error: any) => number |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
|
|
||
| // Ensure at least one progress call included the expected in-progress message and metadata | ||
| const hasInProgressCall = mockWriter.custom.mock.calls.some( | ||
| (c: any) => { |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
|
|
||
| it('handles empty array', async () => { | ||
| const data: Record<string, any>[] = [] | ||
| const data: Array<Record<string, any>> = [] |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
| mockContext | ||
| ) | ||
|
|
||
| const successResult = result as any |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
Summary by Sourcery
Improve type safety, error handling, and linting across chat, network, workflow, and tooling components while enhancing visualization of nested agent/workflow/network executions and tightening ESLint TypeScript rules.
Enhancements:
anyand improve React keying and state handling.Build:
no-unused-vars, enabling stricter boolean and type-safety checks across the codebase.