fix(runtime): reject leaf terminal sends only on controller-proven PTY absence (#12578)

* fix(runtime): reject leaf terminal sends only on controller-proven PTY absence

orca terminal send to a leaf whose ptyId no provider in this process owns
was a silent no-op reported as success: the graph mirror answers
writable=true, every provider write to an unknown id is accepted
fire-and-forget, and bytesWritten is computed from the payload rather than
delivery. sendTerminal and sendTerminalAgentPrompt now consult a controller
liveness probe when the provider does not synchronously know the id
(hasPty), and throw terminal_not_writable only on an exact false — unknown
liveness, probe errors, SSH/remote scopes, and probe-less providers never
reject (#12393's rule: null is not absence), so a restored daemon session
still accepts writes before its pane remounts. Push-on-idle orchestration
delivery gains the same gate so a proven-dead leaf keeps its messages
queued instead of marking them delivered into a void.

The pty controller now exposes probePtyLiveness, routed like write: a
provider probe is preferred, the in-process local provider's refusal is
authoritative (sole owner), and remote-scoped or SSH ids without a probe
answer null after awaiting the cold-start daemon swap. Proven-absent
verdicts cache 15s per ptyId with in-flight dedupe, superseded the moment
the provider re-learns the id.

* fix(runtime): arm one probe-deferred delivery continuation per pty

Review (GPT verifier) confirmed: triggers arriving during one in-flight
absence probe each attached a continuation to the deduped probe promise, and
since Claude-target delivered_at stamps only after the delayed Enter, every
continuation re-read the same unread rows — double payload injection and two
armed Enters. Single-flight the deferred continuation per pty; the one armed
continuation re-reads fresh rows when it fires, so nothing is lost, and the
guard clears on settle so later triggers defer again. The narrower
pre-existing 500ms sync-path window is unchanged and out of scope.

* fix(runtime): single-flight the whole orchestration delivery window per pty

The probe-continuation guard cleared at probe settle, but Claude-target
delivered_at stamps only in the delayed-Enter callback ~500ms later — a
trigger landing in that gap armed a fresh probe cycle, re-read the same
un-stamped rows, and re-injected the payload. The identical window existed
on the pure sync path pre-PR (two triggers within 500ms double-deliver).

Hold a per-pty delivery-in-flight flag from before the payload write until
delivery settles: entry-checked before reading unread rows, cleared through
one settle point covering the failed write, the sync-stamped coordinator and
Cursor branches, any sync throw, and the delayed-Enter callback on submit,
refusal, and throw alike. A trigger arriving mid-flight is not dropped — it
parks the latest leaf per ptyId and re-runs delivery once on settle, so rows
inserted mid-flight deliver without waiting for the next idle event. The
probe single-flight stays; the new guard subsumes its post-settle gap, and
no trigger site bypasses it.

Both strengthened tests are red on the previous commit (first subject
injected twice) and green here: in-window re-trigger on the probe path and
sync-path double-trigger each deliver the first batch exactly once, with the
parked second row delivering alone after settle.

* fix(runtime): retire the armed delivery Enter on pty exit; guard fire-time on current state

Two variants of one root cause — the delayed-Enter callback outliving the
session it was armed for:

1. Cold restore respawns under the same session id. onPtyExit never
   cancelled the armed Enter or the in-flight delivery state, and
   onPtySpawned flips the same leaf writable again — so an exit + same-id
   respawn inside the 500ms window let the stale callback inject \r into
   the replacement session and stamp rows it never received, then settle
   against a newer same-id flight.
2. Graph resync replaces leaf objects, so onPtyExit flips writable=false
   only on the current replacement; a callback trusting its closed-over
   snapshot still read writable=true and fired after exit with no respawn.

The flight record now carries its armed Enter timer and serves as settle
identity: onPtyExit clears the timer and drops the flight and any parked
re-delivery without stamping (rows stay unstamped and re-deliver on the
replacement's next idle — the existing contract), and settle no-ops unless
its own flight is still current, so a stale settle can never clear a newer
same-id flight or flush its parked trigger. At fire time the callback
re-resolves the leaf by key and requires the same ptyId binding and current
writability instead of reading the closure snapshot.

All three regressions are red on the previous commit: same-id respawn saw
\r plus a false delivered_at stamp, exit leaked the flight and parked
state, and the orphaned-snapshot resync variant fired Enter after exit.

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
Jinwoo Hong 2026-08-04 17:22:10 -07:00 committed by GitHub
parent 27da04d50d
commit 3d8131d7ea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 833 additions and 35 deletions

View File

@ -750,12 +750,14 @@ describe('registerPtyHandlers', () => {
spawn: (args: Record<string, unknown>) => Promise<unknown>
write: (ptyId: string, data: string) => boolean
resize: (ptyId: string, cols: number, rows: number) => boolean
probePtyLiveness: (ptyId: string) => Promise<boolean | null>
} {
let controller:
| {
spawn: (args: Record<string, unknown>) => Promise<unknown>
write: (ptyId: string, data: string) => boolean
resize: (ptyId: string, cols: number, rows: number) => boolean
probePtyLiveness: (ptyId: string) => Promise<boolean | null>
}
| undefined
const runtime = {
@ -801,6 +803,78 @@ describe('registerPtyHandlers', () => {
clearProviderPtyState(ptyId)
})
describe('controller probePtyLiveness routing', () => {
it('proves absence for an id the in-process local provider never owned', async () => {
setLocalPtyProvider(new LocalPtyProvider())
const controller = registerAgentClaimController()
await expect(controller.probePtyLiveness('pty-from-prior-run')).resolves.toBe(false)
})
it('delegates to a provider-exposed probe and preserves its answer', async () => {
const provider = {
...createAgentClaimProvider({}),
probePtyLiveness: vi.fn(async () => true)
}
setLocalPtyProvider(provider as never)
const controller = registerAgentClaimController()
await expect(controller.probePtyLiveness('daemon-owned')).resolves.toBe(true)
expect(provider.probePtyLiveness).toHaveBeenCalledWith('daemon-owned')
})
it('answers unknown for a probe-less provider that is not the in-process one', async () => {
// Why: only the in-process provider is its own sole owner; any other
// probe-less provider's ignorance is doubt, not absence.
setLocalPtyProvider(createAgentClaimProvider({}) as never)
const controller = registerAgentClaimController()
await expect(controller.probePtyLiveness('pty-unknown')).resolves.toBeNull()
})
it('answers unknown for SSH-owned ids whose provider has no probe', async () => {
const connectionId = 'ssh-probe-1'
const ptyId = `ssh:${connectionId}@@remote-pty`
setLocalPtyProvider(new LocalPtyProvider())
registerSshPtyProvider(connectionId, createAgentClaimProvider({}) as never)
setPtyOwnership(ptyId, connectionId)
const controller = registerAgentClaimController()
try {
await expect(controller.probePtyLiveness(ptyId)).resolves.toBeNull()
unregisterSshPtyProvider(connectionId)
// A disconnected SSH provider is an error path, and errors never prove absence.
await expect(controller.probePtyLiveness(ptyId)).resolves.toBeNull()
} finally {
unregisterSshPtyProvider(connectionId)
clearPtyOwnershipForConnection(connectionId)
clearProviderPtyState(ptyId)
}
})
it('answers unknown for remote-scoped ids without consulting local providers', async () => {
// Why: a locally routed provider would answer confidently — and wrongly —
// for a PTY that lives on a remote Orca host.
setLocalPtyProvider(new LocalPtyProvider())
const controller = registerAgentClaimController()
await expect(controller.probePtyLiveness('remote:some-remote-pty')).resolves.toBeNull()
})
it('answers unknown when the provider probe throws', async () => {
const provider = {
...createAgentClaimProvider({}),
probePtyLiveness: vi.fn(async () => {
throw new Error('probe transport down')
})
}
setLocalPtyProvider(provider as never)
const controller = registerAgentClaimController()
await expect(controller.probePtyLiveness('daemon-owned')).resolves.toBeNull()
})
})
it('does not dispatch a runtime PTY spawn after its client disconnects', async () => {
const provider = createAgentClaimProvider({})
setLocalPtyProvider(provider as never)

View File

@ -5067,6 +5067,34 @@ export function registerPtyHandlers(
return false
}
},
probePtyLiveness: async (ptyId) => {
try {
// Why: no locally routed provider can authoritatively answer for a
// remote host's PTY, so remote-scoped ids stay unknown, never absent.
if (ptyId.startsWith('remote:')) {
return null
}
const connectionId = ptyOwnership.get(ptyId) ?? parseAppSshPtyId(ptyId)?.connectionId
// Why: during cold start the daemon swap is in flight; the pre-swap
// fallback would answer absent for every daemon-owned id.
const startupPromise = getLocalPtyProviderStartupPromise(connectionId)
if (startupPromise) {
await startupPromise
}
const provider = getProviderForPty(ptyId)
if (provider.probePtyLiveness) {
return await provider.probePtyLiveness(ptyId)
}
// Why: the in-process provider is its own sole owner (#12393), so its
// refusal is authoritative; every other probe-less provider is doubt.
if (provider instanceof LocalPtyProvider) {
return provider.hasPty(ptyId)
}
return null
} catch {
return null
}
},
kill: (ptyId) => {
let connectionId: string | null | undefined = ptyOwnership.get(ptyId)
const parsedSshId = connectionId === undefined ? parseAppSshPtyId(ptyId) : null

View File

@ -1660,6 +1660,8 @@ type RuntimePtyController = {
signal?: AbortSignal
): Promise<boolean>
getSize?(ptyId: string): { cols: number; rows: number } | null
/** False only when the owning provider proved the PTY absent; null = unknown (never a denial). */
probePtyLiveness?(ptyId: string): Promise<boolean | null>
}
type PtyControllerTerminalIdentity = Readonly<{
@ -1721,6 +1723,10 @@ const RECENT_PTY_PATH_CANDIDATE_LIMIT = 1024
const RECENT_PTY_PATH_CANDIDATE_MAX_BYTES = 4 * 1024
const RECENT_PTY_PATH_CANDIDATE_TOTAL_BYTES = 64 * 1024
const SSH_PANE_RECOVERY_GRACE_MS = 30_000
// Why: long enough that a keystroke burst to a proven-dead leaf probes once,
// short enough that a recreated session id regains writability quickly even if
// its runtime record (which also invalidates the verdict) is late.
const PROVEN_ABSENT_LEAF_PTY_TTL_MS = 15_000
function isClientDisconnectedError(error: unknown): boolean {
return error instanceof Error && error.message === 'client_disconnected'
@ -13202,6 +13208,10 @@ export class OrcaRuntimeService {
clearTimeout(pendingSoft.timer)
this.pendingSoftLeavers.delete(ptyId)
}
// Why: a cold restore can respawn under the same session id within the
// delayed-Enter window; the armed Enter would inject \r into the
// replacement and stamp rows it never received.
this.retirePendingMessageDeliveryForPty(ptyId)
if (this.terminalFitOverrides.has(ptyId)) {
this.terminalFitOverrides.delete(ptyId)
@ -16079,6 +16089,67 @@ export class OrcaRuntimeService {
return visibleRead
}
// Why a cache: leaf-branch sends may arrive per keystroke; one proven-absent
// verdict per ptyId serves the burst instead of a probe round-trip each call.
private readonly provenAbsentLeafPtyVerdicts = new Map<string, number>()
private readonly leafPtyAbsenceProbes = new Map<string, Promise<boolean>>()
// Why: probe dedupe shares one promise across callers, but each caller's
// continuation would re-deliver the same unread rows; arm one per pty.
private readonly probeDeferredDeliveryPtyIds = new Set<string>()
private controllerKnowsPtyIsLive(ptyId: string): boolean {
try {
return this.ptyController?.hasPty?.(ptyId) === true
} catch {
// Why: liveness lookup failures are doubt; doubt never gates a write.
return false
}
}
/** True only on controller-proven absence; live, unknown, and probe errors all answer false. */
private isLeafPtyProvenAbsent(ptyId: string): Promise<boolean> {
// Why hasPty and not ptysById: graph sync mirrors a connected record for
// every leaf ptyId — including a prior process's — so runtime records can't
// distinguish live from stale. The controller's exact-id hasPty is the
// provider's own synchronous inventory: a known id is alive, skip probing
// and supersede any cached verdict (the id came back).
if (this.controllerKnowsPtyIsLive(ptyId)) {
this.provenAbsentLeafPtyVerdicts.delete(ptyId)
return Promise.resolve(false)
}
const verdictAt = this.provenAbsentLeafPtyVerdicts.get(ptyId)
if (verdictAt !== undefined) {
if (Date.now() - verdictAt < PROVEN_ABSENT_LEAF_PTY_TTL_MS) {
return Promise.resolve(true)
}
this.provenAbsentLeafPtyVerdicts.delete(ptyId)
}
const probeLiveness = this.ptyController?.probePtyLiveness?.bind(this.ptyController)
if (!probeLiveness) {
return Promise.resolve(false)
}
const inFlight = this.leafPtyAbsenceProbes.get(ptyId)
if (inFlight) {
return inFlight
}
const probe = (async () => {
try {
if ((await probeLiveness(ptyId)) !== false) {
return false
}
this.provenAbsentLeafPtyVerdicts.set(ptyId, Date.now())
return true
} catch {
// Why: a failed probe is unknown, and unknown never rejects a write.
return false
} finally {
this.leafPtyAbsenceProbes.delete(ptyId)
}
})()
this.leafPtyAbsenceProbes.set(ptyId, probe)
return probe
}
async sendTerminal(
handle: string,
action: {
@ -16120,6 +16191,13 @@ export class OrcaRuntimeService {
throw new Error('invalid_terminal_send')
}
await assertTerminalInputWithinLimitWithYield(action.text)
// Why: leaf.writable mirrors the renderer graph, which can still answer for
// a prior process's ptyId — and provider writes to unknown ids are accepted
// no-ops. Only controller-proven absence rejects; unknown proceeds (a
// restored daemon session takes writes before its pane remounts).
if (await this.isLeafPtyProvenAbsent(leaf.ptyId)) {
throw new Error('terminal_not_writable')
}
await this.writeTerminalAction(leaf.ptyId, action, payload, options)
@ -16155,6 +16233,11 @@ export class OrcaRuntimeService {
throw new Error('terminal_not_writable')
}
await assertTerminalInputWithinLimitWithYield(payload)
// Why: same absence gate as sendTerminal — a stale graph mirror must not
// accept a prompt into a void; unknown liveness still proceeds.
if (await this.isLeafPtyProvenAbsent(leaf.ptyId)) {
throw new Error('terminal_not_writable')
}
await this.writeTerminalAgentPrompt(leaf.ptyId, payload, options)
return { handle, accepted: true, bytesWritten }
}
@ -31232,8 +31315,49 @@ 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
// 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<
string,
{ enterTimer: ReturnType<typeof setTimeout> | null }
>()
private readonly parkedMessageRedeliveryLeavesByPtyId = new Map<string, RuntimeLeafRecord>()
private settlePendingMessageDelivery(
ptyId: string,
flight: { enterTimer: ReturnType<typeof setTimeout> | null }
): void {
if (this.messageDeliveryFlightsByPtyId.get(ptyId) !== flight) {
return
}
this.messageDeliveryFlightsByPtyId.delete(ptyId)
const parkedLeaf = this.parkedMessageRedeliveryLeavesByPtyId.get(ptyId)
if (!parkedLeaf) {
return
}
this.parkedMessageRedeliveryLeavesByPtyId.delete(ptyId)
this.deliverPendingMessages(parkedLeaf)
}
// 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.
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)
}
// 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): void {
private deliverPendingMessages(leaf: RuntimeLeafRecord, skipAbsenceProbe = false): void {
if (!this._orchestrationDb) {
return
}
@ -31243,6 +31367,12 @@ export class OrcaRuntimeService {
return
}
// 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)
return
}
const unread = this._orchestrationDb.getUndeliveredUnreadMessages(handle)
if (unread.length === 0) {
return
@ -31252,41 +31382,95 @@ export class OrcaRuntimeService {
return
}
const payload = formatMessagesForInjection(unread)
const wrote = this.ptyController?.write(leaf.ptyId, 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
}
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.
const ptyId = leaf.ptyId
setTimeout(() => {
try {
if (!leaf.writable) {
return
}
const submitted = this.ptyController?.write(ptyId, '\r') ?? false
if (submitted) {
this._orchestrationDb?.markAsDelivered(unread.map((m) => m.id))
}
} catch {
// Terminal may have closed during the delay — messages stay queued (delivered_at NULL) and re-deliver on next idle.
if (
!skipAbsenceProbe &&
this.ptyController?.probePtyLiveness &&
!this.controllerKnowsPtyIsLive(leaf.ptyId)
) {
// Why: a fire-and-forget write to a prior process's ptyId reports success
// and would mark these delivered while losing them. Proven absence keeps
// 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
// continuation re-reads fresh rows when it fires, so nothing is lost.
if (this.probeDeferredDeliveryPtyIds.has(probedPtyId)) {
return
}
}, 500)
this.probeDeferredDeliveryPtyIds.add(probedPtyId)
void this.isLeafPtyProvenAbsent(probedPtyId)
.then((absent) => {
this.probeDeferredDeliveryPtyIds.delete(probedPtyId)
if (!absent && leaf.ptyId === probedPtyId) {
this.deliverPendingMessages(leaf, true)
}
})
.catch(() => {
this.probeDeferredDeliveryPtyIds.delete(probedPtyId)
})
return
}
const deliveryPtyId = leaf.ptyId
const flight: { enterTimer: ReturnType<typeof setTimeout> | null } = { enterTimer: null }
this.messageDeliveryFlightsByPtyId.set(deliveryPtyId, flight)
// Why: every sync outcome — failed write, sync-stamped 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 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
}
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.
flight.enterTimer = setTimeout(() => {
try {
// Why current state, not the closure: graph resync replaces leaf
// objects, so the captured record can read writable=true after the
// pty died, and an exit retire may have superseded this flight.
if (this.messageDeliveryFlightsByPtyId.get(deliveryPtyId) !== flight) {
return
}
const currentLeaf = this.leaves.get(this.getLeafKey(leaf.tabId, leaf.leafId))
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))
}
} catch {
// Terminal may have closed during the delay — messages stay queued (delivered_at NULL) and re-deliver on next idle.
} finally {
// Why finally: every outcome — submit, refusal, throw — ends the flight,
// and settle re-runs any trigger parked during it so nothing strands.
this.settlePendingMessageDelivery(deliveryPtyId, flight)
}
}, 500)
settlesInEnterCallback = true
} finally {
if (!settlesInEnterCallback) {
this.settlePendingMessageDelivery(deliveryPtyId, flight)
}
}
}
private resolveWaiter(waiter: TerminalWaiter, result: RuntimeTerminalWait): void {

View File

@ -0,0 +1,512 @@
import { describe, expect, it, vi } from 'vitest'
import { OrcaRuntimeService } from './orca-runtime'
import { getDefaultWorkspaceSession } from '../../shared/constants'
import type { WorkspaceSessionState } from '../../shared/types'
// STA repro (silent-send incident): `orca terminal send` to a leaf whose ptyId
// no provider in this process owns was a silent no-op reported as success —
// the stale graph mirror answers writable=true and provider writes to unknown
// ids are accepted fire-and-forget. The leaf branch must reject ONLY on
// controller-proven absence; unknown liveness never rejects (a restored daemon
// session legitimately accepts writes before its pane remounts).
const WORKTREE_ID = 'repo-1::/tmp/probe-worktree'
const LEAF_ID = '11111111-1111-4111-8111-111111111111'
const STALE_PTY_ID = 'pty-stale-from-prior-run'
function makeStore() {
const session: WorkspaceSessionState = getDefaultWorkspaceSession()
return {
getWorkspaceSession: vi.fn(() => session),
setWorkspaceSession: vi.fn(),
getRepos: vi.fn(() => [
{
id: 'repo-1',
path: '/tmp/probe-worktree',
displayName: 'probe',
badgeColor: '#000000',
addedAt: 0
}
]),
getAllWorktreeMeta: vi.fn(() => ({})),
getWorktreeMeta: vi.fn(() => undefined),
setWorktreeMeta: vi.fn(),
removeWorktreeMeta: vi.fn(),
getSettings: vi.fn(() => ({ workspaceDir: '/tmp/workspaces' })),
getProjects: vi.fn(() => [])
}
}
async function makeRuntimeWithLeafHandle(options: {
leafPtyId?: string
probePtyLiveness?: (ptyId: string) => Promise<boolean | null>
hasPty?: (ptyId: string) => boolean | null
}): Promise<{
runtime: OrcaRuntimeService
handle: string
write: ReturnType<typeof vi.fn>
}> {
const runtime = new OrcaRuntimeService(makeStore() as never)
const write = vi.fn(() => true)
runtime.setPtyController({
spawn: vi.fn(async () => ({ id: 'never' })),
write,
kill: () => true,
getForegroundProcess: async () => null,
listProcesses: vi.fn(async () => []),
...(options.hasPty ? { hasPty: options.hasPty } : {}),
...(options.probePtyLiveness ? { probePtyLiveness: options.probePtyLiveness } : {})
} as never)
runtime.attachWindow(1)
publishLeafGraph(runtime, options.leafPtyId ?? STALE_PTY_ID)
const { terminals } = await runtime.listTerminals(`id:${WORKTREE_ID}`)
return { runtime, handle: terminals[0].handle, write }
}
// Re-invocable: every graph resync replaces leaf records with fresh objects.
function publishLeafGraph(runtime: OrcaRuntimeService, leafPtyId: string): void {
runtime.syncWindowGraph(1, {
tabs: [
{
tabId: 'tab-1',
worktreeId: WORKTREE_ID,
title: 'Codex',
activeLeafId: LEAF_ID,
layout: null
}
],
leaves: [
{
tabId: 'tab-1',
worktreeId: WORKTREE_ID,
leafId: LEAF_ID,
paneRuntimeId: 1,
ptyId: leafPtyId,
paneTitle: null,
title: ''
}
]
})
}
describe('sendTerminal absence gate for leaf-branch writes', () => {
it('rejects only on controller-proven absence and never dispatches the write', async () => {
const probe = vi.fn(async () => false)
const { runtime, handle, write } = await makeRuntimeWithLeafHandle({ probePtyLiveness: probe })
await expect(runtime.sendTerminal(handle, { text: 'ping' })).rejects.toThrow(
'terminal_not_writable'
)
expect(probe).toHaveBeenCalledWith(STALE_PTY_ID)
expect(write).not.toHaveBeenCalled()
})
it('gates agent prompt sends behind the same proven-absence check', async () => {
const probe = vi.fn(async () => false)
const { runtime, handle, write } = await makeRuntimeWithLeafHandle({ probePtyLiveness: probe })
await expect(runtime.sendTerminalAgentPrompt(handle, 'do the thing')).rejects.toThrow(
'terminal_not_writable'
)
expect(write).not.toHaveBeenCalled()
})
it('proceeds when the probe answers unknown (null) — unknown is not absence', async () => {
const { runtime, handle, write } = await makeRuntimeWithLeafHandle({
probePtyLiveness: async () => null
})
await expect(runtime.sendTerminal(handle, { text: 'ping' })).resolves.toMatchObject({
handle,
accepted: true
})
expect(write).toHaveBeenCalledWith(STALE_PTY_ID, 'ping')
})
it('treats a throwing probe as unknown and proceeds', async () => {
const { runtime, handle, write } = await makeRuntimeWithLeafHandle({
probePtyLiveness: async () => {
throw new Error('probe transport down')
}
})
await expect(runtime.sendTerminal(handle, { text: 'ping' })).resolves.toMatchObject({
accepted: true
})
expect(write).toHaveBeenCalledWith(STALE_PTY_ID, 'ping')
})
it('proceeds when the probe answers live (restored session before its pane remounts)', async () => {
const { runtime, handle, write } = await makeRuntimeWithLeafHandle({
probePtyLiveness: async () => true
})
await expect(runtime.sendTerminal(handle, { text: 'ping' })).resolves.toMatchObject({
accepted: true
})
expect(write).toHaveBeenCalledWith(STALE_PTY_ID, 'ping')
})
it('proceeds unchanged when the controller exposes no probe', async () => {
const { runtime, handle, write } = await makeRuntimeWithLeafHandle({})
await expect(runtime.sendTerminal(handle, { text: 'ping' })).resolves.toMatchObject({
accepted: true
})
expect(write).toHaveBeenCalledWith(STALE_PTY_ID, 'ping')
})
it('never probes when the provider synchronously knows the id (live pty)', async () => {
const probe = vi.fn(async () => false)
const { runtime, handle, write } = await makeRuntimeWithLeafHandle({
probePtyLiveness: probe,
hasPty: (ptyId) => ptyId === STALE_PTY_ID
})
await expect(runtime.sendTerminal(handle, { text: 'ping' })).resolves.toMatchObject({
accepted: true
})
expect(probe).not.toHaveBeenCalled()
expect(write).toHaveBeenCalledWith(STALE_PTY_ID, 'ping')
})
it('reuses a proven-absent verdict across repeated sends instead of re-probing', async () => {
const probe = vi.fn(async () => false)
const { runtime, handle } = await makeRuntimeWithLeafHandle({ probePtyLiveness: probe })
await expect(runtime.sendTerminal(handle, { text: 'a' })).rejects.toThrow(
'terminal_not_writable'
)
await expect(runtime.sendTerminal(handle, { text: 'b' })).rejects.toThrow(
'terminal_not_writable'
)
expect(probe).toHaveBeenCalledTimes(1)
})
it('drops the cached absent verdict once the provider re-learns the id', async () => {
const probe = vi.fn(async () => false)
const livePtyIds = new Set<string>()
const { runtime, handle, write } = await makeRuntimeWithLeafHandle({
probePtyLiveness: probe,
hasPty: (ptyId) => livePtyIds.has(ptyId)
})
await expect(runtime.sendTerminal(handle, { text: 'a' })).rejects.toThrow(
'terminal_not_writable'
)
// Same id recreated by a fresh spawn: provider knowledge must beat the verdict.
livePtyIds.add(STALE_PTY_ID)
await expect(runtime.sendTerminal(handle, { text: 'b' })).resolves.toMatchObject({
accepted: true
})
expect(probe).toHaveBeenCalledTimes(1)
expect(write).toHaveBeenCalledWith(STALE_PTY_ID, 'b')
})
})
type StoredMessageRow = {
id: string
run_id: string
from_handle: string
to_handle: string
subject: string
body: string
type: string
priority: string
thread_id: string | null
payload: string | null
read: number
sequence: number
created_at: string
delivered_at: string | null
sender_pane_key: null
}
function makeOrchestrationDbStub(toHandle: () => string) {
const rows: StoredMessageRow[] = []
const markAsDelivered = vi.fn((ids: string[]) => {
for (const row of rows) {
if (ids.includes(row.id)) {
row.delivered_at = 'now'
}
}
})
return {
rows,
markAsDelivered,
insert(subject: string): void {
rows.push({
id: `msg_${rows.length + 1}`,
run_id: 'run_test',
from_handle: 'term_sender',
to_handle: toHandle(),
subject,
body: '',
type: 'status',
priority: 'normal',
thread_id: null,
payload: null,
read: 0,
sequence: rows.length + 1,
created_at: 'now',
delivered_at: null,
sender_pane_key: null
})
},
db: {
getUndeliveredUnreadMessages: (handle: string) =>
rows.filter((row) => row.to_handle === handle && !row.delivered_at),
getActiveCoordinatorRun: () => null,
// Consulted by onPtyExit's dispatch-failure path.
getActiveDispatchForTerminal: () => null,
markAsDelivered,
close: () => {}
}
}
}
describe('push-on-idle orchestration delivery absence gate', () => {
async function makeIdleLeafWithoutPtyRecord(options: {
probePtyLiveness: (ptyId: string) => Promise<boolean | null>
hasPty?: (ptyId: string) => boolean | null
}) {
const { runtime, handle, write } = await makeRuntimeWithLeafHandle(options)
const stub = makeOrchestrationDbStub(() => handle)
runtime.setOrchestrationDb(stub.db as never)
// Title transitions mark the leaf idle; without a hasPty answering true the
// provider never knew this id, modeling a leaf restored from a prior process.
runtime.onPtyData(STALE_PTY_ID, '\x1b]0;Codex working\x07', 100)
runtime.onPtyData(STALE_PTY_ID, '\x1b]0;Codex done\x07', 101)
return { runtime, handle, write, stub }
}
it('keeps messages queued instead of marking a proven-absent pty delivered', async () => {
const { runtime, handle, write, stub } = await makeIdleLeafWithoutPtyRecord({
probePtyLiveness: async () => false
})
stub.insert('lost forever?')
runtime.deliverPendingMessagesForHandle(handle)
await new Promise((resolve) => setTimeout(resolve, 0))
expect(write).not.toHaveBeenCalled()
expect(stub.markAsDelivered).not.toHaveBeenCalled()
expect(stub.rows[0].delivered_at).toBeNull()
})
it('still delivers on unknown liveness after the probe resolves', async () => {
const { runtime, handle, write, stub } = await makeIdleLeafWithoutPtyRecord({
probePtyLiveness: async () => null
})
stub.insert('hello')
runtime.deliverPendingMessagesForHandle(handle)
await new Promise((resolve) => setTimeout(resolve, 0))
expect(write).toHaveBeenCalledWith(STALE_PTY_ID, expect.stringContaining('Subject: hello'))
})
// 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.
it('delivers once across concurrent probe triggers and an in-window re-trigger, then flushes parked rows', async () => {
vi.useFakeTimers()
try {
let resolveProbe!: (value: boolean | null) => void
const { runtime, handle, write, stub } = await makeIdleLeafWithoutPtyRecord({
probePtyLiveness: () =>
new Promise<boolean | null>((resolve) => {
resolveProbe = resolve
})
})
stub.insert('exactly once')
runtime.deliverPendingMessagesForHandle(handle)
runtime.deliverPendingMessagesForHandle(handle)
runtime.deliverPendingMessagesForHandle(handle)
resolveProbe(null)
await vi.advanceTimersByTimeAsync(0)
const firstSubjectWrites = () =>
write.mock.calls.filter(
([, data]) => typeof data === 'string' && data.includes('Subject: exactly once')
)
expect(firstSubjectWrites()).toHaveLength(1)
// 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.)
stub.insert('second message')
runtime.deliverPendingMessagesForHandle(handle)
resolveProbe(null)
await vi.advanceTimersByTimeAsync(0)
expect(firstSubjectWrites()).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.
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)
await vi.advanceTimersByTimeAsync(500)
expect(stub.rows.every((row) => row.delivered_at !== null)).toBe(true)
} finally {
vi.useRealTimers()
}
})
it('sync-path double-trigger inside the Enter window delivers the first batch once and parks the rest', async () => {
vi.useFakeTimers()
try {
const probe = vi.fn(async () => null)
const { runtime, handle, write, stub } = await makeIdleLeafWithoutPtyRecord({
probePtyLiveness: probe,
// Provider knows the id: delivery takes the pure synchronous path.
hasPty: (ptyId) => ptyId === STALE_PTY_ID
})
stub.insert('first')
runtime.deliverPendingMessagesForHandle(handle)
stub.insert('second')
runtime.deliverPendingMessagesForHandle(handle)
const firstSubjectWrites = () =>
write.mock.calls.filter(
([, data]) => typeof data === 'string' && data.includes('Subject: first')
)
expect(firstSubjectWrites()).toHaveLength(1)
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.
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)
await vi.advanceTimersByTimeAsync(500)
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.
it('retires an armed Enter when the pty exits and respawns under the same id inside the window', async () => {
vi.useFakeTimers()
try {
const { runtime, handle, write, stub } = await makeIdleLeafWithoutPtyRecord({
probePtyLiveness: async () => null,
hasPty: (ptyId) => ptyId === STALE_PTY_ID
})
stub.insert('for the old session')
runtime.deliverPendingMessagesForHandle(handle)
expect(write).toHaveBeenCalledTimes(1)
runtime.onPtyExit(STALE_PTY_ID, 0)
runtime.onPtySpawned(STALE_PTY_ID)
await vi.advanceTimersByTimeAsync(500)
expect(write.mock.calls.filter(([, data]) => data === '\r')).toHaveLength(0)
expect(stub.markAsDelivered).not.toHaveBeenCalled()
expect(stub.rows[0].delivered_at).toBeNull()
// The replacement's own delivery starts a fresh flight and completes.
runtime.deliverPendingMessagesForHandle(handle)
const payloadWrites = write.mock.calls.filter(
([, data]) => typeof data === 'string' && data.includes('Subject: for the old session')
)
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()
} finally {
vi.useRealTimers()
}
})
it('cleans all delivery state on exit without id reuse — no leak, no stray settle effects', async () => {
vi.useFakeTimers()
try {
const { runtime, handle, write, stub } = await makeIdleLeafWithoutPtyRecord({
probePtyLiveness: async () => null,
hasPty: (ptyId) => ptyId === STALE_PTY_ID
})
const internals = runtime as unknown as {
messageDeliveryFlightsByPtyId: Map<string, unknown>
parkedMessageRedeliveryLeavesByPtyId: Map<string, unknown>
}
stub.insert('first')
runtime.deliverPendingMessagesForHandle(handle)
stub.insert('second')
runtime.deliverPendingMessagesForHandle(handle)
expect(internals.messageDeliveryFlightsByPtyId.size).toBe(1)
expect(internals.parkedMessageRedeliveryLeavesByPtyId.size).toBe(1)
runtime.onPtyExit(STALE_PTY_ID, 0)
expect(internals.messageDeliveryFlightsByPtyId.size).toBe(0)
expect(internals.parkedMessageRedeliveryLeavesByPtyId.size).toBe(0)
await vi.advanceTimersByTimeAsync(500)
expect(write.mock.calls.filter(([, data]) => data === '\r')).toHaveLength(0)
expect(stub.markAsDelivered).not.toHaveBeenCalled()
// No stray settle flushed the parked trigger into the dead pty.
expect(write).toHaveBeenCalledTimes(1)
expect(stub.rows.every((row) => row.delivered_at === null)).toBe(true)
} finally {
vi.useRealTimers()
}
})
// Why: graph resync replaces leaf objects, so onPtyExit flips writable only
// on the replacement — a callback trusting its closure snapshot would still
// read writable=true and inject Enter after the exit, without any respawn.
it('does not fire a stale Enter through an orphaned leaf snapshot after resync and exit', async () => {
vi.useFakeTimers()
try {
const { runtime, handle, write, stub } = await makeIdleLeafWithoutPtyRecord({
probePtyLiveness: async () => null,
hasPty: (ptyId) => ptyId === STALE_PTY_ID
})
stub.insert('orphaned snapshot')
runtime.deliverPendingMessagesForHandle(handle)
expect(write).toHaveBeenCalledTimes(1)
// Replace the leaf object the armed callback closed over, then exit.
publishLeafGraph(runtime, STALE_PTY_ID)
runtime.onPtyExit(STALE_PTY_ID, 0)
await vi.advanceTimersByTimeAsync(500)
expect(write.mock.calls.filter(([, data]) => data === '\r')).toHaveLength(0)
expect(stub.markAsDelivered).not.toHaveBeenCalled()
expect(stub.rows[0].delivered_at).toBeNull()
} finally {
vi.useRealTimers()
}
})
})