diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index cabbb9ac0..90b7bb45f 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1029,6 +1029,11 @@ class InMemoryOrchestrationMessages { private messages: MessageRow[] = [] + private runs = new Map< + string, + { id: string; coordinator_handle: string | null; coordinator_pane_key: string | null } + >() + insertMessage(msg: { from: string to: string @@ -1084,6 +1089,30 @@ class InMemoryOrchestrationMessages { return this.activeCoordinatorRun } + setRun(run: { + id: string + coordinator_handle: string | null + coordinator_pane_key?: string | null + }): void { + this.runs.set(run.id, { coordinator_pane_key: null, ...run }) + } + + getRun( + id: string + ): + | { id: string; coordinator_handle: string | null; coordinator_pane_key: string | null } + | undefined { + return this.runs.get(id) + } + + getCurrentRunForPane( + paneKey: string + ): + | { id: string; coordinator_handle: string | null; coordinator_pane_key: string | null } + | undefined { + return [...this.runs.values()].find((run) => run.coordinator_pane_key === paneKey) + } + markAsDelivered(ids: string[]): void { const deliveredIds = new Set(ids) for (const message of this.messages) { @@ -18468,23 +18497,24 @@ describe('OrcaRuntimeService', () => { runtime.deliverPendingMessagesForHandle(terminal.handle) - expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: hello')) - // Why: the split Enter write lands after the 500ms delay; advance past it before asserting on delivered_at. + expect(write).toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('You have 1 orchestration message') + ) await vi.advanceTimersByTimeAsync(500) expect(write).toHaveBeenCalledWith('pty-1', '\r') - // Why: design doc §3.2 splits delivered vs. read — push-on-idle stamps delivered_at but must not flip read; only the agent's check consumes, so rows stay unread. const unread = db.getUnreadMessages(terminal.handle) expect(unread).toHaveLength(1) expect(unread[0].read).toBe(0) - expect(unread[0].delivered_at).not.toBeNull() + expect(unread[0].delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() } }) - it('injects pending orchestration messages into the active coordinator without auto-submitting', async () => { + it('submits the mail pointer in an active coordinator pane', async () => { vi.useFakeTimers() try { const runtime = new OrcaRuntimeService(store) @@ -18512,18 +18542,18 @@ describe('OrcaRuntimeService', () => { expect(write).toHaveBeenCalledWith( 'pty-1', - expect.stringContaining('Subject: hello coordinator') + expect.stringContaining('You have 1 orchestration message') ) await vi.advanceTimersByTimeAsync(500) const submitWrites = write.mock.calls.filter( ([ptyId, text]) => ptyId === 'pty-1' && text === '\r' ) - expect(submitWrites).toHaveLength(0) + expect(submitWrites).toHaveLength(1) const unread = db.getUnreadMessages(terminal.handle) expect(unread).toHaveLength(1) expect(unread[0].read).toBe(0) - expect(unread[0].delivered_at).not.toBeNull() + expect(unread[0].delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() @@ -18551,18 +18581,20 @@ describe('OrcaRuntimeService', () => { runtime.deliverPendingMessagesForHandle(terminal.handle) - expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: hello cursor')) + expect(write).toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('You have 1 orchestration message') + ) await vi.advanceTimersByTimeAsync(500) const submitWrites = write.mock.calls.filter( ([ptyId, text]) => ptyId === 'pty-1' && text === '\r' ) expect(submitWrites).toHaveLength(0) - // Why: Cursor Agent treats injected PTY text as editable prompt input, so the user submits manually but the banner must not replay on the next idle transition. const unread = db.getUnreadMessages(terminal.handle) expect(unread).toHaveLength(1) expect(unread[0].read).toBe(0) - expect(unread[0].delivered_at).not.toBeNull() + expect(unread[0].delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() @@ -18591,7 +18623,10 @@ describe('OrcaRuntimeService', () => { runtime.deliverPendingMessagesForHandle(terminal.handle) await vi.advanceTimersByTimeAsync(500) - expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: hello claude')) + expect(write).toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('You have 1 orchestration message') + ) expect(write).toHaveBeenCalledWith('pty-1', '\r') db.close() } finally { @@ -18622,16 +18657,16 @@ describe('OrcaRuntimeService', () => { await vi.advanceTimersByTimeAsync(500) const firstInjections = write.mock.calls.filter( - (c) => typeof c[1] === 'string' && c[1].includes('Subject: hello') + (c) => typeof c[1] === 'string' && c[1].includes('orca orchestration check') ).length expect(firstInjections).toBe(1) - // Second idle transition: the row is unread but already delivered, so push-on-idle must skip it to avoid the replay bug. + // The row remains pending, so the in-memory sequence watermark prevents replay. runtime.deliverPendingMessagesForHandle(terminal.handle) await vi.advanceTimersByTimeAsync(500) const totalInjections = write.mock.calls.filter( - (c) => typeof c[1] === 'string' && c[1].includes('Subject: hello') + (c) => typeof c[1] === 'string' && c[1].includes('orca orchestration check') ).length expect(totalInjections).toBe(1) db.close() @@ -33284,16 +33319,124 @@ describe('OrcaRuntimeService', () => { // The push is deferred one microtask so it lands behind any resolved check. await Promise.resolve() - expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: after wait')) + expect(write).toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('You have 1 orchestration message') + ) + expect(write).not.toHaveBeenCalledWith('pty-1', expect.stringContaining('after wait')) await vi.advanceTimersByTimeAsync(500) expect(write).toHaveBeenCalledWith('pty-1', '\r') - expect(message.delivered_at).not.toBeNull() + expect(message.delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() } }) + it('points a Run mailbox at its live-idle coordinator without replaying pending rows', async () => { + vi.useFakeTimers() + try { + const runtime = new OrcaRuntimeService(store) + const db = new InMemoryOrchestrationMessages() + const write = vi.fn().mockReturnValue(true) + setInMemoryOrchestrationMessages(runtime, db) + runtime.setPtyController({ + write, + kill: vi.fn(), + getForegroundProcess: async () => null + }) + syncSinglePty(runtime) + + const [terminal] = (await runtime.listTerminals()).terminals + db.setRun({ + id: 'run_mailbox', + coordinator_handle: terminal.handle, + coordinator_pane_key: `${terminal.tabId}:${terminal.leafId}` + }) + runtime.onPtyData('pty-1', '\x1b]0;Codex working\x07', 100) + const message = db.insertMessage({ + from: 'term_worker', + to: 'run:run_mailbox', + subject: 'one P3 finding', + body: 'private worker report', + type: 'worker_done' + }) + + runtime.notifyMessageArrived('run:run_mailbox', 'worker_done') + await Promise.resolve() + expect(write).not.toHaveBeenCalled() + + runtime.onPtyData('pty-1', '\x1b]0;Codex done\x07', 101) + expect(write).toHaveBeenCalledWith( + 'pty-1', + '\nYou have 1 orchestration message. Run `orca orchestration check`.\n' + ) + expect(write).not.toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('private worker report') + ) + await vi.advanceTimersByTimeAsync(500) + expect(write).toHaveBeenCalledWith('pty-1', '\r') + expect(message.delivered_at).toBeNull() + + runtime.notifyMessageArrived('run:run_mailbox', 'worker_done') + await Promise.resolve() + await vi.advanceTimersByTimeAsync(500) + expect( + write.mock.calls.filter( + ([, payload]) => + typeof payload === 'string' && payload.includes('orca orchestration check') + ) + ).toHaveLength(1) + db.close() + } finally { + vi.useRealTimers() + } + }) + + it('points already-idle Run mail after Codex replaces its completion title', async () => { + const runtime = new OrcaRuntimeService(store) + const db = new InMemoryOrchestrationMessages() + const write = vi.fn().mockReturnValue(true) + setInMemoryOrchestrationMessages(runtime, db) + runtime.setPtyController({ + write, + kill: vi.fn(), + getForegroundProcess: async () => 'codex' + }) + syncSinglePty(runtime) + + const [terminal] = (await runtime.listTerminals()).terminals + db.setRun({ + id: 'run_codex_native_title', + coordinator_handle: terminal.handle, + coordinator_pane_key: `${terminal.tabId}:${terminal.leafId}` + }) + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + runtime.onPtyData('pty-1', '\x1b]0;fix-12953-orchestration-mail-pointer\x07', 101) + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + db.insertMessage({ + from: 'term_worker', + to: 'run:run_codex_native_title', + subject: 'real-agent smoke complete', + body: 'The package name is orca.', + type: 'worker_done' + }) + + runtime.notifyMessageArrived('run:run_codex_native_title', 'worker_done') + await Promise.resolve() + + await vi.waitFor(() => { + expect(write).toHaveBeenCalledWith( + 'pty-1', + '\nYou have 1 orchestration message. Run `orca orchestration check`.\n' + ) + }) + db.close() + }) + it('does not inject pending mail on notify when the recipient is still working', async () => { const runtime = new OrcaRuntimeService(store) const db = new InMemoryOrchestrationMessages() @@ -33355,9 +33498,12 @@ describe('OrcaRuntimeService', () => { // is no transition — only the liveness edge can release the row (#12536). runtime.onPtyData('pty-1', '\x1b]0;Codex done\x07', 100) - expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: restored idle')) + expect(write).toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('You have 1 orchestration message') + ) await vi.advanceTimersByTimeAsync(600) - expect(message.delivered_at).not.toBeNull() + expect(message.delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() @@ -33399,9 +33545,12 @@ describe('OrcaRuntimeService', () => { // The first live idle frame authorizes it and the row still delivers. runtime.onPtyData('pty-1', '\x1b]0;Codex working\x07', 100) runtime.onPtyData('pty-1', '\x1b]0;Codex done\x07', 101) - expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: seeded idle')) + expect(write).toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('You have 1 orchestration message') + ) await vi.advanceTimersByTimeAsync(600) - expect(message.delivered_at).not.toBeNull() + expect(message.delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() @@ -33503,9 +33652,11 @@ describe('OrcaRuntimeService', () => { const payloads = write.mock.calls .map(([, data]) => data) .filter((data): data is string => typeof data === 'string') - expect(payloads.some((data) => data.includes('Subject: unclaimed status'))).toBe(true) - expect(payloads.some((data) => data.includes('Subject: reserved completion'))).toBe(false) - expect(status.delivered_at).not.toBeNull() + expect(payloads).toContain( + '\nYou have 1 orchestration message. Run `orca orchestration check`.\n' + ) + expect(payloads.some((data) => data.includes('reserved completion'))).toBe(false) + expect(status.delivered_at).toBeNull() expect(done.delivered_at).toBeNull() db.close() } finally { @@ -33684,10 +33835,10 @@ describe('OrcaRuntimeService', () => { expect(write).toHaveBeenCalledWith( 'pty-1', - expect.stringContaining('Subject: after republish') + expect.stringContaining('You have 1 orchestration message') ) await vi.advanceTimersByTimeAsync(600) - expect(message.delivered_at).not.toBeNull() + expect(message.delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() @@ -33738,10 +33889,10 @@ describe('OrcaRuntimeService', () => { runtime.onPtyData('pty-1', '\x1b]0;Codex done\x07', 200) expect(write).toHaveBeenCalledWith( 'pty-1', - expect.stringContaining('Subject: after same id respawn') + expect.stringContaining('You have 1 orchestration message') ) await vi.advanceTimersByTimeAsync(600) - expect(message.delivered_at).not.toBeNull() + expect(message.delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() @@ -33787,10 +33938,10 @@ describe('OrcaRuntimeService', () => { await Promise.resolve() expect(write).toHaveBeenCalledWith( 'pty-1', - expect.stringContaining('Subject: unfiltered status') + expect.stringContaining('You have 1 orchestration message') ) await vi.advanceTimersByTimeAsync(600) - expect(message.delivered_at).not.toBeNull() + expect(message.delivered_at).toBeNull() // The filtered waiter stays blocked; the push did not consume its wake. await vi.advanceTimersByTimeAsync(5_000) @@ -33865,10 +34016,10 @@ describe('OrcaRuntimeService', () => { runtime.notifyMessageArrived(terminal.handle, 'status') await Promise.resolve() - const payloadWrites = write.mock.calls.filter( - ([, payload]) => typeof payload === 'string' && payload.includes('Subject: once only') + const pointerWrites = write.mock.calls.filter( + ([, payload]) => typeof payload === 'string' && payload.includes('orca orchestration check') ) - expect(payloadWrites).toHaveLength(1) + expect(pointerWrites).toHaveLength(1) await vi.advanceTimersByTimeAsync(500) const enterWrites = write.mock.calls.filter(([, payload]) => payload === '\r') @@ -33910,9 +34061,10 @@ describe('OrcaRuntimeService', () => { await Promise.resolve() expect( write.mock.calls.filter( - ([, payload]) => typeof payload === 'string' && payload.includes('Subject: second') + ([, payload]) => + typeof payload === 'string' && payload.includes('orca orchestration check') ) - ).toHaveLength(0) + ).toHaveLength(1) expect(second.delivered_at).toBeNull() // Why: release must not require another agent-status OSC — only the @@ -33920,13 +34072,18 @@ describe('OrcaRuntimeService', () => { // timer-only settle (CodeRabbit settling-timeout gap, #12584). await vi.advanceTimersByTimeAsync(3_000) expect(write).toHaveBeenCalledWith('pty-1', '\r') - expect(first.delivered_at).not.toBeNull() + expect(first.delivered_at).toBeNull() expect( write.mock.calls.filter( - ([, payload]) => typeof payload === 'string' && payload.includes('Subject: second') + ([, payload]) => + typeof payload === 'string' && payload.includes('orca orchestration check') ) - ).toHaveLength(1) - expect(second.delivered_at).not.toBeNull() + ).toHaveLength(2) + expect(write).toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('You have 2 orchestration messages') + ) + expect(second.delivered_at).toBeNull() db.close() } finally { vi.useRealTimers() @@ -33953,7 +34110,10 @@ describe('OrcaRuntimeService', () => { runtime.deliverPendingMessagesForHandle(terminal.handle) - expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: after wait')) + expect(write).toHaveBeenCalledWith( + 'pty-1', + expect.stringContaining('You have 1 orchestration message') + ) db.close() }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index ea146b523..7983ebacb 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -134,7 +134,7 @@ import type { OrchestrationWorkerServer } from './orchestration/environment-transport' import { syncFederatedDispatch } from './orchestration/federation-sync' -import { formatMessagesForInjection } from './orchestration/formatter' +import { formatMessagePointer } from './orchestration/formatter' import { selectExactWorkerProviderSession } from './orchestration/worker-provider-session' import type { Automation, @@ -2878,6 +2878,8 @@ export class OrcaRuntimeService { private handles = new Map() private handleByLeafKey = new Map() private handleByPtyId = new Map() + // Why: pointer state is process-local; one harmless replay after restart avoids a wire or schema change. + private readonly lastPointedMessageSequenceByHandle = new Map() private syntheticTerminalHandles = new Set() private detachedPreAllocatedLeaves = new Map() private graphSyncCallbacks: (() => void)[] = [] @@ -10318,7 +10320,7 @@ export class OrcaRuntimeService { // prompt) then shows no transition — the row would strand, which is // exactly #12536. Waiter semantics stay transition-only above. if (agentStatus === 'idle' && (prevStatus !== 'idle' || !prevObservedLive)) { - this.deliverPendingMessages(leaf) + this.deliverPendingMessagesForLeaf(leaf) } } return ptyRecordChanged @@ -16793,7 +16795,22 @@ export class OrcaRuntimeService { result.available && recognizeAgentProcess(result.process) !== null ) { - this.ptyTitleTrackersByPtyId.get(ptyId)?.tracker.restoreLastAgentExit() + const restoredStatus = this.ptyTitleTrackersByPtyId + .get(ptyId) + ?.tracker.restoreLastAgentExit() + if (restoredStatus !== null && restoredStatus !== undefined) { + current.lastAgentStatus = restoredStatus + for (const leaf of this.getLeavesForPty(ptyId)) { + if (leaf.lastAgentStatus !== null) { + continue + } + // Why: the foreground agent disproved the neutral title's exit signal; keep runtime delivery state aligned with the restored tracker. + leaf.lastAgentStatus = restoredStatus + if (restoredStatus === 'idle') { + this.deliverPendingMessagesForLeaf(leaf) + } + } + } return } this.recordTerminalSideEffectFact(ptyId, { kind: 'agent-exited' }) @@ -31118,25 +31135,40 @@ export class OrcaRuntimeService { } deliverPendingMessagesForHandle(handle: string, reservedTypes?: ReadonlySet): void { - // Why before the try: `dispatch:`/`run:` mailbox addresses are never terminal - // handles, and federation sync notifies once per relayed item — letting each - // one build and discard a `terminal_handle_stale` Error (stack capture) is - // pure waste. getLiveLeafForHandle would reject them on the same lookup. - if (!this.handles.has(handle)) { - return + let terminalHandle = handle + if (!this.handles.has(terminalHandle)) { + const runId = handle.startsWith('run:') ? handle.slice('run:'.length) : '' + const coordinatorHandle = runId + ? this._orchestrationDb?.getRun(runId)?.coordinator_handle + : null + if (!coordinatorHandle || !this.handles.has(coordinatorHandle)) { + return + } + terminalHandle = coordinatorHandle } try { - const { leaf } = this.getLiveLeafForHandle(handle) + const { leaf } = this.getLiveLeafForHandle(terminalHandle) // Why lastAgentStatusObservedLive: a cold restore seeds `idle` from the // title persisted at snapshot time, so an agent that went busy across the // relaunch still reads idle until its first live frame. Pushing on that - // would type a message plus Enter into a working agent and stamp the row - // delivered. Seeded state waits for a live observation to authorize it. + // would type a message plus Enter into a working agent. Seeded state waits + // for a live observation to authorize it. if (leaf.lastAgentStatus === 'idle' && leaf.lastAgentStatusObservedLive) { - this.deliverPendingMessages(leaf, false, reservedTypes) + this.deliverPendingMessages(leaf, { mailboxHandle: handle, reservedTypes }) } } catch { - // Unknown/stale handles can't be pushed now; the persisted message stays available via explicit check or future idle delivery. + // Unknown/stale handles can't be pointed now; the persisted message stays available via explicit check or future idle delivery. + } + } + + private deliverPendingMessagesForLeaf(leaf: RuntimeLeafRecord): void { + this.deliverPendingMessages(leaf) + if (!this._orchestrationDb) { + return + } + const run = this._orchestrationDb.getCurrentRunForPane?.(`${leaf.tabId}:${leaf.leafId}`) + if (run) { + this.deliverPendingMessages(leaf, { mailboxHandle: `run:${run.id}` }) } } @@ -31147,10 +31179,8 @@ export class OrcaRuntimeService { // deliver now (#12536). deliverPendingMessagesForHandle no-ops when the // leaf is not idle. Main's messageDeliveryFlights serialize mid-Enter // re-notifies without a separate settle barrier. - // Why skip when a waiter will consume this: deliverPendingMessages stamps - // delivered_at but not read, so a blocked orchestration.check --wait would - // still re-read the row and the pane would also receive it (double - // delivery). The pull wins; the push stays pending for a later notify. + // Why skip when a waiter will consume this: a blocked check owns the row, + // so also pointing and submitting the pane would wake it twice. The pull wins. // Why "will consume" and not "exists": a waiter filtered to other types // never returns this row — check re-reads under the same filter on timeout // — so treating it as the consumer strands the message in exactly the @@ -31767,10 +31797,8 @@ export class OrcaRuntimeService { return null } - // Why: delivered_at for Claude targets stamps only in the delayed-Enter - // callback, so the whole write→settle span must be single-flight per pty — - // a second read inside it would re-inject the same unread rows. Triggers - // landing mid-flight park the latest leaf and re-run once on settle. The + // Why: the whole pointer→Enter span must be single-flight per pty. Triggers + // landing mid-flight park their mailbox and re-run once on settle. The // flight object is the settle identity: a stale settle surviving an exit // retire must not clear a newer same-id flight or flush its parked trigger. private readonly messageDeliveryFlightsByPtyId = new Map< @@ -31778,7 +31806,10 @@ export class OrcaRuntimeService { { enterTimer: ReturnType | null } >() - private readonly parkedMessageRedeliveryLeavesByPtyId = new Map() + private readonly parkedMessageRedeliveriesByPtyId = new Map< + string, + Map }> + >() private settlePendingMessageDelivery( ptyId: string, @@ -31788,31 +31819,47 @@ export class OrcaRuntimeService { return } this.messageDeliveryFlightsByPtyId.delete(ptyId) - const parkedLeaf = this.parkedMessageRedeliveryLeavesByPtyId.get(ptyId) - if (!parkedLeaf) { + const parked = this.parkedMessageRedeliveriesByPtyId.get(ptyId) + if (!parked) { return } - this.parkedMessageRedeliveryLeavesByPtyId.delete(ptyId) - this.deliverPendingMessages(parkedLeaf) + this.parkedMessageRedeliveriesByPtyId.delete(ptyId) + for (const [mailboxHandle, delivery] of parked) { + this.deliverPendingMessages(delivery.leaf, { + mailboxHandle, + reservedTypes: delivery.reservedTypes + }) + } } - // Why: an Enter armed for a dead session must not fire into a same-id cold - // restore — it would inject \r and stamp rows the replacement never saw. - // Retire without stamping; the rows re-deliver on the replacement's next idle. + // Why: a dead session's Enter or watermark must not affect a same-id cold restore. private retirePendingMessageDeliveryForPty(ptyId: string): void { const flight = this.messageDeliveryFlightsByPtyId.get(ptyId) if (flight?.enterTimer != null) { clearTimeout(flight.enterTimer) } this.messageDeliveryFlightsByPtyId.delete(ptyId) - this.parkedMessageRedeliveryLeavesByPtyId.delete(ptyId) + this.parkedMessageRedeliveriesByPtyId.delete(ptyId) + for (const leaf of this.getLeavesForPty(ptyId)) { + const handle = this.handleByLeafKey.get(this.getLeafKey(leaf.tabId, leaf.leafId)) + if (handle) { + this.lastPointedMessageSequenceByHandle.delete(handle) + } + const run = this._orchestrationDb?.getCurrentRunForPane?.(`${leaf.tabId}:${leaf.leafId}`) + if (run) { + this.lastPointedMessageSequenceByHandle.delete(`run:${run.id}`) + } + } } // Why: push-on-idle delivery is event-driven (no polling) because the runtime owns both the message store and terminal status detection. private deliverPendingMessages( leaf: RuntimeLeafRecord, - skipAbsenceProbe = false, - reservedTypes?: ReadonlySet + options: { + mailboxHandle?: string + reservedTypes?: ReadonlySet + skipAbsenceProbe?: boolean + } = {} ): void { if (!this._orchestrationDb) { return @@ -31822,10 +31869,20 @@ export class OrcaRuntimeService { if (!handle) { return } + const mailboxHandle = options.mailboxHandle ?? handle - // Why before reading rows: rows read mid-flight are the not-yet-stamped ones. if (leaf.ptyId && this.messageDeliveryFlightsByPtyId.has(leaf.ptyId)) { - this.parkedMessageRedeliveryLeavesByPtyId.set(leaf.ptyId, leaf) + let parked = this.parkedMessageRedeliveriesByPtyId.get(leaf.ptyId) + if (!parked) { + parked = new Map() + this.parkedMessageRedeliveriesByPtyId.set(leaf.ptyId, parked) + } + const priorReservedTypes = parked.get(mailboxHandle)?.reservedTypes + const reservedTypes = + priorReservedTypes || options.reservedTypes + ? new Set([...(priorReservedTypes ?? []), ...(options.reservedTypes ?? [])]) + : undefined + parked.set(mailboxHandle, { leaf, reservedTypes }) return } @@ -31834,12 +31891,13 @@ export class OrcaRuntimeService { // into the pane AND returned by that pull's check. Live waiters cover the // still-blocked case; reservedTypes carries the notify-time snapshot for a // waiter resolved later in the same drain, which is already gone from the map. - const waiters = this.messageWaitersByHandle.get(handle) + const waiters = this.messageWaitersByHandle.get(mailboxHandle) const unread = this._orchestrationDb - .getUndeliveredUnreadMessages(handle) + .getUndeliveredUnreadMessages(mailboxHandle) .filter( (message) => - !reservedTypes?.has(message.type) && !messageTypeHasLiveWaiter(waiters, message.type) + !options.reservedTypes?.has(message.type) && + !messageTypeHasLiveWaiter(waiters, message.type) ) if (unread.length === 0) { return @@ -31848,9 +31906,16 @@ export class OrcaRuntimeService { if (!leaf.writable || !leaf.ptyId) { return } + const newestSequence = unread.at(-1)?.sequence + if ( + newestSequence === undefined || + newestSequence <= (this.lastPointedMessageSequenceByHandle.get(mailboxHandle) ?? -1) + ) { + return + } if ( - !skipAbsenceProbe && + !options.skipAbsenceProbe && this.ptyController?.probePtyLiveness && !this.controllerKnowsPtyIsLive(leaf.ptyId) ) { @@ -31859,8 +31924,7 @@ export class OrcaRuntimeService { // them queued for a future surface; unknown liveness still delivers. const probedPtyId = leaf.ptyId // Why: triggers arriving mid-probe must not each arm a continuation — the - // Claude Enter delay stamps delivered_at late, so every continuation would - // re-read the same unread rows and double-deliver. The single armed + // Every continuation would re-read the same unread rows. The single armed // continuation re-reads fresh rows when it fires, so nothing is lost. if (this.probeDeferredDeliveryPtyIds.has(probedPtyId)) { return @@ -31882,16 +31946,18 @@ export class OrcaRuntimeService { // Why current state, not the closure: the gate that authorized this // push ran before the probe. A same-id cold restore inside the probe // window keeps ptyId identical and makes the leaf writable again, so - // an id-only check would type the payload plus Enter into a process - // whose idle was never observed — and stamp the row delivered, which - // loses it. Re-read the leaf and re-apply the live-idle gate. + // an id-only check would type the pointer plus Enter into a process + // whose idle was never observed. Re-read the live-idle gate. const currentLeaf = this.leaves.get(this.getLeafKey(leaf.tabId, leaf.leafId)) if ( currentLeaf?.ptyId === probedPtyId && currentLeaf.lastAgentStatus === 'idle' && currentLeaf.lastAgentStatusObservedLive ) { - this.deliverPendingMessages(currentLeaf, true) + this.deliverPendingMessages(currentLeaf, { + mailboxHandle, + skipAbsenceProbe: true + }) } }, 0) } @@ -31905,32 +31971,25 @@ export class OrcaRuntimeService { const deliveryPtyId = leaf.ptyId const flight: { enterTimer: ReturnType | null } = { enterTimer: null } this.messageDeliveryFlightsByPtyId.set(deliveryPtyId, flight) - // Why: every sync outcome — failed write, sync-stamped branch, or a throw — + // Why: every sync outcome — failed write, Cursor branch, or a throw — // must end the flight here, or a leaked flag parks this pty's deliveries // forever. Only an armed Enter hands settling to its own callback. let settlesInEnterCallback = false try { - const payload = formatMessagesForInjection(unread) + const payload = formatMessagePointer(unread.length) const wrote = this.ptyController?.write(deliveryPtyId, payload) ?? false if (!wrote) { return } - - // The active coordinator prompt is user-owned input, so push-on-idle must not synthesize Enter. - if (this._orchestrationDb.getActiveCoordinatorRun()?.coordinator_handle === handle) { - this._orchestrationDb.markAsDelivered(unread.map((m) => m.id)) - return - } + this.lastPointedMessageSequenceByHandle.set(mailboxHandle, newestSequence) const tabTitle = this.tabs.get(leaf.tabId)?.title if (isCursorAgentOrchestrationTarget(leaf, tabTitle)) { // Why: Cursor Agent treats injected PTY text as editable prompt input, so submitting must stay under user control. - this._orchestrationDb.markAsDelivered(unread.map((m) => m.id)) return } - // Why: Claude Code treats a large PTY write as a paste and swallows a \r in the same write; send Enter separately after a delay, stamping delivered_at only once \r is confirmed. - // Important (design doc §3.2, feedback #2): stamp delivered_at, not read — read means "a check-caller consumed this"; flipping it would hide the message from check --unread. + // Why: agent TUIs can swallow a \r in the same PTY write; submit separately after a delay. flight.enterTimer = setTimeout(() => { try { // Why current state, not the closure: graph resync replaces leaf @@ -31943,12 +32002,9 @@ export class OrcaRuntimeService { if (!currentLeaf || currentLeaf.ptyId !== deliveryPtyId || !currentLeaf.writable) { return } - const submitted = this.ptyController?.write(deliveryPtyId, '\r') ?? false - if (submitted) { - this._orchestrationDb?.markAsDelivered(unread.map((m) => m.id)) - } + this.ptyController?.write(deliveryPtyId, '\r') } catch { - // Terminal may have closed during the delay — messages stay queued (delivered_at NULL) and re-deliver on next idle. + // Terminal may have closed during the delay; mail remains queued for check. } finally { // Why finally: every outcome — submit, refusal, throw — ends the flight, // and settle re-runs any trigger parked during it so nothing strands. diff --git a/src/main/runtime/orchestration/formatter.test.ts b/src/main/runtime/orchestration/formatter.test.ts index b4732f04a..f7d9c14d8 100644 --- a/src/main/runtime/orchestration/formatter.test.ts +++ b/src/main/runtime/orchestration/formatter.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { formatMessageBanner, formatMessagesForInjection } from './formatter' +import { formatMessageBanner, formatMessagePointer, formatMessagesForInjection } from './formatter' import type { MessageRow } from './types' function makeMessage(overrides: Partial = {}): MessageRow { @@ -183,3 +183,15 @@ describe('formatMessagesForInjection', () => { expect(result).toContain(`${bannerA}\n\n${bannerB}`) }) }) + +describe('formatMessagePointer', () => { + it('formats a singular pointer without message content', () => { + expect(formatMessagePointer(1)).toBe( + '\nYou have 1 orchestration message. Run `orca orchestration check`.\n' + ) + }) + + it('pluralizes a batched pointer', () => { + expect(formatMessagePointer(3)).toContain('3 orchestration messages') + }) +}) diff --git a/src/main/runtime/orchestration/formatter.ts b/src/main/runtime/orchestration/formatter.ts index 269ba2930..678c5e084 100644 --- a/src/main/runtime/orchestration/formatter.ts +++ b/src/main/runtime/orchestration/formatter.ts @@ -107,3 +107,8 @@ export function formatMessagesForInjection(messages: MessageRow[]): string { const banners = messages.map(formatMessageBanner).join('\n\n') return `\n--- Orchestration Messages (${messages.length}) ---\n${banners}\n---\n` } + +export function formatMessagePointer(count: number): string { + const noun = count === 1 ? 'message' : 'messages' + return `\nYou have ${count} orchestration ${noun}. Run \`orca orchestration check\`.\n` +} diff --git a/src/main/runtime/terminal-send-stale-leaf-liveness.test.ts b/src/main/runtime/terminal-send-stale-leaf-liveness.test.ts index 823c04e5a..41e272ecd 100644 --- a/src/main/runtime/terminal-send-stale-leaf-liveness.test.ts +++ b/src/main/runtime/terminal-send-stale-leaf-liveness.test.ts @@ -267,6 +267,7 @@ function makeOrchestrationDbStub(toHandle: () => string) { getUndeliveredUnreadMessages: (handle: string) => rows.filter((row) => row.to_handle === handle && row.read === 0 && !row.delivered_at), getActiveCoordinatorRun: () => null, + getCurrentRunForPane: () => undefined, // Consulted by onPtyExit's dispatch-failure path. getActiveDispatchForTerminal: () => null, markAsDelivered, @@ -327,7 +328,7 @@ describe('push-on-idle orchestration delivery absence gate', () => { expect(write).toHaveBeenCalledWith( STALE_PTY_ID, - expect.stringContaining('Subject: for the old session') + expect.stringContaining('You have 1 orchestration message') ) }) @@ -369,7 +370,10 @@ describe('push-on-idle orchestration delivery absence gate', () => { const payloads = write.mock.calls .map(([, data]) => data) .filter((data): data is string => typeof data === 'string') - expect(payloads.some((data) => data.includes('Subject: unclaimed status'))).toBe(true) + const pointers = payloads.filter((data) => data.includes('orca orchestration check')) + expect(pointers).toHaveLength(1) + expect(pointers[0]).toContain('You have 1 orchestration message') + expect(payloads.some((data) => data.includes('unclaimed status'))).toBe(false) expect(payloads.some((data) => data.includes('Subject: worker completion'))).toBe(false) }) @@ -411,8 +415,11 @@ describe('push-on-idle orchestration delivery absence gate', () => { const payloads = write.mock.calls .map(([, data]) => data) .filter((data): data is string => typeof data === 'string') - expect(payloads.some((data) => data.includes('Subject: unclaimed status'))).toBe(true) - expect(payloads.some((data) => data.includes('Subject: late completion'))).toBe(true) + const pointers = payloads.filter((data) => data.includes('orca orchestration check')) + expect(pointers).toHaveLength(1) + expect(pointers[0]).toContain('You have 2 orchestration messages') + expect(payloads.some((data) => data.includes('unclaimed status'))).toBe(false) + expect(payloads.some((data) => data.includes('late completion'))).toBe(false) }) it('keeps messages queued instead of marking a proven-absent pty delivered', async () => { @@ -440,12 +447,14 @@ describe('push-on-idle orchestration delivery absence gate', () => { // Why twice: the probe continuation yields a turn before delivering. await new Promise((resolve) => setTimeout(resolve, 0)) - expect(write).toHaveBeenCalledWith(STALE_PTY_ID, expect.stringContaining('Subject: hello')) + expect(write).toHaveBeenCalledWith( + STALE_PTY_ID, + expect.stringContaining('You have 1 orchestration message') + ) }) - // Why: delivered_at stamps only in the delayed-Enter callback, so the whole - // write→settle span — not just the probe — must be single-flight; a trigger - // landing inside the 500ms window would re-read the same un-stamped rows. + // Why: the whole pointer→Enter span must be single-flight; a trigger inside + // the 500ms window must park until the sequence watermark advances. it('delivers once across concurrent probe triggers and an in-window re-trigger, then flushes parked rows', async () => { vi.useFakeTimers() try { @@ -464,36 +473,33 @@ describe('push-on-idle orchestration delivery absence gate', () => { resolveProbe(null) await vi.advanceTimersByTimeAsync(0) - const firstSubjectWrites = () => + const pointerWrites = () => write.mock.calls.filter( - ([, data]) => typeof data === 'string' && data.includes('Subject: exactly once') + ([, data]) => typeof data === 'string' && data.includes('orca orchestration check') ) - expect(firstSubjectWrites()).toHaveLength(1) + expect(pointerWrites()).toHaveLength(1) + expect(pointerWrites()[0]?.[1]).toContain('You have 1 orchestration message') - // Re-trigger INSIDE the 500ms Enter window: the first batch is written but - // not yet stamped, so a fresh probe cycle would re-inject it. (On the - // fixed code no new probe is armed — the trigger parks; resolveProbe then - // re-resolves the settled first probe, a no-op.) + // Re-trigger inside the Enter window parks; resolving the settled first + // probe again is a no-op. stub.insert('second message') runtime.deliverPendingMessagesForHandle(handle) resolveProbe(null) await vi.advanceTimersByTimeAsync(0) - expect(firstSubjectWrites()).toHaveLength(1) + expect(pointerWrites()).toHaveLength(1) - // Enter fires, delivered_at stamps, the flight settles, and the parked - // trigger re-runs on its own — arming a fresh probe for the new row. + // Enter settles the flight and re-runs the parked trigger, arming a fresh + // probe for the newer sequence. await vi.advanceTimersByTimeAsync(500) resolveProbe(null) await vi.advanceTimersByTimeAsync(0) - const secondSubjectWrites = write.mock.calls.filter( - ([, data]) => typeof data === 'string' && data.includes('Subject: second message') - ) - expect(secondSubjectWrites).toHaveLength(1) - expect(firstSubjectWrites()).toHaveLength(1) + expect(pointerWrites()).toHaveLength(2) + expect(pointerWrites()[1]?.[1]).toContain('You have 2 orchestration messages') await vi.advanceTimersByTimeAsync(500) - expect(stub.rows.every((row) => row.delivered_at !== null)).toBe(true) + expect(stub.markAsDelivered).not.toHaveBeenCalled() + expect(stub.rows.every((row) => row.delivered_at === null)).toBe(true) } finally { vi.useRealTimers() } @@ -514,35 +520,30 @@ describe('push-on-idle orchestration delivery absence gate', () => { stub.insert('second') runtime.deliverPendingMessagesForHandle(handle) - const firstSubjectWrites = () => + const pointerWrites = () => write.mock.calls.filter( - ([, data]) => typeof data === 'string' && data.includes('Subject: first') + ([, data]) => typeof data === 'string' && data.includes('orca orchestration check') ) - expect(firstSubjectWrites()).toHaveLength(1) + expect(pointerWrites()).toHaveLength(1) + expect(pointerWrites()[0]?.[1]).toContain('You have 1 orchestration message') expect(probe).not.toHaveBeenCalled() - // Settle flushes the parked trigger; the second row delivers alone — - // its batch must not re-contain the already-stamped first row. + // Settle flushes the parked trigger; both still-pending rows are counted, + // while the newer sequence authorizes exactly one fresh pointer. await vi.advanceTimersByTimeAsync(500) - const secondOnlyWrites = write.mock.calls.filter( - ([, data]) => - typeof data === 'string' && - data.includes('Subject: second') && - !data.includes('Subject: first') - ) - expect(secondOnlyWrites).toHaveLength(1) - expect(firstSubjectWrites()).toHaveLength(1) + expect(pointerWrites()).toHaveLength(2) + expect(pointerWrites()[1]?.[1]).toContain('You have 2 orchestration messages') await vi.advanceTimersByTimeAsync(500) - expect(stub.rows.every((row) => row.delivered_at !== null)).toBe(true) + expect(stub.markAsDelivered).not.toHaveBeenCalled() + expect(stub.rows.every((row) => row.delivered_at === null)).toBe(true) } finally { vi.useRealTimers() } }) // Why: cold restore respawns under the SAME session id. An Enter armed for - // the dead incarnation must not fire into the replacement — it would inject - // \r and stamp rows the new session never received. + // the dead incarnation must not submit stale input into the replacement. it('retires an armed Enter when the pty exits and respawns under the same id inside the window', async () => { vi.useFakeTimers() try { @@ -569,18 +570,19 @@ describe('push-on-idle orchestration delivery absence gate', () => { runtime.deliverPendingMessagesForHandle(handle) expect( write.mock.calls.filter( - ([, data]) => typeof data === 'string' && data.includes('Subject: for the old session') + ([, data]) => typeof data === 'string' && data.includes('orca orchestration check') ) ).toHaveLength(1) runtime.onPtyData(STALE_PTY_ID, '\x1b]0;Codex working\x07', 200) runtime.onPtyData(STALE_PTY_ID, '\x1b]0;Codex done\x07', 201) const payloadWrites = write.mock.calls.filter( - ([, data]) => typeof data === 'string' && data.includes('Subject: for the old session') + ([, data]) => typeof data === 'string' && data.includes('orca orchestration check') ) expect(payloadWrites).toHaveLength(2) await vi.advanceTimersByTimeAsync(500) expect(write.mock.calls.filter(([, data]) => data === '\r')).toHaveLength(1) - expect(stub.rows[0].delivered_at).not.toBeNull() + expect(stub.markAsDelivered).not.toHaveBeenCalled() + expect(stub.rows[0].delivered_at).toBeNull() } finally { vi.useRealTimers() } @@ -595,18 +597,21 @@ describe('push-on-idle orchestration delivery absence gate', () => { }) const internals = runtime as unknown as { messageDeliveryFlightsByPtyId: Map - parkedMessageRedeliveryLeavesByPtyId: Map + parkedMessageRedeliveriesByPtyId: Map + lastPointedMessageSequenceByHandle: Map } stub.insert('first') runtime.deliverPendingMessagesForHandle(handle) stub.insert('second') runtime.deliverPendingMessagesForHandle(handle) expect(internals.messageDeliveryFlightsByPtyId.size).toBe(1) - expect(internals.parkedMessageRedeliveryLeavesByPtyId.size).toBe(1) + expect(internals.parkedMessageRedeliveriesByPtyId.size).toBe(1) + expect(internals.lastPointedMessageSequenceByHandle.size).toBe(1) runtime.onPtyExit(STALE_PTY_ID, 0) expect(internals.messageDeliveryFlightsByPtyId.size).toBe(0) - expect(internals.parkedMessageRedeliveryLeavesByPtyId.size).toBe(0) + expect(internals.parkedMessageRedeliveriesByPtyId.size).toBe(0) + expect(internals.lastPointedMessageSequenceByHandle.size).toBe(0) await vi.advanceTimersByTimeAsync(500) expect(write.mock.calls.filter(([, data]) => data === '\r')).toHaveLength(0) diff --git a/src/shared/agent-title-status.ts b/src/shared/agent-title-status.ts index 68c9dd463..09ffc07a6 100644 --- a/src/shared/agent-title-status.ts +++ b/src/shared/agent-title-status.ts @@ -56,7 +56,7 @@ export function createAgentStatusTracker( ): { handleTitle: (title: string) => void seedTitle: (title: string) => void - restoreLastExit: () => void + restoreLastExit: () => AgentStatus | null reset: () => void } { // Why: trackers restored mid-session need a last-known status without firing @@ -92,11 +92,13 @@ export function createAgentStatusTracker( lastStatus = detectAgentStatusFromTitle(title) restorableExitStatus = null }, - restoreLastExit(): void { - if (lastStatus === null && restorableExitStatus !== null) { - lastStatus = restorableExitStatus + restoreLastExit(): AgentStatus | null { + const restoredStatus = lastStatus === null ? restorableExitStatus : null + if (restoredStatus !== null) { + lastStatus = restoredStatus } restorableExitStatus = null + return restoredStatus }, reset(): void { lastStatus = null diff --git a/src/shared/terminal-output-side-effects.ts b/src/shared/terminal-output-side-effects.ts index 227df5bd4..32dc33724 100644 --- a/src/shared/terminal-output-side-effects.ts +++ b/src/shared/terminal-output-side-effects.ts @@ -5,6 +5,7 @@ */ import { + type AgentStatus, clearWorkingIndicators, createAgentStatusTracker, detectAgentStatusFromTitle, @@ -91,7 +92,7 @@ export type TerminalTitleTracker = { */ seedInitialTitle: (rawTitle: string) => void /** Restore the status consumed by the latest exit candidate when process evidence disproves it. */ - restoreLastAgentExit: () => void + restoreLastAgentExit: () => AgentStatus | null /** Last title surfaced through onTitle, after normalization. */ getLastNormalizedTitle: () => string | null /** @@ -264,8 +265,8 @@ export function createTerminalTitleTracker( lastEmittedTitle = normalizeTerminalTitle(rawTitle) agentTracker?.seedTitle(rawTitle) }, - restoreLastAgentExit(): void { - agentTracker?.restoreLastExit() + restoreLastAgentExit(): AgentStatus | null { + return agentTracker?.restoreLastExit() ?? null }, getLastNormalizedTitle: () => lastEmittedTitle, setTransientFactScanningSuppressed(suppressed: boolean): void { diff --git a/tests/e2e/helpers/orchestration-mail-pane-agent.ts b/tests/e2e/helpers/orchestration-mail-pane-agent.ts index f948bfa8c..df9d0311d 100644 --- a/tests/e2e/helpers/orchestration-mail-pane-agent.ts +++ b/tests/e2e/helpers/orchestration-mail-pane-agent.ts @@ -6,7 +6,7 @@ * writing into the pane's foreground process. A shell echoes rather than * records, so it can prove the gate but never the payload. This process owns * both sides — the test drives its title through a control file and it appends - * every stdin chunk to a ledger, which is what makes "the banner and the Enter + * every stdin chunk to a ledger, which is what makes "the pointer and the Enter * reached the agent" an assertion instead of an inference. * * Titles come from a polled file, not stdin, because orchestration writes to @@ -54,12 +54,12 @@ log({ event: 'start' }) // Raw mode is what every agent TUI does, and it is load-bearing here: a cooked // PTY applies ICRNL, so the synthesized Enter would arrive as \\n and be -// indistinguishable from the banner's own newlines. +// indistinguishable from the pointer's own newlines. if (process.stdin.isTTY) { process.stdin.setRawMode(true) } -// Every byte orchestration pushes lands here — banner text and Enter alike. +// Every byte orchestration pushes lands here — pointer text and Enter alike. process.stdin.on('data', (chunk) => log({ event: 'stdin', data: chunk.toString() })) process.stdin.resume() diff --git a/tests/e2e/helpers/orchestration-mail-store.ts b/tests/e2e/helpers/orchestration-mail-store.ts index 520498dad..7232c8d6a 100644 --- a/tests/e2e/helpers/orchestration-mail-store.ts +++ b/tests/e2e/helpers/orchestration-mail-store.ts @@ -3,9 +3,8 @@ * * Why read SQLite instead of `orchestration.check`: check is itself a consumer — * it marks rows read and backfills `delivered_at` — so using it to observe would - * destroy the very distinction these specs exist to test. The two markers are - * independent on purpose: `delivered_at` means a push typed the row into a pane, - * `read` means a pull consumed it. Only an out-of-band read can tell them apart. + * destroy the very distinction these specs exist to test. A pointer changes + * neither marker; only an out-of-band read can prove that before check consumes. */ import path from 'node:path' import Database from '../../../src/main/sqlite/sync-database' @@ -49,13 +48,10 @@ export function readMailbox(userDataDir: string, toHandle: string): MailRow[] { } /** - * Mark `handle` as the running coordinator — the state that makes push delivery - * withhold the synthesized Enter, because that prompt holds user-typed input. + * Mark `handle` as a legacy running coordinator to prove it no longer suppresses Enter. * - * Why seed the row instead of calling `orchestration.run`: that RPC also starts - * a live coordinator loop which dispatches workers on a timer, and its - * scheduling would race every assertion here. The carve-out reads nothing but - * this row. + * Why seed instead of calling `orchestration.run`: that RPC starts a coordinator + * loop whose scheduling would race the assertion. */ export function startCoordinatorRun(userDataDir: string, handle: string): void { withMailDb(userDataDir, (db) => { @@ -67,7 +63,7 @@ export function startCoordinatorRun(userDataDir: string, handle: string): void { } /** - * How a row was consumed, if at all. + * How a row was consumed under either the current or historical push behavior. * * `read` is checked first because a pull backfills `delivered_at` via COALESCE, * so a pulled row also carries a delivery stamp — the stamp alone cannot prove diff --git a/tests/e2e/orchestration-idle-mail-delivery.spec.ts b/tests/e2e/orchestration-idle-mail-delivery.spec.ts index 2d1ec41b6..fcfe81005 100644 --- a/tests/e2e/orchestration-idle-mail-delivery.spec.ts +++ b/tests/e2e/orchestration-idle-mail-delivery.spec.ts @@ -9,7 +9,7 @@ * * These specs drive real PTYs: the recipient is a fake `codex` on PATH whose OSC * titles the test controls through a file, and which appends every stdin chunk - * to a ledger. That ledger is the oracle — it proves the banner and the + * to a ledger. That ledger is the oracle — it proves the pointer and the * synthesized Enter reached the agent process, which no store or DB read can. * * The ordering fixes on this path (microtask deferral, probe-window respawn, @@ -19,6 +19,7 @@ */ import { test, expect } from './helpers/orca-app' import type { ElectronApplication, Page } from '@stablyai/playwright-test' +import { randomUUID } from 'node:crypto' import { waitForSessionReady, waitForActiveWorktree, ensureTerminalVisible } from './helpers/store' import { execInTerminal, @@ -37,16 +38,16 @@ import { } from './helpers/orchestration-mail-pane-agent' import { mailDisposition, + readMailbox, readMailRow, startCoordinatorRun } from './helpers/orchestration-mail-store' import { waitForPtyShellEcho } from './terminal-pty-readiness' -/** The wrapper `formatMessagesForInjection` puts around every pushed batch. */ -const BANNER_PREFIX = '--- Orchestration Messages' +const POINTER_COMMAND = 'orca orchestration check' // Why generous: the push runs a microtask behind the send, may defer once more -// behind a liveness probe, and only stamps delivered_at after a 500ms Enter. +// behind a liveness probe, and submits Enter after a 500ms delay. const DELIVERY_TIMEOUT_MS = 20_000 // Why 3s: long enough to cover that same chain, so "still pending" means the // gate refused rather than that the push had not run yet. @@ -175,18 +176,19 @@ async function sendMail( return sent.result.message.id } -async function expectPushed(pane: AgentPane, subject: string): Promise { +async function expectPointed(pane: AgentPane, count = 1): Promise { await expect .poll(() => pane.agent.readStdin(), { timeout: DELIVERY_TIMEOUT_MS, - message: 'banner never reached the agent process' + message: 'mail pointer never reached the agent process' }) - .toContain(BANNER_PREFIX) - expect(pane.agent.readStdin()).toContain(`Subject: ${subject}`) + .toContain(POINTER_COMMAND) + const noun = count === 1 ? 'message' : 'messages' + expect(pane.agent.readStdin()).toContain(`${count} orchestration ${noun}`) } /** - * The synthesized Enter is a separate write ~500ms after the banner. The banner + * The synthesized Enter is a separate write ~500ms after the pointer. The pointer * itself is `\n`-joined, so a `\r` anywhere in stdin can only be that submit — * which keeps the assertion independent of how the PTY chunks the two writes. */ @@ -219,7 +221,7 @@ async function expectStaysPending( expect(readMailRow(userDataDir, messageId)).toBeDefined() await page.waitForTimeout(NO_DELIVERY_SETTLE_MS) expect(mailDisposition(readMailRow(userDataDir, messageId))).toBe('pending') - expect(pane.agent.readStdin()).not.toContain(BANNER_PREFIX) + expect(pane.agent.readStdin()).not.toContain(POINTER_COMMAND) } test.describe('orchestration push-on-idle mail delivery', () => { @@ -237,13 +239,13 @@ test.describe('orchestration push-on-idle mail delivery', () => { const subject = 'Already idle delivery' const messageId = await sendMail(client, pane.handle, { subject }) - await expectPushed(pane, subject) + await expectPointed(pane) await expectSubmitted(pane) await expect .poll(() => mailDisposition(readMailRow(userDataDir, messageId)), { timeout: DELIVERY_TIMEOUT_MS }) - .toBe('pushed') + .toBe('pending') }) test('holds mail while the agent is working and releases it on the idle frame', async ({ @@ -263,12 +265,12 @@ test.describe('orchestration push-on-idle mail delivery', () => { // Releasing the gate proves the silence above was the working status and not // a harness that never wired the send to this pane at all. pane.agent.setTitle(CODEX_IDLE_TITLE) - await expectPushed(pane, subject) + await expectPointed(pane) await expect .poll(() => mailDisposition(readMailRow(userDataDir, messageId)), { timeout: DELIVERY_TIMEOUT_MS }) - .toBe('pushed') + .toBe('pending') }) // Guards the null→idle path rather than reproducing #12536: a fresh pane has @@ -291,7 +293,7 @@ test.describe('orchestration push-on-idle mail delivery', () => { // Idle is this pane's FIRST live status, so there is no busy→idle edge here // either; delivery has to hang off the liveness of the observation. pane.agent.setTitle(CODEX_IDLE_TITLE) - await expectPushed(pane, subject) + await expectPointed(pane) await expectSubmitted(pane) }) @@ -325,7 +327,7 @@ test.describe('orchestration push-on-idle mail delivery', () => { const pulled = await waiting expect(pulled.result.messages.map((message) => message.subject)).toContain(subject) - expect(pane.agent.readStdin()).not.toContain(BANNER_PREFIX) + expect(pane.agent.readStdin()).not.toContain(POINTER_COMMAND) // Pending, not pushed: the pull won, and the push stays available for a // later notify rather than racing this one. expect(mailDisposition(readMailRow(userDataDir, messageId))).toBe('pending') @@ -355,16 +357,95 @@ test.describe('orchestration push-on-idle mail delivery', () => { const subject = 'Filtered waiter' const messageId = await sendMail(client, pane.handle, { subject, type: 'status' }) - await expectPushed(pane, subject) + await expectPointed(pane) await expect .poll(() => mailDisposition(readMailRow(userDataDir, messageId)), { timeout: DELIVERY_TIMEOUT_MS }) - .toBe('pushed') + .toBe('pending') await waiting }) - test('writes the banner but never Enter for the active coordinator pane', async ({ + test('worker completion points and wakes its idle Run coordinator without consuming mail', async ({ + orcaPage, + electronApp + }) => { + test.setTimeout(180_000) + const { client, userDataDir, openAgentPane } = await setUpMailFixture(orcaPage, electronApp) + const pane = await openAgentPane() + await driveToLiveIdle(client, pane) + + const run = await client.call<{ run: { id: string } }>('orchestration.runCreate', { + objective: 'Verify worker completion pointer delivery', + from: pane.handle + }) + const task = await client.call<{ task: { id: string } }>('orchestration.taskCreate', { + spec: 'Report one P3 finding', + run: run.result.run.id, + callerTerminalHandle: pane.handle + }) + const dispatched = await client.call<{ dispatch: { id: string } }>('orchestration.dispatch', { + task: task.result.task.id, + run: run.result.run.id, + from: pane.handle, + to: pane.handle + }) + const body = 'full private review finding must remain in SQLite' + const payload = JSON.stringify({ + taskId: task.result.task.id, + dispatchId: dispatched.result.dispatch.id, + outcome: 'succeeded' + }) + const sendParams = { + from: pane.handle, + to: pane.handle, + subject: 'review: one P3 finding', + body, + type: 'worker_done', + payload + } + const orchestrationRequestId = randomUUID() + const sent = await client.call<{ message: { id: string; to_handle: string } }>( + 'orchestration.send', + sendParams, + { orchestrationRequestId } + ) + const runAddress = `run:${run.result.run.id}` + expect(sent.result.message.to_handle).toBe(runAddress) + + await expectPointed(pane) + await expectSubmitted(pane) + expect(pane.agent.readStdin()).not.toContain(body) + const pointedRow = readMailRow(userDataDir, sent.result.message.id) + expect(pointedRow).toMatchObject({ to_handle: runAddress, read: 0, delivered_at: null }) + + const stdinAfterFirstPointer = pane.agent.readStdin() + const duplicate = await client.call<{ message: { id: string } }>( + 'orchestration.send', + sendParams, + { orchestrationRequestId } + ) + pane.agent.setTitle(CODEX_WORKING_TITLE) + await waitForObservedTitle(client, pane.handle, CODEX_WORKING_TITLE) + pane.agent.setTitle(CODEX_IDLE_TITLE) + await waitForObservedTitle(client, pane.handle, CODEX_IDLE_TITLE) + await orcaPage.waitForTimeout(NO_DELIVERY_SETTLE_MS) + expect(pane.agent.readStdin()).toBe(stdinAfterFirstPointer) + expect(duplicate.result.message.id).toBe(sent.result.message.id) + expect(readMailbox(userDataDir, runAddress).filter((row) => row.read === 0)).toEqual([ + expect.objectContaining({ id: sent.result.message.id }) + ]) + + const checked = await client.call<{ messages: { id: string; body: string }[] }>( + 'orchestration.check', + { terminal: pane.handle, run: run.result.run.id } + ) + expect(checked.result.messages).toEqual([ + expect.objectContaining({ id: sent.result.message.id, body }) + ]) + }) + + test('writes and submits the pointer for the active coordinator pane', async ({ orcaPage, electronApp }) => { @@ -374,17 +455,14 @@ test.describe('orchestration push-on-idle mail delivery', () => { await driveToLiveIdle(client, pane) startCoordinatorRun(userDataDir, pane.handle) - // The coordinator prompt is user-owned input; synthesizing Enter there would - // submit whatever the human was mid-way through typing (#7337). - const subject = 'Coordinator no-submit' + const subject = 'Coordinator pointer submit' await sendMail(client, pane.handle, { subject }) - await expectPushed(pane, subject) - await orcaPage.waitForTimeout(2_000) - expectNotSubmitted(pane) + await expectPointed(pane) + await expectSubmitted(pane) }) - test('writes the banner but never Enter for a Cursor agent pane', async ({ + test('writes the pointer but never Enter for a Cursor agent pane', async ({ orcaPage, electronApp }) => { @@ -399,7 +477,7 @@ test.describe('orchestration push-on-idle mail delivery', () => { const subject = 'Cursor no-submit' await sendMail(client, pane.handle, { subject }) - await expectPushed(pane, subject) + await expectPointed(pane) await orcaPage.waitForTimeout(2_000) expectNotSubmitted(pane) }) diff --git a/tests/e2e/orchestration-idle-mail-restore.spec.ts b/tests/e2e/orchestration-idle-mail-restore.spec.ts index 6e38b9fd8..4e528614f 100644 --- a/tests/e2e/orchestration-idle-mail-restore.spec.ts +++ b/tests/e2e/orchestration-idle-mail-restore.spec.ts @@ -1,12 +1,12 @@ /** * Mail must survive a restart: never injected on restored state alone, always - * delivered once the agent speaks again (#12536). + * pointed once the agent speaks again (#12536). * * Push-on-idle now fires when mail arrives rather than only on a busy→idle edge, * which puts restart squarely on the delivery path — a pane comes back carrying * the title it had at snapshot time, and anything the runtime infers from that * is a memory, not an observation. Typing on it would submit into an agent that - * may be mid-turn and stamp the row delivered, losing it. + * may be mid-turn. * * Scope, stated plainly: this covers the restart path, not the * `lastAgentStatusObservedLive` gate itself. The seed only reaches leaves that @@ -39,7 +39,7 @@ import { import { mailDisposition, readMailRow } from './helpers/orchestration-mail-store' import { waitForPtyShellEcho } from './terminal-pty-readiness' -const BANNER_PREFIX = '--- Orchestration Messages' +const POINTER_COMMAND = 'orca orchestration check' const NO_DELIVERY_SETTLE_MS = 5_000 const DELIVERY_TIMEOUT_MS = 20_000 @@ -159,10 +159,10 @@ test('keeps mail pending across a restart and delivers it when the agent reports expect(readMailRow(session.userDataDir, messageId)).toBeDefined() await second.page.waitForTimeout(NO_DELIVERY_SETTLE_MS) expect(mailDisposition(readMailRow(session.userDataDir, messageId))).toBe('pending') - expect(agent.readStdin()).not.toContain(BANNER_PREFIX) + expect(agent.readStdin()).not.toContain(POINTER_COMMAND) // Re-emitting the SAME idle title changes no status — only its liveness — so - // the row moving here is delivery resuming on the agent's own signal. + // the pointer appearing here is delivery resuming on the agent's own signal. agent.setTitle(CODEX_IDLE_TITLE) await expect .poll(() => agent.titleEmitCount(), { timeout: 30_000 }) @@ -172,12 +172,8 @@ test('keeps mail pending across a restart and delivers it when the agent reports timeout: DELIVERY_TIMEOUT_MS, message: 'live idle frame never released the pending mail' }) - .toContain(BANNER_PREFIX) - await expect - .poll(() => mailDisposition(readMailRow(session.userDataDir, messageId)), { - timeout: DELIVERY_TIMEOUT_MS - }) - .toBe('pushed') + .toContain(POINTER_COMMAND) + expect(mailDisposition(readMailRow(session.userDataDir, messageId))).toBe('pending') } finally { if (firstApp) { await session.close(firstApp)