Repository navigation
Conversation
- Created `url-tool.test.ts` to cover validation, parsing, normalization, extraction, and manipulation of URLs with multiple operations and tracing. - Implemented tests for success paths, error handling, and lifecycle hooks in `write-note.test.ts`, ensuring proper file operations and tracing context. - Mocked `fs` module for file operations in the write-note tool tests. - Added detailed progress tracking and error handling in both test files. feat: implement financial analysis workflow - Developed `financial-analysis-workflow.ts` to integrate financial data fetching, technical analysis, and report generation. - Utilized Zod for input and output schema validation, ensuring robust data handling. - Included logging for step progress and error handling, enhancing observability. docs: update session notes for continuity - Documented progress and key decisions in `CONTINUITY_ses_450b.md` regarding test coverage for Mastra tools. - Summarized goals, constraints, and next steps for integrating GitHub tools in `CONTINUITY_ses_4519.md`.
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @ssdeanx, your pull request is larger than the review limit of 150000 diff characters
|
🤖 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 by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis pull request introduces a major expansion of Mastra's agent ecosystem with five new specialized agents (Academic Research, Data Processing, Financial Analysis, Web Research), a centralized HTTP client with rate limiting and retry capabilities, comprehensive observability/tracing standardization across tools and workflows, and extensive test coverage. Package dependencies are updated, and the core Mastra initialization is restructured to support richer configuration including workflows, agents, storage, vectors, and observability pipelines. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes This PR introduces substantial new functionality (5 agents, HTTP client, workflow enhancements), standardizes tracing across 40+ tool files with heterogeneous implementation patterns, adds 17+ test files with varying mock strategies, and restructures core Mastra initialization. The changes span multiple domains (observability, HTTP, agents, workflows, tools) with dense logic in agent configurations, tool lifecycle management, and test setup. While individual tool tracing additions are repetitive, the variety in implementation depth, handler signature changes, and test patterns demands careful review. Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
🤖 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 project's stability and expandability by introducing comprehensive test coverage for numerous tools, standardizing external API interactions through a new robust HTTP client, and enforcing consistent observability practices. It also broadens the system's functional scope by adding several specialized agents and a new financial analysis workflow, laying a stronger foundation for future development and integration. 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.
Pull request overview
This pull request adds comprehensive test coverage for Mastra tools and implements a new financial analysis workflow. The changes focus on expanding test coverage, refactoring workflows to use updated tracing patterns, and improving type safety across tool implementations.
Changes:
- Added comprehensive test files for
url-toolandwrite-notetool with coverage for validation, manipulation, error handling, and tracing - Implemented
financial-analysis-workflow.tsintegrating financial data fetching, technical analysis, and report generation - Refactored
weather-workflow.tsandstock-analysis-workflow.tsto use new tracing patterns withSpanTypeand updated context handling - Updated
web-scraper-tool.tswith improved type safety, better JSDOM/Crawlee compatibility, and enhanced error handling - Added type annotations (
TracingContext) across multiple tool files for better type safety - Created test configuration files (
serpapi-config.test.ts,semantic-utils.test.ts) validating type definitions and constants
Reviewed changes
Copilot reviewed 64 out of 66 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| thoughts/ledgers/CONTINUITY_ses_4519.md | Documents progress on financial analysis workflow implementation |
| thoughts/ledgers/CONTINUITY_ses_450b.md | Tracks test coverage improvements for Mastra tools |
| tests/test-results/test-results.json | Updates test results showing improved pass rate (227/260 tests passing) |
| src/mastra/workflows/financial-analysis-workflow.ts | New workflow implementing financial data fetching, technical analysis, and reporting |
| src/mastra/workflows/weather-workflow.ts | Refactored to use updated tracing context patterns |
| src/mastra/workflows/stock-analysis-workflow.ts | Refactored with improved type safety and tracing |
| src/mastra/tools/web-scraper-tool.ts | Enhanced type safety, JSDOM/Crawlee compatibility, httpFetch integration |
| src/mastra/tools/tests/url-tool.test.ts | Comprehensive tests for URL validation and manipulation |
| src/mastra/tools/tests/write-note.test.ts | Detailed tests for write-note tool with tracing and error handling |
| src/mastra/tools/tests/write-note.tool.test.ts | Additional write-note tests with fs mocking |
| src/mastra/tools/weather-tool.ts | Added TracingContext type annotation |
| src/mastra/tools/tests/*.test.ts | Multiple test files updated with improved mocking and assertions |
| src/mastra/tools/*.ts | Added TracingContext type imports for better type safety |
| import { | ||
| alphaVantageCryptoTool, | ||
| alphaVantageStockTool, | ||
| } from '../tools/alpha-vantage.tool' |
There was a problem hiding this comment.
Unused imports alphaVantageCryptoTool, alphaVantageStockTool.
| import { | |
| alphaVantageCryptoTool, | |
| alphaVantageStockTool, | |
| } from '../tools/alpha-vantage.tool' |
| polygonCryptoAggregatesTool, | ||
| polygonCryptoQuotesTool, | ||
| polygonCryptoSnapshotsTool, | ||
| polygonStockAggregatesTool, | ||
| polygonStockFundamentalsTool, |
There was a problem hiding this comment.
Unused imports polygonCryptoAggregatesTool, polygonCryptoQuotesTool, polygonCryptoSnapshotsTool, polygonStockAggregatesTool, polygonStockFundamentalsTool.
| polygonCryptoAggregatesTool, | |
| polygonCryptoQuotesTool, | |
| polygonCryptoSnapshotsTool, | |
| polygonStockAggregatesTool, | |
| polygonStockFundamentalsTool, |
| batchWebScraperTool, | ||
| contentCleanerTool, | ||
| htmlToMarkdownTool, | ||
| linkExtractorTool, | ||
| scrapingSchedulerTool, | ||
| siteMapExtractorTool, |
There was a problem hiding this comment.
Unused imports batchWebScraperTool, contentCleanerTool, linkExtractorTool, scrapingSchedulerTool, siteMapExtractorTool.
| batchWebScraperTool, | |
| contentCleanerTool, | |
| htmlToMarkdownTool, | |
| linkExtractorTool, | |
| scrapingSchedulerTool, | |
| siteMapExtractorTool, | |
| htmlToMarkdownTool, |
| import { | ||
| createCompletenessScorer, | ||
| createTextualDifferenceScorer, | ||
| createToneScorer, | ||
| } from './evals/scorers/prebuilt' |
There was a problem hiding this comment.
Unused imports createCompletenessScorer, createTextualDifferenceScorer, createToneScorer.
| import { | |
| createCompletenessScorer, | |
| createTextualDifferenceScorer, | |
| createToneScorer, | |
| } from './evals/scorers/prebuilt' |
| logStepStart, | ||
| logToolExecution, | ||
| } from '../config/logger' | ||
| import { httpFetch } from '../lib/http-client' |
There was a problem hiding this comment.
Unused import httpFetch.
| } else { | ||
| data = await response.text() | ||
| } | ||
| const contentType = String(resp.headers['content-type'] ?? '') |
There was a problem hiding this comment.
Unused variable contentType.
| generatedAt: z.string(), | ||
| }) | ||
|
|
||
| const fetchFinancialDataStep = createStep({ |
There was a problem hiding this comment.
Unused variable fetchFinancialDataStep.
| }, | ||
| }) | ||
|
|
||
| const performTechnicalAnalysisStep = createStep({ |
There was a problem hiding this comment.
Unused variable performTechnicalAnalysisStep.
| }, | ||
| }) | ||
|
|
||
| const generateFinancialReportStep = createStep({ |
There was a problem hiding this comment.
Unused variable generateFinancialReportStep.
| import { trace, SpanStatusCode } from '@opentelemetry/api'; | ||
| import { createStep, createWorkflow } from '@mastra/core/workflows' | ||
| import { z } from 'zod' | ||
| import { log } from '../config/logger' |
There was a problem hiding this comment.
Unused import log.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 104
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (40)
src/mastra/tools/editor-agent-tool.ts (1)
86-106: Missing abort signal check.Per coding guidelines, tools should respect cancellation signals by checking
abortSignal?.abortedearly in the execute function and before long-running operations. The agent streaming/generation calls on lines 156-200 could benefit from early cancellation.♻️ Suggested implementation
execute: async (inputData, context) => { + // Respect cancellation signals early + if (context?.abortSignal?.aborted) { + throw new Error('Operation aborted') + } + // Emit progress start await context?.writer?.custom({Additionally, check before the agent call:
+ // Check abort signal before long-running agent operation + if (context?.abortSignal?.aborted) { + throw new Error('Operation aborted') + } + const agent = context?.mastra?.getAgent?.('editorAgent') ?? editorAgentsrc/mastra/tools/tests/write-note.tool.test.ts (3)
1-2: Remove@ts-nocheckand use proper type handling.Disabling TypeScript checking defeats the purpose of having type-safe tests. If there are type errors, they should be fixed by properly typing the mocks or using
vi.mocked(). Additionally,afterEachis imported but never used—per coding guidelines, mocks should be restored inafterEachto prevent test pollution.♻️ Suggested fix
-// @ts-nocheck -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'Then add an
afterEachblock:afterEach(() => { vi.restoreAllMocks() })
26-41: Missing tracing context verification.Per coding guidelines, tool tests should verify tracing span creation by mocking
tracingContextwithcurrentSpan.createChildSpanand asserting the span was created withtype: 'tool_call'and the appropriate tool name. ThewriteNoteToolcreates a child span named'write-note'that should be verified.🧪 Suggested test enhancement
it('should write a note successfully with tracing', async () => { const mockWriter = createMockWriter() const mockSpan = { update: vi.fn(), end: vi.fn(), error: vi.fn(), } const mockTracingContext = { currentSpan: { createChildSpan: vi.fn().mockReturnValue(mockSpan), }, } const res = await writeNoteTool.execute( { title: 'test-note', content: '# Hello' }, { writer: mockWriter, tracingContext: mockTracingContext } ) expect(mockTracingContext.currentSpan.createChildSpan).toHaveBeenCalledWith( expect.objectContaining({ type: 'tool_call', name: 'write-note', }) ) expect(mockSpan.update).toHaveBeenCalled() expect(mockSpan.end).toHaveBeenCalled() expect(res).toContain('Successfully wrote to note') })
43-58: Test coverage is insufficient for 97% target.Per coding guidelines, tool tests must achieve 97% coverage including "successful execution, error handling, and context extraction." The current tests are missing:
- Abort signal handling — The tool throws when
abortSignal?.abortedis true- Tracing error path —
noteSpan?.error()should be verified on failure- Lifecycle hooks —
onInputStart,onInputDelta,onInputAvailable,onOutputare not tested- RequestContext extraction —
userIdandworkspaceIdfromrequestContextare not tested🧪 Suggested additional tests
it('should throw when abort signal is already aborted', async () => { const mockWriter = createMockWriter() const abortController = new AbortController() abortController.abort() await expect( writeNoteTool.execute( { title: 'test', content: 'content' }, { writer: mockWriter, abortSignal: abortController.signal } ) ).rejects.toThrow('Write note operation cancelled') }) it('should record error in tracing span on failure', async () => { const mockWriter = createMockWriter() const mockSpan = { update: vi.fn(), end: vi.fn(), error: vi.fn(), } const mockTracingContext = { currentSpan: { createChildSpan: vi.fn().mockReturnValue(mockSpan), }, } vi.mocked(fs.writeFile).mockRejectedValueOnce(new Error('disk full')) await expect( writeNoteTool.execute( { title: 'bad', content: 'x' }, { writer: mockWriter, tracingContext: mockTracingContext } ) ).rejects.toThrow(/disk full/) expect(mockSpan.error).toHaveBeenCalledWith( expect.objectContaining({ endSpan: true }) ) }) it('should call onInputAvailable lifecycle hook', () => { const mockMessages = [{ role: 'user', content: 'test' }] writeNoteTool.onInputAvailable?.({ input: { title: 'test', content: 'content' }, toolCallId: 'test-call-id', messages: mockMessages, abortSignal: undefined, }) expect(log.info).toHaveBeenCalledWith( 'writeNoteTool received input', expect.objectContaining({ hook: 'onInputAvailable' }) ) })src/mastra/tools/copywriter-agent-tool.ts (3)
154-156: Logic issue: condition is always true.
userIdis assigned'anonymous'as a fallback on lines 82-85, souserId !== undefinedwill always evaluate totrue. This means "User: anonymous" will always be appended to the prompt.If the intent is to only include user information when a real user ID is available:
Proposed fix
- if (userId !== undefined) { + if (userId !== 'anonymous') { prompt += `\n\nUser: ${userId}` }
202-204: Unimplemented enum value causes runtime error.The
'general'content type is defined as a valid option ininputSchema(line 31), but selecting it throws an error at runtime. This creates a poor user experience where a seemingly valid option fails unexpectedly.Either implement the
'general'case or remove it from the schema:Option 1: Implement the case
case 'general': { - throw new Error('Not implemented yet: "general" case') + prompt += + '\n\nCreate well-written, informative content tailored to the topic.' + break }Option 2: Remove from schema until implemented
contentType: z .enum([ 'blog', 'marketing', 'social', 'technical', 'business', 'creative', - 'general', ])
420-433: Consider truncating or omitting full content from logs.The
onOutputhook logs the entireoutput.content, which could be substantial (especially for'long'content) and might inadvertently include sensitive information depending on the generated content.Consider logging only metadata or a truncated preview:
Suggested improvement
onOutput: ({ output, toolCallId, toolName, abortSignal }) => { log.info('copywriterTool completed', { toolCallId, toolName, outputData: { - content: output.content, + contentLength: output.content?.length, + contentPreview: output.content?.substring(0, 100), title: output.title, summary: output.summary, keyPoints: output.keyPoints, wordCount: output.wordCount, }, abortSignal: abortSignal?.aborted, hook: 'onOutput', }) },src/mastra/tools/tests/web-scraper-tool.test.ts (6)
1-1: Avoid@ts-nocheck- fix type errors instead.Disabling TypeScript checking entirely masks potential type issues and defeats the purpose of type safety. Per coding guidelines, prefer using proper type definitions or
unknownover suppressing type checking.Consider fixing the specific type errors (likely in mock definitions) or using targeted
@ts-expect-errorcomments with explanations for unavoidable issues.
301-322: Duplicateexecutecall - remove the first invocation.The test calls
webScraperTool.executetwice with identical arguments. The first call (lines 308-312) discards its result, making it redundant.🔧 Proposed fix
try { - await webScraperTool.execute(input, { - requestContext: mockRequestContext, - tracingContext: mockTracingContext, - writer: mockWriter, - }) const res = await webScraperTool.execute(input, { requestContext: mockRequestContext, tracingContext: mockTracingContext, writer: mockWriter, }) expect(res).toHaveProperty('error', true)
396-399: Remove tautological assertion.
expect(true).toBe(true)always passes and verifies nothing. If the intent is to ensure no crash occurs, the test passing without throwing is sufficient - no assertion needed.♻️ Proposed fix
} else { // If tracing isn't wired in test env, ensure no crash - expect(true).toBe(true) }
461-479: Test does not verify HTML sanitization.The test name claims to verify sanitization of dangerous HTML elements, but it only checks that
rawContentandmarkdownContentare defined. The mock HTML contains no dangerous elements (<script>,<iframe>,onclick, etc.) and no sanitization behavior is asserted.Consider:
- Injecting dangerous HTML in the mock (e.g.,
<script>alert('xss')</script>)- Asserting the output does not contain the dangerous elements
💡 Example improvement
it('should sanitize dangerous HTML elements', async () => { // Override mock to return dangerous HTML const dangerousHtml = '<html><body><script>alert("xss")</script><h1>Safe</h1></body></html>' // ... setup mock to return dangerousHtml ... const result = await webScraperTool.execute(input, { /* ... */ }) expect(result.content.rawContent).not.toContain('<script>') expect(result.content.markdownContent).not.toContain('alert') })
481-497: Test does not verify lifecycle hooks.The test claims to verify lifecycle hooks but only asserts that
log.infois a function (which is trivially true). Per coding guidelines, tools should implementonInputStart,onInputDelta,onInputAvailable, andonOutputhooks.Consider asserting that logging or hook-related behavior occurred:
💡 Example improvement
it('should call lifecycle hooks appropriately', async () => { const input = { url: 'https://example.com/hooks-test' } await webScraperTool.execute(input, { requestContext: mockRequestContext, tracingContext: mockTracingContext, writer: mockWriter, }) // Verify lifecycle logging occurred expect(log.info).toHaveBeenCalledWith( expect.stringMatching(/onInputAvailable|lifecycle/i), expect.any(Object) ) })
402-424: Test setup accesses mock results before execution.Lines 403-411 attempt to configure
spanMock.updateto throw, butmockTracingContext.currentSpan.createChildSpan.mock.results[0]is accessed beforeexecuteis called in this test. Sincevi.clearAllMocks()runs inbeforeEach,results[0]will beundefined, causing the else branch to silently skip the setup.To properly test tracing error handling, configure the mock to throw before execution:
🔧 Proposed fix
it('should handle tracing errors gracefully', async () => { - const spanCall = mockTracingContext.currentSpan.createChildSpan.mock.results[0] - if (spanCall?.value) { - const spanMock = spanCall.value - spanMock.update.mockImplementation(() => { - throw new Error('Tracing error') - }) - } else { - // no tracing span available; nothing to simulate - } + // Configure the mock to return a span that throws on update + mockTracingContext.currentSpan.createChildSpan.mockReturnValue({ + createChildSpan: vi.fn().mockReturnValue({ + update: vi.fn(), + end: vi.fn(), + error: vi.fn(), + }), + update: vi.fn().mockImplementation(() => { + throw new Error('Tracing error') + }), + end: vi.fn(), + error: vi.fn(), + }) const input = { url: 'https://example.com/trace-error-test', }src/mastra/tools/git-local.tool.ts (3)
306-542: Missing lifecycle hooks for consistent observability.
gitDiffToolis missing the required lifecycle hooks (onInputStart,onInputDelta,onInputAvailable,onOutput) that are present ingitStatusTool. The same applies togitCommitTool,gitLogTool,gitBranchTool,gitStashTool, andgitConfigTool.As per coding guidelines, all tools must implement complete lifecycle hooks for consistent observability and logging.
♻️ Example lifecycle hooks to add after execute
onInputStart: ({ toolCallId, messages, abortSignal }) => { log.info('Git diff tool input streaming started', { toolCallId, messageCount: messages.length, abortSignal: abortSignal?.aborted, hook: 'onInputStart', }) }, onInputDelta: ({ inputTextDelta, toolCallId, messages, abortSignal }) => { log.info('Git diff tool received input chunk', { toolCallId, inputTextDelta, messageCount: messages.length, abortSignal: abortSignal?.aborted, hook: 'onInputDelta', }) }, onInputAvailable: ({ input, toolCallId, messages, abortSignal }) => { log.info('Git diff tool received input', { toolCallId, messageCount: messages.length, inputData: { target: input.target, paths: input.paths }, abortSignal: abortSignal?.aborted, hook: 'onInputAvailable', }) }, onOutput: ({ output, toolCallId, toolName, abortSignal }) => { log.info('Git diff tool completed', { toolCallId, toolName, abortSignal: abortSignal?.aborted, outputData: { success: output.success, diffLength: output.diff.length }, hook: 'onOutput', }) },
1189-1189: Avoid usinganytype.Using
anybypasses TypeScript's type checking. Define a proper result type or use a union type based on the operation outcomes. The same issue exists ingitStashTool(line 1581) andgitConfigTool(line 1891).♻️ Proposed fix
- let result: any = {} + let result: { + branches?: Array<{ name: string; current: boolean; remote: boolean; lastCommit?: string; tracking?: string }>; + currentBranch?: string; + affectedCommits?: number; + conflicts?: string[]; + message?: string; + } = {}Alternatively, extract a type alias for reuse:
type GitBranchResult = Pick<z.infer<typeof gitBranchTool.outputSchema>, 'branches' | 'currentBranch' | 'affectedCommits' | 'conflicts' | 'message'>;
584-590: Unused variablerequestContext.
requestContexton line 586 is declared but never used. Line 588 separately castscontext?.requestContexttoGitToolContext. Consider removing the unused assignment.♻️ Proposed fix
execute: async (inputData, context) => { const writer = context?.writer - const requestContext = context?.requestContext const requestCtx = context?.requestContext as GitToolContext | undefined const allowAmend = requestCtx?.allowAmend ?? falsesrc/mastra/tools/csv-to-json.tool.ts (2)
17-50: MissingoutputSchemadefinition.The tool returns
{ data: records }but lacks anoutputSchema. Per coding guidelines, all tool definitions must include bothinputSchemaandoutputSchemausing Zod with appropriate descriptions for agent decision-making.Proposed fix: Add outputSchema
inputSchema: z.object({ csvData: z.string().optional().describe('Raw CSV string data'), filePath: z.string().optional().describe('Absolute path to a CSV file'), options: z .object({ delimiter: z .string() .default(',') .describe('CSV delimiter character'), columns: z .boolean() .default(true) .describe('Treat first row as headers'), trim: z .boolean() .default(true) .describe('Trim whitespace from values'), skip_empty_lines: z .boolean() .default(true) .describe('Skip empty lines'), }) .optional() .default({ delimiter: ',', columns: true, trim: true, skip_empty_lines: true, }), }), + outputSchema: z.object({ + data: z.array(z.record(z.string(), z.unknown())).describe('Parsed CSV records as JSON array'), + }), execute: async (input, context) => {
145-148: Inconsistent progress event format.This uses
writer?.writewithtype: 'progress'while other progress events in this tool (lines 62-70, 172-180) usewriter?.customwithtype: 'data-tool-progress'. Per coding guidelines, all tools must emit progress events with the format:type: 'data-tool-progress'.Proposed fix
- await writer?.write({ - type: 'progress', - data: { message: `✅ Converted ${records.length} records` }, - }) + await writer?.custom({ + type: 'data-tool-progress', + data: { + status: 'done', + message: `✅ Converted ${records.length} records`, + stage: 'csv-to-json', + }, + id: 'csv-to-json', + })src/mastra/tools/code-search.tool.ts (1)
287-289: Silent catch block hides errors.This catch block silently ignores all errors when reading files. While the comment says "Skip binary or unreadable files," this also hides permission errors, encoding issues, or other unexpected failures that may be worth logging for debugging purposes.
Proposed fix: Add debug logging
} catch { - // Skip binary or unreadable files + // Skip binary or unreadable files - log at debug level for troubleshooting + log.debug(`Skipped file ${filePath}: unreadable or binary`) }src/mastra/tools/image-tool.ts (3)
61-63: Unused variablerequestCtx.The
requestCtxvariable is declared but never used in theocrTool.executefunction. Either use it (e.g., fordefaultLanguageormaxImageSizefromImageToolContext) or remove it.Proposed fix: Remove unused variable
- const requestCtx = context?.requestContext as - | ImageToolContext - | undefined
363-365: Incorrect null check for optional property.
ops.bluris typed asz.number().optional(), meaning it can beundefinedbut notnull. The conditionops.blur !== nullwill always be true whenblurisundefined, causingpipeline.blur(undefined)to be called.Proposed fix
- if (ops.blur !== null) { + if (ops.blur !== undefined) { pipeline = pipeline.blur(ops.blur) }
374-379: Format chaining logic is incorrect.When
ops.qualityis set withoutops.format, this code chains.jpeg().png().webp()on the pipeline. Sharp's chaining doesn't apply all formats - only the last call determines the output format. This will always output WebP regardless of the input format.Proposed fix: Preserve original format when only quality is specified
} else if (ops.quality) { - pipeline = pipeline - .jpeg({ quality: ops.quality }) - .png({ compressionLevel: 9 }) - .webp({ quality: ops.quality }) + // Apply quality settings based on the original format + const originalFormat = metadata.format + if (originalFormat === 'jpeg' || originalFormat === 'jpg') { + pipeline = pipeline.jpeg({ quality: ops.quality }) + } else if (originalFormat === 'png') { + pipeline = pipeline.png({ quality: Math.round(ops.quality / 10) }) + } else if (originalFormat === 'webp') { + pipeline = pipeline.webp({ quality: ops.quality }) + } }src/mastra/tools/code-chunking.ts (1)
130-145: Misleading empty else block.The comment
// Fallback to recursivesuggests fallback logic belongs here, but the block is empty and execution falls through to the MDocument fallback at line 199. This makes the control flow confusing.Proposed fix: Restructure for clarity
- if (chunks.length === 0 && content.trim().length > 0) { - // Fallback to recursive - } else { + if (chunks.length > 0) { await writer?.custom({ type: 'data-tool-progress', data: { status: 'done', message: `✅ Chunking complete: ${chunks.length} chunk${chunks.length !== 1 ? 's' : ''}`, stage: 'code-chunker', }, id: 'code-chunker', }) span?.update({ metadata: { 'tool.output.chunksCount': chunks.length } }) span?.end() return { chunks } } + // No structural chunks found - fall through to MDocument recursive chunkingsrc/mastra/tools/cytoscape.tool.ts (1)
208-234: Consider adding missing parameters to lifecycle hooks for consistency.Other tools in this PR (e.g.,
calculator.tool.ts) includemessagesandabortSignalparameters in lifecycle hooks for richer observability. These hooks are missing those parameters, which reduces logging detail.💡 Suggested enhancement for consistent observability
- onInputStart: ({ toolCallId }) => { + onInputStart: ({ toolCallId, messages, abortSignal }) => { log.info('Cytoscape generator tool input streaming started', { toolCallId, + messageCount: messages.length, + abortSignal: abortSignal?.aborted, hook: 'onInputStart', }) }, - onInputDelta: ({ inputTextDelta, toolCallId }) => { + onInputDelta: ({ inputTextDelta, toolCallId, messages, abortSignal }) => { log.info('Cytoscape generator tool received input chunk', { toolCallId, inputTextDelta, + messageCount: messages.length, + abortSignal: abortSignal?.aborted, hook: 'onInputDelta', }) },src/mastra/tools/tests/serpapi-shopping.tool.test.ts (3)
118-142: Undefined variableinputDatawill cause test failures.The variable
inputDatais defined locally inside the first test case (lines 80-87) but is referenced in subsequent tests on lines 121 and 133. This will cause aReferenceErrorwhen these tests run.🐛 Proposed fix: Move shared test data to describe scope or define inline
Option 1: Define inline in each test
it('should handle empty search results', async () => { mockGetJson.mockResolvedValue({ search_results: [] }) - const result = await amazonSearchTool.execute(inputData, { + const result = await amazonSearchTool.execute({ + query: 'wireless headphones', + sortBy: 'rating' as const, + primeOnly: true, + numResults: 10, + }, { writer: mockWriter, tracingContext: mockTracingContext, })Option 2: Move shared data to describe scope
describe('amazonSearchTool', () => { + const defaultInputData = { + query: 'wireless headphones', + sortBy: 'rating' as const, + minPrice: 50, + maxPrice: 200, + primeOnly: true, + numResults: 10, + } + it('should successfully search Amazon products', async () => { - const inputData = { - query: 'wireless headphones', ...
207-213: Fix inconsistent input structure in tool tests—three tools use incorrect wrapping.All tool
executesignatures expect(inputData, context)as parameters. TheamazonSearchTooltest correctly passesinputDatadirectly, butwalmartSearchTool,ebaySearchTool, andhomeDepotSearchTooltests incorrectly wrap it as{ context: inputData }. This mismatch means these three tests are not calling the tools correctly and could mask real bugs.Update the failing tests to match the
amazonSearchToolpattern:
- Line 207: Change
{ context: inputData }toinputData- Line 285: Change
{ context: inputData }toinputData- Line 364: Change
{ context: inputData }toinputData
26-28: Fix incorrect mock path for serpapi-config.The mock path should be
'../serpapi-config'instead of'./serpapi-config'. Since the test file is in thetests/subdirectory,./serpapi-configresolves relative to the test file and will not find the actualserpapi-config.tsmodule. The relative path must account for the test being one level deeper in the directory structure.Current mock
vi.mock('./serpapi-config', () => ({ validateSerpApiKey: vi.fn(), }))Change to:
vi.mock('../serpapi-config', () => ({ validateSerpApiKey: vi.fn(), }))docs/tracing-configuration-reference.md (1)
1-4: Missing YAML front matter as required by coding guidelines.Per the coding guidelines for
**/*.mdfiles, markdown files should include YAML front matter at the beginning with required metadata fields. This file lacks the YAML front matter block.Additionally, the coding guidelines specify not to use H1 headings as they "will be generated from the title." Consider changing Line 1 from
# Mastra Tracing Configuration Referenceto use the title via YAML front matter instead.📝 Suggested YAML front matter addition
+--- +post_title: Mastra Tracing Configuration Reference +author1: <author> +post_slug: tracing-configuration-reference +summary: Reference documentation for Mastra observability registry configuration +post_date: 2026-01-12 +--- + -# Mastra Tracing Configuration Reference +## OverviewBased on coding guidelines for markdown files.
src/mastra/tools/web-scraper-tool.ts (3)
702-737: Span lifecycle leak on early cancellation / allowlist failure (span created outside try/catch).
If you throw on Line 724-736,toolSpanwon’t be ended/errored.Proposed fix (move checks into the try so catch records + ends the span)
@@ - // Check if operation was already cancelled - if (abortSignal?.aborted === true) { - throw new Error('Web scraping cancelled') - } - - const allowedDomains = - requestContext?.allowedDomains ?? ValidationUtils.getAllowedDomains() - if (!ValidationUtils.isUrlAllowed(inputData.url, allowedDomains)) { - throw new ScrapingError( - `Domain is not allowlisted for scraping: ${inputData.url}`, - 'DOMAIN_NOT_ALLOWED', - undefined, - inputData.url - ) - } - await writer?.custom({ @@ - try { + try { + // Check if operation was already cancelled + if (abortSignal?.aborted === true) { + throw new Error('Web scraping cancelled') + } + + const allowedDomains = + requestContext?.allowedDomains ?? ValidationUtils.getAllowedDomains() + if (!ValidationUtils.isUrlAllowed(inputData.url, allowedDomains)) { + throw new ScrapingError( + `Domain is not allowlisted for scraping: ${inputData.url}`, + 'DOMAIN_NOT_ALLOWED', + undefined, + inputData.url + ) + }
421-425: Path traversal check is prefix-based and can be bypassed (e.g.,/data_evilstartsWith/data).
Usepath.relative()semantics instead ofstartsWith().Proposed fix
static validateFilePath(filePath: string, intendedDir: string): boolean { - const resolvedPath = path.resolve(filePath) - const resolvedIntendedDir = path.resolve(intendedDir) - return resolvedPath.startsWith(resolvedIntendedDir) + const resolvedPath = path.resolve(filePath) + const resolvedIntendedDir = path.resolve(intendedDir) + const rel = path.relative(resolvedIntendedDir, resolvedPath) + return rel !== '' && !rel.startsWith('..') && !path.isAbsolute(rel) }
3272-3337: Pass prepared headers to httpFetch; also add body/data support to HttpFetchInitThe tool prepares
headersandauthtokens (lines 3272–3290), but thehttpFetchcall (line 3297) does not pass them. Additionally, the schema acceptsbodyfor POST/PUT requests, butHttpFetchInitdoes not support abody/datafield andclient.requestnever receives it.This breaks authenticated APIs and non-GET requests with request bodies.
Required fixes:
- Add
data?: unknowntoHttpFetchInitinterface insrc/mastra/lib/http-client.ts- Pass
data: init?.datatoclient.requestinsrc/mastra/lib/http-client.ts- Pass both
headersandbodytohttpFetchinweb-scraper-tool.tsline 3297Proposed changes to web-scraper-tool.ts
- resp = await httpFetch(inputData.url, { method: inputData.method ?? 'GET', timeout: inputData.timeout ?? 30000, responseType: 'json', signal: context?.abortSignal }) + resp = await httpFetch(inputData.url, { method: inputData.method ?? 'GET', timeout: inputData.timeout ?? 30000, responseType: 'json', headers, data: inputData.body, signal: context?.abortSignal })src/mastra/agents/researchAgent.ts (1)
75-110: Tool Selection Guide in prompt doesn’t match wired tools; model return type is inconsistent.Proposed fix (align prompt + consistent model return)
@@ ## Tool Selection Guide - **Web**: 'webScraperTool' (single URL), 'batchWebScraperTool' (multiple). -- **News/Trends**: 'googleNewsTool', 'googleTrendsTool', 'googleFinanceTool'. +- **News/Trends**: 'googleNewsLiteTool', 'googleTrendsTool', 'googleFinanceTool'. - **Academic**: 'googleScholarTool'. -- **Financial**: Use 'polygon*', 'finnhub*', or 'alphaVantage*' for stocks/crypto. -- **Internal**: 'pgQueryTool' for previously indexed knowledge. +- **Financial**: Use the wired quote tools and google finance for lightweight checks. - **Processing**: 'pdfToMarkdownTool' for PDFs; 'evaluateResultTool' for quality checks. @@ if (userTier === 'enterprise') { // higher quality (chat style) for enterprise return google.chat('gemini-3-pro-preview') } else if (userTier === 'pro') { // cheaper/faster model for pro tier - return 'google/gemini-3-flash-preview' + return google.chat('gemini-3-flash-preview') } // cheaper/faster model for free tier return google.chat('gemini-3-flash-preview')In @mastra/core@1.0.0-beta.21, what types are allowed as the return value of the Agent `model` callback (string model id vs provider model object)?Also applies to: 122-137, 138-158
src/mastra/tools/e2b.ts (1)
249-278: Doublespan?.end()call in success path and finally block.The span is ended at line 250 on success, then again at line 277 in the
finallyblock. This same pattern appears inwriteFiles,listFiles, anddeleteFiletools. While typically harmless, it's inconsistent with other tools in this file (e.g.,createSandbox) that only callspan?.end()in thefinallyblock.♻️ Suggested fix
span?.update({ metadata: { 'operation.status': 'success', 'sandbox.id': inputData.sandboxId, 'file.path': inputData.path, 'file.size': inputData.content.length } }) - span?.end() const result = {Apply the same pattern to
writeFiles(line 362),listFiles(line 479), anddeleteFile(line 578).src/mastra/tools/tests/arxiv.tool.test.ts (1)
40-42: Consider adding more test coverage.The test suite only covers two scenarios. Per coding guidelines, tool tests should achieve 97% coverage including:
- Tracing span creation and metadata
- Context extraction
- Error handling for API failures
- Abort signal behavior
Would you like me to generate additional test cases to improve coverage?
src/mastra/tools/arxiv.tool.ts (2)
1024-1027: Inconsistent use of nativefetchinstead ofhttpFetch.The
arxivPaperDownloaderTooluses nativefetchfor PDF download (line 1025) while other HTTP calls in this file use the centralizedhttpFetch. This creates inconsistency in error handling and abort signal behavior.♻️ Use httpFetch for consistency
try { - const pdfResponse = await fetch(paperMetadata.pdf_url, { - signal: abortSignal, - }) - - if (pdfResponse.ok) { - const pdfBuffer = Buffer.from( - await pdfResponse.arrayBuffer() - ) + const pdfResp = await httpFetch(paperMetadata.pdf_url, { + method: 'GET', + timeout: 300000, + responseType: 'arraybuffer', + signal: abortSignal, + }) + + if (pdfResp.status >= 200 && pdfResp.status < 300) { + const pdfBuffer = Buffer.from(pdfResp.data)
763-820: Inconsistent span handling pattern.The
arxivPdfParserToolerror handler uses the legacy pattern withsetStatus,recordException, and separateend()calls (lines 784-819), while other tools in this file use the simplerspan?.error({ error, endSpan: true })pattern. Consider standardizing for maintainability.src/mastra/tools/polygon-tools.ts (1)
99-145: Tracing spans should passrequestContext, andtracingContextshould be typed.Per your tool tracing docs, span creation should include
requestContext: context?.requestContextfor automatic metadata extraction, andtracingContextshould be typed (instead of inferred).Proposed fix (pattern)
import { SpanType } from '@mastra/core/observability' +import type { TracingContext } from '@mastra/core/observability' // ... -const tracingContext = context?.tracingContext +const tracingContext: TracingContext | undefined = context?.tracingContext const rootSpan = tracingContext?.currentSpan?.createChildSpan({ type: SpanType.TOOL_CALL, name: 'polygon-stock-quotes-tool', input: { function: inputData.function, symbol: inputData.symbol }, + requestContext: context?.requestContext, metadata: { 'tool.id': 'polygon-stock-quotes', // ... }, })Also applies to: 224-253
src/mastra/tools/AGENTS.md (2)
1-4: Markdown guideline: avoid H1 headings in repo docs.Line 3 should be
##(H2), not#.
1-1: Docs metadata / changelog formatting drift.
- Update
last_updatedin the AGENTS-META header to reflect the newest entry (2026-01-11).- Keep changelog entries consistently inside the table (avoid mixing free-form bullets under the table).
Also applies to: 730-746
src/mastra/tools/tests/alpha-vantage.tool.test.ts (1)
1-1: Avoid// @ts-nocheckin tests (it masks real breakages during refactors).Prefer fixing the specific type errors (or use
// @ts-expect-erroron the exact lines).
| import { createTool } from '@mastra/core/tools' | ||
| import { SpanType } from '@mastra/core/observability' | ||
| import type { TracingContext } from '@mastra/core/observability' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM!
The type-only import for TracingContext is correctly added. The tool properly uses the tracing context for span creation and follows the established observability patterns.
💡 Optional: Consider consolidating imports from the same module
import { createTool } from '@mastra/core/tools'
-import { SpanType } from '@mastra/core/observability'
-import type { TracingContext } from '@mastra/core/observability'
+import { SpanType, type TracingContext } from '@mastra/core/observability'
import { z } from 'zod'🤖 Prompt for AI Agents
In @src/mastra/tools/chartjs.tool.ts around lines 1 - 3, Imports from
'@mastra/core/observability' are split; consolidate them into a single import to
reduce lines and improve readability by combining SpanType and the type-only
TracingContext import into one statement (keep the `type { TracingContext }`
syntax) and remove the separate import lines referencing the same module so
symbols like SpanType and TracingContext remain available for use in createTool
and span creation.
| import { SpanType } from '@mastra/core/observability' | ||
| import type { TracingContext } from '@mastra/core/observability' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM!
The type-only import for TracingContext is correctly added using the import type syntax. The tool properly uses the tracing context for span creation and follows the established observability patterns. Based on learnings, the import path @mastra/core/observability is correct for SpanType.
💡 Optional: Consider consolidating imports from the same module
-import { SpanType } from '@mastra/core/observability'
-import type { TracingContext } from '@mastra/core/observability'
+import { SpanType, type TracingContext } from '@mastra/core/observability'
import type { RequestContext } from '@mastra/core/request-context'🤖 Prompt for AI Agents
In @src/mastra/tools/code-analysis.tool.ts around lines 1 - 2, No code change
required—the import for TracingContext is correctly using `import type` and
`SpanType` is correctly imported from the same module; optionally, consolidate
the two imports into a single statement like `import type { TracingContext }
from '@mastra/core/observability'` combined with `import { SpanType }` if you
prefer a single-line import, targeting the import declarations that reference
`TracingContext` and `SpanType`.
| @@ -1,5 +1,6 @@ | |||
| import type { RequestContext } from '@mastra/core/request-context' | |||
| import { createTool } from '@mastra/core/tools' | |||
| import type { TracingContext } from '@mastra/core/observability' | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused type import.
TracingContext is imported but never explicitly used as a type annotation. Consider removing it or using it to type the tracingContext variable on line 76.
🤖 Prompt for AI Agents
In @src/mastra/tools/code-chunking.ts at line 3, Remove the unused type import
or actually use it: either delete the import statement for TracingContext or add
the type annotation TracingContext to the tracingContext variable (named
tracingContext) where it is declared/assigned so the import is used; update the
declaration/assignment to explicitly type tracingContext as TracingContext if
you want to keep the import.
| import { SpanType } from '@mastra/core/observability' | ||
| import type { RequestContext } from '@mastra/core/request-context' | ||
| import { createTool } from '@mastra/core/tools' | ||
| import type { TracingContext } from '@mastra/core/observability' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consolidate imports and remove unused type.
TracingContext is imported but never used. Additionally, lines 1 and 4 both import from @mastra/core/observability - these should be consolidated per import organization guidelines.
Proposed fix
-import { SpanType } from '@mastra/core/observability'
-import type { RequestContext } from '@mastra/core/request-context'
-import { createTool } from '@mastra/core/tools'
-import type { TracingContext } from '@mastra/core/observability'
+import { SpanType } from '@mastra/core/observability'
+import type { RequestContext } from '@mastra/core/request-context'
+import { createTool } from '@mastra/core/tools'If TracingContext is needed for explicit typing:
-import { SpanType } from '@mastra/core/observability'
-import type { RequestContext } from '@mastra/core/request-context'
-import { createTool } from '@mastra/core/tools'
-import type { TracingContext } from '@mastra/core/observability'
+import type { TracingContext } from '@mastra/core/observability'
+import { SpanType } from '@mastra/core/observability'
+import type { RequestContext } from '@mastra/core/request-context'
+import { createTool } from '@mastra/core/tools'🤖 Prompt for AI Agents
In @src/mastra/tools/code-search.tool.ts around lines 1 - 4, The imports from
@mastra/core/observability should be consolidated and the unused TracingContext
type removed: combine the two observability imports into a single import that
exports SpanType (and only include TracingContext if you actually use it for
typing), and leave the existing imports for RequestContext and createTool
unchanged; update the import statement to remove TracingContext if unused so
only SpanType is imported from @mastra/core/observability.
| execute: async ({ action }, context) => { | ||
| const writer = context?.writer | ||
| const tracingContext: TracingContext | undefined = | ||
| context?.tracingContext | ||
|
|
||
| const span = tracingContext?.currentSpan?.createChildSpan({ | ||
| type: SpanType.TOOL_CALL, | ||
| name: 'confirmation', | ||
| input: { action }, | ||
| metadata: { | ||
| 'tool.id': 'confirmation-tool', | ||
| 'tool.input.action': action, | ||
| }, | ||
| requestContext: context?.requestContext, | ||
| }) | ||
|
|
||
| await writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'in-progress', | ||
| message: `Input: action="${action}" - ❓ Requesting confirmation`, | ||
| stage: 'confirmation-tool', | ||
| }, | ||
| id: 'confirmation-tool', | ||
| }) | ||
|
|
||
| const { resumeData, suspend } = context?.agent ?? {} | ||
|
|
||
| // Explicitly check for true to avoid nullable boolean in condition | ||
| if (resumeData?.confirmed !== true) { | ||
| await writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'done', | ||
| message: `Input: action="${action}" - ⏸️ Execution suspended for confirmation`, | ||
| stage: 'confirmation-tool', | ||
| }, | ||
| id: 'confirmation-tool', | ||
| }) | ||
|
|
||
| span?.update({ | ||
| metadata: { | ||
| 'tool.status': 'suspended', | ||
| }, | ||
| }) | ||
| span?.end() | ||
|
|
||
| return suspend?.({ | ||
| message: `Please confirm: ${action}`, | ||
| action, | ||
| }) | ||
| } | ||
|
|
||
| const result = { confirmed: true, action } | ||
|
|
||
| span?.update({ | ||
| output: result, | ||
| metadata: { | ||
| 'tool.output.confirmed': true, | ||
| 'tool.status': 'completed', | ||
| }, | ||
| }) | ||
| span?.end() | ||
|
|
||
| await writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'done', | ||
| message: `Input: action="${action}" - ✅ Confirmed`, | ||
| stage: 'confirmation-tool', | ||
| }, | ||
| id: 'confirmation-tool', | ||
| }) | ||
|
|
||
| return result | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing abortSignal handling and lifecycle hooks.
Per the coding guidelines for tools:
- "Respect cancellation signals by checking
abortSignal?.abortedearly in execute function and before long-running operations" - "Implement complete tool lifecycle hooks in createTool() definition:
onInputStart,onInputDelta,onInputAvailable, andonOutputmust be declared after theexecuteproperty"
💡 Suggested additions
execute: async ({ action }, context) => {
+ // Check for abort signal
+ if (context?.abortSignal?.aborted) {
+ throw new Error('Confirmation cancelled')
+ }
+
const writer = context?.writer
// ... rest of execute
},
+ onInputStart: ({ toolCallId }) => {
+ // Log input start if needed
+ },
+ onInputDelta: ({ toolCallId, delta }) => {
+ // Handle streaming input delta
+ },
+ onInputAvailable: ({ toolCallId, input }) => {
+ // Log when full input is available
+ },
+ onOutput: ({ toolCallId, output }) => {
+ // Log output
+ },
})Based on coding guidelines for tool implementations.
📝 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.
| execute: async ({ action }, context) => { | |
| const writer = context?.writer | |
| const tracingContext: TracingContext | undefined = | |
| context?.tracingContext | |
| const span = tracingContext?.currentSpan?.createChildSpan({ | |
| type: SpanType.TOOL_CALL, | |
| name: 'confirmation', | |
| input: { action }, | |
| metadata: { | |
| 'tool.id': 'confirmation-tool', | |
| 'tool.input.action': action, | |
| }, | |
| requestContext: context?.requestContext, | |
| }) | |
| await writer?.custom({ | |
| type: 'data-tool-progress', | |
| data: { | |
| status: 'in-progress', | |
| message: `Input: action="${action}" - ❓ Requesting confirmation`, | |
| stage: 'confirmation-tool', | |
| }, | |
| id: 'confirmation-tool', | |
| }) | |
| const { resumeData, suspend } = context?.agent ?? {} | |
| // Explicitly check for true to avoid nullable boolean in condition | |
| if (resumeData?.confirmed !== true) { | |
| await writer?.custom({ | |
| type: 'data-tool-progress', | |
| data: { | |
| status: 'done', | |
| message: `Input: action="${action}" - ⏸️ Execution suspended for confirmation`, | |
| stage: 'confirmation-tool', | |
| }, | |
| id: 'confirmation-tool', | |
| }) | |
| span?.update({ | |
| metadata: { | |
| 'tool.status': 'suspended', | |
| }, | |
| }) | |
| span?.end() | |
| return suspend?.({ | |
| message: `Please confirm: ${action}`, | |
| action, | |
| }) | |
| } | |
| const result = { confirmed: true, action } | |
| span?.update({ | |
| output: result, | |
| metadata: { | |
| 'tool.output.confirmed': true, | |
| 'tool.status': 'completed', | |
| }, | |
| }) | |
| span?.end() | |
| await writer?.custom({ | |
| type: 'data-tool-progress', | |
| data: { | |
| status: 'done', | |
| message: `Input: action="${action}" - ✅ Confirmed`, | |
| stage: 'confirmation-tool', | |
| }, | |
| id: 'confirmation-tool', | |
| }) | |
| return result | |
| }, | |
| execute: async ({ action }, context) => { | |
| // Check for abort signal | |
| if (context?.abortSignal?.aborted) { | |
| throw new Error('Confirmation cancelled') | |
| } | |
| const writer = context?.writer | |
| const tracingContext: TracingContext | undefined = | |
| context?.tracingContext | |
| const span = tracingContext?.currentSpan?.createChildSpan({ | |
| type: SpanType.TOOL_CALL, | |
| name: 'confirmation', | |
| input: { action }, | |
| metadata: { | |
| 'tool.id': 'confirmation-tool', | |
| 'tool.input.action': action, | |
| }, | |
| requestContext: context?.requestContext, | |
| }) | |
| await writer?.custom({ | |
| type: 'data-tool-progress', | |
| data: { | |
| status: 'in-progress', | |
| message: `Input: action="${action}" - ❓ Requesting confirmation`, | |
| stage: 'confirmation-tool', | |
| }, | |
| id: 'confirmation-tool', | |
| }) | |
| const { resumeData, suspend } = context?.agent ?? {} | |
| // Explicitly check for true to avoid nullable boolean in condition | |
| if (resumeData?.confirmed !== true) { | |
| await writer?.custom({ | |
| type: 'data-tool-progress', | |
| data: { | |
| status: 'done', | |
| message: `Input: action="${action}" - ⏸️ Execution suspended for confirmation`, | |
| stage: 'confirmation-tool', | |
| }, | |
| id: 'confirmation-tool', | |
| }) | |
| span?.update({ | |
| metadata: { | |
| 'tool.status': 'suspended', | |
| }, | |
| }) | |
| span?.end() | |
| return suspend?.({ | |
| message: `Please confirm: ${action}`, | |
| action, | |
| }) | |
| } | |
| const result = { confirmed: true, action } | |
| span?.update({ | |
| output: result, | |
| metadata: { | |
| 'tool.output.confirmed': true, | |
| 'tool.status': 'completed', | |
| }, | |
| }) | |
| span?.end() | |
| await writer?.custom({ | |
| type: 'data-tool-progress', | |
| data: { | |
| status: 'done', | |
| message: `Input: action="${action}" - ✅ Confirmed`, | |
| stage: 'confirmation-tool', | |
| }, | |
| id: 'confirmation-tool', | |
| }) | |
| return result | |
| }, | |
| onInputStart: ({ toolCallId }) => { | |
| // Log input start if needed | |
| }, | |
| onInputDelta: ({ toolCallId, delta }) => { | |
| // Handle streaming input delta | |
| }, | |
| onInputAvailable: ({ toolCallId, input }) => { | |
| // Log when full input is available | |
| }, | |
| onOutput: ({ toolCallId, output }) => { | |
| // Log output | |
| }, |
🤖 Prompt for AI Agents
In @src/mastra/tools/confirmation.tool.ts around lines 23 - 98, Add abortSignal
handling in the execute function by checking context?.abortSignal?.aborted near
the top and again before any awaited/long-running ops (e.g., before
writer?.custom calls and before suspend invocation) and return/throw
appropriately to respect cancellation; then declare the required lifecycle hook
methods onInputStart, onInputDelta, onInputAvailable, and onOutput immediately
after the execute property in the createTool definition (implement simple no-op
handlers or appropriate behaviors using context and writer) so the tool exposes
the complete lifecycle as required.
| const mockContext = { | ||
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | ||
| abortSignal: { aborted: false }, | ||
| requestContext: {} as UrlToolContext, | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| }, | ||
| } as any |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract repeated mockContext to a factory helper or shared setup.
The identical mockContext object is defined ~20 times throughout this file. This violates DRY and makes maintenance harder if the context structure changes.
♻️ Suggested refactor
+// Helper to create mock context with optional overrides
+function createMockContext(overrides: Partial<{
+ requestContext: Partial<UrlToolContext>;
+ createChildSpan: ReturnType<typeof vi.fn>;
+}> = {}) {
+ const createChildSpanMock = overrides.createChildSpan ?? vi.fn().mockReturnValue({
+ update: vi.fn(),
+ end: vi.fn(),
+ error: vi.fn(),
+ });
+ return {
+ writer: { custom: vi.fn().mockResolvedValue(undefined) },
+ abortSignal: { aborted: false },
+ requestContext: (overrides.requestContext ?? {}) as UrlToolContext,
+ tracingContext: {
+ currentSpan: { createChildSpan: createChildSpanMock },
+ },
+ } as any;
+}
+
describe('urlValidationTool', () => {
// ... then in each test:
- const mockContext = {
- writer: { custom: vi.fn().mockResolvedValue(undefined) },
- abortSignal: { aborted: false },
- requestContext: {} as UrlToolContext,
- tracingContext: {
- currentSpan: {
- createChildSpan: vi.fn().mockReturnValue({
- update: vi.fn(),
- end: vi.fn(),
- error: vi.fn(),
- }),
- },
- },
- } as any
+ const mockContext = createMockContext();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mockContext = { | |
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | |
| abortSignal: { aborted: false }, | |
| requestContext: {} as UrlToolContext, | |
| tracingContext: { | |
| currentSpan: { | |
| createChildSpan: vi.fn().mockReturnValue({ | |
| update: vi.fn(), | |
| end: vi.fn(), | |
| error: vi.fn(), | |
| }), | |
| }, | |
| }, | |
| } as any | |
| // Helper to create mock context with optional overrides | |
| function createMockContext(overrides: Partial<{ | |
| requestContext: Partial<UrlToolContext>; | |
| createChildSpan: ReturnType<typeof vi.fn>; | |
| }> = {}) { | |
| const createChildSpanMock = overrides.createChildSpan ?? vi.fn().mockReturnValue({ | |
| update: vi.fn(), | |
| end: vi.fn(), | |
| error: vi.fn(), | |
| }); | |
| return { | |
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | |
| abortSignal: { aborted: false }, | |
| requestContext: (overrides.requestContext ?? {}) as UrlToolContext, | |
| tracingContext: { | |
| currentSpan: { createChildSpan: createChildSpanMock }, | |
| }, | |
| } as any; | |
| } | |
| describe('urlValidationTool', () => { | |
| it('should validate URLs correctly', () => { | |
| const mockContext = createMockContext(); | |
| // ... rest of test | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In @src/mastra/tools/tests/url-tool.test.ts around lines 16 - 29, Extract the
repeated mockContext object into a single factory helper (e.g.,
createMockContext or makeMockUrlToolContext) that returns the assembled object
with writer.custom, abortSignal, requestContext typed as UrlToolContext, and
tracingContext.currentSpan.createChildSpan stubs; allow an optional overrides
param to customize per-test behavior. Replace every inline mockContext in
url-tool.test.ts with calls to this factory (importing it from a new test-utils
or from the top of the test file) so updates to the shape or stubs are made in
one place.
| const successResult = result as any | ||
| expect(successResult.success).toBe(true) | ||
| expect(successResult.results.validate).toBe(true) | ||
| expect(successResult.originalUrl).toBe('https://example.com/path') |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using proper types instead of as any for result assertions.
The repeated as any casts bypass TypeScript's type checking. Consider importing and using the tool's output schema types for better type safety and IDE support.
// Instead of:
const successResult = result as any
// Consider inferring types from the tool's outputSchema:
import { z } from 'zod'
type ValidationResult = z.infer<typeof urlValidationTool.outputSchema>
const successResult = result as ValidationResult🤖 Prompt for AI Agents
In @src/mastra/tools/tests/url-tool.test.ts around lines 36 - 39, Replace the
unsafe "as any" cast for result with a proper typed alias derived from the
tool's output schema: import the tool's output schema (e.g.
urlValidationTool.outputSchema) and use z.infer (or the exported type) to create
a ValidationResult type, then cast result to that type (const successResult =
result as ValidationResult) and update the assertions to use the typed
successResult; this removes the any cast and restores TypeScript type checking
for result.
| describe('urlManipulationTool', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| vi.restoreAllMocks() | ||
| }) | ||
|
|
||
| describe('add-query operation', () => { | ||
| it('should add query parameters to URL', async () => { | ||
| const mockContext = { | ||
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | ||
| abortSignal: { aborted: false }, | ||
| requestContext: {} as UrlToolContext, | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| }, | ||
| } as any | ||
|
|
||
| const result = await urlManipulationTool.execute( | ||
| { | ||
| baseUrl: 'https://example.com/page', | ||
| operations: ['add-query'], | ||
| parameters: { query: { key: 'value', page: '1' } }, | ||
| }, | ||
| mockContext | ||
| ) | ||
|
|
||
| const successResult = result as any | ||
| expect(successResult.success).toBe(true) | ||
| expect(successResult.resultUrl).toContain('key=value') | ||
| expect(successResult.resultUrl).toContain('page=1') | ||
| expect(successResult.changes).toHaveLength(1) | ||
| }) | ||
| }) | ||
|
|
||
| describe('remove-query operation', () => { | ||
| it('should remove query parameters from URL', async () => { | ||
| const mockContext = { | ||
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | ||
| abortSignal: { aborted: false }, | ||
| requestContext: {} as UrlToolContext, | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| }, | ||
| } as any | ||
|
|
||
| const result = await urlManipulationTool.execute( | ||
| { | ||
| baseUrl: 'https://example.com/page?old=value&keep=this', | ||
| operations: ['remove-query'], | ||
| parameters: { queryKeys: ['old'] }, | ||
| }, | ||
| mockContext | ||
| ) | ||
|
|
||
| const successResult = result as any | ||
| expect(successResult.success).toBe(true) | ||
| expect(successResult.resultUrl).not.toContain('old=value') | ||
| expect(successResult.resultUrl).toContain('keep=this') | ||
| }) | ||
| }) | ||
|
|
||
| describe('add-fragment operation', () => { | ||
| it('should add fragment to URL', async () => { | ||
| const mockContext = { | ||
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | ||
| abortSignal: { aborted: false }, | ||
| requestContext: {} as UrlToolContext, | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| }, | ||
| } as any | ||
|
|
||
| const result = await urlManipulationTool.execute( | ||
| { | ||
| baseUrl: 'https://example.com/page', | ||
| operations: ['add-fragment'], | ||
| parameters: { fragment: 'section2' }, | ||
| }, | ||
| mockContext | ||
| ) | ||
|
|
||
| const successResult = result as any | ||
| expect(successResult.success).toBe(true) | ||
| expect(successResult.resultUrl).toContain('#section2') | ||
| }) | ||
| }) | ||
|
|
||
| describe('change-protocol operation', () => { | ||
| it('should change protocol of URL', async () => { | ||
| const mockContext = { | ||
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | ||
| abortSignal: { aborted: false }, | ||
| requestContext: {} as UrlToolContext, | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| }, | ||
| } as any | ||
|
|
||
| const result = await urlManipulationTool.execute( | ||
| { | ||
| baseUrl: 'https://example.com', | ||
| operations: ['change-protocol'], | ||
| parameters: { protocol: 'http' }, | ||
| }, | ||
| mockContext | ||
| ) | ||
|
|
||
| const successResult = result as any | ||
| expect(successResult.success).toBe(true) | ||
| expect(successResult.resultUrl).toMatch(/^http:\/\//) | ||
| }) | ||
| }) | ||
|
|
||
| describe('add-path operation', () => { | ||
| it('should add path segment to URL', async () => { | ||
| const mockContext = { | ||
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | ||
| abortSignal: { aborted: false }, | ||
| requestContext: {} as UrlToolContext, | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| }, | ||
| } as any | ||
|
|
||
| const result = await urlManipulationTool.execute( | ||
| { | ||
| baseUrl: 'https://example.com', | ||
| operations: ['add-path'], | ||
| parameters: { path: 'api/v1/users' }, | ||
| }, | ||
| mockContext | ||
| ) | ||
|
|
||
| const successResult = result as any | ||
| expect(successResult.success).toBe(true) | ||
| expect(successResult.resultUrl).toContain('/api/v1/users') | ||
| }) | ||
| }) | ||
|
|
||
| describe('multiple operations', () => { | ||
| it('should perform multiple manipulation operations', async () => { | ||
| const mockContext = { | ||
| writer: { custom: vi.fn().mockResolvedValue(undefined) }, | ||
| abortSignal: { aborted: false }, | ||
| requestContext: {} as UrlToolContext, | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| }, | ||
| } as any | ||
|
|
||
| const result = await urlManipulationTool.execute( | ||
| { | ||
| baseUrl: 'https://example.com/page', | ||
| operations: ['add-fragment', 'add-query'], | ||
| parameters: { | ||
| fragment: 'bottom', | ||
| query: { ref: 'header' }, | ||
| }, | ||
| }, | ||
| mockContext | ||
| ) | ||
|
|
||
| const successResult = result as any | ||
| expect(successResult.success).toBe(true) | ||
| expect(successResult.resultUrl).toContain('#bottom') | ||
| expect(successResult.resultUrl).toContain('ref=header') | ||
| expect(successResult.changes).toHaveLength(2) | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Missing tracing verification test for urlManipulationTool.
The urlValidationTool suite includes a tracing test (lines 286-320), but urlManipulationTool lacks equivalent coverage. For consistency and to meet the 97% coverage target, add a similar tracing test.
💡 Suggested test case
describe('tracing', () => {
it('should create tracing span with correct metadata', async () => {
const createChildSpanMock = vi.fn().mockReturnValue({
update: vi.fn(),
end: vi.fn(),
error: vi.fn(),
});
const mockContext = {
writer: { custom: vi.fn().mockResolvedValue(undefined) },
abortSignal: { aborted: false },
requestContext: { userId: 'test-user-123' } as UrlToolContext,
tracingContext: {
currentSpan: { createChildSpan: createChildSpanMock },
},
} as any;
await urlManipulationTool.execute(
{ baseUrl: 'https://example.com', operations: ['add-query'], parameters: { query: { key: 'value' } } },
mockContext
);
expect(createChildSpanMock).toHaveBeenCalledWith(
expect.objectContaining({
type: 'tool_call',
name: 'url-manipulation',
metadata: expect.objectContaining({
'tool.id': 'url-manipulation',
}),
})
);
});
});🤖 Prompt for AI Agents
In @src/mastra/tools/tests/url-tool.test.ts around lines 323 - 531, Add a
tracing unit test for urlManipulationTool similar to the urlValidationTool
tracing test: mock tracingContext.currentSpan.createChildSpan (e.g.,
createChildSpanMock returning an object with update/end/error), call
urlManipulationTool.execute with a simple operation (e.g., operations:
['add-query'] and parameters with a query), and assert createChildSpanMock was
called with an object containing type: 'tool_call', name: 'url-manipulation' and
metadata including 'tool.id': 'url-manipulation'; ensure the mockContext
includes requestContext (e.g., userId) and writer/abortSignal like other tests
and verify createChildSpanMock invocation.
| ## Progress | ||
| ### Done | ||
| - [x] Analyzed test coverage status in `src/mastra/tools/tests/AGENTS.md` | ||
| - [x] Created todo list with 15 test files to create (fs, calendar, arxiv, data-processing, pdf, alpha-vantage, polygon, finnhub, github, code-search, find-symbol, execa, web-scraper, browser, serpapi-news) |
There was a problem hiding this comment.
Capitalize "GitHub" correctly.
Per LanguageTool hint, "GitHub" should be spelled with a capital "H".
-- [x] Created todo list with 15 test files to create (fs, calendar, arxiv, data-processing, pdf, alpha-vantage, polygon, finnhub, github, code-search, find-symbol, execa, web-scraper, browser, serpapi-news)
+- [x] Created todo list with 15 test files to create (fs, calendar, arxiv, data-processing, pdf, alpha-vantage, polygon, finnhub, GitHub, code-search, find-symbol, execa, web-scraper, browser, serpapi-news)📝 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.
| - [x] Created todo list with 15 test files to create (fs, calendar, arxiv, data-processing, pdf, alpha-vantage, polygon, finnhub, github, code-search, find-symbol, execa, web-scraper, browser, serpapi-news) | |
| - [x] Created todo list with 15 test files to create (fs, calendar, arxiv, data-processing, pdf, alpha-vantage, polygon, finnhub, GitHub, code-search, find-symbol, execa, web-scraper, browser, serpapi-news) |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~22-~22: The official name of this software platform is spelled with a capital “H”.
Context: ..., pdf, alpha-vantage, polygon, finnhub, github, code-search, find-symbol, execa, web-s...
(GITHUB)
🤖 Prompt for AI Agents
In @thoughts/ledgers/CONTINUITY_ses_450b.md at line 22, The checklist item uses
the lowercase token "github"; update that token to the correctly capitalized
trademark "GitHub" in the todo list entry (the line containing "github" among
the 15 test files) so the item reads "... github ... " → "... GitHub ...".
| **Type error encountered:** | ||
| ``` | ||
| Type '{ createChildSpan: Mock<Procedure>; }' is not assignable to parameter of type 'Span<keyof SpanTypeMap>'. | ||
| Missing properties: isInternal, observabilityInstance, end, error, update, and 16 more. | ||
| ``` | ||
|
|
||
| **Fix needed:** Cast context mocks with `as any`: | ||
| ```typescript | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| } as any | ||
| ``` |
There was a problem hiding this comment.
Fix markdown formatting for code blocks.
Per markdownlint hints (MD031, MD040), fenced code blocks should be surrounded by blank lines and have a language specified.
📝 Suggested fix
**Type error encountered:**
+
-```
+```text
Type '{ createChildSpan: Mock<Procedure>; }' is not assignable to parameter of type 'Span<keyof SpanTypeMap>'.
Missing properties: isInternal, observabilityInstance, end, error, update, and 16 more.Fix needed: Cast context mocks with as any:
+
tracingContext: {
currentSpan: {📝 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.
| **Type error encountered:** | |
| ``` | |
| Type '{ createChildSpan: Mock<Procedure>; }' is not assignable to parameter of type 'Span<keyof SpanTypeMap>'. | |
| Missing properties: isInternal, observabilityInstance, end, error, update, and 16 more. | |
| ``` | |
| **Fix needed:** Cast context mocks with `as any`: | |
| ```typescript | |
| tracingContext: { | |
| currentSpan: { | |
| createChildSpan: vi.fn().mockReturnValue({ | |
| update: vi.fn(), | |
| end: vi.fn(), | |
| error: vi.fn(), | |
| }), | |
| }, | |
| } as any | |
| ``` | |
| **Type error encountered:** | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
54-54: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
60-60: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In @thoughts/ledgers/CONTINUITY_ses_450b.md around lines 53 - 70, The markdown
in CONTINUITY_ses_450b.md has fenced code blocks that lack surrounding blank
lines and explicit language tags; update each fenced block (the error block
showing "Type '{ createChildSpan: Mock<Procedure>; }'..." and the subsequent
suggested code block for the cast) to have a blank line before and after the
triple-backtick fences and add the appropriate language identifier (e.g.,
```text for the error output and ```typescript for the code snippet) so they
comply with MD031/MD040 and render correctly.
| @@ -0,0 +1 @@ | |||
| {"numTotalTestSuites":0,"numPassedTestSuites":0,"numFailedTestSuites":0,"numPendingTestSuites":0,"numTotalTests":0,"numPassedTests":0,"numFailedTests":0,"numPendingTests":0,"numTodoTests":0,"snapshot":{"added":0,"failure":false,"filesAdded":0,"filesRemoved":0,"filesRemovedList":[],"filesUnmatched":0,"filesUpdated":0,"matched":0,"total":0,"unchecked":0,"uncheckedKeysByFile":[],"unmatched":0,"updated":0,"didUpdate":false},"startTime":1768217443502,"success":false,"testResults":[]} No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if test-results paths are already in .gitignore
echo "=== Checking .gitignore for test-results patterns ==="
if [ -f .gitignore ]; then
grep -n "test-results" .gitignore || echo "No test-results pattern found in .gitignore"
else
echo ".gitignore file not found"
fi
# Check for similar test result files
echo ""
echo "=== Looking for similar test result files ==="
fd -t f "test-results" --exclude node_modules || echo "fd command failed, trying find"
# List all files in the test directory
echo ""
echo "=== Contents of dot/tests directory ==="
find dot/tests -type f 2>/dev/null | head -20
# Check .gitignore contents
echo ""
echo "=== Full .gitignore contents ==="
cat .gitignoreRepository: ssdeanx/AgentStack
Length of output: 2724
🏁 Script executed:
#!/bin/bash
# Check if these test-results files are recently added
echo "=== Git status of test-results files ==="
git status dot/tests/test-results/test-results.json tests/test-results/test-results.json 2>/dev/null || echo "Not in git status"
# Check git history
echo ""
echo "=== Recent commits affecting test-results ==="
git log --oneline --all -- "*test-results*" 2>/dev/null | head -5 || echo "Unable to check git log"
# Verify the files contain test output (not schema)
echo ""
echo "=== File contents verification ==="
echo "dot/tests/test-results/test-results.json size:"
wc -c < dot/tests/test-results/test-results.json
echo ""
echo "tests/test-results/test-results.json size:"
wc -c < tests/test-results/test-results.jsonRepository: ssdeanx/AgentStack
Length of output: 922
Generated test results files should not be committed to version control.
These appear to be generated test output files with zero tests and are not currently in .gitignore. Generated test artifacts change on every test run and can cause merge conflicts.
Two instances found:
dot/tests/test-results/test-results.jsontests/test-results/test-results.json
Add to .gitignore:
# Testing
coverage/
+test-results/🤖 Prompt for AI Agents
In @dot/tests/test-results/test-results.json at line 1, These test-result JSON
files are generated and should be ignored; add entries for the two paths (e.g.,
"tests/test-results/" and "dot/tests/test-results/" or a generic
"*/test-results/" or "test-results/*.json") to .gitignore, then remove the
currently tracked files from Git (git rm --cached) and commit the .gitignore
change so files like dot/tests/test-results/test-results.json and
tests/test-results/test-results.json are no longer tracked; ensure you do not
delete local files so CI/local runs still produce them.
| technicalAnalysisTool, | ||
| } from '../tools' | ||
|
|
||
| type UserTier = 'free' | 'pro' | 'enterprise' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate UserTier type definition.
This type is already defined in src/mastra/tools/alpha-vantage.tool.ts (line 19). Consider importing from a shared types module to maintain consistency and avoid duplication.
♻️ Suggested approach
Create a shared types file (e.g., src/mastra/types/user-tier.ts) and import from there:
// src/mastra/types/user-tier.ts
export type UserTier = 'free' | 'pro' | 'enterprise'Then import in both files:
-type UserTier = 'free' | 'pro' | 'enterprise'
+import type { UserTier } from '../types/user-tier'🤖 Prompt for AI Agents
In @src/mastra/agents/financialAnalysisAgent.ts at line 17, Remove the duplicate
UserTier type declaration and replace it with a single shared exported type;
create (or use) a central type export like UserTier = 'free' | 'pro' |
'enterprise' in a shared types module and import that type into
financialAnalysisAgent.ts (and alpha-vantage.tool.ts) so both files reference
the same UserTier symbol instead of redefining it.
| analysisDepth?: 'basic' | 'detailed' | 'comprehensive' | ||
| } | ||
|
|
||
| log.info('Initializing Financial Analysis Agent...') |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Side effect at module import time.
log.info('Initializing Financial Analysis Agent...') executes when the module is imported, not when the agent is instantiated or used. This can cause unexpected log entries during test runs or when the module is imported for type checking.
♻️ Consider removing or deferring this log
-log.info('Initializing Financial Analysis Agent...')
-
export const financialAnalysisAgent = new Agent({If initialization logging is needed, consider adding it in a factory function or lazy initialization pattern.
📝 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.
| log.info('Initializing Financial Analysis Agent...') | |
| export const financialAnalysisAgent = new Agent({ |
🤖 Prompt for AI Agents
In @src/mastra/agents/financialAnalysisAgent.ts at line 26, The module currently
executes log.info('Initializing Financial Analysis Agent...') at import time
causing side effects; remove that top-level call and instead emit the log from a
runtime entry point such as the FinancialAnalysisAgent constructor, an
initialize() method, or a factory function like createFinancialAnalysisAgent so
logging occurs only when an agent is instantiated or explicitly initialized
(replace the module-scope log.info with a call inside the chosen
constructor/initialize/factory location).
| ## Tool Selection Guide | ||
| - **Real-time Data**: Use 'polygonTool' for current prices, aggregates, quotes. | ||
| - **Historical Data**: Use 'finnhubTool' for historical prices, financials, news. | ||
| - **Fundamental Analysis**: Use 'alphaVantageTool' for company overviews, earnings, sector performance. | ||
| - **Charts**: Use 'financialChartTool' for creating visual representations. | ||
| - **Technical Analysis**: Use 'technicalAnalysisTool' for indicators and signals. |
There was a problem hiding this comment.
Tool Selection Guide references unavailable tools.
The instructions reference polygonTool, finnhubTool, and financialChartTool in the Tool Selection Guide, but these tools are commented out in the tools configuration (lines 132-136). This could confuse the agent and lead to failed tool invocations.
Either uncomment and enable the referenced tools, or update the instructions to only reference the available tools (alphaVantageTool, technicalAnalysisTool).
🤖 Prompt for AI Agents
In @src/mastra/agents/financialAnalysisAgent.ts around lines 54 - 59, The Tool
Selection Guide references unavailable tools (polygonTool, finnhubTool,
financialChartTool) while the active tools configuration only exposes
alphaVantageTool and technicalAnalysisTool; either uncomment and enable those
referenced tools in the tools configuration (restore polygonTool, finnhubTool,
financialChartTool) so the guide matches runtime capabilities, or update the
Tool Selection Guide text to only list alphaVantageTool and
technicalAnalysisTool (and remove or replace any calls/examples that mention the
commented tools) to prevent confused/failed tool invocations — locate the guide
block and the tools configuration (the "tools" variable/object) and make the
guide and active tool list consistent.
| if (userTier === 'enterprise') { | ||
| return google.chat('gemini-3-pro-preview') | ||
| } else if (userTier === 'pro') { | ||
| return 'google/gemini-3-flash-preview' | ||
| } | ||
| return google.chat('gemini-3-flash-preview') |
There was a problem hiding this comment.
Inconsistent model return types.
The model function returns different types based on userTier:
- Line 125:
google.chat('gemini-3-pro-preview')returns aLanguageModelV1object - Line 127: Returns a string
'google/gemini-3-flash-preview' - Line 129:
google.chat('gemini-3-flash-preview')returns aLanguageModelV1object
The pro tier returns a string while others return the chat model object. This inconsistency may cause runtime issues if the Agent expects a consistent return type.
🐛 Proposed fix
model: ({
requestContext,
}: {
requestContext: RequestContext<FinancialAnalysisRuntimeContext>
}) => {
const userTier = requestContext.get('user-tier') ?? 'free'
if (userTier === 'enterprise') {
return google.chat('gemini-3-pro-preview')
- } else if (userTier === 'pro') {
- return 'google/gemini-3-flash-preview'
+ } else if (userTier === 'pro') {
+ return google.chat('gemini-3-flash-preview')
}
return google.chat('gemini-3-flash-preview')
},🤖 Prompt for AI Agents
In @src/mastra/agents/financialAnalysisAgent.ts around lines 124 - 129, The
model selection logic returns inconsistent types: for userTier 'pro' it returns
a plain string whereas the other branches return a LanguageModelV1 from
google.chat; update the 'pro' branch in the model function (the userTier
conditional) to return google.chat('gemini-3-flash-preview') instead of the
string so all branches return the same LanguageModelV1 object; ensure the
references to google.chat and the 'gemini-3-flash-preview' identifier are used
exactly as in the other branches.
| import { createStep, createWorkflow } from '@mastra/core/workflows' | ||
| import { z } from 'zod' | ||
| import { log } from '../config/logger' | ||
| import { SpanType } from '@mastra/core/observability' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused import: log.
The log import from '../config/logger' is not used anywhere in this file. Consider removing it to keep imports clean.
import { createStep, createWorkflow } from '@mastra/core/workflows'
import { z } from 'zod'
-import { log } from '../config/logger'
import { SpanType } from '@mastra/core/observability'📝 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 { createStep, createWorkflow } from '@mastra/core/workflows' | |
| import { z } from 'zod' | |
| import { log } from '../config/logger' | |
| import { SpanType } from '@mastra/core/observability' | |
| import { createStep, createWorkflow } from '@mastra/core/workflows' | |
| import { z } from 'zod' | |
| import { SpanType } from '@mastra/core/observability' |
🤖 Prompt for AI Agents
In @src/mastra/workflows/weather-workflow.ts around lines 1 - 4, The import of
log from '../config/logger' in weather-workflow.ts is unused; remove the unused
symbol by deleting the "log" import from the import list (i.e., adjust the
import line that currently imports createStep, createWorkflow, and log so it
only imports createStep and createWorkflow) to clean up the module imports.
| city: z.string().describe('The city to get weather for'), | ||
| }), | ||
| outputSchema: forecastSchema, | ||
| execute: async (inputData: any, context: any) => { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid any type for step execution parameters.
Both fetchWeather and planActivities steps use execute: async (inputData: any, context: any). This loses type safety that the inputSchema and outputSchema provide. Consider using proper types or letting TypeScript infer from the schemas.
Per coding guidelines: "Never use any type; use unknown or proper type definitions instead."
♻️ Consider using schema inference
- execute: async (inputData: any, context: any) => {
+ execute: async (inputData, context) => {Or define explicit types based on the schemas:
execute: async (inputData: z.infer<typeof inputSchema>, context: StepContext) => {Also applies to: 157-157
🤖 Prompt for AI Agents
In @src/mastra/workflows/weather-workflow.ts at line 44, Replace the use of the
unsafe any types on the step executors with properly inferred or explicit types:
change the execute signatures for the fetchWeather and planActivities steps to
use the schema-inferred types (e.g., z.infer<typeof inputSchema> /
z.infer<typeof outputSchema>) or a defined StepContext type instead of any;
update the execute parameters (inputData and context) to use those types so
TypeScript enforces the inputSchema/outputSchema shapes and avoids any.
| const { fullStream } = response as unknown as { | ||
| fullStream?: ReadableStream<unknown> | ||
| } | ||
| const { textStream } = response as unknown as { | ||
| textStream?: ReadableStream<unknown> | ||
| } | ||
|
|
||
| if (context?.writer !== undefined && context?.writer !== null) { | ||
| if ( | ||
| fullStream !== undefined && | ||
| typeof (fullStream as unknown as { pipeTo?: unknown }) | ||
| .pipeTo === 'function' | ||
| ) { | ||
| await ( | ||
| fullStream as unknown as ReadableStream<unknown> | ||
| ).pipeTo(context?.writer as unknown as WritableStream) | ||
| } else if ( | ||
| textStream !== undefined && | ||
| typeof (textStream as unknown as { pipeTo?: unknown }) | ||
| .pipeTo === 'function' | ||
| ) { | ||
| await ( | ||
| textStream as unknown as ReadableStream<unknown> | ||
| ).pipeTo(context?.writer as unknown as WritableStream) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Complex type assertions for stream handling.
The stream piping logic uses multiple as unknown as casts which obscure the actual types and could mask type errors. Consider creating a helper function or using proper type guards.
♻️ Consider a cleaner approach
// Helper to safely pipe streams
async function pipeStreamToWriter(
stream: unknown,
writer: unknown
): Promise<void> {
if (
stream &&
typeof stream === 'object' &&
'pipeTo' in stream &&
typeof (stream as { pipeTo: unknown }).pipeTo === 'function'
) {
await (stream as ReadableStream).pipeTo(writer as WritableStream)
}
}
// Usage:
await pipeStreamToWriter(fullStream, context?.writer)🤖 Prompt for AI Agents
In @src/mastra/workflows/weather-workflow.ts around lines 245 - 270, The
stream-piping block uses repetitive "as unknown as" casts for fullStream,
textStream and context?.writer which hides type errors; create a small
type-guard/helper (e.g., pipeStreamToWriter(stream: unknown, writer: unknown):
Promise<void>) that checks stream is an object, has a callable pipeTo property
and writer is a WritableStream-compatible object, then call that helper for
fullStream and textStream instead of the current nested casts and conditional
checks; update the existing conditional to simply await
pipeStreamToWriter(fullStream, context?.writer) and await
pipeStreamToWriter(textStream, context?.writer) so the code is clearer and
typesafe.
| ## Progress | ||
| ### Done | ||
| - [x] Read C:/Users/ssdsk/AgentStack/src/mastra/agents/graphingAgents.ts to understand current agent configurations | ||
| - [x] Read C:/Users/ssdsk/AgentStack/src/mastra/tools/github.ts to verify GitHub tool exports |
There was a problem hiding this comment.
Minor: Capitalize "GitHub" consistently.
The official name uses a capital "H": github.ts → should reference as "GitHub tools" in prose.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”.
Context: ...Users/ssdsk/AgentStack/src/mastra/tools/github.ts to verify GitHub tool exports - [x] ...
(GITHUB)
🤖 Prompt for AI Agents
In @thoughts/ledgers/CONTINUITY_ses_4519.md at line 21, Update the prose
reference to capitalize "GitHub" consistently: change instances like "github.ts"
or "github tools" in the checklist line to "GitHub" or "GitHub tools" (e.g.,
"Read C:/Users/ssdsk/AgentStack/src/mastra/tools/github.ts to verify GitHub tool
exports" -> ensure "GitHub" has a capital H) so the official name is used
consistently.
| ## File Operations | ||
| ### Read | ||
| - C:/Users/ssdsk/AgentStack/src/mastra/agents/graphingAgents.ts | ||
| - C:/Users/ssdsk/AgentStack/src/mastra/tools/github.ts | ||
| - C:/Users/ssdsk/AgentStack/src/mastra/tools/technical-analysis.tool.ts | ||
| - C:/Users/ssdsk/AgentStack/src/mastra/tools/financial-chart-tools.ts | ||
| - C:/Users/ssdsk/AgentStack/src/mastra/tools/leaflet.tool.ts | ||
| - C:/Users/ssdsk/AgentStack/src/mastra/workflows/stock-analysis-workflow.ts | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using relative paths in documentation.
Absolute Windows paths (C:/Users/ssdsk/AgentStack/...) reduce portability and may not be helpful for other contributors.
💡 Suggested change
## File Operations
### Read
-- C:/Users/ssdsk/AgentStack/src/mastra/agents/graphingAgents.ts
-- C:/Users/ssdsk/AgentStack/src/mastra/tools/github.ts
+- src/mastra/agents/graphingAgents.ts
+- src/mastra/tools/github.ts📝 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.
| ## File Operations | |
| ### Read | |
| - C:/Users/ssdsk/AgentStack/src/mastra/agents/graphingAgents.ts | |
| - C:/Users/ssdsk/AgentStack/src/mastra/tools/github.ts | |
| - C:/Users/ssdsk/AgentStack/src/mastra/tools/technical-analysis.tool.ts | |
| - C:/Users/ssdsk/AgentStack/src/mastra/tools/financial-chart-tools.ts | |
| - C:/Users/ssdsk/AgentStack/src/mastra/tools/leaflet.tool.ts | |
| - C:/Users/ssdsk/AgentStack/src/mastra/workflows/stock-analysis-workflow.ts | |
| ## File Operations | |
| ### Read | |
| - src/mastra/agents/graphingAgents.ts | |
| - src/mastra/tools/github.ts | |
| - src/mastra/tools/technical-analysis.tool.ts | |
| - src/mastra/tools/financial-chart-tools.ts | |
| - src/mastra/tools/leaflet.tool.ts | |
| - src/mastra/workflows/stock-analysis-workflow.ts |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~57-~57: The official name of this software platform is spelled with a capital “H”.
Context: ...Users/ssdsk/AgentStack/src/mastra/tools/github.ts - C:/Users/ssdsk/AgentStack/src/mast...
(GITHUB)
🤖 Prompt for AI Agents
In @thoughts/ledgers/CONTINUITY_ses_4519.md around lines 54 - 62, In the "File
Operations" section (the header "## File Operations") replace the
machine-specific absolute Windows paths with repository-relative paths (for
example use src/mastra/..., ./src/mastra/..., or Markdown relative links) so the
entries for the listed files become portable; update each listed entry under
that header to remove the C:/Users/ssdsk/... prefix, ensure consistent
formatting (code ticks or link syntax) and validate the links render correctly
in the documentation.
| import { | ||
| // serpapiSearchTool, | ||
| // serpapiAcademicTool, | ||
| arxivTool, | ||
| // pdfTool, | ||
| // documentChunkingTool, | ||
| // dataValidatorTool, | ||
| } from '../tools' |
There was a problem hiding this comment.
Remove commented-out code (guideline violation) and align instructions with actual tools.
Right now the agent is instructed to use tools it can’t call.
Also applies to: 130-137
🤖 Prompt for AI Agents
In @src/mastra/agents/academicResearchAgent.ts around lines 9 - 16, Remove the
commented-out import lines and any references to tools that aren’t actually
callable: in academicResearchAgent.ts clean up the import block so only real
exports like arxivTool remain (delete serpapiSearchTool, serpapiAcademicTool,
pdfTool, documentChunkingTool, dataValidatorTool comments), then update the
agent's instruction payload (the agent prompt/allowed-tools list around the
section that currently mentions tools at ~lines 130-137) to list only the tools
that are truly imported/available (e.g., arxivTool or other actual exports) so
the agent isn’t instructed to call nonexistent tools.
| log.info('Initializing Academic Research Agent...') | ||
|
|
||
| export const academicResearchAgent = new Agent({ |
There was a problem hiding this comment.
Avoid module import-time logging.
Gate behind an env flag or log when registering/starting the network.
🤖 Prompt for AI Agents
In @src/mastra/agents/academicResearchAgent.ts around lines 29 - 31, The
import-time call log.info('Initializing Academic Research Agent...') should be
removed or gated so it does not run on module import; instead, either check an
env flag (e.g. process.env.LOG_ACADEMIC_AGENT) before logging or move the log
into the agent lifecycle where the network is registered/started (e.g. when
creating/starting the exported academicResearchAgent instance or inside its
start/register method). Update the code around the exported
academicResearchAgent (new Agent({...})) to ensure no side-effect logging occurs
at import time and only logs when explicitly enabled or when the agent is
started.
| model: ({ | ||
| requestContext, | ||
| }: { | ||
| requestContext: RequestContext<AcademicResearchRuntimeContext> | ||
| }) => { | ||
| const userTier = requestContext.get('user-tier') ?? 'free' | ||
| if (userTier === 'enterprise') { | ||
| return google.chat('gemini-3-pro-preview') | ||
| } else if (userTier === 'pro') { | ||
| return 'google/gemini-3-flash-preview' | ||
| } | ||
| return google.chat('gemini-3-flash-preview') | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the Agent type definition and model configuration
find . -type f -name "*.ts" -o -name "*.tsx" | head -20
fd -e ts -e tsx "agent|Agent" --type f | head -30Repository: ssdeanx/AgentStack
Length of output: 1838
🏁 Script executed:
# Search for Agent interface/type definition
rg -t ts "interface Agent|type Agent" --max-count 10Repository: ssdeanx/AgentStack
Length of output: 2649
🏁 Script executed:
# Look for model property definition in Agent
rg -t ts "model\s*:" -A 3 --max-count 20Repository: ssdeanx/AgentStack
Length of output: 43084
🏁 Script executed:
cat -n src/mastra/agents/academicResearchAgent.ts | head -150Repository: ssdeanx/AgentStack
Length of output: 5873
🏁 Script executed:
# Find google configuration
cat -n src/mastra/config/google.ts | head -50Repository: ssdeanx/AgentStack
Length of output: 2211
🏁 Script executed:
# Search for Agent type definition and model parameter type
rg -t ts "export.*Agent.*{|class Agent" -A 20 | head -80Repository: ssdeanx/AgentStack
Length of output: 188
🏁 Script executed:
# Look at other agents with dynamic model functions
rg -t ts "model:\s*\(\{" -A 8 | head -100Repository: ssdeanx/AgentStack
Length of output: 7101
🏁 Script executed:
# Check the structure of google.chat() to understand what it returns
rg -t ts "google\s*=|chat\s*\(" src/mastra/config/google.ts -A 5 -B 5Repository: ssdeanx/AgentStack
Length of output: 384
🏁 Script executed:
# Search for where google.chat is used and potentially defined
rg -t ts "google\.chat" -B 2 -A 2 | head -60Repository: ssdeanx/AgentStack
Length of output: 4026
🏁 Script executed:
# Check if there's any typing or definition for the google object
rg -t ts "google" src/mastra/config/google.tsRepository: ssdeanx/AgentStack
Length of output: 2435
🏁 Script executed:
# Look at how other providers handle chat models
rg -t ts "\.chat\(" src/mastra/config/ -A 1 -B 1Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Search for any typescript definitions that show what google returns
fd -e d.ts | xargs rg "google" 2>/dev/null | head -20Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Look at noteTakerAgent to see how string models are used
cat -n src/mastra/agents/noteTakerAgent.ts | head -30Repository: ssdeanx/AgentStack
Length of output: 757
🏁 Script executed:
# Search for Agent type definition in @mastra/core or locally
rg -t ts "class Agent" -A 30 | head -100Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Look for type definitions of model parameter in Agent constructor
rg -t ts "model.*:" -B 2 -A 2 src/mastra | grep -A 5 -B 5 "model.*function\|model.*RequestContext" | head -60Repository: ssdeanx/AgentStack
Length of output: 3314
🏁 Script executed:
# Check if there are any type errors or TypeScript issues with the current code
rg -t ts "google.chat" src/mastra/config/google.tsRepository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Search for Mastra core Agent type definitions
fd -e ts "agent" node_modules/@mastra/core 2>/dev/null | head -10Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Find all agents with conditional model returns to see the pattern
rg -t ts "model:.*requestContext.*=>" -A 8 src/mastra/agents/ | grep -E "(return|google\.|string)" | head -30Repository: ssdeanx/AgentStack
Length of output: 1333
🏁 Script executed:
# Look at dataProcessingAgent model function
cat -n src/mastra/agents/dataProcessingAgent.ts | sed -n '100,150p'Repository: ssdeanx/AgentStack
Length of output: 1851
🏁 Script executed:
# Search for any recent changes or comments explaining this pattern
rg -t ts "google.chat.*gemini-3-flash-preview" -B 3 -A 3Repository: ssdeanx/AgentStack
Length of output: 2499
🏁 Script executed:
# Check if there's documentation or tests showing valid model patterns
fd -e test.ts -e spec.ts | xargs rg "model.*google" 2>/dev/null | head -20Repository: ssdeanx/AgentStack
Length of output: 44
Inconsistent model type returns in conditional logic – use consistent approach across all branches.
The model function returns mixed types: google.chat() (provider instance) in enterprise and free tiers, but a string literal in the pro tier. While the Agent schema accepts both string and ModelSchema types, this pattern creates inconsistency. The established pattern in similar agents (reportAgent, weatherAgent, etc.) uses either all provider instances or all string literals consistently. Align all branches to use google.chat() for uniformity, or switch all to string format for simplicity.
🤖 Prompt for AI Agents
In @src/mastra/agents/academicResearchAgent.ts around lines 116 - 128, The model
function returns inconsistent types across branches; change the pro branch to
return the same provider instance form as the others by replacing the string
'google/gemini-3-flash-preview' with google.chat('gemini-3-flash-preview')
inside the model lambda (the requestContext -> userTier logic) so all branches
consistently return the provider instance; ensure you only modify the pro branch
return in the model function.
| import { | ||
| // documentChunkingTool, | ||
| // codeChunkingTool, | ||
| // dataProcessingTools, | ||
| csvToJsonTool, | ||
| jsonToCsvTool, | ||
| // dataValidatorTool, | ||
| dateTimeTool, | ||
| colorChangeTool, | ||
| randomGeneratorTool, | ||
| calculatorTool, | ||
| } from '../tools' |
There was a problem hiding this comment.
Remove commented-out imports/tools (repo guideline violation).
Either wire these tools, or delete the commented code and track “future enablement” in an issue.
Also applies to: 141-152
🤖 Prompt for AI Agents
In @src/mastra/agents/dataProcessingAgent.ts around lines 9 - 20, Remove the
commented-out import lines for documentChunkingTool, codeChunkingTool,
dataProcessingTools, and dataValidatorTool from the import block in
dataProcessingAgent.ts (and any other commented tool imports around the 141-152
region); either fully wire those tools into the module (export/use them in this
agent) or delete the commented code and create a tracking issue referencing
these tools for future enablement so the repo guideline is respected.
| log.info('Initializing Data Processing Agent...') | ||
|
|
||
| export const dataProcessingAgent = new Agent({ |
There was a problem hiding this comment.
Avoid side-effectful module init logging for library modules.
Consider logging only where the agent is constructed/registered (or gate behind a debug env flag), to avoid noisy logs in tests/imports.
🤖 Prompt for AI Agents
In @src/mastra/agents/dataProcessingAgent.ts around lines 33 - 35, The
module-level log.info('Initializing Data Processing Agent...') causes
side-effectful logging on import; remove that direct call and instead either
move the log into the Agent construction/registration site or guard it behind a
debug flag (e.g., check process.env.DEBUG or a config) so logging only occurs
when the agent is explicitly instantiated/registered; update references around
dataProcessingAgent and the Agent(...) creation so initialization logging
happens deterministically at runtime rather than on module import.
| ok: true, | ||
| json: () => Promise.resolve(mockResponse), | ||
| }) | ||
| httpFetchMock.mockResolvedValueOnce({ status: 200, statusText: 'OK', data: mockResponse }) |
There was a problem hiding this comment.
CRITICAL: tests still return legacy fetch() Response shapes; tool expects httpFetch response shape.
Update all mocks to return { status, statusText, data } (and reject via mockRejectedValueOnce for network errors). The current { ok: true, json: ... } will break when the tool reads resp.status / resp.data.
Proposed fix (example replacements)
-fetchMock.mockResolvedValueOnce({
- ok: true,
- json: () => Promise.resolve(mockResponse),
-})
+httpFetchMock.mockResolvedValueOnce({
+ status: 200,
+ statusText: 'OK',
+ data: mockResponse,
+})Also applies to: 203-203, 266-270, 404-405, 434-438, 465-469, 547-551, 621-625, 670-674, 717-721, 789-793, 834-838
🤖 Prompt for AI Agents
In @src/mastra/tools/tests/alpha-vantage.tool.test.ts at line 110, The tests are
returning legacy fetch Response shapes but the tool expects the httpFetch shape;
update every httpFetchMock.mockResolvedValueOnce call (and similar mocks) to
return an object like { status: 200, statusText: 'OK', data: mockResponse }
instead of { ok: true, json: ... }, and replace network error mocks to use
httpFetchMock.mockRejectedValueOnce(new Error('...')) so the tool reads
resp.status and resp.data correctly; search for usages of
httpFetchMock.mockResolvedValueOnce, httpFetchMock.mockRejectedValueOnce, and
mockResponse in alpha-vantage.tool.test.ts and replace each legacy fetch-style
mock with the { status, statusText, data } shape (and use mockRejectedValueOnce
for errors).
| const fetchFinancialDataStep = createStep({ | ||
| id: 'fetch-financial-data', | ||
| description: 'Fetches financial data using Polygon tools', | ||
| inputSchema: z.object({ | ||
| symbol: z.string(), | ||
| }), | ||
| outputSchema: financialDataSchema, | ||
| retries: 3, | ||
| execute: async ({ inputData, writer }) => { | ||
| const startTime = Date.now() | ||
| logStepStart('fetch-financial-data', { symbol: inputData.symbol }) | ||
|
|
||
| try { | ||
| await writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'in-progress', | ||
| message: `📊 Fetching financial data for ${inputData.symbol}...`, | ||
| stage: 'fetch-financial-data', | ||
| }, | ||
| id: 'fetch-financial-data', | ||
| }) | ||
|
|
||
| // This would use polygonStockQuotesTool or polygonStockFundamentalsTool | ||
| // For now, return mock data structure | ||
| const result = { | ||
| symbol: inputData.symbol, | ||
| price: 150.25, | ||
| volume: 1000000, | ||
| marketCap: 1500000000, | ||
| peRatio: 25.5, | ||
| eps: 6.0, | ||
| raw: { source: 'polygon' }, | ||
| } | ||
|
|
||
| await writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'done', | ||
| message: `✅ Financial data retrieved for ${inputData.symbol}`, | ||
| stage: 'fetch-financial-data', | ||
| }, | ||
| id: 'fetch-financial-data', | ||
| }) | ||
|
|
||
| logStepEnd( | ||
| 'fetch-financial-data', | ||
| { symbol: result.symbol }, | ||
| Date.now() - startTime | ||
| ) | ||
| return result | ||
| } catch (error) { | ||
| logError('fetch-financial-data', error, { | ||
| symbol: inputData.symbol, | ||
| }) | ||
|
|
||
| await writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'done', | ||
| message: `❌ Error fetching financial data for ${inputData.symbol}`, | ||
| stage: 'fetch-financial-data', | ||
| }, | ||
| id: 'fetch-financial-data', | ||
| }) | ||
|
|
||
| throw error | ||
| } | ||
| }, | ||
| }) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the financial analysis workflow file
fd "financial-analysis-workflow.ts" | head -5Repository: ssdeanx/AgentStack
Length of output: 113
🏁 Script executed:
# Read the complete file to see step definitions and workflow composition
cat -n src/mastra/workflows/financial-analysis-workflow.tsRepository: ssdeanx/AgentStack
Length of output: 11167
🏁 Script executed:
# Search for other workflow examples using .then() pattern in the codebase
rg -A 5 "\.then\(" src/mastra/workflows/ -t tsRepository: ssdeanx/AgentStack
Length of output: 8305
🏁 Script executed:
# Check for workflow creation patterns and .commit() usage
rg -B 3 -A 5 "\.commit\(\)" src/mastra/workflows/ -t tsRepository: ssdeanx/AgentStack
Length of output: 5959
Compose steps into workflow using .then() before calling .commit().
The fetchFinancialDataStep, performTechnicalAnalysisStep, and generateFinancialReportStep are defined but never connected to the workflow. The workflow calls .commit() without any step composition, making it non-functional. All other workflows in the codebase chain steps via .then(), .branch(), .foreach(), or .parallel() before calling .commit().
Fix
export const financialAnalysisWorkflow = createWorkflow({
id: 'financialAnalysisWorkflow',
description:
'Comprehensive financial analysis workflow using technical analysis tools',
inputSchema: z.object({
symbol: z
.string()
.describe('Stock symbol to analyze (e.g., AAPL, MSFT, GOOGL)'),
}),
outputSchema: reportSchema,
})
-
-financialAnalysisWorkflow.commit()
+ .then(fetchFinancialDataStep)
+ .then(performTechnicalAnalysisStep)
+ .then(generateFinancialReportStep)
+ .commit()Also replace mock data with actual tool calls in all three steps (lines 73–83, 144–158, 220–229).
| // This would use polygonStockQuotesTool or polygonStockFundamentalsTool | ||
| // For now, return mock data structure | ||
| const result = { | ||
| symbol: inputData.symbol, | ||
| price: 150.25, | ||
| volume: 1000000, | ||
| marketCap: 1500000000, | ||
| peRatio: 25.5, | ||
| eps: 6.0, | ||
| raw: { source: 'polygon' }, | ||
| } |
There was a problem hiding this comment.
Don’t ship exported workflows with mock data paths.
Either:
- wire real tool calls now, or
- mark this workflow as experimental/internal (and/or avoid exporting), and track real integration in an issue.
Also applies to: 144-158, 220-229
🤖 Prompt for AI Agents
In @src/mastra/workflows/financial-analysis-workflow.ts around lines 73 - 83,
The exported workflow financial-analysis-workflow.ts currently returns a
hard-coded mock result (the result object built in the workflow) instead of
calling real tools like polygonStockQuotesTool or polygonStockFundamentalsTool;
either replace the mock by invoking the actual tool functions (use the workflow
handler that currently constructs result to call
polygonStockQuotesTool/polygonStockFundamentalsTool and map their responses into
the same result shape), or if you cannot integrate now, mark the workflow as
internal/experimental (remove it from public exports or add an explicit
isExperimental flag and update its name/documentation) and open a ticket to
track real integration; apply the same change for the other mock-returning
blocks noted (around the other result constructions at lines referenced in the
review).
| execute: async (params) => { | ||
| const { inputData, writer } = params as FetchStockDataParams | ||
| const context = (params as FetchStockDataParams).context |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "FetchStockDataParams" --type tsRepository: ssdeanx/AgentStack
Length of output: 297
🏁 Script executed:
ast-grep --pattern 'type FetchStockDataParams = $_'Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
ast-grep --pattern 'interface FetchStockDataParams $_'Repository: ssdeanx/AgentStack
Length of output: 44
🏁 Script executed:
# Get the full stock-analysis-workflow.ts file to examine context
cat -n src/mastra/workflows/stock-analysis-workflow.ts | head -120Repository: ssdeanx/AgentStack
Length of output: 4717
🏁 Script executed:
# Search for where FetchStockDataParams might be imported or defined
rg "import.*FetchStockDataParams|export.*FetchStockDataParams" --type ts
# Search for step parameter types from @mastra/core
rg "StepExecuteParams|StepParams" --type ts
# Look at imports from @mastra/core to understand the parameter structure
head -10 src/mastra/workflows/stock-analysis-workflow.ts | grep -E "import|from"
# Search more broadly for how context is typically accessed in workflow steps
rg -A 3 "params as.*Step.*Params\)|execute: async \(params\)" --type ts | head -50Repository: ssdeanx/AgentStack
Length of output: 673
🏁 Script executed:
# Search for type definitions in the stock-analysis-workflow file
rg "^type |^interface " src/mastra/workflows/stock-analysis-workflow.ts
# Look for how other workflow files handle step params
fd "workflow" --type f src/mastra/workflows/ | head -5 | xargs -I {} rg -A 2 "execute: async \(params\)" {}
# Check the @mastra/core exports to understand param structure
rg "createStep|StepExecuteParams" src/mastra/ --type ts | grep -i "type\|interface" | head -20Repository: ssdeanx/AgentStack
Length of output: 161
🏁 Script executed:
# Search for how other workflow files handle params typing
fd "\.ts$" src/mastra/workflows/ | xargs rg -B 2 -A 2 "execute: async \(params\)" | head -100
# Look for any as-casts in the workflow directory to see pattern usage
rg "params as" src/mastra/workflows/ --type ts -B 1 -A 1
# Check if there's documentation or types in @mastra/core about step params
fd -t f "package.json" | xargs grep -l "mastra" | head -3Repository: ssdeanx/AgentStack
Length of output: 928
🏁 Script executed:
# Comprehensive search to confirm FetchStockDataParams is truly undefined
rg "FetchStockDataParams" --type ts --type tsx
# Look at the full execute function context to understand what params should be
sed -n '79,110p' src/mastra/workflows/stock-analysis-workflow.ts
# Search for any type aliases or interfaces defined in the file
cat src/mastra/workflows/stock-analysis-workflow.ts | grep -n "^type \|^interface " | head -20Repository: ssdeanx/AgentStack
Length of output: 1321
🏁 Script executed:
# Search for how other similar execute functions are typed
rg -B 5 -A 5 "execute: async" src/mastra/workflows/ --type ts | grep -E "(execute:|as \w+Params|context|inputData|writer)" | head -40
# Check entire file structure to see all steps and their execute patterns
wc -l src/mastra/workflows/stock-analysis-workflow.tsRepository: ssdeanx/AgentStack
Length of output: 3977
🏁 Script executed:
# Examine lines around the other execute functions in stock-analysis-workflow.ts to confirm the pattern difference
grep -n "execute: async" src/mastra/workflows/stock-analysis-workflow.tsRepository: ssdeanx/AgentStack
Length of output: 296
Use consistent parameter destructuring instead of undefined type casts.
Lines 91-92 use an undefined FetchStockDataParams type that doesn't exist in the codebase. Other execute functions in this file (lines 199, 356, 515) use direct destructuring: async ({ inputData, writer, context }) =>. Refactor to match the established pattern:
execute: async ({ inputData, writer, context }) => {
🤖 Prompt for AI Agents
In @src/mastra/workflows/stock-analysis-workflow.ts around lines 90 - 92, The
execute handler currently uses an undefined type cast via "params" and
"FetchStockDataParams"; refactor the function signature to use direct parameter
destructuring like the other handlers: change "execute: async (params) => {
const { inputData, writer } = params as FetchStockDataParams; const context =
(params as FetchStockDataParams).context" to a single destructured parameter
"execute: async ({ inputData, writer, context }) =>" and remove all references
to FetchStockDataParams in this function so it matches the pattern used by the
other execute functions.
| // Pipe partial text deltas into the workflow writer for live progress if provided | ||
| await stream.textStream?.pipeTo?.(writer) |
There was a problem hiding this comment.
Stream piping may fail silently.
stream.textStream?.pipeTo?.(writer) uses optional chaining but doesn't handle errors from the pipe operation. If the writer is unavailable or the pipe fails, this fails silently.
🔧 Add error handling
- await stream.textStream?.pipeTo?.(writer)
+ if (stream.textStream && writer) {
+ try {
+ await stream.textStream.pipeTo(writer)
+ } catch (pipeError) {
+ // Log but don't fail the step for streaming errors
+ logError('run-analysis', pipeError, {
+ symbol: inputData.stockData.symbol,
+ phase: 'stream-pipe'
+ })
+ }
+ }🤖 Prompt for AI Agents
In @src/mastra/workflows/stock-analysis-workflow.ts around lines 428 - 429, The
optional pipe call await stream.textStream?.pipeTo?.(writer) may fail silently;
wrap the pipe invocation in an explicit presence check and try/catch: first
ensure stream?.textStream and stream.textStream.pipeTo and writer exist, then
call const pipePromise = stream.textStream.pipeTo(writer) and await it inside a
try/catch block so any rejection is handled; in the catch block log the error
using the module's logger (or rethrow) to surface failures instead of letting
them fail silently.
![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 significant updates across the Mastra project, focusing on enhanced observability, improved HTTP client reliability, and expanded agent capabilities with comprehensive testing. Key changes include adding new agents for academic research, data processing, financial analysis, and web research, along with new agents for social media, SEO, translation, customer support, and project management. The core Mastra instance (src/mastra/index.ts) has been updated to register these new agents and workflows, and its observability configuration now leverages OtelBridge and SensitiveDataFilter. A new shared HTTP client (src/mastra/lib/http-client.ts) has been implemented using axios with axios-retry and bottleneck for rate-limited and retrying fetch operations. Many existing tools (alpha-vantage, arxiv, polygon, finnhub, web-scraper, etc.) have been refactored to utilize this new httpFetch client, incorporate TracingContext for better observability, and improve error handling and progress reporting. New tracing requirements and a checklist for PRs modifying tools have been added to src/mastra/tools/AGENTS.md. Additionally, a large number of new unit tests have been added or updated for various tools (alpha-vantage, arxiv, calendar, color-change, confirmation, cytoscape, data-file-manager, evaluateResultTool, finnhub, semantic-utils, serpapi-config, serpapi-shopping, url-tool, web-scraper, write-note), ensuring broader test coverage and adherence to new tracing standards. Several commented-out code blocks and imports were removed for cleaner code. Review comments highlighted the need to consistently include the context parameter in workflow step execute functions for tracing, remove commented-out code, and ensure proper Markdown formatting in the changelog. Minor suggestions were also made to improve type safety and clarity in HTTP response handling and JSDOM mocking.
| }), | ||
| outputSchema: stockDataSchema, | ||
| retries: 3, | ||
| execute: async (params) => { |
There was a problem hiding this comment.
The execute function for fetchStockDataStep is missing the context parameter in its destructuring. This is necessary to access the tracingContext for span creation and management. Please update the signature to include context.
execute: async (params) => {
const { inputData, writer, context } = params as FetchStockDataParams| }, | ||
| }) | ||
|
|
||
| const generateFinancialReportStep = createStep({ |
There was a problem hiding this comment.
The generateFinancialReportStep is missing a context parameter in its execute function signature. This is necessary to access tracingContext and other execution context properties. Please update the signature to include context.
execute: async ({ inputData, writer, context }) => {| financial: financialDataSchema, | ||
| technical: technicalAnalysisSchema, | ||
| }), | ||
| execute: async ({ inputData, writer }) => { |
There was a problem hiding this comment.
The performTechnicalAnalysisStep is missing a context parameter in its execute function signature. This is necessary to access tracingContext and other execution context properties. Please update the signature to include context.
| execute: async ({ inputData, writer }) => { | |
| execute: async ({ inputData, writer, context }) => { |
| generatedAt: z.string(), | ||
| }) | ||
|
|
||
| const fetchFinancialDataStep = createStep({ |
There was a problem hiding this comment.
The fetchFinancialDataStep is missing a context parameter in its execute function signature. This is necessary to access tracingContext and other execution context properties. Please update the signature to include context.
| const fetchFinancialDataStep = createStep({ | |
| const fetchFinancialDataStep = createStep({ | |
| id: 'fetch-financial-data', | |
| description: 'Fetches financial data using Polygon tools', | |
| inputSchema: z.object({ | |
| symbol: z.string(), | |
| }), | |
| outputSchema: financialDataSchema, | |
| retries: 3, | |
| execute: async ({ inputData, writer, context }) => { |
| onInputStart: ({ toolCallId, abortSignal }) => { | ||
| log.info('API data fetcher tool input streaming started', { | ||
| toolCallId, | ||
| messageCount: messages.length, | ||
| abortSignal: abortSignal?.aborted, | ||
| hook: 'onInputStart', | ||
| }) |
There was a problem hiding this comment.
The onInputStart hook is missing the messages parameter in its signature, but it's used in the log.info call. This could lead to a runtime error or unexpected behavior if messages is accessed when not provided. Please add messages: unknown[] to the parameter list.
onInputStart: ({ toolCallId, messages, abortSignal }: { toolCallId: string; messages: unknown[]; abortSignal?: AbortSignal }) => {| - [ ] Fixing TypeScript errors in fs.tool.test.ts (tracingContext.currentSpan type incompatibility) | ||
|
|
||
| ### Blocked | ||
| - Type errors in test file: `tracingContext.currentSpan` mock is missing Span properties (isInternal, observabilityInstance, end, error, update, and 16 more) | ||
| - Lifecycle hooks `onInputStart` and `onInputAvailable` not exposed on Tool type for direct testing |
There was a problem hiding this comment.
The 'Blocked' section mentions tracingContext.currentSpan mock missing properties. This issue has been addressed in the fs.tool.test.ts file by casting the mock to any. Please update this section to reflect that the issue is resolved and the tests are passing.
| 1. Fix TypeScript errors in `fs.tool.test.ts` by using `as any` casts for context objects | ||
| 2. Remove direct lifecycle hook tests (can't access hooks on Tool type) |
There was a problem hiding this comment.
The next step to 'Fix TypeScript errors in fs.tool.test.ts' has been completed by casting the mock to any. Please update this step to reflect the current status.
| **Type error encountered:** | ||
| ``` | ||
| Type '{ createChildSpan: Mock<Procedure>; }' is not assignable to parameter of type 'Span<keyof SpanTypeMap>'. | ||
| Missing properties: isInternal, observabilityInstance, end, error, update, and 16 more. | ||
| ``` |
There was a problem hiding this comment.
The 'Type error encountered' section describes an issue that has been resolved by casting the mock to any. Please update this section to reflect that the issue is resolved.
| **Fix needed:** Cast context mocks with `as any`: | ||
| ```typescript | ||
| tracingContext: { | ||
| currentSpan: { | ||
| createChildSpan: vi.fn().mockReturnValue({ | ||
| update: vi.fn(), | ||
| end: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| }, | ||
| } as any |
There was a problem hiding this comment.
The 'Fix needed' section describes the solution that has been implemented. Please update this section to reflect that the fix has been applied.
| - `src/mastra/tools/tests/fs.tool.test.ts` - Created test file (needs fixes) | ||
| - `src/mastra/tools/tests/AGENTS.md` - (Read only, not modified) |
There was a problem hiding this comment.
The 'Modified' section lists src/mastra/tools/tests/fs.tool.test.ts as 'Created test file (needs fixes)'. Since the fixes have been applied, please update this to reflect the current status, e.g., 'Updated test file'.
url-tool.test.tsto cover validation, parsing, normalization, extraction, and manipulation of URLs with multiple operations and tracing.write-note.test.ts, ensuring proper file operations and tracing context.fsmodule for file operations in the write-note tool tests.feat: implement financial analysis workflow
financial-analysis-workflow.tsto integrate financial data fetching, technical analysis, and report generation.docs: update session notes for continuity
CONTINUITY_ses_450b.mdregarding test coverage for Mastra tools.CONTINUITY_ses_4519.md.