Skip to content

fix(chat): send pi and acp messages after a model-only change - #32004

Merged
AmoabaKelvin merged 2 commits into
mainfrom
pi-acp-option-admission
Oct 22, 2026
Merged

AmoabaKelvin merged 2 commits into
mainfrom
pi-acp-option-admission

Conversation

@AmoabaKelvin

@AmoabaKelvin AmoabaKelvin commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​66 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​66
Prod 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​17 $\color{#cf222e}{\Huge{\mathbf{−}}}$​7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​10

What the user saw

In a Pi or ACP chat (OpenCode 2, OMP, Grok, Antigravity), picking a different model in the picker and sending showed "OpenCode 2 didn't accept this message." (or the OMP / Grok / Pi equivalent). The message never reached the agent. Every later send on that model was refused the same way.

Root cause

Orca decided whether to admit a message by looking at the wrong facts. It should have used the choice Orca last applied.

  • Pi: a model write also stamps an effort token in optionProof.writes, which exists to cancel an in-flight effort write. Dispatch read that same token as "Orca set an effort". So a message with a model and no effort was refused right after its own model switch (rpc-message-dispatch.ts).
  • ACP: a message with no effort, on a model with no known default effort, was refused whenever the agent reported an effort (acp-message-execution-options.ts). OMP always reports one. OpenCode 2 starts on a model with no effort option, then reports thought_level: "default" after a switch to a model that has one, such as opencode/deepseek-v4.1-flash-free. So the first send after that switch is refused, though the model switch itself already went through.

Both have been there since #28046.

The fix

This follows the contract #30982 set for Claude: record the last successfully applied choice after a successful control, and never treat a settings readback as admission authority.

A message that leaves a key unspecified is now refused only when Orca's last successful write for that key set a value other than the one the session opened with.

  • Pi reads session.selected, which was already set only after a successful write. The model write's effort token still supersedes in-flight writes.
  • ACP gets the same record, AcpStructuredOptions.applied, set where a write is confirmed.

Restoring the opening effort when returning to the opening model doesn't count as an override. So switching away, back, and away again keeps working. An explicit earlier effort the new model has no known default for is still refused, as #28046 intended.

Codex is separate (STA-11443).

Linked Issue

Linear STA-11450 (no GitHub issue). #31998 is a community PR for the ACP half of the same refusal.

Visual Proof

OpenCode 2

Real OpenCode 2.1.3 with isolated config and data, in a hidden dev build. Steps: open an OpenCode 2 chat (it starts on Big Pickle), pick DeepSeek V4.1 Flash Free, send twice. The saved chat options were {"model":"opencode/deepseek-v4.1-flash-free"} in both runs. On the branch, OpenCode's own store records both turns on deepseek-v4.1-flash-free with variant: "default".

Before (main):

before-crop.png

After (this branch):

after-crop.png

OMP

Real OMP 18.12.2 in an isolated config with two models (Alpha, Beta) served by a local fake OpenAI endpoint, in a hidden dev build. Steps: send a first message on Alpha, pick only Beta in the picker, send again. The saved chat options were {"model":"scratch/beta"} in both runs.

Before (main):

before-crop.png

After (this branch):

after-crop.png

Testing

  • New adapter tests for Pi and ACP send model-only messages away from the opening model, back, and away again. Both fail on main with "The default effort for this model is unknown" and pass here.
  • The ACP test also fails against a variant that only drops the readback check, which covers the back-and-away case.
  • Focused suites pass: src/main/pi, src/main/acp and the Pi startup / effort-at-rest runtime tests.

Admission now reads the value Orca last applied per key. A model write's
effort marker and the agent's reported effort no longer refuse a send.
@AmoabaKelvin
AmoabaKelvin marked this pull request as ready for review October 22, 2026 20:54
@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5b91d9a0-1a5c-41a1-89f9-5bc3cd44100e
📥 Commits

Reviewing files that changed from the base of the PR and between 4dd97f0 and 0b498c6.

📒 Files selected for processing: 7
  • src/main/acp/acp-message-execution-options.ts
  • src/main/acp/acp-structured-options.ts
  • src/main/acp/acp-structured-session-adapter.test.ts
  • src/main/acp/acp-structured-session-adapter.ts
  • src/main/pi/rpc-message-dispatch.ts
  • src/main/pi/rpc-session-adapter.test-fixture.ts
  • src/main/pi/rpc-session-adapter.test.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.

📝 Walkthrough

ACP and Pi now handle undefined execution-option values by comparing selected values with initial values. ACP records values from adopted session-option writes. Tests cover repeated model-only choices, including returning to the opening model and restoring its opening effort level.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 0b498

Model-only messages can use the last successfully applied option choice without treating an agent-reported effort as a user choice. No actionable merge-blocking risk remains after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title "fix(chat): send pi and acp messages after a model-only change" matches the main change. The PR fixes admission of model-only sends in Pi and ACP chats, which were wrongly refused because of…
Description check Passed The description follows the template well. It has a clear user-facing before/after, a root cause, the fix mechanism, a linked issue (Linear STA-11450, with no GitHub issue), visual proof for both Open…
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.
Full details: Title check

Explanation

The title "fix(chat): send pi and acp messages after a model-only change" matches the main change. The PR fixes admission of model-only sends in Pi and ACP chats, which were wrongly refused because of stale effort markers and agent-reported effort. The title names the affected adapters and the triggering case. It does not name the root cause, but it does not need to cover every detail.

Full details: Description check

Explanation

The description follows the template well. It has a clear user-facing before/after, a root cause, the fix mechanism, a linked issue (Linear STA-11450, with no GitHub issue), visual proof for both OpenCode 2 and OMP, and testing notes. Some template parts are missing or weak. The ELI5 section is absent. The Testing checklist does not show whether the author checked the manual-testing box. The AI Disclosure section is empty. The 'Fixes #' line is blank, and the description links a Linear issue rather than a GitHub issue. The Review, Agent skill upstream boundary, and Notes sections are also not addressed. The checklist items for lint, typecheck, test, and build are not marked.

Resolution

Optional improvements: add a short ELI5 summary; state whether the manual-testing box applies (the Visual Proof section suggests it does); fill in or remove the empty 'Fixes #' line; complete the AI Disclosure section if an AI model was used; and confirm the checklist items for lint, typecheck, test, and build.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR

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.

@AmoabaKelvin
AmoabaKelvin merged commit 752f531 into main Oct 22, 2026
71 checks passed
@AmoabaKelvin
AmoabaKelvin deleted the pi-acp-option-admission branch October 22, 2026 21:10
jordanyeo pushed a commit to Statvis/orca that referenced this pull request Oct 23, 2026
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