Repository navigation
fix(native-chat): chats from before permission modes keep the mode they were running with #28099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
brennanb2025
merged 6 commits into
main
from
brennanb2025/chat-permission-old-chats-use-default
Oct 13, 2026
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
eb43000
fix(native-chat): chats saved before permission modes run the new-cha…
brennanb2025 509e512
fix(native-chat): an older chat keeps the new-chat mode its first sta…
brennanb2025 dbdf865
fix(native-chat): save older chats' permission mode once at host startup
brennanb2025 fe9903b
fix(native-chat): fill each older chat from its own agent's arguments
brennanb2025 b24b80a
refactor(native-chat): read each chat's agent arguments directly in t…
brennanb2025 3a7a4b9
test(native-chat): give stub runtimes the launch seed the attach path…
brennanb2025 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
203 changes: 203 additions & 0 deletions
203
src/main/runtime/agent-session-permission-backfill.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,203 @@ | ||
| import { mkdtemp, rm } from 'node:fs/promises' | ||
| import { tmpdir } from 'node:os' | ||
| import { join } from 'node:path' | ||
| import { afterEach, beforeEach, describe, expect, it } from 'vitest' | ||
| import { codexProviderHandle } from '../../shared/agent-session-provider-handle-encoding' | ||
| import { foundAgentSessionRecord } from './agent-session-record-founding' | ||
| import { | ||
| openTestAgentSessionRecordStore, | ||
| seedTestAgentSessionRecordStore, | ||
| seedTestAgentSessionStoreFromNewerBuild | ||
| } from './agent-session-record-store-test-harness' | ||
| import { openTestJournalHostDatabase } from '../native-chat/agent-session-journal/journal-host-database-test-support' | ||
|
|
||
| let root: string | ||
| beforeEach(async () => { | ||
| root = await mkdtemp(join(tmpdir(), 'orca-permission-backfill-')) | ||
| }) | ||
| afterEach(async () => { | ||
| await rm(root, { recursive: true, force: true }) | ||
| }) | ||
|
|
||
| function record(provider: string, options?: Record<string, string>) { | ||
| return foundAgentSessionRecord( | ||
| { | ||
| sessionId: `session-${provider}`, | ||
| provider, | ||
| location: { | ||
| executionHostId: 'local', | ||
| workspaceId: 'folder-1', | ||
| workspaceKind: 'folder', | ||
| wslDistro: null | ||
| }, | ||
| accountHome: { | ||
| variable: provider === 'claude' ? 'CLAUDE_CONFIG_DIR' : 'CODEX_HOME', | ||
| path: '/account' | ||
| }, | ||
| ...(options ? { options } : {}) | ||
| }, | ||
| { claimKeyId: 'key-1', now: 1 } | ||
| ) | ||
| } | ||
|
|
||
| describe('existing chat permission backfill', () => { | ||
| it.each(['full-access', 'ask'] as const)( | ||
| "saves %s from the agent's own Arguments for old Claude and Codex rows, keeping other options", | ||
| async (mode) => { | ||
| await seedTestAgentSessionRecordStore(root, { | ||
| records: [record('claude', { model: 'opus', effort: 'high' }), record('codex')] | ||
| }) | ||
| const store = await openTestAgentSessionRecordStore(root) | ||
| const beforeClaude = store.getRecord('session-claude') | ||
| const beforeCodex = store.getRecord('session-codex') | ||
| await store.backfillChatPermissionModes(() => mode === 'full-access') | ||
| expect(store.getRecord('session-claude')?.options).toEqual({ | ||
| model: 'opus', | ||
| effort: 'high', | ||
| permissionMode: mode | ||
| }) | ||
| expect(store.getRecord('session-codex')?.options).toEqual({ permissionMode: mode }) | ||
| expect(store.getRecord('session-claude')).toEqual({ | ||
| ...beforeClaude, | ||
| options: { model: 'opus', effort: 'high', permissionMode: mode } | ||
| }) | ||
| expect(store.getRecord('session-codex')).toEqual({ | ||
| ...beforeCodex, | ||
| options: { permissionMode: mode } | ||
| }) | ||
| const reopened = await openTestAgentSessionRecordStore(root) | ||
| expect(reopened.getRecord('session-claude')?.options).toEqual( | ||
| store.getRecord('session-claude')?.options | ||
| ) | ||
| expect(reopened.getRecord('session-codex')?.options).toEqual( | ||
| store.getRecord('session-codex')?.options | ||
| ) | ||
| } | ||
| ) | ||
|
|
||
| it.each(['ask', 'full-access', 'invalid-saved-mode'] as const)( | ||
| 'never replaces saved %s or options on other providers', | ||
| async (mode) => { | ||
| await seedTestAgentSessionRecordStore(root, { | ||
| records: [ | ||
| record('claude', { permissionMode: mode }), | ||
| record('codex', { permissionMode: mode }), | ||
| record('grok', { model: 'grok' }) | ||
| ] | ||
| }) | ||
| const store = await openTestAgentSessionRecordStore(root) | ||
| const before = store.listRecords() | ||
| await store.backfillChatPermissionModes(() => mode !== 'full-access') | ||
| expect(store.listRecords()).toEqual(before) | ||
| expect(store.getRecord('session-grok')?.options).toEqual({ model: 'grok' }) | ||
| } | ||
| ) | ||
|
|
||
| it('does no row write on its second pass, even with changed Arguments', async () => { | ||
| await seedTestAgentSessionRecordStore(root, { records: [record('claude'), record('codex')] }) | ||
| const store = await openTestAgentSessionRecordStore(root) | ||
| await store.backfillChatPermissionModes(() => true) | ||
| const before = store.listRecords() | ||
| const db = openTestJournalHostDatabase(root).db | ||
| db.exec( | ||
| "CREATE TRIGGER refuse_backfill_write BEFORE UPDATE ON agent_session_records BEGIN SELECT RAISE(ABORT, 'unexpected second write'); END" | ||
| ) | ||
| await expect(store.backfillChatPermissionModes(() => false)).resolves.toBeUndefined() | ||
| expect(store.listRecords()).toEqual(before) | ||
| expect(store.getRecord('session-claude')).toBe(before.find((row) => row.provider === 'claude')) | ||
| expect(store.getRecord('session-codex')?.options?.permissionMode).toBe('full-access') | ||
| }) | ||
|
|
||
| it('skips a store written by a newer build without an error or in-memory change', async () => { | ||
| await seedTestAgentSessionRecordStore(root, { records: [record('claude')] }) | ||
| await seedTestAgentSessionStoreFromNewerBuild(root) | ||
| const store = await openTestAgentSessionRecordStore(root) | ||
| expect(store.readOnly).toBe(true) | ||
| expect(store.getRecord('session-claude')).not.toBeNull() | ||
| await expect(store.backfillChatPermissionModes(() => true)).resolves.toBeUndefined() | ||
| expect(store.getRecord('session-claude')?.options).toBeUndefined() | ||
| }) | ||
|
|
||
| it.each([ | ||
| ['claude', 'full-access', 'ask'], | ||
| ['codex', 'ask', 'full-access'] | ||
| ] as const)( | ||
| "fills each row from its own agent's Arguments when only %s skips prompts", | ||
| async (bypassing, claude, codex) => { | ||
| await seedTestAgentSessionRecordStore(root, { | ||
| records: [record('claude'), record('codex'), record('grok')] | ||
| }) | ||
| const store = await openTestAgentSessionRecordStore(root) | ||
| await store.backfillChatPermissionModes((provider) => provider === bypassing) | ||
| expect(store.getRecord('session-claude')?.options).toEqual({ permissionMode: claude }) | ||
| expect(store.getRecord('session-codex')?.options).toEqual({ permissionMode: codex }) | ||
| expect(store.getRecord('session-grok')?.options).toBeUndefined() | ||
| } | ||
| ) | ||
|
|
||
| it('preserves the existing owner identity, fence, timestamps and provider history', async () => { | ||
| const old = record('codex', { model: 'old-model', effort: 'high' }) | ||
| await seedTestAgentSessionRecordStore(root, { | ||
| records: [ | ||
| { | ||
| ...old, | ||
| lease: { | ||
| ...old.lease, | ||
| runtimeFence: 7, | ||
| claimStatus: 'reserved', | ||
| handoffStage: 'new-owner-proving', | ||
| reservedSpawnToken: 'old-spawn', | ||
| ownerProcess: { | ||
| hostId: 'local', | ||
| pid: 42, | ||
| processStartTimeMs: 1, | ||
| spawnToken: 'old-spawn' | ||
| } | ||
| }, | ||
| providerHandleChain: [ | ||
| { | ||
| linkId: 'old-link', | ||
| handle: codexProviderHandle('old-thread'), | ||
| origin: 'created', | ||
| mintedAtFence: 6, | ||
| observedAt: 1 | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }) | ||
| const store = await openTestAgentSessionRecordStore(root) | ||
| const before = store.getRecord('session-codex') | ||
| expect(before).not.toBeNull() | ||
| await store.backfillChatPermissionModes(() => true) | ||
| expect(store.getRecord('session-codex')).toEqual({ | ||
| ...before, | ||
| options: { | ||
| model: 'old-model', | ||
| effort: 'high', | ||
| permissionMode: 'full-access' | ||
| } | ||
| }) | ||
| }) | ||
|
|
||
| it('rolls back every row if saving fails, then succeeds on the next startup pass', async () => { | ||
| await seedTestAgentSessionRecordStore(root, { records: [record('claude'), record('codex')] }) | ||
| const store = await openTestAgentSessionRecordStore(root) | ||
| const db = openTestJournalHostDatabase(root).db | ||
| db.exec( | ||
| "CREATE TRIGGER refuse_backfill_write BEFORE UPDATE ON agent_session_records WHEN NEW.session_id = 'session-codex' BEGIN SELECT RAISE(ABORT, 'disk full'); END" | ||
| ) | ||
| await expect(store.backfillChatPermissionModes(() => true)).rejects.toThrow('disk full') | ||
| expect(store.listRecords().every((row) => row.options === undefined)).toBe(true) | ||
| expect( | ||
| (await openTestAgentSessionRecordStore(root)) | ||
| .listRecords() | ||
| .every((row) => row.options === undefined) | ||
| ).toBe(true) | ||
| db.exec('DROP TRIGGER refuse_backfill_write') | ||
| await store.backfillChatPermissionModes(() => true) | ||
| expect(store.listRecords().every((row) => row.options?.permissionMode === 'full-access')).toBe( | ||
| true | ||
| ) | ||
| }) | ||
| }) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { | ||
| AGENT_CHAT_PERMISSION_MODE_OPTION_ID, | ||
| newChatPermissionMode | ||
| } from '../../shared/agent-chat-permission-mode' | ||
| import type { AgentSessionStoreTransactions } from './agent-session-store-transactions' | ||
|
|
||
| /** One transaction as the chat host installs; before modes, chats ran their own agent's Arguments. */ | ||
| export function backfillAgentSessionPermissionModes( | ||
| store: AgentSessionStoreTransactions, | ||
| ranWithFullAccess: (provider: string) => boolean | ||
| ): Promise<void> { | ||
| if (store.readOnly) { | ||
| return Promise.resolve() | ||
| } | ||
| return store.transact((draft) => { | ||
| for (const [sessionId, record] of draft.records) { | ||
| if (record.options?.[AGENT_CHAT_PERMISSION_MODE_OPTION_ID] !== undefined) { | ||
| continue | ||
| } | ||
| const permissionMode = newChatPermissionMode( | ||
| record.provider, | ||
| ranWithFullAccess(record.provider) ? 'full-access' : 'ask' | ||
| ) | ||
| if (permissionMode) { | ||
| draft.records.set(sessionId, { | ||
| ...record, | ||
| options: { ...record.options, [AGENT_CHAT_PERMISSION_MODE_OPTION_ID]: permissionMode } | ||
| }) | ||
| } | ||
| } | ||
| }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 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. CallcloseTestJournalHostDatabases()beforerm, as the adjacent store-transaction tests do. (raw.githubusercontent.com)