Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import type { AgentSessionModelCatalogResult } from '../../../shared/agent-sessi
import { claudeAndCodexDeclared } from './structured-agent-session-adapter-router-test-support'
import { structuredAgentSessionLaunchPermission } from './structured-agent-session-launch-permission'

function record(provider: string, options: Record<string, string> | undefined): AgentSessionRecord {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch mode reads only the record's id, agent and options.
return { sessionId: 'session-1', provider, ...(options ? { options } : {}) } as AgentSessionRecord
function record(provider: string, saved: Record<string, string> | undefined): AgentSessionRecord {
const { permissionMode, ...options } = saved ?? {}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch mode reads only the record's id, agent, options and mode.
return { sessionId: 'session-1', provider, options, permissionMode } as AgentSessionRecord
}

const listing = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import type { AgentSessionRecord } from '../../../shared/agent-session-record'
import {
AGENT_CHAT_PERMISSION_MODE_OPTION_ID,
agentChatPermissionModes,
chatModelSupportsAutoMode,
chatPermissionModeNeedsModelSupport,
Expand Down Expand Up @@ -42,7 +41,7 @@ export async function structuredAgentSessionLaunchPermission(
if (!support || !deps.agents.definition(record.provider)) {
return null
}
const intent = record.options?.[AGENT_CHAT_PERMISSION_MODE_OPTION_ID]
const intent = record.permissionMode
const autoSupported =
chatPermissionModeNeedsModelSupport(support, intent) &&
(await listedAutoModeSupport(deps.modelCatalog, record))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ describe('an option picked while the chat is at rest', () => {
store: {
getRecord: () =>
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the intent reads only the record's provider and options.
({ provider: 'codex', options: {} }) as unknown as AgentSessionRecord
({ provider: 'codex', options: {} }) as unknown as AgentSessionRecord,
replaceSessionOptions: vi.fn()
},
agents: claudeAndCodexAgents()
},
{ sessionId: SESSION, persistOptions, publish: () => {} },
{ sessionId: SESSION, fence: 1, persistOptions, publish: () => {}, now: () => 0 },
{ key: 'notAnOption', value: 'x' }
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,10 @@ async function readStructuredAgentSessionOptionsAtRest(
/** Records a pick for the next start. Only a key the provider would accept is kept. */
export async function recordStructuredAgentSessionOptionIntent(
deps: {
store: Pick<AgentSessionRecordStore, 'getRecord'>
store: Pick<AgentSessionRecordStore, 'getRecord' | 'replaceSessionOptions'>
agents: Pick<StructuredAgentRegistry, 'definition'>
},
ctx: Pick<AgentSessionTurnContext, 'sessionId' | 'persistOptions' | 'publish'>,
ctx: Pick<AgentSessionTurnContext, 'sessionId' | 'fence' | 'persistOptions' | 'publish' | 'now'>,
input: { key: string; value: string }
): Promise<TurnOutcome<AgentSessionOptionResult>> {
const record = deps.store.getRecord(ctx.sessionId)
Expand All @@ -165,7 +165,15 @@ export async function recordStructuredAgentSessionOptionIntent(
}
}
const options = { ...record.options, [input.key]: input.value }
await ctx.persistOptions(options)
// The answer still names the mode among the options, as the wire always has.
await (input.key === AGENT_CHAT_PERMISSION_MODE_OPTION_ID
? deps.store.replaceSessionOptions({
sessionId: ctx.sessionId,
fence: ctx.fence,
permissionMode: input.value,
now: ctx.now()
})
: ctx.persistOptions(options))
ctx.publish()
return { ok: true, value: { ...input, options } }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,14 @@ describe('retireOutgrownStructuredAgentSessionChild', () => {
stop: vi.fn(),
providerStarted: vi.fn()
})
const launch = (options: Record<string, string>, rows = listed) => {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch mode reads only the record's id, agent and options.
const record = { sessionId: 'session', provider: 'claude', options } as AgentSessionRecord
const launch = ({ permissionMode, ...options }: Record<string, string>, rows = listed) => {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch mode reads only the record's id, agent, options and mode.
const record = {
sessionId: 'session',
provider: 'claude',
options,
permissionMode
} as AgentSessionRecord
return () =>
structuredAgentSessionLaunchPermission(
{ agents: claudeAndCodexDeclared(), modelCatalog: catalog(rows) },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ describe('a chat permission mode', () => {
})

const fence = () => rig.store.getRecord(SESSION)?.lease.runtimeFence ?? 1
const pick = (value: string) => {
const fields = { key: 'permissionMode', value }
const pick = (value: string, key = 'permissionMode') => {
const fields = { key, value }
return rig.host.setOption(CALLER, {
envelope: {
sessionId: SESSION,
Expand Down Expand Up @@ -59,17 +59,33 @@ describe('a chat permission mode', () => {
value: { options: { permissionMode: 'full-access' } }
})
expect(setOption).not.toHaveBeenCalled()
expect(rig.store.getRecord(SESSION)?.options?.permissionMode).toBe('full-access')
expect(rig.store.getRecord(SESSION)?.permissionMode).toBe('full-access')
expect((await rig.host.readOptions(SESSION)).current.permissionMode).toBe('full-access')
expect(rig.host.readStatusSummary(SESSION)?.permissionMode).toBe('full-access')
})

it('keeps Full access through a model pick on the running agent and the next start', async () => {
await foundRestTestChat(rig)
await pick('full-access')
expect((await rig.host.send(CALLER, restTestSend('on full access', fence()))).ok).toBe(true)
await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2))

expect(await pick('gpt-picked', 'model')).toMatchObject({ ok: true })
await rig.restart()
expect((await rig.host.send(CALLER, restTestSend('after relaunch', fence()))).ok).toBe(true)
await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(3))
expect(rig.adapter.acquire).toHaveBeenLastCalledWith(
expect.objectContaining({ permissionMode: 'full-access' })
)
expect(rig.host.readStatusSummary(SESSION)?.permissionMode).toBe('full-access')
})

it('refuses a mode the agent does not offer', async () => {
await foundRestTestChat(rig)
// Codex has no edits-only mode.
expect(await pick('accept-edits')).toMatchObject({ ok: false })
expect(await pick('bypassPermissions')).toMatchObject({ ok: false })
expect(rig.store.getRecord(SESSION)?.options?.permissionMode).toBeUndefined()
expect(rig.store.getRecord(SESSION)?.permissionMode).toBeUndefined()
})

it('starts a new agent with the new mode for the next send, and keeps it after', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ function fakeAdapter(): StructuredAgentSessionAdapter {
function pick(agents: StructuredAgentRegistry, record: AgentSessionRecord | null, key: string) {
const persistOptions = vi.fn(async () => {})
const result = recordStructuredAgentSessionOptionIntent(
{ store: { getRecord: () => record }, agents },
{ sessionId: RECORD.sessionId, persistOptions, publish: vi.fn() },
{ store: { getRecord: () => record, replaceSessionOptions: vi.fn() }, agents },
{ sessionId: RECORD.sessionId, fence: 1, persistOptions, publish: vi.fn(), now: () => 0 },
{ key, value: 'enabled' }
)
return { result, persistOptions }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -652,9 +652,9 @@ describe('a chat after startup permission backfill', () => {
await host.handleAdapterEvent(startedEvent())
await vi.advanceTimersByTimeAsync(50)
await host.flushStreamedEvents(SESSION)
expect(store.getRecord(SESSION)?.options?.permissionMode).toBe(mode)
expect(store.getRecord(SESSION)?.permissionMode).toBe(mode)
await store.backfillChatPermissionModes(() => mode === 'ask')
expect(store.getRecord(SESSION)?.options?.permissionMode).toBe(mode)
expect(store.getRecord(SESSION)?.permissionMode).toBe(mode)
}
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ import type { StructuredAgentSessionStatusState } from './structured-agent-sessi
import type { StructuredAgentSessionProviderChild } from './structured-agent-session-host-types'
import { structuredAgentSessionProviderSessionMetadata } from './structured-agent-session-history-result'
import { agentSessionPinnedLaunchDirectory } from '../../runtime/agent-session-record-launch-directory'
import {
AGENT_CHAT_PERMISSION_MODE_OPTION_ID,
isAgentChatPermissionMode
} from '../../../shared/agent-chat-permission-mode'
import { isAgentChatPermissionMode } from '../../../shared/agent-chat-permission-mode'

export function structuredAgentSessionStatusSummary({
sessionId,
Expand Down Expand Up @@ -41,7 +38,7 @@ export function structuredAgentSessionStatusSummary({
// The journal has no model: the record's acknowledged options are where a mid-session
// switch lands, so the row follows whichever is in force.
const model = normalizeOptionalField(record?.options?.model, AGENT_MODEL_MAX_LENGTH)
const permissionMode = record?.options?.[AGENT_CHAT_PERMISSION_MODE_OPTION_ID]
const permissionMode = record?.permissionMode
const launchDirectory = record ? agentSessionPinnedLaunchDirectory(record) : undefined
return {
sessionId,
Expand Down
70 changes: 27 additions & 43 deletions src/main/runtime/agent-session-permission-backfill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,45 +51,38 @@ describe('existing chat permission backfill', () => {
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 }
})
expect(store.getRecord('session-claude')).toEqual({ ...beforeClaude, permissionMode: mode })
expect(store.getRecord('session-codex')).toEqual({ ...beforeCodex, 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
)
expect(reopened.getRecord('session-claude')?.permissionMode).toBe(mode)
expect(reopened.getRecord('session-codex')?.permissionMode).toBe(mode)
}
)

it.each(['ask', 'full-access', 'invalid-saved-mode'] as const)(
'never replaces saved %s or options on other providers',
'never replaces saved %s, even one an earlier build saved among the options',
async (mode) => {
// The codex row is what an hourly build before the field wrote.
const earlier = { ...record('codex'), options: { model: 'gpt', permissionMode: mode } }
await seedTestAgentSessionRecordStore(root, {
records: [
record('claude', { permissionMode: mode }),
record('codex', { permissionMode: mode }),
earlier,
record('grok', { model: 'grok' })
]
})
const store = await openTestAgentSessionRecordStore(root)
const before = store.listRecords()
const [claude, codex, grok] = ['claude', 'codex', 'grok'].map((provider) =>
store.getRecord(`session-${provider}`)
)
await store.backfillChatPermissionModes(() => mode !== 'full-access')
expect(store.listRecords()).toEqual(before)
expect(store.getRecord('session-grok')?.options).toEqual({ model: 'grok' })
expect(store.getRecord('session-claude')).toEqual(claude)
expect(store.getRecord('session-codex')).toEqual({
...codex,
options: { model: 'gpt' },
permissionMode: mode
})
expect(store.getRecord('session-grok')).toEqual(grok)
}
)

Expand All @@ -105,7 +98,7 @@ describe('existing chat permission backfill', () => {
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')
expect(store.getRecord('session-codex')?.permissionMode).toBe('full-access')
})

it('skips a store written by a newer build without an error or in-memory change', async () => {
Expand All @@ -115,7 +108,7 @@ describe('existing chat permission backfill', () => {
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()
expect(store.getRecord('session-claude')?.permissionMode).toBeUndefined()
})

it.each([
Expand All @@ -129,9 +122,9 @@ describe('existing chat permission backfill', () => {
})
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()
expect(store.getRecord('session-claude')?.permissionMode).toBe(claude)
expect(store.getRecord('session-codex')?.permissionMode).toBe(codex)
expect(store.getRecord('session-grok')?.permissionMode).toBeUndefined()
}
)

Expand Down Expand Up @@ -170,14 +163,7 @@ describe('existing chat permission backfill', () => {
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'
}
})
expect(store.getRecord('session-codex')).toEqual({ ...before, permissionMode: 'full-access' })
})

it('rolls back every row if saving fails, then succeeds on the next startup pass', async () => {
Expand All @@ -188,16 +174,14 @@ describe('existing chat permission backfill', () => {
"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(store.listRecords().every((row) => row.permissionMode === undefined)).toBe(true)
expect(
(await openTestAgentSessionRecordStore(root))
.listRecords()
.every((row) => row.options === undefined)
.every((row) => row.permissionMode === 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
)
expect(store.listRecords().every((row) => row.permissionMode === 'full-access')).toBe(true)
})
})
18 changes: 9 additions & 9 deletions src/main/runtime/agent-session-permission-backfill.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import {
AGENT_CHAT_PERMISSION_MODE_OPTION_ID,
newChatPermissionMode
} from '../../shared/agent-chat-permission-mode'
import { 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. */
Expand All @@ -14,18 +11,21 @@ export function backfillAgentSessionPermissionModes(
}
return store.transact((draft) => {
for (const [sessionId, record] of draft.records) {
if (record.options?.[AGENT_CHAT_PERMISSION_MODE_OPTION_ID] !== undefined) {
if (record.permissionMode !== undefined) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
continue
}
// Temporary: only the dev team's hourly builds saved the mode among options; remove after 1.4.226 ships.
const { permissionMode: saved, ...options } = record.options ?? {}
if (saved !== undefined) {
draft.records.set(sessionId, { ...record, options, permissionMode: saved })
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 }
})
draft.records.set(sessionId, { ...record, permissionMode })
}
}
})
Expand Down
7 changes: 6 additions & 1 deletion src/main/runtime/agent-session-record-founding.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AGENT_CHAT_PERMISSION_MODE_OPTION_ID } from '../../shared/agent-chat-permission-mode'
import {
AGENT_SESSION_RECORD_SCHEMA_VERSION,
type AgentSessionRecord
Expand All @@ -19,13 +20,17 @@ export function agentSessionRecordIdentityFields(
identity: AgentSessionRecordIdentity,
now: number
) {
// Callers name the mode among the options, as the wire does; the record keeps it as its own field.
const { [AGENT_CHAT_PERMISSION_MODE_OPTION_ID]: permissionMode, ...options } =
identity.options ?? {}
return {
schemaVersion: AGENT_SESSION_RECORD_SCHEMA_VERSION,
sessionId: identity.sessionId,
location: identity.location,
provider: identity.provider,
accountHome: identity.accountHome,
...(identity.options ? { options: { ...identity.options } } : {}),
...(Object.keys(options).length > 0 ? { options } : {}),
...(permissionMode ? { permissionMode } : {}),
...(identity.launchArgs ? { launchArgs: [...identity.launchArgs] } : {}),
// A /clear continues in the same tab, so it stays in the folder the chat ran in.
...(identity.launchDirectory ? { launchDirectory: identity.launchDirectory } : {}),
Expand Down
4 changes: 3 additions & 1 deletion src/main/runtime/agent-session-record-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ export function replaceAgentSessionRecordOptions(
if (lease.runtimeFence !== replacement.fence || (lease.claimStatus !== 'live' && !atRest)) {
throw agentSessionRefusalError('agent_session_ownership_unknown', { reason: 'leaseMoved' })
}
return { ...record, options: { ...replacement.options }, updatedAt: replacement.now }
return 'options' in replacement
? { ...record, options: { ...replacement.options }, updatedAt: replacement.now }
: { ...record, permissionMode: replacement.permissionMode, updatedAt: replacement.now }
}
2 changes: 1 addition & 1 deletion src/main/runtime/agent-session-record-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ describe('acquisition path', () => {
})
)
expect(store.getRecord('session-alpha')?.lease.runtimeFence).toBe(2)
expect(store.getRecord('session-alpha')?.options).toEqual({ permissionMode: 'ask' })
expect(store.getRecord('session-alpha')?.permissionMode).toBe('ask')
})
})

Expand Down
Loading
Loading