Conversation
- Changed imports in multiple tool files to use type imports for `InferUITool` and standard imports for `createTool` for consistency and clarity. - Updated progress logging in various tools to use `writer?.custom` with `type: 'data-tool-progress'` instead of `writer?.write` with `type: 'progress'` to standardize the logging approach. - Cleaned up unnecessary whitespace in several files for better readability. - Enhanced error handling messages to use nullish coalescing operator (`??`) for default values in Python analysis and complexity functions. - Improved the test generator tool by ensuring consistent formatting and handling of edge cases in test case generation.
|
Keep this PR in a mergeable state → Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
Reviewer's GuideRefactors Mastra tools to standardize type-only imports and progress logging, improves robustness and null-handling in analysis utilities, consolidates workflow/network routing in the Mastra server, and enhances the test generator and semantic utilities for better correctness and consistency. Sequence diagram for standardized tool execution progress logging with writer.customsequenceDiagram
actor User
participant Caller as ToolCaller
participant Tool as MastraTool
participant Writer as UIWriter
participant Span as TracingSpan
participant API as ExternalService
User->>Caller: Invoke tool (e.g. listRepositories)
Caller->>Tool: execute({ context, writer, tracingContext })
Tool->>Span: createChildSpan(type=TOOL_CALL)
activate Span
Tool->>Writer: custom({ type: data-tool-progress, message: "Starting ..." })
note right of Writer: Previously writer.write({ type: progress, ... })
Tool->>API: Perform external call
activate API
API-->>Tool: Response / error
deactivate API
alt Success
Tool->>Writer: custom({ type: data-tool-progress, message: "✅ Completed" })
Tool->>Span: end({ output: { success: true, ... } })
else Failure
Tool->>Writer: custom({ type: data-tool-progress, message: "❌ Error ..." })
Tool->>Span: error({ error, endSpan: true })
end
deactivate Span
Tool-->>Caller: Structured tool result
Caller-->>User: Streamed UI updates based on data-tool-progress events
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbitRelease NotesNew Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR performs a large-scale refactoring across 30+ tool files and core modules, consolidating import patterns (converting Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25–35 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
🤖 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. |
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 significantly enhances the codebase's maintainability and consistency through a series of targeted refactorings. It streamlines how modules are imported, standardizes the way tools report their progress, and improves the robustness of various functions by adopting more precise null-checking mechanisms. Additionally, it simplifies API route configurations and refines the test generation process, contributing to a more robust and developer-friendly environment. 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
|
|
🤖 I'm sorry @ssdeanx, but I was unable to process your request. Please see the logs for more details. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
src/mastra/tools/github.ts, the changes tolistIssuesintroduce regressions: checkingcontext.labels !== nullwill still appendlabels=undefinedwhenlabelsisundefined, and filtering withissue.pull_request === nullwill exclude issues wherepull_requestisundefinedrather than simply falsy; consider restoring the previousif (context.labels)guard and!issue.pull_requestfilter semantics. - In
src/mastra/tools/pnpm-tool.ts, the added import ofvaluefrom../../../app/components/monaco/monaco-themesand destructuring{ value }fromcontextinpnpmChangesetPublishlook unused and unrelated to the tool’s concerns; they should be removed to avoid confusion and potential type/runtime issues. - In
src/mastra/index.ts, the new importsResearchRuntimeContextandmetadataare not used anywhere in the file; consider removing them to keep the entrypoint lean and avoid dead dependencies.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `src/mastra/tools/github.ts`, the changes to `listIssues` introduce regressions: checking `context.labels !== null` will still append `labels=undefined` when `labels` is `undefined`, and filtering with `issue.pull_request === null` will exclude issues where `pull_request` is `undefined` rather than simply falsy; consider restoring the previous `if (context.labels)` guard and `!issue.pull_request` filter semantics.
- In `src/mastra/tools/pnpm-tool.ts`, the added import of `value` from `../../../app/components/monaco/monaco-themes` and destructuring `{ value }` from `context` in `pnpmChangesetPublish` look unused and unrelated to the tool’s concerns; they should be removed to avoid confusion and potential type/runtime issues.
- In `src/mastra/index.ts`, the new imports `ResearchRuntimeContext` and `metadata` are not used anywhere in the file; consider removing them to keep the entrypoint lean and avoid dead dependencies.
## Individual Comments
### Comment 1
<location> `src/mastra/index.ts:238-247` </location>
<code_context>
+ thread: {
+ id: 'network',
+ resourceId: 'network',
+ metadata: { agentId: ':agentId' }
+ },
+ resource: "network",
+ options: {
+ lastMessages: 500,
+ semanticRecall: true,
+ workingMemory: { enabled: true },
+ threads: { generateTitle: true }
+ }
+ },
+ maxSteps: 200,
+ telemetry: {
+ isEnabled: true,
+ recordInputs: true,
+ recordOutputs: true,
+ },
+ includeRawChunks: true,
+ }
+ }),
</code_context>
<issue_to_address>
**issue (bug_risk):** Network memory thread metadata and save-per-step behavior may not match the intended semantics.
Two specific checks for this `networkRoute` config:
1. `metadata: { agentId: ':agentId' }` will persist the literal string `":agentId"`, not the route param value. If you plan to read `agentId` from metadata later, this likely needs to be the resolved param.
2. The previous config had `savePerStep: true` in `defaultOptions`, but that’s now missing. If per‑step persistence is still required for network runs, please re‑add it; otherwise this is an intentional behavioral change that may affect any code assuming step‑level saves.
</issue_to_address>
### Comment 2
<location> `src/mastra/tools/github.ts:207-213` </location>
<code_context>
try {
let path = `/repos/${context.owner}/${context.repo}/issues?state=${context.state}&sort=${context.sort}&direction=${context.direction}&per_page=${context.perPage}`;
- if (context.labels) path += `&labels=${context.labels}`;
+ if (context.labels !== null) {path += `&labels=${context.labels}`;}
const data = await githubFetch<Array<Record<string, unknown>>>(path);
// Filter out pull requests (GitHub API returns PRs as issues too)
const issues = data
- .filter((issue) => !issue.pull_request)
+ .filter((issue) => issue.pull_request === null)
.map((issue) => ({
number: issue.number as number,
</code_context>
<issue_to_address>
**issue:** Label handling and PR filtering logic in listIssues will misbehave with undefined fields.
Two edge cases in `listIssues` need adjustment:
1. The condition `context.labels !== null` still allows `undefined`, producing `labels=undefined` in the query string. Consider a stricter guard like `context.labels != null && context.labels !== ''`.
2. Changing `!issue.pull_request` to `issue.pull_request === null` assumes the field is explicitly `null`. The GitHub API often omits `pull_request` entirely for issues, so PRs with `pull_request: undefined` may now pass the filter. Using `issue.pull_request == null` or reverting to the falsy check will preserve the intended “exclude PRs” behavior.
</issue_to_address>
### Comment 3
<location> `src/mastra/tools/code-analysis.tool.ts:108-110` </location>
<code_context>
} else if (language === 'python' || language === 'ruby') {
if (trimmed.startsWith('#')) {continue}
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/merge-nested-ifs))
```suggestion
} else if ((language === 'python' || language === 'ruby') && trimmed.startsWith('#')) {
continue
}
```
<br/><details><summary>Explanation</summary>Reading deeply nested conditional code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| metadata: { agentId: ':agentId' } | ||
| }, | ||
| resource: "network", | ||
| options: { | ||
| lastMessages: 500, | ||
| semanticRecall: true, | ||
| workingMemory: { enabled: true }, | ||
| threads: { generateTitle: true } | ||
| } | ||
| }, |
There was a problem hiding this comment.
issue (bug_risk): Network memory thread metadata and save-per-step behavior may not match the intended semantics.
Two specific checks for this networkRoute config:
metadata: { agentId: ':agentId' }will persist the literal string":agentId", not the route param value. If you plan to readagentIdfrom metadata later, this likely needs to be the resolved param.- The previous config had
savePerStep: trueindefaultOptions, but that’s now missing. If per‑step persistence is still required for network runs, please re‑add it; otherwise this is an intentional behavioral change that may affect any code assuming step‑level saves.
| if (context.labels !== null) {path += `&labels=${context.labels}`;} | ||
|
|
||
| const data = await githubFetch<Array<Record<string, unknown>>>(path); | ||
|
|
||
| // Filter out pull requests (GitHub API returns PRs as issues too) | ||
| const issues = data | ||
| .filter((issue) => !issue.pull_request) | ||
| .filter((issue) => issue.pull_request === null) |
There was a problem hiding this comment.
issue: Label handling and PR filtering logic in listIssues will misbehave with undefined fields.
Two edge cases in listIssues need adjustment:
- The condition
context.labels !== nullstill allowsundefined, producinglabels=undefinedin the query string. Consider a stricter guard likecontext.labels != null && context.labels !== ''. - Changing
!issue.pull_requesttoissue.pull_request === nullassumes the field is explicitlynull. The GitHub API often omitspull_requestentirely for issues, so PRs withpull_request: undefinedmay now pass the filter. Usingissue.pull_request == nullor reverting to the falsy check will preserve the intended “exclude PRs” behavior.
| } else if (language === 'python' || language === 'ruby') { | ||
| if (trimmed.startsWith('#')) continue | ||
| if (trimmed.startsWith('#')) {continue} | ||
| } |
There was a problem hiding this comment.
suggestion (code-quality): Merge nested if conditions (merge-nested-ifs)
| } else if (language === 'python' || language === 'ruby') { | |
| if (trimmed.startsWith('#')) continue | |
| if (trimmed.startsWith('#')) {continue} | |
| } | |
| } else if ((language === 'python' || language === 'ruby') && trimmed.startsWith('#')) { | |
| continue | |
| } | |
Explanation
Reading deeply nested conditional code is confusing, since you have to keep track of whichconditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two
if conditions can be combined usingand is an easy win.
Greptile OverviewGreptile SummaryThis PR refactors tool imports and standardizes progress logging across 41 files for better consistency and TypeScript optimization. Key Changes:
Issues Found:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Tool as Tool (createTool)
participant Writer as Stream Writer
participant Client as Client/UI
Dev->>Tool: Import type InferUITool (type-only)
Dev->>Tool: Import createTool (runtime)
Dev->>Tool: execute({ context, writer })
Tool->>Writer: writer?.custom({ type: 'data-tool-progress' })
Note over Tool,Writer: Changed from writer?.write({ type: 'progress' })
Writer->>Client: Stream progress updates
Tool->>Tool: Process data
Tool->>Writer: writer?.custom({ type: 'data-tool-progress' })
Writer->>Client: Stream completion status
Tool-->>Dev: Return result
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import { ResearchRuntimeContext } from './agents/index'; | ||
| import { metadata } from '../../app/docs/layout'; |
There was a problem hiding this comment.
syntax: unused imports - ResearchRuntimeContext and metadata are imported but never used
| import { ResearchRuntimeContext } from './agents/index'; | |
| import { metadata } from '../../app/docs/layout'; | |
| import { specGenerationWorkflow } from './workflows/spec-generation-workflow'; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mastra/index.ts
Line: 89:90
Comment:
**syntax:** unused imports - `ResearchRuntimeContext` and `metadata` are imported but never used
```suggestion
import { specGenerationWorkflow } from './workflows/spec-generation-workflow';
```
How can I resolve this? If you propose a fix, please make it concise.| client: unknown, | ||
| options: StreamToAISdkOptions |
There was a problem hiding this comment.
style: parameter names changed from _client and _options to client and options, but they're still unused in the function that throws an error - consider keeping the underscore prefix to indicate intentionally unused parameters
| client: unknown, | |
| options: StreamToAISdkOptions | |
| _client: unknown, | |
| _options: StreamToAISdkOptions |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/client-stream-to-ai-sdk.ts
Line: 123:124
Comment:
**style:** parameter names changed from `_client` and `_options` to `client` and `options`, but they're still unused in the function that throws an error - consider keeping the underscore prefix to indicate intentionally unused parameters
```suggestion
_client: unknown,
_options: StreamToAISdkOptions
```
How can I resolve this? If you propose a fix, please make it concise.![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 introduces several positive changes, including refactoring tool imports for better type safety, standardizing progress logging to use writer.custom, and simplifying API routes for improved maintainability. The code cleanup and use of modern JavaScript features like nullish coalescing are also welcome improvements. I've identified a few minor issues, such as unused imports and a potential logic error in filtering GitHub issues, which are detailed in the review comments. Overall, this is a strong contribution that enhances code quality and consistency.
| // Filter out pull requests (GitHub API returns PRs as issues too) | ||
| const issues = data | ||
| .filter((issue) => !issue.pull_request) | ||
| .filter((issue) => issue.pull_request === null) |
There was a problem hiding this comment.
This change to filter pull requests from issues appears to be incorrect. The GitHub API typically omits the pull_request property for regular issues (making it undefined), while it's an object for pull requests. The original check !issue.pull_request correctly handled this. The new check issue.pull_request === null will likely fail to filter out pull requests, as undefined === null is false. This could lead to pull requests being incorrectly processed as issues. I recommend reverting to the previous logic.
| .filter((issue) => issue.pull_request === null) | |
| .filter((issue) => !issue.pull_request) |
| import { ResearchRuntimeContext } from './agents/index'; | ||
| import { metadata } from '../../app/docs/layout'; |
There was a problem hiding this comment.
These imports for ResearchRuntimeContext and metadata appear to be unused within the scope of this file's changes. To keep the codebase clean and avoid unnecessary dependencies, it's best to remove them if they are not needed.
| import { log } from '../config/logger' | ||
| import { RuntimeContext } from '@mastra/core/runtime-context' | ||
| import type { RuntimeContext } from '@mastra/core/runtime-context' | ||
| import value from '../../../app/components/monaco/monaco-themes'; |
There was a problem hiding this comment.
This import seems to be unused and unrelated to the functionality of this file. It should be removed to avoid unnecessary dependencies and potential confusion.
|
|
||
| const { } = context | ||
| await writer?.write({ type: 'progress', data: { message: '🚀 Publishing changesets...' } }); | ||
| const {value} = context |
There was a problem hiding this comment.
The pnpmChangesetPublish tool has an empty input schema (z.object({})), which means context will be an empty object. Destructuring value from it will result in value being undefined. Since value is also unused, this line should be removed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (26)
src/mastra/tools/financial-chart-tools.ts (1)
92-95: Progress logging not updated per PR objectives.The PR objectives state: "Standardized progress logging by switching from
writer?.writewithtype: 'progress'towriter?.customwithtype: 'data-tool-progress'across various tools." However, this file still uses the oldwriter?.write({ type: 'progress', ... })pattern in all progress logging calls.If this standardization was intentionally applied to other tools but skipped for this file, please clarify. Otherwise, consider updating for consistency.
Also applies to: 124-124, 157-157, 239-242, 275-275, 305-305, 379-382, 417-417, 452-455, 545-548, 614-617
src/mastra/tools/code-chunking.ts (3)
29-31: Add JSDoc documentation to the exported tool.The public
codeChunkierToolexport lacks documentation explaining its purpose, parameters, capabilities, and failure modes.+ /** + * Code chunking tool that intelligently splits code files based on syntax structure. + * + * Supports syntax-aware chunking for TypeScript/JavaScript and Python by parsing + * function/class declarations. Falls back to recursive text chunking for unsupported + * file types. + * + * @example + * const result = await codeChunkierTool.execute({ + * context: { + * filePath: 'example.ts', + * content: 'export function foo() { return 42; }', + * options: { chunkSize: 512, chunkOverlap: 50 } + * } + * }); + */ export const codeChunkierTool = createTool({
37-37: Replaceany[]type with explicit chunk type.Using
any[]reduces type safety and makes it harder to catch bugs. Align the runtime type with the output schema.- const chunks: any[] = []; + const chunks: z.infer<typeof codeChunkingOutputSchema>['chunks'] = [];Alternatively, define a shared chunk type:
type CodeChunk = z.infer<typeof codeChunkingOutputSchema>['chunks'][number]; const chunks: CodeChunk[] = [];
29-149: Add observability instrumentation using @mastra/core/ai-tracing to track tool execution.The tool lacks observability spans that nearly all other tools in this codebase implement. Add AISpanType and InternalSpans imports, include tracingContext in the execute function signature, and instrument the execute function with span creation, error handling, and completion tracking to match the observability pattern used across the codebase.
src/mastra/tools/extractLearningsTool.ts (4)
7-20: Add explicit output schema to createTool configuration.The tool validates output internally (line 72-75) but doesn't declare an
outputSchemain thecreateToolconfiguration. Per coding guidelines, tools should have explicit Zod schemas for both input and output.Based on coding guidelines for
src/mastra/tools/**/*.ts.Apply this diff to add the output schema:
export const extractLearningsTool = createTool({ id: 'extract-learnings', description: 'Extract key learnings and follow-up questions from a search result', inputSchema: z.object({ query: z.string().describe('The original research query'), result: z .object({ title: z.string(), url: z.string(), content: z.string(), }) .describe('The search result to process'), }), + outputSchema: z.object({ + learning: z.string().describe('Key insight extracted from the content'), + followUpQuestions: z.array(z.string()).max(1).describe('Follow-up questions for deeper research'), + }), execute: async ({ context, mastra, writer, runtimeContext, tracingContext }) => {
56-70: Consider adding retry logic for the agent API call.The agent call at line 56 lacks retry logic, which is recommended by the coding guidelines for tools. AI agent calls can fail due to rate limits, transient network issues, or API errors.
Based on coding guidelines for
src/mastra/tools/**/*.{ts,tsx}.Consider implementing exponential backoff retry logic:
const MAX_RETRIES = 3; const INITIAL_DELAY = 1000; let lastError: Error | null = null; for (let attempt = 0; attempt < MAX_RETRIES; attempt++) { try { const response = await learningExtractionAgent.generate([ { role: 'user', content: `The user is researching "${query}". Extract a key learning and generate follow-up questions from this search result: Title: ${result.title} URL: ${result.url} Content: ${result.content.substring(0, 8000)}... Respond with a JSON object containing: - learning: string with the key insight from the content - followUpQuestions: array of up to 1 follow-up question for deeper research`, }, ]); break; // Success, exit retry loop } catch (error) { lastError = error instanceof Error ? error : new Error(String(error)); if (attempt < MAX_RETRIES - 1) { const delay = INITIAL_DELAY * Math.pow(2, attempt); await writer?.custom({ type: 'data-tool-progress', data: { message: `⏳ Retrying agent call (attempt ${attempt + 2}/${MAX_RETRIES})` } }); await new Promise(resolve => setTimeout(resolve, delay)); } } } if (lastError) { throw lastError; }
64-64: Document the content truncation limit.The content is truncated to 8000 characters without explanation. This could lose important information from longer documents.
As per coding guidelines for documenting non-obvious business logic, add a comment explaining the truncation limit:
- Content: ${result.content.substring(0, 8000)}... + // Truncate to 8000 chars to stay within LLM context window limits + // TODO: Consider smarter truncation (e.g., summarization, middle truncation) + Content: ${result.content.substring(0, 8000)}...Alternatively, consider extracting this as a named constant:
const MAX_CONTENT_LENGTH = 8000; // LLM context window limit // Then in the template: Content: ${result.content.substring(0, MAX_CONTENT_LENGTH)}...
91-107: Consider simplifying the type checking logic.The manual type checking logic for extracting metadata from an invalid response is verbose and could be more concise.
Apply this diff to simplify using optional chaining and type guards:
- // Safely compute lengths from the potentially unknown response.object - const respObj = response.object as unknown - let learningLength = 0 - let followUpQuestionsCount = 0 - if ( - respObj !== null && - respObj !== undefined && - typeof respObj === 'object' - ) { - const maybeLearning = (respObj as { learning?: unknown }).learning - if (typeof maybeLearning === 'string') { - learningLength = maybeLearning.length - } - const maybeFollowUpQuestions = (respObj as { followUpQuestions?: unknown }).followUpQuestions - if (Array.isArray(maybeFollowUpQuestions)) { - followUpQuestionsCount = maybeFollowUpQuestions.length - } - } + // Safely compute lengths from the potentially invalid response.object + const respObj = response.object as Record<string, unknown> | null | undefined; + const learningLength = typeof respObj?.learning === 'string' ? respObj.learning.length : 0; + const followUpQuestionsCount = Array.isArray(respObj?.followUpQuestions) ? respObj.followUpQuestions.length : 0;src/mastra/tools/test-generator.tool.ts (1)
235-270: Add Arize spans for observability.The tool's execute function is missing Arize spans, which is required per coding guidelines: "Add Arize spans on all tool execute functions for observability."
As per coding guidelines, all tool execute functions should include observability spans to track execution, performance, and errors. This helps with debugging and monitoring tool usage in production.
src/mastra/tools/code-analysis.tool.ts (3)
118-139: Avoid potential double-counting in complexity heuristicThe current pattern order (
/\bif\b/gbefore/\belse\s+if\b/g) meanselse ifbranches will likely be matched by both patterns, slightly inflating complexity for those constructs. Not a bug, but you can make the heuristic more consistent by ordering or scoping patterns to avoid overlap, e.g. matchingelse iffirst or refining theifregex.
141-184: Type changes todetectIssuesare fine; consider tightening heuristicsSwitching to
Array<z.infer<typeof issueSchema>>for both the return type andissuesvariable is type-equivalent and keeps things consistent with the rest of the PR. The regex patterns plus thelastIndex = 0reset correctly handle global regex state across lines.If you iterate further on this, you might:
- Narrow some patterns (e.g.,
/any(?:\s|;|,|\))/g) to avoid incidental matches in identifiers likemanywhile still catching explicitanytype usage.- Potentially branch on
languagefor rules like==/varso you don’t report language-irrelevant issues in non-JS/TS files.These are heuristics, so current behavior is acceptable.
194-254: Defaults and main analysis loop behavior look correct; consider hardening Python complexity pathThe
??defaults forincludeMetrics,detectPatterns, andmaxFileSizecorrectly preserve explicitfalsewhile providing sane fallbacks. The file filtering (existence, size, directory skip) and conditional metrics/issues computation also remain logically correct.Two improvement points to consider:
- Python complexity fallback when data shape changes: Inside the
pythonbranch, ifPythonParser.analyzeComplexityresolves but returns an object without a numericcyclomaticComplexity,complexitywill becomeundefinedand later contaminateavgComplexity(NaN). You can defensively fall back to the heuristic even on “successful” responses, e.g.:try { const complexityData = await PythonParser.analyzeComplexity(content) complexity = complexityData.cyclomaticComplexity ?? estimateComplexity(content, language) } catch { complexity = estimateComplexity(content, language) }- Top-level resilience for filesystem failures: At the moment, any unexpected
fserror (e.g., permissions, transient IO) will bubble out ofexecute. Wrapping the main loop in atry...catchthat returns a structured failure (or partial results plus an error summary) would align better with the “comprehensive error handling” expectation for tools and make this more robust in batch runs.src/mastra/tools/code-search.tool.ts (2)
64-69: Validate and clampmaxResults/contextLinesfrom user input
options.maxResultsandoptions.contextLinesare taken as-is; zero/negative values lead to surprising behavior (e.g., immediate truncation whenmaxResults <= 0, odd context slicing for negativecontextLines). It’s safer to normalize these to sensible bounds before use.Consider clamping these values:
- const isRegex = options?.isRegex ?? false - const caseSensitive = options?.caseSensitive ?? false - const maxResults = options?.maxResults ?? 100 - const includeContext = options?.includeContext ?? true - const contextLines = options?.contextLines ?? 2 + const isRegex = options?.isRegex ?? false + const caseSensitive = options?.caseSensitive ?? false + const rawMaxResults = options?.maxResults ?? 100 + const maxResults = Math.max(1, rawMaxResults) + const includeContext = options?.includeContext ?? true + const rawContextLines = options?.contextLines ?? 2 + const contextLines = Math.max(0, rawContextLines)As per coding guidelines (input sanitization in tools).
62-69: Add Arize span instrumentation aroundexecutefor observabilityThis tool’s
executefunction currently has no visible Arize span or similar tracing, even though tools undersrc/mastra/toolsare expected to emit spans for observability.Align this implementation with whatever standard helper you use elsewhere (e.g., wrapping the body in a tool-level span) so code-search runs are traceable and debuggable in Arize. I can help sketch the pattern once the existing span helper API is confirmed. As per coding guidelines (Add Arize spans on all tool execute functions).
src/mastra/tools/github.ts (1)
16-35: Add timeout, retry, and secret masking togithubFetchThe
githubFetchhelper lacks timeout, retry, and rate-limit handling required by tool guidelines. Rawfetchcalls can hang indefinitely and transient failures (5xx, 429 responses) are not retried. Additionally, the error message could expose the GitHub token if token context leaks.Implement:
AbortControllerwith a configurable timeout (reference:src/mastra/config/openai.tshas a timeout+fetch pattern)- Retry logic with exponential backoff for 429/5xx responses
- Use
maskSensitiveMessageData()fromsrc/mastra/config/pg-storage.tswhen logging errors- Optionally extract and surface
X-RateLimit-*headers in errors without logging sensitive dataThe existing API can remain unchanged; wrap these improvements in the helper.
src/mastra/tools/polygon-tools.ts (2)
920-932: Fix inconsistent logToolExecution tool ids for crypto toolsSeveral crypto tools log under the wrong tool id (
polygon-stock-fundamentals), which makes logs and traces misleading and harder to filter.I suggest aligning the ids with the actual tool names:
- logToolExecution('polygon-stock-fundamentals', { + logToolExecution('polygon-crypto-quotes', { input: context, governance: { userId, tenantId, roles, subscriptionTier, classificationLevel, currentTime, requestId: governanceCtx?.requestId, correlationId: governanceCtx?.correlationId } });- logToolExecution('polygon-stock-fundamentals', { + logToolExecution('polygon-crypto-aggregates', { input: context, governance: { userId, tenantId, roles, subscriptionTier, classificationLevel, currentTime, requestId: governanceCtx?.requestId, correlationId: governanceCtx?.correlationId } });- logToolExecution('polygon-stock-fundamentals', { + logToolExecution('polygon-crypto-snapshots', { input: context, governance: { userId, tenantId, roles, subscriptionTier, classificationLevel, currentTime, requestId: governanceCtx?.requestId, correlationId: governanceCtx?.correlationId } });Also applies to: 1164-1176, 1359-1371
214-217: Add timeouts (and ideally basic retry) to Polygon fetch callsAll Polygon tools call
fetch(finalUrl)directly with no timeout or retry logic, so hung or slow upstreams can stall tool execution and degrade UX.Consider centralizing a small
fetchWithTimeout(and optional retry/backoff) helper and using it in these execute functions to align with the “timeouts for external API calls” guidance.Also applies to: 464-467, 754-757, 1001-1004, 1232-1234, 1410-1413
src/mastra/tools/pdf-data-conversion.tool.ts (1)
413-447: Constrain pdfPath to allowed data directories / runtime context
pdfPathis taken directly from input and resolved againstprocess.cwd()with no restriction, so this tool can read arbitrary files the process user can access, which conflicts with the Kilocode header’sallowedDataPathsintent.Consider validating
pdfPathagainst an allowed base (e.g., a corpus directory) and/or usingRuntimeContextto enforce tenant/workspace scoping before callingfs.stat/fs.readFile.Also applies to: 502-561
src/mastra/tools/copywriter-agent-tool.ts (1)
73-82: Avoidmastra!non-null assertion; mirror the defensive pattern from evaluateResultToolHere
mastra!.getAgent('copywriterAgent')will throw ifmastrais ever undefined, whereasevaluateResultToolnow explicitly checksif (!mastra)and returns a structured error.Refactor execute to guard
mastrasimilarly before using it, so failures are surfaced as tool errors instead of crashing the call stack.Also applies to: 100-103, 191-208
src/mastra/tools/finnhub-tools.ts (1)
65-88: Add shared timeout/retry handling for all Finnhub fetch callsEach Finnhub tool uses
await fetch(url)directly; if Finnhub hangs or is very slow, these execute functions can block indefinitely and degrade downstream UX.Introduce a small shared helper (e.g.,
fetchWithTimeout+ basic retry/backoff) and use it in these fetch sites so external API calls obey reasonable timeouts and are more resilient.Also applies to: 218-269, 402-455, 583-628, 966-988
src/mastra/tools/pg-sql-tool.ts (1)
18-30: ExecuteQuery runs raw SQL strings without parameters (SQL injection / policy concern)
executeQueryforwards the inputquerystring directly toclient.query(query)and the tool only checks that it starts with"select", which doesn’t prevent injection or unsafe access patterns and conflicts with the “use parameterized queries or an ORM” guideline.Consider reshaping the tool API to accept a query template plus a
valuesarray and callingclient.query({ text: query, values }), or otherwise constraining the allowed SQL (e.g., whitelisting specific views/procedures) so arbitrary strings from upstream callers can’t be executed verbatim.Also applies to: 40-56
src/mastra/tools/data-file-manager.ts (1)
33-50: Tighten DATA_DIR sandbox checks to avoid prefix-based escapes
validateDataPathand several call sites rely onabsolutePath.startsWith(DATA_DIR)(or similar) to enforce the docs/data sandbox; this is prefix-based, so paths like../data_evil/...can resolve to/.../docs/data_evil/...which still start with/.../docs/datastring-wise but are outside the intendeddocs/datadirectory.At minimum, update the checks to respect path boundaries, e.g.:
- if (!absolutePath.startsWith(DATA_DIR)) { + if (!(absolutePath === DATA_DIR || absolutePath.startsWith(`${DATA_DIR}${path.sep}`))) { throw new Error( `Access denied: File path "${filePath}" is outside the allowed data directory.` ) }and apply the same boundary-aware condition where you currently check
*.startsWith(DATA_DIR)for real paths and directory paths, so onlyDATA_DIRitself or true descendants underDATA_DIR/are allowed.Also applies to: 77-82, 130-141, 185-189, 234-238, 570-576, 616-621, 677-680, 755-759
src/mastra/tools/document-chunking.tool.ts (1)
161-161: Incomplete refactoring: Progress logging not updated.These lines still use the old
writer?.write({ type: 'progress', data: { ... } })pattern instead of the newwriter?.custom({ type: 'data-tool-progress', data: { ... } })pattern that was applied in other files. This creates inconsistency across the codebase.According to the PR objectives, all progress logging should be standardized to use
writer?.customwithtype: 'data-tool-progress'.Apply this pattern to make the logging consistent:
-await writer?.write({ type: 'progress', data: { message: '📄 Starting Mastra chunker' } }); +await writer?.custom({ type: 'data-tool-progress', data: { message: '📄 Starting Mastra chunker' } });Repeat this change for all occurrences on lines 161, 427, 579, 600, 653, 745, 762, 777, 794, 804, and 873.
Also applies to: 427-427, 579-579, 600-600, 653-653, 745-745, 762-762, 777-777, 794-794, 804-804, 873-873
src/mastra/tools/pnpm-tool.ts (1)
130-150: Remove unusedvaluedestructuring fromcontext
const { value } = contextis unused and, with an empty input schema, is likely a TypeScript error (valuedoes not exist on type{}). It’s also misleading sincevalueis never referenced.I recommend removing the destructuring:
- const {value} = context - await writer?.custom({ type: 'data-tool-progress', data: { message: '🚀 Publishing changesets...' } }); + await writer?.custom({ type: 'data-tool-progress', data: { message: '🚀 Publishing changesets...' } });src/mastra/tools/browser-tool.ts (1)
13-24: Connectivity check and const usage are fine; consider browser lifecycleThe switch to
if (!browserInstance?.isConnected())and makingpreviousContentaconstare both safe and behaviorally equivalent to the prior code. However, this file still never closes the cached browser instance; in long‑lived processes that repeatedly use these tools, it may be worth adding an explicit shutdown path or hook to avoid leaking a headless browser.Also applies to: 485-490
src/mastra/tools/fs.ts (1)
46-46: Align error progress logging with success path pattern.The error handling path still uses the old
writer?.write({ type: 'progress', ... })pattern, while the success paths (lines 27, 41) use the newwriter?.custom({ type: 'data-tool-progress', ... })pattern. This creates inconsistent event types within the same tool.Apply this diff to align the error path:
- await writer?.write({ type: 'progress', data: { message: `❌ FS error: ${errorMsg}` } }); + await writer?.custom({ type: 'data-tool-progress', data: { message: `❌ FS error: ${errorMsg}` } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (41)
lib/client-stream-to-ai-sdk.ts(6 hunks)src/mastra/index.ts(2 hunks)src/mastra/tools/alpha-vantage.tool.ts(9 hunks)src/mastra/tools/arxiv.tool.ts(1 hunks)src/mastra/tools/browser-tool.ts(3 hunks)src/mastra/tools/code-analysis.tool.ts(9 hunks)src/mastra/tools/code-chunking.ts(3 hunks)src/mastra/tools/code-search.tool.ts(2 hunks)src/mastra/tools/copywriter-agent-tool.ts(1 hunks)src/mastra/tools/csv-to-json.tool.ts(2 hunks)src/mastra/tools/data-file-manager.ts(1 hunks)src/mastra/tools/data-processing-tools.ts(2 hunks)src/mastra/tools/data-validator.tool.ts(1 hunks)src/mastra/tools/document-chunking.tool.ts(1 hunks)src/mastra/tools/editor-agent-tool.ts(4 hunks)src/mastra/tools/evaluateResultTool.ts(3 hunks)src/mastra/tools/execa-tool.ts(3 hunks)src/mastra/tools/extractLearningsTool.ts(5 hunks)src/mastra/tools/financial-chart-tools.ts(6 hunks)src/mastra/tools/find-references.tool.ts(5 hunks)src/mastra/tools/find-symbol.tool.ts(7 hunks)src/mastra/tools/finnhub-tools.ts(1 hunks)src/mastra/tools/fs.ts(2 hunks)src/mastra/tools/github.ts(17 hunks)src/mastra/tools/json-to-csv.tool.ts(4 hunks)src/mastra/tools/jwt-auth.tool.ts(3 hunks)src/mastra/tools/multi-string-edit.tool.ts(1 hunks)src/mastra/tools/pdf-data-conversion.tool.ts(1 hunks)src/mastra/tools/pdf.ts(2 hunks)src/mastra/tools/pg-sql-tool.ts(2 hunks)src/mastra/tools/pnpm-tool.ts(10 hunks)src/mastra/tools/polygon-tools.ts(1 hunks)src/mastra/tools/semantic-utils.ts(8 hunks)src/mastra/tools/serpapi-academic-local.tool.ts(4 hunks)src/mastra/tools/serpapi-news-trends.tool.ts(11 hunks)src/mastra/tools/serpapi-search.tool.ts(8 hunks)src/mastra/tools/serpapi-shopping.tool.ts(10 hunks)src/mastra/tools/test-generator.tool.ts(11 hunks)src/mastra/tools/weather-tool.ts(4 hunks)src/mastra/tools/web-scraper-tool.ts(11 hunks)src/mastra/tools/write-note.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/next-js.instructions.md)
**/*.{js,jsx,ts,tsx}: Usenext/dynamicfor dynamic imports to load components only when needed, improving initial load time.
Usenext/imagecomponent for automatic image optimization, including lazy loading and responsive images.
Use React.memo to prevent unnecessary re-renders of components.
Use the<Link prefetch>tag to prefetch pages that are likely to be visited.
Use getServerSideProps, getStaticProps, or server components for fetching data on the server-side.
Use SWR or React Query for client-side data fetching and caching.
Use CSS Modules, Styled Components, or Tailwind CSS for component-level styling. Prefer Tailwind CSS for rapid development.
Use React Context, Zustand, Jotai, or Recoil for managing global state. Avoid Redux unless necessary.
Usereact-hook-formfor managing forms and validation.
Only fetch the data that is needed by the component to avoid over-fetching.
Avoid long-running synchronous operations in the main thread to prevent blocking.
Always usesetStateor hooks to update state instead of mutating state directly.
Include a complete dependency array inuseEffecthooks to prevent unexpected behavior.
Avoid writing server-side code in client components to prevent exposing secrets or causing unexpected behavior.
Usetry...catchblocks for handling errors in asynchronous operations.
Implement error boundary components usinggetDerivedStateFromErrororcomponentDidCatchlifecycle methods.
Sanitize user input to prevent Cross-Site Scripting (XSS) attacks. Be especially careful when rendering HTML directly from user input.
Store authentication tokens in HTTP-only cookies or local storage securely.
Implement role-based access control to restrict access to sensitive resources.
Clean up event listeners and timers inuseEffecthooks to avoid memory leaks.
Only update state when necessary to reduce the number of re-renders and improve performance.
Use immutable data structures and avoid mutating data directly to prevent unexpected...
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/index.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tslib/client-stream-to-ai-sdk.tssrc/mastra/tools/test-generator.tool.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/instructions/next-js.instructions.md)
Use parameterized queries or an ORM to prevent SQL injection attacks.
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/index.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tslib/client-stream-to-ai-sdk.tssrc/mastra/tools/test-generator.tool.ts
src/mastra/tools/**/*.ts
📄 CodeRabbit inference engine (src/mastra/AGENTS.md)
src/mastra/tools/**/*.ts: Use thecreateToolpattern with Zod schemas when adding new tools undersrc/mastra/tools
Use explicit Zod schemas for every tool input/output
src/mastra/tools/**/*.ts: Use Zod schemas for strict validation of tool inputs and outputs in all Mastra tools
Implement tools using the createTool({ id, inputSchema, outputSchema, execute }) pattern
Organize tools into categories: Financial (Polygon, Finnhub, AlphaVantage), Research (SerpAPI, ArXiv), Data (CSV, JSON), RAG (chunking, embeddings)
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tssrc/mastra/tools/test-generator.tool.ts
src/mastra/{tools,workflows}/**/*.ts
📄 CodeRabbit inference engine (src/mastra/AGENTS.md)
Use
RuntimeContextto enforce access control in tools and workflows
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tssrc/mastra/tools/test-generator.tool.ts
src/mastra/tools/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/mastra/tools/AGENTS.md)
src/mastra/tools/**/*.{ts,tsx}: All tools must include comprehensive error handling with try-catch blocks, clear error messages, and retry logic for API calls
Sanitize inputs, mask secrets in logs, and implement rate-limiting in tools
Implement caching, stream large data, and set timeouts for external API calls in tools
Add Arize spans on all tool execute functions for observability
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tssrc/mastra/tools/test-generator.tool.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit API keys or secrets to the repository; use maskSensitiveMessageData() helper from src/mastra/config/pg-storage.ts when logging
**/*.{ts,tsx}: Document interface and type definitions with TSDoc comments explaining their purpose and usage context
Document interface properties with /** */ comments explaining each field's purpose and constraints
Document generic type parameters with @template tags explaining what each type parameter represents
Use type guards with comments explaining the runtime validation logic being performed
Document advanced/complex TypeScript types with explanatory comments about their purpose and use cases
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/index.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tslib/client-stream-to-ai-sdk.tssrc/mastra/tools/test-generator.tool.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run eslint with --max-warnings=0 on src/**/*.{ts,tsx} to enforce linting standards
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/index.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tssrc/mastra/tools/test-generator.tool.ts
**/*.{ts,tsx,js,jsx,py,java,cs,rb,go,rs,cpp,c,h,hpp,swift,kotlin,php,scala,clj,groovy,lua,sh,bash}
📄 CodeRabbit inference engine (.github/instructions/self-explanatory-code-commenting.instructions.md)
**/*.{ts,tsx,js,jsx,py,java,cs,rb,go,rs,cpp,c,h,hpp,swift,kotlin,php,scala,clj,groovy,lua,sh,bash}: Write code that speaks for itself. Comment only when necessary to explain WHY, not WHAT. Avoid obvious comments that state what the code literally does.
Avoid redundant comments that simply repeat what the code is doing
Keep comments accurate and up-to-date with code changes. Remove or update outdated comments that no longer match the implementation.
Write comments for complex business logic that explain the WHY behind specific calculations or business rules
Document non-obvious algorithms with comments explaining the algorithm choice and its reasoning
Add comments explaining what regex patterns match, especially for complex patterns
Document API constraints, rate limits, gotchas, and external dependencies with explanatory comments
Avoid commenting out dead code. Use version control instead of maintaining commented code blocks.
Do not maintain code change history or modification logs as comments. Rely on git history and commit messages instead.
Avoid decorative divider comments (e.g., lines of equals signs or asterisks) for section separation
Ensure comments are placed appropriately above or adjacent to the code they describe
Write comments using proper grammar, spelling, and professional language
Prefer self-documenting code with clear variable/function names over adding comments to explain unclear code
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/index.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tslib/client-stream-to-ai-sdk.tssrc/mastra/tools/test-generator.tool.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/instructions/self-explanatory-code-commenting.instructions.md)
**/*.{ts,tsx,js,jsx}: Document public APIs with TSDoc/JSDoc comments including parameter descriptions, return types, examples, and thrown exceptions
Add TSDoc comments to configuration constants and environment variables explaining their source, reasoning, or constraints
Use TSDoc annotation tags (TODO, FIXME, HACK, NOTE, WARNING, PERF, SECURITY, BUG, REFACTOR, DEPRECATED) to mark special comments
Include file headers with @fileoverview, @author, @copyright, and @license tags to document file purpose and ownership
Document function parameters with @param tags, return values with @returns tags, and exceptions with @throws tags in TSDoc comments
Use @see tags in TSDoc comments to reference related functions, methods, or documentation
Include @example tags in public API documentation with code examples showing typical usage
Files:
src/mastra/tools/copywriter-agent-tool.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/web-scraper-tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/semantic-utils.tssrc/mastra/tools/evaluateResultTool.tssrc/mastra/tools/fs.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/data-file-manager.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/finnhub-tools.tssrc/mastra/tools/editor-agent-tool.tssrc/mastra/tools/polygon-tools.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/execa-tool.tssrc/mastra/tools/pg-sql-tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/index.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/code-chunking.tssrc/mastra/tools/pdf.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/browser-tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/extractLearningsTool.tssrc/mastra/tools/write-note.tssrc/mastra/tools/weather-tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/pnpm-tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/financial-chart-tools.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/data-processing-tools.tslib/client-stream-to-ai-sdk.tssrc/mastra/tools/test-generator.tool.ts
src/mastra/tools/**/*.tool.ts
📄 CodeRabbit inference engine (src/mastra/tools/AGENTS.md)
src/mastra/tools/**/*.tool.ts: Define Zod schema for inputs and outputs when creating tools
Tool IDs should follow the 'namespace:toolName' format (e.g., 'alpha-vantage:stockTool')
Files:
src/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/pdf-data-conversion.tool.tssrc/mastra/tools/arxiv.tool.tssrc/mastra/tools/serpapi-news-trends.tool.tssrc/mastra/tools/multi-string-edit.tool.tssrc/mastra/tools/serpapi-academic-local.tool.tssrc/mastra/tools/json-to-csv.tool.tssrc/mastra/tools/code-analysis.tool.tssrc/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/find-symbol.tool.tssrc/mastra/tools/code-search.tool.tssrc/mastra/tools/jwt-auth.tool.tssrc/mastra/tools/data-validator.tool.tssrc/mastra/tools/document-chunking.tool.tssrc/mastra/tools/find-references.tool.tssrc/mastra/tools/csv-to-json.tool.tssrc/mastra/tools/alpha-vantage.tool.tssrc/mastra/tools/test-generator.tool.ts
🧬 Code graph analysis (4)
src/mastra/tools/serpapi-shopping.tool.ts (2)
src/mastra/tools/serpapi-config.ts (1)
validateSerpApiKey(35-42)lib/types/mastra-api.ts (1)
AISpanType(177-184)
src/mastra/tools/serpapi-news-trends.tool.ts (2)
src/mastra/tools/serpapi-config.ts (1)
validateSerpApiKey(35-42)lib/types/mastra-api.ts (1)
AISpanType(177-184)
src/mastra/tools/weather-tool.ts (1)
src/mastra/config/logger.ts (1)
log(14-18)
src/mastra/tools/pnpm-tool.ts (1)
src/mastra/config/logger.ts (1)
log(14-18)
🪛 ast-grep (0.40.0)
src/mastra/tools/code-search.tool.ts
[warning] 94-94: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern, caseSensitive ? 'g' : 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Sourcery review
- GitHub Check: Codacy Security Scan
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
| if (done) {break;} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use idiomatic brace formatting.
The formatting if (done) {break;} is unusual. For consistency with common style guides, either omit braces for single statements or use multi-line formatting.
Apply this diff for the single-statement style (more common in TypeScript):
- if (done) {break;}
+ if (done) break;Or use multi-line formatting if braces are required by your linter:
- if (done) {break;}
+ if (done) {
+ break;
+ }📝 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.
| if (done) {break;} | |
| if (done) break; |
🤖 Prompt for AI Agents
In lib/client-stream-to-ai-sdk.ts around line 42, the statement uses
unconventional single-line brace formatting ("if (done) {break;}"); change it to
the idiomatic single-statement style by removing the braces and placing the
break on the same line as the condition (i.e., "if (done) break;") so it matches
common TypeScript style and the project's linting conventions.
| client: unknown, | ||
| options: StreamToAISdkOptions |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and view the function context
wc -l lib/client-stream-to-ai-sdk.tsRepository: ssdeanx/AgentStack
Length of output: 96
🏁 Script executed:
# View the function containing lines 123-124 with context
sed -n '110,140p' lib/client-stream-to-ai-sdk.tsRepository: ssdeanx/AgentStack
Length of output: 713
🏁 Script executed:
# Check for linting configuration
ls -la | grep -E "(eslint|prettier|tsconfig)"
cat tsconfig.json | grep -A 5 "compilerOptions"Repository: ssdeanx/AgentStack
Length of output: 437
🏁 Script executed:
# Check if eslint is available and run it on the file
if command -v eslint &> /dev/null; then
echo "=== ESLint check ==="
eslint lib/client-stream-to-ai-sdk.ts 2>&1 | head -50
else
echo "ESLint not found"
fiRepository: ssdeanx/AgentStack
Length of output: 78
🏁 Script executed:
# View the complete tsconfig.json file
cat tsconfig.jsonRepository: ssdeanx/AgentStack
Length of output: 1559
🏁 Script executed:
# Check if there are any compilation errors or warnings
if command -v tsc &> /dev/null; then
echo "=== TypeScript compiler check ==="
tsc --noEmit 2>&1 | grep -i "client\|options\|unused" | head -20
else
echo "TypeScript compiler not available in sandbox"
fiRepository: ssdeanx/AgentStack
Length of output: 106
🏁 Script executed:
# Search for other uses of _client or _options pattern in the codebase to understand the convention
rg "_client|_options" --type ts --type tsx -B 2 -A 2 | head -50Repository: ssdeanx/AgentStack
Length of output: 89
🏁 Script executed:
# Double-check for unused variable warnings in the ESLint config
cat eslint.config.cjs | grep -A 5 -B 5 "no-unused"Repository: ssdeanx/AgentStack
Length of output: 415
🏁 Script executed:
# Check git history to understand if underscores were actually removed in this PR
git log --oneline -n 20 lib/client-stream-to-ai-sdk.ts 2>/dev/null || echo "git log not available"Repository: ssdeanx/AgentStack
Length of output: 126
Add underscore prefix to unused function parameters to suppress ESLint warnings.
The function parameters client and options are unused since the function immediately throws an error. ESLint is configured with @typescript-eslint/no-unused-vars set to 'warn', which will flag these unused parameters. The underscore prefix convention (_client, _options) is the standard way to indicate intentionally unused parameters and suppress this warning.
Change to:
export async function createMastraStreamResponse(
_client: unknown,
_options: StreamToAISdkOptions
): Promise<Response> {🤖 Prompt for AI Agents
In lib/client-stream-to-ai-sdk.ts around lines 123-124, the exported function
createMastraStreamResponse currently declares parameters client and options but
immediately throws, leaving them unused and triggering ESLint no-unused-vars
warnings; rename the parameters to _client and _options (underscore-prefixed) to
mark them intentionally unused and suppress the ESLint warnings, keeping the
rest of the function unchanged.
| import { ResearchRuntimeContext } from './agents/index'; | ||
| import { metadata } from '../../app/docs/layout'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if ResearchRuntimeContext and metadata imports are used
echo "=== File content (focusing on imports and usage) ==="
# Show the imports first
head -95 src/mastra/index.ts | tail -10
echo -e "\n=== Searching for ResearchRuntimeContext usage (excluding import) ==="
rg -n "ResearchRuntimeContext" src/mastra/index.ts
echo -e "\n=== Searching for metadata variable usage (excluding import and object keys) ==="
rg -n "metadata" src/mastra/index.ts | head -30Repository: ssdeanx/AgentStack
Length of output: 1017
Remove unused imports on lines 89-90.
The imports ResearchRuntimeContext and metadata are not used anywhere in this file. They should be removed to keep the codebase clean.
🤖 Prompt for AI Agents
In src/mastra/index.ts around lines 89-90, the imports "ResearchRuntimeContext"
and "metadata" are unused; remove these two named imports from the import
statements (or delete the whole import lines if they become empty) and save the
file so linters/formatters reflect the change; run the project linter/build to
confirm no remaining references.
| threads: { generateTitle: true } | ||
| } | ||
| }, | ||
| maxSteps: 200, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify the high maxSteps value is intentional.
The maxSteps: 200 for the network route is 4x higher than the maxSteps: 50 used for chat routes. While networks may legitimately require more steps, this high value could impact API latency, resource consumption, and cost. Please verify this is intentional and consider documenting the rationale.
🤖 Prompt for AI Agents
In src/mastra/index.ts around line 248, the network route sets maxSteps: 200
which is much higher than the chat routes' maxSteps: 50; verify whether 200 is
intentional and either (A) if it is intentional, add an inline comment/docstring
near this setting explaining the rationale (why networks need 4x steps and the
expected trade-offs for latency/cost), or (B) if unintentional, reduce the value
to the intended threshold (e.g., 50 or a shared constant) and update any related
tests/configs to use the consistent maxSteps constant; ensure the final change
either documents the reason or consolidates the value into a shared constant for
clarity.
| if (node.isKind(SyntaxKind.FunctionDeclaration) || | ||
| node.isKind(SyntaxKind.ClassDeclaration) || | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unnecessary blank line.
The blank line added before the conditional does not improve readability and breaks the cohesion of the variable declaration and conditional logic.
let name: string | undefined;
-
if (node.isKind(SyntaxKind.FunctionDeclaration) ||📝 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.
| let name: string | undefined; | |
| if (node.isKind(SyntaxKind.FunctionDeclaration) || |
🤖 Prompt for AI Agents
In src/mastra/tools/code-chunking.ts around line 48, there is an unnecessary
blank line inserted before the conditional that separates a variable declaration
from its immediately related if-statement; remove that blank line so the
declaration and the conditional are adjacent to restore cohesion and readability
(keep surrounding indentation and formatting unchanged).
| runtimeContext: runtimeContext | ||
| runtimeContext | ||
| }) | ||
| await writer?.write({ type: 'progress', data: { message: '📡 Querying SerpAPI for AI overview...' } }); |
There was a problem hiding this comment.
Missed in refactoring: Old progress logging pattern still present.
Line 260 still uses the old writer?.write({ type: 'progress', ... }) pattern while all other progress logging in this file (lines 92, 132, 182, 187, 247, 294, 304) has been updated to use writer?.custom({ type: 'data-tool-progress', ... }).
This creates an inconsistency within the same file.
Apply this fix:
-await writer?.write({ type: 'progress', data: { message: '📡 Querying SerpAPI for AI overview...' } });
+await writer?.custom({ type: 'data-tool-progress', data: { message: '📡 Querying SerpAPI for AI overview...' } });📝 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.
| await writer?.write({ type: 'progress', data: { message: '📡 Querying SerpAPI for AI overview...' } }); | |
| await writer?.custom({ type: 'data-tool-progress', data: { message: '📡 Querying SerpAPI for AI overview...' } }); |
🤖 Prompt for AI Agents
In src/mastra/tools/serpapi-search.tool.ts around line 260, the progress log
uses the old writer?.write({ type: 'progress', ... }) pattern which is
inconsistent with the rest of the file; replace this call with writer?.custom({
type: 'data-tool-progress', data: { message: '📡 Querying SerpAPI for AI
overview...' } }) so it matches the updated progress logging convention and
payload shape used on lines 92, 132, 182, 187, 247, 294, and 304.
| if (exportMatch[1]) {exports.push(`default (${exportMatch[1]})`)} | ||
| else {exports.push('default')} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider multi-line formatting for consistency.
The single-line if-else blocks reduce readability. Consider reformatting to multi-line for better code clarity.
Apply this diff:
- if (exportMatch[1]) {exports.push(`default (${exportMatch[1]})`)}
- else {exports.push('default')}
+ if (exportMatch[1]) {
+ exports.push(`default (${exportMatch[1]})`)
+ } else {
+ exports.push('default')
+ }📝 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.
| if (exportMatch[1]) {exports.push(`default (${exportMatch[1]})`)} | |
| else {exports.push('default')} | |
| if (exportMatch[1]) { | |
| exports.push(`default (${exportMatch[1]})`) | |
| } else { | |
| exports.push('default') | |
| } |
🤖 Prompt for AI Agents
In src/mastra/tools/test-generator.tool.ts around lines 112 to 113, the
single-line if/else reduces readability; replace the inline conditional with
multi-line blocks: put the if block on its own lines with braces and the push
call inside, and the else block similarly on its own lines, preserving the same
logic (push `default (${exportMatch[1]})` when exportMatch[1] is truthy,
otherwise push `default`).
| for (const test of testCases) { | ||
| content += `test('${func.name} - ${test.name}', ${func.isAsync ? 'async ' : ''}() => { | ||
| ${test.code.replace(/^ /gm, '')} | ||
| ${test.code.replace(/^ {2}/gm, '')} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a clarifying comment for the regex replacement.
The regex replace(/^ {2}/gm, '') adjusts indentation but the reasoning isn't immediately obvious.
Consider adding a brief comment:
-${test.code.replace(/^ {2}/gm, '')}
+${test.code.replace(/^ {2}/gm, '')} // Remove 2-space indentation for test-only styleCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/mastra/tools/test-generator.tool.ts around line 216, add a brief
clarifying comment immediately above the replace call that explains the regex
replace(/^ {2}/gm, '') removes leading two-space indentation from each line to
normalize the generated test code's indentation (e.g., trimming common
indentation introduced by template literals or block formatting). Keep the
comment short and to the point.
| const { sourceFile, outputPath, options } = context | ||
| const { includeEdgeCases, mockExternals, testStyle } = options || {} | ||
|
|
||
| if (!await fileExists(sourceFile)) { | ||
| throw new Error(`Source file not found: ${sourceFile}`) | ||
| } |
There was a problem hiding this comment.
Validate and sanitize the sourceFile path.
The sourceFile input is not validated against path traversal attacks. A malicious user could provide paths like ../../../etc/passwd to access files outside the intended scope.
Consider adding path validation:
const { sourceFile, outputPath, options } = context
const { includeEdgeCases, mockExternals, testStyle } = options || {}
+
+// Validate path to prevent traversal attacks
+const resolvedPath = path.resolve(sourceFile)
+const normalizedPath = path.normalize(sourceFile)
+if (normalizedPath.includes('..')) {
+ throw new Error('Path traversal detected in sourceFile')
+}
if (!await fileExists(sourceFile)) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/mastra/tools/test-generator.tool.ts around lines 236-241, the sourceFile
path is used without validation which allows path traversal; normalize and
resolve the path against a known base (e.g., project root or configured
workspace directory), reject absolute paths or any path that, after
path.resolve(base, sourceFile) and path.normalize, does not start with the base
directory, and also optionally enforce allowed file extensions (.ts, .tsx, .js)
and deny traversal tokens; after resolving and validating, use that safePath for
fileExists and subsequent operations and fail early with a clear error if
validation fails.
| const content = await fs.readFile(sourceFile, 'utf-8') | ||
| const functions = parseTypeScriptFunctions(content) | ||
| const exports = parseExports(content) | ||
|
|
||
| const { content: testContent, tests } = generateTestFile( | ||
| sourceFile, | ||
| functions, | ||
| exports, | ||
| { testStyle, mockExternals, includeEdgeCases } | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Enhance error handling for file operations.
The fs.readFile operation at line 243 is not wrapped in a try-catch block. If the file read fails (e.g., permissions issue, encoding problem), the error message won't be user-friendly.
Consider wrapping file operations in try-catch:
-const content = await fs.readFile(sourceFile, 'utf-8')
-const functions = parseTypeScriptFunctions(content)
-const exports = parseExports(content)
-
-const { content: testContent, tests } = generateTestFile(
- sourceFile,
- functions,
- exports,
- { testStyle, mockExternals, includeEdgeCases }
-)
+try {
+ const content = await fs.readFile(sourceFile, 'utf-8')
+ const functions = parseTypeScriptFunctions(content)
+ const exports = parseExports(content)
+
+ const { content: testContent, tests } = generateTestFile(
+ sourceFile,
+ functions,
+ exports,
+ { testStyle, mockExternals, includeEdgeCases }
+ )
+} catch (error) {
+ throw new Error(`Failed to process source file: ${error instanceof Error ? error.message : 'Unknown error'}`)
+}🤖 Prompt for AI Agents
In src/mastra/tools/test-generator.tool.ts around lines 243 to 252, the call to
fs.readFile is not protected and will throw raw errors on IO failures; wrap the
file read (and any subsequent parsing/generation steps that depend on its
result) in a try-catch block, on error log/throw a clear, user-friendly message
that includes the file path and the original error details, and ensure you
either rethrow or return a handled error value so callers can react
appropriately.
There was a problem hiding this comment.
Pull request overview
This PR aims to refactor tool imports and standardize progress logging across the codebase. The changes include converting to type imports for InferUITool, standardizing progress logging from writer.write to writer.custom with updated event types, improving error handling with nullish coalescing operators, and cleaning up code formatting.
Key Changes:
- Import statements refactored to use
import typefor type-only imports - Progress logging standardized to use
writer.custom({ type: 'data-tool-progress', ... }) - Error handling improved with nullish coalescing operator (
??) replacing logical OR (||)
Critical Issues Found:
Several unrelated and potentially breaking changes have been introduced that are outside the scope of this PR, including accidental imports, incorrect conditional logic changes, and a major API route reorganization that could break existing clients.
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mastra/tools/write-note.ts | Type imports added, progress logging updated |
| src/mastra/tools/web-scraper-tool.ts | Progress logging updated across multiple tools |
| src/mastra/tools/weather-tool.ts | Type imports added, progress logging updated |
| src/mastra/tools/test-generator.tool.ts | Formatting improvements, Array syntax standardized |
| src/mastra/tools/serpapi-*.ts | Progress logging updated (partially incomplete) |
| src/mastra/tools/semantic-utils.ts | Nullish coalescing improvements, formatting cleanup |
| src/mastra/tools/polygon-tools.ts | Type imports added |
| src/mastra/tools/pnpm-tool.ts | Type imports added, progress logging updated, unrelated/buggy changes |
| src/mastra/tools/pg-sql-tool.ts | Type imports added, progress logging updated |
| src/mastra/tools/pdf.ts | Progress logging updated |
| src/mastra/tools/pdf-data-conversion.tool.ts | Type imports added |
| src/mastra/tools/multi-string-edit.tool.ts | Nullish coalescing improvements |
| src/mastra/tools/jwt-auth.tool.ts | Import added, progress logging updated, tracing improvements |
| src/mastra/tools/json-to-csv.tool.ts | Type imports added, progress logging updated |
| src/mastra/tools/github.ts | Type imports, nullish coalescing, progress logging, incorrect condition changes |
| src/mastra/tools/fs.ts | Progress logging updated |
| src/mastra/tools/finnhub-tools.ts | Type imports added |
| src/mastra/tools/find-symbol.tool.ts | Type imports, formatting cleanup |
| src/mastra/tools/find-references.tool.ts | Type imports, formatting cleanup |
| src/mastra/tools/financial-chart-tools.ts | Type imports, nullish coalescing improvements |
| src/mastra/tools/extractLearningsTool.ts | Type imports, progress logging updated |
| src/mastra/tools/execa-tool.ts | Type imports, progress logging updated |
| src/mastra/tools/evaluateResultTool.ts | Type imports, progress logging updated |
| src/mastra/tools/editor-agent-tool.ts | Type imports, progress logging updated |
| src/mastra/tools/document-chunking.tool.ts | Type imports added |
| src/mastra/tools/data-validator.tool.ts | Type imports added |
| src/mastra/tools/data-processing-tools.ts | Type imports, progress logging partially updated |
| src/mastra/tools/data-file-manager.ts | Type imports added |
| src/mastra/tools/csv-to-json.tool.ts | Type imports, shorthand property |
| src/mastra/tools/copywriter-agent-tool.ts | Type imports added |
| src/mastra/tools/code-search.tool.ts | Formatting cleanup |
| src/mastra/tools/code-chunking.ts | Formatting cleanup |
| src/mastra/tools/code-analysis.tool.ts | Formatting cleanup, Array syntax |
| src/mastra/tools/browser-tool.ts | Type imports, nullish coalescing, incorrect const usage |
| src/mastra/tools/arxiv.tool.ts | Type imports added |
| src/mastra/tools/alpha-vantage.tool.ts | Type imports, progress logging updated |
| src/mastra/index.ts | Major unrelated API route reorganization and accidental imports |
| lib/client-stream-to-ai-sdk.ts | Parameter naming changes, formatting cleanup |
Comments suppressed due to low confidence (4)
src/mastra/index.ts:89
- Unused import ResearchRuntimeContext.
import { ResearchRuntimeContext } from './agents/index';
src/mastra/index.ts:90
- Unused import metadata.
import { metadata } from '../../app/docs/layout';
src/mastra/tools/pnpm-tool.ts:12
- Unused import value.
src/mastra/tools/pnpm-tool.ts:130 - Unused variable value.
| import { log } from '../config/logger' | ||
| import { RuntimeContext } from '@mastra/core/runtime-context' | ||
| import type { RuntimeContext } from '@mastra/core/runtime-context' | ||
| import value from '../../../app/components/monaco/monaco-themes'; |
There was a problem hiding this comment.
This import appears to be unrelated to the stated PR purpose of refactoring tool imports and progress logging. The import value from '../../../app/components/monaco/monaco-themes' is suspicious and seems unintentional. Please remove this unused import.
| import { repoIngestionWorkflow } from './workflows/repo-ingestion-workflow'; | ||
| import { specGenerationWorkflow } from './workflows/spec-generation-workflow'; | ||
|
|
||
| import { ResearchRuntimeContext } from './agents/index'; |
There was a problem hiding this comment.
This import appears unrelated to the PR purpose and seems to be unused. The ResearchRuntimeContext import from './agents/index' should be removed unless it's being used elsewhere in this file.
| import { ResearchRuntimeContext } from './agents/index'; |
| try { | ||
| let path = `/repos/${context.owner}/${context.repo}/issues?state=${context.state}&sort=${context.sort}&direction=${context.direction}&per_page=${context.perPage}`; | ||
| if (context.labels) path += `&labels=${context.labels}`; | ||
| if (context.labels !== null) {path += `&labels=${context.labels}`;} |
There was a problem hiding this comment.
The condition context.labels !== null is incorrect. The labels field is defined as optional string in the schema, so it should be checked for truthiness, not just null. An empty string would pass this check but shouldn't be appended. The original condition context.labels (or undefined check) was more appropriate. Please revert to the original condition or use a more specific check.
| if (context.labels !== null) {path += `&labels=${context.labels}`;} | |
| if (context.labels) { path += `&labels=${context.labels}`; } |
| // Filter out pull requests (GitHub API returns PRs as issues too) | ||
| const issues = data | ||
| .filter((issue) => !issue.pull_request) | ||
| .filter((issue) => issue.pull_request === null) |
There was a problem hiding this comment.
This filter condition has been changed incorrectly. The GitHub API uses pull_request field to distinguish pull requests from issues - when this field exists (not just null), it's a PR. The filter should check for the absence of the field with !issue.pull_request, not issue.pull_request === null. An empty object {} in this field would pass this new check and incorrectly include PRs. Please revert to the original condition.
| .filter((issue) => issue.pull_request === null) | |
| .filter((issue) => !issue.pull_request) |
| runtimeContext: runtimeContext | ||
| runtimeContext | ||
| }) | ||
| await writer?.write({ type: 'progress', data: { message: '📡 Querying SerpAPI for AI overview...' } }); |
There was a problem hiding this comment.
This progress message update was missed during the refactoring. For consistency with the rest of the PR, this should be changed from writer?.write({ type: 'progress', ...}) to writer?.custom({ type: 'data-tool-progress', ...}).
|
|
||
| const { } = context | ||
| await writer?.write({ type: 'progress', data: { message: '🚀 Publishing changesets...' } }); | ||
| const {value} = context |
There was a problem hiding this comment.
The destructured value variable is extracted from context but is never used in the function. The inputSchema for this tool is z.object({}) (empty), so context won't have a value property. This appears to be an accidental change unrelated to the PR purpose. Please remove this line.
| import { specGenerationWorkflow } from './workflows/spec-generation-workflow'; | ||
|
|
||
| import { ResearchRuntimeContext } from './agents/index'; | ||
| import { metadata } from '../../app/docs/layout'; |
There was a problem hiding this comment.
This import appears unrelated to the PR purpose and seems to be unused. The metadata import from '../../app/docs/layout' should be removed unless it's being used elsewhere in this file.
| import { metadata } from '../../app/docs/layout'; |
|
|
||
| try { | ||
| const path = context.org | ||
| const path = context.org !== undefined |
There was a problem hiding this comment.
The condition context.org !== undefined is incorrect for determining if an optional string should be used. An empty string "" would still be considered "defined" but shouldn't be used as a truthy value. The original condition context.org was more appropriate as it correctly handles both undefined and empty strings. Please revert to the original condition or use a more specific check like context.org && context.org.trim().length > 0.
| const path = context.org !== undefined | |
| const path = context.org |
| try { | ||
| let apiPath = `/repos/${context.owner}/${context.repo}/contents/${context.path}`; | ||
| if (context.ref) apiPath += `?ref=${context.ref}`; | ||
| if (context.ref !== null) {apiPath += `?ref=${context.ref}`;} |
There was a problem hiding this comment.
The condition context.ref !== null is incorrect. The ref field is defined as optional string in the schema, so it could be undefined. This check should use context.ref (truthy check) or context.ref !== undefined to correctly handle both undefined and empty strings. Please revert to the original condition or use an appropriate check.
| if (context.ref !== null) {apiPath += `?ref=${context.ref}`;} | |
| if (context.ref !== undefined) {apiPath += `?ref=${context.ref}`;} |
|
|
||
| await page.goto(context.url, { waitUntil: 'domcontentloaded' }); | ||
| let previousContent = await page.$eval(selector, el => el.textContent ?? ''); | ||
| const previousContent = await page.$eval(selector, el => el.textContent ?? ''); |
There was a problem hiding this comment.
Changing previousContent from let to const is incorrect. This variable is reassigned later in the monitoring loop (line 494 or similar in the full code). Using const will cause a runtime error when attempting to reassign. Please revert this to let.
1 participant
InferUITooland standard imports forcreateToolfor consistency and clarity.writer?.customwithtype: 'data-tool-progress'instead ofwriter?.writewithtype: 'progress'to standardize the logging approach.??) for default values in Python analysis and complexity functions.Summary by Sourcery
Standardize tool progress logging and type imports while simplifying workflow and network routes.
Bug Fixes:
Enhancements:
data-tool-progresschannel.Tests: