From b6e7ab997ffaa2d176bb02a7da92100478f35cf0 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 9 Sep 2026 23:30:34 -0400 Subject: [PATCH 1/9] fix(orchestration): retire read deliveries and clarify mailbox recovery --- src/cli/specs/orchestration.ts | 4 +- .../db/messages/message-inbox.ts | 17 ++- .../db/messages/role-mailbox-delivery.ts | 63 ++++++-- .../runtime/orchestration/formatter.test.ts | 2 +- src/main/runtime/orchestration/formatter.ts | 2 +- .../orchestration-retired-delivery.test.ts | 142 ++++++++++++++++++ .../messaging/check-delivery-history.test.ts | 33 ++++ .../orchestration/messaging/check-run.ts | 1 + src/shared/orchestration-check-output.test.ts | 21 ++- src/shared/orchestration-check-output.ts | 2 +- 10 files changed, 266 insertions(+), 21 deletions(-) create mode 100644 src/main/runtime/orchestration/orchestration-retired-delivery.test.ts create mode 100644 src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts diff --git a/src/cli/specs/orchestration.ts b/src/cli/specs/orchestration.ts index e62911b2c767..3f63ff3a47d0 100644 --- a/src/cli/specs/orchestration.ts +++ b/src/cli/specs/orchestration.ts @@ -110,8 +110,10 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [ notes: [ 'On Windows PowerShell, quote comma-separated type filters, e.g. --types "worker_done,escalation".', '--types is the wake condition for --wait; a returned Delivery is always the whole FIFO batch, so it is never filtered by type. Only --peek and --all filter their rows.', + 'Without --wait, --types does not filter a consuming check. An outstanding Delivery replays before the wake condition is tested.', + '--all shows the outstanding deliveryId for recovery; its history rows are not necessarily that batch. Check the full Delivery before --ack.', '--format renders the returned rows as local text only; it never writes to another terminal.', - 'A bound Run replays the same Delivery until --ack; process every message before acknowledging.' + 'A bound Run replays the same Delivery until --ack or all its messages are retired; process every message before acknowledging.' ] }, { diff --git a/src/main/runtime/orchestration/db/messages/message-inbox.ts b/src/main/runtime/orchestration/db/messages/message-inbox.ts index e9b800d431e4..1ef57776b008 100644 --- a/src/main/runtime/orchestration/db/messages/message-inbox.ts +++ b/src/main/runtime/orchestration/db/messages/message-inbox.ts @@ -9,7 +9,8 @@ const MESSAGE_MUTATION_SAVEPOINT = 'message_id_mutation' function runBatchedMessageMutation( db: OrchestrationDb, ids: string[], - sqlForPlaceholders: (placeholders: string) => string + sqlForPlaceholders: (placeholders: string) => string, + retireReadDeliveries = false ): void { if (ids.length === 0) { return @@ -20,6 +21,14 @@ function runBatchedMessageMutation( const batch = ids.slice(offset, offset + MESSAGE_ID_UPDATE_BATCH_SIZE) const placeholders = batch.map(() => '?').join(',') db.db.prepare(sqlForPlaceholders(placeholders)).run(...batch) + if (retireReadDeliveries) { + const mailboxes = db.db + .prepare(`SELECT DISTINCT to_handle FROM messages WHERE id IN (${placeholders})`) + .all(...batch) as { to_handle: string }[] + for (const mailbox of mailboxes) { + db.retireReadMailboxDelivery(mailbox.to_handle) + } + } } db.db.exec(`RELEASE ${MESSAGE_MUTATION_SAVEPOINT}`) } catch (error) { @@ -160,7 +169,8 @@ export function markAsRead(this: OrchestrationDb, ids: string[]): void { `UPDATE messages SET read = 1, pointer_enter_pending = 0, pointer_pty_id = NULL, pointer_process_incarnation = NULL - WHERE id IN (${placeholders})` + WHERE id IN (${placeholders})`, + true ) } @@ -216,7 +226,8 @@ export function markAsReadAndDelivered(this: OrchestrationDb, ids: string[]): vo SET read = 1, delivered_at = COALESCE(delivered_at, datetime('now')), pointer_enter_pending = 0, pointer_pty_id = NULL, pointer_process_incarnation = NULL - WHERE id IN (${placeholders})` + WHERE id IN (${placeholders})`, + true ) } diff --git a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts index c7553c089b7b..8a5968c36f25 100644 --- a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts +++ b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts @@ -1,6 +1,6 @@ import type { DeliveryRow, MessageRow, MessageType } from '../../types' import { OrchestrationError } from '../../orchestration-error' -import { generateId } from '../generated-id' +import { generateId, isGeneratedId } from '../generated-id' import type { OrchestrationDb } from '../orchestration-db' import { exposeDeliveryTimestamps, exposeMessageListTimestamps } from '../utc-timestamp' import { ORCHESTRATION_DELIVERY_BATCH_LIMIT } from './mailbox-routing-page' @@ -43,9 +43,7 @@ export function getOrCreateMailboxDelivery( if (params.requireCurrentRunConsumer) { this.requireCurrentConsumer(params.runId, params.consumerGeneration) } - const existing = this.db - .prepare("SELECT * FROM deliveries WHERE mailbox_handle = ? AND status = 'outstanding'") - .get(params.mailboxHandle) as DeliveryRow | undefined + const existing = this.getOutstandingMailboxDelivery(params.mailboxHandle) if (existing) { if (existing.consumer_generation !== params.consumerGeneration) { throw new OrchestrationError( @@ -54,8 +52,11 @@ export function getOrCreateMailboxDelivery( ) } const messages = this.getDeliveryMessages(existing) - this.db.exec('COMMIT') - return { delivery: exposeDeliveryTimestamps(existing), messages, replayed: true } + if (messages.some((message) => message.read === 0)) { + this.db.exec('COMMIT') + return { delivery: exposeDeliveryTimestamps(existing), messages, replayed: true } + } + this.retireReadMailboxDelivery(params.mailboxHandle) } if (params.wakeTypes?.length) { const placeholders = params.wakeTypes.map(() => '?').join(',') @@ -124,6 +125,20 @@ export function acknowledgeMailboxDelivery( if (params.requireCurrentRunConsumer) { this.requireCurrentConsumer(params.runId, params.consumerGeneration) } + if (isGeneratedId(params.deliveryId, 'msg')) { + const outstanding = this.getOutstandingMailboxDelivery(params.mailboxHandle) + const containsMessage = + outstanding && + outstanding.run_id === params.runId && + (JSON.parse(outstanding.message_ids) as string[]).includes(params.deliveryId) + const guidance = containsMessage + ? `Process the entire batch, then use --ack ${outstanding.id}.` + : 'Run orchestration check to obtain the Delivery id.' + throw new OrchestrationError( + 'stale_delivery', + `--ack takes a Delivery id, not message id ${params.deliveryId}. ${guidance}` + ) + } const delivery = this.getDeliveryRaw(params.deliveryId) if ( !delivery || @@ -174,17 +189,35 @@ export function acknowledgeMailboxDelivery( } } +export function getOutstandingMailboxDelivery( + this: OrchestrationDb, + mailboxHandle: string +): DeliveryRow | undefined { + const row = this.db + .prepare("SELECT * FROM deliveries WHERE mailbox_handle = ? AND status = 'outstanding'") + .get(mailboxHandle) as DeliveryRow | undefined + return row ? exposeDeliveryTimestamps(row) : undefined +} + +// Acknowledgment keeps late consumer acks idempotent after lifecycle suppression. +export function retireReadMailboxDelivery(this: OrchestrationDb, mailboxHandle: string): void { + this.db + .prepare( + `UPDATE deliveries SET status = 'acknowledged', acknowledged_at = datetime('now') + WHERE mailbox_handle = ? AND status = 'outstanding' + AND NOT EXISTS ( + SELECT 1 FROM json_each(deliveries.message_ids) AS member + JOIN messages ON messages.id = member.value WHERE messages.read = 0 + )` + ) + .run(mailboxHandle) +} + export function hasOutstandingMailboxDelivery( this: OrchestrationDb, mailboxHandle: string ): boolean { - return Boolean( - this.db - .prepare( - "SELECT 1 FROM deliveries WHERE mailbox_handle = ? AND status = 'outstanding' LIMIT 1" - ) - .get(mailboxHandle) - ) + return this.getOutstandingMailboxDelivery(mailboxHandle) !== undefined } export function fenceOutstandingMailboxDelivery( @@ -199,6 +232,8 @@ export function fenceOutstandingMailboxDelivery( } export type RoleMailboxDeliveryMethods = { + getOutstandingMailboxDelivery: typeof getOutstandingMailboxDelivery + retireReadMailboxDelivery: typeof retireReadMailboxDelivery getDeliveryRaw: typeof getDeliveryRaw getDeliveryMessages: typeof getDeliveryMessages getOrCreateMailboxDelivery: typeof getOrCreateMailboxDelivery @@ -209,6 +244,8 @@ export type RoleMailboxDeliveryMethods = { export function attachRoleMailboxDelivery(ctor: { prototype: object }): void { Object.assign(ctor.prototype, { + getOutstandingMailboxDelivery, + retireReadMailboxDelivery, getDeliveryRaw, getDeliveryMessages, getOrCreateMailboxDelivery, diff --git a/src/main/runtime/orchestration/formatter.test.ts b/src/main/runtime/orchestration/formatter.test.ts index 148bfad9b738..e42b80f3f46c 100644 --- a/src/main/runtime/orchestration/formatter.test.ts +++ b/src/main/runtime/orchestration/formatter.test.ts @@ -187,7 +187,7 @@ describe('formatMessagesForInjection', () => { describe('formatMessagePointer', () => { it('formats a singular pointer without message content', () => { expect(formatMessagePointer(1, 'run:run_1')).toBe( - '\nYou have 1 orchestration message. Run `orca orchestration check --run run_1`.\n' + '\n[Orca orchestration notification] You have 1 orchestration message. Run `orca orchestration check --run run_1`.\n' ) }) diff --git a/src/main/runtime/orchestration/formatter.ts b/src/main/runtime/orchestration/formatter.ts index 2dd4f86777bb..540e1def1ba2 100644 --- a/src/main/runtime/orchestration/formatter.ts +++ b/src/main/runtime/orchestration/formatter.ts @@ -118,5 +118,5 @@ export function formatMessagePointer( const runFlag = mailboxHandle?.startsWith('run:') ? ` --run ${mailboxHandle.slice('run:'.length)}` : '' - return `\nYou have ${count} orchestration ${noun}. Run \`${cliCommand} orchestration check${runFlag}\`.\n` + return `\n[Orca orchestration notification] You have ${count} orchestration ${noun}. Run \`${cliCommand} orchestration check${runFlag}\`.\n` } diff --git a/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts b/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts new file mode 100644 index 000000000000..820f69d147c8 --- /dev/null +++ b/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts @@ -0,0 +1,142 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { OrchestrationDb } from './db' +import { createRootDispatch } from './db/root-dispatch-test-fixture' +import { reconcileLifecycleMessage } from './lifecycle-reconciliation' + +describe('retired mailbox deliveries', () => { + let db: OrchestrationDb + afterEach(() => db?.close()) + + function setup() { + db = new OrchestrationDb(':memory:') + const run = db.createRun({ + objective: 'Retired delivery', + coordinatorHandle: 'term_coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const params = { runId: run.id, consumerGeneration: run.consumer_generation } + const insert = (subject: string) => + db.insertMessage({ runId: run.id, from: 'worker', to: `run:${run.id}`, subject }) + return { run, params, insert } + } + + it('retires the heartbeat delivery atomically when completion suppresses its contents', () => { + const { run, params } = setup() + const task = db.createTask({ runId: run.id, spec: 'work' }) + const dispatch = createRootDispatch(db, task.id, 'worker') + const insert = (type: 'heartbeat' | 'worker_done') => + db.insertMessage({ + runId: run.id, + from: 'worker', + to: `run:${run.id}`, + subject: type, + type, + payload: JSON.stringify({ taskId: task.id, dispatchId: dispatch.id, outcome: 'succeeded' }) + }) + insert('heartbeat') + const first = db.getOrCreateRunDelivery(params)! + const done = insert('worker_done') + expect(reconcileLifecycleMessage(db, done).action).toBe('completed') + expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('acknowledged') + expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) + expect( + db + .getOrCreateRunDelivery({ ...params, wakeTypes: ['worker_done'] }) + ?.messages.map((m) => m.id) + ).toEqual([done.id]) + expect(db.acknowledgeRunDelivery({ ...params, deliveryId: first.delivery.id }).duplicate).toBe( + true + ) + }) + + it('repairs an already fully-read outstanding delivery before replay', () => { + const { params, insert } = setup() + const old = insert('old') + const first = db.getOrCreateRunDelivery(params)! + db.db.prepare('UPDATE messages SET read = 1 WHERE id = ?').run(old.id) + const next = insert('next') + const current = db.getOrCreateRunDelivery(params)! + expect(current.messages.map((m) => m.id)).toEqual([next.id]) + expect(current.replayed).toBe(false) + expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('acknowledged') + }) + + it('preserves the entire replay batch while any member is unread', () => { + const { params, insert } = setup() + const a = insert('a') + const b = insert('b') + const first = db.getOrCreateRunDelivery(params)! + db.markAsReadAndDelivered([a.id]) + insert('later') + const replay = db.getOrCreateRunDelivery(params)! + expect(replay.delivery.id).toBe(first.delivery.id) + expect(replay.messages.map((m) => m.id)).toEqual([a.id, b.id]) + expect(replay.replayed).toBe(true) + }) + + it('rolls retirement back with the enclosing lifecycle transaction', () => { + const { params, insert } = setup() + const message = insert('old') + const first = db.getOrCreateRunDelivery(params)! + db.db.exec('BEGIN') + db.markAsReadAndDelivered([message.id]) + expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('acknowledged') + db.db.exec('ROLLBACK') + expect(db.getMessageById(message.id)?.read).toBe(0) + expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') + }) + + it('names the owning delivery when ack receives a message ID without consuming mail', () => { + const { params, insert } = setup() + const message = insert('pending') + const first = db.getOrCreateRunDelivery(params)! + expect(() => db.acknowledgeRunDelivery({ ...params, deliveryId: message.id })).toThrow( + `Process the entire batch, then use --ack ${first.delivery.id}.` + ) + expect(db.getMessageById(message.id)?.read).toBe(0) + expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') + }) + + it('does not reveal a delivery from another mailbox in a wrong-ID error', () => { + const { params, insert } = setup() + const foreign = db.createRun({ + objective: 'other', + coordinatorHandle: 'other', + coordinatorPaneKey: 'other:22222222-2222-4222-9222-222222222222' + }) + const message = insert('private') + const first = db.getOrCreateRunDelivery(params)! + expect(() => + db.acknowledgeRunDelivery({ + runId: foreign.id, + consumerGeneration: foreign.consumer_generation, + deliveryId: message.id + }) + ).toThrow('Run orchestration check to obtain the Delivery id.') + expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') + }) + + it.each(['markAsRead', 'markAsReadAndDelivered'] as const)( + '%s retires dispatch mail without retiring a different mailbox', + (method) => { + const { run, params, insert } = setup() + insert('coordinator mail') + const coordinator = db.getOrCreateRunDelivery(params)! + const task = db.createTask({ runId: run.id, spec: 'worker mail' }) + const dispatch = createRootDispatch(db, task.id, 'worker') + const mailboxHandle = `dispatch:${dispatch.id}` + const message = db.insertMessage({ + runId: run.id, + from: 'term_coord', + to: mailboxHandle, + subject: 'worker mail' + }) + const workerParams = { ...params, mailboxHandle } + const worker = db.getOrCreateMailboxDelivery(workerParams)! + db[method]([message.id]) + expect(db.getDeliveryRaw(worker.delivery.id)?.status).toBe('acknowledged') + expect(db.getOrCreateMailboxDelivery(workerParams)).toBeUndefined() + expect(db.getDeliveryRaw(coordinator.delivery.id)?.status).toBe('outstanding') + } + ) +}) diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts b/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts new file mode 100644 index 000000000000..59a0718b66ad --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts @@ -0,0 +1,33 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { createOrchestrationRpcHarness } from '../rpc-test-harness' + +describe('Run delivery history', () => { + const h = createOrchestrationRpcHarness() + afterEach(() => h.cleanup()) + + it('exposes the outstanding delivery without minting or acknowledging it', async () => { + const { db, ctx, activeRunId } = h.setup() + const params = { terminal: 'term_coord', run: activeRunId, all: true } + db.insertMessage({ + from: 'worker', + to: `run:${activeRunId}`, + runId: activeRunId, + subject: 'waiting' + }) + expect(await h.call('orchestration.check', params, ctx)).toMatchObject({ + deliveryId: null, + count: 1 + }) + expect(db.hasOutstandingRunDelivery(activeRunId!)).toBe(false) + const delivery = db.getOrCreateRunDelivery({ + runId: activeRunId!, + consumerGeneration: db.getRun(activeRunId!)!.consumer_generation + })! + expect(await h.call('orchestration.check', { ...params, format: true }, ctx)).toMatchObject({ + deliveryId: delivery.delivery.id, + count: 1 + }) + expect(db.getDeliveryRaw(delivery.delivery.id)?.status).toBe('outstanding') + expect(db.getMessageById(delivery.messages[0].id)?.read).toBe(0) + }) +}) diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts b/src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts index 6db89cf4a201..598c080623a7 100644 --- a/src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts +++ b/src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts @@ -101,6 +101,7 @@ export async function checkRunMailbox(args: { const result = { messages: exposeMessages(messages), count: messages.length, + deliveryId: db.getOutstandingMailboxDelivery(address)?.id ?? null, acknowledged: acknowledged?.delivery.id ?? null } if (params.format || params.inject) { diff --git a/src/shared/orchestration-check-output.test.ts b/src/shared/orchestration-check-output.test.ts index 00ffbb79da3a..df3248bd2fdb 100644 --- a/src/shared/orchestration-check-output.test.ts +++ b/src/shared/orchestration-check-output.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { prepareOrchestrationCheckOutput } from './orchestration-check-output' +import { + formatOrchestrationCheckText, + prepareOrchestrationCheckOutput +} from './orchestration-check-output' describe('prepareOrchestrationCheckOutput', () => { it('keeps mixed read-only mail safe and current Run replies executable', () => { @@ -40,3 +43,19 @@ describe('prepareOrchestrationCheckOutput', () => { expect(prepared.formatted).not.toContain('unsafe stale formatter output') }) }) + +describe('formatted delivery acknowledgment', () => { + it('retains the delivery ID above formatted message bodies', () => { + expect( + formatOrchestrationCheckText( + { + messages: [{ id: 'msg_one', from_handle: 'worker' }], + count: 1, + deliveryId: 'delivery_one', + formatted: 'Message body' + }, + 'term_coord' + ) + ).toBe('Delivery delivery_one\nMessage body') + }) +}) diff --git a/src/shared/orchestration-check-output.ts b/src/shared/orchestration-check-output.ts index e736b92f4dda..5429c5423410 100644 --- a/src/shared/orchestration-check-output.ts +++ b/src/shared/orchestration-check-output.ts @@ -81,7 +81,7 @@ export function formatOrchestrationCheckText( : '' const deliveryNotice = formatCurrentDeliveryNotice(prepared.legacyCompatibility?.currentDelivery) if (prepared.formatted) { - return `${legacyHeader}${prepared.formatted}${deliveryNotice}` + return `${legacyHeader}${prepared.deliveryId ? `Delivery ${prepared.deliveryId}\n` : ''}${prepared.formatted}${deliveryNotice}` } if (prepared.count === 0) { if (prepared.timedOut) { From 733b18a1c19378f6e4772e83993d957eeb27f848 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 10 Sep 2026 01:10:08 -0400 Subject: [PATCH 2/9] fix(orchestration): simplify delivery recovery and update nudge contracts --- src/cli/specs/orchestration.ts | 6 +-- ...creation-and-orchestration-part-02.spec.ts | 4 +- ...creation-and-orchestration-part-03.spec.ts | 2 +- .../db/messages/role-mailbox-delivery.ts | 52 ++++++------------- .../orchestration-retired-delivery.test.ts | 26 ++++------ .../messaging/check-delivery-history.test.ts | 23 ++++++-- .../orchestration/messaging/check-run.ts | 1 - 7 files changed, 51 insertions(+), 63 deletions(-) diff --git a/src/cli/specs/orchestration.ts b/src/cli/specs/orchestration.ts index 3f63ff3a47d0..7ff110aacefc 100644 --- a/src/cli/specs/orchestration.ts +++ b/src/cli/specs/orchestration.ts @@ -109,11 +109,9 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [ ], notes: [ 'On Windows PowerShell, quote comma-separated type filters, e.g. --types "worker_done,escalation".', - '--types is the wake condition for --wait; a returned Delivery is always the whole FIFO batch, so it is never filtered by type. Only --peek and --all filter their rows.', - 'Without --wait, --types does not filter a consuming check. An outstanding Delivery replays before the wake condition is tested.', - '--all shows the outstanding deliveryId for recovery; its history rows are not necessarily that batch. Check the full Delivery before --ack.', + '--types is the wake condition for --wait; a returned Delivery is always the whole FIFO batch, so it is never filtered by type. Without --wait it has no effect on consuming checks. Only --peek and --all filter their rows.', '--format renders the returned rows as local text only; it never writes to another terminal.', - 'A bound Run replays the same Delivery until --ack or all its messages are retired; process every message before acknowledging.' + 'A bound Run replays the same Delivery until --ack or all its messages are marked read, even with --types; process every message before acknowledging.' ] }, { diff --git a/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.ts b/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.ts index 9291c30c1a2e..fb5396bf37c7 100644 --- a/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.ts +++ b/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.ts @@ -94,7 +94,7 @@ describe('OrcaRuntimeService', () => { runtime.onPtyData('pty-1', '\x1b]0;Codex done\x07', 101) expect(write).toHaveBeenCalledWith( 'pty-1', - '\nYou have 1 orchestration message. Run `orca-dev orchestration check --run run_mailbox`.\n' + '\n[Orca orchestration notification] You have 1 orchestration message. Run `orca-dev orchestration check --run run_mailbox`.\n' ) expect(write).not.toHaveBeenCalledWith( 'pty-1', @@ -472,7 +472,7 @@ describe('OrcaRuntimeService', () => { await vi.waitFor(() => { expect(write).toHaveBeenCalledWith( 'pty-1', - '\nYou have 1 orchestration message. Run `orca-dev orchestration check --run run_codex_native_title`.\n' + '\n[Orca orchestration notification] You have 1 orchestration message. Run `orca-dev orchestration check --run run_codex_native_title`.\n' ) }) db.close() diff --git a/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec.ts b/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec.ts index 48cd70703e08..ec36fb9fc8ba 100644 --- a/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec.ts +++ b/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec.ts @@ -62,7 +62,7 @@ describe('OrcaRuntimeService', () => { .map(([, data]) => data) .filter((data): data is string => typeof data === 'string') expect(payloads).toContain( - '\nYou have 1 orchestration message. Run `orca-dev orchestration check --run run_test`.\n' + '\n[Orca orchestration notification] You have 1 orchestration message. Run `orca-dev orchestration check --run run_test`.\n' ) expect(payloads.some((data) => data.includes('reserved completion'))).toBe(false) expect(status.delivered_at).toEqual(expect.any(String)) diff --git a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts index 8a5968c36f25..8ff0602d1ebb 100644 --- a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts +++ b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts @@ -1,6 +1,6 @@ import type { DeliveryRow, MessageRow, MessageType } from '../../types' import { OrchestrationError } from '../../orchestration-error' -import { generateId, isGeneratedId } from '../generated-id' +import { generateId } from '../generated-id' import type { OrchestrationDb } from '../orchestration-db' import { exposeDeliveryTimestamps, exposeMessageListTimestamps } from '../utc-timestamp' import { ORCHESTRATION_DELIVERY_BATCH_LIMIT } from './mailbox-routing-page' @@ -43,7 +43,9 @@ export function getOrCreateMailboxDelivery( if (params.requireCurrentRunConsumer) { this.requireCurrentConsumer(params.runId, params.consumerGeneration) } - const existing = this.getOutstandingMailboxDelivery(params.mailboxHandle) + const existing = this.db + .prepare("SELECT * FROM deliveries WHERE mailbox_handle = ? AND status = 'outstanding'") + .get(params.mailboxHandle) as DeliveryRow | undefined if (existing) { if (existing.consumer_generation !== params.consumerGeneration) { throw new OrchestrationError( @@ -51,12 +53,11 @@ export function getOrCreateMailboxDelivery( 'This mailbox Delivery belongs to a fenced consumer generation.' ) } - const messages = this.getDeliveryMessages(existing) - if (messages.some((message) => message.read === 0)) { + if (!this.retireReadMailboxDelivery(params.mailboxHandle)) { + const messages = this.getDeliveryMessages(existing) this.db.exec('COMMIT') return { delivery: exposeDeliveryTimestamps(existing), messages, replayed: true } } - this.retireReadMailboxDelivery(params.mailboxHandle) } if (params.wakeTypes?.length) { const placeholders = params.wakeTypes.map(() => '?').join(',') @@ -125,20 +126,6 @@ export function acknowledgeMailboxDelivery( if (params.requireCurrentRunConsumer) { this.requireCurrentConsumer(params.runId, params.consumerGeneration) } - if (isGeneratedId(params.deliveryId, 'msg')) { - const outstanding = this.getOutstandingMailboxDelivery(params.mailboxHandle) - const containsMessage = - outstanding && - outstanding.run_id === params.runId && - (JSON.parse(outstanding.message_ids) as string[]).includes(params.deliveryId) - const guidance = containsMessage - ? `Process the entire batch, then use --ack ${outstanding.id}.` - : 'Run orchestration check to obtain the Delivery id.' - throw new OrchestrationError( - 'stale_delivery', - `--ack takes a Delivery id, not message id ${params.deliveryId}. ${guidance}` - ) - } const delivery = this.getDeliveryRaw(params.deliveryId) if ( !delivery || @@ -147,7 +134,7 @@ export function acknowledgeMailboxDelivery( ) { throw new OrchestrationError( 'stale_delivery', - `Delivery ${params.deliveryId} does not belong to this mailbox.` + `Delivery ${params.deliveryId} does not belong to this mailbox. --ack requires a delivery_* ID returned by orchestration check; process the entire batch before acknowledging.` ) } if ( @@ -189,19 +176,9 @@ export function acknowledgeMailboxDelivery( } } -export function getOutstandingMailboxDelivery( - this: OrchestrationDb, - mailboxHandle: string -): DeliveryRow | undefined { - const row = this.db - .prepare("SELECT * FROM deliveries WHERE mailbox_handle = ? AND status = 'outstanding'") - .get(mailboxHandle) as DeliveryRow | undefined - return row ? exposeDeliveryTimestamps(row) : undefined -} - // Acknowledgment keeps late consumer acks idempotent after lifecycle suppression. -export function retireReadMailboxDelivery(this: OrchestrationDb, mailboxHandle: string): void { - this.db +export function retireReadMailboxDelivery(this: OrchestrationDb, mailboxHandle: string): boolean { + const result = this.db .prepare( `UPDATE deliveries SET status = 'acknowledged', acknowledged_at = datetime('now') WHERE mailbox_handle = ? AND status = 'outstanding' @@ -211,13 +188,20 @@ export function retireReadMailboxDelivery(this: OrchestrationDb, mailboxHandle: )` ) .run(mailboxHandle) + return result.changes > 0 } export function hasOutstandingMailboxDelivery( this: OrchestrationDb, mailboxHandle: string ): boolean { - return this.getOutstandingMailboxDelivery(mailboxHandle) !== undefined + return Boolean( + this.db + .prepare( + "SELECT 1 FROM deliveries WHERE mailbox_handle = ? AND status = 'outstanding' LIMIT 1" + ) + .get(mailboxHandle) + ) } export function fenceOutstandingMailboxDelivery( @@ -232,7 +216,6 @@ export function fenceOutstandingMailboxDelivery( } export type RoleMailboxDeliveryMethods = { - getOutstandingMailboxDelivery: typeof getOutstandingMailboxDelivery retireReadMailboxDelivery: typeof retireReadMailboxDelivery getDeliveryRaw: typeof getDeliveryRaw getDeliveryMessages: typeof getDeliveryMessages @@ -244,7 +227,6 @@ export type RoleMailboxDeliveryMethods = { export function attachRoleMailboxDelivery(ctor: { prototype: object }): void { Object.assign(ctor.prototype, { - getOutstandingMailboxDelivery, retireReadMailboxDelivery, getDeliveryRaw, getDeliveryMessages, diff --git a/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts b/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts index 820f69d147c8..c97a94f3a7f3 100644 --- a/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts +++ b/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts @@ -86,33 +86,29 @@ describe('retired mailbox deliveries', () => { expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') }) - it('names the owning delivery when ack receives a message ID without consuming mail', () => { + it('explains the delivery ID contract for invalid acknowledgements without consuming mail', () => { const { params, insert } = setup() const message = insert('pending') const first = db.getOrCreateRunDelivery(params)! expect(() => db.acknowledgeRunDelivery({ ...params, deliveryId: message.id })).toThrow( - `Process the entire batch, then use --ack ${first.delivery.id}.` + '--ack requires a delivery_* ID returned by orchestration check; process the entire batch before acknowledging.' ) expect(db.getMessageById(message.id)?.read).toBe(0) expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') }) - it('does not reveal a delivery from another mailbox in a wrong-ID error', () => { - const { params, insert } = setup() - const foreign = db.createRun({ - objective: 'other', - coordinatorHandle: 'other', - coordinatorPaneKey: 'other:22222222-2222-4222-9222-222222222222' - }) - const message = insert('private') + it('checks the consumer generation before repairing an already read delivery', () => { + const { run, params, insert } = setup() + const message = insert('old') const first = db.getOrCreateRunDelivery(params)! + db.db.prepare('UPDATE messages SET read = 1 WHERE id = ?').run(message.id) expect(() => - db.acknowledgeRunDelivery({ - runId: foreign.id, - consumerGeneration: foreign.consumer_generation, - deliveryId: message.id + db.getOrCreateMailboxDelivery({ + ...params, + mailboxHandle: `run:${run.id}`, + consumerGeneration: params.consumerGeneration + 1 }) - ).toThrow('Run orchestration check to obtain the Delivery id.') + ).toThrow(expect.objectContaining({ code: 'consumer_fenced' })) expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') }) diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts b/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts index 59a0718b66ad..e3713bd31a26 100644 --- a/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts +++ b/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts @@ -5,7 +5,7 @@ describe('Run delivery history', () => { const h = createOrchestrationRpcHarness() afterEach(() => h.cleanup()) - it('exposes the outstanding delivery without minting or acknowledging it', async () => { + it('does not label filtered history as an acknowledgeable delivery', async () => { const { db, ctx, activeRunId } = h.setup() const params = { terminal: 'term_coord', run: activeRunId, all: true } db.insertMessage({ @@ -15,7 +15,6 @@ describe('Run delivery history', () => { subject: 'waiting' }) expect(await h.call('orchestration.check', params, ctx)).toMatchObject({ - deliveryId: null, count: 1 }) expect(db.hasOutstandingRunDelivery(activeRunId!)).toBe(false) @@ -23,10 +22,24 @@ describe('Run delivery history', () => { runId: activeRunId!, consumerGeneration: db.getRun(activeRunId!)!.consumer_generation })! - expect(await h.call('orchestration.check', { ...params, format: true }, ctx)).toMatchObject({ - deliveryId: delivery.delivery.id, - count: 1 + db.insertMessage({ + from: 'worker', + to: `run:${activeRunId}`, + runId: activeRunId, + subject: 'later completion', + type: 'worker_done' }) + const history = await h.call( + 'orchestration.check', + { + ...params, + format: true, + types: 'worker_done' + }, + ctx + ) + expect(history).toMatchObject({ count: 1, messages: [{ subject: 'later completion' }] }) + expect(history).not.toHaveProperty('deliveryId') expect(db.getDeliveryRaw(delivery.delivery.id)?.status).toBe('outstanding') expect(db.getMessageById(delivery.messages[0].id)?.read).toBe(0) }) diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts b/src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts index 598c080623a7..6db89cf4a201 100644 --- a/src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts +++ b/src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts @@ -101,7 +101,6 @@ export async function checkRunMailbox(args: { const result = { messages: exposeMessages(messages), count: messages.length, - deliveryId: db.getOutstandingMailboxDelivery(address)?.id ?? null, acknowledged: acknowledged?.delivery.id ?? null } if (params.format || params.inject) { From 795d8e56c29948606a6f817d69fe221a712fdbd7 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 10 Sep 2026 01:18:14 -0400 Subject: [PATCH 3/9] test: align orchestration check help expectation --- src/cli/specs/orchestration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/specs/orchestration.test.ts b/src/cli/specs/orchestration.test.ts index cd69aa507084..cea858a9cfdf 100644 --- a/src/cli/specs/orchestration.test.ts +++ b/src/cli/specs/orchestration.test.ts @@ -64,7 +64,7 @@ describe('orchestration check command spec', () => { expect(checkSpec?.notes).toEqual( expect.arrayContaining([ - '--types is the wake condition for --wait; a returned Delivery is always the whole FIFO batch, so it is never filtered by type. Only --peek and --all filter their rows.' + '--types is the wake condition for --wait; a returned Delivery is always the whole FIFO batch, so it is never filtered by type. Without --wait it has no effect on consuming checks. Only --peek and --all filter their rows.' ]) ) }) From d008782bce9339371680d3ac304620588b3928a5 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 10 Sep 2026 01:54:15 -0400 Subject: [PATCH 4/9] refactor(orchestration): derive delivery eligibility from messages --- .../orchestration-delivery-storage.md | 19 ++ ...stration-dispatch-mailbox-delivery.test.ts | 2 +- ...stration-message-delivery-identity.test.ts | 14 +- src/main/runtime/orchestration/db.ts | 1 - .../orchestration/db/contract-constants.ts | 3 +- .../dispatch-mailbox-consumer-fencing.test.ts | 21 +- .../db/messages/mailbox-consumer.ts | 35 +++ .../db/messages/message-inbox.ts | 17 +- .../db/messages/role-mailbox-delivery.ts | 58 ++--- .../orchestration/db/runs/run-delivery.ts | 6 +- .../db/schema/adopt-legacy-run.ts | 6 +- .../db/schema/delivery-v40-test-fixture.ts | 30 +++ .../schema/derived-delivery-migration.test.ts | 237 ++++++++++++++++++ .../migrate-role-mailbox-delivery-v34.ts | 2 +- .../orchestration/db/schema/migrate-v2-v12.ts | 3 +- .../orchestration/db/schema/migrate-v35.ts | 16 +- .../orchestration/db/schema/migrate-v41.ts | 46 ++++ .../orchestration/db/schema/migrate.ts | 5 + .../db/schema/schema-column-probes.ts | 2 +- ...atch-consumer-generation-migration.test.ts | 4 + ...ederation-acknowledgment-migration.test.ts | 2 + .../nested-worker-depth-migration.test.ts | 3 + ...rchestration-delivery-consumption.test.ts} | 44 ++-- .../orchestration-derived-delivery.test.ts | 63 +++++ ...chestration-federated-legacy-probe.test.ts | 8 +- .../orchestration-legacy-storage-db.test.ts | 2 +- ...chestration-legacy-storage-test-fixture.ts | 2 + .../orchestration-schema-version-skew.ts | 26 +- ...chestration-version-skew-migration.test.ts | 47 ++-- src/main/runtime/orchestration/types.ts | 4 +- .../messaging/check-delivery-history.test.ts | 2 +- .../orchestration/messaging/check-worker.ts | 7 +- 32 files changed, 589 insertions(+), 148 deletions(-) create mode 100644 docs/reference/orchestration-delivery-storage.md create mode 100644 src/main/runtime/orchestration/db/messages/mailbox-consumer.ts create mode 100644 src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts create mode 100644 src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts create mode 100644 src/main/runtime/orchestration/db/schema/migrate-v41.ts rename src/main/runtime/orchestration/{orchestration-retired-delivery.test.ts => orchestration-delivery-consumption.test.ts} (74%) create mode 100644 src/main/runtime/orchestration/orchestration-derived-delivery.test.ts diff --git a/docs/reference/orchestration-delivery-storage.md b/docs/reference/orchestration-delivery-storage.md new file mode 100644 index 000000000000..ecb0ee2c4027 --- /dev/null +++ b/docs/reference/orchestration-delivery-storage.md @@ -0,0 +1,19 @@ +# Orchestration delivery storage + +A message records whether it still needs attention (`read`). A delivery records a stable batch ID, ordered message IDs, mailbox, consumer generation, and creation time. Acknowledgement time and fencing record actual events. There is no stored delivery status. + +`outstanding_deliveries` is a SQLite view, not a table or cached projection. It selects batches that have not been acknowledged or fenced and still contain unread messages. Both consuming checks and notification eligibility query this view. + +When completion makes an earlier heartbeat obsolete, only the message changes. Its batch stops appearing in the view without a second write. Checking an old database with a fully read heartbeat batch requires no repair operation. A partially read batch still replays its complete original membership under the same ID. + +Acknowledgement marks the batch's messages read and records `acknowledged_at` in one transaction. A first explicit acknowledgement after lifecycle suppression records the actual event; subsequent acknowledgements are idempotent. Suppression never invents an acknowledgement timestamp. An acknowledgement cannot consume newer messages outside the batch. + +Creation and acknowledgement use `BEGIN IMMEDIATE` and validate the current consumer inside that transaction. Run coordinators use the Run generation. Workers use the generation on their local Dispatch or federated attachment, according to the caller's existing routing. Loopback federation can contain both records, so the source is explicit. The insertion constraint consults the same view to prevent two outstanding batches in a mailbox, while allowing historical batches. + +## Migration and compatibility + +Schema v41 removes `deliveries.status` and its partial unique index. It preserves batch IDs, ordered membership, mailbox addresses, generations, creation times, and acknowledgement times. The old fenced status becomes a boolean fencing fact. The new view replaces stored eligibility; an insertion trigger replaces the old uniqueness backstop. + +This is a one-way local database upgrade for shipped versions that query the removed status column. Those versions cannot open the upgraded database. Rolling the app back requires a compatible pre-upgrade database backup; this migration does not create a backup. A future reverse migration would need to materialize the old status before using an older binary. + +Mixed client/server versions are a different boundary. The existing check RPC returns delivery IDs and messages, not the storage row's status. Its response shape and acknowledgement commands remain compatible, including on SSH and federated worker hosts. This migration is owned by the runtime that opens the database, not by a paired client. diff --git a/src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts b/src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts index 958a93392927..1b86ae9bab57 100644 --- a/src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts +++ b/src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts @@ -204,7 +204,7 @@ describe('Dispatch mailbox Delivery', () => { coordinatorPaneKey: 'tab_new:leaf_new' }) - expect(db.getDeliveryRaw(delivery.delivery.id)?.status).toBe('outstanding') + expect(db.hasOutstandingMailboxDelivery(`dispatch:${dispatch.id}`)).toBe(true) expect( db.getOrCreateMailboxDelivery({ runId: run.id, diff --git a/src/main/runtime/orchestration-message-delivery-identity.test.ts b/src/main/runtime/orchestration-message-delivery-identity.test.ts index 21c8a64f56a0..ff02135eb2e4 100644 --- a/src/main/runtime/orchestration-message-delivery-identity.test.ts +++ b/src/main/runtime/orchestration-message-delivery-identity.test.ts @@ -237,10 +237,10 @@ describe('STA-4325 message and delivery identity', () => { expect(all.count).toBe(3) const deliveryRowsBeforeAck = sqliteFor(db) - .prepare('SELECT id, status, message_ids FROM deliveries ORDER BY rowid') + .prepare('SELECT id, acknowledged_at, message_ids FROM deliveries ORDER BY rowid') .all() as { id: string; status: string; message_ids: string }[] expect(deliveryRowsBeforeAck).toEqual([ - { id: first.deliveryId, status: 'outstanding', message_ids: JSON.stringify(expectedIds) } + { id: first.deliveryId, acknowledged_at: null, message_ids: JSON.stringify(expectedIds) } ]) for (const id of expectedIds) { expect(db.getMessageById(id)).toMatchObject({ to_handle: `run:${run.id}`, read: 0 }) @@ -258,9 +258,9 @@ describe('STA-4325 message and delivery identity', () => { expect.arrayContaining(expectedIds) ) expect(acknowledgedHistory.count).toBe(3) - expect(sqliteFor(db).prepare('SELECT id, status FROM deliveries ORDER BY rowid').all()).toEqual( - [{ id: first.deliveryId, status: 'acknowledged' }] - ) + expect( + sqliteFor(db).prepare('SELECT id, acknowledged_at FROM deliveries ORDER BY rowid').all() + ).toEqual([{ id: first.deliveryId, acknowledged_at: expect.any(String) }]) for (const id of expectedIds) { expect(db.getMessageById(id)?.read).toBe(1) } @@ -488,8 +488,8 @@ describe('STA-4325 message and delivery identity', () => { deliveryId: firstPayload.result.deliveryId, replayed: true }) - expect(sqliteFor(db).prepare('SELECT id, status FROM deliveries').all()).toEqual([ - { id: firstPayload.result.deliveryId, status: 'outstanding' } + expect(sqliteFor(db).prepare('SELECT id, acknowledged_at FROM deliveries').all()).toEqual([ + { id: firstPayload.result.deliveryId, acknowledged_at: null } ]) expect(db.getMessageById(status.id)).toMatchObject({ to_handle: `run:${run.id}`, diff --git a/src/main/runtime/orchestration/db.ts b/src/main/runtime/orchestration/db.ts index 4af72ff07e5c..5669e28aa990 100644 --- a/src/main/runtime/orchestration/db.ts +++ b/src/main/runtime/orchestration/db.ts @@ -45,7 +45,6 @@ export type { WorkerReportSettlement, RunRow, DeliveryRow, - DeliveryStatus, LegacyAdoptionRow, LegacyCompatibilityPrincipalRow, LegacyPrincipalRole, diff --git a/src/main/runtime/orchestration/db/contract-constants.ts b/src/main/runtime/orchestration/db/contract-constants.ts index 56c2e2d542ff..a8ddf9994d0c 100644 --- a/src/main/runtime/orchestration/db/contract-constants.ts +++ b/src/main/runtime/orchestration/db/contract-constants.ts @@ -17,4 +17,5 @@ export const LEGACY_CONTRACT_VERSION = 0 export const CURRENT_CONTRACT_VERSION = ORCHESTRATION_CONTRACT_VERSION // Schema versions: v2 'heartbeat'+last_heartbeat_at, v3 delivered_at, v4 task-creator terminal, v5 task_title/display_name, v6 pane identity, v7 lightweight Runs, v8 crash-safe Run deliveries, v9 durable question threads, v10 Dispatch capabilities, v11 durable mutation receipts, v12 composed worker state, v18 post-v6 version-skew repair, v19 adopted legacy Runs and compatibility receipts, v20 legacy question backfill, v21 legacy scheduler-loss provenance, v22 dispatch assignee lookup, v23 worker terminal resource ownership, v24 creator-incarnation authority, v25 active Dispatch handle lookup, v26 indexed mutation receipt capacity, v27 durable federation acknowledgments, v28 durable local mutation caller identity, v31 dispatch/resource identity links, v32 bounded worker-terminal recovery metadata, v33 durable mailbox pointer Enter state, v34 role-addressed mailbox deliveries, v35 mailbox delivery default and index-predicate repair, v36 dispatch mailbox consumer generation, v37 recorded dispatch creator identity, v39 structured session journal archives. -export const SCHEMA_VERSION = 40 +// v41: derive outstanding deliveries from unread messages. +export const SCHEMA_VERSION = 41 diff --git a/src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts b/src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts index 29a2e468ee89..110ec2bbb493 100644 --- a/src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts +++ b/src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts @@ -1,7 +1,6 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { OrchestrationDb } from '../db' import { ORCHESTRATION_CONTRACT_VERSION } from '../../../../shared/protocol-version' -import { ORCHESTRATION_LEGACY_RUN_ID } from '../../../../shared/orchestration-rpc-contract' import { createRootDispatch } from './root-dispatch-test-fixture' import type { DeliveryRow } from '../types' @@ -35,11 +34,17 @@ describe('dispatch mailbox consumer fencing', () => { return { id: dispatch.id, runId: dispatch.run_id } } - function openDelivery(dispatchId: string, runId: string, generation: number) { + function openDelivery( + dispatchId: string, + runId: string, + generation: number, + consumerSource: 'dispatch' | 'attachment' = 'dispatch' + ) { return db.getOrCreateMailboxDelivery({ runId, mailboxHandle: `dispatch:${dispatchId}`, - consumerGeneration: generation + consumerGeneration: generation, + consumerSource }) } @@ -65,7 +70,7 @@ describe('dispatch mailbox consumer fencing', () => { }) const generationB = generationOf(dispatch.id) expect(generationB).toBe(generationA + 1) - expect(db.getDeliveryRaw(deliveryA!.delivery.id)?.status).toBe('fenced') + expect(db.getDeliveryRaw(deliveryA!.delivery.id)?.fenced).toBe(1) expect(() => db.acknowledgeMailboxDelivery({ @@ -146,7 +151,7 @@ describe('dispatch mailbox consumer fencing', () => { }) expect(generationOf(dispatchId)).toBe(1) - expect(db.getDeliveryRaw(stale!.delivery.id)?.status).toBe('fenced') + expect(db.getDeliveryRaw(stale!.delivery.id)?.fenced).toBe(1) }) it('gives a federated attachment its own generation on the worker host', () => { @@ -169,9 +174,9 @@ describe('dispatch mailbox consumer fencing', () => { from: 'home-peer', to: `dispatch:${dispatchId}`, subject: 'relayed before attach', - runId: ORCHESTRATION_LEGACY_RUN_ID + runId: 'run-home' }) - const stale = openDelivery(dispatchId, ORCHESTRATION_LEGACY_RUN_ID, 0) + const stale = openDelivery(dispatchId, 'run-home', 0, 'attachment') // The worker host holds no dispatch_contexts row for a federated Dispatch. expect(db.getDispatchContextById(dispatchId)).toBeUndefined() @@ -187,7 +192,7 @@ describe('dispatch mailbox consumer fencing', () => { }) expect(db.getRemoteDispatchAttachment(dispatchId)?.consumer_generation).toBe(1) - expect((db.getDeliveryRaw(stale!.delivery.id) as DeliveryRow).status).toBe('fenced') + expect((db.getDeliveryRaw(stale!.delivery.id) as DeliveryRow).fenced).toBe(1) }) it('starts a retry Dispatch on a fresh mailbox address rather than sharing the old one', () => { diff --git a/src/main/runtime/orchestration/db/messages/mailbox-consumer.ts b/src/main/runtime/orchestration/db/messages/mailbox-consumer.ts new file mode 100644 index 000000000000..9ed2b2d4cc96 --- /dev/null +++ b/src/main/runtime/orchestration/db/messages/mailbox-consumer.ts @@ -0,0 +1,35 @@ +import type { OrchestrationDb } from '../orchestration-db' +import { OrchestrationError } from '../../orchestration-error' + +// Validate inside the delivery transaction, so another connection cannot replace the consumer mid-check. +export function requireMailboxConsumer( + db: OrchestrationDb, + params: { + runId: string + mailboxHandle: string + consumerGeneration: number + consumerSource?: 'dispatch' | 'attachment' + } +): void { + if (params.mailboxHandle === `run:${params.runId}`) { + db.requireCurrentConsumer(params.runId, params.consumerGeneration) + return + } + const dispatchId = params.mailboxHandle.startsWith('dispatch:') + ? params.mailboxHandle.slice('dispatch:'.length) + : '' + // A loopback runtime has both records; use the counter belonging to the caller's attachment. + const sql = + params.consumerSource === 'attachment' + ? 'SELECT home_run_id AS run_id, consumer_generation FROM remote_dispatch_attachments WHERE dispatch_id = ?' + : 'SELECT run_id, consumer_generation FROM dispatch_contexts WHERE id = ?' + const consumer = db.db.prepare(sql).get(dispatchId) as + | { run_id: string; consumer_generation: number } + | undefined + if ( + consumer?.run_id !== params.runId || + consumer.consumer_generation !== params.consumerGeneration + ) { + throw new OrchestrationError('consumer_fenced', 'This mailbox consumer has been replaced.') + } +} diff --git a/src/main/runtime/orchestration/db/messages/message-inbox.ts b/src/main/runtime/orchestration/db/messages/message-inbox.ts index 1ef57776b008..e9b800d431e4 100644 --- a/src/main/runtime/orchestration/db/messages/message-inbox.ts +++ b/src/main/runtime/orchestration/db/messages/message-inbox.ts @@ -9,8 +9,7 @@ const MESSAGE_MUTATION_SAVEPOINT = 'message_id_mutation' function runBatchedMessageMutation( db: OrchestrationDb, ids: string[], - sqlForPlaceholders: (placeholders: string) => string, - retireReadDeliveries = false + sqlForPlaceholders: (placeholders: string) => string ): void { if (ids.length === 0) { return @@ -21,14 +20,6 @@ function runBatchedMessageMutation( const batch = ids.slice(offset, offset + MESSAGE_ID_UPDATE_BATCH_SIZE) const placeholders = batch.map(() => '?').join(',') db.db.prepare(sqlForPlaceholders(placeholders)).run(...batch) - if (retireReadDeliveries) { - const mailboxes = db.db - .prepare(`SELECT DISTINCT to_handle FROM messages WHERE id IN (${placeholders})`) - .all(...batch) as { to_handle: string }[] - for (const mailbox of mailboxes) { - db.retireReadMailboxDelivery(mailbox.to_handle) - } - } } db.db.exec(`RELEASE ${MESSAGE_MUTATION_SAVEPOINT}`) } catch (error) { @@ -169,8 +160,7 @@ export function markAsRead(this: OrchestrationDb, ids: string[]): void { `UPDATE messages SET read = 1, pointer_enter_pending = 0, pointer_pty_id = NULL, pointer_process_incarnation = NULL - WHERE id IN (${placeholders})`, - true + WHERE id IN (${placeholders})` ) } @@ -226,8 +216,7 @@ export function markAsReadAndDelivered(this: OrchestrationDb, ids: string[]): vo SET read = 1, delivered_at = COALESCE(delivered_at, datetime('now')), pointer_enter_pending = 0, pointer_pty_id = NULL, pointer_process_incarnation = NULL - WHERE id IN (${placeholders})`, - true + WHERE id IN (${placeholders})` ) } diff --git a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts index 8ff0602d1ebb..d5ff9e04221a 100644 --- a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts +++ b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts @@ -3,6 +3,7 @@ import { OrchestrationError } from '../../orchestration-error' import { generateId } from '../generated-id' import type { OrchestrationDb } from '../orchestration-db' import { exposeDeliveryTimestamps, exposeMessageListTimestamps } from '../utc-timestamp' +import { requireMailboxConsumer } from './mailbox-consumer' import { ORCHESTRATION_DELIVERY_BATCH_LIMIT } from './mailbox-routing-page' export function getDeliveryRaw(this: OrchestrationDb, id: string): DeliveryRow | undefined { @@ -29,9 +30,9 @@ export function getOrCreateMailboxDelivery( runId: string mailboxHandle: string consumerGeneration: number + consumerSource?: 'dispatch' | 'attachment' limit?: number wakeTypes?: MessageType[] - requireCurrentRunConsumer?: boolean } ): { delivery: DeliveryRow; messages: MessageRow[]; replayed: boolean } | undefined { const limit = Math.min( @@ -40,11 +41,9 @@ export function getOrCreateMailboxDelivery( ) this.db.exec('BEGIN IMMEDIATE') try { - if (params.requireCurrentRunConsumer) { - this.requireCurrentConsumer(params.runId, params.consumerGeneration) - } + requireMailboxConsumer(this, params) const existing = this.db - .prepare("SELECT * FROM deliveries WHERE mailbox_handle = ? AND status = 'outstanding'") + .prepare('SELECT * FROM outstanding_deliveries WHERE mailbox_handle = ?') .get(params.mailboxHandle) as DeliveryRow | undefined if (existing) { if (existing.consumer_generation !== params.consumerGeneration) { @@ -53,11 +52,9 @@ export function getOrCreateMailboxDelivery( 'This mailbox Delivery belongs to a fenced consumer generation.' ) } - if (!this.retireReadMailboxDelivery(params.mailboxHandle)) { - const messages = this.getDeliveryMessages(existing) - this.db.exec('COMMIT') - return { delivery: exposeDeliveryTimestamps(existing), messages, replayed: true } - } + const messages = this.getDeliveryMessages(existing) + this.db.exec('COMMIT') + return { delivery: exposeDeliveryTimestamps(existing), messages, replayed: true } } if (params.wakeTypes?.length) { const placeholders = params.wakeTypes.map(() => '?').join(',') @@ -117,15 +114,13 @@ export function acknowledgeMailboxDelivery( runId: string mailboxHandle: string consumerGeneration: number + consumerSource?: 'dispatch' | 'attachment' deliveryId: string - requireCurrentRunConsumer?: boolean } ): { delivery: DeliveryRow; duplicate: boolean } { this.db.exec('BEGIN IMMEDIATE') try { - if (params.requireCurrentRunConsumer) { - this.requireCurrentConsumer(params.runId, params.consumerGeneration) - } + requireMailboxConsumer(this, params) const delivery = this.getDeliveryRaw(params.deliveryId) if ( !delivery || @@ -137,16 +132,13 @@ export function acknowledgeMailboxDelivery( `Delivery ${params.deliveryId} does not belong to this mailbox. --ack requires a delivery_* ID returned by orchestration check; process the entire batch before acknowledging.` ) } - if ( - delivery.consumer_generation !== params.consumerGeneration || - delivery.status === 'fenced' - ) { + if (delivery.consumer_generation !== params.consumerGeneration || delivery.fenced === 1) { throw new OrchestrationError( 'consumer_fenced', 'This mailbox Delivery belongs to a fenced consumer generation.' ) } - if (delivery.status === 'acknowledged') { + if (delivery.acknowledged_at !== null) { this.db.exec('COMMIT') return { delivery: exposeDeliveryTimestamps(delivery), duplicate: true } } @@ -163,9 +155,7 @@ export function acknowledgeMailboxDelivery( .run(...messageIds) } this.db - .prepare( - "UPDATE deliveries SET status = 'acknowledged', acknowledged_at = datetime('now') WHERE id = ?" - ) + .prepare("UPDATE deliveries SET acknowledged_at = datetime('now') WHERE id = ?") .run(delivery.id) const acknowledged = this.getDeliveryRaw(delivery.id) as DeliveryRow this.db.exec('COMMIT') @@ -176,30 +166,13 @@ export function acknowledgeMailboxDelivery( } } -// Acknowledgment keeps late consumer acks idempotent after lifecycle suppression. -export function retireReadMailboxDelivery(this: OrchestrationDb, mailboxHandle: string): boolean { - const result = this.db - .prepare( - `UPDATE deliveries SET status = 'acknowledged', acknowledged_at = datetime('now') - WHERE mailbox_handle = ? AND status = 'outstanding' - AND NOT EXISTS ( - SELECT 1 FROM json_each(deliveries.message_ids) AS member - JOIN messages ON messages.id = member.value WHERE messages.read = 0 - )` - ) - .run(mailboxHandle) - return result.changes > 0 -} - export function hasOutstandingMailboxDelivery( this: OrchestrationDb, mailboxHandle: string ): boolean { return Boolean( this.db - .prepare( - "SELECT 1 FROM deliveries WHERE mailbox_handle = ? AND status = 'outstanding' LIMIT 1" - ) + .prepare('SELECT 1 FROM outstanding_deliveries WHERE mailbox_handle = ? LIMIT 1') .get(mailboxHandle) ) } @@ -210,13 +183,13 @@ export function fenceOutstandingMailboxDelivery( ): void { this.db .prepare( - "UPDATE deliveries SET status = 'fenced' WHERE mailbox_handle = ? AND status = 'outstanding'" + `UPDATE deliveries SET fenced = 1 + WHERE id IN (SELECT id FROM outstanding_deliveries WHERE mailbox_handle = ?)` ) .run(mailboxHandle) } export type RoleMailboxDeliveryMethods = { - retireReadMailboxDelivery: typeof retireReadMailboxDelivery getDeliveryRaw: typeof getDeliveryRaw getDeliveryMessages: typeof getDeliveryMessages getOrCreateMailboxDelivery: typeof getOrCreateMailboxDelivery @@ -227,7 +200,6 @@ export type RoleMailboxDeliveryMethods = { export function attachRoleMailboxDelivery(ctor: { prototype: object }): void { Object.assign(ctor.prototype, { - retireReadMailboxDelivery, getDeliveryRaw, getDeliveryMessages, getOrCreateMailboxDelivery, diff --git a/src/main/runtime/orchestration/db/runs/run-delivery.ts b/src/main/runtime/orchestration/db/runs/run-delivery.ts index b2fc0ba2b0e2..b941c8ae9511 100644 --- a/src/main/runtime/orchestration/db/runs/run-delivery.ts +++ b/src/main/runtime/orchestration/db/runs/run-delivery.ts @@ -32,8 +32,7 @@ export function getOrCreateRunDelivery( mailboxHandle: `run:${params.runId}`, consumerGeneration: params.consumerGeneration, limit: params.limit, - wakeTypes: params.wakeTypes, - requireCurrentRunConsumer: true + wakeTypes: params.wakeTypes }) } @@ -49,8 +48,7 @@ export function acknowledgeRunDelivery( runId: params.runId, mailboxHandle: `run:${params.runId}`, consumerGeneration: params.consumerGeneration, - deliveryId: params.deliveryId, - requireCurrentRunConsumer: true + deliveryId: params.deliveryId }) } diff --git a/src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts b/src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts index a67a6a2f6047..67184333021b 100644 --- a/src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts +++ b/src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts @@ -57,8 +57,10 @@ export function adoptLegacyRunIfNeeded(this: OrchestrationDb): void { this.db .prepare( - `UPDATE deliveries SET status = 'fenced' - WHERE run_id = ? AND status = 'outstanding'` + this.hasColumn('deliveries', 'status') + ? `UPDATE deliveries SET status = 'fenced' WHERE run_id = ? AND status = 'outstanding'` + : `UPDATE deliveries SET fenced = 1 + WHERE run_id = ? AND acknowledged_at IS NULL AND fenced = 0` ) .run(LEGACY_RUN_ID) for (const table of [ diff --git a/src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts b/src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts new file mode 100644 index 000000000000..1a296864b093 --- /dev/null +++ b/src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts @@ -0,0 +1,30 @@ +import type { OrchestrationDb } from '../orchestration-db' + +// Older-version tests must seed the actual historical schema, not just lower user_version. +export function restoreV40DeliverySchema(db: OrchestrationDb['db']): void { + const columns = db.pragma('table_info(deliveries)') as { name: string }[] + if (!columns.some((column) => column.name === 'fenced')) { + return + } + db.exec(` + DROP TRIGGER IF EXISTS trg_deliveries_one_outstanding; + DROP VIEW IF EXISTS outstanding_deliveries; + CREATE TABLE deliveries_v40 ( + id TEXT PRIMARY KEY, run_id TEXT NOT NULL, mailbox_handle TEXT NOT NULL DEFAULT '', + consumer_generation INTEGER NOT NULL, message_ids TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'outstanding' CHECK(status IN ('outstanding', 'acknowledged', 'fenced')), + created_at TEXT NOT NULL DEFAULT (datetime('now')), acknowledged_at TEXT + ); + INSERT INTO deliveries_v40 + SELECT id, run_id, mailbox_handle, consumer_generation, message_ids, + CASE WHEN fenced = 1 THEN 'fenced' + WHEN acknowledged_at IS NOT NULL THEN 'acknowledged' ELSE 'outstanding' END, + created_at, acknowledged_at + FROM deliveries ORDER BY rowid; + DROP TABLE deliveries; + ALTER TABLE deliveries_v40 RENAME TO deliveries; + CREATE UNIQUE INDEX idx_deliveries_one_outstanding + ON deliveries(mailbox_handle) WHERE status = 'outstanding' AND mailbox_handle != ''; + CREATE INDEX idx_deliveries_run_created ON deliveries(run_id, created_at); + `) +} diff --git a/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts b/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts new file mode 100644 index 000000000000..fb83990d8221 --- /dev/null +++ b/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts @@ -0,0 +1,237 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import Database from '../../../../sqlite/sync-database' +import { OrchestrationDb } from '../orchestration-db' +import { restoreV40DeliverySchema } from './delivery-v40-test-fixture' +import { resolveOrchestrationMigrationStartVersion } from '../../orchestration-schema-version-skew' +import { createRootDispatch } from '../root-dispatch-test-fixture' +import { SCHEMA_VERSION } from '../contract-constants' + +describe('derived delivery migration', () => { + const connections: OrchestrationDb[] = [] + const directories: string[] = [] + afterEach(() => { + for (const db of connections.splice(0)) { + db.close() + } + for (const directory of directories.splice(0)) { + rmSync(directory, { recursive: true, force: true }) + } + }) + function open(path: string) { + const db = new OrchestrationDb(path) + connections.push(db) + return db + } + function databasePath() { + const directory = mkdtempSync(join(tmpdir(), 'orca-derived-delivery-')) + directories.push(directory) + return join(directory, 'orchestration.db') + } + + it('preserves batch identity and terminal facts while removing a persisted wedge', () => { + const path = databasePath() + const original = open(path) + const run = original.createRun({ + objective: 'upgrade', + coordinatorHandle: 'coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const params = { runId: run.id, consumerGeneration: run.consumer_generation } + const old = original.insertMessage({ + runId: run.id, + from: 'worker', + to: `run:${run.id}`, + subject: 'old' + }) + const batch = original.getDeliveryRaw(original.getOrCreateRunDelivery(params)!.delivery.id)! + const next = original.insertMessage({ + runId: run.id, + from: 'worker', + to: `run:${run.id}`, + subject: 'next' + }) + connections.pop()!.close() + const raw = new Database(path) + restoreV40DeliverySchema(raw) + raw.prepare('UPDATE messages SET read = 1 WHERE id = ?').run(old.id) + raw.exec(` + INSERT INTO deliveries (id, run_id, mailbox_handle, consumer_generation, message_ids, status, created_at, acknowledged_at) + VALUES ('history_ack', '${run.id}', 'run:${run.id}', 1, '[]', 'acknowledged', '2026-01-01 00:00:00', '2026-01-02 00:00:00'), + ('history_fence', '${run.id}', 'run:${run.id}', 1, '[]', 'fenced', '2026-01-03 00:00:00', NULL); + `) + raw.pragma('user_version = 40') + raw.close() + const db = open(path) + expect(db.getDeliveryRaw(batch.id)).toEqual(batch) + expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) + expect(db.getDeliveryRaw('history_ack')).toMatchObject({ + acknowledged_at: '2026-01-02 00:00:00', + fenced: 0 + }) + expect(db.getDeliveryRaw('history_fence')).toMatchObject({ acknowledged_at: null, fenced: 1 }) + expect(db.getOrCreateRunDelivery(params)?.messages.map((message) => message.id)).toEqual([ + next.id + ]) + expect(db.getDeliveryRaw(batch.id)?.acknowledged_at).toBeNull() + expect(resolveOrchestrationMigrationStartVersion(db.db, SCHEMA_VERSION, SCHEMA_VERSION)).toBe( + SCHEMA_VERSION + ) + const reopened = open(path) + expect(reopened.getOrCreateRunDelivery(params)?.messages.map((message) => message.id)).toEqual([ + next.id + ]) + }) + + it('enforces one active batch using the same derived view and permits history', () => { + const db = open(':memory:') + const run = db.createRun({ + objective: 'constraint', + coordinatorHandle: 'coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const message = db.insertMessage({ + runId: run.id, + from: 'worker', + to: `run:${run.id}`, + subject: 'one' + }) + const params = { runId: run.id, consumerGeneration: run.consumer_generation } + const first = db.getDeliveryRaw(db.getOrCreateRunDelivery(params)!.delivery.id)! + const insert = db.db.prepare(`INSERT INTO deliveries + (id, run_id, mailbox_handle, consumer_generation, message_ids, acknowledged_at, fenced) + VALUES (?, ?, ?, ?, ?, ?, ?)`) + const values = [run.id, `run:${run.id}`, run.consumer_generation, JSON.stringify([message.id])] + expect(() => insert.run('duplicate', ...values, null, 0)).toThrow( + 'Mailbox already has an outstanding delivery' + ) + expect(() => insert.run('ack_history', ...values, '2026-01-01 00:00:00', 0)).not.toThrow() + expect(() => insert.run('fenced_history', ...values, null, 1)).not.toThrow() + db.markAsRead([message.id]) + expect(() => insert.run('consumed_history', ...values, null, 0)).not.toThrow() + expect(db.getDeliveryRaw(first.id)).toEqual(first) + expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) + }) + + it('shares a single batch across connections and rejects replaced consumers after it is consumed', () => { + const path = databasePath() + const first = open(path) + const run = first.createRun({ + objective: 'connections', + coordinatorHandle: 'coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const params = { runId: run.id, consumerGeneration: run.consumer_generation } + const message = first.insertMessage({ + runId: run.id, + from: 'worker', + to: `run:${run.id}`, + subject: 'one' + }) + const second = open(path) + const batch = first.getOrCreateRunDelivery(params)! + expect(second.getOrCreateRunDelivery(params)?.delivery.id).toBe(batch.delivery.id) + second.markAsRead([message.id]) + const replacement = second.bindRun({ + runId: run.id, + coordinatorHandle: 'replacement', + coordinatorPaneKey: 'other:22222222-2222-4222-9222-222222222222' + })! + first.insertMessage({ runId: run.id, from: 'worker', to: `run:${run.id}`, subject: 'next' }) + expect(() => first.getOrCreateRunDelivery(params)).toThrow( + expect.objectContaining({ code: 'consumer_fenced' }) + ) + expect(() => + first.acknowledgeRunDelivery({ ...params, deliveryId: batch.delivery.id }) + ).toThrow(expect.objectContaining({ code: 'consumer_fenced' })) + expect( + second.getOrCreateRunDelivery({ + ...params, + consumerGeneration: replacement.consumer_generation + })?.messages[0].subject + ).toBe('next') + }) + + it.each(['dispatch', 'attachment'] as const)( + 'fences a stale %s consumer across connections even after its batch is read', + (consumerSource) => { + const path = databasePath() + const db = open(path) + const run = db.createRun({ + objective: 'worker connections', + coordinatorHandle: 'coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const dispatchId = + consumerSource === 'dispatch' + ? createRootDispatch(db, db.createTask({ spec: 'work', runId: run.id }).id, 'worker').id + : 'ctx_remote' + if (consumerSource === 'attachment') { + db.createRemoteDispatchAttachment({ + runId: run.id, + dispatchId, + taskId: 'task_remote', + homePeerFingerprint: 'peer', + runtimeEpoch: 'epoch', + protocolVersion: 1, + mutationReceipt: { + callerFingerprint: 'peer', + requestId: 'attach', + method: 'orchestration.federationAttachStart', + payloadHash: 'hash' + } + }) + } + const mailboxHandle = `dispatch:${dispatchId}` + const message = db.insertMessage({ + runId: run.id, + from: 'coord', + to: mailboxHandle, + subject: 'old' + }) + const params = { runId: run.id, mailboxHandle, consumerGeneration: 0, consumerSource } + const batch = db.getOrCreateMailboxDelivery(params)! + const peer = open(path) + peer.markAsRead([message.id]) + const authority = { + dispatchId, + paneKey: 'other:22222222-2222-4222-9222-222222222222', + processIncarnation: 'worker:2' + } + if (consumerSource === 'dispatch') { + peer.mintDispatchCapability(authority) + } else { + peer.prepareRemoteAttachmentAuthority({ + ...authority, + worktreeId: 'folder', + terminalHandle: 'replacement', + setupState: 'not_applicable', + effects: [] + }) + } + peer.insertMessage({ runId: run.id, from: 'coord', to: mailboxHandle, subject: 'next' }) + expect(() => db.getOrCreateMailboxDelivery(params)).toThrow( + expect.objectContaining({ code: 'consumer_fenced' }) + ) + expect(() => + db.acknowledgeMailboxDelivery({ ...params, deliveryId: batch.delivery.id }) + ).toThrow(expect.objectContaining({ code: 'consumer_fenced' })) + expect( + peer.getOrCreateMailboxDelivery({ ...params, consumerGeneration: 1 })?.messages[0].subject + ).toBe('next') + } + ) + + it('recreates a missing derived view on reopen without changing batch records', () => { + const path = databasePath() + const db = open(path) + db.db.exec('DROP VIEW outstanding_deliveries') + const reopened = open(path) + expect(reopened.hasOutstandingMailboxDelivery('run:missing')).toBe(false) + expect( + resolveOrchestrationMigrationStartVersion(reopened.db, SCHEMA_VERSION, SCHEMA_VERSION) + ).toBe(SCHEMA_VERSION) + }) +}) diff --git a/src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts b/src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts index eee649ad6664..544b8cbcb5a7 100644 --- a/src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts +++ b/src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts @@ -1,7 +1,7 @@ import type { OrchestrationDb } from '../orchestration-db' export function migrateRoleMailboxDeliveryV34(this: OrchestrationDb, current: number): void { - if (current >= 34) { + if (current >= 34 || !this.hasColumn('deliveries', 'status')) { return } diff --git a/src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts b/src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts index 140a41160303..07b4f0b26eea 100644 --- a/src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts +++ b/src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts @@ -113,8 +113,7 @@ export function applySchemaMigrationsV2ToV12(this: OrchestrationDb, current: num created_at TEXT NOT NULL DEFAULT (datetime('now')), acknowledged_at TEXT ); - CREATE UNIQUE INDEX IF NOT EXISTS idx_deliveries_one_outstanding - ON deliveries(run_id) WHERE status = 'outstanding'; + ${this.hasColumn('deliveries', 'fenced') ? '' : `CREATE UNIQUE INDEX IF NOT EXISTS idx_deliveries_one_outstanding ON deliveries(run_id) WHERE status = 'outstanding';`} CREATE INDEX IF NOT EXISTS idx_deliveries_run_created ON deliveries(run_id, created_at); diff --git a/src/main/runtime/orchestration/db/schema/migrate-v35.ts b/src/main/runtime/orchestration/db/schema/migrate-v35.ts index 93a9613d68cf..c2c876cca94c 100644 --- a/src/main/runtime/orchestration/db/schema/migrate-v35.ts +++ b/src/main/runtime/orchestration/db/schema/migrate-v35.ts @@ -40,13 +40,15 @@ export function migrateV35(this: OrchestrationDb, current: number): void { DROP TABLE IF EXISTS lifecycle_transition_receipts; ${ADDITIVE_LIFECYCLE_DELETE_TRIGGERS_SQL} `) - rebuildDeliveriesWithMailboxDefault.call(this) - recreateIndexMissingPredicate.call( - this, - 'idx_deliveries_one_outstanding', - "mailbox_handle != ''", - ONE_OUTSTANDING_INDEX_SQL - ) + if (this.hasColumn('deliveries', 'status')) { + rebuildDeliveriesWithMailboxDefault.call(this) + recreateIndexMissingPredicate.call( + this, + 'idx_deliveries_one_outstanding', + "mailbox_handle != ''", + ONE_OUTSTANDING_INDEX_SQL + ) + } recreateIndexMissingPredicate.call( this, 'idx_messages_pending_pointer_enter', diff --git a/src/main/runtime/orchestration/db/schema/migrate-v41.ts b/src/main/runtime/orchestration/db/schema/migrate-v41.ts new file mode 100644 index 000000000000..2d576fbaa140 --- /dev/null +++ b/src/main/runtime/orchestration/db/schema/migrate-v41.ts @@ -0,0 +1,46 @@ +import type { OrchestrationDb } from '../orchestration-db' + +export function migrateV41(this: OrchestrationDb): void { + if (this.hasColumn('deliveries', 'status')) { + this.db.exec(` + CREATE TABLE deliveries_v41 ( + id TEXT PRIMARY KEY, + run_id TEXT NOT NULL, + mailbox_handle TEXT NOT NULL DEFAULT '', + consumer_generation INTEGER NOT NULL, + message_ids TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT (datetime('now')), + acknowledged_at TEXT, + fenced INTEGER NOT NULL DEFAULT 0 CHECK(fenced IN (0, 1)) + ); + INSERT INTO deliveries_v41 + (id, run_id, mailbox_handle, consumer_generation, message_ids, created_at, acknowledged_at, fenced) + SELECT id, run_id, mailbox_handle, consumer_generation, message_ids, created_at, + acknowledged_at, + CASE WHEN status = 'fenced' THEN 1 ELSE 0 END + FROM deliveries ORDER BY rowid; + DROP TABLE deliveries; + ALTER TABLE deliveries_v41 RENAME TO deliveries; + `) + } + this.db.exec(` + CREATE INDEX IF NOT EXISTS idx_deliveries_run_created ON deliveries(run_id, created_at); + CREATE INDEX IF NOT EXISTS idx_deliveries_unacknowledged_mailbox + ON deliveries(mailbox_handle) WHERE acknowledged_at IS NULL AND fenced = 0; + CREATE VIEW IF NOT EXISTS outstanding_deliveries AS + SELECT * FROM deliveries + WHERE acknowledged_at IS NULL AND fenced = 0 + AND EXISTS ( + SELECT 1 FROM json_each(deliveries.message_ids) AS member + JOIN messages ON messages.id = member.value WHERE messages.read = 0 + ); + CREATE TRIGGER IF NOT EXISTS trg_deliveries_one_outstanding + AFTER INSERT ON deliveries + WHEN NEW.mailbox_handle != '' AND EXISTS ( + SELECT 1 FROM outstanding_deliveries WHERE mailbox_handle = NEW.mailbox_handle LIMIT 1 OFFSET 1 + ) + BEGIN + SELECT RAISE(ABORT, 'Mailbox already has an outstanding delivery'); + END; + `) +} diff --git a/src/main/runtime/orchestration/db/schema/migrate.ts b/src/main/runtime/orchestration/db/schema/migrate.ts index 582dedf4752c..1d99ce7f07b4 100644 --- a/src/main/runtime/orchestration/db/schema/migrate.ts +++ b/src/main/runtime/orchestration/db/schema/migrate.ts @@ -11,6 +11,7 @@ import { migrateV37 } from './migrate-v37' import { migrateV38 } from './migrate-v38' import { migrateV39 } from './migrate-v39' import { migrateV40 } from './migrate-v40' +import { migrateV41 } from './migrate-v41' // Why: CREATE TABLE IF NOT EXISTS won't alter existing DBs; migrate in a txn that bumps user_version only on success (atomic all-or-nothing). export function migrate(this: OrchestrationDb): void { @@ -22,6 +23,9 @@ export function migrate(this: OrchestrationDb): void { this.db.exec('BEGIN IMMEDIATE') try { + this.db.exec( + 'DROP TRIGGER IF EXISTS trg_deliveries_one_outstanding; DROP VIEW IF EXISTS outstanding_deliveries;' + ) applySchemaMigrationsV2ToV12.call(this, current) applySchemaMigrationsV13ToV30.call(this, current) migrateMailboxPointerEnterV33.call(this, current) @@ -32,6 +36,7 @@ export function migrate(this: OrchestrationDb): void { migrateV38.call(this, current) migrateV39.call(this, current) migrateV40.call(this, current) + migrateV41.call(this) this.createMailboxDeliveryIndexesIfPossible() this.db.pragma(`user_version = ${SCHEMA_VERSION}`) this.db.exec('COMMIT') diff --git a/src/main/runtime/orchestration/db/schema/schema-column-probes.ts b/src/main/runtime/orchestration/db/schema/schema-column-probes.ts index 07e71e9e9d35..1e9b672620b0 100644 --- a/src/main/runtime/orchestration/db/schema/schema-column-probes.ts +++ b/src/main/runtime/orchestration/db/schema/schema-column-probes.ts @@ -6,7 +6,7 @@ export function hasColumn(this: OrchestrationDb, table: string, column: string): } export function createMailboxDeliveryIndexesIfPossible(this: OrchestrationDb): void { - if (this.hasColumn('deliveries', 'mailbox_handle')) { + if (this.hasColumn('deliveries', 'mailbox_handle') && this.hasColumn('deliveries', 'status')) { // Excluding '' trades the pre-v34 per-run one-outstanding backstop for downgraded binaries; the // app-level BEGIN IMMEDIATE still serializes one process. this.db.exec(` diff --git a/src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts b/src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts index 9cc195d64023..4b26c0cfb854 100644 --- a/src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts +++ b/src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import Database from '../../sqlite/sync-database' +import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' import { OrchestrationDb } from './db' import { SCHEMA_VERSION } from './db/contract-constants' import { createRootDispatch } from './db/root-dispatch-test-fixture' @@ -48,6 +49,7 @@ describe('OrchestrationDb v35 to v36 migration', () => { seed.close() const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec(` ALTER TABLE dispatch_contexts DROP COLUMN consumer_generation; ALTER TABLE remote_dispatch_attachments DROP COLUMN consumer_generation; @@ -78,6 +80,7 @@ describe('OrchestrationDb v35 to v36 migration', () => { it('does not send a v35 stamp back to the pre-Run repair floor', () => { const v35 = createV35Database() const raw = new Database(v35.path) + restoreV40DeliverySchema(raw) try { expect(resolveOrchestrationMigrationStartVersion(raw, 35, SCHEMA_VERSION)).toBe(35) } finally { @@ -88,6 +91,7 @@ describe('OrchestrationDb v35 to v36 migration', () => { it('repairs a database stamped v36 that never got the columns', () => { const v35 = createV35Database() const raw = new Database(v35.path) + restoreV40DeliverySchema(raw) raw.pragma('user_version = 36') try { // Why: the skew repair is the only thing that catches a partially-written v36. diff --git a/src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts b/src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts index ede53f0d521c..856ffa1f85f2 100644 --- a/src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts +++ b/src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import Database from '../../sqlite/sync-database' +import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' import { OrchestrationDb } from './db' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' import { SCHEMA_VERSION } from './db/contract-constants' @@ -26,6 +27,7 @@ describe('federation acknowledgment migration', () => { db = undefined const oldDb = new Database(dbPath) + restoreV40DeliverySchema(oldDb) oldDb.exec('ALTER TABLE federated_dispatches DROP COLUMN to_home_acknowledged_sequence') oldDb.pragma('user_version = 26') expect(resolveOrchestrationMigrationStartVersion(oldDb, 26, 28)).toBe(26) diff --git a/src/main/runtime/orchestration/nested-worker-depth-migration.test.ts b/src/main/runtime/orchestration/nested-worker-depth-migration.test.ts index 9d05c0dac07f..0341f41655ee 100644 --- a/src/main/runtime/orchestration/nested-worker-depth-migration.test.ts +++ b/src/main/runtime/orchestration/nested-worker-depth-migration.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import Database from '../../sqlite/sync-database' +import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' import { OrchestrationDb } from './db' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' import { SCHEMA_VERSION } from './db/contract-constants' @@ -32,6 +33,7 @@ describe('nested worker depth migration (v30)', () => { fresh.close() const oldDb = new Database(dbPath) + restoreV40DeliverySchema(oldDb) oldDb.exec('ALTER TABLE dispatch_contexts DROP COLUMN depth') oldDb.exec('ALTER TABLE remote_dispatch_attachments DROP COLUMN depth') oldDb.exec('ALTER TABLE remote_dispatch_attachments DROP COLUMN home_run_id') @@ -95,6 +97,7 @@ describe('nested worker depth migration (v30)', () => { // replay migrations from v6 instead of starting at 29. const dbPath = createV29Database() const oldDb = new Database(dbPath) + restoreV40DeliverySchema(oldDb) expect(resolveOrchestrationMigrationStartVersion(oldDb, 29, SCHEMA_VERSION)).toBe(29) oldDb.close() }) diff --git a/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts b/src/main/runtime/orchestration/orchestration-delivery-consumption.test.ts similarity index 74% rename from src/main/runtime/orchestration/orchestration-retired-delivery.test.ts rename to src/main/runtime/orchestration/orchestration-delivery-consumption.test.ts index c97a94f3a7f3..49c8977c417b 100644 --- a/src/main/runtime/orchestration/orchestration-retired-delivery.test.ts +++ b/src/main/runtime/orchestration/orchestration-delivery-consumption.test.ts @@ -3,7 +3,7 @@ import { OrchestrationDb } from './db' import { createRootDispatch } from './db/root-dispatch-test-fixture' import { reconcileLifecycleMessage } from './lifecycle-reconciliation' -describe('retired mailbox deliveries', () => { +describe('mailbox delivery consumption', () => { let db: OrchestrationDb afterEach(() => db?.close()) @@ -20,7 +20,7 @@ describe('retired mailbox deliveries', () => { return { run, params, insert } } - it('retires the heartbeat delivery atomically when completion suppresses its contents', () => { + it('advances past a heartbeat batch when completion suppresses its contents', () => { const { run, params } = setup() const task = db.createTask({ runId: run.id, spec: 'work' }) const dispatch = createRootDispatch(db, task.id, 'worker') @@ -37,7 +37,7 @@ describe('retired mailbox deliveries', () => { const first = db.getOrCreateRunDelivery(params)! const done = insert('worker_done') expect(reconcileLifecycleMessage(db, done).action).toBe('completed') - expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('acknowledged') + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).toBeNull() expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) expect( db @@ -45,11 +45,11 @@ describe('retired mailbox deliveries', () => { ?.messages.map((m) => m.id) ).toEqual([done.id]) expect(db.acknowledgeRunDelivery({ ...params, deliveryId: first.delivery.id }).duplicate).toBe( - true + false ) }) - it('repairs an already fully-read outstanding delivery before replay', () => { + it('ignores a fully read batch without rewriting it', () => { const { params, insert } = setup() const old = insert('old') const first = db.getOrCreateRunDelivery(params)! @@ -58,7 +58,7 @@ describe('retired mailbox deliveries', () => { const current = db.getOrCreateRunDelivery(params)! expect(current.messages.map((m) => m.id)).toEqual([next.id]) expect(current.replayed).toBe(false) - expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('acknowledged') + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).toBeNull() }) it('preserves the entire replay batch while any member is unread', () => { @@ -74,16 +74,20 @@ describe('retired mailbox deliveries', () => { expect(replay.replayed).toBe(true) }) - it('rolls retirement back with the enclosing lifecycle transaction', () => { + it('derives eligibility again when a read transaction rolls back', () => { const { params, insert } = setup() const message = insert('old') const first = db.getOrCreateRunDelivery(params)! + const before = db.getDeliveryRaw(first.delivery.id) db.db.exec('BEGIN') db.markAsReadAndDelivered([message.id]) - expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('acknowledged') + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).toBeNull() + expect(db.hasOutstandingRunDelivery(params.runId)).toBe(false) db.db.exec('ROLLBACK') + expect(db.hasOutstandingRunDelivery(params.runId)).toBe(true) + expect(db.getDeliveryRaw(first.delivery.id)).toEqual(before) expect(db.getMessageById(message.id)?.read).toBe(0) - expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).toBeNull() }) it('explains the delivery ID contract for invalid acknowledgements without consuming mail', () => { @@ -94,10 +98,10 @@ describe('retired mailbox deliveries', () => { '--ack requires a delivery_* ID returned by orchestration check; process the entire batch before acknowledging.' ) expect(db.getMessageById(message.id)?.read).toBe(0) - expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).toBeNull() }) - it('checks the consumer generation before repairing an already read delivery', () => { + it('checks the consumer generation even when the prior batch is already read', () => { const { run, params, insert } = setup() const message = insert('old') const first = db.getOrCreateRunDelivery(params)! @@ -109,15 +113,15 @@ describe('retired mailbox deliveries', () => { consumerGeneration: params.consumerGeneration + 1 }) ).toThrow(expect.objectContaining({ code: 'consumer_fenced' })) - expect(db.getDeliveryRaw(first.delivery.id)?.status).toBe('outstanding') + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).toBeNull() }) it.each(['markAsRead', 'markAsReadAndDelivered'] as const)( - '%s retires dispatch mail without retiring a different mailbox', + '%s releases dispatch mail without changing a different mailbox', (method) => { const { run, params, insert } = setup() insert('coordinator mail') - const coordinator = db.getOrCreateRunDelivery(params)! + db.getOrCreateRunDelivery(params)! const task = db.createTask({ runId: run.id, spec: 'worker mail' }) const dispatch = createRootDispatch(db, task.id, 'worker') const mailboxHandle = `dispatch:${dispatch.id}` @@ -127,12 +131,16 @@ describe('retired mailbox deliveries', () => { to: mailboxHandle, subject: 'worker mail' }) - const workerParams = { ...params, mailboxHandle } - const worker = db.getOrCreateMailboxDelivery(workerParams)! + const workerParams = { + ...params, + mailboxHandle, + consumerGeneration: dispatch.consumer_generation + } + db.getOrCreateMailboxDelivery(workerParams)! db[method]([message.id]) - expect(db.getDeliveryRaw(worker.delivery.id)?.status).toBe('acknowledged') + expect(db.hasOutstandingMailboxDelivery(mailboxHandle)).toBe(false) expect(db.getOrCreateMailboxDelivery(workerParams)).toBeUndefined() - expect(db.getDeliveryRaw(coordinator.delivery.id)?.status).toBe('outstanding') + expect(db.hasOutstandingRunDelivery(run.id)).toBe(true) } ) }) diff --git a/src/main/runtime/orchestration/orchestration-derived-delivery.test.ts b/src/main/runtime/orchestration/orchestration-derived-delivery.test.ts new file mode 100644 index 000000000000..9639dd4d312a --- /dev/null +++ b/src/main/runtime/orchestration/orchestration-derived-delivery.test.ts @@ -0,0 +1,63 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { OrchestrationDb } from './db' + +describe('delivery eligibility derived from messages', () => { + let db: OrchestrationDb + afterEach(() => db?.close()) + + function setup() { + db = new OrchestrationDb(':memory:') + const run = db.createRun({ + objective: 'Derived delivery', + coordinatorHandle: 'coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const params = { runId: run.id, consumerGeneration: run.consumer_generation } + const message = db.insertMessage({ + runId: run.id, + from: 'worker', + to: `run:${run.id}`, + subject: 'old' + }) + const first = db.getOrCreateRunDelivery(params)! + return { run, params, message, first } + } + + it('does not store delivery status', () => { + setup() + expect( + (db.db.pragma('table_info(deliveries)') as { name: string }[]).map((c) => c.name) + ).not.toContain('status') + }) + + it.each(['read mutation', 'lifecycle suppression', 'direct SQL'])( + '%s changes eligibility without updating the batch', + (path) => { + const { run, params, message, first } = setup() + const before = db.getDeliveryRaw(first.delivery.id) + if (path === 'read mutation') { + db.markAsRead([message.id]) + } else if (path === 'lifecycle suppression') { + db.markAsReadAndDelivered([message.id]) + } else { + db.db.prepare('UPDATE messages SET read = 1 WHERE id = ?').run(message.id) + } + expect(db.getDeliveryRaw(first.delivery.id)).toEqual(before) + expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) + const changes = db.db.prepare('SELECT total_changes() AS n').get() + expect(db.getOrCreateRunDelivery(params)).toBeUndefined() + expect(db.db.prepare('SELECT total_changes() AS n').get()).toEqual(changes) + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).toBeNull() + } + ) + + it('records only an actual acknowledgement, including after all messages were suppressed', () => { + const { params, message, first } = setup() + db.markAsReadAndDelivered([message.id]) + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).toBeNull() + const ack = { ...params, deliveryId: first.delivery.id } + expect(db.acknowledgeRunDelivery(ack).duplicate).toBe(false) + expect(db.getDeliveryRaw(first.delivery.id)?.acknowledged_at).not.toBeNull() + expect(db.acknowledgeRunDelivery(ack).duplicate).toBe(true) + }) +}) diff --git a/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts b/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts index 5eff7569d27d..2bd5af9dc3c0 100644 --- a/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts +++ b/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts @@ -69,9 +69,9 @@ describe('federated mailbox legacy-adoption probe', () => { }) } else { expect( - db.db.prepare("SELECT status FROM deliveries WHERE id = 'delivery_probe'").get() + db.db.prepare("SELECT fenced FROM deliveries WHERE id = 'delivery_probe'").get() ).toEqual({ - status: 'outstanding' + fenced: 0 }) } } @@ -115,9 +115,9 @@ describe('federated mailbox legacy-adoption probe', () => { }) } else { expect( - db.db.prepare("SELECT status FROM deliveries WHERE id = 'delivery_probe'").get() + db.db.prepare("SELECT fenced FROM deliveries WHERE id = 'delivery_probe'").get() ).toEqual({ - status: 'fenced' + fenced: 1 }) } } diff --git a/src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts b/src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts index 2dda528333a3..3aa7739ed517 100644 --- a/src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts +++ b/src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts @@ -112,7 +112,7 @@ describe('OrchestrationDb legacy contract storage', () => { ).toMatchObject({ run_id: adoptedRunId, mailbox_handle: `run:${LEGACY_RUN_ID}`, - status: 'fenced' + fenced: 1 }) expect(db.getDispatchContextById(fixture.currentDispatchId)).toMatchObject({ run_id: fixture.currentRunId, diff --git a/src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts b/src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts index 886f2383db57..c11d6d35d7ce 100644 --- a/src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts +++ b/src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import Database from '../../sqlite/sync-database' import { LEGACY_RUN_ID, OrchestrationDb } from './db' +import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' import { createRootDispatch } from './db/root-dispatch-test-fixture' export type LegacyStorageCutoverFixture = { @@ -175,6 +176,7 @@ export function createLegacyStorageCutoverFixture(): { first.close() const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) const legacyDeliveryId = 'delivery_legacy_outstanding' raw .prepare( diff --git a/src/main/runtime/orchestration/orchestration-schema-version-skew.ts b/src/main/runtime/orchestration/orchestration-schema-version-skew.ts index 85e0a78b3ae4..a9e9cef0a747 100644 --- a/src/main/runtime/orchestration/orchestration-schema-version-skew.ts +++ b/src/main/runtime/orchestration/orchestration-schema-version-skew.ts @@ -43,7 +43,8 @@ const VERSIONED_POST_V6_COLUMNS = [ { version: 36, table: 'remote_dispatch_attachments', column: 'consumer_generation' }, { version: 37, table: 'dispatch_contexts', column: 'creator_handle' }, { version: 37, table: 'dispatch_contexts', column: 'creator_pane_key' }, - { version: 40, table: 'remote_dispatch_attachments', column: 'home_run_id' } + { version: 40, table: 'remote_dispatch_attachments', column: 'home_run_id' }, + { version: 41, table: 'deliveries', column: 'fenced' } ] as const // Why: v34 shipped without these two, so a v34 stamp proves nothing about them; v35 repairs both @@ -53,7 +54,6 @@ const VERSIONED_POST_V6_COLUMN_DEFAULTS = [ ] as const const VERSIONED_POST_V6_INDEX_PREDICATES = [ - { version: 35, index: 'idx_deliveries_one_outstanding', predicate: "mailbox_handle != ''" }, { version: 35, index: 'idx_messages_pending_pointer_enter', @@ -68,7 +68,6 @@ const POST_V6_INDEXES = [ 'idx_dispatch_run_status', 'idx_gates_run_status', 'idx_runs_coordinator_pane', - 'idx_deliveries_one_outstanding', 'idx_deliveries_run_created', 'idx_questions_dispatch_status', 'idx_federation_relay_pending', @@ -99,8 +98,12 @@ function hasOrchestrationColumnDefault( return rows.some((row) => row.name === column && row.dflt_value === defaultValue) } -function hasOrchestrationIndex(db: Database.Database, index: string): boolean { - return !!db.prepare("SELECT 1 FROM sqlite_master WHERE type = 'index' AND name = ?").get(index) +function hasOrchestrationSchemaObject( + db: Database.Database, + name: string, + type: 'index' | 'view' | 'trigger' = 'index' +): boolean { + return !!db.prepare('SELECT 1 FROM sqlite_master WHERE type = ? AND name = ?').get(type, name) } function hasOrchestrationIndexPredicate( @@ -171,7 +174,18 @@ function hasCompletePostV6Schema(db: Database.Database, storedVersion: number): ({ version, index, predicate }) => storedVersion < version || hasOrchestrationIndexPredicate(db, index, predicate) ) && - POST_V6_INDEXES.every((index) => hasOrchestrationIndex(db, index)) && + (storedVersion >= 41 + ? hasOrchestrationSchemaObject(db, 'idx_deliveries_unacknowledged_mailbox') && + hasOrchestrationSchemaObject(db, 'outstanding_deliveries', 'view') && + hasOrchestrationSchemaObject(db, 'trg_deliveries_one_outstanding', 'trigger') + : hasOrchestrationSchemaObject(db, 'idx_deliveries_one_outstanding') && + (storedVersion < 35 || + hasOrchestrationIndexPredicate( + db, + 'idx_deliveries_one_outstanding', + "mailbox_handle != ''" + ))) && + POST_V6_INDEXES.every((index) => hasOrchestrationSchemaObject(db, index)) && messagesAllowQuestions(db) && hasConsistentLegacyAdoption(db) ) diff --git a/src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts b/src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts index f8fa7a5df484..beaab4511d07 100644 --- a/src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts +++ b/src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts @@ -6,6 +6,7 @@ import Database from '../../sqlite/sync-database' import { LEGACY_CONTRACT_VERSION, LEGACY_RUN_ID, OrchestrationDb } from './db' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' import { createRootDispatch } from './db/root-dispatch-test-fixture' +import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' import { SCHEMA_VERSION } from './db/contract-constants' describe('OrchestrationDb version-skew migration', () => { @@ -25,6 +26,7 @@ describe('OrchestrationDb version-skew migration', () => { tempDir = mkdtempSync(join(tmpdir(), 'orca-db-version-skew-')) const dbPath = join(tempDir, 'orchestration.db') const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec(` CREATE TABLE messages ( id TEXT NOT NULL, @@ -185,6 +187,7 @@ describe('OrchestrationDb version-skew migration', () => { it('does not repair an incomplete schema written by a future binary', () => { const dbPath = createLegacySchemaClaimingVersion(20) const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) expect(resolveOrchestrationMigrationStartVersion(raw, 20, 19)).toBe(20) expect(raw.pragma('user_version', { simple: true })).toBe(20) @@ -200,6 +203,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec( 'ALTER TABLE worker_terminal_resources DROP COLUMN recovery_attempt_count; ALTER TABLE worker_terminal_resources DROP COLUMN last_recovery_at;' ) @@ -241,6 +245,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec( 'ALTER TABLE worker_terminal_resources DROP COLUMN recovery_attempt_count; ALTER TABLE worker_terminal_resources DROP COLUMN last_recovery_at;' ) @@ -267,6 +272,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec(` DROP INDEX idx_deliveries_one_outstanding; ALTER TABLE deliveries DROP COLUMN mailbox_handle; @@ -303,6 +309,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec(` DROP INDEX idx_deliveries_one_outstanding; ALTER TABLE deliveries DROP COLUMN mailbox_handle; @@ -329,7 +336,7 @@ describe('OrchestrationDb version-skew migration', () => { const migratedDeliveries = db.db .prepare( `SELECT id, run_id, mailbox_handle, consumer_generation, message_ids, - status, created_at, acknowledged_at + fenced, created_at, acknowledged_at FROM deliveries` ) .all() @@ -342,7 +349,7 @@ describe('OrchestrationDb version-skew migration', () => { mailbox_handle: `run:${run.id}`, consumer_generation: run.consumer_generation, message_ids: originalMessageIds, - status: 'acknowledged', + fenced: 0, created_at: '2026-01-02 03:04:05', acknowledged_at: '2026-01-02 04:05:06' }, @@ -352,7 +359,7 @@ describe('OrchestrationDb version-skew migration', () => { mailbox_handle: `run:${run.id}`, consumer_generation: run.consumer_generation, message_ids: '["msg_fenced"]', - status: 'fenced', + fenced: 1, created_at: '2026-03-04 05:06:07', acknowledged_at: null }, @@ -362,7 +369,7 @@ describe('OrchestrationDb version-skew migration', () => { mailbox_handle: `run:${run.id}`, consumer_generation: run.consumer_generation, message_ids: '["msg_outstanding"]', - status: 'outstanding', + fenced: 0, created_at: '2026-02-03 04:05:06', acknowledged_at: null } @@ -372,17 +379,12 @@ describe('OrchestrationDb version-skew migration', () => { .prepare("SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'deliveries'") .all() as { name: string }[] expect(deliveryIndexes.map(({ name }) => name)).toEqual( - expect.arrayContaining(['idx_deliveries_one_outstanding', 'idx_deliveries_run_created']) + expect.arrayContaining([ + 'idx_deliveries_unacknowledged_mailbox', + 'idx_deliveries_run_created' + ]) ) - expect(() => - db!.db - .prepare( - `INSERT INTO deliveries ( - id, run_id, mailbox_handle, consumer_generation, message_ids - ) VALUES (?, ?, ?, ?, '[]')` - ) - .run('delivery_v34_duplicate', run.id, `run:${run.id}`, run.consumer_generation) - ).toThrow(/UNIQUE constraint failed/) + expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) }) it('cleans additive lifecycle rows when a v30 writer resets tasks before re-upgrade', () => { @@ -413,6 +415,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) // v30 resetTasks predates both additive tables, so it only deletes their legacy parents. raw.exec(` DELETE FROM worker_dispatches; @@ -433,6 +436,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec( `DROP INDEX IF EXISTS idx_messages_pending_pointer_enter; DROP INDEX IF EXISTS idx_messages_pending_pointer_pty; @@ -489,6 +493,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec(` DROP TABLE deliveries; CREATE TABLE deliveries ( @@ -514,10 +519,12 @@ describe('OrchestrationDb version-skew migration', () => { expect( ( db.db - .prepare("SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_one_outstanding'") + .prepare( + "SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_unacknowledged_mailbox'" + ) .get() as { sql: string } ).sql - ).toContain("mailbox_handle != ''") + ).toContain('acknowledged_at IS NULL AND fenced = 0') const run = db.createRun({ objective: 'already stamped', @@ -541,6 +548,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec(` DROP INDEX IF EXISTS idx_messages_pending_pointer_enter; CREATE INDEX idx_messages_pending_pointer_enter @@ -567,6 +575,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) + restoreV40DeliverySchema(raw) raw.exec(` DROP INDEX IF EXISTS idx_deliveries_one_outstanding; CREATE UNIQUE INDEX idx_deliveries_one_outstanding @@ -579,9 +588,11 @@ describe('OrchestrationDb version-skew migration', () => { expect( ( db.db - .prepare("SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_one_outstanding'") + .prepare( + "SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_unacknowledged_mailbox'" + ) .get() as { sql: string } ).sql - ).toContain("mailbox_handle != ''") + ).toContain('acknowledged_at IS NULL AND fenced = 0') }) }) diff --git a/src/main/runtime/orchestration/types.ts b/src/main/runtime/orchestration/types.ts index 85d5dcfc1596..d8104136305e 100644 --- a/src/main/runtime/orchestration/types.ts +++ b/src/main/runtime/orchestration/types.ts @@ -52,15 +52,13 @@ export type RunRow = { updated_at: string } -export type DeliveryStatus = 'outstanding' | 'acknowledged' | 'fenced' - export type DeliveryRow = { id: string run_id: string mailbox_handle: string | null consumer_generation: number message_ids: string - status: DeliveryStatus + fenced: number created_at: string acknowledged_at: string | null } diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts b/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts index e3713bd31a26..75f69c06ee5d 100644 --- a/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts +++ b/src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts @@ -40,7 +40,7 @@ describe('Run delivery history', () => { ) expect(history).toMatchObject({ count: 1, messages: [{ subject: 'later completion' }] }) expect(history).not.toHaveProperty('deliveryId') - expect(db.getDeliveryRaw(delivery.delivery.id)?.status).toBe('outstanding') + expect(db.hasOutstandingRunDelivery(activeRunId!)).toBe(true) expect(db.getMessageById(delivery.messages[0].id)?.read).toBe(0) }) }) diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/check-worker.ts b/src/main/runtime/rpc/methods/orchestration/messaging/check-worker.ts index 355a12be5c1d..fc9fa6b04e9d 100644 --- a/src/main/runtime/rpc/methods/orchestration/messaging/check-worker.ts +++ b/src/main/runtime/rpc/methods/orchestration/messaging/check-worker.ts @@ -172,6 +172,7 @@ export async function checkWorkerMailbox(args: { runId: deliveryRunId, mailboxHandle: address, consumerGeneration: workerMailbox.generation, + consumerSource: activeDispatch ? 'dispatch' : 'attachment', deliveryId: params.ack }) : undefined @@ -181,16 +182,12 @@ export async function checkWorkerMailbox(args: { const showAll = params.all === true || (params.unread === false && params.peek !== true) const readPeek = () => db.getUnreadMessages(address, typeFilter) const readDelivery = (wakeTypes?: MessageType[]) => { - // Why: re-read live, or a re-attach landing on an await above mints a Delivery at a generation - // the row has already left, which then fences the legitimate worker on every later check. - if (readCurrentGeneration() !== workerMailbox.generation) { - throw dispatchFenced() - } try { return db.getOrCreateMailboxDelivery({ runId: deliveryRunId, mailboxHandle: address, consumerGeneration: workerMailbox.generation, + consumerSource: activeDispatch ? 'dispatch' : 'attachment', wakeTypes }) } catch (error) { From 0e70dbcd406487dd720522acbbbb9af2891189d5 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 10 Sep 2026 18:12:50 -0400 Subject: [PATCH 5/9] fix(orchestration): validate live consumers and simplify batch revocation --- .../orchestration-delivery-storage.md | 4 +- .../dispatch-context/dispatch-capability.ts | 2 +- .../remote-dispatch-attachment-authority.ts | 2 +- ...mailbox-consumer-lifecycle-fencing.test.ts | 234 ++++++++++++++++++ .../db/messages/mailbox-consumer.ts | 14 +- .../db/messages/role-mailbox-delivery.ts | 8 +- .../orchestration/db/runs/run-binding.ts | 2 +- .../orchestration/db/runs/run-lookup.ts | 10 +- .../schema/derived-delivery-migration.test.ts | 11 + .../worker-dispatch-authority.ts | 2 +- 10 files changed, 269 insertions(+), 20 deletions(-) create mode 100644 src/main/runtime/orchestration/db/messages/mailbox-consumer-lifecycle-fencing.test.ts diff --git a/docs/reference/orchestration-delivery-storage.md b/docs/reference/orchestration-delivery-storage.md index ecb0ee2c4027..bf4d4f927e4c 100644 --- a/docs/reference/orchestration-delivery-storage.md +++ b/docs/reference/orchestration-delivery-storage.md @@ -1,6 +1,6 @@ # Orchestration delivery storage -A message records whether it still needs attention (`read`). A delivery records a stable batch ID, ordered message IDs, mailbox, consumer generation, and creation time. Acknowledgement time and fencing record actual events. There is no stored delivery status. +A message records whether it still needs attention (`read`). A delivery records a stable batch ID, ordered message IDs, mailbox, consumer generation, and creation time. Acknowledgement time records receipt. Fencing permanently revokes an unacknowledged batch when its consumer is replaced or legacy ownership is adopted, independently of message reads. Historical adoption can revoke a batch without changing its generation, so generation validation alone cannot replace this fact. There is no stored delivery status. `outstanding_deliveries` is a SQLite view, not a table or cached projection. It selects batches that have not been acknowledged or fenced and still contain unread messages. Both consuming checks and notification eligibility query this view. @@ -8,7 +8,7 @@ When completion makes an earlier heartbeat obsolete, only the message changes. I Acknowledgement marks the batch's messages read and records `acknowledged_at` in one transaction. A first explicit acknowledgement after lifecycle suppression records the actual event; subsequent acknowledgements are idempotent. Suppression never invents an acknowledgement timestamp. An acknowledgement cannot consume newer messages outside the batch. -Creation and acknowledgement use `BEGIN IMMEDIATE` and validate the current consumer inside that transaction. Run coordinators use the Run generation. Workers use the generation on their local Dispatch or federated attachment, according to the caller's existing routing. Loopback federation can contain both records, so the source is explicit. The insertion constraint consults the same view to prevent two outstanding batches in a mailbox, while allowing historical batches. +Creation and acknowledgement use `BEGIN IMMEDIATE` and validate the current consumer inside that transaction. Worker validation also checks lifecycle state under the same lock, so a settled worker cannot consume mail awaiting rerouting. Run coordinators use the Run generation. Workers use the generation on their local Dispatch or federated attachment, according to the caller's existing routing. Loopback federation can contain both records, so the source is explicit. The insertion constraint consults the same view to prevent two outstanding batches in a mailbox, while allowing historical batches. ## Migration and compatibility diff --git a/src/main/runtime/orchestration/db/dispatch-context/dispatch-capability.ts b/src/main/runtime/orchestration/db/dispatch-context/dispatch-capability.ts index 4f6861a17d0a..007e81d10164 100644 --- a/src/main/runtime/orchestration/db/dispatch-context/dispatch-capability.ts +++ b/src/main/runtime/orchestration/db/dispatch-context/dispatch-capability.ts @@ -38,7 +38,7 @@ export function mintDispatchCapability( params.processIncarnation, params.dispatchId ) - this.fenceOutstandingMailboxDelivery(`dispatch:${params.dispatchId}`) + this.fenceUnacknowledgedMailboxDeliveries(`dispatch:${params.dispatchId}`) this.db.exec('COMMIT') } catch (error) { this.db.exec('ROLLBACK') diff --git a/src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-authority.ts b/src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-authority.ts index 5b2dc60615c6..ac85fc7c3c99 100644 --- a/src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-authority.ts +++ b/src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-authority.ts @@ -71,7 +71,7 @@ export function prepareRemoteAttachmentAuthority( `Remote Dispatch ${params.dispatchId} is not starting.` ) } - this.fenceOutstandingMailboxDelivery(`dispatch:${params.dispatchId}`) + this.fenceUnacknowledgedMailboxDeliveries(`dispatch:${params.dispatchId}`) if (params.terminalOwnership && !this.getWorkerTerminalResourceByOwner(params.dispatchId)) { const resource = params.terminalOwnership === 'external' diff --git a/src/main/runtime/orchestration/db/messages/mailbox-consumer-lifecycle-fencing.test.ts b/src/main/runtime/orchestration/db/messages/mailbox-consumer-lifecycle-fencing.test.ts new file mode 100644 index 000000000000..3f81901b208a --- /dev/null +++ b/src/main/runtime/orchestration/db/messages/mailbox-consumer-lifecycle-fencing.test.ts @@ -0,0 +1,234 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { ORCHESTRATION_CONTRACT_VERSION } from '../../../../../shared/protocol-version' +import { OrchestrationDb } from '../orchestration-db' +import { createRootDispatch } from '../root-dispatch-test-fixture' + +type Settlement = 'local completion' | 'local failure' | 'remote stop' | 'remote failure' +type DeliveryOperation = 'create' | 'acknowledge' + +describe('mailbox consumer lifecycle fencing', () => { + const connections: OrchestrationDb[] = [] + const directories: string[] = [] + + afterEach(() => { + for (const db of connections.splice(0)) { + db.close() + } + for (const directory of directories.splice(0)) { + rmSync(directory, { recursive: true, force: true }) + } + }) + + function open(path: string): OrchestrationDb { + const db = new OrchestrationDb(path) + connections.push(db) + return db + } + + function databasePath(): string { + const directory = mkdtempSync(join(tmpdir(), 'orca-mailbox-consumer-lifecycle-')) + directories.push(directory) + return join(directory, 'orchestration.db') + } + + function setup(settlement: Settlement): { + db: OrchestrationDb + peer: OrchestrationDb + messageId: string + params: { + runId: string + mailboxHandle: string + consumerGeneration: number + consumerSource: 'dispatch' | 'attachment' + } + settle: () => void + } { + const path = databasePath() + const db = open(path) + const run = db.createRun({ + objective: 'Fence settled mailbox consumers', + coordinatorHandle: 'coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const remote = settlement.startsWith('remote') + const dispatchId = remote + ? `ctx_${settlement.replace(' ', '_')}` + : createRootDispatch(db, db.createTask({ runId: run.id, spec: settlement }).id, 'worker').id + let consumerGeneration = 0 + + if (remote) { + db.createRemoteDispatchAttachment({ + runId: run.id, + dispatchId, + taskId: `task_${dispatchId}`, + homePeerFingerprint: 'home-peer', + protocolVersion: ORCHESTRATION_CONTRACT_VERSION, + runtimeEpoch: 'epoch-1', + mutationReceipt: { + callerFingerprint: 'home-peer', + requestId: `request_${dispatchId}`, + method: 'orchestration.federationAttachStart', + payloadHash: `hash_${dispatchId}` + } + }) + if (settlement === 'remote stop') { + db.prepareRemoteAttachmentAuthority({ + dispatchId, + paneKey: 'worker:22222222-2222-4222-9222-222222222222', + processIncarnation: 'runtime:worker:1', + worktreeId: 'folder', + terminalHandle: 'worker', + setupState: 'not_applicable', + effects: [] + }) + db.markRemoteAttachmentReady(dispatchId) + consumerGeneration = 1 + } + } + + const mailboxHandle = `dispatch:${dispatchId}` + const message = db.insertMessage({ + runId: run.id, + from: 'coord', + to: mailboxHandle, + subject: 'must remain unread' + }) + const peer = open(path) + const settle = (): void => { + if (settlement === 'local completion') { + peer.completeDispatch(dispatchId) + } else if (settlement === 'local failure') { + peer.failDispatch(dispatchId, 'settled by peer') + } else if (settlement === 'remote stop') { + peer.beginRemoteAttachmentStop(dispatchId) + peer.settleRemoteAttachmentStop(dispatchId) + } else { + peer.failRemoteAttachment(dispatchId, 'peer_failure', 'settled by peer', false) + } + } + + return { + db, + peer, + messageId: message.id, + params: { + runId: run.id, + mailboxHandle, + consumerGeneration, + consumerSource: remote ? 'attachment' : 'dispatch' + }, + settle + } + } + + function currentGeneration( + db: OrchestrationDb, + params: { + mailboxHandle: string + consumerSource: 'dispatch' | 'attachment' + } + ): number | undefined { + const dispatchId = params.mailboxHandle.slice('dispatch:'.length) + return params.consumerSource === 'dispatch' + ? db.getDispatchContextById(dispatchId)?.consumer_generation + : db.getRemoteDispatchAttachment(dispatchId)?.consumer_generation + } + + it.each<{ + operation: DeliveryOperation + settlement: Settlement + }>([ + { operation: 'create', settlement: 'local completion' }, + { operation: 'create', settlement: 'local failure' }, + { operation: 'create', settlement: 'remote stop' }, + { operation: 'create', settlement: 'remote failure' }, + { operation: 'acknowledge', settlement: 'local completion' }, + { operation: 'acknowledge', settlement: 'local failure' }, + { operation: 'acknowledge', settlement: 'remote stop' }, + { operation: 'acknowledge', settlement: 'remote failure' } + ])('rejects $operation after $settlement on another connection', ({ operation, settlement }) => { + const { db, peer, messageId, params, settle } = setup(settlement) + const delivery = + operation === 'acknowledge' ? db.getOrCreateMailboxDelivery(params)?.delivery : undefined + + settle() + + const operationCall = (): unknown => + operation === 'create' + ? db.getOrCreateMailboxDelivery(params) + : db.acknowledgeMailboxDelivery({ ...params, deliveryId: delivery!.id }) + expect(operationCall).toThrow(expect.objectContaining({ code: 'consumer_fenced' })) + expect(db.getMessageById(messageId)?.read).toBe(0) + expect(currentGeneration(peer, params)).toBe(params.consumerGeneration) + if (delivery) { + expect(db.getDeliveryRaw(delivery.id)?.acknowledged_at).toBeNull() + } + }) + + it.each(['start_unknown', 'stop_unknown'] as const)( + 'keeps a remote %s attachment eligible to consume mail', + (state) => { + const path = databasePath() + const db = open(path) + const run = db.createRun({ + objective: 'Preserve unverifiable remote consumers', + coordinatorHandle: 'coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const dispatchId = `ctx_${state}` + db.createRemoteDispatchAttachment({ + runId: run.id, + dispatchId, + taskId: `task_${state}`, + homePeerFingerprint: 'home-peer', + protocolVersion: ORCHESTRATION_CONTRACT_VERSION, + runtimeEpoch: 'epoch-1', + mutationReceipt: { + callerFingerprint: 'home-peer', + requestId: `request_${state}`, + method: 'orchestration.federationAttachStart', + payloadHash: `hash_${state}` + } + }) + let consumerGeneration = 0 + if (state === 'start_unknown') { + db.failRemoteAttachment(dispatchId, 'start_unknown', 'contact lost', true) + } else { + db.prepareRemoteAttachmentAuthority({ + dispatchId, + paneKey: 'worker:22222222-2222-4222-9222-222222222222', + processIncarnation: 'runtime:worker:1', + worktreeId: 'folder', + terminalHandle: 'worker', + setupState: 'not_applicable', + effects: [] + }) + db.markRemoteAttachmentReady(dispatchId) + db.beginRemoteAttachmentStop(dispatchId) + db.markRemoteAttachmentStopUnknown(dispatchId, 'contact lost') + consumerGeneration = 1 + } + const mailboxHandle = `dispatch:${dispatchId}` + const message = db.insertMessage({ + runId: run.id, + from: 'coord', + to: mailboxHandle, + subject: 'still deliverable' + }) + + expect( + db + .getOrCreateMailboxDelivery({ + runId: run.id, + mailboxHandle, + consumerGeneration, + consumerSource: 'attachment' + }) + ?.messages.map((row) => row.id) + ).toEqual([message.id]) + } + ) +}) diff --git a/src/main/runtime/orchestration/db/messages/mailbox-consumer.ts b/src/main/runtime/orchestration/db/messages/mailbox-consumer.ts index 9ed2b2d4cc96..66d72fb63d4f 100644 --- a/src/main/runtime/orchestration/db/messages/mailbox-consumer.ts +++ b/src/main/runtime/orchestration/db/messages/mailbox-consumer.ts @@ -1,5 +1,15 @@ import type { OrchestrationDb } from '../orchestration-db' import { OrchestrationError } from '../../orchestration-error' +import { potentiallyLiveRemoteAttachmentSql } from '../federation/remote-attachment-liveness' + +const ACTIVE_DISPATCH_CONSUMER_SQL = ` + SELECT run_id, consumer_generation FROM dispatch_contexts + WHERE id = ? AND status IN ('pending', 'dispatched') +` +const ACTIVE_ATTACHMENT_CONSUMER_SQL = ` + SELECT home_run_id AS run_id, consumer_generation FROM remote_dispatch_attachments + WHERE dispatch_id = ? AND ${potentiallyLiveRemoteAttachmentSql()} +` // Validate inside the delivery transaction, so another connection cannot replace the consumer mid-check. export function requireMailboxConsumer( @@ -21,8 +31,8 @@ export function requireMailboxConsumer( // A loopback runtime has both records; use the counter belonging to the caller's attachment. const sql = params.consumerSource === 'attachment' - ? 'SELECT home_run_id AS run_id, consumer_generation FROM remote_dispatch_attachments WHERE dispatch_id = ?' - : 'SELECT run_id, consumer_generation FROM dispatch_contexts WHERE id = ?' + ? ACTIVE_ATTACHMENT_CONSUMER_SQL + : ACTIVE_DISPATCH_CONSUMER_SQL const consumer = db.db.prepare(sql).get(dispatchId) as | { run_id: string; consumer_generation: number } | undefined diff --git a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts index d5ff9e04221a..8c50b8ccf2fb 100644 --- a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts +++ b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts @@ -177,14 +177,14 @@ export function hasOutstandingMailboxDelivery( ) } -export function fenceOutstandingMailboxDelivery( +export function fenceUnacknowledgedMailboxDeliveries( this: OrchestrationDb, mailboxHandle: string ): void { this.db .prepare( `UPDATE deliveries SET fenced = 1 - WHERE id IN (SELECT id FROM outstanding_deliveries WHERE mailbox_handle = ?)` + WHERE mailbox_handle = ? AND acknowledged_at IS NULL AND fenced = 0` ) .run(mailboxHandle) } @@ -195,7 +195,7 @@ export type RoleMailboxDeliveryMethods = { getOrCreateMailboxDelivery: typeof getOrCreateMailboxDelivery acknowledgeMailboxDelivery: typeof acknowledgeMailboxDelivery hasOutstandingMailboxDelivery: typeof hasOutstandingMailboxDelivery - fenceOutstandingMailboxDelivery: typeof fenceOutstandingMailboxDelivery + fenceUnacknowledgedMailboxDeliveries: typeof fenceUnacknowledgedMailboxDeliveries } export function attachRoleMailboxDelivery(ctor: { prototype: object }): void { @@ -205,6 +205,6 @@ export function attachRoleMailboxDelivery(ctor: { prototype: object }): void { getOrCreateMailboxDelivery, acknowledgeMailboxDelivery, hasOutstandingMailboxDelivery, - fenceOutstandingMailboxDelivery + fenceUnacknowledgedMailboxDeliveries }) } diff --git a/src/main/runtime/orchestration/db/runs/run-binding.ts b/src/main/runtime/orchestration/db/runs/run-binding.ts index a4110dd589f9..e2ff77ec8182 100644 --- a/src/main/runtime/orchestration/db/runs/run-binding.ts +++ b/src/main/runtime/orchestration/db/runs/run-binding.ts @@ -142,7 +142,7 @@ export function bindRun( WHERE id = ?` ) .run(params.coordinatorHandle, params.coordinatorPaneKey, params.runId) - this.fenceOutstandingDelivery(params.runId) + this.fenceUnacknowledgedMailboxDeliveries(`run:${params.runId}`) if (params.takeoverLegacy || replacesLegacyCoordinator) { this.promoteLegacyCoordinatorMailForTakeover(params.runId, retainedCoordinatorHandle) } diff --git a/src/main/runtime/orchestration/db/runs/run-lookup.ts b/src/main/runtime/orchestration/db/runs/run-lookup.ts index 7563effa8c3d..194cceffcf0c 100644 --- a/src/main/runtime/orchestration/db/runs/run-lookup.ts +++ b/src/main/runtime/orchestration/db/runs/run-lookup.ts @@ -141,7 +141,7 @@ export function unbindOtherRunsForPane( WHERE id = ?` ) .run(run.id) - this.fenceOutstandingDelivery(run.id) + this.fenceUnacknowledgedMailboxDeliveries(`run:${run.id}`) } } } @@ -152,10 +152,6 @@ export function requireRun(this: OrchestrationDb, runId: string): void { } } -export function fenceOutstandingDelivery(this: OrchestrationDb, runId: string): void { - this.fenceOutstandingMailboxDelivery(`run:${runId}`) -} - export type RunLookupMethods = { getRun: typeof getRun getLegacyAdoptedRunMailboxOwner: typeof getLegacyAdoptedRunMailboxOwner @@ -166,7 +162,6 @@ export type RunLookupMethods = { getRunRaw: typeof getRunRaw unbindOtherRunsForPane: typeof unbindOtherRunsForPane requireRun: typeof requireRun - fenceOutstandingDelivery: typeof fenceOutstandingDelivery } export function attachRunLookup(ctor: { prototype: object }): void { @@ -179,7 +174,6 @@ export function attachRunLookup(ctor: { prototype: object }): void { runsBoundToPane, getRunRaw, unbindOtherRunsForPane, - requireRun, - fenceOutstandingDelivery + requireRun }) } diff --git a/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts b/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts index fb83990d8221..633541f8d355 100644 --- a/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts +++ b/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts @@ -72,6 +72,9 @@ describe('derived delivery migration', () => { fenced: 0 }) expect(db.getDeliveryRaw('history_fence')).toMatchObject({ acknowledged_at: null, fenced: 1 }) + expect(() => db.acknowledgeRunDelivery({ ...params, deliveryId: 'history_fence' })).toThrow( + expect.objectContaining({ code: 'consumer_fenced' }) + ) expect(db.getOrCreateRunDelivery(params)?.messages.map((message) => message.id)).toEqual([ next.id ]) @@ -139,6 +142,10 @@ describe('derived delivery migration', () => { coordinatorHandle: 'replacement', coordinatorPaneKey: 'other:22222222-2222-4222-9222-222222222222' })! + expect(first.getDeliveryRaw(batch.delivery.id)).toMatchObject({ + fenced: 1, + acknowledged_at: null + }) first.insertMessage({ runId: run.id, from: 'worker', to: `run:${run.id}`, subject: 'next' }) expect(() => first.getOrCreateRunDelivery(params)).toThrow( expect.objectContaining({ code: 'consumer_fenced' }) @@ -211,6 +218,10 @@ describe('derived delivery migration', () => { effects: [] }) } + expect(db.getDeliveryRaw(batch.delivery.id)).toMatchObject({ + fenced: 1, + acknowledged_at: null + }) peer.insertMessage({ runId: run.id, from: 'coord', to: mailboxHandle, subject: 'next' }) expect(() => db.getOrCreateMailboxDelivery(params)).toThrow( expect.objectContaining({ code: 'consumer_fenced' }) diff --git a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts index 57ec354a8cce..b33a8798dc3e 100644 --- a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts +++ b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts @@ -75,7 +75,7 @@ export function prepareStartingWorkerAuthority( `Dispatch ${params.dispatchId} is not starting.` ) } - this.fenceOutstandingMailboxDelivery(`dispatch:${params.dispatchId}`) + this.fenceUnacknowledgedMailboxDeliveries(`dispatch:${params.dispatchId}`) const workerUpdate = this.db .prepare( `UPDATE worker_dispatches From a4115b2c7a170c83d829643dc8d36f33c38e043a Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 11 Sep 2026 23:19:46 -0400 Subject: [PATCH 6/9] refactor(orchestration): keep deliveries.status and derive eligibility without a column drop The outstanding_deliveries view now reads status = 'outstanding' plus unread membership, so v41 only drops uniqueness from idx_deliveries_one_outstanding and adds the view and trigger. Older binaries can still open the database. Removes the column-drop migration, the v40 test fixture and hasColumn guards, the fenced skew probe, and the unrelated nudge-text change. --- .../orchestration-delivery-storage.md | 18 ++--- docs/reference/windows-process-enumeration.md | 4 +- ...creation-and-orchestration-part-02.spec.ts | 4 +- ...creation-and-orchestration-part-03.spec.ts | 2 +- ...stration-dispatch-mailbox-delivery.test.ts | 2 +- ...stration-message-delivery-identity.test.ts | 14 ++-- src/main/runtime/orchestration/db.ts | 1 + .../dispatch-mailbox-consumer-fencing.test.ts | 6 +- .../db/messages/role-mailbox-delivery.ts | 14 ++-- .../db/schema/adopt-legacy-run.ts | 6 +- .../orchestration/db/schema/create-tables.ts | 2 + .../db/schema/delivery-v40-test-fixture.ts | 30 -------- .../schema/derived-delivery-migration.test.ts | 43 ++++++++--- .../schema/derived-delivery-test-fixture.ts | 9 +++ .../migrate-role-mailbox-delivery-v34.ts | 2 +- .../orchestration/db/schema/migrate-v2-v12.ts | 3 +- .../orchestration/db/schema/migrate-v35.ts | 16 ++--- .../orchestration/db/schema/migrate-v41.ts | 72 ++++++++----------- .../orchestration/db/schema/migrate.ts | 7 +- .../db/schema/schema-column-probes.ts | 10 +-- ...atch-consumer-generation-migration.test.ts | 8 +-- ...ederation-acknowledgment-migration.test.ts | 4 +- .../runtime/orchestration/formatter.test.ts | 2 +- src/main/runtime/orchestration/formatter.ts | 2 +- .../nested-worker-depth-migration.test.ts | 6 +- .../orchestration-derived-delivery.test.ts | 7 -- ...chestration-federated-legacy-probe.test.ts | 8 +-- .../orchestration-legacy-storage-db.test.ts | 2 +- ...chestration-legacy-storage-test-fixture.ts | 4 +- .../orchestration-schema-version-skew.ts | 26 ++----- ...chestration-version-skew-migration.test.ts | 42 ++++------- src/main/runtime/orchestration/types.ts | 4 +- 32 files changed, 166 insertions(+), 214 deletions(-) delete mode 100644 src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts create mode 100644 src/main/runtime/orchestration/db/schema/derived-delivery-test-fixture.ts diff --git a/docs/reference/orchestration-delivery-storage.md b/docs/reference/orchestration-delivery-storage.md index bf4d4f927e4c..58bb6bf6250c 100644 --- a/docs/reference/orchestration-delivery-storage.md +++ b/docs/reference/orchestration-delivery-storage.md @@ -1,19 +1,11 @@ # Orchestration delivery storage -A message records whether it still needs attention (`read`). A delivery records a stable batch ID, ordered message IDs, mailbox, consumer generation, and creation time. Acknowledgement time records receipt. Fencing permanently revokes an unacknowledged batch when its consumer is replaced or legacy ownership is adopted, independently of message reads. Historical adoption can revoke a batch without changing its generation, so generation validation alone cannot replace this fact. There is no stored delivery status. +A message records whether it still needs attention (`read`). A delivery is a stable batch receipt: ID, ordered message IDs, mailbox, consumer generation, creation time, and `status`. `status` only records terminal facts written by an explicit event: `acknowledged` when the consumer acks, `fenced` when the consumer is replaced or legacy ownership is adopted. `outstanding` means neither has happened; it does not mean the batch is still eligible. -`outstanding_deliveries` is a SQLite view, not a table or cached projection. It selects batches that have not been acknowledged or fenced and still contain unread messages. Both consuming checks and notification eligibility query this view. +Eligibility is derived, never stored. `outstanding_deliveries` is a SQLite view over batches that are `outstanding` and still contain at least one unread message. Consuming checks, replay, notification pointers, and the single-active-batch insert trigger all read this view. When lifecycle reconciliation marks a heartbeat read, its batch stops appearing without any delivery write, so a later `worker_done` is never hidden behind it. A partially read batch still replays its complete original membership under the same ID. -When completion makes an earlier heartbeat obsolete, only the message changes. Its batch stops appearing in the view without a second write. Checking an old database with a fully read heartbeat batch requires no repair operation. A partially read batch still replays its complete original membership under the same ID. +Creation and acknowledgement run under `BEGIN IMMEDIATE` and validate the current consumer inside that transaction: Run coordinators against the Run generation, workers against their local Dispatch or federated attachment, chosen explicitly by the caller because a loopback runtime has both. Worker validation also requires a live lifecycle state, so a settled worker cannot consume mail awaiting rerouting; remote states with unverifiable liveness stay eligible. -Acknowledgement marks the batch's messages read and records `acknowledged_at` in one transaction. A first explicit acknowledgement after lifecycle suppression records the actual event; subsequent acknowledgements are idempotent. Suppression never invents an acknowledgement timestamp. An acknowledgement cannot consume newer messages outside the batch. +## Compatibility -Creation and acknowledgement use `BEGIN IMMEDIATE` and validate the current consumer inside that transaction. Worker validation also checks lifecycle state under the same lock, so a settled worker cannot consume mail awaiting rerouting. Run coordinators use the Run generation. Workers use the generation on their local Dispatch or federated attachment, according to the caller's existing routing. Loopback federation can contain both records, so the source is explicit. The insertion constraint consults the same view to prevent two outstanding batches in a mailbox, while allowing historical batches. - -## Migration and compatibility - -Schema v41 removes `deliveries.status` and its partial unique index. It preserves batch IDs, ordered membership, mailbox addresses, generations, creation times, and acknowledgement times. The old fenced status becomes a boolean fencing fact. The new view replaces stored eligibility; an insertion trigger replaces the old uniqueness backstop. - -This is a one-way local database upgrade for shipped versions that query the removed status column. Those versions cannot open the upgraded database. Rolling the app back requires a compatible pre-upgrade database backup; this migration does not create a backup. A future reverse migration would need to materialize the old status before using an older binary. - -Mixed client/server versions are a different boundary. The existing check RPC returns delivery IDs and messages, not the storage row's status. Its response shape and acknowledgement commands remain compatible, including on SSH and federated worker hosts. This migration is owned by the runtime that opens the database, not by a paired client. +Schema v41 drops the unique constraint from `idx_deliveries_one_outstanding` (name and predicate unchanged) and adds the view and trigger. No column changes. An older binary that opens a v41 database keeps working: it sees a higher `user_version` and skips migration, its `status` queries still resolve, and the index it expects still exists. diff --git a/docs/reference/windows-process-enumeration.md b/docs/reference/windows-process-enumeration.md index fac8f7c58d16..685103ba9ba6 100644 --- a/docs/reference/windows-process-enumeration.md +++ b/docs/reference/windows-process-enumeration.md @@ -583,8 +583,8 @@ breakaway hands the whole tree its escape. The per-PTY job therefore omits `BREAKAWAY_OK` whenever `msys-2.0.dll` or `cygwin1.dll` sits on the shell's DLL search path — beside the executable, or under `usr/bin` for Git's `bin` launcher. Native shells keep explicit breakaway. Denying it costs Cygwin -nothing, because it *pre-checks* the limit rather than retrying, so no spawn -fails; but a *native* program that passes `CREATE_BREAKAWAY_FROM_JOB` itself +nothing, because it _pre-checks_ the limit rather than retrying, so no spawn +fails; but a _native_ program that passes `CREATE_BREAKAWAY_FROM_JOB` itself inside such a pane now gets `ERROR_ACCESS_DENIED`. `nohup` and `disown` are unaffected — they are Cygwin signal/session concepts, unrelated to job membership. The daemon's host job is unchanged. diff --git a/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.ts b/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.ts index fb5396bf37c7..9291c30c1a2e 100644 --- a/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.ts +++ b/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.ts @@ -94,7 +94,7 @@ describe('OrcaRuntimeService', () => { runtime.onPtyData('pty-1', '\x1b]0;Codex done\x07', 101) expect(write).toHaveBeenCalledWith( 'pty-1', - '\n[Orca orchestration notification] You have 1 orchestration message. Run `orca-dev orchestration check --run run_mailbox`.\n' + '\nYou have 1 orchestration message. Run `orca-dev orchestration check --run run_mailbox`.\n' ) expect(write).not.toHaveBeenCalledWith( 'pty-1', @@ -472,7 +472,7 @@ describe('OrcaRuntimeService', () => { await vi.waitFor(() => { expect(write).toHaveBeenCalledWith( 'pty-1', - '\n[Orca orchestration notification] You have 1 orchestration message. Run `orca-dev orchestration check --run run_codex_native_title`.\n' + '\nYou have 1 orchestration message. Run `orca-dev orchestration check --run run_codex_native_title`.\n' ) }) db.close() diff --git a/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec.ts b/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec.ts index ec36fb9fc8ba..48cd70703e08 100644 --- a/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec.ts +++ b/src/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec.ts @@ -62,7 +62,7 @@ describe('OrcaRuntimeService', () => { .map(([, data]) => data) .filter((data): data is string => typeof data === 'string') expect(payloads).toContain( - '\n[Orca orchestration notification] You have 1 orchestration message. Run `orca-dev orchestration check --run run_test`.\n' + '\nYou have 1 orchestration message. Run `orca-dev orchestration check --run run_test`.\n' ) expect(payloads.some((data) => data.includes('reserved completion'))).toBe(false) expect(status.delivered_at).toEqual(expect.any(String)) diff --git a/src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts b/src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts index 1b86ae9bab57..958a93392927 100644 --- a/src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts +++ b/src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts @@ -204,7 +204,7 @@ describe('Dispatch mailbox Delivery', () => { coordinatorPaneKey: 'tab_new:leaf_new' }) - expect(db.hasOutstandingMailboxDelivery(`dispatch:${dispatch.id}`)).toBe(true) + expect(db.getDeliveryRaw(delivery.delivery.id)?.status).toBe('outstanding') expect( db.getOrCreateMailboxDelivery({ runId: run.id, diff --git a/src/main/runtime/orchestration-message-delivery-identity.test.ts b/src/main/runtime/orchestration-message-delivery-identity.test.ts index ff02135eb2e4..21c8a64f56a0 100644 --- a/src/main/runtime/orchestration-message-delivery-identity.test.ts +++ b/src/main/runtime/orchestration-message-delivery-identity.test.ts @@ -237,10 +237,10 @@ describe('STA-4325 message and delivery identity', () => { expect(all.count).toBe(3) const deliveryRowsBeforeAck = sqliteFor(db) - .prepare('SELECT id, acknowledged_at, message_ids FROM deliveries ORDER BY rowid') + .prepare('SELECT id, status, message_ids FROM deliveries ORDER BY rowid') .all() as { id: string; status: string; message_ids: string }[] expect(deliveryRowsBeforeAck).toEqual([ - { id: first.deliveryId, acknowledged_at: null, message_ids: JSON.stringify(expectedIds) } + { id: first.deliveryId, status: 'outstanding', message_ids: JSON.stringify(expectedIds) } ]) for (const id of expectedIds) { expect(db.getMessageById(id)).toMatchObject({ to_handle: `run:${run.id}`, read: 0 }) @@ -258,9 +258,9 @@ describe('STA-4325 message and delivery identity', () => { expect.arrayContaining(expectedIds) ) expect(acknowledgedHistory.count).toBe(3) - expect( - sqliteFor(db).prepare('SELECT id, acknowledged_at FROM deliveries ORDER BY rowid').all() - ).toEqual([{ id: first.deliveryId, acknowledged_at: expect.any(String) }]) + expect(sqliteFor(db).prepare('SELECT id, status FROM deliveries ORDER BY rowid').all()).toEqual( + [{ id: first.deliveryId, status: 'acknowledged' }] + ) for (const id of expectedIds) { expect(db.getMessageById(id)?.read).toBe(1) } @@ -488,8 +488,8 @@ describe('STA-4325 message and delivery identity', () => { deliveryId: firstPayload.result.deliveryId, replayed: true }) - expect(sqliteFor(db).prepare('SELECT id, acknowledged_at FROM deliveries').all()).toEqual([ - { id: firstPayload.result.deliveryId, acknowledged_at: null } + expect(sqliteFor(db).prepare('SELECT id, status FROM deliveries').all()).toEqual([ + { id: firstPayload.result.deliveryId, status: 'outstanding' } ]) expect(db.getMessageById(status.id)).toMatchObject({ to_handle: `run:${run.id}`, diff --git a/src/main/runtime/orchestration/db.ts b/src/main/runtime/orchestration/db.ts index 5669e28aa990..4af72ff07e5c 100644 --- a/src/main/runtime/orchestration/db.ts +++ b/src/main/runtime/orchestration/db.ts @@ -45,6 +45,7 @@ export type { WorkerReportSettlement, RunRow, DeliveryRow, + DeliveryStatus, LegacyAdoptionRow, LegacyCompatibilityPrincipalRow, LegacyPrincipalRole, diff --git a/src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts b/src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts index 110ec2bbb493..608b91a2586e 100644 --- a/src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts +++ b/src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts @@ -70,7 +70,7 @@ describe('dispatch mailbox consumer fencing', () => { }) const generationB = generationOf(dispatch.id) expect(generationB).toBe(generationA + 1) - expect(db.getDeliveryRaw(deliveryA!.delivery.id)?.fenced).toBe(1) + expect(db.getDeliveryRaw(deliveryA!.delivery.id)?.status).toBe('fenced') expect(() => db.acknowledgeMailboxDelivery({ @@ -151,7 +151,7 @@ describe('dispatch mailbox consumer fencing', () => { }) expect(generationOf(dispatchId)).toBe(1) - expect(db.getDeliveryRaw(stale!.delivery.id)?.fenced).toBe(1) + expect(db.getDeliveryRaw(stale!.delivery.id)?.status).toBe('fenced') }) it('gives a federated attachment its own generation on the worker host', () => { @@ -192,7 +192,7 @@ describe('dispatch mailbox consumer fencing', () => { }) expect(db.getRemoteDispatchAttachment(dispatchId)?.consumer_generation).toBe(1) - expect((db.getDeliveryRaw(stale!.delivery.id) as DeliveryRow).fenced).toBe(1) + expect((db.getDeliveryRaw(stale!.delivery.id) as DeliveryRow).status).toBe('fenced') }) it('starts a retry Dispatch on a fresh mailbox address rather than sharing the old one', () => { diff --git a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts index 8c50b8ccf2fb..6c96fc282ac5 100644 --- a/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts +++ b/src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts @@ -132,13 +132,16 @@ export function acknowledgeMailboxDelivery( `Delivery ${params.deliveryId} does not belong to this mailbox. --ack requires a delivery_* ID returned by orchestration check; process the entire batch before acknowledging.` ) } - if (delivery.consumer_generation !== params.consumerGeneration || delivery.fenced === 1) { + if ( + delivery.consumer_generation !== params.consumerGeneration || + delivery.status === 'fenced' + ) { throw new OrchestrationError( 'consumer_fenced', 'This mailbox Delivery belongs to a fenced consumer generation.' ) } - if (delivery.acknowledged_at !== null) { + if (delivery.status === 'acknowledged') { this.db.exec('COMMIT') return { delivery: exposeDeliveryTimestamps(delivery), duplicate: true } } @@ -155,7 +158,9 @@ export function acknowledgeMailboxDelivery( .run(...messageIds) } this.db - .prepare("UPDATE deliveries SET acknowledged_at = datetime('now') WHERE id = ?") + .prepare( + "UPDATE deliveries SET status = 'acknowledged', acknowledged_at = datetime('now') WHERE id = ?" + ) .run(delivery.id) const acknowledged = this.getDeliveryRaw(delivery.id) as DeliveryRow this.db.exec('COMMIT') @@ -183,8 +188,7 @@ export function fenceUnacknowledgedMailboxDeliveries( ): void { this.db .prepare( - `UPDATE deliveries SET fenced = 1 - WHERE mailbox_handle = ? AND acknowledged_at IS NULL AND fenced = 0` + "UPDATE deliveries SET status = 'fenced' WHERE mailbox_handle = ? AND status = 'outstanding'" ) .run(mailboxHandle) } diff --git a/src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts b/src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts index 67184333021b..a67a6a2f6047 100644 --- a/src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts +++ b/src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts @@ -57,10 +57,8 @@ export function adoptLegacyRunIfNeeded(this: OrchestrationDb): void { this.db .prepare( - this.hasColumn('deliveries', 'status') - ? `UPDATE deliveries SET status = 'fenced' WHERE run_id = ? AND status = 'outstanding'` - : `UPDATE deliveries SET fenced = 1 - WHERE run_id = ? AND acknowledged_at IS NULL AND fenced = 0` + `UPDATE deliveries SET status = 'fenced' + WHERE run_id = ? AND status = 'outstanding'` ) .run(LEGACY_RUN_ID) for (const table of [ diff --git a/src/main/runtime/orchestration/db/schema/create-tables.ts b/src/main/runtime/orchestration/db/schema/create-tables.ts index 70baf6eca74d..c0fa229eb3ce 100644 --- a/src/main/runtime/orchestration/db/schema/create-tables.ts +++ b/src/main/runtime/orchestration/db/schema/create-tables.ts @@ -1,10 +1,12 @@ import type { OrchestrationDb } from '../orchestration-db' import { createCoreTablesSql } from './create-core-tables-sql' import { createGraphTablesSql } from './create-graph-tables-sql' +import { DERIVED_DELIVERY_SCHEMA_SQL } from './migrate-v41' export function createTables(this: OrchestrationDb): void { this.db.exec(`${createCoreTablesSql()}\n${createGraphTablesSql()}`) this.createMailboxDeliveryIndexesIfPossible() + this.db.exec(DERIVED_DELIVERY_SCHEMA_SQL) } export type CreateTablesMethods = { diff --git a/src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts b/src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts deleted file mode 100644 index 1a296864b093..000000000000 --- a/src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts +++ /dev/null @@ -1,30 +0,0 @@ -import type { OrchestrationDb } from '../orchestration-db' - -// Older-version tests must seed the actual historical schema, not just lower user_version. -export function restoreV40DeliverySchema(db: OrchestrationDb['db']): void { - const columns = db.pragma('table_info(deliveries)') as { name: string }[] - if (!columns.some((column) => column.name === 'fenced')) { - return - } - db.exec(` - DROP TRIGGER IF EXISTS trg_deliveries_one_outstanding; - DROP VIEW IF EXISTS outstanding_deliveries; - CREATE TABLE deliveries_v40 ( - id TEXT PRIMARY KEY, run_id TEXT NOT NULL, mailbox_handle TEXT NOT NULL DEFAULT '', - consumer_generation INTEGER NOT NULL, message_ids TEXT NOT NULL, - status TEXT NOT NULL DEFAULT 'outstanding' CHECK(status IN ('outstanding', 'acknowledged', 'fenced')), - created_at TEXT NOT NULL DEFAULT (datetime('now')), acknowledged_at TEXT - ); - INSERT INTO deliveries_v40 - SELECT id, run_id, mailbox_handle, consumer_generation, message_ids, - CASE WHEN fenced = 1 THEN 'fenced' - WHEN acknowledged_at IS NOT NULL THEN 'acknowledged' ELSE 'outstanding' END, - created_at, acknowledged_at - FROM deliveries ORDER BY rowid; - DROP TABLE deliveries; - ALTER TABLE deliveries_v40 RENAME TO deliveries; - CREATE UNIQUE INDEX idx_deliveries_one_outstanding - ON deliveries(mailbox_handle) WHERE status = 'outstanding' AND mailbox_handle != ''; - CREATE INDEX idx_deliveries_run_created ON deliveries(run_id, created_at); - `) -} diff --git a/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts b/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts index 633541f8d355..a868a310e71e 100644 --- a/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts +++ b/src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts @@ -4,7 +4,7 @@ import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import Database from '../../../../sqlite/sync-database' import { OrchestrationDb } from '../orchestration-db' -import { restoreV40DeliverySchema } from './delivery-v40-test-fixture' +import { dropDerivedDeliverySchema } from './derived-delivery-test-fixture' import { resolveOrchestrationMigrationStartVersion } from '../../orchestration-schema-version-skew' import { createRootDispatch } from '../root-dispatch-test-fixture' import { SCHEMA_VERSION } from '../contract-constants' @@ -55,7 +55,7 @@ describe('derived delivery migration', () => { }) connections.pop()!.close() const raw = new Database(path) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) raw.prepare('UPDATE messages SET read = 1 WHERE id = ?').run(old.id) raw.exec(` INSERT INTO deliveries (id, run_id, mailbox_handle, consumer_generation, message_ids, status, created_at, acknowledged_at) @@ -69,9 +69,12 @@ describe('derived delivery migration', () => { expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) expect(db.getDeliveryRaw('history_ack')).toMatchObject({ acknowledged_at: '2026-01-02 00:00:00', - fenced: 0 + status: 'acknowledged' + }) + expect(db.getDeliveryRaw('history_fence')).toMatchObject({ + acknowledged_at: null, + status: 'fenced' }) - expect(db.getDeliveryRaw('history_fence')).toMatchObject({ acknowledged_at: null, fenced: 1 }) expect(() => db.acknowledgeRunDelivery({ ...params, deliveryId: 'history_fence' })).toThrow( expect.objectContaining({ code: 'consumer_fenced' }) ) @@ -104,16 +107,18 @@ describe('derived delivery migration', () => { const params = { runId: run.id, consumerGeneration: run.consumer_generation } const first = db.getDeliveryRaw(db.getOrCreateRunDelivery(params)!.delivery.id)! const insert = db.db.prepare(`INSERT INTO deliveries - (id, run_id, mailbox_handle, consumer_generation, message_ids, acknowledged_at, fenced) + (id, run_id, mailbox_handle, consumer_generation, message_ids, acknowledged_at, status) VALUES (?, ?, ?, ?, ?, ?, ?)`) const values = [run.id, `run:${run.id}`, run.consumer_generation, JSON.stringify([message.id])] - expect(() => insert.run('duplicate', ...values, null, 0)).toThrow( + expect(() => insert.run('duplicate', ...values, null, 'outstanding')).toThrow( 'Mailbox already has an outstanding delivery' ) - expect(() => insert.run('ack_history', ...values, '2026-01-01 00:00:00', 0)).not.toThrow() - expect(() => insert.run('fenced_history', ...values, null, 1)).not.toThrow() + expect(() => + insert.run('ack_history', ...values, '2026-01-01 00:00:00', 'acknowledged') + ).not.toThrow() + expect(() => insert.run('fenced_history', ...values, null, 'fenced')).not.toThrow() db.markAsRead([message.id]) - expect(() => insert.run('consumed_history', ...values, null, 0)).not.toThrow() + expect(() => insert.run('consumed_history', ...values, null, 'outstanding')).not.toThrow() expect(db.getDeliveryRaw(first.id)).toEqual(first) expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) }) @@ -143,7 +148,7 @@ describe('derived delivery migration', () => { coordinatorPaneKey: 'other:22222222-2222-4222-9222-222222222222' })! expect(first.getDeliveryRaw(batch.delivery.id)).toMatchObject({ - fenced: 1, + status: 'fenced', acknowledged_at: null }) first.insertMessage({ runId: run.id, from: 'worker', to: `run:${run.id}`, subject: 'next' }) @@ -219,7 +224,7 @@ describe('derived delivery migration', () => { }) } expect(db.getDeliveryRaw(batch.delivery.id)).toMatchObject({ - fenced: 1, + status: 'fenced', acknowledged_at: null }) peer.insertMessage({ runId: run.id, from: 'coord', to: mailboxHandle, subject: 'next' }) @@ -235,6 +240,22 @@ describe('derived delivery migration', () => { } ) + it('keeps the pre-v41 column and index shape a downgraded binary reads', () => { + const db = open(':memory:') + expect( + (db.db.pragma('table_info(deliveries)') as { name: string }[]).map((c) => c.name) + ).toContain('status') + const index = db.db + .prepare("SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_one_outstanding'") + .get() as { sql: string } + expect(index.sql).not.toContain('UNIQUE') + expect(index.sql).toContain("status = 'outstanding' AND mailbox_handle != ''") + // Why: a v40 binary probes exactly these objects before trusting the stamp; nothing it needs is gone. + expect(resolveOrchestrationMigrationStartVersion(db.db, SCHEMA_VERSION, 40)).toBe( + SCHEMA_VERSION + ) + }) + it('recreates a missing derived view on reopen without changing batch records', () => { const path = databasePath() const db = open(path) diff --git a/src/main/runtime/orchestration/db/schema/derived-delivery-test-fixture.ts b/src/main/runtime/orchestration/db/schema/derived-delivery-test-fixture.ts new file mode 100644 index 000000000000..0c1247b753b5 --- /dev/null +++ b/src/main/runtime/orchestration/db/schema/derived-delivery-test-fixture.ts @@ -0,0 +1,9 @@ +import type { OrchestrationDb } from '../orchestration-db' + +// Tests that hand-edit the deliveries table must drop the derived objects first: SQLite refuses +// DROP COLUMN / RENAME while a trigger or view still references the table. Reopening recreates them. +export function dropDerivedDeliverySchema(db: OrchestrationDb['db']): void { + db.exec( + 'DROP TRIGGER IF EXISTS trg_deliveries_one_outstanding; DROP VIEW IF EXISTS outstanding_deliveries;' + ) +} diff --git a/src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts b/src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts index 544b8cbcb5a7..eee649ad6664 100644 --- a/src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts +++ b/src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts @@ -1,7 +1,7 @@ import type { OrchestrationDb } from '../orchestration-db' export function migrateRoleMailboxDeliveryV34(this: OrchestrationDb, current: number): void { - if (current >= 34 || !this.hasColumn('deliveries', 'status')) { + if (current >= 34) { return } diff --git a/src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts b/src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts index 07b4f0b26eea..140a41160303 100644 --- a/src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts +++ b/src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts @@ -113,7 +113,8 @@ export function applySchemaMigrationsV2ToV12(this: OrchestrationDb, current: num created_at TEXT NOT NULL DEFAULT (datetime('now')), acknowledged_at TEXT ); - ${this.hasColumn('deliveries', 'fenced') ? '' : `CREATE UNIQUE INDEX IF NOT EXISTS idx_deliveries_one_outstanding ON deliveries(run_id) WHERE status = 'outstanding';`} + CREATE UNIQUE INDEX IF NOT EXISTS idx_deliveries_one_outstanding + ON deliveries(run_id) WHERE status = 'outstanding'; CREATE INDEX IF NOT EXISTS idx_deliveries_run_created ON deliveries(run_id, created_at); diff --git a/src/main/runtime/orchestration/db/schema/migrate-v35.ts b/src/main/runtime/orchestration/db/schema/migrate-v35.ts index c2c876cca94c..93a9613d68cf 100644 --- a/src/main/runtime/orchestration/db/schema/migrate-v35.ts +++ b/src/main/runtime/orchestration/db/schema/migrate-v35.ts @@ -40,15 +40,13 @@ export function migrateV35(this: OrchestrationDb, current: number): void { DROP TABLE IF EXISTS lifecycle_transition_receipts; ${ADDITIVE_LIFECYCLE_DELETE_TRIGGERS_SQL} `) - if (this.hasColumn('deliveries', 'status')) { - rebuildDeliveriesWithMailboxDefault.call(this) - recreateIndexMissingPredicate.call( - this, - 'idx_deliveries_one_outstanding', - "mailbox_handle != ''", - ONE_OUTSTANDING_INDEX_SQL - ) - } + rebuildDeliveriesWithMailboxDefault.call(this) + recreateIndexMissingPredicate.call( + this, + 'idx_deliveries_one_outstanding', + "mailbox_handle != ''", + ONE_OUTSTANDING_INDEX_SQL + ) recreateIndexMissingPredicate.call( this, 'idx_messages_pending_pointer_enter', diff --git a/src/main/runtime/orchestration/db/schema/migrate-v41.ts b/src/main/runtime/orchestration/db/schema/migrate-v41.ts index 2d576fbaa140..e37167031427 100644 --- a/src/main/runtime/orchestration/db/schema/migrate-v41.ts +++ b/src/main/runtime/orchestration/db/schema/migrate-v41.ts @@ -1,46 +1,36 @@ import type { OrchestrationDb } from '../orchestration-db' -export function migrateV41(this: OrchestrationDb): void { - if (this.hasColumn('deliveries', 'status')) { - this.db.exec(` - CREATE TABLE deliveries_v41 ( - id TEXT PRIMARY KEY, - run_id TEXT NOT NULL, - mailbox_handle TEXT NOT NULL DEFAULT '', - consumer_generation INTEGER NOT NULL, - message_ids TEXT NOT NULL, - created_at TEXT NOT NULL DEFAULT (datetime('now')), - acknowledged_at TEXT, - fenced INTEGER NOT NULL DEFAULT 0 CHECK(fenced IN (0, 1)) +// Why: eligibility comes from unread membership, so a fully read but unacknowledged batch must not +// block the next insert. The index keeps its pre-v41 name and predicate so downgraded binaries' +// IF NOT EXISTS create and schema probes still pass; only uniqueness is dropped. +export const OUTSTANDING_MAILBOX_INDEX_SQL = ` + CREATE INDEX IF NOT EXISTS idx_deliveries_one_outstanding + ON deliveries(mailbox_handle) WHERE status = 'outstanding' AND mailbox_handle != ''; +` + +export const DERIVED_DELIVERY_SCHEMA_SQL = ` + CREATE VIEW IF NOT EXISTS outstanding_deliveries AS + SELECT * FROM deliveries + WHERE status = 'outstanding' + AND EXISTS ( + SELECT 1 FROM json_each(deliveries.message_ids) AS member + JOIN messages ON messages.id = member.value WHERE messages.read = 0 ); - INSERT INTO deliveries_v41 - (id, run_id, mailbox_handle, consumer_generation, message_ids, created_at, acknowledged_at, fenced) - SELECT id, run_id, mailbox_handle, consumer_generation, message_ids, created_at, - acknowledged_at, - CASE WHEN status = 'fenced' THEN 1 ELSE 0 END - FROM deliveries ORDER BY rowid; - DROP TABLE deliveries; - ALTER TABLE deliveries_v41 RENAME TO deliveries; - `) + CREATE TRIGGER IF NOT EXISTS trg_deliveries_one_outstanding + AFTER INSERT ON deliveries + WHEN NEW.mailbox_handle != '' AND EXISTS ( + SELECT 1 FROM outstanding_deliveries WHERE mailbox_handle = NEW.mailbox_handle LIMIT 1 OFFSET 1 + ) + BEGIN + SELECT RAISE(ABORT, 'Mailbox already has an outstanding delivery'); + END; +` + +export function migrateV41(this: OrchestrationDb, current: number): void { + if (current >= 41) { + return } - this.db.exec(` - CREATE INDEX IF NOT EXISTS idx_deliveries_run_created ON deliveries(run_id, created_at); - CREATE INDEX IF NOT EXISTS idx_deliveries_unacknowledged_mailbox - ON deliveries(mailbox_handle) WHERE acknowledged_at IS NULL AND fenced = 0; - CREATE VIEW IF NOT EXISTS outstanding_deliveries AS - SELECT * FROM deliveries - WHERE acknowledged_at IS NULL AND fenced = 0 - AND EXISTS ( - SELECT 1 FROM json_each(deliveries.message_ids) AS member - JOIN messages ON messages.id = member.value WHERE messages.read = 0 - ); - CREATE TRIGGER IF NOT EXISTS trg_deliveries_one_outstanding - AFTER INSERT ON deliveries - WHEN NEW.mailbox_handle != '' AND EXISTS ( - SELECT 1 FROM outstanding_deliveries WHERE mailbox_handle = NEW.mailbox_handle LIMIT 1 OFFSET 1 - ) - BEGIN - SELECT RAISE(ABORT, 'Mailbox already has an outstanding delivery'); - END; - `) + this.db.exec( + `DROP INDEX IF EXISTS idx_deliveries_one_outstanding;\n${OUTSTANDING_MAILBOX_INDEX_SQL}` + ) } diff --git a/src/main/runtime/orchestration/db/schema/migrate.ts b/src/main/runtime/orchestration/db/schema/migrate.ts index 1d99ce7f07b4..bcf1b830ebc2 100644 --- a/src/main/runtime/orchestration/db/schema/migrate.ts +++ b/src/main/runtime/orchestration/db/schema/migrate.ts @@ -11,7 +11,7 @@ import { migrateV37 } from './migrate-v37' import { migrateV38 } from './migrate-v38' import { migrateV39 } from './migrate-v39' import { migrateV40 } from './migrate-v40' -import { migrateV41 } from './migrate-v41' +import { DERIVED_DELIVERY_SCHEMA_SQL, migrateV41 } from './migrate-v41' // Why: CREATE TABLE IF NOT EXISTS won't alter existing DBs; migrate in a txn that bumps user_version only on success (atomic all-or-nothing). export function migrate(this: OrchestrationDb): void { @@ -36,8 +36,11 @@ export function migrate(this: OrchestrationDb): void { migrateV38.call(this, current) migrateV39.call(this, current) migrateV40.call(this, current) - migrateV41.call(this) + // Why: older steps recreate the unique index; v41 must run after them. + migrateV41.call(this, current) this.createMailboxDeliveryIndexesIfPossible() + // Why: rebuild steps above RENAME the table, which SQLite refuses while a view names it. + this.db.exec(DERIVED_DELIVERY_SCHEMA_SQL) this.db.pragma(`user_version = ${SCHEMA_VERSION}`) this.db.exec('COMMIT') } catch (err) { diff --git a/src/main/runtime/orchestration/db/schema/schema-column-probes.ts b/src/main/runtime/orchestration/db/schema/schema-column-probes.ts index 1e9b672620b0..dad0f64d0ad2 100644 --- a/src/main/runtime/orchestration/db/schema/schema-column-probes.ts +++ b/src/main/runtime/orchestration/db/schema/schema-column-probes.ts @@ -1,4 +1,5 @@ import type { OrchestrationDb } from '../orchestration-db' +import { OUTSTANDING_MAILBOX_INDEX_SQL } from './migrate-v41' export function hasColumn(this: OrchestrationDb, table: string, column: string): boolean { const rows = this.db.pragma(`table_info(${table})`) as { name: string }[] @@ -6,13 +7,8 @@ export function hasColumn(this: OrchestrationDb, table: string, column: string): } export function createMailboxDeliveryIndexesIfPossible(this: OrchestrationDb): void { - if (this.hasColumn('deliveries', 'mailbox_handle') && this.hasColumn('deliveries', 'status')) { - // Excluding '' trades the pre-v34 per-run one-outstanding backstop for downgraded binaries; the - // app-level BEGIN IMMEDIATE still serializes one process. - this.db.exec(` - CREATE UNIQUE INDEX IF NOT EXISTS idx_deliveries_one_outstanding - ON deliveries(mailbox_handle) WHERE status = 'outstanding' AND mailbox_handle != ''; - `) + if (this.hasColumn('deliveries', 'mailbox_handle')) { + this.db.exec(OUTSTANDING_MAILBOX_INDEX_SQL) } const hasDeliveredAt = this.hasColumn('messages', 'delivered_at') if (hasDeliveredAt) { diff --git a/src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts b/src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts index 4b26c0cfb854..d6bc9ca9477a 100644 --- a/src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts +++ b/src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import Database from '../../sqlite/sync-database' -import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' +import { dropDerivedDeliverySchema } from './db/schema/derived-delivery-test-fixture' import { OrchestrationDb } from './db' import { SCHEMA_VERSION } from './db/contract-constants' import { createRootDispatch } from './db/root-dispatch-test-fixture' @@ -49,7 +49,7 @@ describe('OrchestrationDb v35 to v36 migration', () => { seed.close() const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) raw.exec(` ALTER TABLE dispatch_contexts DROP COLUMN consumer_generation; ALTER TABLE remote_dispatch_attachments DROP COLUMN consumer_generation; @@ -80,7 +80,7 @@ describe('OrchestrationDb v35 to v36 migration', () => { it('does not send a v35 stamp back to the pre-Run repair floor', () => { const v35 = createV35Database() const raw = new Database(v35.path) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) try { expect(resolveOrchestrationMigrationStartVersion(raw, 35, SCHEMA_VERSION)).toBe(35) } finally { @@ -91,7 +91,7 @@ describe('OrchestrationDb v35 to v36 migration', () => { it('repairs a database stamped v36 that never got the columns', () => { const v35 = createV35Database() const raw = new Database(v35.path) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) raw.pragma('user_version = 36') try { // Why: the skew repair is the only thing that catches a partially-written v36. diff --git a/src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts b/src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts index 856ffa1f85f2..9791dbde9b37 100644 --- a/src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts +++ b/src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import Database from '../../sqlite/sync-database' -import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' +import { dropDerivedDeliverySchema } from './db/schema/derived-delivery-test-fixture' import { OrchestrationDb } from './db' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' import { SCHEMA_VERSION } from './db/contract-constants' @@ -27,7 +27,7 @@ describe('federation acknowledgment migration', () => { db = undefined const oldDb = new Database(dbPath) - restoreV40DeliverySchema(oldDb) + dropDerivedDeliverySchema(oldDb) oldDb.exec('ALTER TABLE federated_dispatches DROP COLUMN to_home_acknowledged_sequence') oldDb.pragma('user_version = 26') expect(resolveOrchestrationMigrationStartVersion(oldDb, 26, 28)).toBe(26) diff --git a/src/main/runtime/orchestration/formatter.test.ts b/src/main/runtime/orchestration/formatter.test.ts index e42b80f3f46c..148bfad9b738 100644 --- a/src/main/runtime/orchestration/formatter.test.ts +++ b/src/main/runtime/orchestration/formatter.test.ts @@ -187,7 +187,7 @@ describe('formatMessagesForInjection', () => { describe('formatMessagePointer', () => { it('formats a singular pointer without message content', () => { expect(formatMessagePointer(1, 'run:run_1')).toBe( - '\n[Orca orchestration notification] You have 1 orchestration message. Run `orca orchestration check --run run_1`.\n' + '\nYou have 1 orchestration message. Run `orca orchestration check --run run_1`.\n' ) }) diff --git a/src/main/runtime/orchestration/formatter.ts b/src/main/runtime/orchestration/formatter.ts index 540e1def1ba2..2dd4f86777bb 100644 --- a/src/main/runtime/orchestration/formatter.ts +++ b/src/main/runtime/orchestration/formatter.ts @@ -118,5 +118,5 @@ export function formatMessagePointer( const runFlag = mailboxHandle?.startsWith('run:') ? ` --run ${mailboxHandle.slice('run:'.length)}` : '' - return `\n[Orca orchestration notification] You have ${count} orchestration ${noun}. Run \`${cliCommand} orchestration check${runFlag}\`.\n` + return `\nYou have ${count} orchestration ${noun}. Run \`${cliCommand} orchestration check${runFlag}\`.\n` } diff --git a/src/main/runtime/orchestration/nested-worker-depth-migration.test.ts b/src/main/runtime/orchestration/nested-worker-depth-migration.test.ts index 0341f41655ee..eb0f53a30d5c 100644 --- a/src/main/runtime/orchestration/nested-worker-depth-migration.test.ts +++ b/src/main/runtime/orchestration/nested-worker-depth-migration.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import Database from '../../sqlite/sync-database' -import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' +import { dropDerivedDeliverySchema } from './db/schema/derived-delivery-test-fixture' import { OrchestrationDb } from './db' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' import { SCHEMA_VERSION } from './db/contract-constants' @@ -33,7 +33,7 @@ describe('nested worker depth migration (v30)', () => { fresh.close() const oldDb = new Database(dbPath) - restoreV40DeliverySchema(oldDb) + dropDerivedDeliverySchema(oldDb) oldDb.exec('ALTER TABLE dispatch_contexts DROP COLUMN depth') oldDb.exec('ALTER TABLE remote_dispatch_attachments DROP COLUMN depth') oldDb.exec('ALTER TABLE remote_dispatch_attachments DROP COLUMN home_run_id') @@ -97,7 +97,7 @@ describe('nested worker depth migration (v30)', () => { // replay migrations from v6 instead of starting at 29. const dbPath = createV29Database() const oldDb = new Database(dbPath) - restoreV40DeliverySchema(oldDb) + dropDerivedDeliverySchema(oldDb) expect(resolveOrchestrationMigrationStartVersion(oldDb, 29, SCHEMA_VERSION)).toBe(29) oldDb.close() }) diff --git a/src/main/runtime/orchestration/orchestration-derived-delivery.test.ts b/src/main/runtime/orchestration/orchestration-derived-delivery.test.ts index 9639dd4d312a..666c0aeae6f0 100644 --- a/src/main/runtime/orchestration/orchestration-derived-delivery.test.ts +++ b/src/main/runtime/orchestration/orchestration-derived-delivery.test.ts @@ -23,13 +23,6 @@ describe('delivery eligibility derived from messages', () => { return { run, params, message, first } } - it('does not store delivery status', () => { - setup() - expect( - (db.db.pragma('table_info(deliveries)') as { name: string }[]).map((c) => c.name) - ).not.toContain('status') - }) - it.each(['read mutation', 'lifecycle suppression', 'direct SQL'])( '%s changes eligibility without updating the batch', (path) => { diff --git a/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts b/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts index 2bd5af9dc3c0..5eff7569d27d 100644 --- a/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts +++ b/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts @@ -69,9 +69,9 @@ describe('federated mailbox legacy-adoption probe', () => { }) } else { expect( - db.db.prepare("SELECT fenced FROM deliveries WHERE id = 'delivery_probe'").get() + db.db.prepare("SELECT status FROM deliveries WHERE id = 'delivery_probe'").get() ).toEqual({ - fenced: 0 + status: 'outstanding' }) } } @@ -115,9 +115,9 @@ describe('federated mailbox legacy-adoption probe', () => { }) } else { expect( - db.db.prepare("SELECT fenced FROM deliveries WHERE id = 'delivery_probe'").get() + db.db.prepare("SELECT status FROM deliveries WHERE id = 'delivery_probe'").get() ).toEqual({ - fenced: 1 + status: 'fenced' }) } } diff --git a/src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts b/src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts index 3aa7739ed517..2dda528333a3 100644 --- a/src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts +++ b/src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts @@ -112,7 +112,7 @@ describe('OrchestrationDb legacy contract storage', () => { ).toMatchObject({ run_id: adoptedRunId, mailbox_handle: `run:${LEGACY_RUN_ID}`, - fenced: 1 + status: 'fenced' }) expect(db.getDispatchContextById(fixture.currentDispatchId)).toMatchObject({ run_id: fixture.currentRunId, diff --git a/src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts b/src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts index c11d6d35d7ce..42087d7e79a8 100644 --- a/src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts +++ b/src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import Database from '../../sqlite/sync-database' import { LEGACY_RUN_ID, OrchestrationDb } from './db' -import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' +import { dropDerivedDeliverySchema } from './db/schema/derived-delivery-test-fixture' import { createRootDispatch } from './db/root-dispatch-test-fixture' export type LegacyStorageCutoverFixture = { @@ -176,7 +176,7 @@ export function createLegacyStorageCutoverFixture(): { first.close() const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) const legacyDeliveryId = 'delivery_legacy_outstanding' raw .prepare( diff --git a/src/main/runtime/orchestration/orchestration-schema-version-skew.ts b/src/main/runtime/orchestration/orchestration-schema-version-skew.ts index a9e9cef0a747..85e0a78b3ae4 100644 --- a/src/main/runtime/orchestration/orchestration-schema-version-skew.ts +++ b/src/main/runtime/orchestration/orchestration-schema-version-skew.ts @@ -43,8 +43,7 @@ const VERSIONED_POST_V6_COLUMNS = [ { version: 36, table: 'remote_dispatch_attachments', column: 'consumer_generation' }, { version: 37, table: 'dispatch_contexts', column: 'creator_handle' }, { version: 37, table: 'dispatch_contexts', column: 'creator_pane_key' }, - { version: 40, table: 'remote_dispatch_attachments', column: 'home_run_id' }, - { version: 41, table: 'deliveries', column: 'fenced' } + { version: 40, table: 'remote_dispatch_attachments', column: 'home_run_id' } ] as const // Why: v34 shipped without these two, so a v34 stamp proves nothing about them; v35 repairs both @@ -54,6 +53,7 @@ const VERSIONED_POST_V6_COLUMN_DEFAULTS = [ ] as const const VERSIONED_POST_V6_INDEX_PREDICATES = [ + { version: 35, index: 'idx_deliveries_one_outstanding', predicate: "mailbox_handle != ''" }, { version: 35, index: 'idx_messages_pending_pointer_enter', @@ -68,6 +68,7 @@ const POST_V6_INDEXES = [ 'idx_dispatch_run_status', 'idx_gates_run_status', 'idx_runs_coordinator_pane', + 'idx_deliveries_one_outstanding', 'idx_deliveries_run_created', 'idx_questions_dispatch_status', 'idx_federation_relay_pending', @@ -98,12 +99,8 @@ function hasOrchestrationColumnDefault( return rows.some((row) => row.name === column && row.dflt_value === defaultValue) } -function hasOrchestrationSchemaObject( - db: Database.Database, - name: string, - type: 'index' | 'view' | 'trigger' = 'index' -): boolean { - return !!db.prepare('SELECT 1 FROM sqlite_master WHERE type = ? AND name = ?').get(type, name) +function hasOrchestrationIndex(db: Database.Database, index: string): boolean { + return !!db.prepare("SELECT 1 FROM sqlite_master WHERE type = 'index' AND name = ?").get(index) } function hasOrchestrationIndexPredicate( @@ -174,18 +171,7 @@ function hasCompletePostV6Schema(db: Database.Database, storedVersion: number): ({ version, index, predicate }) => storedVersion < version || hasOrchestrationIndexPredicate(db, index, predicate) ) && - (storedVersion >= 41 - ? hasOrchestrationSchemaObject(db, 'idx_deliveries_unacknowledged_mailbox') && - hasOrchestrationSchemaObject(db, 'outstanding_deliveries', 'view') && - hasOrchestrationSchemaObject(db, 'trg_deliveries_one_outstanding', 'trigger') - : hasOrchestrationSchemaObject(db, 'idx_deliveries_one_outstanding') && - (storedVersion < 35 || - hasOrchestrationIndexPredicate( - db, - 'idx_deliveries_one_outstanding', - "mailbox_handle != ''" - ))) && - POST_V6_INDEXES.every((index) => hasOrchestrationSchemaObject(db, index)) && + POST_V6_INDEXES.every((index) => hasOrchestrationIndex(db, index)) && messagesAllowQuestions(db) && hasConsistentLegacyAdoption(db) ) diff --git a/src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts b/src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts index beaab4511d07..a732729a1d52 100644 --- a/src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts +++ b/src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts @@ -6,7 +6,7 @@ import Database from '../../sqlite/sync-database' import { LEGACY_CONTRACT_VERSION, LEGACY_RUN_ID, OrchestrationDb } from './db' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' import { createRootDispatch } from './db/root-dispatch-test-fixture' -import { restoreV40DeliverySchema } from './db/schema/delivery-v40-test-fixture' +import { dropDerivedDeliverySchema } from './db/schema/derived-delivery-test-fixture' import { SCHEMA_VERSION } from './db/contract-constants' describe('OrchestrationDb version-skew migration', () => { @@ -26,7 +26,6 @@ describe('OrchestrationDb version-skew migration', () => { tempDir = mkdtempSync(join(tmpdir(), 'orca-db-version-skew-')) const dbPath = join(tempDir, 'orchestration.db') const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) raw.exec(` CREATE TABLE messages ( id TEXT NOT NULL, @@ -187,7 +186,6 @@ describe('OrchestrationDb version-skew migration', () => { it('does not repair an incomplete schema written by a future binary', () => { const dbPath = createLegacySchemaClaimingVersion(20) const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) expect(resolveOrchestrationMigrationStartVersion(raw, 20, 19)).toBe(20) expect(raw.pragma('user_version', { simple: true })).toBe(20) @@ -203,7 +201,6 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) raw.exec( 'ALTER TABLE worker_terminal_resources DROP COLUMN recovery_attempt_count; ALTER TABLE worker_terminal_resources DROP COLUMN last_recovery_at;' ) @@ -245,7 +242,6 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) raw.exec( 'ALTER TABLE worker_terminal_resources DROP COLUMN recovery_attempt_count; ALTER TABLE worker_terminal_resources DROP COLUMN last_recovery_at;' ) @@ -272,7 +268,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) raw.exec(` DROP INDEX idx_deliveries_one_outstanding; ALTER TABLE deliveries DROP COLUMN mailbox_handle; @@ -309,7 +305,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) raw.exec(` DROP INDEX idx_deliveries_one_outstanding; ALTER TABLE deliveries DROP COLUMN mailbox_handle; @@ -336,7 +332,7 @@ describe('OrchestrationDb version-skew migration', () => { const migratedDeliveries = db.db .prepare( `SELECT id, run_id, mailbox_handle, consumer_generation, message_ids, - fenced, created_at, acknowledged_at + status, created_at, acknowledged_at FROM deliveries` ) .all() @@ -349,7 +345,7 @@ describe('OrchestrationDb version-skew migration', () => { mailbox_handle: `run:${run.id}`, consumer_generation: run.consumer_generation, message_ids: originalMessageIds, - fenced: 0, + status: 'acknowledged', created_at: '2026-01-02 03:04:05', acknowledged_at: '2026-01-02 04:05:06' }, @@ -359,7 +355,7 @@ describe('OrchestrationDb version-skew migration', () => { mailbox_handle: `run:${run.id}`, consumer_generation: run.consumer_generation, message_ids: '["msg_fenced"]', - fenced: 1, + status: 'fenced', created_at: '2026-03-04 05:06:07', acknowledged_at: null }, @@ -369,7 +365,7 @@ describe('OrchestrationDb version-skew migration', () => { mailbox_handle: `run:${run.id}`, consumer_generation: run.consumer_generation, message_ids: '["msg_outstanding"]', - fenced: 0, + status: 'outstanding', created_at: '2026-02-03 04:05:06', acknowledged_at: null } @@ -379,10 +375,7 @@ describe('OrchestrationDb version-skew migration', () => { .prepare("SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'deliveries'") .all() as { name: string }[] expect(deliveryIndexes.map(({ name }) => name)).toEqual( - expect.arrayContaining([ - 'idx_deliveries_unacknowledged_mailbox', - 'idx_deliveries_run_created' - ]) + expect.arrayContaining(['idx_deliveries_one_outstanding', 'idx_deliveries_run_created']) ) expect(db.hasOutstandingRunDelivery(run.id)).toBe(false) }) @@ -415,7 +408,6 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) // v30 resetTasks predates both additive tables, so it only deletes their legacy parents. raw.exec(` DELETE FROM worker_dispatches; @@ -436,7 +428,6 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) raw.exec( `DROP INDEX IF EXISTS idx_messages_pending_pointer_enter; DROP INDEX IF EXISTS idx_messages_pending_pointer_pty; @@ -493,7 +484,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) raw.exec(` DROP TABLE deliveries; CREATE TABLE deliveries ( @@ -519,12 +510,10 @@ describe('OrchestrationDb version-skew migration', () => { expect( ( db.db - .prepare( - "SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_unacknowledged_mailbox'" - ) + .prepare("SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_one_outstanding'") .get() as { sql: string } ).sql - ).toContain('acknowledged_at IS NULL AND fenced = 0') + ).toContain("mailbox_handle != ''") const run = db.createRun({ objective: 'already stamped', @@ -548,7 +537,6 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) raw.exec(` DROP INDEX IF EXISTS idx_messages_pending_pointer_enter; CREATE INDEX idx_messages_pending_pointer_enter @@ -575,7 +563,7 @@ describe('OrchestrationDb version-skew migration', () => { db = undefined const raw = new Database(dbPath) - restoreV40DeliverySchema(raw) + dropDerivedDeliverySchema(raw) raw.exec(` DROP INDEX IF EXISTS idx_deliveries_one_outstanding; CREATE UNIQUE INDEX idx_deliveries_one_outstanding @@ -588,11 +576,9 @@ describe('OrchestrationDb version-skew migration', () => { expect( ( db.db - .prepare( - "SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_unacknowledged_mailbox'" - ) + .prepare("SELECT sql FROM sqlite_master WHERE name = 'idx_deliveries_one_outstanding'") .get() as { sql: string } ).sql - ).toContain('acknowledged_at IS NULL AND fenced = 0') + ).toContain("mailbox_handle != ''") }) }) diff --git a/src/main/runtime/orchestration/types.ts b/src/main/runtime/orchestration/types.ts index d8104136305e..85d5dcfc1596 100644 --- a/src/main/runtime/orchestration/types.ts +++ b/src/main/runtime/orchestration/types.ts @@ -52,13 +52,15 @@ export type RunRow = { updated_at: string } +export type DeliveryStatus = 'outstanding' | 'acknowledged' | 'fenced' + export type DeliveryRow = { id: string run_id: string mailbox_handle: string | null consumer_generation: number message_ids: string - fenced: number + status: DeliveryStatus created_at: string acknowledged_at: string | null } From 74c613ad75c996b7c8116db35a4c25b881a8c999 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 11 Sep 2026 23:22:03 -0400 Subject: [PATCH 7/9] docs(orchestration): drop delivery storage reference The compatibility caveat it existed to explain no longer applies; the view and index comments carry the remaining rationale. --- docs/reference/orchestration-delivery-storage.md | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 docs/reference/orchestration-delivery-storage.md diff --git a/docs/reference/orchestration-delivery-storage.md b/docs/reference/orchestration-delivery-storage.md deleted file mode 100644 index 58bb6bf6250c..000000000000 --- a/docs/reference/orchestration-delivery-storage.md +++ /dev/null @@ -1,11 +0,0 @@ -# Orchestration delivery storage - -A message records whether it still needs attention (`read`). A delivery is a stable batch receipt: ID, ordered message IDs, mailbox, consumer generation, creation time, and `status`. `status` only records terminal facts written by an explicit event: `acknowledged` when the consumer acks, `fenced` when the consumer is replaced or legacy ownership is adopted. `outstanding` means neither has happened; it does not mean the batch is still eligible. - -Eligibility is derived, never stored. `outstanding_deliveries` is a SQLite view over batches that are `outstanding` and still contain at least one unread message. Consuming checks, replay, notification pointers, and the single-active-batch insert trigger all read this view. When lifecycle reconciliation marks a heartbeat read, its batch stops appearing without any delivery write, so a later `worker_done` is never hidden behind it. A partially read batch still replays its complete original membership under the same ID. - -Creation and acknowledgement run under `BEGIN IMMEDIATE` and validate the current consumer inside that transaction: Run coordinators against the Run generation, workers against their local Dispatch or federated attachment, chosen explicitly by the caller because a loopback runtime has both. Worker validation also requires a live lifecycle state, so a settled worker cannot consume mail awaiting rerouting; remote states with unverifiable liveness stay eligible. - -## Compatibility - -Schema v41 drops the unique constraint from `idx_deliveries_one_outstanding` (name and predicate unchanged) and adds the view and trigger. No column changes. An older binary that opens a v41 database keeps working: it sees a higher `user_version` and skips migration, its `status` queries still resolve, and the index it expects still exists. From a82838f5f06efa5da25cc10c2b9f41203418539e Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 11 Sep 2026 23:22:23 -0400 Subject: [PATCH 8/9] docs: revert unrelated formatter churn --- docs/reference/windows-process-enumeration.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/windows-process-enumeration.md b/docs/reference/windows-process-enumeration.md index 685103ba9ba6..fac8f7c58d16 100644 --- a/docs/reference/windows-process-enumeration.md +++ b/docs/reference/windows-process-enumeration.md @@ -583,8 +583,8 @@ breakaway hands the whole tree its escape. The per-PTY job therefore omits `BREAKAWAY_OK` whenever `msys-2.0.dll` or `cygwin1.dll` sits on the shell's DLL search path — beside the executable, or under `usr/bin` for Git's `bin` launcher. Native shells keep explicit breakaway. Denying it costs Cygwin -nothing, because it _pre-checks_ the limit rather than retrying, so no spawn -fails; but a _native_ program that passes `CREATE_BREAKAWAY_FROM_JOB` itself +nothing, because it *pre-checks* the limit rather than retrying, so no spawn +fails; but a *native* program that passes `CREATE_BREAKAWAY_FROM_JOB` itself inside such a pane now gets `ERROR_ACCESS_DENIED`. `nohup` and `disown` are unaffected — they are Cygwin signal/session concepts, unrelated to job membership. The daemon's host job is unchanged. From 52d8ecc034a7b0190f2f8411ebab7d721a9125f2 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sat, 12 Sep 2026 01:11:29 -0400 Subject: [PATCH 9/9] test(orchestration): verify historical database downgrade round trip --- ...hestration-delivery-downgrade.unit.test.ts | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts diff --git a/tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts b/tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts new file mode 100644 index 000000000000..e53b3bc9e6cf --- /dev/null +++ b/tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts @@ -0,0 +1,68 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { expect, test } from 'vitest' +import { OrchestrationDb } from '../../../src/main/runtime/orchestration/db' +import { importReleaseCheckoutModule, materializeReleaseCheckout } from './release-checkout' + +// Pin the last pre-v41 implementation: this contract specifically exercises status-only readers. +const PRE_V41 = 'aac38d698ff75ac4c8658addab48ef5a83617619' + +test('pre-v41 code opens, acknowledges and writes a v41 database, then current code reopens it', async () => { + const checkout = await materializeReleaseCheckout(PRE_V41) + const baseline = await importReleaseCheckoutModule( + checkout, + 'src/main/runtime/orchestration/db.ts' + ) + const OldDb = baseline.OrchestrationDb as typeof OrchestrationDb + const directory = mkdtempSync(join(tmpdir(), 'orca-delivery-downgrade-')) + const path = join(directory, 'orchestration.db') + let db: OrchestrationDb | undefined + try { + db = new OldDb(path) + expect(db.db.pragma('user_version', { simple: true })).toBe(40) + const run = db.createRun({ + objective: 'downgrade round trip', + coordinatorHandle: 'coord', + coordinatorPaneKey: 'tab:11111111-1111-4111-8111-111111111111' + }) + const params = { runId: run.id, consumerGeneration: run.consumer_generation } + const insert = (subject: string) => + db!.insertMessage({ + runId: run.id, + from: 'worker', + to: `run:${run.id}`, + subject + }) + const oldMessage = insert('obsolete heartbeat') + const oldBatch = db.getOrCreateRunDelivery(params)! + db.close() + + db = new OrchestrationDb(path) + db.markAsRead([oldMessage.id]) + const completion = insert('completion') + const currentBatch = db.getOrCreateRunDelivery(params)! + expect(currentBatch.messages.map((message) => message.id)).toEqual([completion.id]) + expect(currentBatch.delivery.id).not.toBe(oldBatch.delivery.id) + db.close() + + db = new OldDb(path) + expect(db.db.pragma('user_version', { simple: true })).toBe(41) + // Old readers retain their original replay semantics, but can acknowledge either stored batch. + db.acknowledgeRunDelivery({ ...params, deliveryId: oldBatch.delivery.id }) + expect(db.getOrCreateRunDelivery(params)?.delivery.id).toBe(currentBatch.delivery.id) + db.acknowledgeRunDelivery({ ...params, deliveryId: currentBatch.delivery.id }) + const next = insert('written after downgrade') + const nextBatch = db.getOrCreateRunDelivery(params)! + expect(nextBatch.messages.map((message) => message.id)).toEqual([next.id]) + db.close() + + db = new OrchestrationDb(path) + expect(db.getOrCreateRunDelivery(params)?.delivery.id).toBe(nextBatch.delivery.id) + db.acknowledgeRunDelivery({ ...params, deliveryId: nextBatch.delivery.id }) + expect(db.getOrCreateRunDelivery(params)).toBeUndefined() + } finally { + db?.close() + rmSync(directory, { recursive: true, force: true }) + } +})