From 21ec21f65a1ffcacecc08534c0ffc76c16aa004b Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Sun, 6 Sep 2026 18:49:42 -0700 Subject: [PATCH] fix(native-chat): widen adopted-home discovery and keep ordinary launches untouched Three corrections from review of the first commit. The adoption's account-home candidates now include the extra Codex homes session discovery already scans. A row this host listed could otherwise refuse to resume, which reads as the feature being broken rather than as a scope. Ordinary launches call `createStructuredAgentSessionLaunchIntent` with two arguments again. Passing the resume source unconditionally appended a trailing `undefined` that four existing call-site assertions had to absorb; the churn was the caller's fault, not the tests'. The transactional adoption guard's comment claimed the self-exemption is what lets a committed create replay. It is not: replay is settled earlier by the operation ledger, and an adoption always arrives with a null expected fence, so a request naming an existing session id is refused a few lines below either way. The exemption is part of what "another record" means, and the comment now says that instead. --- src/main/ai-vault/cached-session-list.ts | 6 ++++++ .../agent-session-reservation-admission.ts | 14 ++++++++++---- ...olve-recovered-structured-tui-transcript.ts | 4 ++++ .../launch-agent-structured-chat-guard.test.ts | 18 +++--------------- .../structured-agent-session-launch.test.ts | 5 ++--- .../src/lib/structured-agent-session-launch.ts | 7 ++++++- 6 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/main/ai-vault/cached-session-list.ts b/src/main/ai-vault/cached-session-list.ts index c8d04205091..c46e4bdd4a6 100644 --- a/src/main/ai-vault/cached-session-list.ts +++ b/src/main/ai-vault/cached-session-list.ts @@ -49,6 +49,12 @@ export function configureAiVaultSessionSources(next: AiVaultSessionSources): voi sources = next } +/** The extra Codex homes session discovery scans. Anything that decides what a listed row may be + * resumed from must read the same set, or a row can be listed and then refuse to resume. */ +export function configuredAdditionalCodexHomePaths(): readonly string[] { + return sources.getAdditionalCodexHomePaths?.() ?? [] +} + export async function listAiVaultSessions( args?: AiVaultListArgs, options: { signal?: AbortSignal } = {} diff --git a/src/main/runtime/agent-session-reservation-admission.ts b/src/main/runtime/agent-session-reservation-admission.ts index 2a8b84c7654..82176a05297 100644 --- a/src/main/runtime/agent-session-reservation-admission.ts +++ b/src/main/runtime/agent-session-reservation-admission.ts @@ -194,11 +194,17 @@ export function applyAgentSessionReservation( } /** - * Refuse an adoption whose conversation another record already holds. + * Refuse an adoption whose conversation ANOTHER record already holds. * - * Exempts the request's own session id: a retry of a create that already committed re-runs every - * pre-commit check, and by then the record it created holds the conversation itself. Without the - * exemption the replay would refuse as a conflict instead of replaying. + * The self-exemption is part of that definition, not a replay mechanism: replay is settled earlier + * by the operation ledger, and an adoption always arrives with a null expected fence, so a request + * naming an existing session id is refused a few lines below regardless. Keeping the scan scoped to + * other records is what makes this guard mean what its name says. + * + * It runs inside the store transaction because the pre-commit check in the RPC resolver cannot be + * the guard: two concurrent adoptions of one conversation mint different session ids, so the + * compare-and-swap never collides and both would pass. Codex permits two app-servers on one thread + * silently, so the cost of missing this is a corrupted conversation rather than an error. */ function assertAdoptedConversationUnowned( state: AgentSessionStoreState, diff --git a/src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts b/src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts index b5488c924af..17e5051327e 100644 --- a/src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts +++ b/src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts @@ -14,6 +14,7 @@ import { structuredAdoptionConflictError } from '../native-chat/structured-agent-session-history-adoption' import { resolveSessionFilePath } from '../native-chat/session-file-resolver' +import { configuredAdditionalCodexHomePaths } from '../ai-vault/cached-session-list' import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' import type { AgentStatusIpcPayload } from '../../shared/agent-status-types' import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options' @@ -166,6 +167,9 @@ export class OrcaRuntimeWithResolveRecoveredStructuredTuiTranscript extends Orca return [ selectedAccountHomePath, ...managedHomes, + // The same extra homes session discovery scans. Without these a row that this host listed + // could refuse to resume, which reads as the feature being broken rather than as a scope. + ...configuredAdditionalCodexHomePaths(), getOrcaManagedCodexHomePath(), getSystemCodexHomePath() ] diff --git a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts index ee42413328e..179f6203d0b 100644 --- a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts +++ b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts @@ -174,11 +174,7 @@ describe('structured chat adoption guard on the launch path', () => { focusAfterMenuClose: 'structured-session' }) expect(shouldQueueTerminalFocusAfterMenuClose(result!)).toBe(false) - expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledWith( - 'wt-1', - 'codex', - undefined - ) + expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledWith('wt-1', 'codex') expect(mockLaunchStructuredCodexSession).toHaveBeenCalledWith( expect.objectContaining({ worktreeId: 'wt-1' }) ) @@ -199,11 +195,7 @@ describe('structured chat adoption guard on the launch path', () => { const result = launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) expect(result).toMatchObject({ tabId: null, focusAfterMenuClose: 'structured-session' }) - expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledWith( - 'wt-1', - 'codex', - undefined - ) + expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledWith('wt-1', 'codex') expect(mockCreateTab).not.toHaveBeenCalled() }) @@ -213,11 +205,7 @@ describe('structured chat adoption guard on the launch path', () => { const result = launchAgentInNewTab({ agent: 'claude', worktreeId: 'wt-1' }) expect(result).toMatchObject({ tabId: null, focusAfterMenuClose: 'structured-session' }) - expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledWith( - 'wt-1', - 'claude', - undefined - ) + expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledWith('wt-1', 'claude') expect(mockCreateTab).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/lib/structured-agent-session-launch.test.ts b/src/renderer/src/lib/structured-agent-session-launch.test.ts index ec2f0cce2ed..61d24b248d2 100644 --- a/src/renderer/src/lib/structured-agent-session-launch.test.ts +++ b/src/renderer/src/lib/structured-agent-session-launch.test.ts @@ -173,9 +173,8 @@ describe('startStructuredAgentLaunch', () => { const codex = startStructuredAgentLaunch(worktreeId, 'codex') await flushLaunchSettlement() - // Third argument is the adopted conversation; a blank launch passes none. - expect(mocks.createIntent).toHaveBeenNthCalledWith(1, worktreeId, 'claude', undefined) - expect(mocks.createIntent).toHaveBeenNthCalledWith(2, worktreeId, 'codex', undefined) + expect(mocks.createIntent).toHaveBeenNthCalledWith(1, worktreeId, 'claude') + expect(mocks.createIntent).toHaveBeenNthCalledWith(2, worktreeId, 'codex') expect(mocks.launch).toHaveBeenCalledTimes(2) expect(vi.mocked(mocks.launch).mock.calls.map(([intent]) => intent.params.agent)).toEqual([ 'claude', diff --git a/src/renderer/src/lib/structured-agent-session-launch.ts b/src/renderer/src/lib/structured-agent-session-launch.ts index 85751c0e3b8..7543c176180 100644 --- a/src/renderer/src/lib/structured-agent-session-launch.ts +++ b/src/renderer/src/lib/structured-agent-session-launch.ts @@ -252,7 +252,12 @@ function structuredAgentLaunchState( } } - const intent = createStructuredAgentSessionLaunchIntent(worktreeId, agent, options.resumeFrom) + // Only pass the third argument when adopting: every ordinary launch keeps the two-argument call + // it has always made, so this change adds no trailing `undefined` for call-site assertions to + // absorb. + const intent = options.resumeFrom + ? createStructuredAgentSessionLaunchIntent(worktreeId, agent, options.resumeFrom) + : createStructuredAgentSessionLaunchIntent(worktreeId, agent) const text = options.prompt?.trim() ?? '' const stagedPrompt = text ? enqueueStructuredAgentSessionLaunchPrompt(intent.sessionId, text)