Skip to content

feat(server,contracts): add Claude advisor model setting - #9640

Closed
schorke wants to merge 1 commit into
pingdotgg:mainfrom
schorke:feat/claude-advisor-model
Closed

schorke wants to merge 1 commit into
pingdotgg:mainfrom
schorke:feat/claude-advisor-model

Conversation

@schorke

@schorke schorke commented Sep 4, 2026 •

Copy link
Copy Markdown

Problem

Claude Code can consult a stronger model in the middle of a turn through its advisor tool. T3 Code had no way to turn this on.

It was also already broken when it did run. CLAUDE_SETTING_SOURCES includes user, so a developer with advisorModel in ~/.claude/settings.json already got advisor consults inside T3 Code. The advisor is a server-side tool: the server_tool_use block opens the item, but the result arrives in the assistant stream as an advisor_tool_result block, never as a tool_result user message. handleUserMessage is the only path that completes a tool item, and it matches on tool_result, so nothing closed the item. The consult rendered as a generic "Tool call" and spun until the end-of-turn sweep swept it up — a lying spinner.

Fix

  • Adds an Advisor model field to the Claude provider settings, and passes it to the Agent SDK as the advisorModel settings key.
  • Completes the advisor item when its own advisor_tool_result block arrives, matched by tool_use_id rather than block index (the result lands at a different index from the request).
  • Titles the item "Advisor consult" instead of the generic tool-call label, and drops the input summary, which is always empty.

The settings key is deliberate, not the hidden --advisor CLI flag. The flag aborts session start when the advisor cannot advise the thread model; the settings path logs and runs the turn without an advisor, which is the behavior you want for a per-instance setting.

Before / after

Claude provider settings, Runtime section:

Before After

The field is derived from the contracts schema by deriveProviderSettingsFields, so web and desktop pick it up with no component change. Mobile has no provider config form, so there is nothing to add there.

Tests

  • ClaudeAdapter.test.ts: the advisor model reaches the SDK, is omitted when the field is empty, and the item completes on its result block. The completion test emits no result message on purpose, so the end-of-turn sweep cannot mask the bug. Stubbing the new branch out makes it hang and fail.
  • settings.test.ts: default, round-trip, and server-patch decoding for the new field.
  • ProviderSettingsForm.test.ts: field order.

Not in this PR

  • Advisor token and cost attribution. usage.iterations carries advisor_message entries and the final modelUsage gains a separate entry for the advisor model with its own costUSD, but emitThreadTokenUsage reads top-level usage only. Advisor spend stays invisible in the context meter. That is a separate change.
  • /advisor in the composer. Claude's init message lists it, so it appears in the command menu and does nothing useful, because it is an interactive picker that mutates Claude's own settings.json. That is why this PR is a setting and not a chat command. T3 Code forwards every Claude built-in unfiltered, so /model, /config, and about forty others behave the same way. Filtering interactive-only commands is a separate concern.

Written by Claude Opus 5 in Claude Code.


Note

Medium Risk
Touches Claude stream event handling and session settings for a server-side tool path; mistakes could leave tool items stuck or mis-complete unrelated tools, but scope is narrow and heavily tested.

Overview
Adds a configurable Advisor model on the Claude provider (contracts + settings UI field order) and forwards it to the Agent SDK as advisorModel when set, omitting it when empty so Claude Code’s own config can apply.

Fixes advisor timeline handling in ClaudeAdapter: the advisor is a server-side tool whose result arrives as an advisor_tool_result stream block (not a user tool_result). The adapter now completes the in-flight tool item when that block arrives (matched by tool_use_id), labels it Advisor consult without empty input detail, and keeps advisor_tool_result blocks from being dropped in subagent stream filtering.

Tests cover SDK option wiring, completion without an end-of-turn sweep, settings decode/patch, and provider form fields; user docs describe fable / opus / sonnet and capability rules.

Reviewed by Cursor Bugbot for commit ef51c0d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add advisorModel setting to Claude provider and handle advisor tool lifecycle in ClaudeAdapter

  • Adds advisorModel as a trimmed-string field to ClaudeSettings and ClaudeSettingsPatch in settings.ts, with an empty default and a form-field position between homePath and autoCompactWindow.
  • makeClaudeAdapter in ClaudeAdapter.ts now passes a non-empty advisorModel into Claude query settings, and handles advisor tool blocks: advisor tool starts use the title "Advisor consult" and omit the generic empty-input detail; advisor result blocks (type advisor_tool_result) from nested streams complete the in-flight advisor item by tool-use ID, emitting item.completed with no result text.
  • Adds user-facing documentation for the advisor feature in providers-claude.md.
  • Behavioral Change: completed advisor timeline items contain no advice text in their result detail; consumers that previously expected tool-result text for advisor consults will receive an empty result.

Macroscope summarized ef51c0d.

Claude Code can consult a stronger model mid-turn through its advisor tool,
but T3 Code had no way to turn it on, and a consult that did run (from the
user's own Claude settings) rendered as a generic "Tool call" that spun until
the end of the turn. The advisor is a server-side tool: its result arrives in
the assistant stream as an `advisor_tool_result` block, never as a
`tool_result` user message, so nothing ever closed the item.

Adds an "Advisor model" field to the Claude provider settings and passes it to
the Agent SDK as the `advisorModel` settings key. Completes the advisor item
on its own result block, and titles it "Advisor consult" instead of the
generic tool-call label.

Written by Claude Opus 5 in Claude Code.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 4, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ef51c0d. Configure here.

title: titleForTool(itemType),
detail,
title: isAdvisor ? "Advisor consult" : titleForTool(itemType),
...(detail ? { detail } : {}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisor title lost on input deltas

Medium Severity

The advisor start path sets title to Advisor consult and omits the empty input summary, but input_json_delta always recomputes title via titleForTool and detail via summarizeToolRequest. Those frames write the generic Tool call label and advisor: {} back onto the in-flight item, so item.completed publishes the label this change was meant to replace.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ef51c0d. Configure here.

@macroscopeapp

macroscopeapp Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a configurable second-model Claude advisor and changes shared stream-processing and timeline behavior, including additional model usage and cost. The new defaulted setting and an unresolved concrete concern about advisor titles being overwritten during input deltas require human review.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge

Copy link
Copy Markdown
Member

Thanks for the PR. We're not taking changes to the orchestration and provider layers right now: that part of the server is being rewritten for V2, and merging into the current code would either conflict with or be thrown away by that work.

Closing for now. If this is still an issue once V2 lands, please reopen (or open a fresh PR against the new code) and we'll take a proper look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants