mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 08:03:12 +00:00
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.
This commit is contained in:
@@ -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 } = {}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
]
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user