Skip to content

fix(native-chat): drop a remembered model option a saved listing no longer admits - #31576

Open
hualong1009 wants to merge 1 commit into
stablyai:mainfrom
hualong1009:fix/structured-seed-option-admission
Open

hualong1009 wants to merge 1 commit into
stablyai:mainfrom
hualong1009:fix/structured-seed-option-admission

Conversation

@hualong1009

Copy link
Copy Markdown

Fixes #31493.

Root cause

A new chat seeds its remembered model selection into the create and admits the seed against the saved catalog rows (#30561). When the saved row for the remembered model no longer admits a remembered value (e.g. an effort remembered from a listing that offered it, while the current saved row for that model lists none — seen after a CLI/model-listing change, e.g. upgrading Orca from 1.4.225 to 1.4.237 alongside pi 1.3.0), the create is refused with agent_session_operation_invalid (reason: optionRejected) before any effect: nothing is created and no process is spawned.

The launch executor does not classify that create refusal as failed-without-effects, so it is wrapped as agent_session_operation_unknown. The Chat UI then waits out its launch confirmation budget and shows "Chat connection could not be confirmed" — with no error surface, and the same result for every retry, so the agent's Chat UI is permanently broken while terminal launches still work.

Trace signature: the agentSession.create span ends right after reserve_owner with no acquire_owner phase and no new processes.

Fix

A remembered value its model's saved row no longer admits is now dropped from the seed instead of refusing the create — a launch is refused only on options its caller named for it:

  • seedOptionsAdmittedOnSavedListing filters the seed against the saved rows, keeping keys the caller named explicitly this launch (their values stay their caller's own picks and keep their refusal).
  • The seed's model itself always stays: an unlisted model is unknown evidence, which admits everything, and a listed row admits its own id.
  • Explicit --model launches are unchanged: an option the caller named is still admitted strictly.

Verification

  • Reproduced on Windows (Orca 1.4.237, pi 1.3.0) with a remembered model selection whose saved row no longer admits its remembered effort: every chat launch for that agent ended in agent_session_operation_unknown after the 30s budget with no spawned process, while the same launch from a fresh profile succeeded.
  • With this change (simulated by clearing the remembered effort value), the same launch proceeds and starts the chat — verified live on the poisoned profile.
  • New unit tests cover the filter: a remembered value its row no longer admits is dropped, admitted values are kept, caller-named values survive, and unlisted models / absent rows admit everything; a prepare-level test covers the create path dropping a remembered effort while keeping an admitted one.

A complementary hardening (separate follow-up, not included here): launchFailureWithoutEffectsCode could treat AgentLaunchStructuredSessionRefusedError as failed-without-effects, since a refused create is the launch's first effect — that would surface the typed refusal to the caller instead of agent_session_operation_unknown for any remaining admission refusal.

…onger admits

A new chat seeds the remembered model selection into its create and admits
the seed against the saved catalog rows. A remembered value its model's row
no longer admits refused the create with agent_session_operation_invalid
(optionRejected) before any effect, which the launch wraps as
agent_session_operation_unknown: every new chat for that agent then fails
after the confirmation budget, shown as a chat that could not connect.

A remembered value is now dropped instead — a launch is refused only on
options its caller named for it. Values this launch named itself keep their
refusal, and the seed's model always stays, since a listed row admits its
own id.
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Structured agent-session creation now filters remembered seed options against the saved model listing when no model option was explicitly supplied. Explicitly named option keys remain. The change adds helper and session-creation tests for admitted and rejected options, explicit options, unlisted models, and empty saved listings.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to c7b2e

The changed create path retains the saved model needed to filter remembered options. Strengthen the explicit-effort test to protect that behavior; the remaining coverage gap is bounded and does not establish a launch failure.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary fix: removing a remembered model option that a saved listing no longer admits.
Description check Passed The description explains the root cause, user impact, fix, linked issue, verification, test coverage, and follow-up scope. It does not use every template heading and does not include the requested Vis…
Linked Issues check Passed The change addresses #31493. prepareStructuredAgentSessionCreateForWorktree filters remembered seed options against saved model rows when the caller does not provide model. It drops stale remember…
Out of Scope Changes check Passed All changed source code implements the #31493 launch-seed admission fix. The added unit tests verify that behavior. No unrelated production change or unrelated test change is shown.
Docstring Coverage Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files.
Full details: Description check

Explanation

The description explains the root cause, user impact, fix, linked issue, verification, test coverage, and follow-up scope. It does not use every template heading and does not include the requested Visual Proof N/A statement or checklist confirmations, but the required change rationale and validation details are substantially complete.

Full details: Linked Issues check

Explanation

The change addresses #31493. prepareStructuredAgentSessionCreateForWorktree filters remembered seed options against saved model rows when the caller does not provide model. It drops stale remembered values, keeps admitted values, retains caller-named values, and preserves seeds for unlisted models or absent rows. Explicit model launches remain unchanged. Unit tests cover the filter and the create preparation path.

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

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.

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

🧹 Nitpick comments: 1
src/main/runtime/rpc/methods/structured-agent-session-create-seed-admission.test.ts: 1

79-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the explicit-effort test use the seed model during admission.

The helper omits atRest, so the preparation path passes only { effort: 'off' } to admission. For the pi agent, the seed-listing helper returns that value unchanged because it only reads catalogs for claude. No model row matches, so the test can pass even if explicit keys are not forwarded.

Exercise the at-rest path, retain { model: 'some/model' }, and provide its required host stubs. Keep the assertion that explicit effort: 'off' survives the matching row that rejects off.


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c37e0ef0-9dab-4850-a0ae-d646c7f48001
📥 Commits

Reviewing files that changed from the base of the PR and between 84a9f99 and c7b2e79.

📒 Files selected for processing: 4
  • src/main/runtime/agent-session-record-options.test.ts
  • src/main/runtime/agent-session-record-options.ts
  • src/main/runtime/rpc/methods/structured-agent-session-create-seed-admission.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-create.ts

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

This branch has not been deployed

No deployments
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.

[Bug]: Pi Agent Chat UI fails with "Chat connection could not be confirmed" on Windows

1 participant