diff --git a/src/main/runtime/orchestration/db/contract-constants.ts b/src/main/runtime/orchestration/db/contract-constants.ts index 0a2e27f000ff..56c2e2d542ff 100644 --- a/src/main/runtime/orchestration/db/contract-constants.ts +++ b/src/main/runtime/orchestration/db/contract-constants.ts @@ -1,7 +1,11 @@ -import { ORCHESTRATION_LEGACY_RUN_ID } from '../../../../shared/orchestration-rpc-contract' +import { + ORCHESTRATION_LEGACY_RUN_ID, + ORCHESTRATION_UNBOUND_RUN_ID +} from '../../../../shared/orchestration-rpc-contract' import { ORCHESTRATION_CONTRACT_VERSION } from '../../../../shared/protocol-version' export const LEGACY_RUN_ID = ORCHESTRATION_LEGACY_RUN_ID +export const UNBOUND_RUN_ID = ORCHESTRATION_UNBOUND_RUN_ID // Why: a v1.4.198 coordinator sends no Run id, so its remote workers file mail under a per-attachment stub Run. export const FEDERATED_STUB_HOME_RUN_ID_PREFIX = 'run_federated_' diff --git a/src/main/runtime/orchestration/db/messages/message-insert.ts b/src/main/runtime/orchestration/db/messages/message-insert.ts index 82573305479a..51ce7aaaa50e 100644 --- a/src/main/runtime/orchestration/db/messages/message-insert.ts +++ b/src/main/runtime/orchestration/db/messages/message-insert.ts @@ -3,6 +3,7 @@ import { generateId } from '../generated-id' import { exposeMessageTimestamps } from '../utc-timestamp' import type { OrchestrationDb } from '../orchestration-db' import { runLifecycleWriteTransaction } from '../lifecycle-write-transaction-runner' +import { UNBOUND_RUN_ID } from '../contract-constants' // ── Messages ── @@ -25,9 +26,17 @@ export type MessageInsert = { } export function insertMessage(this: OrchestrationDb, msg: MessageInsert): MessageRow { - const runId = msg.runId - if (!runId) { - throw new Error('Run is required') + // A sender in no Run (two plain terminals, `send --to `) still gets durable mail. It is + // filed under the unbound Run, never the legacy one, which the schema-skew probe reads as pre-Runs. + // Created on first use so `run list` shows it only to a user who has such mail. + const runId = msg.runId ?? UNBOUND_RUN_ID + if (msg.runId == null) { + this.db + .prepare( + `INSERT OR IGNORE INTO runs (id, objective, home_database, consumer_generation, legacy) + VALUES (?, 'Mail from terminals in no Run', 'this_database', 0, 0)` + ) + .run(UNBOUND_RUN_ID) } const deliveryContract = msg.deliveryContract ?? 'current_delivery' this.requireRun(runId) diff --git a/src/main/runtime/orchestration/db/writer-run-required.test.ts b/src/main/runtime/orchestration/db/writer-run-required.test.ts index 21fcbeb28d63..682fabb1b60b 100644 --- a/src/main/runtime/orchestration/db/writer-run-required.test.ts +++ b/src/main/runtime/orchestration/db/writer-run-required.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { OrchestrationDb } from './orchestration-db' +import { UNBOUND_RUN_ID } from './contract-constants' describe('writers require a Run', () => { let db: OrchestrationDb @@ -8,11 +9,11 @@ describe('writers require a Run', () => { }) afterEach(() => db.close()) - it('rejects a message without a Run instead of using the legacy Run', () => { - expect(() => db.insertMessage({ from: 'sender', to: 'worker', subject: 'mail' })).toThrow( - 'Run is required' - ) - expect(db.db.prepare('SELECT id FROM messages').all()).toEqual([]) + it('files a message without a Run under the unbound Run, never the legacy one', () => { + const message = db.insertMessage({ from: 'sender', to: 'worker', subject: 'mail' }) + expect(message.run_id).toBe(UNBOUND_RUN_ID) + expect(db.getRun(UNBOUND_RUN_ID)).toMatchObject({ legacy: 0 }) + expect(db.getUnreadMessages('worker').map((row) => row.id)).toEqual([message.id]) }) it('rejects a Task without a Run instead of using the legacy Run', () => { 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 24d18f0baef5..5eff7569d27d 100644 --- a/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts +++ b/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { LEGACY_RUN_ID, OrchestrationDb } from './db' -import { federatedStubHomeRunId, SCHEMA_VERSION } from './db/contract-constants' +import { federatedStubHomeRunId, SCHEMA_VERSION, UNBOUND_RUN_ID } from './db/contract-constants' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' describe('federated mailbox legacy-adoption probe', () => { @@ -122,4 +122,19 @@ describe('federated mailbox legacy-adoption probe', () => { } } ) + + it('keeps mail from a terminal in no Run across a reopen without replaying adoption', () => { + directory = mkdtempSync(join(tmpdir(), 'orca-unbound-mail-probe-')) + const path = join(directory, 'orchestration.db') + db = new OrchestrationDb(path) + const sent = db.insertMessage({ from: 'term_a', to: 'term_b', subject: 'hi' }) + expect(sent.run_id).toBe(UNBOUND_RUN_ID) + expect(resolveOrchestrationMigrationStartVersion(db.db, SCHEMA_VERSION, SCHEMA_VERSION)).toBe( + SCHEMA_VERSION + ) + db.close() + db = new OrchestrationDb(path) + expect(db.getLegacyAdoption()).toBeUndefined() + expect(db.getUnreadMessages('term_b').map((row) => row.id)).toEqual([sent.id]) + }) }) diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/send-unbound-terminals.test.ts b/src/main/runtime/rpc/methods/orchestration/messaging/send-unbound-terminals.test.ts new file mode 100644 index 000000000000..84cb250a19e6 --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration/messaging/send-unbound-terminals.test.ts @@ -0,0 +1,31 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createOrchestrationRpcHarness } from '../rpc-test-harness' + +describe('orchestration.send between terminals in no Run', () => { + const h = createOrchestrationRpcHarness() + afterEach(() => h.cleanup()) + + it('delivers terminal-to-terminal mail when neither terminal is in a Run', async () => { + // Two plain panes and `send --to `: the first command the guide teaches. #19542 + // refused this with a bare "Run is required"; it files under the unbound Run instead. + const { db, runtime, ctx } = h.setup(false) + vi.spyOn(runtime, 'getTerminalPaneKey').mockImplementation((handle) => + handle === 'term_a' ? 'tab_a:leaf_a' : handle === 'term_b' ? 'tab_b:leaf_b' : null + ) + vi.spyOn(runtime, 'deliverPendingMessagesForHandle').mockImplementation(() => {}) + + const result = (await h.call( + 'orchestration.send', + { from: 'term_a', to: 'term_b', subject: 'hello from no Run' }, + ctx + )) as { message: { id: string; run_id: string; to_handle: string } } + + expect(result.message).toMatchObject({ run_id: 'run_unbound', to_handle: 'term_b' }) + expect(db.getRun('run_unbound')).toMatchObject({ legacy: 0 }) + expect(db.getUnreadMessages('term_b').map((row) => row.id)).toEqual([result.message.id]) + const checked = (await h.call('orchestration.check', { terminal: 'term_b' }, ctx)) as { + messages: { id: string }[] + } + expect(checked.messages.map((row) => row.id)).toEqual([result.message.id]) + }) +}) diff --git a/src/shared/orchestration-rpc-contract.ts b/src/shared/orchestration-rpc-contract.ts index 3067d5ad6b04..fec4d4ed4e7f 100644 --- a/src/shared/orchestration-rpc-contract.ts +++ b/src/shared/orchestration-rpc-contract.ts @@ -14,6 +14,9 @@ export const ORCHESTRATION_SKILL_COMMAND_ARGS = [ ] as const export const ORCHESTRATION_LEGACY_RUN_ID = 'run_legacy_local' +/** Files mail sent from a terminal in no Run. Distinct from the legacy Run on purpose: rows under + * the legacy id read as a pre-Runs database and trigger adoption on the next open. */ +export const ORCHESTRATION_UNBOUND_RUN_ID = 'run_unbound' const ORCHESTRATION_MUTATION_METHODS = new Set([ 'orchestration.runCreate', diff --git a/tests/e2e/orchestration-idle-mail-delivery.spec.ts b/tests/e2e/orchestration-idle-mail-delivery.spec.ts index 8ca6657ab7c9..cffa64152080 100644 --- a/tests/e2e/orchestration-idle-mail-delivery.spec.ts +++ b/tests/e2e/orchestration-idle-mail-delivery.spec.ts @@ -35,7 +35,6 @@ import { waitForPaneIdentitySnapshot } from './helpers/terminal' import { RuntimeClient, type RuntimeRpcSuccess } from '../../src/cli/runtime-client' -import { RuntimeRpcFailureError } from '../../src/cli/runtime/types' import type { RuntimeTerminalListResult } from '../../src/shared/runtime-types' import { CODEX_IDLE_TITLE, @@ -354,7 +353,7 @@ test.describe('orchestration push-on-idle mail delivery', () => { // #19542 deleted the legacy-Run write fallback, so a sender in no Run has // nowhere to file mail to a bare handle: the send is refused outright, which // is what keeps an unsafe pointer out of the pane on the next idle frame. - test('refuses unbound direct mail from a sender in no Run instead of pushing it', async ({ + test('keeps unbound direct mail durable without pointing to an unsafe check', async ({ orcaPage, electronApp }) => { @@ -363,40 +362,22 @@ test.describe('orchestration push-on-idle mail delivery', () => { const pane = await openAgentPane() await driveToLiveIdle(client, pane) + // Two plain terminals, neither in a Run: `send --to ` must still land durably. It + // files under the unbound Run, so a reopen never reads it as pre-Runs state (#19542 regression). const stdinBeforeScan = pane.agent.readStdin() - const refusal = await sendMail(client, pane.handle, { subject: 'Unbound direct mail' }).then( - () => undefined, - (error: unknown) => error - ) - // Why runtime_error and not run_required: insertMessage throws a plain - // Error, which the dispatcher passes through with its message and no - // recovery data — the sibling no-recipient path is the one that adds it. - // The request-id suffix and stamp are the client's durable-mutation bookkeeping. - expect(refusal).toBeInstanceOf(RuntimeRpcFailureError) - expect(refusal).toMatchObject({ - code: 'runtime_error', - message: expect.stringContaining('Run is required') - }) - // Pins that the SERVER attached no orchestrationSkillRecoveryData, without - // freezing whatever else the client may stamp alongside its request id. - const refusalData = (refusal as RuntimeRpcFailureError).data - expect(refusalData).toMatchObject({ orchestrationRequestId: expect.any(String) }) - expect(refusalData).not.toHaveProperty('effectsApplied') - expect(refusalData).not.toHaveProperty('guide') - expect(refusalData).not.toHaveProperty('nextCommandArgs') - expect(refusalData).not.toHaveProperty('nextSteps') - expect(readMailbox(userDataDir, pane.handle)).toEqual([]) - - // A busy→idle edge is the push trigger. Walking one proves the refusal left - // nothing behind for the scan to point at, not merely that the push was slow. + const messageId = await sendMail(client, pane.handle, { subject: 'Unbound direct mail' }) pane.agent.setTitle(CODEX_WORKING_TITLE) await waitForObservedTitle(client, pane.handle, CODEX_WORKING_TITLE) pane.agent.setTitle(CODEX_IDLE_TITLE) await waitForObservedTitle(client, pane.handle, CODEX_IDLE_TITLE) - // The ledger only proves anything once the push window has fully elapsed. await orcaPage.waitForTimeout(NO_DELIVERY_SETTLE_MS) - expect(readMailbox(userDataDir, pane.handle)).toEqual([]) + expect(readMailRow(userDataDir, messageId)).toMatchObject({ + to_handle: pane.handle, + run_id: 'run_unbound', + read: 0, + delivered_at: null + }) expect(pane.agent.readStdin()).toBe(stdinBeforeScan) })