mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
perf(orchestration): stop recompiling a mailbox lookup on every agent title (#30792)
* perf(orchestration): stop recompiling the remote-attachment lookup on every agent title Co-Authored-By: Claude <noreply@anthropic.com> * chore(orchestration): state why the attachment row casts are safe Co-Authored-By: Claude <noreply@anthropic.com> * fix(ci): run the mailbox owner test in the Node runtime project Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: E2E Test <e2e@test.local> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude
E2E Test
parent
846cba6e96
commit
a1ea2e3978
@@ -225,6 +225,7 @@ export const SQLITE_RUNTIME_INCLUDE = [
|
||||
'src/main/runtime/host-terminal-close-persistence-durability.test.ts',
|
||||
'src/main/runtime/orca-runtime-terminal-close-records.test.ts',
|
||||
'src/main/runtime/orca-runtime-terminal-surface-close.test.ts',
|
||||
'src/main/runtime/orchestration/mailbox-owner.test.ts',
|
||||
'src/main/runtime/orchestration/orchestration-party-location.test.ts',
|
||||
'src/main/runtime/orchestration/structured-worker-journal-page.test.ts',
|
||||
'src/main/runtime/rpc/methods/agent-launch-at-rest.test.ts',
|
||||
|
||||
@@ -10,7 +10,10 @@ import type { OrchestrationDb } from './orchestration-db'
|
||||
import type { OrcaSessionId } from '../../../../shared/orca-session-address'
|
||||
import type { DispatchContextRow, RemoteDispatchAttachmentRow } from '../types'
|
||||
import { potentiallyLiveRemoteAttachmentSql } from './federation/remote-attachment-liveness'
|
||||
import { DISPATCH_CONTEXT_COLUMN_LIST } from './row-column-lists'
|
||||
import {
|
||||
DISPATCH_CONTEXT_COLUMN_LIST,
|
||||
REMOTE_DISPATCH_ATTACHMENT_COLUMN_LIST
|
||||
} from './row-column-lists'
|
||||
|
||||
/**
|
||||
* Who is creating a dispatch row, for nesting-depth purposes.
|
||||
@@ -172,9 +175,10 @@ function findPotentiallyLiveAttachmentsForCreator(
|
||||
if (!creator.paneKey || !creator.processIncarnation) {
|
||||
return []
|
||||
}
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The schema-pinned attachment projection returns RemoteDispatchAttachmentRow rows; the adapter exposes unknown.
|
||||
const rows = this.db
|
||||
.prepare(
|
||||
`SELECT * FROM remote_dispatch_attachments
|
||||
`SELECT ${REMOTE_DISPATCH_ATTACHMENT_COLUMN_LIST} FROM remote_dispatch_attachments
|
||||
WHERE process_incarnation = ?
|
||||
AND pane_key IS NOT NULL
|
||||
AND ${potentiallyLiveRemoteAttachmentSql()}`
|
||||
|
||||
@@ -4,6 +4,7 @@ import { ensureMutationReceiptCapacity } from '../../mutation-receipt-capacity'
|
||||
import type { OrchestrationDb } from '../orchestration-db'
|
||||
import { federatedStubHomeRunId } from '../contract-constants'
|
||||
import { insertRemoteDispatchAttachmentRow } from '../dispatch-row-writer'
|
||||
import { REMOTE_DISPATCH_ATTACHMENT_COLUMN_LIST } from '../row-column-lists'
|
||||
|
||||
export function createRemoteDispatchAttachment(
|
||||
this: OrchestrationDb,
|
||||
@@ -92,8 +93,11 @@ export function getRemoteDispatchAttachment(
|
||||
this: OrchestrationDb,
|
||||
dispatchId: string
|
||||
): RemoteDispatchAttachmentRow | undefined {
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The schema-pinned attachment projection returns RemoteDispatchAttachmentRow rows; the adapter exposes unknown.
|
||||
return this.db
|
||||
.prepare('SELECT * FROM remote_dispatch_attachments WHERE dispatch_id = ?')
|
||||
.prepare(
|
||||
`SELECT ${REMOTE_DISPATCH_ATTACHMENT_COLUMN_LIST} FROM remote_dispatch_attachments WHERE dispatch_id = ?`
|
||||
)
|
||||
.get(dispatchId) as RemoteDispatchAttachmentRow | undefined
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ import { OrchestrationError } from '../../orchestration-error'
|
||||
import { paneKeyMatchSuffix, REMOTE_ATTACHMENT_PANE_KEY_MATCH_SUFFIX_SQL } from '../pane-key-match'
|
||||
import type { OrchestrationDb } from '../orchestration-db'
|
||||
import { potentiallyLiveRemoteAttachmentSql } from './remote-attachment-liveness'
|
||||
import { REMOTE_DISPATCH_ATTACHMENT_COLUMN_LIST } from '../row-column-lists'
|
||||
|
||||
export function beginRemoteAttachmentStop(
|
||||
this: OrchestrationDb,
|
||||
@@ -71,17 +72,19 @@ export function findActiveRemoteAttachmentForPane(
|
||||
paneKey: string
|
||||
): RemoteDispatchAttachmentRow | undefined {
|
||||
if (!parsePaneKey(paneKey)) {
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The schema-pinned attachment projection returns RemoteDispatchAttachmentRow rows; the adapter exposes unknown.
|
||||
return this.db
|
||||
.prepare(
|
||||
`SELECT * FROM remote_dispatch_attachments
|
||||
`SELECT ${REMOTE_DISPATCH_ATTACHMENT_COLUMN_LIST} FROM remote_dispatch_attachments
|
||||
WHERE ${potentiallyLiveRemoteAttachmentSql()} AND pane_key = ?
|
||||
ORDER BY rowid DESC LIMIT 1`
|
||||
)
|
||||
.get(paneKey) as RemoteDispatchAttachmentRow | undefined
|
||||
}
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The schema-pinned attachment projection returns RemoteDispatchAttachmentRow rows; the adapter exposes unknown.
|
||||
return this.db
|
||||
.prepare(
|
||||
`SELECT * FROM remote_dispatch_attachments
|
||||
`SELECT ${REMOTE_DISPATCH_ATTACHMENT_COLUMN_LIST} FROM remote_dispatch_attachments
|
||||
WHERE ${potentiallyLiveRemoteAttachmentSql()} AND pane_key IS NOT NULL
|
||||
AND instr(pane_key, ':') > 1
|
||||
AND ${REMOTE_ATTACHMENT_PANE_KEY_MATCH_SUFFIX_SQL} = ?
|
||||
|
||||
@@ -3,6 +3,7 @@ import { OrchestrationDb } from './orchestration-db'
|
||||
import {
|
||||
ATTEMPT_OBSERVATION_FACT_COLUMNS,
|
||||
DISPATCH_CONTEXT_COLUMNS,
|
||||
REMOTE_DISPATCH_ATTACHMENT_COLUMNS,
|
||||
RUN_COLUMNS,
|
||||
selectColumns,
|
||||
TASK_COLUMNS
|
||||
@@ -27,7 +28,8 @@ describe('row column lists', () => {
|
||||
['runs', RUN_COLUMNS],
|
||||
['tasks', TASK_COLUMNS],
|
||||
['dispatch_contexts', DISPATCH_CONTEXT_COLUMNS],
|
||||
['attempt_observation_facts', ATTEMPT_OBSERVATION_FACT_COLUMNS]
|
||||
['attempt_observation_facts', ATTEMPT_OBSERVATION_FACT_COLUMNS],
|
||||
['remote_dispatch_attachments', REMOTE_DISPATCH_ATTACHMENT_COLUMNS]
|
||||
])('projects every %s column the migrated schema declares', (table, columns) => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { AttemptObservationStorageRow } from './attempt-observation-store'
|
||||
import type { DispatchContextRow, RunRow, TaskRow } from '../types'
|
||||
import type { DispatchContextRow, RemoteDispatchAttachmentRow, RunRow, TaskRow } from '../types'
|
||||
|
||||
// Why: `SyncDatabase` refuses to cache any `SELECT *` (node:sqlite can build the first row after a
|
||||
// schema change from stale column names), so a wildcard read recompiles its SQL on every call.
|
||||
@@ -84,6 +84,31 @@ export const ATTEMPT_OBSERVATION_FACT_COLUMNS = [
|
||||
'created_at'
|
||||
] as const satisfies readonly (keyof AttemptObservationStorageRow)[]
|
||||
|
||||
export const REMOTE_DISPATCH_ATTACHMENT_COLUMNS = [
|
||||
'home_run_id',
|
||||
'dispatch_id',
|
||||
'task_id',
|
||||
'home_peer_fingerprint',
|
||||
'protocol_version',
|
||||
'runtime_epoch',
|
||||
'capability_hash',
|
||||
'pane_key',
|
||||
'process_incarnation',
|
||||
'state',
|
||||
'stage',
|
||||
'worktree_id',
|
||||
'terminal_handle',
|
||||
'setup_state',
|
||||
'effects',
|
||||
'residual_resources',
|
||||
'to_worker_imported_sequence',
|
||||
'depth',
|
||||
'consumer_generation',
|
||||
'last_error',
|
||||
'created_at',
|
||||
'updated_at'
|
||||
] as const satisfies readonly (keyof RemoteDispatchAttachmentRow)[]
|
||||
|
||||
// Compile check: a row field added without its column here would silently vanish from the
|
||||
// projection that used to be `SELECT *`, so the missing key must fail the build.
|
||||
type UnprojectedRunColumn = Exclude<keyof RunRow, (typeof RUN_COLUMNS)[number]>
|
||||
@@ -96,12 +121,17 @@ type UnprojectedAttemptObservationColumn = Exclude<
|
||||
keyof AttemptObservationStorageRow,
|
||||
(typeof ATTEMPT_OBSERVATION_FACT_COLUMNS)[number]
|
||||
>
|
||||
type UnprojectedRemoteDispatchAttachmentColumn = Exclude<
|
||||
keyof RemoteDispatchAttachmentRow,
|
||||
(typeof REMOTE_DISPATCH_ATTACHMENT_COLUMNS)[number]
|
||||
>
|
||||
const assertEveryRowColumnProjected: [
|
||||
UnprojectedRunColumn extends never ? true : never,
|
||||
UnprojectedTaskColumn extends never ? true : never,
|
||||
UnprojectedDispatchContextColumn extends never ? true : never,
|
||||
UnprojectedAttemptObservationColumn extends never ? true : never
|
||||
] = [true, true, true, true]
|
||||
UnprojectedAttemptObservationColumn extends never ? true : never,
|
||||
UnprojectedRemoteDispatchAttachmentColumn extends never ? true : never
|
||||
] = [true, true, true, true, true]
|
||||
void assertEveryRowColumnProjected
|
||||
|
||||
/** Projection list for a `SELECT`; `alias` qualifies each name for a joined table (`t.id, …`). */
|
||||
@@ -112,3 +142,6 @@ export function selectColumns(columns: readonly string[], alias?: string): strin
|
||||
export const RUN_COLUMN_LIST = selectColumns(RUN_COLUMNS)
|
||||
export const DISPATCH_CONTEXT_COLUMN_LIST = selectColumns(DISPATCH_CONTEXT_COLUMNS)
|
||||
export const ATTEMPT_OBSERVATION_FACT_COLUMN_LIST = selectColumns(ATTEMPT_OBSERVATION_FACT_COLUMNS)
|
||||
export const REMOTE_DISPATCH_ATTACHMENT_COLUMN_LIST = selectColumns(
|
||||
REMOTE_DISPATCH_ATTACHMENT_COLUMNS
|
||||
)
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { OrchestrationDb } from './db/orchestration-db'
|
||||
import { OrchestrationMailboxOwner, type OrchestrationMailboxLeaf } from './mailbox-owner'
|
||||
|
||||
let db: OrchestrationDb | undefined
|
||||
|
||||
afterEach(() => {
|
||||
db?.close()
|
||||
db = undefined
|
||||
})
|
||||
|
||||
function leafFor(tabId: string, leafId: string): OrchestrationMailboxLeaf {
|
||||
return {
|
||||
tabId,
|
||||
leafId,
|
||||
ptyId: `pty_${leafId}`,
|
||||
writable: true,
|
||||
lastAgentStatus: 'idle',
|
||||
lastAgentStatusObservedLive: true,
|
||||
lastOscTitle: null
|
||||
}
|
||||
}
|
||||
|
||||
function ownerFor(database: OrchestrationDb): OrchestrationMailboxOwner {
|
||||
return new OrchestrationMailboxOwner({
|
||||
getDb: () => database,
|
||||
getLeaf: () => undefined,
|
||||
getLeafKey: (tabId, leafId) => `${tabId}:${leafId}`,
|
||||
getTerminalHandleForLeafKey: (leafKey) => `term_${leafKey}`,
|
||||
getTerminalProcessIncarnation: () => 'process_worker',
|
||||
onRoutedMessageTypes: () => {},
|
||||
onForeignMailboxRouted: () => {}
|
||||
})
|
||||
}
|
||||
|
||||
function attachRemoteDispatch(database: OrchestrationDb, paneKey: string): void {
|
||||
database.createRemoteDispatchAttachment({
|
||||
runId: 'run-home',
|
||||
dispatchId: 'ctx_remote',
|
||||
taskId: 'task_remote',
|
||||
homePeerFingerprint: 'home_peer',
|
||||
protocolVersion: 1,
|
||||
runtimeEpoch: 'worker_epoch',
|
||||
mutationReceipt: {
|
||||
callerFingerprint: 'home_peer',
|
||||
requestId: 'remote_start',
|
||||
method: 'orchestration.federationAttachStart',
|
||||
payloadHash: 'remote_payload'
|
||||
}
|
||||
})
|
||||
database.prepareRemoteAttachmentAuthority({
|
||||
dispatchId: 'ctx_remote',
|
||||
paneKey,
|
||||
processIncarnation: 'process_worker',
|
||||
worktreeId: 'repo::worktree',
|
||||
terminalHandle: `term_${paneKey}`,
|
||||
setupState: 'not_applicable',
|
||||
effects: []
|
||||
})
|
||||
}
|
||||
|
||||
describe('OrchestrationMailboxOwner.resolve', () => {
|
||||
// Why: every idle agent title re-resolves its pane's mailbox, so a statement the SQLite cache
|
||||
// refuses (a `SELECT *`) recompiles on each title while output streams.
|
||||
it.each([
|
||||
['a plain pane', false],
|
||||
['a pane with a remote Dispatch', true]
|
||||
])('reuses every statement it prepares for %s', (_label, attached) => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const leaf = leafFor('tab_a', 'leaf_a')
|
||||
if (attached) {
|
||||
attachRemoteDispatch(db, 'tab_a:leaf_a')
|
||||
}
|
||||
const owner = ownerFor(db)
|
||||
const prepare = db.db.prepare.bind(db.db)
|
||||
const prepared = new Set<string>()
|
||||
vi.spyOn(db.db, 'prepare').mockImplementation((sql) => {
|
||||
prepared.add(sql)
|
||||
return prepare(sql)
|
||||
})
|
||||
|
||||
const mailbox = owner.resolve(leaf)
|
||||
|
||||
expect(mailbox).toBe(attached ? 'dispatch:ctx_remote' : 'term_tab_a:leaf_a')
|
||||
expect(prepared.size).toBeGreaterThan(0)
|
||||
for (const sql of prepared) {
|
||||
expect(prepare(sql), sql).toBe(prepare(sql))
|
||||
}
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user