Conversation
ssdeanx
commented
Dec 13, 2025
- Updated the test generator tool to include detailed progress reporting with status and stage identifiers.
- Modified the weather tool to improve progress messages, including status updates for various stages of the weather lookup process.
- Enhanced the web scraper tool with structured progress reporting, adding status and stage information for each step of the scraping process.
- Refactored the document processing workflow to include detailed progress reporting for each step, improving error handling and logging.
- Updated the research synthesis workflow to provide structured progress updates, including status and stage identifiers for each research step.
- Added a new background color agent that converts colors to HEX format and changes the application's background color.
- Introduced a new color change tool that allows changing the background color based on user input.
- Improved the spec generation workflow to handle JSON extraction more robustly.
- Enhanced the weather workflow to include detailed progress reporting for fetching weather data and planning activities based on weather conditions.
- Updated the test generator tool to include detailed progress reporting with status and stage identifiers. - Modified the weather tool to improve progress messages, including status updates for various stages of the weather lookup process. - Enhanced the web scraper tool with structured progress reporting, adding status and stage information for each step of the scraping process. - Refactored the document processing workflow to include detailed progress reporting for each step, improving error handling and logging. - Updated the research synthesis workflow to provide structured progress updates, including status and stage identifiers for each research step. - Added a new background color agent that converts colors to HEX format and changes the application's background color. - Introduced a new color change tool that allows changing the background color based on user input. - Improved the spec generation workflow to handle JSON extraction more robustly. - Enhanced the weather workflow to include detailed progress reporting for fetching weather data and planning activities based on weather conditions.
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
There was a problem hiding this comment.
Sorry @ssdeanx, your pull request is larger than the review limit of 150000 diff characters
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis pull request systematically enriches progress event payloads across numerous tools and workflows by adding structured Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
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. |
|
🤖 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 significantly enhances the application's capabilities by introducing new AI agents and tools, primarily focusing on improving the user experience through detailed progress reporting. It refines existing agent configurations, adds new functionalities like dynamic background color changes and advanced GitHub interactions, and strengthens workflow robustness and observability across various data processing and generation tasks. The changes aim to provide more transparent and reliable execution feedback for complex operations. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new bgColorAgent and colorChangeTool for changing background colors, though review comments highlight that the colorChangeTool is missing an execute function and outputSchema, and its client-side DOM manipulation is incompatible with server-side agent execution, suggesting a custom UI event instead. The PR also updates the 4.1-Beast.agent.md file by renaming the agent, updating its description, adding a target for GitHub Copilot, refining its tool list, correcting a get_errors tool reference to read/problems, expanding the todo list example with sub-steps (with a noted typo in sub-step numbering), and adding a 'Final Instructions' section with several @runSubagent calls. A similar get_errors update is made in Thinking-Beast-Mode.agent.md. The package.json file gains a new @octokit/plugin-retry dependency. Across numerous tool files (alpha-vantage.tool.ts, arxiv.tool.ts, code-analysis.tool.ts, csv-to-json.tool.ts, data-file-manager.ts, data-processing-tools.ts, data-validator.tool.ts, diff-review.tool.ts, document-chunking.tool.ts, evaluateResultTool.ts, execa-tool.ts, fs.ts, github.ts, json-to-csv.tool.ts, jwt-auth.tool.ts, multi-string-edit.tool.ts, pdf.ts, pnpm-tool.ts, serpapi-news-trends.tool.ts, serpapi-search.tool.ts, serpapi-shopping.tool.ts, test-generator.tool.ts, weather-tool.ts, web-scraper-tool.ts), context?.writer?.custom calls are standardized to a new 'Data Tool Progress Events' format, which includes status, message, stage within the data object, and a top-level id, as documented in src/mastra/tools/AGENTS.md. However, some editor-agent-tool.ts and extractLearningsTool.ts changes use type: 'data-tool-agent' instead of the documented data-tool-progress, raising a consistency concern. The github.ts file also introduces new types for GitHub API responses and adds listCommits and createRelease tools, leveraging the @octokit/plugin-retry for improved resilience. Workflow files (document-processing-workflow.ts, research-synthesis-workflow.ts, weather-workflow.ts) are updated to use the new data-workflow-step-start, data-workflow-progress, data-workflow-step-complete, and data-workflow-step-error event types, and document-processing-workflow.ts removes an import of a development-only authentication script. Lastly, spec-generation-workflow.ts improves JSON matching with the nullish coalescing operator, though a review comment suggests a more robust regex for JSON extraction.
| export const colorChangeTool = createTool({ | ||
| id: "changeColor", | ||
| description: "Changes the background color", | ||
| inputSchema: z.object({ | ||
| color: z.string(), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
The colorChangeTool is missing the required execute function, which will cause a runtime error when the bgColorAgent attempts to use it. Additionally, an outputSchema should be defined to specify the tool's return structure.
The changeBgColor function contains client-side code (document.body.style) which cannot be executed by a server-side agent. An agent should typically send a custom UI message to the client, which then handles the DOM manipulation.
export const colorChangeTool = createTool({
id: "changeColor",
description: "Changes the background color",
inputSchema: z.object({
color: z.string(),
}),
outputSchema: z.object({
success: z.boolean(),
}),
execute: async ({ color }, { writer }) => {
// On the server, we can't change the DOM directly.
// Instead, we send a custom event to the client to handle the color change.
await writer?.custom({
type: 'ui-update',
data: {
component: 'background',
properties: {
color: color,
},
},
});
return { success: true };
},
});| Your task is to convert this color to HEX format (e.g., "#FF0000") and call the colorChangeTool with the HEX color. | ||
| `, | ||
| model: "openai/gpt-4o-mini", | ||
| tools: { colorChangeTool }, |
There was a problem hiding this comment.
The bgColorAgent is a server-side agent, but it's configured to use colorChangeTool, which appears to be a client-side tool (as it manipulates the DOM). A server-side agent cannot directly execute client-side code. This architectural mismatch will likely lead to runtime errors. Consider having the agent emit a custom UI event that the client can listen for to change the background color.
| import { z } from 'zod'; | ||
|
|
||
| import { logError, logStepEnd, logStepStart } from '../config/logger'; | ||
| import main from '../../../lib/auth-dev'; |
There was a problem hiding this comment.
| - [ ] Sub-step 1.2: Description of the second sub-step | ||
| - [ ] Step 2: Description of the second step | ||
| - [ ] Sub-step 2.1: Description of the first sub-step | ||
| - [ ] Sub-step 1.2: Description of the second sub-step |
There was a problem hiding this comment.
There appears to be a typo in the todo list example. The sub-step numbering for "Step 2" is incorrect and seems to be a copy-paste from "Step 1". It should likely be Sub-step 2.2.
| - [ ] Sub-step 1.2: Description of the second sub-step | |
| - [ ] Sub-step 2.2: Description of the second sub-step |
| }), | ||
| execute: async (inputData, context) => { | ||
| await context?.writer?.custom({ type: 'data-tool-progress', data: { message: '📝 Starting editor agent' } }); | ||
| await context?.writer?.custom({ type: 'data-tool-agent', data: { message: '📝 Starting editor agent' }, id: 'editor-agent' }); |
There was a problem hiding this comment.
The progress event type is set to 'data-tool-agent'. The new standard defined in src/mastra/tools/AGENTS.md specifies 'data-tool-progress'. Is this a new event type, or should it be aligned with the standard? If it's a new type, it should be documented. If not, it should be changed for consistency.
| await context?.writer?.custom({ type: 'data-tool-agent', data: { message: '📝 Starting editor agent' }, id: 'editor-agent' }); | |
| await context?.writer?.custom({ type: 'data-tool-progress', data: { message: '📝 Starting editor agent' }, id: 'editor-agent' }); |
| }), | ||
| execute: async (inputData, context) => { | ||
| await context?.writer?.custom({ type: 'data-tool-progress', data: { message: '🧠 Extracting learnings from search result' } }); | ||
| await context?.writer?.custom({ type: 'data-tool-agent', data: { message: '🧠 Extracting learnings from search result' }, id: 'extract-learnings' }); |
There was a problem hiding this comment.
The progress event type is set to 'data-tool-agent'. The new standard defined in src/mastra/tools/AGENTS.md specifies 'data-tool-progress'. Is this a new event type, or should it be aligned with the standard? If it's a new type, it should be documented. This is consistent with the issue in editor-agent-tool.ts.
| await context?.writer?.custom({ type: 'data-tool-agent', data: { message: '🧠 Extracting learnings from search result' }, id: 'extract-learnings' }); | |
| await context?.writer?.custom({ type: 'data-tool-progress', data: { message: '🧠 Extracting learnings from search result' }, id: 'extract-learnings' }); |
| @@ -118,7 +119,8 @@ const loadDocumentStep = createStep({ | |||
| type: "workflow", | |||
| data: "load-document", | |||
| id: "load-document", | |||
There was a problem hiding this comment.
The new progress event format includes a redundant id field inside the data object, in addition to the top-level id. According to the new standard defined in src/mastra/tools/AGENTS.md, the id should only be at the top level. This should be removed for consistency. This issue is present in multiple places in this file.
| id: "load-document", | |
| data: "load-document", |
| await writer?.custom({ | ||
| type: 'data-workflow-step-start', | ||
| data: { | ||
| type: 'data-workflow-step-start', | ||
| data: { | ||
| 'initialize-research': { | ||
| topicsCount: inputData.topics.length, | ||
| concurrency: inputData.concurrency, | ||
| }, | ||
| }, | ||
| }, | ||
| id: 'initialize-research', | ||
| }); |
There was a problem hiding this comment.
The new progress event structure seems overly nested. For example, the data-workflow-step-start event has a data object which in turn contains another type and data object. This could be simplified for consistency and easier parsing on the client-side.
await writer?.custom({
type: 'data-workflow-step-start',
data: {
stepId: 'initialize-research',
topicsCount: inputData.topics.length,
concurrency: inputData.concurrency,
},
id: 'initialize-research',
});| const { text } = result; | ||
| // Simple heuristic to find JSON in markdown code blocks if present | ||
| const jsonMatch = (/```json\n([\s\S]*?)\n```/.exec(text)) || (/\{[\s\S]*\}/.exec(text)); | ||
| const jsonMatch = (/```json\n([\s\S]*?)\n```/.exec(text)) ?? (/\{[\s\S]*\}/.exec(text)); |
There was a problem hiding this comment.
Using the nullish coalescing operator (??) is a good improvement over the logical OR (||) for handling potentially falsy but valid values (like an empty string). However, the regex (/\{[\s\S]*\}/.exec(text)) is quite broad and could incorrectly match non-JSON content that happens to contain curly braces. For more robust JSON extraction from text, consider a more specific regex or a parsing strategy that looks for JSON within markdown code blocks first.
Greptile OverviewGreptile SummaryThis PR enhances tool progress reporting across the codebase and introduces a new background color agent/tool. The progress reporting changes follow the documented format from the custom instructions, adding structured progress events with Major Changes:
Critical Issue: Confidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Workflow
participant Tool
participant Writer
participant UI
User->>Workflow: Execute workflow/tool
activate Workflow
Workflow->>Writer: custom({ type: 'data-workflow-step-start' })
Writer->>UI: Display step started
Workflow->>Tool: Execute tool (weatherTool/webScraperTool/testGenerator)
activate Tool
Tool->>Writer: custom({ type: 'data-tool-progress', status: 'in-progress', stage: 'starting' })
Writer->>UI: Update progress: Starting
Tool->>Tool: Perform work (geocoding, fetching, parsing)
Tool->>Writer: custom({ type: 'data-tool-progress', status: 'in-progress', stage: 'processing' })
Writer->>UI: Update progress: Processing
Tool->>Writer: custom({ type: 'data-tool-progress', status: 'done', stage: 'complete' })
Writer->>UI: Update progress: Complete
Tool-->>Workflow: Return result
deactivate Tool
Workflow->>Writer: custom({ type: 'data-workflow-step-complete' })
Writer->>UI: Display step completed
Workflow-->>User: Return final result
deactivate Workflow
|
| export const colorChangeTool = createTool({ | ||
| id: "changeColor", | ||
| description: "Changes the background color", | ||
| inputSchema: z.object({ | ||
| color: z.string(), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
syntax: tool missing execute function - will cause runtime error
| export const colorChangeTool = createTool({ | |
| id: "changeColor", | |
| description: "Changes the background color", | |
| inputSchema: z.object({ | |
| color: z.string(), | |
| }), | |
| }); | |
| export const colorChangeTool = createTool({ | |
| id: "changeColor", | |
| description: "Changes the background color", | |
| inputSchema: z.object({ | |
| color: z.string(), | |
| }), | |
| execute: async ({ context }) => { | |
| changeBgColor(context.color); | |
| return { success: true, color: context.color }; | |
| }, | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mastra/color-change-tool.ts
Line: 10:16
Comment:
**syntax:** tool missing `execute` function - will cause runtime error
```suggestion
export const colorChangeTool = createTool({
id: "changeColor",
description: "Changes the background color",
inputSchema: z.object({
color: z.string(),
}),
execute: async ({ context }) => {
changeBgColor(context.color);
return { success: true, color: context.color };
},
});
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 39
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (35)
src/mastra/tools/AGENTS.md (2)
3-3: Remove H1 heading—it will be auto-generated from the title.The heading structure should start with H2 (
##), not H1 (#), per coding guidelines.-# 🔧 Tools Directory (`/src/mastra/tools`) +## 🔧 Tools Directory (`/src/mastra/tools`)
22-91: Restructure tables to comply with 80-character line length limit.Multiple table rows significantly exceed the 80-character limit specified in the coding guidelines. For example:
- Line 24 (Alpha Vantage): 289 characters
- Line 25 (Finnhub): 306 characters
- Line 80 (Code Search): 362 characters
Consider moving badge URLs to a separate column, abbreviating descriptions with links to detailed sections, or using a list-based format instead of tables.
src/mastra/workflows/spec-generation-workflow.ts (1)
48-257: Add standardized progress instrumentation to all workflow steps.Per coding guidelines and the PR's stated objective of enhancing progress reporting, all workflow steps must include standardized
writerinstrumentation:
- step-start event at the beginning of each
executefunction usingdata-workflow-step-startmessage type- Percentage-based progress updates (e.g., '20%', '50%', '90%') at logical checkpoints
- step-complete event with success status and duration in milliseconds
- step-error event in catch blocks with error message details
Currently, none of the four steps (
planStep,prdStep,archStep,tasksStep) implement this pattern. While the steps pipe agent streams to the writer, this doesn't provide the structured progress tracking required by the guidelines.As per coding guidelines for
src/mastra/workflows/**/*.ts.Example implementation for
planStep:execute: async ({ inputData, mastra, requestContext, writer }) => { + const startTime = Date.now(); + + writer.custom({ + type: 'data-workflow-step-start', + data: { step: 'create-plan', timestamp: new Date().toISOString() } + }); + + try { const { request, context, githubRepo, githubIssue } = inputData; const agent = mastra?.getAgent('code-architect'); if (!agent) { throw new Error('Agent code-architect not found'); } + + writer.custom({ + type: 'progress', + data: { step: 'create-plan', status: '20%', stage: 'agent-initialized' } + }); // ... existing code ... + writer.custom({ + type: 'progress', + data: { step: 'create-plan', status: '50%', stage: 'prompt-generated' } + }); + const stream = await agent.stream(prompt); await stream.textStream?.pipeTo?.(writer); const finalText = await stream.text; + + writer.custom({ + type: 'progress', + data: { step: 'create-plan', status: '90%', stage: 'parsing-response' } + }); + // ... parsing logic ... + writer.custom({ + type: 'step-complete', + data: { + step: 'create-plan', + status: 'success', + duration: Date.now() - startTime + } + }); + + return { /* ... */ }; + } catch (e) { + writer.custom({ + type: 'step-error', + data: { + step: 'create-plan', + error: e instanceof Error ? e.message : String(e) + } + }); + throw e; + } }Apply similar instrumentation to
prdStep,archStep, andtasksStep.src/mastra/tools/jwt-auth.tool.ts (1)
42-72: Blocker: tool returns mocked claims without verifying JWT (auth bypass unless strictly dev-only).
As written, any non-emptyjwtyieldssub: 'mock-user'androles: ['user'], regardless of token content; that’s not “Verify JWT”.At minimum, fail closed until real verification is wired (and preserve the real error message/cause):
try { - // const result = await AuthenticationService.verifyJWT(jwt) + // const result = await AuthenticationService.verifyJWT(jwt) span?.setAttribute('success', true); span?.end(); - // Mock return for now as the service call is commented out - return { - sub: 'mock-user', - roles: ['user'], - tenant: 'mock-tenant', - stepUp: false, - exp: Date.now() + 3600, - iat: Date.now() - } - // return result + throw new Error('JWT verification is not implemented; refusing to return mock claims.'); } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error) log.error(`JWT verification failed: ${errorMessage}`) span?.recordException(new Error(errorMessage)); span?.setStatus({ code: SpanStatusCode.ERROR, message: errorMessage }); span?.end(); - throw new Error('JWT verification failed: Unknown error') + throw new Error(`JWT verification failed: ${errorMessage}`, { cause: error as unknown }); }src/mastra/tools/find-references.tool.ts (1)
65-191: Consider differentiating thestagefield across execution phases.All progress events currently use the same
stage: 'semantic:find-references'value, which limits the utility of structured progress reporting. Using phase-specific stage identifiers would improve observability and enable UI/orchestration layers to react differently to each phase.Consider using distinct stage values that reflect the actual execution phase:
-await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🔎 Starting semantic find-references for '${inputData.symbolName}' at ${inputData.projectPath}`, stage: 'semantic:find-references' }, id: 'semantic:find-references' }); +await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🔎 Starting semantic find-references for '${inputData.symbolName}' at ${inputData.projectPath}`, stage: 'init' }, id: 'semantic:find-references' }); -await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🧭 Starting TypeScript/JavaScript analysis`, stage: 'semantic:find-references' }, id: 'semantic:find-references' }); +await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🧭 Starting TypeScript/JavaScript analysis`, stage: 'ts-analysis' }, id: 'semantic:find-references' }); -await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `📌 Precise lookup: ${filePath}:${line}`, stage: 'semantic:find-references' }, id: 'semantic:find-references' }); +await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `📌 Precise lookup: ${filePath}:${line}`, stage: 'precise-lookup' }, id: 'semantic:find-references' }); -await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `📁 Name-based search across project files...`, stage: 'semantic:find-references' }, id: 'semantic:find-references' }); +await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `📁 Name-based search across project files...`, stage: 'name-search' }, id: 'semantic:find-references' }); -await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🐍 Starting Python analysis`, stage: 'semantic:find-references' }, id: 'semantic:find-references' }); +await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🐍 Starting Python analysis`, stage: 'python-analysis' }, id: 'semantic:find-references' });This pattern would enable consumers to:
- Track progress more granularly
- Implement phase-specific UI feedback
- Measure time spent in each phase
- Handle phase-specific errors or cancellations
src/mastra/tools/fs.ts (4)
28-48: Early returns bypass progress “done” andspan.end()(leaks spans + inconsistent telemetry).
case 'read'(Line 35-38) anddefault(Line 42-44) return before emitting the “done” progress event (Line 45) and beforespan.end()(Line 47). This will produce missing completion events and leave spans open for those paths.@@ - try { - switch (action) { + try { + let message: string; + switch (action) { case 'write': - await fsPromises.writeFile(file, data) + await fsPromises.writeFile(file, data, 'utf8') + message = 'Success' break case 'read': { - const readContent = await fsPromises.readFile(file, 'utf8') - return { message: readContent } + message = await fsPromises.readFile(file, 'utf8') break } case 'append': - await fsPromises.appendFile(file, data) + await fsPromises.appendFile(file, data, 'utf8') + message = 'Success' break default: - return { message: 'Invalid action' } + throw new Error(`Invalid action: ${action}`) } await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: '✅ FS operation complete', stage: 'fsTool' }, id: 'fsTool' }); span?.setAttribute('success', true); - span?.end(); - return { message: 'Success' } + return { message } } catch (e) { @@ span?.setStatus({ code: SpanStatusCode.ERROR, message: errorMsg }); - span?.end(); return { message: `Error: ${errorMsg}`, }; + } finally { + span?.end(); }Also applies to: 49-61
8-18: Tighten the tool schema:actionshould be an enum;datashould be optional for reads.
Right nowactionis unconstrained anddatais required even when unused, which pushes validation burden downstream and increases invalid-action handling.@@ inputSchema: z.object({ - action: z.string(), + action: z.enum(['write', 'read', 'append']), file: z.string(), - data: z.string(), + data: z.string().optional().default(''), }),
19-41: Add access control checks and path validation to prevent path traversal and unrestricted filesystem access.The tool currently accepts any file path without validation and ignores the context parameter for authorization checks. This allows users to read/write/append to arbitrary files on the system.
Implement:
- Extract and validate user context (userId, roles, etc.) from the context parameter, following the pattern in
polygon-tools.ts- Validate the
filepath usingpath.resolve()+startsWith()to ensure it stays within an allowlisted base directory, matching the pattern inweb-scraper-tool.ts- Reject absolute paths and relative paths containing
..This aligns with the repository guideline: "Use
RuntimeContextto enforce access control in tools and workflows."
29-52: Replacestatus: 'done'withstatus: 'success'for successful operations; usestatus: 'error'for failures.The consumer at
app/workflows/providers/workflow-context.tsx:419remaps progress status values:"success"→"done","pending"→"in-progress", and any other value →"error". Since this tool emitsstatus: 'done'for both success (line 45) and error (line 51) cases, the consumer incorrectly classifies success as an error. Usestatus: 'success'on line 45 and eitherstatus: 'error'or keep'done'with an additional error flag on line 51 to match expected consumer behavior.src/mastra/tools/copywriter-agent-tool.ts (1)
8-12: Fix schema parsing:copywriterToolContextSchemais an object schema but receives a string value. The schema expects{ userId?: string, contentType?: string }but line 83-85 attempts to parse just a string. This will throw a Zod validation error at runtime.Either retrieve the full
'copywriterToolContext'object fromrequestContext?.get()and parse it with the object schema, or parse theuserIdfield directly as a string:- const userId = copywriterToolContextSchema.parse( - context?.requestContext?.get('userId') ?? 'anonymous' - ) + const userId = + z.string().optional().parse(requestContext?.get('userId')) ?? 'anonymous'Also add role-based access control to this tool. Implement governance checks (userId, tenantId, roles validation) as required by guidelines for tools in
src/mastra/tools/**/*.ts. Seepolygon-tools.tsfor the expected pattern.src/mastra/workflows/research-synthesis-workflow.ts (2)
125-126: Retries are effectively disabled bycatch { return fallback }
researchTopicStepsetsretries: 2, but thecatchreturns a fallback result, so the step likely won’t throw → the retry mechanism won’t engage. Decide which behavior you want:
- If you want retries/fail-fast: emit the error event, then
throw.- If you want best-effort with fallback: remove
retries: 2(or implement explicit retry inside).Example (retry-enabled):
} catch (error) { ... await writer?.custom({ type: 'data-workflow-topic-progress', data: { ... }, id: 'research-topic-item' }); - return { - topic: inputData.topic, - summary: `Research failed for "${inputData.topic}"`, - keyFindings: [], - sources: [], - confidence: 0, - }; + throw error; }Also applies to: 229-252
6-11: Fix hardcodedsynthesisTypeandconcurrencyparameters to respect user inputThe workflow schema accepts
synthesisTypeandconcurrencyfrom users, but both are hardcoded downstream:
- Line 635:
.foreach(researchTopicStep, { concurrency: 2 })ignoresinputData.concurrency- Line 623:
foreachWrapperStephardcodessynthesisType: 'summary', ignoring the user-provided value expected bysynthesizeResearchStepThe
initializeResearchStepreceives the full input but only outputs topic data, losing both parameters. TheforeachWrapperStepcannot recover them since it only receives foreach output. Thread these parameters through the workflow: either pass them via step context, store in a shared state, or restructure to preserve the original input across all steps.src/mastra/tools/execa-tool.ts (1)
41-48: Emit terminal progress on failures + aligntool.id/ stage naming.
As-is, errors won’t send a final{status:'done'}event, and telemetry uses mixed ids (execa-toolvsexecaTool).-import { trace } from "@opentelemetry/api"; +import { trace, SpanStatusCode } from "@opentelemetry/api"; ... const span = tracer.startSpan('execa-tool', { attributes: { - 'tool.id': 'execa-tool', + 'tool.id': 'execaTool', 'tool.input.command': inputData.command, 'tool.input.args': inputData.args.join(' '), } }); ... } catch (e) { const errorMsg = e instanceof Error ? e.message : String(e); log.error(errorMsg) span.recordException(e instanceof Error ? e : new Error(errorMsg)); - span.setStatus({ code: 2, message: errorMsg }); + span.setStatus({ code: SpanStatusCode.ERROR, message: errorMsg }); span.end(); + await writer?.custom({ + type: 'data-tool-progress', + data: { status: 'done', message: `❌ Command failed: ${errorMsg}`, stage: 'execaTool' }, + id: 'execaTool', + }); const execaErr = e as ExecaErrorType; ... }Also applies to: 51-66, 72-83
src/mastra/tools/data-processing-tools.ts (1)
58-66: FixvalidateDataToolprogress lifecycle + output schema mismatch + hardenvalidateDataPath.
validateDataToolstarts withstatus:'done'(Line 1173) and later uses legacywriter.write(Line 1289).- The catch return includes
elementCount(Line 1306-1312) which is not inoutputSchema.validateDataPath()usesstartsWith(DATA_DIR)(Line 58-65) which is not a safe directory containment check.function validateDataPath(filePath: string): string { - const absolutePath = path.resolve(DATA_DIR, filePath) - if (!absolutePath.startsWith(DATA_DIR)) { + const absolutePath = path.resolve(DATA_DIR, filePath) + const relative = path.relative(DATA_DIR, absolutePath) + if (relative === '' || relative.startsWith('..' + path.sep) || relative === '..') { throw new Error( `Access denied: File path "${filePath}" is outside the allowed data directory.` ) } return absolutePath } ... export const validateDataTool = createTool({ id: 'validate-data', ... execute: async (inputData, context) => { const writer = context?.writer; ... - await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `✅ Validating ${inputData.schemaType} data`, stage: 'validate-data' }, id: 'validate-data' }); + await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🔍 Validating ${inputData.schemaType} data`, stage: 'validate-data' }, id: 'validate-data' }); ... - await writer?.write({ type: 'progress', data: { message: `✅ Validation complete. Valid: ${isValid}` } }); + await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `✅ Validation complete. Valid: ${isValid}`, stage: 'validate-data' }, id: 'validate-data' }); ... } catch (error) { ... return { isValid: false, errors: [`Validation failed: ${errorMsg}`], warnings, - elementCount: 0, } } }, })Also applies to: 145-146, 197-205, 1173-1314
src/mastra/tools/serpapi-search.tool.ts (1)
83-85: Align progressidwith the actual tool id (google-search).
google-searchcurrently emitsid: 'serpapi-search', unlikegoogle-ai-overviewwhich usesid: 'google-ai-overview'.export const googleSearchTool = createTool({ id: 'google-search', ... - await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🔍 Searching Google for "${input.query}" (${input.numResults} results)`, stage: 'serpapi-search' }, id: 'serpapi-search' }); + await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: `🔍 Searching Google for "${input.query}" (${input.numResults} results)`, stage: 'google-search' }, id: 'google-search' }); ... - await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: '📡 Querying SerpAPI...', stage: 'serpapi-search' }, id: 'serpapi-search' }); + await writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: '📡 Querying SerpAPI...', stage: 'google-search' }, id: 'google-search' }); ... - await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `✅ Search complete: ${organicResults.length} organic results`, stage: 'serpapi-search' }, id: 'serpapi-search' }); + await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `✅ Search complete: ${organicResults.length} organic results`, stage: 'google-search' }, id: 'google-search' }); ... - await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `❌ Search failed: ${errorMessage}`, stage: 'serpapi-search' }, id: 'serpapi-search' }); + await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `❌ Search failed: ${errorMessage}`, stage: 'google-search' }, id: 'google-search' });Also applies to: 92-95, 124-125, 175-180
src/mastra/tools/alpha-vantage.tool.ts (3)
60-79: Ensure spans + progress are finalized on all return paths (crypto tool).
The crypto tool starts a span (Line 61) but returns early (e.g., missing API key at Line 73, and later “Error Message”/“Note” returns) withoutspan.end(), and without a terminal progress event in some branches. This can leak spans and leave the UI “in-progress” forever.execute: async (inputData, context) => { const span = trace.getTracer('alpha-vantage-crypto-tool', '1.0.0').startSpan('alpha-vantage-crypto', { @@ - if (typeof apiKey !== "string" || apiKey.trim() === '') { + if (typeof apiKey !== "string" || apiKey.trim() === '') { await context?.writer?.custom({ type: 'data-tool-progress', data: { message: '❌ Missing ALPHA_VANTAGE_API_KEY', status: 'done', stage: 'alpha-vantage-crypto' }, id: 'alpha-vantage-crypto' }); + span.setStatus({ code: 2, message: 'Missing ALPHA_VANTAGE_API_KEY' }); + span.end(); return { data: null, error: "ALPHA_VANTAGE_API_KEY environment variable is required" }; } @@ - if (errorMessage !== null && errorMessage !== undefined && String(errorMessage).trim() !== '') { + if (errorMessage !== null && errorMessage !== undefined && String(errorMessage).trim() !== '') { + await context?.writer?.custom({ type: 'data-tool-progress', data: { message: `❌ Crypto API error: ${String(errorMessage)}`, status: 'done', stage: 'alpha-vantage-crypto' }, id: 'alpha-vantage-crypto' }); + span.setStatus({ code: 2, message: String(errorMessage) }); + span.end(); return { data: null, error: String(errorMessage) }; } @@ - if (note !== null && note !== undefined && String(note).trim() !== '') { + if (note !== null && note !== undefined && String(note).trim() !== '') { + await context?.writer?.custom({ type: 'data-tool-progress', data: { message: `⚠️ Alpha Vantage note: ${String(note)}`, status: 'done', stage: 'alpha-vantage-crypto' }, id: 'alpha-vantage-crypto' }); + span.setStatus({ code: 2, message: String(note) }); + span.end(); return { data: null, error: String(note) // API limit reached }; }Also applies to: 102-122, 159-187
246-266: Stock tool: add terminal progress + end span on early returns.
Same pattern as crypto: span is created (Line 247) but there’s no success “done” progress, and early returns for API errors don’t end the span.execute: async (inputData, context) => { @@ - await context?.writer?.custom({ type: 'data-tool-progress', data: { message: `📈 Fetching Alpha Vantage stock data for ${inputData.symbol || 'symbol'}`, status: 'in-progress', stage: 'alpha-vantage-stock' }, id: 'alpha-vantage-stock' }); + await context?.writer?.custom({ type: 'data-tool-progress', data: { message: `📈 Fetching Alpha Vantage stock data for ${inputData.symbol || 'symbol'}`, status: 'in-progress', stage: 'alpha-vantage-stock' }, id: 'alpha-vantage-stock' }); @@ if (typeof apiKey !== "string" || apiKey.trim() === '') { await context?.writer?.custom({ type: 'data-tool-progress', data: { message: '❌ Missing ALPHA_VANTAGE_API_KEY', status: 'done', stage: 'alpha-vantage-stock' }, id: 'alpha-vantage-stock' }); + span.setStatus({ code: 2, message: 'Missing ALPHA_VANTAGE_API_KEY' }); + span.end(); return { data: null, error: "ALPHA_VANTAGE_API_KEY environment variable is required" }; }Also add a final success progress before returning
result(and “done” progress incatch), mirroring the crypto tool’s behavior.
447-569: General tool: same span/progress finalization gaps + add fetch timeout.
The general tool now emits start/query/done progress (Lines 456, 506, 542), but early returns for API “Error Message” / “Note” (later in the function) still skipspan.end(). Alsofetch(url)has no timeout, risking hung tool calls.Suggested approach: wrap the body in
try { ... } finally { span.end() }and setspan.setStatus(...)before each return/error; and useAbortSignal.timeout(...)if Node supports it in your runtime.src/mastra/tools/web-scraper-tool.ts (2)
952-973: Fix completion message + rawContent gating (both are currently wrong).
- Line 952:
savedFilePath !== nullis always true whensavedFilePathisundefined, so the message can includesaved to undefined.- Lines 958-964:
inputData.selector !== nullis true when selector isundefined, sorawContentwill beundefinedeven when no selector was provided (breaking expected output).-await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `✅ Scraping complete: ${extractedData.length} elements${(savedFilePath !== null) ? ', saved to ' + savedFilePath : ''}`, stage: 'web:scraper' }, id: 'web:scraper' }); +await writer?.custom({ + type: 'data-tool-progress', + data: { + status: 'done', + message: `✅ Scraping complete: ${extractedData.length} elements${ + typeof savedFilePath === 'string' && savedFilePath.trim() !== '' + ? ', saved to ' + savedFilePath + : '' + }`, + stage: 'web:scraper', + }, + id: 'web:scraper', +}); @@ -rawContent: - inputData.selector !== null - ? undefined - : typeof rawContent === 'string' && - rawContent.trim().length > 0 - ? rawContent - : undefined, +rawContent: + typeof inputData.selector === 'string' && inputData.selector.trim() !== '' + ? undefined + : typeof rawContent === 'string' && rawContent.trim().length > 0 + ? rawContent + : undefined,
900-934: Harden path traversal check:startsWithis bypass-prone.
The save-to-file flow relies onValidationUtils.validateFilePath(...)(Line 919) which currently usesresolvedPath.startsWith(resolvedIntendedDir). This can be bypassed by sibling prefixes (e.g.,/data_evilstartsWith/data).static validateFilePath(filePath: string, intendedDir: string): boolean { const resolvedPath = path.resolve(filePath) const resolvedIntendedDir = path.resolve(intendedDir) - return resolvedPath.startsWith(resolvedIntendedDir) + return ( + resolvedPath === resolvedIntendedDir || + resolvedPath.startsWith(resolvedIntendedDir + path.sep) + ) }src/mastra/tools/arxiv.tool.ts (2)
175-223: End span on “no search terms” early return.
Line 215 returns an error result but doesn’tspan.setStatus(...)/span.end(), leaving an orphan span for that tool call.} else { await context?.writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: '❌ No search terms provided', stage: 'arxiv' }, id: 'arxiv' }); + span.setStatus({ code: 2, message: 'No search terms provided' }); + span.end(); return { papers: [], total_results: 0,
334-363: PDF parser: fix span leak on 404 + progress message ordering.
- Line 348 emits done, but the 404 return path doesn’t end the span.
- Line 345 says “Extracting text…” before the PDF is fetched (Line 345-346), which is misleading.
if (!response.ok) { await context?.writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: '❌ PDF download failed', stage: 'arxiv-pdf-parser' }, id: 'arxiv-pdf-parser' }); if (response.status === 404) { + span.setStatus({ code: 2, message: 'PDF not found (404)' }); + span.end(); return { success: false, @@ } throw new Error(`Failed to download PDF: ${response.status} ${response.statusText}`); }Also swap the “Extracting text…” progress to after
pdfBufferis created (or rename it to “Downloading PDF…”).Also applies to: 419-463
src/mastra/tools/financial-chart-tools.ts (1)
93-179: Progress payload enrichment is consistent; consider emitting terminal progress on errors too.
Current catch paths return/throw without a final{status:'done'}progress event, which can strand the UI in “in-progress” for failures.Also applies to: 241-322, 382-488, 546-636
src/mastra/workflows/weather-workflow.ts (1)
57-66: Align workflow instrumentation to the standardized step events (step-start/progress%/step-complete/step-error).
The addedidis good, but this workflow still doesn’t follow the required workflow writer pattern (step-start event type, percentage checkpoints, and step-error in catch). As-is, tooling that expects those standard events won’t be able to reliably visualize progress/errors.At minimum, emit:
data-workflow-step-startat the top of each step- progress updates with percentages (
'20%','50%','90%')data-workflow-step-completeon success (include durationMs)data-workflow-step-errorin each catch block (include error message)Also applies to: 106-115, 153-162, 229-238
src/mastra/tools/serpapi-news-trends.tool.ts (1)
66-137: Progress payload enrichment is consistent; add terminal progress on errors for UX parity.
On exceptions, the catch blocks throw without emitting a{status:'done'}progress update, which can leave consumers stuck “in-progress”.Also applies to: 163-206, 250-321, 350-395
src/mastra/tools/document-chunking.tool.ts (3)
162-375: mastraChunker: add completion progress + end span on error paths.
You emit a start progress (Line 162), but never emit a terminal “done” progress on success, and the catch path records the exception but never callsspan.end().execute: async (inputData, context) => { await context?.writer?.custom({ type: 'data-tool-progress', data: { status: 'in-progress', message: '📄 Starting Mastra chunker', stage: 'mastra:chunker' }, id: 'mastra:chunker' }); @@ - try { + try { @@ - span.end(); + span.end(); + await context?.writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `✅ Chunked into ${chunks.length} chunks`, stage: 'mastra:chunker' }, id: 'mastra:chunker' }); return output } catch (error) { @@ span.recordException(error instanceof Error ? error : new Error(errorMessage)); span.setStatus({ code: SpanStatusCode.ERROR, message: errorMessage }); + span.end(); return {
569-640: mdocumentChunker: useinputData.indexName(currently ignored) + end spans in catch.
The schema supportsindexName(Line 55), but upsert hardcodes'memory_messages_3072'(Line 597), so callers can’t actually target a different index. Also, error paths don’t end spans.- await pgVector.upsert({ - indexName: 'memory_messages_3072', + await pgVector.upsert({ + indexName: inputData.indexName, @@ - span.recordException(error instanceof Error ? error : new Error(errorMessage)); - span.setStatus({ code: SpanStatusCode.ERROR, message: errorMessage }); + span.recordException(error instanceof Error ? error : new Error(errorMessage)); + span.setStatus({ code: SpanStatusCode.ERROR, message: errorMessage }); + span.end();
724-871: documentRerankerTool: ensure span ends on failures too.
In the catch path you set span status/exception but don’tspan.end(), leaking spans on error.} catch (error) { @@ span.recordException(error instanceof Error ? error : new Error(errorMessage)); span.setStatus({ code: SpanStatusCode.ERROR, message: errorMessage }); + span.end(); return {src/mastra/config/google.ts (1)
7-55: Remove commented-out code blocks; voice env var config is correct.Lines 3–40 contain large blocks of commented-out code that violate the guideline to avoid dead code. Remove them entirely.
The env var configuration is correct:
voiceusesprocess.env.GOOGLE_API_KEY(per@mastra/voice-googledocumentation) whileGOOGLE_GENERATIVE_AI_API_KEY. These are separate Google services and should use distinct environment variables. Do not apply the fallback pattern from the original suggested diff.src/mastra/workflows/document-processing-workflow.ts (2)
401-445: Add error handling for consistency.This step lacks a
try/catchblock andstep-errorevent emission, unlike other steps in this workflow. While the logic is simple, wrapping the execution in error handling ensures consistent error reporting if something unexpected occurs.Per coding guidelines: "Include step-error in catch blocks to report workflow step failures."
execute: async ({ inputData, writer }) => { const startTime = Date.now(); + logStepStart('pass-text-through', { contentType: inputData.contentType }); + try { await writer?.custom({ type: 'data-workflow-step-start', // ... existing code ... }); // ... rest of implementation ... await writer?.custom({ type: 'data-workflow-step-complete', // ... existing code ... }); + logStepEnd('pass-text-through', {}, Date.now() - startTime); return result; + } catch (error) { + logError('pass-text-through', error, { contentType: inputData.contentType }); + await writer?.custom({ + type: 'data-workflow-step-error', + data: { + stepId: 'pass-text-through', + error: error instanceof Error ? error.message : 'Unknown error', + }, + id: "pass-text-through", + }); + throw error; + } },
519-528: Add missingidfield for consistency.This nested progress event is missing the
idfield that all otherwriter.customcalls in this file include. This inconsistency could cause issues when filtering or correlating events.if (chunkIndex % 10 === 0) { await writer?.custom({ type: 'data-workflow-progress', data: { status: `${20 + Math.min(60, (position / content.length) * 60)}%`, message: `Processed ${chunkIndex} chunks...`, stage: "workflow", - } + }, + id: "chunk-document", }); }src/mastra/tools/github.ts (4)
110-124: Removeas anyinmapTypeForOrg(it defeats the whole point of the union return type).function mapTypeForOrg(type?: string): 'all' | 'public' | 'private' | 'forks' | 'sources' | 'member' | undefined { if (type === undefined || type === null || type === '') { return undefined; } switch (type) { case 'all': case 'public': case 'private': case 'forks': case 'sources': case 'member': - return type as any; + return type; default: return undefined; } }
319-330: Fix span error status codes (code: 2→SpanStatusCode.ERROR) for consistent OTEL semantics.
Several tools set a numeric code instead ofSpanStatusCode.ERROR, which breaks consistency in tracing backends.- span.setStatus({ code: 2, message: errorMsg }); + span.setStatus({ code: SpanStatusCode.ERROR, message: errorMsg }); span.end();Apply similarly in:
listPullRequestscatch (Line 326-327)createIssuecatch (Line 558-560)getRepositoryInfocatch (Line 711-712)Also applies to: 551-562, 704-715
142-223: AddRequestContextimport and implement access control checks for all GitHub tools.The file uses
context?.writerbut does not accesscontext?.requestContextor implement any access control enforcement, unlike other tools in the codebase (polygon-tools.ts, weather-tool.ts, etc.). All 10 tools in this file (lines 142–932) should follow the established pattern:
- Import
type { RequestContext } from '@mastra/core/request-context'- Define a
GitHubRuntimeContextinterface extendingRequestContextwith necessary governance fields- Extract and cast
requestContextin each tool's execute function:const requestContext = context?.requestContext as RequestContext<GitHubRuntimeContext>- Enforce access control checks (userId, tenantId, roles, etc.) before executing GitHub API calls
This ensures compliance with the guideline requiring
RuntimeContextto enforce access control in tools.
1-12: Add token validation and clarify the retry delay comment.The
getOctokit()function constructs an Octokit client withauth: tokenwithout checking if the token exists, which can lead to opaque 401 errors when bothGITHUB_API_KEYandGITHUB_PERSONAL_ACCESS_TOKENare missing. Additionally,retryAfter: 60is configured in seconds, making the "short retryAfter" comment misleading—60 seconds is a significant delay.function getOctokit() { const token = process.env.GITHUB_API_KEY ?? process.env.GITHUB_PERSONAL_ACCESS_TOKEN; - // Set retry configuration with a short retryAfter and maximum retries for github calls - return new OctokitWithRetry({ auth: token, request: { retries: 3, retryAfter: 60 } }); + if (!token) { + throw new Error('Missing GitHub token: set GITHUB_API_KEY or GITHUB_PERSONAL_ACCESS_TOKEN'); + } + // Retry configuration: 3 retries with 60-second delay between attempts + return new OctokitWithRetry({ auth: token, request: { retries: 3, retryAfter: 60 } }); }
| - [ ] Sub-step 1.1: Description of the first sub-step | ||
| - [ ] Sub-step 1.2: Description of the second sub-step | ||
| - [ ] Step 2: Description of the second step | ||
| - [ ] Sub-step 2.1: Description of the first sub-step | ||
| - [ ] Sub-step 1.2: Description of the second sub-step | ||
| - [ ] Step 3: Description of the third step | ||
| - [ ] Sub-step 3.1: Description of the first sub-step | ||
| - [ ] Step 4: Description of the fourth step | ||
| - [ ] Sub-step 4.1: Description of the first sub-step | ||
| - [ ] Step 5: Description of the fifth step | ||
| - [ ] Step 6: Description of the sixth step | ||
| - [ ] Step 7: Description of the seventh step | ||
| - [ ] Step 8: Description of the eighth step |
There was a problem hiding this comment.
Typo in sub-step numbering.
Line 112 has "Sub-step 1.2" under "Step 2", which should be "Sub-step 2.2" for consistency.
Apply this diff to fix the typo:
- [ ] Sub-step 2.1: Description of the first sub-step
- - [ ] Sub-step 1.2: Description of the second sub-step
+ - [ ] Sub-step 2.2: Description of the second sub-step📝 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.
| - [ ] Sub-step 1.1: Description of the first sub-step | |
| - [ ] Sub-step 1.2: Description of the second sub-step | |
| - [ ] Step 2: Description of the second step | |
| - [ ] Sub-step 2.1: Description of the first sub-step | |
| - [ ] Sub-step 1.2: Description of the second sub-step | |
| - [ ] Step 3: Description of the third step | |
| - [ ] Sub-step 3.1: Description of the first sub-step | |
| - [ ] Step 4: Description of the fourth step | |
| - [ ] Sub-step 4.1: Description of the first sub-step | |
| - [ ] Step 5: Description of the fifth step | |
| - [ ] Step 6: Description of the sixth step | |
| - [ ] Step 7: Description of the seventh step | |
| - [ ] Step 8: Description of the eighth step | |
| - [ ] Sub-step 1.1: Description of the first sub-step | |
| - [ ] Sub-step 1.2: Description of the second sub-step | |
| - [ ] Step 2: Description of the second step | |
| - [ ] Sub-step 2.1: Description of the first sub-step | |
| - [ ] Sub-step 2.2: Description of the second sub-step | |
| - [ ] Step 3: Description of the third step | |
| - [ ] Sub-step 3.1: Description of the first sub-step | |
| - [ ] Step 4: Description of the fourth step | |
| - [ ] Sub-step 4.1: Description of the first sub-step | |
| - [ ] Step 5: Description of the fifth step | |
| - [ ] Step 6: Description of the sixth step | |
| - [ ] Step 7: Description of the seventh step | |
| - [ ] Step 8: Description of the eighth step |
🤖 Prompt for AI Agents
.github/agents/4.1-Beast.agent.md around lines 108 to 120: there is a numbering
typo where "Sub-step 1.2" is listed under "Step 2" (line 112); change that entry
to "Sub-step 2.2" so sub-step numbering matches its parent step and maintain
consistency across the checklist.
| @@ -0,0 +1,18 @@ | |||
| import { Agent } from "@mastra/core/agent"; | |||
| import { Memory } from "@mastra/memory"; | |||
| import { colorChangeTool } from "../tools/color-change-tool"; | |||
There was a problem hiding this comment.
Incorrect import path.
The import path references ../tools/color-change-tool, but the file is located at src/mastra/color-change-tool.ts (not in a tools subdirectory). This will cause a module resolution error.
Apply this diff to fix the import:
-import { colorChangeTool } from "../tools/color-change-tool";
+import { colorChangeTool } from "../color-change-tool";Alternatively, move color-change-tool.ts to the tools directory to maintain consistency with other tools in the codebase.
📝 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.
| import { colorChangeTool } from "../tools/color-change-tool"; | |
| import { colorChangeTool } from "../color-change-tool"; |
🤖 Prompt for AI Agents
In src/mastra/agents/bgColorAgent.ts around line 3, the import path points to
../tools/color-change-tool but the actual file lives at
src/mastra/color-change-tool.ts; update the import to reference the correct
relative path ../color-change-tool (remove the tools segment) so the module
resolves, or alternatively move color-change-tool.ts into a tools subdirectory
and keep the existing import to maintain consistency with other tools.
| export const bgColorAgent = new Agent({ | ||
| id: "bg-color-agent", | ||
| name: "Background Color Agent", | ||
| description: | ||
| "This agent takes in a color, converts it to HEX format and changes the background color of the application.", | ||
| instructions: ` | ||
| You are a helpful assistant to change the background color of the application. | ||
| You will be given a color in any format (e.g., "red", "rgb(255, 0, 0)", "hsl(0, 100%, 50%)"). | ||
| Your task is to convert this color to HEX format (e.g., "#FF0000") and call the colorChangeTool with the HEX color. | ||
| `, | ||
| model: "openai/gpt-4o-mini", | ||
| tools: { colorChangeTool }, | ||
| memory: new Memory(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the actual filename and list all agent files with their naming conventions
fd -e ts . src/mastra/agents/ -type f | sortRepository: ssdeanx/AgentStack
Length of output: 232
🏁 Script executed:
# List all TypeScript files in src/mastra/agents directory
fd -e ts src/mastra/agents/Repository: ssdeanx/AgentStack
Length of output: 439
🏁 Script executed:
# List all TypeScript files in src/mastra/agents directory
fd . 'src/mastra/agents/' -e tsRepository: ssdeanx/AgentStack
Length of output: 1160
Rename file to follow kebab-case naming convention.
The file bgColorAgent.ts should be renamed to bg-color-agent.ts to comply with the agent file naming convention specified in the coding guidelines, which requires kebab-case for all agent files in /src/mastra/agents/.
🤖 Prompt for AI Agents
In src/mastra/agents/bgColorAgent.ts around lines 5 to 18, the filename uses
camelCase (bgColorAgent.ts) but must follow the kebab-case agent naming
convention; rename the file to src/mastra/agents/bg-color-agent.ts and update
any imports that reference the old filename to the new kebab-case path (search
the repo for "bgColorAgent" imports and replace them with "bg-color-agent") to
ensure consistency and avoid broken imports.
| import { createTool } from "@mastra/client-js"; | ||
| import z from "zod"; | ||
|
|
||
| export function changeBgColor(color: string) { | ||
| if (typeof window !== "undefined") { | ||
| document.body.style.setProperty("--background", color); | ||
| } | ||
| } | ||
|
|
||
| export const colorChangeTool = createTool({ | ||
| id: "changeColor", | ||
| description: "Changes the background color", | ||
| inputSchema: z.object({ | ||
| color: z.string(), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add TSDoc documentation for exported entities.
Per coding guidelines, public APIs should have TSDoc comments including parameter descriptions and purpose.
+/**
+ * Updates the application's background color by setting a CSS custom property.
+ * @param color - The color value to apply (any valid CSS color format)
+ */
export function changeBgColor(color: string) {
if (typeof window !== "undefined") {
document.body.style.setProperty("--background", color);
}
}
+/**
+ * Tool for changing the application's background color.
+ * Accepts a color string and applies it to the --background CSS variable.
+ */
export const colorChangeTool = createTool({📝 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.
| import { createTool } from "@mastra/client-js"; | |
| import z from "zod"; | |
| export function changeBgColor(color: string) { | |
| if (typeof window !== "undefined") { | |
| document.body.style.setProperty("--background", color); | |
| } | |
| } | |
| export const colorChangeTool = createTool({ | |
| id: "changeColor", | |
| description: "Changes the background color", | |
| inputSchema: z.object({ | |
| color: z.string(), | |
| }), | |
| }); | |
| /** | |
| * Updates the application's background color by setting a CSS custom property. | |
| * @param color - The color value to apply (any valid CSS color format) | |
| */ | |
| export function changeBgColor(color: string) { | |
| if (typeof window !== "undefined") { | |
| document.body.style.setProperty("--background", color); | |
| } | |
| } | |
| /** | |
| * Tool for changing the application's background color. | |
| * Accepts a color string and applies it to the --background CSS variable. | |
| */ | |
| export const colorChangeTool = createTool({ | |
| id: "changeColor", | |
| description: "Changes the background color", | |
| inputSchema: z.object({ | |
| color: z.string(), | |
| }), | |
| }); |
🤖 Prompt for AI Agents
In src/mastra/color-change-tool.ts around lines 1 to 16, the exported function
changeBgColor and the exported constant colorChangeTool lack TSDoc comments; add
TSDoc above changeBgColor describing its purpose, the color parameter (type and
expected format), and any return/side-effects (mutates document body CSS
variable), and add TSDoc above colorChangeTool describing the tool's purpose and
the expected input schema (color string) so both public APIs have proper
documentation per guidelines.
| export function changeBgColor(color: string) { | ||
| if (typeof window !== "undefined") { | ||
| document.body.style.setProperty("--background", color); | ||
| } | ||
| } |
There was a problem hiding this comment.
CSS variable may not propagate correctly.
Setting --background on document.body.style may not work as expected if the CSS variable is consumed from :root or html. Also, this function is exported but not connected to the tool's execution.
Consider using document.documentElement for :root-level variables:
export function changeBgColor(color: string) {
if (typeof window !== "undefined") {
- document.body.style.setProperty("--background", color);
+ document.documentElement.style.setProperty("--background", color);
}
}📝 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.
| export function changeBgColor(color: string) { | |
| if (typeof window !== "undefined") { | |
| document.body.style.setProperty("--background", color); | |
| } | |
| } | |
| export function changeBgColor(color: string) { | |
| if (typeof window !== "undefined") { | |
| document.documentElement.style.setProperty("--background", color); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/mastra/color-change-tool.ts around lines 4 to 8, the function sets the
CSS variable on document.body which may not affect variables defined on
:root/html and the exported function is not connected to the tool execution;
change the implementation to set the property on document.documentElement (so it
affects :root-level variables) and ensure the export is actually used by wiring
this function into the tool execution path (either import and call it where the
tool initializes or export as default/attach to the tool API so the color change
runs when intended).
| await writer?.custom({ | ||
| type: 'data-workflow-topic-progress', | ||
| data: { | ||
| topic: inputData.topic, | ||
| status: 'started', | ||
| timestamp: Date.now(), | ||
| }, | ||
| id: 'research-topic-item', | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Topic progress events: make percent/status/timestamp fields consistent
Within data-workflow-topic-progress, you mix status: 'started'|'researching'|'completed' with percent: 30|100 and sometimes timestamp. If downstream UI expects a uniform contract, this will be brittle. Recommend consistently including { status, percent/statusPercent, stage, timestamp } (and keep percent as a percentage-string if aligning to the workflow guideline).
Also applies to: 149-157, 210-219, 235-243
🤖 Prompt for AI Agents
In src/mastra/workflows/research-synthesis-workflow.ts around lines 130-138 (and
similarly at 149-157, 210-219, 235-243), the emitted
data-workflow-topic-progress events are inconsistent (some send status only,
others percent or timestamp), so update each writer.custom call to always
include a uniform payload with keys: status (one of
'started'|'researching'|'completed'), stage (a short descriptor), percent or
statusPercent as a percentage string (e.g. "30%") to follow the workflow
guideline, and timestamp (Date.now()); ensure the payload uses the same field
names across all four occurrences and convert numeric percents to
percentage-strings before sending.
| await writer?.custom({ | ||
| type: 'data-workflow-step-start', | ||
| data: { | ||
| id: 'synthesize-research', | ||
| timestamp: Date.now(), | ||
| }, | ||
| id: 'synthesize-research', | ||
| }); |
There was a problem hiding this comment.
data-workflow-step-start uses id inside data (should be stepId, and match other steps)
This emits data: { id: 'synthesize-research', timestamp } while other steps use stepId. Standardize naming to avoid silent consumer bugs.
data: {
- id: 'synthesize-research',
+ stepId: 'synthesize-research',
+ stage: 'workflow',
timestamp: Date.now(),
},📝 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?.custom({ | |
| type: 'data-workflow-step-start', | |
| data: { | |
| id: 'synthesize-research', | |
| timestamp: Date.now(), | |
| }, | |
| id: 'synthesize-research', | |
| }); | |
| await writer?.custom({ | |
| type: 'data-workflow-step-start', | |
| data: { | |
| stepId: 'synthesize-research', | |
| stage: 'workflow', | |
| timestamp: Date.now(), | |
| }, | |
| id: 'synthesize-research', | |
| }); |
🤖 Prompt for AI Agents
In src/mastra/workflows/research-synthesis-workflow.ts around lines 268 to 275,
the emitted event uses data.id instead of data.stepId which is inconsistent with
other steps; change the payload to use data: { stepId: 'synthesize-research',
timestamp: Date.now() } (keeping the outer id field as-is if required) so the
field name matches other workflow steps and consumers.
| await writer?.custom({ | ||
| type: 'data-workflow-progress', | ||
| data: { | ||
| percent: 20, | ||
| message: 'Synthesizing research across topics...', | ||
| }, | ||
| id: 'synthesize-research', | ||
| }); |
There was a problem hiding this comment.
Progress events should use percentage strings ('20%', '50%', '90%') per workflow guidelines
These use percent: 20|50|90 as numbers. Align to the standardized writer pattern your guidelines require.
data: {
- percent: 20,
+ status: '20%',
+ stage: 'workflow',
+ timestamp: Date.now(),
message: 'Synthesizing research across topics...',
},Also applies to: 300-307, 383-390
🤖 Prompt for AI Agents
In src/mastra/workflows/research-synthesis-workflow.ts around lines 286-293 (and
also update occurrences at 300-307 and 383-390), the writer custom progress
events use numeric percent values (e.g., 20, 50, 90) but must follow the
workflow guideline of using percentage strings; change the percent fields to
string values with a trailing percent sign (e.g., '20%', '50%', '90%') for each
writer?.custom call, preserving the rest of the payload and IDs.
| await writer?.custom({ | ||
| type: 'data-workflow-step-complete', | ||
| data: { | ||
| type: 'step-complete', | ||
| stepId: 'synthesize-research', | ||
| success: true, | ||
| duration: Date.now() - startTime, | ||
| }, | ||
| id: 'synthesize-research', | ||
| }); |
There was a problem hiding this comment.
Avoid conflicting/extra type fields inside data for step-complete; add stage/timestamp consistently
You already have type: 'data-workflow-step-complete' at the top-level, but inside data you set type: 'step-complete' (and other steps don’t). That’s easy for consumers to misinterpret. Keep one “type source of truth” (the top-level type).
Also consider adding stage/timestamp consistently to step-complete and step-error.
Also applies to: 431-438
🤖 Prompt for AI Agents
In src/mastra/workflows/research-synthesis-workflow.ts around lines 412-421 (and
also apply the same change to 431-438), the emitted event uses a conflicting
inner data.type ('step-complete') while the top-level type is
'data-workflow-step-complete'; remove the redundant/contradictory data.type
field so the top-level type is the single source of truth, and add consistent
stage and timestamp fields inside data (e.g., stage: 'synthesize-research',
timestamp: Date.now()) for both step-complete and step-error emissions so
consumers get uniform metadata.
| await writer?.custom({ | ||
| type: 'data-workflow-step-start', | ||
| data: { | ||
| stepId: 'generate-research-report', | ||
| timestamp: Date.now(), | ||
| }, | ||
| id: 'generate-research-report', | ||
| }); |
There was a problem hiding this comment.
Generate-report step: same event schema inconsistencies (percent vs status; inconsistent stage/timestamp; inner type)
This step repeats the same schema problems:
- progress uses numeric
percent(20/50/90) while other places usestatus: '50%' stageis only present on one progress and the error event- step-complete includes inner
type: 'step-complete'
Recommend fixing here in the same way as synthesize-research to keep all workflow steps uniform.
Also applies to: 471-478, 484-491, 552-560, 578-587, 597-605
🤖 Prompt for AI Agents
In src/mastra/workflows/research-synthesis-workflow.ts around lines 454-461 (and
also apply to ranges 471-478, 484-491, 552-560, 578-587, 597-605), the events
emitted for the generate-report step use inconsistent schema (numeric percent vs
string status, missing stage on some progress events, and an extra inner type on
step-complete). Change all progress events to use a consistent status string
like "50%" instead of numeric percent, include a stage property consistently on
all progress and error events, and remove the redundant inner type field from
the step-complete payload so the event shape matches the synthesize-research
step schema used elsewhere.
There was a problem hiding this comment.
Pull request overview
This PR enhances progress reporting across workflows and tools by standardizing the event structure with explicit status, stage, and id fields. The changes improve consistency in how progress updates are communicated throughout the application, making it easier to track execution states. Additionally, new GitHub tools for listing commits and creating releases are introduced, along with a background color agent and corresponding tool.
Key Changes
- Standardized progress reporting format across all workflows and tools with structured
status,stage, andidfields - Added new GitHub integration tools (
listCommits,createRelease) with improved type safety - Introduced background color agent and color change tool for UI customization
Reviewed changes
Copilot reviewed 41 out of 42 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mastra/workflows/weather-workflow.ts | Added id field to progress reporting calls for consistency |
| src/mastra/workflows/spec-generation-workflow.ts | Changed logical OR operator to nullish coalescing for JSON extraction |
| src/mastra/workflows/research-synthesis-workflow.ts | Refactored progress reporting to use structured custom() calls with data wrapper |
| src/mastra/workflows/document-processing-workflow.ts | Standardized progress reporting and added unused import |
| src/mastra/tools/*.ts | Updated 20+ tools to use standardized progress reporting format |
| src/mastra/tools/github.ts | Added retry plugin, new tools, and improved type safety |
| src/mastra/tools/AGENTS.md | Added documentation for progress event format |
| src/mastra/color-change-tool.ts | New tool for changing background color |
| src/mastra/agents/bgColorAgent.ts | New agent for color conversion and background changes |
| package.json | Added @octokit/plugin-retry dependency |
Comments suppressed due to low confidence (2)
src/mastra/workflows/document-processing-workflow.ts:1
- Unused import
mainfrom '../../../lib/auth-dev' should be removed as it's not referenced anywhere in the file.
src/mastra/workflows/document-processing-workflow.ts:1 - The data value 'input-convert-pdf-to-markdown' is inconsistent with other workflow step identifiers in the same file which use the format 'convert-pdf-to-markdown' without the 'input-' prefix.
| // approvedBy: sam | ||
| // approvalDate: 9/22 | ||
| import type { RequestContext } from '@mastra/core/request-context'; | ||
| //import type { RequestContext } from '@mastra/core/request-context'; |
There was a problem hiding this comment.
Commented-out import should be removed if it's no longer needed. If it's needed for future use, add a TODO comment explaining why it's commented out.
| //import type { RequestContext } from '@mastra/core/request-context'; |
| }), | ||
| execute: async (inputData, context) => { | ||
| await context?.writer?.custom({ type: 'data-tool-progress', data: { message: '🧠 Extracting learnings from search result' } }); | ||
| await context?.writer?.custom({ type: 'data-tool-agent', data: { message: '🧠 Extracting learnings from search result' }, id: 'extract-learnings' }); |
There was a problem hiding this comment.
This tool uses 'data-tool-agent' as the event type instead of 'data-tool-progress' used by other tools. According to the documentation in AGENTS.md, all tools should emit progress events with type 'data-tool-progress' for consistency.
| }), | ||
| execute: async (inputData, context) => { | ||
| await context?.writer?.custom({ type: 'data-tool-progress', data: { message: '📝 Starting editor agent' } }); | ||
| await context?.writer?.custom({ type: 'data-tool-agent', data: { message: '📝 Starting editor agent' }, id: 'editor-agent' }); |
There was a problem hiding this comment.
This tool uses 'data-tool-agent' as the event type instead of 'data-tool-progress'. According to the documentation in AGENTS.md, all tools should use 'data-tool-progress' with status and stage fields for consistency.
| } = inputData | ||
|
|
||
| await writer?.custom({ type: 'data-tool-progress', data: { message: `✍️ Starting copywriter agent for ${contentType} about "${topic}"` } }); | ||
| await writer?.custom({ type: 'data-tool-progress', data: { message: `✍️ Starting copywriter agent for ${contentType} about "${topic}"` }, id: 'copywriter-agent' }); |
There was a problem hiding this comment.
Missing required status and stage fields in the progress event data. According to the documentation in AGENTS.md, all progress events should include these fields for consistency.
| const token = process.env.GITHUB_API_KEY ?? process.env.GITHUB_PERSONAL_ACCESS_TOKEN; | ||
| return new Octokit({ auth: token }); | ||
| // Set retry configuration with a short retryAfter and maximum retries for github calls | ||
| return new OctokitWithRetry({ auth: token, request: { retries: 3, retryAfter: 60 } }); |
There was a problem hiding this comment.
The retryAfter value of 60 seconds seems quite long for an API retry configuration. This could lead to significant delays during transient failures. Consider using a smaller value (e.g., 1-5 seconds) or implementing exponential backoff.
| return new OctokitWithRetry({ auth: token, request: { retries: 3, retryAfter: 60 } }); | |
| return new OctokitWithRetry({ auth: token, request: { retries: 3, retryAfter: 3 } }); |
| description: "Changes the background color", | ||
| inputSchema: z.object({ | ||
| color: z.string(), | ||
| }), |
There was a problem hiding this comment.
The tool is missing an execute function that actually calls changeBgColor. The tool definition should include an execute function that takes the input and invokes the color change functionality.
| }), | |
| }), | |
| execute: ({ color }: { color: string }) => changeBgColor(color), |
| @@ -0,0 +1,18 @@ | |||
| import { Agent } from "@mastra/core/agent"; | |||
| import { Memory } from "@mastra/memory"; | |||
| import { colorChangeTool } from "../tools/color-change-tool"; | |||
There was a problem hiding this comment.
The import path '../tools/color-change-tool' is inconsistent. The file is actually located at '../color-change-tool' (not in a tools subdirectory based on the PR diff).
| import { colorChangeTool } from "../tools/color-change-tool"; | |
| import { colorChangeTool } from "../color-change-tool"; |
