Repository navigation
fix(native-chat): store a chat's permission mode on its own, so a model pick can't reset it - #28268
Conversation
…rmission mode A model or effort pick on a running chat replaced the record's options with the provider's answer, which names only the provider's own options. The saved permission mode was dropped, and the next start read it as Ask. The pick now keeps the record's permission mode beside the provider's answer; everything the provider reset still resets.
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
…mission-pick-retain
…rd field A model or effort pick replaces the record's options with the provider's answer, which dropped the permission mode saved among them. The mode now lives beside the options, so no provider answer can touch it. Clients still pick it with setOption and read it from the same published fields.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
760c5897-b367-4ae6-8a87-bd74c48b5e63
📒 Files selected for processing: 18
src/main/native-chat/agent-session-wire/structured-agent-session-launch-permission.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-launch-permission.tssrc/main/native-chat/agent-session-wire/structured-agent-session-operation-start-refusal.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-options-read.tssrc/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-permission-mode.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-registered-definition.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-status-summary.tssrc/main/runtime/agent-session-permission-backfill.test.tssrc/main/runtime/agent-session-permission-backfill.tssrc/main/runtime/agent-session-record-founding.tssrc/main/runtime/agent-session-record-options.tssrc/main/runtime/agent-session-record-store.test.tssrc/main/runtime/claude-structured-startup-unanswered-control-request.test.tssrc/main/runtime/rpc/methods/structured-agent-session-create-message.test.tssrc/main/runtime/structured-agent-session-runtime.test.tssrc/shared/agent-session-record.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
…mong options Hourly builds before the separate field saved the mode in options.permissionMode. The startup pass now moves it into the field instead of re-deriving it from the agent's arguments. Temporary; removed after 1.4.226 ships.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Fall back to the legacy permission mode after… · structured-agent-session-launch-permission.ts:40-48
src/main/native-chat/agent-session-wire/structured-agent-session-launch-permission.ts:40-48
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFall back to the legacy permission mode after backfill failure.
A failed backfill leaves the legacy
options.permissionModepersisted, and startup continues. The launch reader now uses onlyrecord.permissionMode. A legacy session with a non-askmode can therefore launch asask.Suggested fix
import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { + AGENT_CHAT_PERMISSION_MODE_OPTION_ID, agentChatPermissionModes, chatModelSupportsAutoMode, chatPermissionModeNeedsModelSupport, effectiveChatPermissionMode, type AgentChatPermissionMode @@ - const intent = record.permissionMode + const intent = + record.permissionMode ?? record.options?.[AGENT_CHAT_PERMISSION_MODE_OPTION_ID]
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
0c1f4400-df3a-45a4-832d-82eae7cdd1c6
📒 Files selected for processing: 2
src/main/runtime/agent-session-permission-backfill.test.tssrc/main/runtime/agent-session-permission-backfill.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
…el pick can't reset it (stablyai#28268) * fix(native-chat): picking a Codex model or effort keeps the chat's permission mode A model or effort pick on a running chat replaced the record's options with the provider's answer, which names only the provider's own options. The saved permission mode was dropped, and the next start read it as Ask. The pick now keeps the record's permission mode beside the provider's answer; everything the provider reset still resets. * chore: re-run CI against current main * chore: re-run CI against current main * test(native-chat): keep one permission-mode-survives-pick test * refactor(native-chat): store a chat's permission mode as its own record field A model or effort pick replaces the record's options with the provider's answer, which dropped the permission mode saved among them. The mode now lives beside the options, so no provider answer can touch it. Clients still pick it with setOption and read it from the same published fields. * fix(native-chat): keep a permission mode an earlier dev build saved among options Hourly builds before the separate field saved the mode in options.permissionMode. The startup pass now moves it into the field instead of re-deriving it from the agent's arguments. Temporary; removed after 1.4.226 ships. * chore: re-run CI against current main (cherry picked from commit b91795a)
…el pick can't reset it (#28268) * fix(native-chat): picking a Codex model or effort keeps the chat's permission mode A model or effort pick on a running chat replaced the record's options with the provider's answer, which names only the provider's own options. The saved permission mode was dropped, and the next start read it as Ask. The pick now keeps the record's permission mode beside the provider's answer; everything the provider reset still resets. * chore: re-run CI against current main * chore: re-run CI against current main * test(native-chat): keep one permission-mode-survives-pick test * refactor(native-chat): store a chat's permission mode as its own record field A model or effort pick replaces the record's options with the provider's answer, which dropped the permission mode saved among them. The mode now lives beside the options, so no provider answer can touch it. Clients still pick it with setOption and read it from the same published fields. * fix(native-chat): keep a permission mode an earlier dev build saved among options Hourly builds before the separate field saved the mode in options.permissionMode. The startup pass now moves it into the field instead of re-deriving it from the agent's arguments. Temporary; removed after 1.4.226 ships. * chore: re-run CI against current main (cherry picked from commit b91795a)
ELI5
Picking a different model or reasoning effort in a native chat could quietly switch a chat from Full access back to Ask for approval. You never touched the permission button, but the agent started asking before every command again. This PR stores the chat's permission mode on its own, so changing the model can't affect it.
What Changed
Before (on main): set a Codex or Claude chat to Full access, then pick a different model or effort. The button flips to Ask for approval, and the next command stops at an approval card ("Run a command?" or "Allow Bash?"). For Claude, the agent restarts without its skip-permissions flag. This hits new chats as well as old ones.
After: the chat keeps the permission mode you chose through any number of model and effort changes, a chat restart, and quitting and reopening Orca. Anything the agent itself resets on a model change still resets; for example, a new Codex model still drops an effort chosen for the old one.
Mechanism. Each chat's saved record had one options map. It held the agent's settings (model, effort, speed) and also Orca's permission mode under
permissionMode. When you pick a model or effort, the agent answers with its own options, and the host saved that answer as the record's whole options map, which deleted the permission mode. Launch then read the missing mode as Ask.The permission mode is now its own field on the chat record (
permissionMode, besideoptions):agent-session-record-founding.ts), which also takes the mode out of create requests that still send it among options, as older clients do;structured-agent-session-options-read.ts);agent-session-permission-backfill.ts, from fix(native-chat): chats from before permission modes keep the mode they were running with #28099).structured-agent-session-launch-permission.ts) and the status summary (structured-agent-session-status-summary.ts).options, so it can't reach the mode any more.Nothing on the wire changes. Clients still set the mode with
setOption{key:'permissionMode'}, and the host publishes it in the same fields as before, so older desktop and mobile clients and remote Orca servers keep working against either version.Temporary: builds from main since #26116 (the team's hourly builds; no release has them) saved the mode inside
options. For those chats, the startup pass moves that saved mode into the new field before it falls back to the agent's launch arguments. Without that step, a chat set to Full access on one of those builds would open as Ask after updating, or the reverse. The step is marked temporary and comes out after 1.4.226 ships.Why
The common pattern keeps a chat's permission mode as a separate setting from its model selection, each set and resolved on its own, so a model change can't reach the permission mode. An earlier version of this PR instead copied the one key across the agent's answer in the model-pick code (8 lines). That fixed one save path. Any other path that saves an agent's answer, and any future Orca-owned setting kept in the same map, could hit the same bug. A separate field removes the whole class.
It also costs nothing extra now: #26116 and its follow-ups haven't shipped to any user (release hold STA-10426), so no user data has the old shape and no migration is needed. Only the dev-team builds noted above, which the temporary step covers.
Linked Issue
No separate issue. Found while reviewing #28099 and split out of it. Related: #26116, #28099, STA-10345.
Visual Proof
Hidden dev builds on macOS with real Codex and Claude agents. Agent Permissions was set to Manual and new chats default to Ask, so each chat was switched to Full access with its own button.
Before (main): a Codex chat on Full access, after picking a different model, shows Ask for approval.
After: the same steps on this branch. After a model pick and an effort pick, the button still shows Full access.
After closing and reopening the chat, and then quitting and relaunching Orca, it still shows Full access. All three commands ran without an approval prompt.
A Claude chat after switching to Sonnet and High effort also keeps Full access.
Testing
Bug test (one, new):
structured-agent-session-permission-mode.test.ts› "keeps Full access through a model pick on the running agent and the next start". It runs the real host, record store and journal: set a Codex chat to Full access, send, pick a model on the running agent, restart the host, send again. It expects the restarted agent to launch on Full access. It fails on main, where the relaunch starts onask.Startup pass: the existing fix(native-chat): chats from before permission modes keep the mode they were running with #28099 backfill tests now check the field. One existing case also covers the temporary step: a dev-build record with the mode inside
optionskeeps that mode, for Ask, Full access and an unrecognised value. That case fails without the step.Other test edits are mechanical: fixtures that seeded
options.permissionModenow seed the field. The earlier carry-over test is gone.Runs:
src/main/native-chatandsrc/main/runtimethat mention the permission mode: 226/226 pass;src/main/runtime,native-chat,claude,codex,acp,pi,agent-launchandtests/e2e/cross-version-wire: 21,770 tests pass. Six cross-version files timed out on a shared build lock and pass when rerun alone; one Claude-teams launch test fails the same way on main;pnpm tc:nodepasses;orca-ci-checks --no-typecheckpasses, apart from the local Bun tooling step.Live QA, macOS (hidden dev builds, real Codex and Claude, CDP-driven):
approval_policy never; Claude kept--dangerously-skip-permissions).CI: on
16bf28807ba, the only red check ismax-linesonsrc/main/orcad/orcad-entry.ts(301 lines), which this PR doesn't touch: it came in on main (fix(accounts): launches a WSL host starts itself use the account selected for its distro #28464/fix(orcad): keep Codex status hooks in a managed account home on a WSL host, and list its sessions #28469). The test shards were skipped by fail-fast and will run once main is fixed.I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Agent skill upstream boundary
Notes
options, and the temporary step would need to stay.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)