[MarcoDotIO]

Problem

When Claude's advisor tool call fails, the Anthropic API returns an advisor_tool_result whose content is an advisor_tool_result_error (for example overloaded, too_many_requests or max_uses_exceeded). The Claude adapter still settled that tool row as completed, so a failed advisor consult looked like a successful one.

To reproduce, set "advisorModel" in ~/.claude/settings.json. Then run a Claude thread where the model calls advisor and the advisor call fails. The row shows as completed.

Change

isClaudeTypedToolResultErrorContent in ClaudeAdapterV2.ts lists the *_tool_result_error content types for server tool results. It already handles advisor_tool_result blocks, but its error check covered every server tool's error block except the advisor's. The switch has a default: false arm, so the type checker never caught the gap. This change adds advisor_tool_result_error to the list, so the row settles as failed.

Scope and approval

This is a one-line fix for an obvious omission. The advisor result block is already handled in outputFromClaudeToolResult, isClaudeToolResultError and toolNameFromClaudeToolResult, and its error variant (BetaAdvisorToolResultError in @anthropic-ai/sdk) has the same shape as the other *_tool_result_error types already in the list. No behavior changes for any other tool. I found no existing issue or PR for it (searched "advisor_tool_result_error", "advisor tool result error", "advisor failed claude", "advisor completed error"; #24129 and #24139 are about advisor usage accounting, not tool status).

Verification

I added a focused adapter test, settles an advisor call from its result content, in ClaudeAdapterV2.test.ts. It streams a server_tool_use advisor call and then an advisor_tool_result through the wake harness, and checks how the dynamic_tool row settles: an error result must end as failed and an advisor_result must end as completed.

• Without the source fix (git show upstream/main:<ClaudeAdapterV2.ts>), node_modules/.bin/vp test run src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts -t advisor:
  × settles an advisor call from its result content: failed with AssertionError: expected 'completed' to equal 'failed' (1 failed, 1 passed).
• With the fix, the same command passes: 2 passed.
• vp test run src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts src/orchestration-v2/testkit/ClaudeReplayFixtures.integration.test.ts: 2 files, 248 tests passed.
• vp lint --report-unused-disable-directives apps/server/src/orchestration-v2/Adapters: no new findings. Two no-unused-vars warnings already exist on lines this change does not touch.
• vp fmt --check apps/server/src/orchestration-v2/Adapters: clean.
• tsc --noEmit in apps/server: exit 0.

I did not check this against a live advisor failure. The test uses the API's documented block shapes.

Claude Opus 5.5 (Claude Code desktop app)

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at fd30035

Macroscope's review found this PR approvable — This is a one-line production fix that correctly marks existing Claude advisor error results as failed, backed by a focused regression test. It does not introduce a new capability, change defaults, or affect unrelated tool handling.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 1b78b65
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: fad360d0-381a-4a01-a7de-c64edfc6d467

Commits

Reviewing files that changed from the base of the PR and between fd30035 and d2b7686.

Files selected for processing: 2

• apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
• apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Walkthrough

The Claude adapter now classifies advisor_tool_result_error as an error. A parameterized test checks that an advisor dynamic tool fails for an overload error and completes for a successful result.

Changes: Advisor result classification

Layer / File(s)	Summary
Classify and test advisor results apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts	The typed tool-result error switch now treats advisor_tool_result_error as an error. The test checks failed status for an overload error and completed status for a successful result.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Possibly related PRs

• pingdotgg/t3code#24328: Both changes update Claude result classification. This PR handles message.is_error result frames when the terminal reason is missing or completed.

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: Minimal · up to d2b76

The adapter change is mergeable; live advisor-provider behavior remains unverified but no source-level risk is established.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title is concise, specific, and accurately describes the main change: failed Claude advisor calls now report as failed instead of completed.
Description check		The description covers the problem, change, scope rationale, verification results, limitations, and the agent information required by the template.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

@MarcoDotIO, thanks for finding the missing advisor error discriminator and covering both error and success. i kept your implementation and tests unchanged, and merged current main into your branch without rewriting your commits. the final diff is still one production line plus 60 lines in the existing adapter test file.

verification at c98b82b2bd52c861ef67acd0a553013d2ae7a676:

• reproduced on main f40c7c6435: the error regression fails with expected failed, received completed; the success case passes.
• with the fix: 275 adapter and deterministic replay tests passed through blacksmith-check.
• vp run check:changed f40c7c6435e390fc91f1d7eac6caf61bd9fe0e0e passed through blacksmith-check: format, lint, both knip passes, graphql, affected typechecks and adapter tests. the explicit base supplies history absent from the worker snapshot.
• github ci: 20 successful, 8 skipped, no failures or pending checks. github reports clean and mergeable.
• two independent read-only gpt-6.1-sol reviews approved this exact head with no findings. the original bot reviews had no actionable findings; coderabbit's new review was seat-limited, so that fresh bot review remains unavailable.

unverified: a live claude advisor call, because claude is unauthenticated on this vm; electron, mobile, relay and tunnel were not launched. the server adapter supplies the same status contract to all clients. no contracts, other providers, controls or docs changed.

the screenshots below show the real web client at 1440 × 1000 with matching seeded tool data. they demonstrate rendering only; the adapter regression tests prove classification through sdk frames. the error row changes to the failed color while the successful advisor row remains unchanged.

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

checked github/main at 3ef16100796c77567ab8bdfeb05d77565ae02d53 first, then the exact pr head c98b82b2bd52c861ef67acd0a553013d2ae7a676. installed each checkout with vp i and ran one background dev server at a time with isolated state. drove the actual web client using the repo's playwright and /usr/bin/google-chrome. created projects and threads through the app; all provider turns were real, through this vm's local proxy. no mock frames, fixtures or demo data were injected.

on the pr head, claude sonnet 5.5 and codex gpt-6.1-sol each executed separate successful and failing shell calls. the server persisted the successful commands as completed and the exit-7 commands as failed. the web client showed the failure and its output. claude continued successfully in the same thread after the advisor attempt. reloading the codex thread preserved its results; the recording shows opening the persisted command group and failure details.

claude: printf neighbor-success -> completed
claude: sh -c 'printf neighbor-failure >&2; exit 7' -> failed
codex:  printf codex-neighbor-success -> completed, exit 0
codex:  sh -c 'printf codex-neighbor-failure >&2; exit 7' -> failed, exit 7

the existing advisor regression cases passed through blacksmith-check: 2 passed, covering error -> failed and success -> completed. these synthetic sdk-frame tests do not establish live advisor behavior. cafe reports 28 successful ci contexts, no failures or pending checks. no source changes or pushes were made.

short runtime transcript

Not verified

the changed advisor classification remains unverified. set advisorModel to claude-opus-5-5 and explicitly requested an advisor call on both main and the pr. both real claude sessions exposed no advisor tool; their sdk initialization frames confirm its absence, and tool search returned no matches. a separate claude cli attempt with an explicit --settings file also exposed no advisor. no advisor_tool_result or advisor_tool_result_error reached the adapter, so i could neither reproduce the original bug on main nor prove the fix with a live advisor failure. codex cannot exercise this claude-specific discriminator.

these matching-prompt captures show that environment limitation, not a successful before/after reproduction:

native electron, ios/android, relay and tunnel were not exercised. this pass cannot certify the pr's changed behavior; it needs an environment that exposes the real claude advisor tool. uploaded video playback and seeking were checked in chrome.