mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 00:03:52 +00:00
fix(orchestration): harden pointer reservation lifecycle
This commit is contained in:
@@ -8809,12 +8809,13 @@
|
||||
"invariant": "Every injected orchestration pointer must select the same mailbox as the exact check from that pane and expose at least the notified unread messages through that mailbox's acknowledgment semantics. Rebinding cannot let foreign direct mail wake a new owner, newer Run mail cannot be announced behind an outstanding fixed Delivery, and restart cannot replay a successfully submitted pointer. Notification records only a durable pointer cursor; it never marks mail read or globally clears it.",
|
||||
"oracle": "Create Run A with unread current-delivery mail addressed directly to its coordinator handle, bind the same pane to Run B, drive a controlled working-to-idle event, and require zero PTY pointers while Run B check returns zero and Run A mail stays unread. Reopen the database and repeat. For same-Run mail, require one submitted pointer, a bound Delivery containing that row, explicit acknowledgment, and no replay after a fresh runtime. Require an active worker's direct mail to route through its Dispatch check, suppress newer Run pointers behind an outstanding Delivery, normalize mail that arrived while its Run was displaced, avoid full mailbox materialization while working, and let bound waiters consume without a duplicate PTY pointer.",
|
||||
"commands": [
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration-mailbox-notification-consistency.test.ts src/main/runtime/orchestration/orchestration-run-delivery-db.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration-mailbox-notification-consistency.test.ts src/main/runtime/orchestration/orchestration-run-delivery-db.test.ts src/main/runtime/orchestration/orchestration-worker-dispatch-db.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/terminal-send-stale-leaf-liveness.test.ts src/main/runtime/rpc/methods/orchestration.test.ts"
|
||||
],
|
||||
"testFiles": [
|
||||
"src/main/runtime/orchestration-mailbox-notification-consistency.test.ts",
|
||||
"src/main/runtime/orchestration/orchestration-run-delivery-db.test.ts",
|
||||
"src/main/runtime/orchestration/orchestration-worker-dispatch-db.test.ts",
|
||||
"src/main/runtime/orca-runtime.test.ts",
|
||||
"src/main/runtime/terminal-send-stale-leaf-liveness.test.ts",
|
||||
"src/main/runtime/rpc/methods/orchestration.test.ts"
|
||||
@@ -8826,6 +8827,8 @@
|
||||
"Run A direct mail cannot point after the pane binds Run B",
|
||||
"a fresh runtime cannot replay the unactionable pointer",
|
||||
"a delayed pointer submit cannot press Enter after the agent becomes working",
|
||||
"a staged pointer follows its Run to a new live pane without cross-PTY duplication",
|
||||
"equivalent panes serialize one mailbox while newer mail arrives during Enter delay",
|
||||
"same-Run direct mail is delivered and acknowledged through the bound Run",
|
||||
"a submitted actionable pointer is durably deduplicated across restart without marking mail read",
|
||||
"newer Run mail is not pointed behind an outstanding fixed Delivery",
|
||||
@@ -8834,6 +8837,12 @@
|
||||
"a bound waiter wakes without a duplicate PTY pointer",
|
||||
"working-agent arrival uses bounded ownership lookup without a full mailbox scan"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/runtime/orchestration/orchestration-worker-dispatch-db.test.ts",
|
||||
"assertions": [
|
||||
"remote Dispatch attachment lookup remains bounded across pane remints and malformed suffix collisions"
|
||||
]
|
||||
}
|
||||
],
|
||||
"evidenceRuns": [
|
||||
@@ -8841,10 +8850,19 @@
|
||||
"date": "2026-08-10",
|
||||
"runner": "local",
|
||||
"platform": "macos",
|
||||
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration-mailbox-notification-consistency.test.ts src/main/runtime/orchestration/orchestration-run-delivery-db.test.ts",
|
||||
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration-mailbox-notification-consistency.test.ts src/main/runtime/orchestration/orchestration-run-delivery-db.test.ts src/main/runtime/orchestration/orchestration-worker-dispatch-db.test.ts",
|
||||
"result": "passed",
|
||||
"durationSeconds": 3.1,
|
||||
"summary": "Candidate passed all 10 mailbox-identity, durable restart, submit-race, bounded-batch, outstanding-Delivery, Dispatch, acknowledgment, waiter, and bounded-arrival scenarios. The identical oracle failed 10/10 on origin/main plus the test commit and again with the implementation commit explicitly reverted, including pointerCount > 0 with bound check count 0."
|
||||
"summary": "31 tests passed across mailbox identity, cross-PTY serialization, live ownership transfer, durable restart, submit races, bounded batches, outstanding Deliveries, Dispatch ownership, acknowledgment, waiters, Run Delivery, and remote pane remints. The original 10-scenario mailbox oracle failed 10/10 on origin/main plus the test commit and again with the implementation commit explicitly reverted, including pointerCount > 0 with bound check count 0."
|
||||
},
|
||||
{
|
||||
"date": "2026-08-10",
|
||||
"runner": "local",
|
||||
"platform": "macos",
|
||||
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/terminal-send-stale-leaf-liveness.test.ts src/main/runtime/rpc/methods/orchestration.test.ts",
|
||||
"result": "passed",
|
||||
"durationSeconds": 10.1,
|
||||
"summary": "1,250 tests passed and 1 was skipped across runtime pointer delivery, PTY retirement and respawn, stale-leaf liveness, and orchestration RPC behavior."
|
||||
}
|
||||
],
|
||||
"runtimeBudget": {
|
||||
|
||||
@@ -19589,7 +19589,7 @@ describe('OrcaRuntimeService', () => {
|
||||
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).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
@@ -19635,7 +19635,7 @@ describe('OrcaRuntimeService', () => {
|
||||
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).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
@@ -19676,7 +19676,7 @@ describe('OrcaRuntimeService', () => {
|
||||
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).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
@@ -34599,7 +34599,7 @@ describe('OrcaRuntimeService', () => {
|
||||
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).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
@@ -34650,7 +34650,7 @@ describe('OrcaRuntimeService', () => {
|
||||
)
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
expect(write).toHaveBeenCalledWith('pty-1', '\r')
|
||||
expect(message.delivered_at).not.toBeNull()
|
||||
expect(message.delivered_at).toEqual(expect.any(String))
|
||||
|
||||
runtime.notifyMessageArrived('run:run_mailbox', 'worker_done')
|
||||
await Promise.resolve()
|
||||
@@ -35062,7 +35062,7 @@ describe('OrcaRuntimeService', () => {
|
||||
expect.stringContaining('You have 1 orchestration message')
|
||||
)
|
||||
await vi.advanceTimersByTimeAsync(600)
|
||||
expect(message.delivered_at).not.toBeNull()
|
||||
expect(message.delivered_at).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
@@ -35109,7 +35109,7 @@ describe('OrcaRuntimeService', () => {
|
||||
expect.stringContaining('You have 1 orchestration message')
|
||||
)
|
||||
await vi.advanceTimersByTimeAsync(600)
|
||||
expect(message.delivered_at).not.toBeNull()
|
||||
expect(message.delivered_at).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
@@ -35215,7 +35215,7 @@ describe('OrcaRuntimeService', () => {
|
||||
'\nYou have 1 orchestration message. Run `orca orchestration check`.\n'
|
||||
)
|
||||
expect(payloads.some((data) => data.includes('reserved completion'))).toBe(false)
|
||||
expect(status.delivered_at).not.toBeNull()
|
||||
expect(status.delivered_at).toEqual(expect.any(String))
|
||||
expect(done.delivered_at).toBeNull()
|
||||
db.close()
|
||||
} finally {
|
||||
@@ -35397,7 +35397,7 @@ describe('OrcaRuntimeService', () => {
|
||||
expect.stringContaining('You have 1 orchestration message')
|
||||
)
|
||||
await vi.advanceTimersByTimeAsync(600)
|
||||
expect(message.delivered_at).not.toBeNull()
|
||||
expect(message.delivered_at).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
@@ -35451,7 +35451,7 @@ describe('OrcaRuntimeService', () => {
|
||||
expect.stringContaining('You have 1 orchestration message')
|
||||
)
|
||||
await vi.advanceTimersByTimeAsync(600)
|
||||
expect(message.delivered_at).not.toBeNull()
|
||||
expect(message.delivered_at).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
@@ -35500,7 +35500,7 @@ describe('OrcaRuntimeService', () => {
|
||||
expect.stringContaining('You have 1 orchestration message')
|
||||
)
|
||||
await vi.advanceTimersByTimeAsync(600)
|
||||
expect(message.delivered_at).not.toBeNull()
|
||||
expect(message.delivered_at).toEqual(expect.any(String))
|
||||
|
||||
// The filtered waiter stays blocked; the push did not consume its wake.
|
||||
await vi.advanceTimersByTimeAsync(5_000)
|
||||
@@ -35639,7 +35639,7 @@ 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).toEqual(expect.any(String))
|
||||
expect(
|
||||
write.mock.calls.filter(
|
||||
([, payload]) =>
|
||||
@@ -35650,7 +35650,7 @@ describe('OrcaRuntimeService', () => {
|
||||
'pty-1',
|
||||
expect.stringContaining('You have 1 orchestration message')
|
||||
)
|
||||
expect(second.delivered_at).not.toBeNull()
|
||||
expect(second.delivered_at).toEqual(expect.any(String))
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
|
||||
@@ -2899,13 +2899,22 @@ export class OrcaRuntimeService {
|
||||
private handles = new Map<string, TerminalHandleRecord>()
|
||||
private handleByLeafKey = new Map<string, string>()
|
||||
private handleByPtyId = new Map<string, string>()
|
||||
// Why: pointer state is process-local; one harmless replay after restart avoids a wire or schema change.
|
||||
// Why: provisional pointer state serializes PTY injection before the durable delivered_at commit.
|
||||
private readonly lastPointedMessageSequenceByHandle = new Map<string, number>()
|
||||
// Why: a waiter can reserve an older row while a newer row advances the sequence watermark.
|
||||
private readonly pointedMessageIdsByHandle = new Map<string, Set<string>>()
|
||||
private readonly mailPointerRepointScheduler = new MailPointerRepointScheduler((handle) =>
|
||||
this.repointPendingMessagesForHandle(handle)
|
||||
)
|
||||
private readonly pointedMessageWatermarkOwnerByHandle = new Map<
|
||||
string,
|
||||
{ ptyId: string; sequence: number; leafKey: string; active: boolean }
|
||||
>()
|
||||
private readonly pointedMessageMailboxHandlesByPtyId = new Map<string, Set<string>>()
|
||||
private readonly parkedMessageRedeliveryTypesByMailboxHandle = new Map<
|
||||
string,
|
||||
ReadonlySet<string> | null
|
||||
>()
|
||||
private syntheticTerminalHandles = new Set<string>()
|
||||
private detachedPreAllocatedLeaves = new Map<string, RuntimeLeafRecord>()
|
||||
private graphSyncCallbacks: (() => void)[] = []
|
||||
@@ -33202,13 +33211,114 @@ export class OrcaRuntimeService {
|
||||
}
|
||||
this.parkedMessageRedeliveriesByPtyId.delete(ptyId)
|
||||
for (const [mailboxHandle, delivery] of parked) {
|
||||
this.deliverPendingMessages(delivery.leaf, {
|
||||
const currentLeaf = this.leaves.get(
|
||||
this.getLeafKey(delivery.leaf.tabId, delivery.leaf.leafId)
|
||||
)
|
||||
if (
|
||||
currentLeaf?.ptyId !== ptyId ||
|
||||
this.resolveActionableMailboxForLeaf(currentLeaf, mailboxHandle) !== mailboxHandle
|
||||
) {
|
||||
this.parkMessageRedeliveryForMailbox(mailboxHandle, delivery.reservedTypes)
|
||||
this.redrivePendingMessageMailbox(mailboxHandle)
|
||||
continue
|
||||
}
|
||||
this.deliverPendingMessages(currentLeaf, {
|
||||
mailboxHandle,
|
||||
reservedTypes: delivery.reservedTypes
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
private setPointedMessageWatermark(
|
||||
mailboxHandle: string,
|
||||
sequence: number,
|
||||
ptyId: string,
|
||||
leafKey: string
|
||||
): void {
|
||||
const prior = this.pointedMessageWatermarkOwnerByHandle.get(mailboxHandle)
|
||||
if (prior && prior.ptyId !== ptyId) {
|
||||
const priorMailboxes = this.pointedMessageMailboxHandlesByPtyId.get(prior.ptyId)
|
||||
priorMailboxes?.delete(mailboxHandle)
|
||||
if (priorMailboxes?.size === 0) {
|
||||
this.pointedMessageMailboxHandlesByPtyId.delete(prior.ptyId)
|
||||
}
|
||||
}
|
||||
this.lastPointedMessageSequenceByHandle.set(mailboxHandle, sequence)
|
||||
this.pointedMessageWatermarkOwnerByHandle.set(mailboxHandle, {
|
||||
ptyId,
|
||||
sequence,
|
||||
leafKey,
|
||||
active: true
|
||||
})
|
||||
const mailboxes = this.pointedMessageMailboxHandlesByPtyId.get(ptyId) ?? new Set<string>()
|
||||
mailboxes.add(mailboxHandle)
|
||||
this.pointedMessageMailboxHandlesByPtyId.set(ptyId, mailboxes)
|
||||
}
|
||||
|
||||
private clearPointedMessageWatermark(
|
||||
mailboxHandle: string,
|
||||
sequence: number,
|
||||
ptyId: string
|
||||
): boolean {
|
||||
const owner = this.pointedMessageWatermarkOwnerByHandle.get(mailboxHandle)
|
||||
if (!owner || owner.ptyId !== ptyId || owner.sequence !== sequence) {
|
||||
return false
|
||||
}
|
||||
this.pointedMessageWatermarkOwnerByHandle.delete(mailboxHandle)
|
||||
if (this.lastPointedMessageSequenceByHandle.get(mailboxHandle) === sequence) {
|
||||
this.lastPointedMessageSequenceByHandle.delete(mailboxHandle)
|
||||
}
|
||||
const mailboxes = this.pointedMessageMailboxHandlesByPtyId.get(ptyId)
|
||||
mailboxes?.delete(mailboxHandle)
|
||||
if (mailboxes?.size === 0) {
|
||||
this.pointedMessageMailboxHandlesByPtyId.delete(ptyId)
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
private deactivatePointedMessageWatermark(
|
||||
mailboxHandle: string,
|
||||
sequence: number,
|
||||
ptyId: string
|
||||
): boolean {
|
||||
const owner = this.pointedMessageWatermarkOwnerByHandle.get(mailboxHandle)
|
||||
if (!owner || owner.ptyId !== ptyId || owner.sequence !== sequence) {
|
||||
return false
|
||||
}
|
||||
owner.active = false
|
||||
return true
|
||||
}
|
||||
|
||||
private parkMessageRedeliveryForMailbox(
|
||||
mailboxHandle: string,
|
||||
reservedTypes: ReadonlySet<string> | undefined
|
||||
): void {
|
||||
const prior = this.parkedMessageRedeliveryTypesByMailboxHandle.get(mailboxHandle)
|
||||
if (prior === null || reservedTypes === undefined) {
|
||||
this.parkedMessageRedeliveryTypesByMailboxHandle.set(mailboxHandle, null)
|
||||
return
|
||||
}
|
||||
this.parkedMessageRedeliveryTypesByMailboxHandle.set(
|
||||
mailboxHandle,
|
||||
new Set([...(prior ?? []), ...reservedTypes])
|
||||
)
|
||||
}
|
||||
|
||||
private redrivePendingMessageMailbox(mailboxHandle: string, force = false): void {
|
||||
const parkedTypes = this.parkedMessageRedeliveryTypesByMailboxHandle.get(mailboxHandle)
|
||||
if (!force && parkedTypes === undefined) {
|
||||
return
|
||||
}
|
||||
this.parkedMessageRedeliveryTypesByMailboxHandle.delete(mailboxHandle)
|
||||
queueMicrotask(() => {
|
||||
try {
|
||||
this.deliverPendingMessagesForHandle(mailboxHandle, parkedTypes ?? undefined)
|
||||
} catch {
|
||||
// The durable row remains available to a later idle edge or explicit check.
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// 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)
|
||||
@@ -33231,6 +33341,17 @@ export class OrcaRuntimeService {
|
||||
this.mailPointerRepointScheduler.schedule(`run:${run.id}`)
|
||||
}
|
||||
}
|
||||
const mailboxes = [...(this.pointedMessageMailboxHandlesByPtyId.get(ptyId) ?? [])]
|
||||
for (const mailboxHandle of mailboxes) {
|
||||
const owner = this.pointedMessageWatermarkOwnerByHandle.get(mailboxHandle)
|
||||
if (
|
||||
owner?.ptyId === ptyId &&
|
||||
this.clearPointedMessageWatermark(mailboxHandle, owner.sequence, ptyId)
|
||||
) {
|
||||
this.redrivePendingMessageMailbox(mailboxHandle, true)
|
||||
}
|
||||
}
|
||||
this.pointedMessageMailboxHandlesByPtyId.delete(ptyId)
|
||||
}
|
||||
|
||||
private canSubmitMessagePointer(
|
||||
@@ -33300,6 +33421,11 @@ export class OrcaRuntimeService {
|
||||
return
|
||||
}
|
||||
|
||||
if (this.pointedMessageWatermarkOwnerByHandle.get(mailboxHandle)?.active) {
|
||||
this.parkMessageRedeliveryForMailbox(mailboxHandle, options.reservedTypes)
|
||||
return
|
||||
}
|
||||
|
||||
// Why filter here and not at the trigger: the push reads every pending row,
|
||||
// not just the one that woke it, so a row a pull has claimed would be typed
|
||||
// into the pane AND returned by that pull's check. Live waiters cover the
|
||||
@@ -33359,6 +33485,19 @@ export class OrcaRuntimeService {
|
||||
if (newestSequence === undefined) {
|
||||
return
|
||||
}
|
||||
const pointedSequence = this.lastPointedMessageSequenceByHandle.get(mailboxHandle) ?? -1
|
||||
if (newestSequence <= pointedSequence) {
|
||||
const owner = this.pointedMessageWatermarkOwnerByHandle.get(mailboxHandle)
|
||||
const leafKey = this.getLeafKey(leaf.tabId, leaf.leafId)
|
||||
if (
|
||||
!owner ||
|
||||
owner.active ||
|
||||
(owner.ptyId === leaf.ptyId && owner.leafKey === leafKey) ||
|
||||
!this.clearPointedMessageWatermark(mailboxHandle, owner.sequence, owner.ptyId)
|
||||
) {
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if (
|
||||
!options.skipAbsenceProbe &&
|
||||
@@ -33437,28 +33576,58 @@ export class OrcaRuntimeService {
|
||||
pointedIdsAfterWrite.add(message.id)
|
||||
}
|
||||
this.pointedMessageIdsByHandle.set(mailboxHandle, pointedIdsAfterWrite)
|
||||
this.setPointedMessageWatermark(
|
||||
mailboxHandle,
|
||||
newestSequence,
|
||||
deliveryPtyId,
|
||||
this.getLeafKey(leaf.tabId, leaf.leafId)
|
||||
)
|
||||
|
||||
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((message) => message.id))
|
||||
let delivered = false
|
||||
try {
|
||||
this._orchestrationDb.markAsDelivered(unread.map((message) => message.id))
|
||||
delivered = true
|
||||
} finally {
|
||||
if (delivered) {
|
||||
this.clearPointedMessageWatermark(mailboxHandle, newestSequence, deliveryPtyId)
|
||||
} else {
|
||||
this.deactivatePointedMessageWatermark(mailboxHandle, newestSequence, deliveryPtyId)
|
||||
}
|
||||
}
|
||||
this.redrivePendingMessageMailbox(mailboxHandle)
|
||||
return
|
||||
}
|
||||
|
||||
// Why: agent TUIs can swallow a \r in the same PTY write; submit separately after a delay.
|
||||
flight.enterTimer = setTimeout(() => {
|
||||
let clearAndRedrive = false
|
||||
let delivered = false
|
||||
let finalizeReservation = true
|
||||
void this.isLeafPtyProvenAbsent(deliveryPtyId)
|
||||
.then((absent) => {
|
||||
// Why current state, not the closure: ownership, graph records, or the
|
||||
// process can change during the Enter delay and liveness probe.
|
||||
if (absent || this.messageDeliveryFlightsByPtyId.get(deliveryPtyId) !== flight) {
|
||||
if (absent) {
|
||||
clearAndRedrive = true
|
||||
return
|
||||
}
|
||||
if (this.messageDeliveryFlightsByPtyId.get(deliveryPtyId) !== flight) {
|
||||
finalizeReservation = false
|
||||
return
|
||||
}
|
||||
const currentLeaf = this.leaves.get(this.getLeafKey(leaf.tabId, leaf.leafId))
|
||||
if (!currentLeaf || currentLeaf.ptyId !== deliveryPtyId || !currentLeaf.writable) {
|
||||
clearAndRedrive = true
|
||||
return
|
||||
}
|
||||
if (this.resolveActionableMailboxForLeaf(currentLeaf) !== mailboxHandle) {
|
||||
clearAndRedrive = true
|
||||
return
|
||||
}
|
||||
if (
|
||||
!currentLeaf ||
|
||||
currentLeaf.ptyId !== deliveryPtyId ||
|
||||
!currentLeaf.writable ||
|
||||
currentLeaf.lastAgentStatus !== 'idle' ||
|
||||
!currentLeaf.lastAgentStatusObservedLive ||
|
||||
!this.canSubmitMessagePointer(currentLeaf, mailboxHandle, unread)
|
||||
@@ -33468,12 +33637,29 @@ export class OrcaRuntimeService {
|
||||
const submitted = this.ptyController?.write(deliveryPtyId, '\r') ?? false
|
||||
if (submitted) {
|
||||
this._orchestrationDb?.markAsDelivered(unread.map((message) => message.id))
|
||||
delivered = true
|
||||
}
|
||||
})
|
||||
.catch(() => {
|
||||
// Terminal liveness is uncertain; mail remains queued for explicit check.
|
||||
})
|
||||
.finally(() => this.settlePendingMessageDelivery(deliveryPtyId, flight))
|
||||
.finally(() => {
|
||||
let released = false
|
||||
if (finalizeReservation) {
|
||||
released =
|
||||
delivered || clearAndRedrive
|
||||
? this.clearPointedMessageWatermark(mailboxHandle, newestSequence, deliveryPtyId)
|
||||
: this.deactivatePointedMessageWatermark(
|
||||
mailboxHandle,
|
||||
newestSequence,
|
||||
deliveryPtyId
|
||||
)
|
||||
}
|
||||
this.settlePendingMessageDelivery(deliveryPtyId, flight)
|
||||
if (released) {
|
||||
this.redrivePendingMessageMailbox(mailboxHandle, clearAndRedrive)
|
||||
}
|
||||
})
|
||||
}, 500)
|
||||
settlesInEnterCallback = true
|
||||
} finally {
|
||||
|
||||
@@ -21,6 +21,12 @@ const LEAF_ID = '22222222-2222-4222-8222-222222222222'
|
||||
const PANE_KEY = `${TAB_ID}:${LEAF_ID}`
|
||||
const PTY_ID = 'pty-mailbox'
|
||||
const TERMINAL_HANDLE = 'term_mailbox_consistency'
|
||||
const SECOND_TAB_ID = '33333333-3333-4333-8333-333333333333'
|
||||
const SECOND_LEAF_ID = '44444444-4444-4444-8444-444444444444'
|
||||
const SECOND_PANE_KEY = `${SECOND_TAB_ID}:${SECOND_LEAF_ID}`
|
||||
const SECOND_PTY_ID = 'pty-mailbox-second'
|
||||
const SECOND_TERMINAL_HANDLE = 'term_mailbox_consistency_second'
|
||||
const SECOND_LAUNCH_TOKEN = 'mailbox-consistency-second-launch'
|
||||
const WORKTREE_ID = 'repo-mailbox::/tmp/mailbox'
|
||||
const LAUNCH_TOKEN = 'mailbox-consistency-launch'
|
||||
const temporaryDirectories: string[] = []
|
||||
@@ -71,7 +77,9 @@ type CheckResult = {
|
||||
function createRuntime(db: OrchestrationDb): Harness {
|
||||
const runtime = new OrcaRuntimeService(null, undefined, {
|
||||
attestAgentHookCompatibilityAuthority: ({ paneKey }) =>
|
||||
paneKey === PANE_KEY ? { paneKey, source: 'current_hook' } : null
|
||||
paneKey === PANE_KEY || paneKey.startsWith(`${SECOND_TAB_ID}:`)
|
||||
? { paneKey, source: 'current_hook' }
|
||||
: null
|
||||
})
|
||||
const write = vi.fn(() => true)
|
||||
runtime.setOrchestrationDb(db)
|
||||
@@ -111,6 +119,62 @@ function createRuntime(db: OrchestrationDb): Harness {
|
||||
return { runtime, write }
|
||||
}
|
||||
|
||||
function registerSecondPane(
|
||||
runtime: OrcaRuntimeService,
|
||||
leafId = SECOND_LEAF_ID,
|
||||
includeFirstPane = true
|
||||
): void {
|
||||
runtime.registerPty(SECOND_PTY_ID, WORKTREE_ID, null, {
|
||||
tabId: SECOND_TAB_ID,
|
||||
leafId,
|
||||
incarnationId: 'mailbox-second-incarnation',
|
||||
agentLaunchAuthority: { launchToken: SECOND_LAUNCH_TOKEN, launchAgent: 'codex' }
|
||||
})
|
||||
runtime.registerPreAllocatedHandleForPty(SECOND_PTY_ID, SECOND_TERMINAL_HANDLE)
|
||||
runtime.syncWindowGraph(1, {
|
||||
tabs: [
|
||||
...(includeFirstPane
|
||||
? [
|
||||
{
|
||||
tabId: TAB_ID,
|
||||
worktreeId: WORKTREE_ID,
|
||||
title: 'Codex',
|
||||
activeLeafId: LEAF_ID,
|
||||
layout: null
|
||||
}
|
||||
]
|
||||
: []),
|
||||
{
|
||||
tabId: SECOND_TAB_ID,
|
||||
worktreeId: WORKTREE_ID,
|
||||
title: 'Codex',
|
||||
activeLeafId: leafId,
|
||||
layout: null
|
||||
}
|
||||
],
|
||||
leaves: [
|
||||
...(includeFirstPane
|
||||
? [
|
||||
{
|
||||
tabId: TAB_ID,
|
||||
worktreeId: WORKTREE_ID,
|
||||
leafId: LEAF_ID,
|
||||
paneRuntimeId: 1,
|
||||
ptyId: PTY_ID
|
||||
}
|
||||
]
|
||||
: []),
|
||||
{
|
||||
tabId: SECOND_TAB_ID,
|
||||
worktreeId: WORKTREE_ID,
|
||||
leafId,
|
||||
paneRuntimeId: 2,
|
||||
ptyId: SECOND_PTY_ID
|
||||
}
|
||||
]
|
||||
})
|
||||
}
|
||||
|
||||
async function driveToLiveIdle(runtime: OrcaRuntimeService): Promise<void> {
|
||||
await runtime.listTerminals()
|
||||
runtime.onPtyData(PTY_ID, '\x1b]0;Codex working\x07', 1)
|
||||
@@ -126,20 +190,27 @@ function pointerCount(write: ReturnType<typeof vi.fn>): number {
|
||||
|
||||
async function checkBoundMailbox(
|
||||
runtime: OrcaRuntimeService,
|
||||
options: { ack?: string; wait?: boolean } = {}
|
||||
options: {
|
||||
ack?: string
|
||||
wait?: boolean
|
||||
terminal?: string
|
||||
paneKey?: string
|
||||
launchToken?: string
|
||||
} = {}
|
||||
): Promise<CheckResult> {
|
||||
const terminal = options.terminal ?? TERMINAL_HANDLE
|
||||
const response = await new RpcDispatcher({ runtime, methods: ORCHESTRATION_METHODS }).dispatch({
|
||||
id: 'req-mailbox-consistency',
|
||||
authToken: 'test-auth-token',
|
||||
method: 'orchestration.check',
|
||||
orchestrationContractVersion: ORCHESTRATION_CONTRACT_VERSION,
|
||||
orchestrationCompatibilityEvidence: {
|
||||
terminalHandle: TERMINAL_HANDLE,
|
||||
paneKey: PANE_KEY,
|
||||
launchToken: LAUNCH_TOKEN
|
||||
terminalHandle: terminal,
|
||||
paneKey: options.paneKey ?? PANE_KEY,
|
||||
launchToken: options.launchToken ?? LAUNCH_TOKEN
|
||||
},
|
||||
params: {
|
||||
terminal: TERMINAL_HANDLE,
|
||||
terminal,
|
||||
...(options.ack ? { ack: options.ack } : {}),
|
||||
...(options.wait ? { wait: true, timeoutMs: 5_000 } : {})
|
||||
}
|
||||
@@ -218,7 +289,91 @@ describe('orchestration notification mailbox consistency', () => {
|
||||
restartedDb.close()
|
||||
})
|
||||
|
||||
it('does not submit a pointer after the agent becomes working', async () => {
|
||||
it('releases a staged pointer when its Run moves to another live pane', async () => {
|
||||
vi.useFakeTimers()
|
||||
const db = createDatabase('orca-mailbox-live-rebind-')
|
||||
const harness = createRuntime(db)
|
||||
const runA = createBoundRun(db, 'Run A')
|
||||
const message = insertDirectRunMessage(db, runA.id, 'Run A completion')
|
||||
await driveToLiveIdle(harness.runtime)
|
||||
expect(pointerCount(harness.write)).toBe(1)
|
||||
|
||||
createBoundRun(db, 'Run B')
|
||||
registerSecondPane(harness.runtime, SECOND_LEAF_ID, false)
|
||||
db.bindRun({
|
||||
runId: runA.id,
|
||||
coordinatorHandle: SECOND_TERMINAL_HANDLE,
|
||||
coordinatorPaneKey: SECOND_PANE_KEY
|
||||
})
|
||||
harness.runtime.onPtyData(SECOND_PTY_ID, '\x1b]0;Codex working\x07', 1)
|
||||
harness.runtime.onPtyData(SECOND_PTY_ID, '\x1b]0;Codex done\x07', 2)
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
|
||||
expect(
|
||||
harness.write.mock.calls.filter(
|
||||
([ptyId, payload]) =>
|
||||
ptyId === SECOND_PTY_ID && String(payload).includes('orca orchestration check')
|
||||
)
|
||||
).toHaveLength(1)
|
||||
expect(
|
||||
harness.write.mock.calls.filter(([ptyId, payload]) => ptyId === PTY_ID && payload === '\r')
|
||||
).toHaveLength(0)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
const checked = await checkBoundMailbox(harness.runtime, {
|
||||
terminal: SECOND_TERMINAL_HANDLE,
|
||||
paneKey: SECOND_PANE_KEY,
|
||||
launchToken: SECOND_LAUNCH_TOKEN
|
||||
})
|
||||
expect(checked).toMatchObject({ runId: runA.id, count: 1 })
|
||||
expect(checked.messages).toEqual([expect.objectContaining({ id: message.id })])
|
||||
db.close()
|
||||
})
|
||||
|
||||
it('serializes equivalent panes while newer mail arrives during Enter delay', async () => {
|
||||
vi.useFakeTimers()
|
||||
const db = createDatabase('orca-mailbox-equivalent-pane-flight-')
|
||||
const harness = createRuntime(db)
|
||||
const run = createBoundRun(db, 'Equivalent pane Run')
|
||||
const first = insertDirectRunMessage(db, run.id, 'First status')
|
||||
await driveToLiveIdle(harness.runtime)
|
||||
expect(pointerCount(harness.write)).toBe(1)
|
||||
|
||||
registerSecondPane(harness.runtime, LEAF_ID)
|
||||
const second = db.insertMessage({
|
||||
from: 'term_worker',
|
||||
to: `run:${run.id}`,
|
||||
subject: 'Second status',
|
||||
runId: run.id
|
||||
})
|
||||
harness.runtime.onPtyData(SECOND_PTY_ID, '\x1b]0;Codex working\x07', 1)
|
||||
harness.runtime.onPtyData(SECOND_PTY_ID, '\x1b]0;Codex done\x07', 2)
|
||||
await Promise.resolve()
|
||||
expect(
|
||||
harness.write.mock.calls.filter(
|
||||
([ptyId, payload]) =>
|
||||
ptyId === SECOND_PTY_ID && String(payload).includes('orca orchestration check')
|
||||
)
|
||||
).toHaveLength(0)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
expect(
|
||||
harness.write.mock.calls.filter(
|
||||
([ptyId, payload]) =>
|
||||
ptyId === PTY_ID && String(payload).includes('orca orchestration check')
|
||||
)
|
||||
).toHaveLength(2)
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
|
||||
const checked = await checkBoundMailbox(harness.runtime)
|
||||
expect(checked).toMatchObject({ runId: run.id, count: 2 })
|
||||
expect(checked.messages).toEqual(
|
||||
[first, second].map((message) => expect.objectContaining({ id: message.id }))
|
||||
)
|
||||
db.close()
|
||||
})
|
||||
|
||||
it('does not submit or duplicate a pointer after the agent becomes working', async () => {
|
||||
vi.useFakeTimers()
|
||||
const db = createDatabase('orca-mailbox-working-before-enter-')
|
||||
const harness = createRuntime(db)
|
||||
@@ -232,6 +387,9 @@ describe('orchestration notification mailbox consistency', () => {
|
||||
|
||||
expect(harness.write.mock.calls.filter(([, payload]) => payload === '\r')).toHaveLength(0)
|
||||
expect(db.getMessageById(message.id)?.delivered_at).toBeNull()
|
||||
harness.runtime.onPtyData(PTY_ID, '\x1b]0;Codex done\x07', 4)
|
||||
await Promise.resolve()
|
||||
expect(pointerCount(harness.write)).toBe(1)
|
||||
db.close()
|
||||
})
|
||||
|
||||
@@ -249,6 +407,8 @@ describe('orchestration notification mailbox consistency', () => {
|
||||
})
|
||||
)
|
||||
const notificationQuery = vi.spyOn(db, 'getUndeliveredUnreadMessages')
|
||||
const submitRevalidation = vi.spyOn(db, 'areUndeliveredUnreadMessages')
|
||||
const prepare = vi.spyOn(sqliteFor(db), 'prepare')
|
||||
|
||||
await driveToLiveIdle(harness.runtime)
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
@@ -260,13 +420,19 @@ describe('orchestration notification mailbox consistency', () => {
|
||||
expect.stringContaining('You have 50 orchestration messages')
|
||||
)
|
||||
expect(notificationQuery.mock.results[0]?.value).toHaveLength(50)
|
||||
expect(submitRevalidation).toHaveBeenCalledWith(
|
||||
`run:${run.id}`,
|
||||
messages.slice(0, 50).map((message) => message.id)
|
||||
)
|
||||
const revalidationSql = prepare.mock.calls
|
||||
.map(([sql]) => sql)
|
||||
.find((sql) => sql.includes('SELECT COUNT(*)') && sql.includes('id IN'))
|
||||
expect(revalidationSql).toContain('INDEXED BY idx_messages_id')
|
||||
const revalidationPlan = sqliteFor(db)
|
||||
.prepare(
|
||||
`EXPLAIN QUERY PLAN SELECT COUNT(*) FROM messages INDEXED BY idx_messages_id
|
||||
WHERE to_handle = ? AND read = 0 AND delivered_at IS NULL
|
||||
AND delivery_contract = 'current_delivery' AND id IN (?)`
|
||||
)
|
||||
.all(`run:${run.id}`, messages[0]!.id) as { detail: string }[]
|
||||
.prepare(`EXPLAIN QUERY PLAN ${revalidationSql}`)
|
||||
.all(`run:${run.id}`, ...messages.slice(0, 50).map((message) => message.id)) as {
|
||||
detail: string
|
||||
}[]
|
||||
expect(revalidationPlan.map((row) => row.detail).join(' ')).toContain('idx_messages_id')
|
||||
expect(checked).toMatchObject({ runId: run.id, count: 50 })
|
||||
expect(checked.deliveryId).toBeTruthy()
|
||||
@@ -384,6 +550,16 @@ describe('orchestration notification mailbox consistency', () => {
|
||||
read: 1,
|
||||
delivered_at: expect.any(String)
|
||||
})
|
||||
const internals = harness.runtime as unknown as {
|
||||
lastPointedMessageSequenceByHandle: Map<string, number>
|
||||
pointedMessageWatermarkOwnerByHandle: Map<string, unknown>
|
||||
pointedMessageMailboxHandlesByPtyId: Map<string, Set<string>>
|
||||
}
|
||||
expect(internals.lastPointedMessageSequenceByHandle.has(`dispatch:${dispatch.id}`)).toBe(false)
|
||||
expect(internals.pointedMessageWatermarkOwnerByHandle.has(`dispatch:${dispatch.id}`)).toBe(
|
||||
false
|
||||
)
|
||||
expect(internals.pointedMessageMailboxHandlesByPtyId.has(PTY_ID)).toBe(false)
|
||||
db.close()
|
||||
})
|
||||
|
||||
|
||||
@@ -5124,15 +5124,18 @@ export class OrchestrationDb {
|
||||
if (exact) {
|
||||
return exact
|
||||
}
|
||||
const rows = this.db
|
||||
if (!parsePaneKey(paneKey)) {
|
||||
return undefined
|
||||
}
|
||||
return this.db
|
||||
.prepare(
|
||||
`SELECT * FROM remote_dispatch_attachments
|
||||
WHERE state IN ('starting', 'ready') AND pane_key IS NOT NULL
|
||||
AND instr(pane_key, ':') > 1
|
||||
AND ${REMOTE_ATTACHMENT_PANE_KEY_MATCH_SUFFIX_SQL} = ?
|
||||
ORDER BY rowid DESC`
|
||||
ORDER BY rowid DESC LIMIT 1`
|
||||
)
|
||||
.all(paneKeyMatchSuffix(paneKey)) as RemoteDispatchAttachmentRow[]
|
||||
return rows.find((row) => row.pane_key && isEquivalentPaneKey(row.pane_key, paneKey))
|
||||
.get(paneKeyMatchSuffix(paneKey)) as RemoteDispatchAttachmentRow | undefined
|
||||
}
|
||||
|
||||
enqueueFederationRelay(params: {
|
||||
|
||||
@@ -288,6 +288,48 @@ describe('OrchestrationDb worker Dispatch state', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('bounds remote attachment lookup across pane remints and malformed suffix collisions', () => {
|
||||
const d = createDb()
|
||||
const leafId = '11111111-1111-4111-8111-111111111111'
|
||||
const attach = (dispatchId: string, paneKey: string): void => {
|
||||
d.createRemoteDispatchAttachment({
|
||||
dispatchId,
|
||||
taskId: `task_${dispatchId}`,
|
||||
homePeerFingerprint: 'home_peer',
|
||||
protocolVersion: 1,
|
||||
runtimeEpoch: 'worker_epoch',
|
||||
mutationReceipt: {
|
||||
callerFingerprint: 'home_peer',
|
||||
requestId: `request_${dispatchId}`,
|
||||
method: 'orchestration.federationAttachStart',
|
||||
payloadHash: `payload_${dispatchId}`
|
||||
}
|
||||
})
|
||||
d.prepareRemoteAttachmentAuthority({
|
||||
dispatchId,
|
||||
paneKey,
|
||||
processIncarnation: `process_${dispatchId}`,
|
||||
worktreeId: 'repo::worktree',
|
||||
terminalHandle: `term_${dispatchId}`,
|
||||
setupState: 'not_applicable',
|
||||
effects: []
|
||||
})
|
||||
}
|
||||
|
||||
attach('ctx_valid_old', `tab_old:${leafId}`)
|
||||
for (let index = 0; index < 64; index += 1) {
|
||||
attach(`ctx_malformed_${index}`, `:${leafId}`)
|
||||
}
|
||||
|
||||
expect(d.findActiveRemoteAttachmentForPane(`tab_reminted:${leafId}`)?.dispatch_id).toBe(
|
||||
'ctx_valid_old'
|
||||
)
|
||||
attach('ctx_valid_new', `tab_new:${leafId}`)
|
||||
expect(d.findActiveRemoteAttachmentForPane(`tab_reminted:${leafId}`)?.dispatch_id).toBe(
|
||||
'ctx_valid_new'
|
||||
)
|
||||
})
|
||||
|
||||
it('returns already-settled when completion wins before stop', () => {
|
||||
const d = createDb()
|
||||
const task = d.createTask({ spec: 'race' })
|
||||
|
||||
@@ -504,7 +504,9 @@ describe('push-on-idle orchestration delivery absence gate', () => {
|
||||
resolveProbe(null)
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
expect(stub.markAsDelivered).toHaveBeenCalledTimes(2)
|
||||
expect(stub.rows.every((row) => row.delivered_at !== null)).toBe(true)
|
||||
expect(stub.rows.map((row) => row.delivered_at)).toEqual(
|
||||
stub.rows.map(() => expect.any(String))
|
||||
)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
@@ -541,7 +543,9 @@ describe('push-on-idle orchestration delivery absence gate', () => {
|
||||
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
expect(stub.markAsDelivered).toHaveBeenCalledTimes(2)
|
||||
expect(stub.rows.every((row) => row.delivered_at !== null)).toBe(true)
|
||||
expect(stub.rows.map((row) => row.delivered_at)).toEqual(
|
||||
stub.rows.map(() => expect.any(String))
|
||||
)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
@@ -587,7 +591,7 @@ describe('push-on-idle orchestration delivery absence gate', () => {
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
expect(write.mock.calls.filter(([, data]) => data === '\r')).toHaveLength(1)
|
||||
expect(stub.markAsDelivered).toHaveBeenCalledOnce()
|
||||
expect(stub.rows[0].delivered_at).not.toBeNull()
|
||||
expect(stub.rows[0].delivered_at).toEqual(expect.any(String))
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user