Repository navigation
fix(native-chat): chats from before permission modes keep the mode they were running with - #28099
Conversation
|
Problem. Claude and Codex chats made before #26116 have no saved permission mode, and main reads that as Ask for approval. Before #26116 those chats did whatever Settings → Agents → Agent Permissions said, which is Yolo (no prompts) by default. So on upgrade, most chat users would find every existing chat asking before each command (reported in STA-10345). What changes for the user. The first time the host's chat service starts on the new version, each old Claude or Codex chat gets the mode it was already running with: Full access if that agent's own arguments skip permissions, otherwise Ask. The button shows the right mode immediately. From then on these chats behave like any other chat: the "Permissions for new chats" default affects new chats only. How. One store transaction over the saved chats, before any chat can be read or started. It fills only a missing mode, never overwrites a saved one, and leaves every other option alone. It skips a read-only store. A failure is logged, doesn't block startup, and is retried on the next start. A chat created by attaching to a client-supplied location now gets the same new-chat seed every other create path already uses, so no new chat is ever missing a mode. This replaces the earlier designs: live inheritance of the default, then saving the mode at first start, which had two reproduced timing bugs. The launch path is unchanged from main. Review (final head
Verified.
Not verified. SSH and remote servers live (code path only), Linux, Windows, mobile. CI. Green on Separate, related: on main, picking a model or effort erases a chat's saved mode, so it falls back to Ask. Fixed in #28268 (draft). Draft. Not merged. |
…t default A Claude or Codex chat created before chats had their own permission mode has none saved, and it ran Ask, so every existing chat of a Yolo user started asking for approval after the update. A chat with no saved mode now runs Settings -> Chat -> Permissions, read from the host's settings on each launch, read and restart check.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 45.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 37 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly identifies the main change: preserving permission modes for chats created before per-chat permission modes existed. Description check The description is comprehensive and covers the user impact, implementation, rationale, linked issue context, visual proof, testing, limitations, and compatibility notes. Some template sections and ch…
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
-
- 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.
Comment @coderabbitai help to get the list of available commands.
…rt ran with A chat saved before chats had permission modes ran the new-chat default on every start, so it kept following that setting. Its first start now writes the mode it ran into the chat's saved options, through the same write that already carries the chat's other options over a start; from then on it is a chat with its own mode.
514dea1 to
509e512
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
cc376ad9-1d29-4907-817f-201adcef069d
📒 Files selected for processing (9)
src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-options.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-acquisition.tssrc/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.tssrc/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.tssrc/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-provider-started.tssrc/main/runtime/orca-runtime-get-worktree-ps.tssrc/main/runtime/structured-agent-session-runtime.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.
brennanb2025
changed the title
Replace the save-at-first-start path with one startup pass: before the chat host is exposed, every Claude and Codex record with no saved permissionMode gets the host's new-chat default, in one store transaction. Saved values, including invalid ones, and every other option are left alone; a read-only store is skipped and a failure is logged and retried next start. The runtime fallback in launch, acquisition, provider-started and attach is restored to main's behaviour. A client-supplied attach that creates a new record now gets the same new-chat seed as every other create path.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
016f4a9e-1f8b-4e8c-9369-1cf1c81e49a6
📒 Files selected for processing (12)
config/scripts/vitest-sqlite-runtime-files.mjssrc/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.tssrc/main/runtime/agent-session-permission-backfill.test.tssrc/main/runtime/agent-session-permission-backfill.tssrc/main/runtime/agent-session-record-store.tssrc/main/runtime/orca-runtime-get-worktree-ps.tssrc/main/runtime/orca-runtime-structured-launch-settings-wiring.test.tssrc/main/runtime/rpc/methods/structured-agent-session-create-handler.tssrc/main/runtime/rpc/methods/structured-agent-session-permission-create.test.tssrc/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.tssrc/main/runtime/structured-agent-session-runtime.test.tssrc/main/runtime/structured-agent-session-runtime.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| root = await mkdtemp(join(tmpdir(), 'orca-permission-backfill-')) | ||
| }) | ||
| afterEach(async () => { | ||
| await rm(root, { recursive: true, force: true }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Close the test database before removing root.
The test database registry keeps its SQLite connection open after each test. On Windows, the open database can block rm(root), so the cleanup fails. Call closeTestJournalHostDatabases() before rm, as the adjacent store-transaction tests do. (raw.githubusercontent.com)
| logger | ||
| }) | ||
| try { | ||
| await recordStore.store.backfillChatPermissionModes(deps.initialChatPermissionMode) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not backfill when initialChatPermissionMode is absent.
initialChatPermissionMode is optional, but this call runs with undefined when a caller omits it. newChatPermissionMode then selects Ask for Claude and Codex. The transaction saves Ask permanently, so a later start with a Full access default cannot correct those chats. Require the dependency or skip the backfill when it is absent. (raw.githubusercontent.com)
An older Claude or Codex chat ran with that agent's own Arguments before permission modes existed, so the startup pass now saves full access or ask per agent instead of the combined new-chat default. A user with Claude on bypass and Codex on Manual keeps Claude chats on full access. The attach create path always passes its permission seed; the reservation already applies options only when it creates the record.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Preserve attach options when adding the… · structured-agent-session-create-handler.ts:42-44
src/main/runtime/rpc/methods/structured-agent-session-create-handler.ts:42-44
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve attach options when adding the permission seed.
When an attach request creates a new record, the handler replaces all existing
params.optionswith the permission seed. This drops other valid initial options. Merge the seed into the existing options.Proposed change
- ? { options: { [AGENT_CHAT_PERMISSION_MODE_OPTION_ID]: permissionMode } } + ? { options: { ...params.options, [AGENT_CHAT_PERMISSION_MODE_OPTION_ID]: permissionMode } }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
6da23a99-c6d8-42bb-9abd-e6551200abc3
📒 Files selected for processing (10)
src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.tssrc/main/runtime/agent-session-permission-backfill.test.tssrc/main/runtime/agent-session-permission-backfill.tssrc/main/runtime/agent-session-record-store.test.tssrc/main/runtime/agent-session-record-store.tssrc/main/runtime/orca-runtime-structured-launch-settings-wiring.test.tssrc/main/runtime/rpc/methods/structured-agent-session-create-handler.tssrc/main/runtime/rpc/methods/structured-agent-session-permission-create.test.tssrc/main/runtime/structured-agent-session-runtime.test.tssrc/main/runtime/structured-agent-session-runtime.ts
💤 Files with no reviewable changes (1)
- src/main/runtime/rpc/methods/structured-agent-session-permission-create.test.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.
| if (record.options?.[AGENT_CHAT_PERMISSION_MODE_OPTION_ID] !== undefined) { | ||
| continue | ||
| } | ||
| const permissionMode = modeFor(record.provider) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- changed file ---'
nl -ba src/main/runtime/agent-session-permission-backfill.ts
printf '%s\n' '--- references ---'
rg -n -F --glob '*.{ts,tsx}' -e 'backfillAgentSessionPermissionModes' -e 'executionHostId' -e 'interface AgentSession' -e 'type AgentSession' src/main src | head -240
printf '%s\n' '--- relevant files ---'
rg --files src/main | rg 'agent-session|permission|runtime|store' | head -160Repository: stablyai/orca
Length of output: 41652
🏁 Script executed:
set -o pipefail
nl -ba src/main/runtime/agent-session-permission-backfill.ts
printf '%s\n' '--- direct references ---'
rg -n -F --glob '*.{ts,tsx}' -e 'backfillAgentSessionPermissionModes' -e 'executionHostId' src/main srcRepository: stablyai/orca
Length of output: 45657
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- exact backfill references ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -- 'backfillAgentSessionPermissionModes' . || test "$?" -eq 1
printf '%s\n' '--- exact ranWithFullAccess references ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -- 'ranWithFullAccess' . || test "$?" -eq 1
printf '%s\n' '--- store transaction declaration and references ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'export interface AgentSessionStoreTransactions' -e 'export type AgentSessionStoreTransactions' -e 'AgentSessionStoreTransactions' src/main/native-chat src/main/runtime || test "$?" -eq 1
printf '%s\n' '--- agent session record declarations ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'AgentSessionRecord' -e 'type AgentSessionRecord' -e 'interface AgentSessionRecord' -e 'location: {' src/main/native-chat src/main/runtime | head -240Repository: stablyai/orca
Length of output: 38991
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- agent-session-record-store ---'
nl -ba src/main/runtime/agent-session-record-store.ts | sed -n '70,135p'
printf '%s\n' '--- agent-session-store-transactions ---'
nl -ba src/main/runtime/agent-session-store-transactions.ts | sed -n '1,220p'
printf '%s\n' '--- shared record and fixture ---'
nl -ba src/shared/agent-session-record.ts | sed -n '1,180p'
nl -ba src/main/native-chat/agent-session-record-test-fixture.ts | sed -n '1,90p'
printf '%s\n' '--- journal schema and backfill tests ---'
nl -ba src/main/native-chat/agent-session-journal/journal-database-schema.ts | sed -n '1,130p'
nl -ba src/main/runtime/agent-session-permission-backfill.test.ts | sed -n '1,190p'Repository: stablyai/orca
Length of output: 36842
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- backfill method callers ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -- 'backfillChatPermissionModes' . || test "$?" -eq 1
printf '%s\n' '--- launch/default resolution candidates ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'ranWithFullAccess' -e 'permissionMode' -e 'bypassPermissions' -e 'dangerouslySkipPermissions' src/main src/shared | head -320Repository: stablyai/orca
Length of output: 30457
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- runtime call and dependency declaration ---'
nl -ba src/main/runtime/structured-agent-session-runtime.ts | sed -n '220,285p'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'resolveAgentFullAccess' -e 'resolveAgentPermissionMode' src/main src/sharedRepository: stablyai/orca
Length of output: 5129
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- runtime dependency type ---'
nl -ba src/main/runtime/structured-agent-session-runtime.ts | sed -n '95,135p'
printf '%s\n' '--- resolver wiring ---'
nl -ba src/main/runtime/orca-runtime-get-worktree-ps.ts | sed -n '205,240p'
nl -ba src/main/runtime/orca-runtime-structured-launch-settings-wiring.test.ts | sed -n '1,60p'Repository: stablyai/orca
Length of output: 8811
Resolve permission defaults for the chat’s execution host.
When record.location.executionHostId differs from the current runtime host, resolveAgentFullAccess uses this host’s configured arguments. The backfill then saves that mode, and the existing-mode guard prevents a later correction. Resolve the default for record.location.executionHostId, or skip records whose host-specific default is unavailable.
…ey were running with (stablyai#28099) * fix(native-chat): chats saved before permission modes run the new-chat default A Claude or Codex chat created before chats had their own permission mode has none saved, and it ran Ask, so every existing chat of a Yolo user started asking for approval after the update. A chat with no saved mode now runs Settings -> Chat -> Permissions, read from the host's settings on each launch, read and restart check. * fix(native-chat): an older chat keeps the new-chat mode its first start ran with A chat saved before chats had permission modes ran the new-chat default on every start, so it kept following that setting. Its first start now writes the mode it ran into the chat's saved options, through the same write that already carries the chat's other options over a start; from then on it is a chat with its own mode. * fix(native-chat): save older chats' permission mode once at host startup Replace the save-at-first-start path with one startup pass: before the chat host is exposed, every Claude and Codex record with no saved permissionMode gets the host's new-chat default, in one store transaction. Saved values, including invalid ones, and every other option are left alone; a read-only store is skipped and a failure is logged and retried next start. The runtime fallback in launch, acquisition, provider-started and attach is restored to main's behaviour. A client-supplied attach that creates a new record now gets the same new-chat seed as every other create path. * fix(native-chat): fill each older chat from its own agent's arguments An older Claude or Codex chat ran with that agent's own Arguments before permission modes existed, so the startup pass now saves full access or ask per agent instead of the combined new-chat default. A user with Claude on bypass and Codex on Manual keeps Claude chats on full access. The attach create path always passes its permission seed; the reservation already applies options only when it creates the record. * refactor(native-chat): read each chat's agent arguments directly in the permission backfill * test(native-chat): give stub runtimes the launch seed the attach path reads (cherry picked from commit c5d7628)
…ey were running with (#28099) * fix(native-chat): chats saved before permission modes run the new-chat default A Claude or Codex chat created before chats had their own permission mode has none saved, and it ran Ask, so every existing chat of a Yolo user started asking for approval after the update. A chat with no saved mode now runs Settings -> Chat -> Permissions, read from the host's settings on each launch, read and restart check. * fix(native-chat): an older chat keeps the new-chat mode its first start ran with A chat saved before chats had permission modes ran the new-chat default on every start, so it kept following that setting. Its first start now writes the mode it ran into the chat's saved options, through the same write that already carries the chat's other options over a start; from then on it is a chat with its own mode. * fix(native-chat): save older chats' permission mode once at host startup Replace the save-at-first-start path with one startup pass: before the chat host is exposed, every Claude and Codex record with no saved permissionMode gets the host's new-chat default, in one store transaction. Saved values, including invalid ones, and every other option are left alone; a read-only store is skipped and a failure is logged and retried next start. The runtime fallback in launch, acquisition, provider-started and attach is restored to main's behaviour. A client-supplied attach that creates a new record now gets the same new-chat seed as every other create path. * fix(native-chat): fill each older chat from its own agent's arguments An older Claude or Codex chat ran with that agent's own Arguments before permission modes existed, so the startup pass now saves full access or ask per agent instead of the combined new-chat default. A user with Claude on bypass and Codex on Manual keeps Claude chats on full access. The attach create path always passes its permission seed; the reservation already applies options only when it creates the record. * refactor(native-chat): read each chat's agent arguments directly in the permission backfill * test(native-chat): give stub runtimes the launch seed the attach path reads (cherry picked from commit c5d7628)
1 participant
ELI5
#26116 (merged, not in a release yet) gave each Claude and Codex chat its own permission setting, saved on the chat: Ask for approval before running commands, or Full access to just run them. A button next to "+" in the chat shows it and changes it.
The chats people already have when they update were made before that setting existed, so nothing is saved on them, and #26116 treats "nothing saved" as Ask for approval. Before #26116, a chat simply did what Settings → Agents → Agent Permissions said, and that is Yolo (no prompts) by default. So on the day #26116 ships, most chat users would find every chat they already have asking before each command. A user has already reported exactly this ("every single command needs manual confirmation", STA-10345).
With this PR, the first time Orca starts after the update, it saves a permission mode onto every Claude and Codex chat that doesn't have one: the mode the chat was already running with, taken from that chat's own agent's Agent Permissions setting. Users on Yolo keep Full access and users on Manual keep Ask. The new-chat default only affects new chats. From then on those chats are ordinary chats with their own saved mode, and the button shows the right mode straight away.
What Changed
Terms used below
What you see
Mechanism: one step when the host's chat service starts (on the desktop that's at app start, when saved chats are restored; on a remote server it's the first chat request). After the chat record store is open, and before any chat can be read or started, the host runs one pass over its saved chats in a single store transaction. Each Claude or Codex chat whose saved options have no permission mode gets the mode it was already running with: Full access if that agent's own launch arguments skip permissions (the same check Orca used for these chats before #26116), otherwise Ask. Nothing else in the saved options changes.
agentSession.ensure). That path now gets the same new-chat seed. Without it, such a chat would run Ask and then switch to Full access after the next restart's pass. An existing chat is never reseeded: the store applies options only when it creates the record.The code that launches a chat, decides whether it must restart after a mode change, and draws the button is unchanged from main. It reads the chat's saved mode as before; the difference is that, after the pass, every Claude and Codex chat has one.
Why
Without this, the release that ships #26116 turns existing chats for most chat users from "no prompts" into "prompt for everything". The new button can fix that only one chat at a time, and only after the user works out what changed.
When a required per-item setting is added, the common pattern is to fill in every existing item once with the behaviour it already had, so that afterwards nothing ever has to handle the setting being missing. This PR does that.
Alternatives considered (both built and reviewed on this branch before):
Old chats keep following the new-chat default until changed. This was the first version. Keeping those chats in sync with the setting took a settings-change listener, a button refresh and an extra field sent between client and host (+539 lines). It also contradicted the setting's own description, "applies to new chats only".
Save the mode on an old chat the first time it starts. This was the second version. An independent review reproduced two timing bugs:
It also left the button possibly wrong before the first start.
The startup pass removes all three, because no chat ever starts without a saved mode. It is also smaller: no change to the launch path at all.
Keep Ask for those chats. That's the regression this PR fixes.
Cost: one write per old Claude or Codex chat, once, on the first launch after the update.
Linked Issue
STA-10345 (user feedback), following #26116.
Visual Proof
Upgrade path on macOS, hidden dev builds: a profile with real Claude and Codex chats created on a build from just before #26116, with Agent Permissions on Yolo for both. A copy of that same profile was opened by main and by this PR (
fe9903b0fdf).Before (main): the old Claude chat's button shows Ask for approval…
…and asking it to write a file stops at Allow Bash?, though this user never chose Ask.
After: the same old Claude chat shows Full access, and the same kind of request runs with no prompt (the new file is in the file list).
The old Codex chat on the same profile, also Full access with no prompt:
Control: a copy of the same profile with Agent Permissions on Manual. The old chats get Ask for approval, and a write shows a real approval card.
Testing
Automated (
pnpm teston the 8 permission, backfill, create, runtime and record-store test files): 122/122 pass. New tests cover:Fail-before controls: the new tests fail on the previous head of this branch (the mixed-arguments cases fail with "expected 'ask' to be 'full-access'") and pass now. The store test fails if the store takes request options on an existing chat.
Checks:
pnpm tc:nodepasses.orca-ci-checks --no-typecheckpasses except two checks that fail the same way on the branch's earlier head: five mobile import-cycle warnings, and a local Bun tooling error.CI: green on
3a7a4b98fcc. The wider runs also cover every test file undersrc/main/runtimeandsrc/main/native-chatlocally (1,736 tests). They caught three test stubs missing the launch-seed method that the attach path now reads, which are fixed.Live upgrade QA, macOS (M4Air, hidden dev builds, CDP-driven): on
fe9903b0fdf, fresh copies of the real pre-feat(native-chat): each Claude/Codex chat gets its own permission mode #26116 profile. Later commits only inline a small cache and fix test stubs, so behaviour is unchanged.Live QA on the previous head
dbdf865(same pass, filled from the shared default; not re-run on the final head):Not tested live: SSH and remote servers (code path only: each host runs the pass with its own settings), Linux, Windows, mobile. Picking a Codex model or effort is a separate bug, fixed in fix(native-chat): store a chat's permission mode on its own, so a model pick can't reset it #28268.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Notes
brennanb2025/chat-permission-old-chats-use-default-live-backup(first version), and in the branch history at509e5125558(second version).