mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 16:03:49 +00:00
fix(native-chat): let the reservation holder win an overlapping adoption
Two adoptions of the same Codex pane destroyed each other. The pane binding was released by pty id alone, gated on a `priorBoundSessionId` sampled before the reservation, and both attempts carry the same sessionId — so the attempt whose reservation had been superseded unbound the pane the winner was proving on, `admitProof` refused the winner's `/status` keystroke, and both failed. The write gate now records which attempt holds a pane, keyed on the spawn token the reservation already mints: a claim supersedes another attempt's but never a settled owner's, the release is a compare-and-clear, and a proven owner settles the binding so no later attempt can take it. No lock is needed; the reservation release beside it was already fence-scoped and unchanged. A failed `releaseAdoptedTuiReservation` stays swallowed — the proof error is the one the user must act on — but is now logged with session, fence and spawn token, because a release that never persisted leaves the lease latched and refuses every later attempt with no record of why. Adoption's no-record branch is deleted. The only caller reserves the lease first and nothing deletes a record, so it minted an owner for a session the store had never heard of in unit tests only; its test now drives the real reserve-then-adopt sequence, and a new one pins the refusal. Launch-at-chat discarded `waitForAgentReady`'s result, so a readiness timeout still flipped the tab to chat and adoption then probed a shell. It stays in terminal view instead: the terminal is usable and the view toggle re-runs adoption whenever the user wants it. Each fix was reverted against its new test to confirm the test reddens; all four defects had been living under green suites.
This commit is contained in:
+33
-9
@@ -79,15 +79,34 @@ afterEach(async () => {
|
||||
await rm(root, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
/** What the runtime does before it may type `/status` into the pane it is adopting. */
|
||||
async function reserveAdoption(attach: AgentSessionAttachParams, spawnToken: string) {
|
||||
return host.reserveAdoptedTuiOwner({
|
||||
caller: CALLER,
|
||||
sessionId: attach.envelope.sessionId,
|
||||
clientOperationId: attach.envelope.clientOperationId,
|
||||
fingerprint: attach.envelope.payloadFingerprint,
|
||||
location: attach.location,
|
||||
provider: 'codex',
|
||||
accountHome: attach.accountHome,
|
||||
spawnToken,
|
||||
claimKeyId: 'key-1'
|
||||
})
|
||||
}
|
||||
|
||||
describe('structured TUI adoption', () => {
|
||||
it('mints a durable TUI owner without starting a native provider', async () => {
|
||||
const adoptedOwner = owner()
|
||||
const attach = params()
|
||||
await expect(reserveAdoption(attach, adoptedOwner.process.spawnToken)).resolves.toMatchObject({
|
||||
ok: true,
|
||||
fence: 1
|
||||
})
|
||||
|
||||
const result = await host.adoptTuiOwner({
|
||||
caller: CALLER,
|
||||
params: params(),
|
||||
owner: adoptedOwner,
|
||||
claimKeyId: 'key-1'
|
||||
params: attach,
|
||||
owner: adoptedOwner
|
||||
})
|
||||
|
||||
expect(result).toMatchObject({ ok: true, replayed: false, fence: 1 })
|
||||
@@ -107,12 +126,7 @@ describe('structured TUI adoption', () => {
|
||||
const invalid = params()
|
||||
invalid.envelope.payloadFingerprint = '0'.repeat(64)
|
||||
|
||||
const result = await host.adoptTuiOwner({
|
||||
caller: CALLER,
|
||||
params: invalid,
|
||||
owner: owner(),
|
||||
claimKeyId: 'key-1'
|
||||
})
|
||||
const result = await host.adoptTuiOwner({ caller: CALLER, params: invalid, owner: owner() })
|
||||
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
@@ -121,4 +135,14 @@ describe('structured TUI adoption', () => {
|
||||
expect(store.getRecord(SESSION)).toBeNull()
|
||||
expect(acquire).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// The reservation is the only way in: adoption has no second mode that acquires the lease
|
||||
// itself, so nothing here may invent an owner for a session the store has never heard of.
|
||||
it('refuses to adopt a session that holds no reservation', async () => {
|
||||
await expect(
|
||||
host.adoptTuiOwner({ caller: CALLER, params: params(), owner: owner() })
|
||||
).rejects.toThrow('agent_session_ownership_unknown')
|
||||
expect(store.getRecord(SESSION)).toBeNull()
|
||||
expect(acquire).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -5,11 +5,7 @@ import type {
|
||||
AgentSessionMutationResult
|
||||
} from '../../../shared/agent-session-wire'
|
||||
import type { AgentSessionAttachParams } from './structured-agent-session-attach'
|
||||
import {
|
||||
admitAttachOrRefuse,
|
||||
attachJournal,
|
||||
reserveRequestFor
|
||||
} from './structured-agent-session-attach'
|
||||
import { admitAttachOrRefuse, attachJournal } from './structured-agent-session-attach'
|
||||
import type {
|
||||
StructuredAgentSessionCaller,
|
||||
StructuredAgentSessionHostDeps,
|
||||
@@ -22,8 +18,6 @@ export type StructuredTuiAdoptionRequest = {
|
||||
caller: StructuredAgentSessionCaller
|
||||
params: AgentSessionAttachParams
|
||||
owner: StructuredTuiOwner
|
||||
claimKeyId: string
|
||||
launchEnv?: Record<string, string>
|
||||
}
|
||||
|
||||
export function adoptStructuredTuiOwner(
|
||||
@@ -48,32 +42,19 @@ async function adopt(
|
||||
return admitted
|
||||
}
|
||||
let record = input.deps.store.getRecord(sessionId)
|
||||
let replayed = record !== null
|
||||
if (record && isStructuredTuiAdoptionReservation(record, owner.process.spawnToken)) {
|
||||
if (!record) {
|
||||
// Adoption reserves the lease before it may prove the pane, and nothing deletes a record, so
|
||||
// this call always runs against one. Refusing beats reserving here: a second acquisition mode
|
||||
// that no caller can enter is coverage that proves nothing.
|
||||
throw new Error('agent_session_ownership_unknown')
|
||||
}
|
||||
let replayed = true
|
||||
if (isStructuredTuiAdoptionReservation(record, owner.process.spawnToken)) {
|
||||
// This adoption's own pre-proof reservation: turn it into an owner rather than reserve again.
|
||||
replayed = false
|
||||
record = await proveAdoptedTuiOwner(input, record)
|
||||
} else if (record) {
|
||||
assertMatchingTuiOwner(record, owner)
|
||||
} else {
|
||||
const reserved = await input.deps.store.reserveOwner(
|
||||
reserveRequestFor({
|
||||
sessionId,
|
||||
params,
|
||||
authority: {
|
||||
spawnToken: owner.process.spawnToken,
|
||||
claimKeyId: input.claimKeyId,
|
||||
handoffOperationId: params.envelope.clientOperationId,
|
||||
probe: { outcome: 'reservation-unused' },
|
||||
...(input.launchEnv ? { launchEnv: input.launchEnv } : {})
|
||||
},
|
||||
callerKey: input.caller.callerKey,
|
||||
fingerprint: admitted.fingerprint,
|
||||
now: input.now()
|
||||
})
|
||||
)
|
||||
replayed = reserved.disposition === 'replayed'
|
||||
record = await proveAdoptedTuiOwner(input, reserved.record)
|
||||
assertMatchingTuiOwner(record, owner)
|
||||
}
|
||||
const attached = await attachJournal({
|
||||
record,
|
||||
|
||||
@@ -76,6 +76,48 @@ describe('binding lifecycle', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('overlapping adoption attempts on one pane', () => {
|
||||
beforeEach(() => {
|
||||
gate.attachRecordLookup((sessionId) => records.get(sessionId) ?? null)
|
||||
})
|
||||
|
||||
it('keeps the newer attempt bound when the one it superseded gives up', () => {
|
||||
gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-a')
|
||||
expect(gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-b')).toBe(true)
|
||||
|
||||
// Both attempts carry the same session, so only the spawn token can tell this release apart.
|
||||
expect(gate.releasePtyAttempt(PTY_ID, 'spawn-a')).toBe(false)
|
||||
expect(gate.boundSessionId(PTY_ID)).toBe(SESSION_ID)
|
||||
expect(gate.releasePtyAttempt(PTY_ID, 'spawn-b')).toBe(true)
|
||||
expect(gate.boundSessionId(PTY_ID)).toBeNull()
|
||||
})
|
||||
|
||||
it('leaves a settled owner pane alone when a later attempt fails', () => {
|
||||
gate.bindPty(PTY_ID, SESSION_ID)
|
||||
|
||||
expect(gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-late')).toBe(false)
|
||||
expect(gate.releasePtyAttempt(PTY_ID, 'spawn-late')).toBe(false)
|
||||
expect(gate.boundSessionId(PTY_ID)).toBe(SESSION_ID)
|
||||
})
|
||||
|
||||
it('settles a proven attempt so no later attempt can release its pane', () => {
|
||||
gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-a')
|
||||
expect(gate.settlePtyAttempt(PTY_ID, 'spawn-a')).toBe(true)
|
||||
|
||||
expect(gate.settlePtyAttempt(PTY_ID, 'spawn-a')).toBe(false)
|
||||
expect(gate.releasePtyAttempt(PTY_ID, 'spawn-a')).toBe(false)
|
||||
expect(gate.boundSessionId(PTY_ID)).toBe(SESSION_ID)
|
||||
})
|
||||
|
||||
it('settles nothing once the pane is gone', () => {
|
||||
gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-a')
|
||||
gate.unbindPty(PTY_ID)
|
||||
|
||||
expect(gate.settlePtyAttempt(PTY_ID, 'spawn-a')).toBe(false)
|
||||
expect(gate.boundSessionId(PTY_ID)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('admission through the store', () => {
|
||||
beforeEach(() => {
|
||||
gate.attachRecordLookup((sessionId) => records.get(sessionId) ?? null)
|
||||
|
||||
@@ -31,8 +31,17 @@ const ADMITTED_UNBOUND: AgentSessionPtyWriteAdmission = {
|
||||
runtimeFence: null
|
||||
}
|
||||
|
||||
/**
|
||||
* A pane binding, plus the adoption attempt that is still proving it.
|
||||
*
|
||||
* `attemptToken` is null once an owner is proven — a settled binding no attempt may take away.
|
||||
* While it is non-null the binding belongs to that one in-flight attempt, which is the only thing
|
||||
* that tells two overlapping adoptions apart: both carry the same sessionId.
|
||||
*/
|
||||
type BoundPane = { sessionId: string; attemptToken: string | null }
|
||||
|
||||
export class AgentSessionPtyWriteGate {
|
||||
private readonly sessionIdByPtyId = new Map<string, string>()
|
||||
private readonly panesByPtyId = new Map<string, BoundPane>()
|
||||
private lookup: AgentSessionRecordLookup | null = null
|
||||
|
||||
/** Point the gate at the durable store. Until this is called nothing can be enforced. */
|
||||
@@ -42,24 +51,62 @@ export class AgentSessionPtyWriteGate {
|
||||
|
||||
detachRecordLookup(): void {
|
||||
this.lookup = null
|
||||
this.sessionIdByPtyId.clear()
|
||||
this.panesByPtyId.clear()
|
||||
}
|
||||
|
||||
bindPty(ptyId: string, sessionId: string): void {
|
||||
this.sessionIdByPtyId.set(ptyId, sessionId)
|
||||
this.panesByPtyId.set(ptyId, { sessionId, attemptToken: null })
|
||||
}
|
||||
|
||||
unbindPty(ptyId: string): void {
|
||||
this.sessionIdByPtyId.delete(ptyId)
|
||||
this.panesByPtyId.delete(ptyId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Claim the pane for one adoption attempt, identified by the spawn token its reservation minted.
|
||||
* A settled owner keeps the pane; another attempt's claim is superseded, because the newest
|
||||
* reservation is the one the record now names. Callers must already have refused a pane bound to
|
||||
* a different session.
|
||||
*/
|
||||
bindPtyForAttempt(ptyId: string, sessionId: string, attemptToken: string): boolean {
|
||||
const current = this.panesByPtyId.get(ptyId)
|
||||
if (current && current.attemptToken === null) {
|
||||
return false
|
||||
}
|
||||
this.panesByPtyId.set(ptyId, { sessionId, attemptToken })
|
||||
return true
|
||||
}
|
||||
|
||||
/** Promote this attempt's claim to a settled binding once its owner is proven. */
|
||||
settlePtyAttempt(ptyId: string, attemptToken: string): boolean {
|
||||
const current = this.panesByPtyId.get(ptyId)
|
||||
if (current?.attemptToken !== attemptToken) {
|
||||
return false
|
||||
}
|
||||
this.panesByPtyId.set(ptyId, { sessionId: current.sessionId, attemptToken: null })
|
||||
return true
|
||||
}
|
||||
|
||||
/**
|
||||
* Compare-and-clear: hand the pane back only while this attempt still holds it. A losing attempt
|
||||
* that unbinds by pty alone rips the pane out from under the attempt that superseded it, and then
|
||||
* that one's proof is refused too — both fail where one should have won.
|
||||
*/
|
||||
releasePtyAttempt(ptyId: string, attemptToken: string): boolean {
|
||||
if (this.panesByPtyId.get(ptyId)?.attemptToken !== attemptToken) {
|
||||
return false
|
||||
}
|
||||
this.panesByPtyId.delete(ptyId)
|
||||
return true
|
||||
}
|
||||
|
||||
boundSessionId(ptyId: string): string | null {
|
||||
return this.sessionIdByPtyId.get(ptyId) ?? null
|
||||
return this.panesByPtyId.get(ptyId)?.sessionId ?? null
|
||||
}
|
||||
|
||||
/** False while no PTY is bound, which is every write path in today's builds. */
|
||||
get enforcing(): boolean {
|
||||
return this.lookup !== null && this.sessionIdByPtyId.size > 0
|
||||
return this.lookup !== null && this.panesByPtyId.size > 0
|
||||
}
|
||||
|
||||
admit(ptyId: string): AgentSessionPtyWriteAdmission {
|
||||
@@ -117,11 +164,11 @@ export class AgentSessionPtyWriteGate {
|
||||
}
|
||||
|
||||
private binding(ptyId: string): AgentSessionPtyBinding | null {
|
||||
const sessionId = this.sessionIdByPtyId.get(ptyId)
|
||||
if (sessionId === undefined) {
|
||||
const pane = this.panesByPtyId.get(ptyId)
|
||||
if (pane === undefined) {
|
||||
return null
|
||||
}
|
||||
return { sessionId, record: this.lookup?.(sessionId) ?? null }
|
||||
return { sessionId: pane.sessionId, record: this.lookup?.(pane.sessionId) ?? null }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -252,6 +252,52 @@ describe('structured Codex adoption against the real PTY write gate', () => {
|
||||
})
|
||||
}, 30_000)
|
||||
|
||||
it('lets the attempt holding the reservation win when two adoptions overlap', async () => {
|
||||
// Both attempts park here: reservation taken and pane bound, proof not yet written.
|
||||
const arrived: string[] = []
|
||||
let releaseBoth: () => void = () => {}
|
||||
const bothArrived = new Promise<void>((resolve) => {
|
||||
releaseBoth = resolve
|
||||
})
|
||||
readStructuredTuiProcessIdentity.mockImplementation(
|
||||
async (input: { hostId: string; spawnToken: string }) => {
|
||||
if (!arrived.includes(input.spawnToken)) {
|
||||
arrived.push(input.spawnToken)
|
||||
if (arrived.length === 2) {
|
||||
releaseBoth()
|
||||
}
|
||||
await bothArrived
|
||||
}
|
||||
return {
|
||||
hostId: input.hostId,
|
||||
pid: 4242,
|
||||
processStartTimeMs: 1_700_000_000_000,
|
||||
spawnToken: input.spawnToken
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
const superseded = rig.runtime.adoptStructuredAgentSessionTerminal(
|
||||
{ ...adoptInput(), threadId: THREAD_ID },
|
||||
{ callerKey: 'renderer-1' }
|
||||
)
|
||||
superseded.catch(() => undefined)
|
||||
await vi.waitFor(() => expect(arrived).toHaveLength(1))
|
||||
// The second attempt carries the same sessionId and supersedes the first one's reservation.
|
||||
const winner = rig.runtime.adoptStructuredAgentSessionTerminal(
|
||||
{ ...adoptInput(), threadId: THREAD_ID },
|
||||
{ callerKey: 'renderer-1' }
|
||||
)
|
||||
|
||||
await expect(winner).resolves.toMatchObject({ ok: true })
|
||||
await expect(superseded).rejects.toThrow('could not verify its Codex session')
|
||||
// The loser's cleanup ran against a pane and a reservation that were no longer its own.
|
||||
expect(agentSessionPtyWriteGate.boundSessionId(PTY_ID)).toBe(SESSION_ID)
|
||||
expect(rig.store.getRecord(SESSION_ID)).toMatchObject({
|
||||
lease: { runtimeKind: 'tui', claimStatus: 'live', handoffStage: null }
|
||||
})
|
||||
}, 30_000)
|
||||
|
||||
it('refuses a pane another structured session already owns', async () => {
|
||||
agentSessionPtyWriteGate.bindPty(PTY_ID, 'someone-elses-session')
|
||||
|
||||
|
||||
@@ -215,6 +215,63 @@ describe('structured TUI launch tab binding', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('reports a reservation release that fails, without masking the proof error', async () => {
|
||||
const runtime = terminalAdoptionHarness()
|
||||
const host = structuredHostDouble({
|
||||
releaseAdoptedTuiReservation: vi.fn(async () => {
|
||||
throw new Error('store write failed')
|
||||
})
|
||||
})
|
||||
setStructuredAgentSessionHost(host as never)
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
proveCodexTuiRollout.mockRejectedValueOnce(
|
||||
new Error('The agent terminal did not prove the expected Codex rollout.')
|
||||
)
|
||||
readStructuredTuiProcessIdentity.mockResolvedValueOnce({
|
||||
hostId: 'local',
|
||||
pid: 4242,
|
||||
processStartTimeMs: 1_700_000_000_000,
|
||||
spawnToken: 'spawn-adopt'
|
||||
})
|
||||
|
||||
try {
|
||||
await expect(
|
||||
runtime.adoptStructuredAgentSessionTerminal(
|
||||
{
|
||||
envelope: {
|
||||
sessionId: 'session-release-failure',
|
||||
clientOperationId: 'operation-release-failure',
|
||||
expectedRuntimeFence: null,
|
||||
payloadFingerprint: 'f'.repeat(64)
|
||||
},
|
||||
worktree: `id:${WORKTREE_ID}`,
|
||||
tabId: 'tab-adopt',
|
||||
paneKey: 'tab-adopt:leaf-adopt',
|
||||
ptyId: 'pty-adopt',
|
||||
threadId: 'thread-adopt'
|
||||
},
|
||||
{ callerKey: 'renderer-1' }
|
||||
)
|
||||
).rejects.toThrow('did not prove the expected Codex rollout')
|
||||
|
||||
const reserved = host.reserveAdoptedTuiOwner.mock.calls[0]?.[0] as { spawnToken: string }
|
||||
const logged = warn.mock.calls.find(
|
||||
([message]) => message === '[native-chat] adopted TUI reservation release failed'
|
||||
)?.[1] as { error: Error } | undefined
|
||||
// A latched lease refuses every later attempt, so the cause cannot vanish with the throw.
|
||||
expect(logged).toMatchObject({
|
||||
sessionId: 'session-release-failure',
|
||||
fence: 1,
|
||||
spawnToken: reserved.spawnToken
|
||||
})
|
||||
expect(logged?.error.message).toBe('store write failed')
|
||||
} finally {
|
||||
warn.mockRestore()
|
||||
setStructuredAgentSessionHost(null)
|
||||
agentSessionPtyWriteGate.unbindPty('pty-adopt')
|
||||
}
|
||||
})
|
||||
|
||||
it('refuses a thread proof when the Codex process changes during it', async () => {
|
||||
const runtime = terminalAdoptionHarness()
|
||||
const host = structuredHostDouble()
|
||||
|
||||
@@ -11497,7 +11497,7 @@ export class OrcaRuntimeService {
|
||||
return reserved
|
||||
}
|
||||
const { fence, spawnToken } = reserved
|
||||
agentSessionPtyWriteGate.bindPty(input.ptyId, sessionId)
|
||||
agentSessionPtyWriteGate.bindPtyForAttempt(input.ptyId, sessionId, spawnToken)
|
||||
let owned = false
|
||||
try {
|
||||
return await this.attachAdoptedStructuredTui({
|
||||
@@ -11505,7 +11505,6 @@ export class OrcaRuntimeService {
|
||||
caller,
|
||||
host,
|
||||
intent,
|
||||
launchEnv,
|
||||
fence,
|
||||
spawnToken,
|
||||
rootProcessId,
|
||||
@@ -11515,15 +11514,27 @@ export class OrcaRuntimeService {
|
||||
}
|
||||
})
|
||||
} finally {
|
||||
if (!owned) {
|
||||
if (owned) {
|
||||
agentSessionPtyWriteGate.settlePtyAttempt(input.ptyId, spawnToken)
|
||||
} else {
|
||||
// A failed proof must leave nothing behind: an unreleased reservation is exactly the
|
||||
// latched lease that makes the next attempt refuse itself.
|
||||
if (priorBoundSessionId === null) {
|
||||
agentSessionPtyWriteGate.unbindPty(input.ptyId)
|
||||
}
|
||||
// latched lease that makes the next attempt refuse itself. Both releases are scoped to this
|
||||
// attempt's own spawn token, because a second adoption of the same pane carries the same
|
||||
// sessionId — clearing by pty alone unbinds the pane the attempt that superseded us is
|
||||
// proving on, and then neither can finish.
|
||||
agentSessionPtyWriteGate.releasePtyAttempt(input.ptyId, spawnToken)
|
||||
await host
|
||||
.releaseAdoptedTuiReservation({ sessionId, fence, spawnToken })
|
||||
.catch(() => undefined)
|
||||
.catch((error: unknown) => {
|
||||
// Swallowed on purpose: the proof error below is the one the user must see. But a
|
||||
// release that never persisted leaves the lease latched, so it cannot also be silent.
|
||||
console.warn('[native-chat] adopted TUI reservation release failed', {
|
||||
sessionId,
|
||||
fence,
|
||||
spawnToken,
|
||||
error
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -11533,14 +11544,13 @@ export class OrcaRuntimeService {
|
||||
caller: StructuredAgentSessionCaller
|
||||
host: NonNullable<ReturnType<typeof getStructuredAgentSessionHost>>
|
||||
intent: AgentSessionAttachParams
|
||||
launchEnv: Record<string, string>
|
||||
fence: number
|
||||
spawnToken: string
|
||||
rootProcessId: number
|
||||
pty: RuntimePtyWorktreeRecord
|
||||
onOwned: () => void
|
||||
}): Promise<AgentSessionMutationResult<AgentSessionAttachResult>> {
|
||||
const { input, caller, host, intent, launchEnv, fence, spawnToken, rootProcessId, pty } = args
|
||||
const { input, caller, host, intent, fence, spawnToken, rootProcessId, pty } = args
|
||||
const readPty = (): RuntimePtyWorktreeRecord => {
|
||||
const current = this.ptysById.get(input.ptyId)
|
||||
if (!current?.connected || current.paneKey !== input.paneKey) {
|
||||
@@ -11633,13 +11643,7 @@ export class OrcaRuntimeService {
|
||||
runtimeKind: 'tui',
|
||||
providerHandle: { kind: 'codex', threadId }
|
||||
}
|
||||
const adopted = await host.adoptTuiOwner({
|
||||
caller,
|
||||
params,
|
||||
owner,
|
||||
claimKeyId: this.agentSessionClaimSigner.keyId,
|
||||
launchEnv
|
||||
})
|
||||
const adopted = await host.adoptTuiOwner({ caller, params, owner })
|
||||
if (adopted.ok) {
|
||||
// The pane stays bound to the reservation this call already made; only the owner is new.
|
||||
this.adoptedStructuredTuiOwners.set(input.envelope.sessionId, owner)
|
||||
|
||||
@@ -259,7 +259,17 @@ export function launchAgentInNewTab(args: LaunchAgentInNewTabArgs): LaunchAgentI
|
||||
void (promptDeliveryCompletion ?? Promise.resolve())
|
||||
.catch(() => undefined)
|
||||
.then(async () => {
|
||||
await waitForAgentReady(tab.id, TUI_AGENT_CONFIG.codex.expectedProcess)
|
||||
const ready = await waitForAgentReady(tab.id, TUI_AGENT_CONFIG.codex.expectedProcess)
|
||||
if (!ready.ready) {
|
||||
// Why: chat adoption proves the pane's foreground process. Flipping a tab whose Codex is
|
||||
// not up yet trades a working terminal for an adoption error; the view toggle re-runs
|
||||
// adoption whenever the user wants it.
|
||||
console.warn('Codex was not ready for structured chat; staying in terminal view', {
|
||||
tabId: tab.id,
|
||||
reason: ready.reason
|
||||
})
|
||||
return
|
||||
}
|
||||
useAppStore.getState().setTabViewMode(tab.id, 'chat')
|
||||
})
|
||||
}
|
||||
|
||||
@@ -81,6 +81,21 @@ describe('structured chat adoption guard on the launch path', () => {
|
||||
await vi.waitFor(() => expect(mockSetTabViewMode).toHaveBeenCalledWith('tab-1', 'chat'))
|
||||
})
|
||||
|
||||
it('stays in terminal view when Codex never became ready', async () => {
|
||||
mockWaitForAgentReady.mockResolvedValue({ ready: false, reason: 'timeout' })
|
||||
const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab')
|
||||
|
||||
launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' })
|
||||
|
||||
await vi.waitFor(() => expect(mockWaitForAgentReady).toHaveBeenCalled())
|
||||
// Let every continuation the ready wait could have queued run before asserting it did not flip.
|
||||
await new Promise((resolve) => setTimeout(resolve, 0))
|
||||
await new Promise((resolve) => setTimeout(resolve, 0))
|
||||
// Chat adoption probes the pane's foreground process: flipping now hands the user an adoption
|
||||
// error in place of the terminal Codex is still starting in.
|
||||
expect(mockSetTabViewMode).not.toHaveBeenCalledWith('tab-1', 'chat')
|
||||
})
|
||||
|
||||
it('keeps an SSH Codex tab on the bridge', async () => {
|
||||
store.repos = [{ id: 'repo-1', connectionId: 'ssh-a', path: '/repo' }]
|
||||
const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab')
|
||||
|
||||
Reference in New Issue
Block a user