Skip to content

fix(native-chat): a resting chat lists its model the way the running agent does, so no stray effort picker appears - #24267

Merged
brennanb2025 merged 4 commits into
mainfrom
brennanb2025/chat-options-at-rest-match-live
Oct 1, 2026
Merged

brennanb2025 merged 4 commits into
mainfrom
brennanb2025/chat-options-at-rest-match-live

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​123 $\color{#cf222e}{\Huge{\mathbf{−}}}$​10 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​113
Prod 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​61 $\color{#cf222e}{\Huge{\mathbf{−}}}$​18 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​43

What this fixes, and where

The composer's option pickers in a resting native Claude or Codex chat, in two cases: the chat's current model is one the model catalog does not list (for example a newer or pinned model id), or the host has no model catalog for the account yet.

A chat rests after the idle sweep today, and after every Stop once #24235 lands. A resting chat on a listed model whose catalog the host holds, PTY sessions, and running chats are unaffected.

ELI5

When a native chat has no live agent process (it is "resting"), the composer can show an extra effort picker ("Medium") that the same chat does not show while the agent is running.

The extra picker is a stray: Orca adds it only because the resting answer leaves the chat's model off the model list. It goes away again as soon as the agent starts.

This PR makes the resting answer list the chat's model exactly the way the running agent does, so the extra picker never appears.

This must land before #24235. That PR rests a Claude chat on every Stop, so without this fix the stray picker would show up right after every Stop.

What Changed

Before: a resting chat on an unlisted model showed an effort picker ("Medium") next to the model. Once the agent started again, the picker disappeared. A pick made in it was saved, but the user could no longer see or change it while the agent ran.

After: a resting chat shows the same pickers as the running chat. There is no effort picker for a model whose effort levels nobody reported, whether or not an agent is running. A listed model keeps its effort picker even before the host has a catalog for the account.

Mechanism:

  • There were three places that build the model list a chat's options answer carries: the running Claude adapter, the running Codex adapter, and the host's answer for a resting chat.
  • The two running adapters already added the current model with no effort levels when the catalog did not list it. The resting answer did not.
  • The client then filled the gap from its built-in defaults, which give an unknown Claude model an effort control.
  • All three now call one new host function, structuredAgentSessionOptionModels. It returns the catalog's rows plus the current model when the catalog does not list it, with no effort levels.
  • Running and resting answers now cannot disagree about that row.
  • When the host has no catalog for the account yet (the first read before its background probe returns, a failing probe, or a failed account lookup), the resting answer starts from the same list a running agent falls back to.
    • For Claude that is its built-in model list, with effort levels, so a listed model such as opus keeps its effort picker.
    • Only a real catalog names the account's default model. With no catalog and no saved pick, the resting answer names no model, so the client keeps the model the agent last reported instead of the built-in list's guess.
    • Codex has no built-in list on the host, so the resting Codex answer leaves the list empty, and the client fills the current model from its own unknown-model defaults. That is unchanged from before this PR.

Wire and versions: this is host-only and the options message is unchanged.

  • An old client reading a new host gets a model list that includes the current model, which old clients already handle, because running agents have always sent it.
  • A new client reading an old host keeps today's behaviour.

Why

The rule this enforces: whether a model has an effort control should depend on the model and the catalog, not on whether an agent process happens to be alive.

Why one shared function, not a patch to the resting path: the bug came from two copies of the same rule drifting apart. A shared function removes the drift instead of adding a third copy.

What I did not do:

  • Remove the client's built-in fallback for unknown models. The PTY launcher relies on it on purpose.
  • Reshape the options message. Neither is needed for this fix.

Not in scope: why a given Claude install reports its running model as an id its own catalog does not list. That is a separate question.

Differences from the common pattern

The mechanism matches the common pattern: the effort control is derived from the catalog and the selected model, the same way whether or not a session is live.

On a model the catalog does not list, the common pattern splits: one shows no effort control, the other shows the default model's levels. Intended: this PR keeps what Orca's running agents already do (no effort control), so the composer does not change across a Stop. Showing another model's levels would offer choices nothing says the chat's model accepts.

With no catalog at all, the common pattern falls back to a built-in list that has effort levels. This PR does the same for Claude, with the list a running Claude agent already falls back to. For Codex the host has no built-in list, so the client fills the current model from its own unknown-model defaults, unchanged from before this PR.

Linked Issue

Found while reviewing #24235 (Claude Stop ends the agent process). Needed before #24235 lands.

Visual Proof

Live Claude on an M4 Air, a chat whose model the catalog does not list (shown as the raw id), composer at rest (no Claude process for the chat). Builds: 2cedb9d; the head differs only in the no-catalog default model.

Before After
Main, at rest after relaunch: a phantom "Effort" control
Main: Effort control at relaunch rest
This PR, at rest after relaunch: no effort control
This PR: no effort control at relaunch rest
#24235 without this PR, at rest after Stop: a stray "Medium" control
Before: Medium effort control at rest
#24235 with this PR, at rest after Stop: no effort control
After: rest composer matches live

For comparison, the same chat while Claude runs shows the same controls as at rest:

After: live composer

Testing

  • Red first:

    • New test in src/main/claude/claude-structured-effort-default-at-rest.test.ts: a running Claude agent whose model the catalog does not list, compared with the same chat at rest. The row for that model must be identical, and the client must show no effort picker.
    • Before the fix it failed: the resting answer had no row for the model.
    • A second test checks the same for Codex: the resting model list equals the running answer's list, the model's row offers no effort levels, and no effort picker shows.
    • Two more cover a host with no catalog yet.
      • A resting Claude chat on opus lists exactly what a running agent whose CLI lists nothing lists, and keeps its effort picker at the saved level.
      • A resting Codex chat keeps its effort picker from the client's defaults.
      • A resting Claude chat with no saved pick names no model, and the client keeps the model the running agent reported (opus), not the built-in list's default (sonnet). It failed before this fix with expected 'sonnet' to be ''.
    • Both failed before the follow-up fix: the Claude answer listed only opus, with no effort levels, and the Codex answer listed gpt-5.5 with none.
  • Kept green: the existing at-rest effort cases for a listed model (opus[1m] showing its learned default, or the user's pick).

  • Ablations:

    • Taking the resting answer off the shared function fails both unlisted-model tests.
    • Making the shared function drop the unlisted model fails both unlisted-model tests.
    • Removing the built-in fallback at rest fails the Claude no-catalog test.
    • Adding the unlisted row to a Codex chat with no catalog fails the Codex no-catalog test.
    • Naming the built-in list's default at rest fails the no-pick test.
  • Runs (explicit files):

    • 76 options, effort and model-catalog test files for Claude, Codex, the host and the renderer, plus tests of every file that imports the touched code: 863 passed.
    • A wider set of 254 files, adding all agent-session wire tests, the structured session suites and the runtime integration suites: 2378 passed.
  • Lint: oxlint, the changed-code quality gate and the import-cycle check are clean.

  • I manually tested these changes locally (live on an M4 Air, see Visual Proof)

  • Automated tests added/updated, or explained why not below

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: no change.
  • Cross-platform: no platform-specific code.
  • SSH/remote: the change runs on the execution host. A remote host answers resting chats the same way.
  • Mobile: it reads the same options message and gets the same fix from the host.
  • Backwards compatibility: no wire change.
  • Performance: one extra list check per options read.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…ild does, with no phantom effort

A chat at rest (no live child) answered its options from the host's model
catalog alone. When the chat's model is one the catalog does not list, the
answer left it out, the client filled the gap from its static seed, and the
composer showed an effort control ("Medium") that the live child never offers;
the control vanished again once the child was back.

The live Claude and Codex answers and the resting answer now build their model
list through one function: the catalog's rows, plus the current model when the
catalog does not list it, with no effort levels. Live and rest can no longer
disagree about it.
…e child would, so a listed model keeps its effort

When the host has no model catalog for the account yet (the first read before
its probe returns, a failing probe, an account-home lookup that failed, or a
WSL-pinned chat no live child has written one for), the resting answer listed
only the saved model, as unlisted, and a listed model such as opus lost its
effort control. It now rests on the list a running child falls back to:
Claude's built-in models, then the shared function. Codex lists nothing
without its catalog, so the resting answer leaves the list to the client's own
defaults, as a failed live read does.
… model, as before

With no catalog, the built-in list's default model is a guess, not the
account's: naming it moved a chat the running agent reported on Opus to
Sonnet. Only a real listing names the default now; otherwise the answer names
none and the client keeps the model the agent last reported. The comment on
Codex's no-catalog answer now says what it does: the client fills the current
model from its own defaults, unchanged from before.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: ready for review (must land before #24235)

The problem. When a native Claude or Codex chat is resting (no agent process running), the composer could show an effort control the running chat does not have: "Medium" on chats whose model the model list does not include, or a bare "Effort" on main. It disappeared once the agent started, and a pick made in it could not be seen afterwards. #24235 rests a Claude chat after every Stop, so without this fix the stray control would appear after each Stop.

What changes for you. A resting chat shows exactly the controls the running chat shows. A normal model keeps its effort control even before Orca has fetched the model list.

Fixed during review:

  • With no model list yet, a resting chat lost its effort control even on a normal model like Opus; it now uses the same built-in list a running Claude falls back to.
  • That fix then made a resting chat with no saved model show "Sonnet" (the built-in list's guess); a default model is now named only from a real model list, otherwise the chat keeps the model the agent reported.
  • The Codex test now fails if the fix is removed.

Verified. Live on an M4 Air with real Claude (screenshots in the description): main shows a phantom "Effort" control at rest; #24235 without this PR shows "Medium" after a Stop; with this PR the resting composer matches the running one, both after a Stop and after a relaunch. Tests written to fail first; 254 related test files pass; typecheck clean.

Not verified. The idle sweep's own rest (it needs about 35 minutes idle; a quit and relaunch was used instead), Windows, Linux, SSH hosts and the phone app.

Deferred. Minor: a saved resolved model id can add a second row beside its alias, only when the running agent also has no model list (live and rest still agree).

@brennanb2025
brennanb2025 marked this pull request as ready for review October 1, 2026 06:12
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Refactors model listing logic across session types.

The PR should not merge until a first resting Claude read stops presenting an unverified model as selected.

Findings

  1. P1 Unverified Claude model appears selected ▶

Summary

The PR shares model-list construction across live Claude, live Codex, and resting chats, and adds Claude's built-in list when the host has no catalog.

  • This aligns unlisted-model rows between live and resting answers.
  • The no-catalog fallback can also make a first resting read display an unverified sonnet selection.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Resting options read] --> B{Host catalog available?}
  B -->|Yes| C[Catalog rows]
  B -->|No, Claude| D[Built-in rows]
  C --> E[Add unlisted saved model]
  D --> E
  E --> F[Client model picker]
Loading

Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."

Comment on lines +48 to +49
const listed =
catalog.origin === 'unknown' ? restingFallbackModels(record.provider) : catalog.models

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Unverified Claude model appears selected When a Claude chat has no saved model and the host has no catalog, this fallback list marks sonnet as the default even though the response leaves the current model empty. On a first resting read, the client therefore shows sonnet as selected without knowing what the account runs, and a user can explicitly pick and persist that guess. The new no-pick test applies a live answer first, so it does not cover this case.

Knowledge Base Used: Agent workspaces and sessions

@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c47fdc38-bf38-49c8-bdaa-be3d6e49abbe

📥 Commits

Reviewing files that changed from the base of the PR and between 07e9fdf and c7d6b22.

📒 Files selected for processing (5)
  • src/main/claude/claude-structured-effort-default-at-rest.test.ts
  • src/main/claude/claude-structured-session-options.ts
  • src/main/codex/codex-structured-model-catalog.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-option-models.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts

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


📝 Walkthrough

Walkthrough

Claude and Codex catalog construction now use a shared helper to include an unlisted current model. At-rest reads use Claude’s built-in models when the catalog is unknown, but do not infer a current model from those fallback models. Tests compare live and at-rest options for unlisted models, missing catalogs, and saved picks.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to c7d6b

This change makes resting native chats list models the same way running chats do, so the stray effort picker should no longer appear. No merge-blocking risk was found. Windows, Linux, SSH hosts, and the phone app were not verified by the author.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c7d6b

The change adjusts displayed options without adding write or execution authority. No new security issue was established, but access-control and downstream coverage remain incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated changed exposure is session-option metadata: seeded fallback rows and an existing current selection represented in the returned list. The inspected delta does not add a data-store write, process launch, credential operation, or privileged sink. Broader transport authorization and downstream exposure were not fully established.

Trust Boundaries and Controls

  • observed — The resting reader rejects a missing session record, and the live path retains child-fence propagation. These are identity and lifecycle checks, not evidence that an RPC caller is authorized to access the requested session; that transport-level control remains unverified.

Resilience and Maintainability Implications

  • observed — Catalog read failures are treated as unknown catalogs. The new fallback supplies provider-specific option metadata without selecting a current model from a guessed default, preserving the distinction between available choices and confirmed session state.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: resting native chats now list the current model like running agents to prevent an incorrect effort picker.
Description check ✅ Passed The description is comprehensive and covers the user impact, mechanism, rationale, linked context, visual proof, testing, compatibility, and checklist. The Linked Issue section does not use the requir…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

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

@pullfrog pullfrog 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.

Important

A resting Claude chat with no saved model and no host catalog now renders the built-in default ("Sonnet") as its selected model, and an at-rest effort change can persist that guess as the model for the next launch. The fix's own guard (leaving current.model empty) is defeated by the fallback list carrying isDefault.

Reviewed changes

  • Shared model-list rule: new structuredAgentSessionOptionModels appends the current model to a catalog when the catalog does not list it, with no effort levels; the Claude live adapter, the Codex live composer and the at-rest host answer all call it, replacing three copies of the rule.
  • At-rest Claude fallback: with no host catalog, the resting answer now lists Claude's built-in seed models instead of [], and names a default only from a real catalog.
  • Tests: five new cases in claude-structured-effort-default-at-rest.test.ts covering an unlisted model at rest vs live, the no-catalog fallback for both providers, and the no-pick case.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

function restingFallbackModels(
provider: AgentSessionRecord['provider']
): AgentSessionModelOption[] | null {
return provider === 'claude' ? claudeFallbackModelOptions() : null

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.

claudeFallbackModelOptions() keeps isDefault: true on the seed's default row. The at-rest answer sends this list with current.model: '', and the client's structuredAgentSessionOptionCatalog unconditionally sets defaultModelIsCliDefault: true, so the picker names Sonnet even though the host named no model — exactly the guess the comment above says to avoid. Before this PR the no-catalog list was empty, so nothing was named, and a running agent always reports its real model so it never falls back to the seed default either.

Technical details
# No-catalog Claude fallback names a guessed default

## Affected sites
- `src/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts:34` — fallback rows retain `isDefault` from `seedModels()`.
- `src/main/claude/claude-structured-session-options.ts:246` — `claudeFallbackModelOptions` wires `seedModels()` including `isDefault`.
- `src/shared/structured-agent-session-options.ts:89` — client `structuredAgentSessionOptionCatalog` sets `defaultModelIsCliDefault: true` unconditionally.
- `src/shared/native-chat-session-option-snapshot.ts:190` — `cliDefaultModelId` then returns `models.find((m) => m.isDefault)?.id`.

## Required outcome
- A no-catalog, no-saved-pick resting answer must not render a selected model, matching both the pre-PR behavior and a running agent (which reports its actual model).

## Suggested approach (optional)
- Strip `isDefault` from the no-catalog fallback, or gate the client's `defaultModelIsCliDefault` on a non-empty `current.model`.
- `claude-structured-effort-default-at-rest.test.ts:280` asserts the resting and live fallback lists are equal, so stripping only the resting side needs that expectation reconciled.

## Open questions for the human (optional)
- Is a record with no saved model actually reachable at rest (e.g. a first start that never proved)? The added no-pick test treats it as reachable; if it is not, the origin guard here is dead code. The new test only checks `resting.current.model === ''` and applies a prior live read first, so it does not cover the rendered picker.

@brennanb2025
brennanb2025 merged commit a8a434d into main Oct 1, 2026
60 checks passed
@pullfrog pullfrog Bot mentioned this pull request Oct 1, 2026
7 of 9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant