Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/main/runtime/orchestration/db/contract-constants.ts
Original file line number Diff line number Diff line change
@@ -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_'
Expand Down
15 changes: 12 additions & 3 deletions src/main/runtime/orchestration/db/messages/message-insert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ──

Expand All @@ -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 <handle>`) 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)
Expand Down
11 changes: 6 additions & 5 deletions src/main/runtime/orchestration/db/writer-run-required.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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])
})
})
Original file line number Diff line number Diff line change
@@ -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 <handle>`: 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])
})
})
3 changes: 3 additions & 0 deletions src/shared/orchestration-rpc-contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
39 changes: 10 additions & 29 deletions tests/e2e/orchestration-idle-mail-delivery.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}) => {
Expand All @@ -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 <handle>` 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)
})

Expand Down