feat(mastracode): auth routing fix, tool injection, and auth storage init#13611
Merged
Conversation
* feat(harness): rename images→files and use AI SDK FilePart shape - sendMessage now accepts `files` (matching AI SDK FilePart: data, mediaType, filename) instead of `images` (custom shape with mimeType field mismatch) - Mapping is now a direct passthrough instead of pointless field rename - Add `type: 'file'` to HarnessMessageContent union for generic file parts - Handle `file` parts in convertToHarnessMessage for history reload * fix(mastracode): remap images→files in TUI fireMessage The harness sendMessage param was renamed from `images` (custom shape) to `files` (AI SDK FilePart shape). Remap at the TUI boundary so clipboard paste still works.
- Add afterEach cleanup in auth storage test for isolation - Skip malformed file parts (non-string data) instead of silently coercing - Add changesets for mastracode (minor) and @mastra/core (patch) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 87c626f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@Kitenite is attempting to deploy a commit to the Mastra Team on Vercel. A member of the Team first needs to authorize it. |
fix(mastracode): use auth storage as source of truth for anthropic/openai auth
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds OpenAI API-key path and getOpenAIApiKey(), exports createAuthStorage() and wires credential-driven auth, wraps tools with pre/post hooks and disabled-tools filtering, renames harness message parts from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.changeset/thin-walls-bet.md (1)
1-2: Empty frontmatter — changeset may be unnecessary for infrastructure-only changes.The YAML frontmatter has no packages listed. If this GitHub Action workflow doesn't affect any published package, a changeset file may not be needed at all since changesets are used to generate release notes for npm packages. Consider removing this file if no package changelog entries are required.
If a changeset is intentionally desired for documentation purposes, ensure the frontmatter explicitly declares at least one package with a change type (e.g.,
patch).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/thin-walls-bet.md around lines 1 - 2, The changeset file has empty YAML frontmatter and likely isn't needed for infra-only changes; either delete .changeset/thin-walls-bet.md if it doesn't affect any published package, or replace the empty frontmatter with a proper entry that names the affected package and a change type (e.g., add "packages/my-package: patch" or similar frontmatter with a valid package id and "patch/minor/major") so the changeset will be picked up for release notes.mastracode/src/tui/commands/models-pack.ts (1)
472-484: Extract provider-access mapping into one shared helper.This access-level block now duplicates
buildProviderAccessinmastracode/src/tui/mastra-tui.ts. Centralizing it will prevent future drift in provider IDs/precedence (for example,openai-codexmapping).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mastracode/src/tui/commands/models-pack.ts` around lines 472 - 484, Extract the duplicated provider-access logic into a single shared helper by creating/using a common function (e.g., buildProviderAccess) instead of the local accessLevel/access implementation; replace the local const accessLevel and const access in models-pack.ts with a call to the shared buildProviderAccess helper so provider ID-to-access precedence (like 'openai-codex') is centralized and reuseable across mastracode/src/tui/mastra-tui.ts and this file, preserving the same return types (ProviderAccessLevel/ProviderAccess) and behavior for providers anthropic, openai, cerebras, google, and deepseek.mastracode/src/agents/model.ts (1)
129-145: You can simplify duplicate Anthropic API-key resolution.
storedCredis already read in this branch, so callinggetAnthropicApiKey()immediately afterward re-checks the same source with equivalent logic.♻️ Optional simplification
} else if (isAnthropicModel) { const bareModelId = modelId.substring('anthropic/'.length); const storedCred = authStorage.get('anthropic'); // Primary path: explicit OAuth credential if (storedCred?.type === 'oauth') { return opencodeClaudeMaxProvider(bareModelId); } - // Secondary path: explicit stored API key credential - if (storedCred?.type === 'api_key' && storedCred.key.trim().length > 0) { - return anthropicApiKeyProvider(bareModelId, storedCred.key.trim()); - } - - // Fallback: direct API key from AuthStorage - const apiKey = getAnthropicApiKey(); + const apiKey = storedCred?.type === 'api_key' ? storedCred.key.trim() : undefined; if (apiKey) { return anthropicApiKeyProvider(bareModelId, apiKey); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mastracode/src/agents/model.ts` around lines 129 - 145, The branch redundantly re-reads Anthropic API key via getAnthropicApiKey() after already fetching storedCred; change the logic to use storedCred.key when storedCred?.type === 'api_key' (trim and validate) and only call getAnthropicApiKey() when storedCred is undefined/not present, then pass that key into anthropicApiKeyProvider(bareModelId, key) or return opencodeClaudeMaxProvider(bareModelId) for storedCred?.type === 'oauth'; update the block using the existing symbols storedCred, getAnthropicApiKey, anthropicApiKeyProvider, opencodeClaudeMaxProvider and bareModelId so you no longer duplicate the same storage lookup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/real-wolves-thank.md:
- Around line 2-8: Rewrite the changeset to be outcome-focused and
package-scoped: for the mastracode entry describe the developer-visible outcomes
(e.g., "mastracode now runs tool executions with configurable pre/post hooks via
HookManager" and "developers can initialize auth storage directly using exported
createAuthStorage for standalone auth providers"), remove internal dependency
bookkeeping language, and ensure the `@mastra/core` frontmatter is justified by
stating that the release includes a patch-level peer dependency bump required by
mastracode (or any consumer-facing API change in `@mastra/core` if applicable);
make sure the narrative only describes effects relevant to the packages listed
in the frontmatter and adjust the frontmatter if any package mentioned does not
actually change.
---
Nitpick comments:
In @.changeset/thin-walls-bet.md:
- Around line 1-2: The changeset file has empty YAML frontmatter and likely
isn't needed for infra-only changes; either delete .changeset/thin-walls-bet.md
if it doesn't affect any published package, or replace the empty frontmatter
with a proper entry that names the affected package and a change type (e.g., add
"packages/my-package: patch" or similar frontmatter with a valid package id and
"patch/minor/major") so the changeset will be picked up for release notes.
In `@mastracode/src/agents/model.ts`:
- Around line 129-145: The branch redundantly re-reads Anthropic API key via
getAnthropicApiKey() after already fetching storedCred; change the logic to use
storedCred.key when storedCred?.type === 'api_key' (trim and validate) and only
call getAnthropicApiKey() when storedCred is undefined/not present, then pass
that key into anthropicApiKeyProvider(bareModelId, key) or return
opencodeClaudeMaxProvider(bareModelId) for storedCred?.type === 'oauth'; update
the block using the existing symbols storedCred, getAnthropicApiKey,
anthropicApiKeyProvider, opencodeClaudeMaxProvider and bareModelId so you no
longer duplicate the same storage lookup.
In `@mastracode/src/tui/commands/models-pack.ts`:
- Around line 472-484: Extract the duplicated provider-access logic into a
single shared helper by creating/using a common function (e.g.,
buildProviderAccess) instead of the local accessLevel/access implementation;
replace the local const accessLevel and const access in models-pack.ts with a
call to the shared buildProviderAccess helper so provider ID-to-access
precedence (like 'openai-codex') is centralized and reuseable across
mastracode/src/tui/mastra-tui.ts and this file, preserving the same return types
(ProviderAccessLevel/ProviderAccess) and behavior for providers anthropic,
openai, cerebras, google, and deepseek.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.changeset/huge-boxes-travel.md.changeset/real-wolves-thank.md.changeset/thin-walls-bet.md.github/workflows/sync-upstream-fork.ymlREADME.mdmastracode/src/__tests__/create-auth-storage.test.tsmastracode/src/agents/__tests__/model.test.tsmastracode/src/agents/__tests__/tools.test.tsmastracode/src/agents/model.tsmastracode/src/agents/tools.tsmastracode/src/index.tsmastracode/src/providers/claude-max.tsmastracode/src/providers/openai-codex.tsmastracode/src/tui/commands/models-pack.tsmastracode/src/tui/mastra-tui.tspackages/core/src/harness/harness.tspackages/core/src/harness/types.ts
# Conflicts: # packages/core/src/harness/harness.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/harness/harness.ts`:
- Around line 1317-1323: The code in harness.ts currently drops a part when
part.data is not a string; update the handling around the conditional that
builds the file object (the branch that does content.push({ type: 'file', data:
part.data, mediaType: ... })) to detect and convert known binary shapes instead
of silently skipping: accept ArrayBuffer/Uint8Array/Buffer and convert them to a
base64 or UTF-8 string as appropriate (or wrap binary payloads with a clear
binary marker), only skip when the shape is truly unsupported, and ensure
mediaType is preserved; modify the logic around part.data checks and the
content.push call to perform these conversions and fallback logging/error for
unsupported types.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mastracode/src/agents/tools.tspackages/core/src/harness/harness.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- mastracode/src/agents/tools.ts
Comment on lines
+1317
to
1323
| if (typeof part.data !== 'string') { | ||
| break; | ||
| } | ||
| content.push({ | ||
| type: 'file', | ||
| data: typeof part.data === 'string' ? part.data : '', | ||
| data: part.data, | ||
| mediaType: |
Contributor
There was a problem hiding this comment.
Avoid silently dropping non-string file payloads.
Line 1317 currently skips the part entirely when part.data is not a string. That can lose file content in history for binary-backed file parts. Prefer converting known binary types and only skipping truly unsupported shapes.
Suggested patch
- case 'file':
- if (typeof part.data !== 'string') {
- break;
- }
- content.push({
- type: 'file',
- data: part.data,
+ case 'file': {
+ let fileData: string | undefined;
+ if (typeof part.data === 'string') {
+ fileData = part.data;
+ } else if (part.data instanceof Uint8Array) {
+ fileData = Buffer.from(part.data).toString('base64');
+ } else if (part.data instanceof ArrayBuffer) {
+ fileData = Buffer.from(new Uint8Array(part.data)).toString('base64');
+ }
+ if (!fileData) {
+ break;
+ }
+ content.push({
+ type: 'file',
+ data: fileData,
mediaType:
(part as { mediaType?: string }).mediaType ??
(part as { mimeType?: string }).mimeType ??
'application/octet-stream',
...((part as { filename?: string }).filename ? { filename: (part as { filename?: string }).filename } : {}),
});
- break;
+ break;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (typeof part.data !== 'string') { | |
| break; | |
| } | |
| content.push({ | |
| type: 'file', | |
| data: typeof part.data === 'string' ? part.data : '', | |
| data: part.data, | |
| mediaType: | |
| case 'file': { | |
| let fileData: string | undefined; | |
| if (typeof part.data === 'string') { | |
| fileData = part.data; | |
| } else if (part.data instanceof Uint8Array) { | |
| fileData = Buffer.from(part.data).toString('base64'); | |
| } else if (part.data instanceof ArrayBuffer) { | |
| fileData = Buffer.from(new Uint8Array(part.data)).toString('base64'); | |
| } | |
| if (!fileData) { | |
| break; | |
| } | |
| content.push({ | |
| type: 'file', | |
| data: fileData, | |
| mediaType: | |
| (part as { mediaType?: string }).mediaType ?? | |
| (part as { mimeType?: string }).mimeType ?? | |
| 'application/octet-stream', | |
| ...((part as { filename?: string }).filename ? { filename: (part as { filename?: string }).filename } : {}), | |
| }); | |
| break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/harness/harness.ts` around lines 1317 - 1323, The code in
harness.ts currently drops a part when part.data is not a string; update the
handling around the conditional that builds the file object (the branch that
does content.push({ type: 'file', data: part.data, mediaType: ... })) to detect
and convert known binary shapes instead of silently skipping: accept
ArrayBuffer/Uint8Array/Buffer and convert them to a base64 or UTF-8 string as
appropriate (or wrap binary payloads with a clear binary marker), only skip when
the shape is truly unsupported, and ensure mediaType is preserved; modify the
logic around part.data checks and the content.push call to perform these
conversions and fallback logging/error for unsupported types.
# Conflicts: # packages/core/src/harness/harness.ts
# Conflicts: # packages/core/src/harness/harness.ts
feat(mastracode): add disabledTools API and extract permission rule utils
…ain-upstream # Conflicts: # mastracode/src/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mastracode/src/agents/tools.ts`:
- Around line 22-60: The wrapToolWithHooks and createDynamicTools APIs use broad
any types allowing invalid tool shapes at compile time; define a proper Tool
interface (e.g., interface Tool { execute(input: unknown, context?: unknown):
Promise<unknown> | unknown; [key: string]: unknown }) and replace usages of any:
change wrapToolWithHooks(toolName: string, tool: Tool, hookManager?:
HookManager): Tool and type extraTools as Record<string, Tool>; ensure
hookManager is typed as HookManager | undefined and adjust internal variables
(output, preResult) to use appropriate types from the Tool and HookManager
contracts so TypeScript enforces the execute method shape instead of allowing
any.
In `@mastracode/src/index.ts`:
- Around line 56-57: The subagent path is not applying the disabledTools filter,
so models can still reach tools like execute_command, write_file, and
string_replace_lsp via subagent; update the code that builds the subagent tool
maps (the code that registers/returns tools for subagent execution) to apply the
same disabledTools filtering logic used by createDynamicTools: when constructing
the subagent tool map, remove any tool whose key/name appears in disabledTools
(not just the three hardcoded names) and reuse the same predicate or helper used
by createDynamicTools to keep behavior consistent across both main and subagent
tool sets.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mastracode/src/agents/__tests__/extra-tools.test.tsmastracode/src/agents/tools.tsmastracode/src/index.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
mastracode/src/index.ts (1)
122-129:⚠️ Potential issue | 🟠 MajorApply
disabledToolsfiltering to subagent tool maps too.Line 128 applies
disabledToolsonly to the main dynamic toolset. Subagent maps (Lines 168-175) still expose tool entries directly, so disabled tools can be reached through delegation.Suggested fix
+ const disabledToolSet = new Set(config?.disabledTools ?? []); + const filterDisabledTools = <T extends Record<string, unknown>>(toolMap: T): Partial<T> => + Object.fromEntries(Object.entries(toolMap).filter(([name]) => !disabledToolSet.has(name))) as Partial<T>; @@ - const readOnlyTools = { + const readOnlyTools = filterDisabledTools({ view: viewTool, search_content: grepTool, find_files: globTool, - }; + }); @@ - tools: { + tools: { ...readOnlyTools, - string_replace_lsp: stringReplaceLspTool, - write_file: writeFileTool, - execute_command: executeCommandTool, + ...filterDisabledTools({ + string_replace_lsp: stringReplaceLspTool, + write_file: writeFileTool, + execute_command: executeCommandTool, + }), task_write: taskWriteTool, task_check: taskCheckTool, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mastracode/src/index.ts` around lines 122 - 129, The main Agent is correctly created with createDynamicTools(..., config?.disabledTools) but the subagent tool maps are not filtered, allowing disabled tools to be reachable via delegation; update the code that builds subagent tool maps (where subagents are constructed—look for the code that creates per-subagent tool maps around the subagent creation block) to call the same filtering logic (reuse createDynamicTools or the internal filter used by createDynamicTools) and pass config?.disabledTools so each subagent's tools explicitly exclude disabled entries; ensure both the main Agent instantiation (Agent) and all subagent tool map constructions use the same disabledTools parameter.
🧹 Nitpick comments (2)
mastracode/src/agents/__tests__/model.test.ts (1)
197-233: Consider adding whitespace-trimming key tests for both key getters.Both getters trim keys before returning; adding one case like
' sk-key '(and optionally whitespace-only) would lock that behavior down.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mastracode/src/agents/__tests__/model.test.ts` around lines 197 - 233, Add unit tests for getAnthropicApiKey and getOpenAIApiKey that verify they trim surrounding whitespace and handle whitespace-only values: mock mockAuthStorageInstance.get to return { type: 'api_key', key: ' sk-trimmed-key ' } and assert the getters return 'sk-trimmed-key', and also mock a whitespace-only key like ' ' and assert the getters return undefined; place these new cases alongside the existing tests for getAnthropicApiKey and getOpenAIApiKey to lock down trimming behavior.mastracode/src/agents/model.ts (1)
154-163: Remove redundant Anthropic API-key fallback check.Line 147 already reads
storedCredafterauthStorage.reload(), and Lines 155-157 already handle theapi_keycase. ThegetAnthropicApiKey()fallback at Line 160 currently re-checks the same condition.Refactor
- // Fallback: direct API key from AuthStorage - const apiKey = getAnthropicApiKey(); - if (apiKey) { - return anthropicApiKeyProvider(bareModelId, apiKey); - } // No auth configured — attempt OAuth provider which will prompt login return opencodeClaudeMaxProvider(bareModelId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mastracode/src/agents/model.ts` around lines 154 - 163, The fallback call to getAnthropicApiKey() is redundant because storedCred (reloaded from authStorage) is already checked for type 'api_key' and handled by anthropicApiKeyProvider; remove the fallback block that reads getAnthropicApiKey() and returns anthropicApiKeyProvider(bareModelId, apiKey). Keep the existing storedCred check (storedCred?.type === 'api_key') and its call to anthropicApiKeyProvider; if you intended a true fallback for non-stored credentials, instead change the logic to only call getAnthropicApiKey() when storedCred is null/undefined before invoking anthropicApiKeyProvider.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@mastracode/src/index.ts`:
- Around line 122-129: The main Agent is correctly created with
createDynamicTools(..., config?.disabledTools) but the subagent tool maps are
not filtered, allowing disabled tools to be reachable via delegation; update the
code that builds subagent tool maps (where subagents are constructed—look for
the code that creates per-subagent tool maps around the subagent creation block)
to call the same filtering logic (reuse createDynamicTools or the internal
filter used by createDynamicTools) and pass config?.disabledTools so each
subagent's tools explicitly exclude disabled entries; ensure both the main Agent
instantiation (Agent) and all subagent tool map constructions use the same
disabledTools parameter.
---
Nitpick comments:
In `@mastracode/src/agents/__tests__/model.test.ts`:
- Around line 197-233: Add unit tests for getAnthropicApiKey and getOpenAIApiKey
that verify they trim surrounding whitespace and handle whitespace-only values:
mock mockAuthStorageInstance.get to return { type: 'api_key', key: '
sk-trimmed-key ' } and assert the getters return 'sk-trimmed-key', and also
mock a whitespace-only key like ' ' and assert the getters return undefined;
place these new cases alongside the existing tests for getAnthropicApiKey and
getOpenAIApiKey to lock down trimming behavior.
In `@mastracode/src/agents/model.ts`:
- Around line 154-163: The fallback call to getAnthropicApiKey() is redundant
because storedCred (reloaded from authStorage) is already checked for type
'api_key' and handled by anthropicApiKeyProvider; remove the fallback block that
reads getAnthropicApiKey() and returns anthropicApiKeyProvider(bareModelId,
apiKey). Keep the existing storedCred check (storedCred?.type === 'api_key') and
its call to anthropicApiKeyProvider; if you intended a true fallback for
non-stored credentials, instead change the logic to only call
getAnthropicApiKey() when storedCred is null/undefined before invoking
anthropicApiKeyProvider.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
mastracode/src/agents/__tests__/extra-tools.test.tsmastracode/src/agents/__tests__/model.test.tsmastracode/src/agents/model.tsmastracode/src/agents/tools.tsmastracode/src/index.tsmastracode/src/tui/mastra-tui.tspackages/core/src/harness/harness.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- mastracode/src/agents/tests/extra-tools.test.ts
- mastracode/src/agents/tools.ts
- packages/core/src/harness/harness.ts
Member
|
@Kitenite please address coderabbit comments |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
mastracode/src/index.ts (1)
63-64:⚠️ Potential issue | 🟠 MajorApply
disabledToolsfiltering to subagent tool maps too.
disabledToolsis only enforced viacreateDynamicTools(Line 130). Subagent maps still expose unfiltered tools, so hard-disabled tools can be reached through subagent execution.Suggested patch
+ const disabledToolSet = new Set(config?.disabledTools ?? []); + const filterDisabledTools = <T extends Record<string, unknown>>(toolMap: T) => + Object.fromEntries(Object.entries(toolMap).filter(([name]) => !disabledToolSet.has(name))); - const readOnlyTools = { + const readOnlyTools = filterDisabledTools({ view: viewTool, search_content: grepTool, find_files: globTool, - }; + }); @@ tools: { ...readOnlyTools, - string_replace_lsp: stringReplaceLspTool, - write_file: writeFileTool, - execute_command: executeCommandTool, - task_write: taskWriteTool, - task_check: taskCheckTool, + ...filterDisabledTools({ + string_replace_lsp: stringReplaceLspTool, + write_file: writeFileTool, + execute_command: executeCommandTool, + task_write: taskWriteTool, + task_check: taskCheckTool, + }), },Also applies to: 124-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mastracode/src/index.ts` around lines 63 - 64, The subagent tool maps are not being filtered by disabledTools (filtering only happens in createDynamicTools), so update the code that builds subagent tool maps (e.g., wherever subagent tool map construction or a function like createSubagentToolMaps / buildSubagentTools is defined) to apply the same disabledTools filtering logic: either call/createDynamicTools for each subagent or reuse the exact predicate that removes tools present in disabledTools when assembling subagentMaps, handle the optional/undefined disabledTools safely, and add/update tests to ensure tools listed in disabledTools are not present in any subagent map.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@mastracode/src/index.ts`:
- Around line 63-64: The subagent tool maps are not being filtered by
disabledTools (filtering only happens in createDynamicTools), so update the code
that builds subagent tool maps (e.g., wherever subagent tool map construction or
a function like createSubagentToolMaps / buildSubagentTools is defined) to apply
the same disabledTools filtering logic: either call/createDynamicTools for each
subagent or reuse the exact predicate that removes tools present in
disabledTools when assembling subagentMaps, handle the optional/undefined
disabledTools safely, and add/update tests to ensure tools listed in
disabledTools are not present in any subagent map.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mastracode/src/index.tspackages/core/src/harness/harness.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/harness/harness.ts
Kitenite
changed the title
- Apply disabledTools filtering to subagent tool maps so disabled tools can't be bypassed through subagent delegation - Replace broad `any` types with ToolLike interface in tools.ts - Add console.warn for non-string file payloads in harness stream parser - Remove invalid changeset with empty frontmatter (infra-only change) - Accept undefined in setAuthStorage to enable clean test teardown - Remove `as any` casts in create-auth-storage test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Kitenite
changed the title
Kitenite
changed the title
- Merged upstream's function-form extraTools support with ToolLike types - Fixed upstream test that asserted on workspace-provided tool (view) instead of a createDynamicTools tool (web_search) - Removed duplicate changeset (huge-boxes-travel.md) - Updated real-wolves-thank.md to be mastracode-scoped only Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Kitenite
commented
Mar 3, 2026
| // Test environment: use API key | ||
| if (process.env.NODE_ENV === 'test' || process.env.VITEST) { | ||
| const anthropic = createAnthropic({ | ||
| apiKey: process.env.ANTHROPIC_API_KEY || 'test-api-key', |
Contributor
Author
There was a problem hiding this comment.
Btw lmk if this is not ok, it was picking up real api keys in my test so i removed it
Remove README fork note and sync-upstream-fork workflow — these are fork-specific and should not be upstreamed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Kitenite
commented
Mar 3, 2026
|
|
||
| Information about the project setup can be found in the [development documentation](./DEVELOPMENT.md) | ||
|
|
||
| For fork maintainers: this repository includes a daily GitHub Action at `.github/workflows/sync-upstream-fork.yml` that syncs your fork's default branch from `mastra-ai/mastra`. |
Contributor
Author
There was a problem hiding this comment.
Gonna remove this real quick
| @@ -0,0 +1,52 @@ | |||
| name: Sync Fork With Upstream | |||
DanielSLew
approved these changes
Mar 4, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep ToolLike type from PR and request_access rename from upstream. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wardpeet
pushed a commit
that referenced
this pull request
Mar 9, 2026
…init (#13611) Co-authored-by: Satya Patel <satyapatel111@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Roamin <97863888+roaminro@users.noreply.github.com> Co-authored-by: Daniel Lew <51924260+DanielSLew@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR combines two efforts: auth routing fixes and external tool injection / auth storage init.
Auth routing fix (was #13611)
Fixes Anthropic/OpenAI auth resolution by making AuthStorage credential type (
api_keyvsoauth) the authoritative signal for model routing and provider access checks. Previously, API-key auth could be routed through OAuth bearer-token paths, causing401 Invalid bearer tokenerrors.External tool injection & auth storage init (was #13561)
createAuthStorage()for standalone auth provider initializationextraTools(static or function) andHookManagerpre/post hooks increateDynamicToolsimages→filesin harnesssendMessageto use AI SDKFilePartshapedisabledToolsfiltering to subagent tool maps to prevent bypass through delegationChanges
mastracode/src/agents/model.ts— Direct OpenAI API-key model path; resolve auth via AuthStoragemastracode/src/agents/tools.ts—ToolLikeinterface,wrapToolWithHooks, function-formextraToolsmastracode/src/index.ts—createAuthStorageexport,filterDisabledfor subagent tools, typed configmastracode/src/providers/claude-max.ts,openai-codex.ts—setAuthStorageacceptsundefinedmastracode/src/tui/— Provider access checks aligned with stored credential typepackages/core/src/harness/harness.ts—filesinstead ofimages, warn on non-string file datapackages/core/src/harness/types.ts—filevariant inHarnessMessageContentValidation
Closes #13561
🤖 Generated with Claude Code