diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 031cdbaac2c..6d681ca4ea4 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -31,6 +31,7 @@ jobs: static_analysis: ${{ steps.filter.outputs.static_analysis }} typecheck: ${{ steps.filter.outputs.typecheck }} git_compatibility: ${{ steps.filter.outputs.git_compatibility }} + codex_index_heal_contract: ${{ steps.filter.outputs.codex_index_heal_contract }} xterm_patch_sync: ${{ steps.filter.outputs.xterm_patch_sync }} shell_contracts: ${{ steps.filter.outputs.shell_contracts }} test: ${{ steps.filter.outputs.test }} @@ -294,6 +295,47 @@ jobs: done exit "$status" + # Why this job: Orca's session index-heal depends on a Codex behavior — a + # `thread/read` of an unindexed rollout performs a read-repair that inserts the + # `threads` row. Every unit test drives a stub app-server and asserts only that the + # call did not error, so if Codex dropped the repair they would all stay green while + # the subsystem went inert. This runs the pinned real binary and fails when the + # repair stops happening. Pinned because the binary is the thing expected to drift. + codex_index_heal_contract: + name: Codex index-heal contract + needs: [code_paths] + if: needs.code_paths.outputs.codex_index_heal_contract == 'true' + runs-on: ubuntu-latest + env: + CODEX_CLI_VERSION: '0.150.1' + + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + persist-credentials: false + + - uses: ./.github/actions/install-node-dependencies + + - name: Install pinned Codex CLI + run: | + set -euo pipefail + npm install --no-audit --no-fund --prefix "$RUNNER_TEMP/codex-cli" \ + "@openai/codex@$CODEX_CLI_VERSION" + + - name: Verify Codex index-heal contract + env: + # Why REQUIRED: without a binary the suite skips, and a job that skips + # reports success. This turns a failed or missing install into a red test + # instead of a green no-op. + ORCA_CODEX_CONTRACT_REQUIRED: '1' + ORCA_CODEX_CONTRACT_VERSION: ${{ env.CODEX_CLI_VERSION }} + run: | + set -euo pipefail + ORCA_CODEX_CONTRACT_BINARY="$RUNNER_TEMP/codex-cli/node_modules/.bin/codex" \ + pnpm exec vitest run --config config/vitest.config.ts \ + src/main/codex/codex-index-heal-binary-contract.test.ts + xterm_patch_sync: name: xterm patch sync needs: [code_paths] @@ -843,6 +885,7 @@ jobs: - root_directory_guard - typecheck - git_compatibility + - codex_index_heal_contract - xterm_patch_sync - shell_contracts - test @@ -873,6 +916,8 @@ jobs: TYPECHECK_SHOULD_RUN: ${{ needs.code_paths.outputs.typecheck }} GIT_COMPATIBILITY: ${{ needs.git_compatibility.result }} GIT_COMPATIBILITY_SHOULD_RUN: ${{ needs.code_paths.outputs.git_compatibility }} + CODEX_INDEX_HEAL_CONTRACT: ${{ needs.codex_index_heal_contract.result }} + CODEX_INDEX_HEAL_CONTRACT_SHOULD_RUN: ${{ needs.code_paths.outputs.codex_index_heal_contract }} XTERM_PATCH_SYNC: ${{ needs.xterm_patch_sync.result }} XTERM_PATCH_SYNC_SHOULD_RUN: ${{ needs.code_paths.outputs.xterm_patch_sync }} SHELL_CONTRACTS: ${{ needs.shell_contracts.result }} @@ -918,6 +963,7 @@ jobs: check_job static_analysis "$STATIC_ANALYSIS" "$STATIC_ANALYSIS_SHOULD_RUN" check_job typecheck "$TYPECHECK" "$TYPECHECK_SHOULD_RUN" check_job git_compatibility "$GIT_COMPATIBILITY" "$GIT_COMPATIBILITY_SHOULD_RUN" + check_job codex_index_heal_contract "$CODEX_INDEX_HEAL_CONTRACT" "$CODEX_INDEX_HEAL_CONTRACT_SHOULD_RUN" check_job xterm_patch_sync "$XTERM_PATCH_SYNC" "$XTERM_PATCH_SYNC_SHOULD_RUN" check_job shell_contracts "$SHELL_CONTRACTS" "$SHELL_CONTRACTS_SHOULD_RUN" check_job test "$TEST" "$TEST_SHOULD_RUN" diff --git a/config/scripts/codex-index-heal-contract-workflow.test.mjs b/config/scripts/codex-index-heal-contract-workflow.test.mjs new file mode 100644 index 00000000000..62c9787a12e --- /dev/null +++ b/config/scripts/codex-index-heal-contract-workflow.test.mjs @@ -0,0 +1,37 @@ +import { readFileSync } from 'node:fs' +import { parse } from 'yaml' +import { describe, expect, it } from 'vitest' + +describe('Codex index-heal contract PR gate', () => { + const workflow = parse(readFileSync('.github/workflows/pr.yml', 'utf8')) + const job = workflow.jobs.codex_index_heal_contract + + it('installs and verifies against one pinned Codex version', () => { + const install = job.steps.find((step) => step.name === 'Install pinned Codex CLI') + const verify = job.steps.find((step) => step.name === 'Verify Codex index-heal contract') + + // Why one source: the install and the runtime version assertion drifting apart is + // the failure that would leave this job verifying a Codex nobody declared. + expect(job.env.CODEX_CLI_VERSION).toMatch(/^\d+\.\d+\.\d+$/) + expect(install.run).toContain('"@openai/codex@$CODEX_CLI_VERSION"') + expect(verify.env.ORCA_CODEX_CONTRACT_VERSION).toBe('${{ env.CODEX_CLI_VERSION }}') + + // The install prefix and the binary the test is pointed at must be the same tree. + expect(install.run).toContain('--prefix "$RUNNER_TEMP/codex-cli"') + expect(verify.run).toContain( + 'ORCA_CODEX_CONTRACT_BINARY="$RUNNER_TEMP/codex-cli/node_modules/.bin/codex"' + ) + expect(verify.run).toContain('src/main/codex/codex-index-heal-binary-contract.test.ts') + }) + + it('fails rather than skipping when the Codex binary is missing', () => { + const verify = job.steps.find((step) => step.name === 'Verify Codex index-heal contract') + + // Why asserted: the contract skips itself without a binary, so a failed install + // would otherwise turn this job into a green no-op that verifies nothing. + expect(verify.env.ORCA_CODEX_CONTRACT_REQUIRED).toBe('1') + expect(job.steps.find((step) => step.name === 'Install pinned Codex CLI').run).toContain( + 'set -euo pipefail' + ) + }) +}) diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index 3ea588aaf8f..c296ad9e893 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -20,6 +20,7 @@ export const PR_CHECK_JOBS = [ 'static_analysis', 'typecheck', 'git_compatibility', + 'codex_index_heal_contract', 'xterm_patch_sync', 'shell_contracts', 'test', @@ -48,6 +49,24 @@ const GIT_COMPAT_PREFIXES = [ 'config/scripts/git-binary-compatibility' ] +// Why narrow: the contract pins Codex's read-repair, so it runs when the heal that +// depends on it, its app-server transport, or the contract itself changes. +const CODEX_INDEX_HEAL_CONTRACT_PREFIXES = [ + 'src/main/codex/codex-index-heal-binary-contract', + 'src/main/codex/codex-session-index-heal', + 'src/main/codex/codex-app-server-session', + 'src/main/codex/codex-state-db', + 'src/main/sqlite/sync-database', + 'src/main/codex/codex-app-server-capability-signal', + 'src/main/codex/codex-process-exit-deadline', + 'src/main/codex/codex-session-backfill', + 'src/main/codex/codex-session-index-heal-state', + 'src/main/codex-cli/command', + 'src/main/win32-utils', + 'src/shared/node-cli-command-resolution', + 'src/shared/windows-batch-spawn' +] + const XTERM_PREFIXES = [ 'config/patches/xterm-upstream.json', 'config/patches/@xterm', @@ -256,6 +275,9 @@ function jobDetector(job) { switch (job) { case 'git_compatibility': return (files) => files.some((file) => matchesPrefix(file, GIT_COMPAT_PREFIXES)) + case 'codex_index_heal_contract': + return (files) => + files.some((file) => matchesPrefix(file, CODEX_INDEX_HEAL_CONTRACT_PREFIXES)) case 'xterm_patch_sync': return (files) => files.some((file) => matchesPrefix(file, XTERM_PREFIXES)) case 'shell_contracts': diff --git a/config/scripts/pr-code-change-scope.test.mjs b/config/scripts/pr-code-change-scope.test.mjs index 3f35dae1dd9..f9411eed956 100644 --- a/config/scripts/pr-code-change-scope.test.mjs +++ b/config/scripts/pr-code-change-scope.test.mjs @@ -17,6 +17,7 @@ const expensiveJobs = [ 'static_analysis', 'typecheck', 'git_compatibility', + 'codex_index_heal_contract', 'xterm_patch_sync', 'shell_contracts', 'test', @@ -118,6 +119,49 @@ describe('per-job path classification', () => { }) }) + it('runs the Codex index-heal contract only when the heal or its transport changes', () => { + expectClassification(['src/main/codex/codex-session-index-heal.ts'], { + codex_index_heal_contract: true, + package: true, + package_windows: true + }) + expectClassification(['src/main/sqlite/sync-database.ts'], { + codex_index_heal_contract: true, + package: true, + package_windows: true + }) + expectClassification(['src/main/codex/codex-app-server-session.ts'], { + codex_index_heal_contract: true, + package: true, + package_windows: true + }) + expectClassification(['src/main/codex/codex-index-heal-binary-contract.test.ts'], { + codex_index_heal_contract: true + }) + // Keep the real-binary gate live when a transport or launch dependency changes. + for (const file of [ + 'src/main/codex/codex-app-server-capability-signal.ts', + 'src/main/codex/codex-process-exit-deadline.ts', + 'src/main/codex/codex-session-backfill.ts', + 'src/main/codex/codex-session-index-heal-state.ts', + 'src/main/codex-cli/command.ts', + 'src/main/win32-utils.ts', + 'src/shared/node-cli-command-resolution.ts', + 'src/shared/windows-batch-spawn.ts' + ]) { + expectClassification([file], { + codex_index_heal_contract: true, + package: true, + package_windows: true + }) + } + // A neighbouring Codex module must not drag the real-binary job in. + expectClassification(['src/main/codex/codex-home-paths.ts'], { + package: true, + package_windows: true + }) + }) + it('runs xterm patch sync only when xterm inputs change', () => { expectClassification(['config/patches/xterm-upstream.json'], { xterm_patch_sync: true diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index 40c110fc8d9..ba03c1c2d3f 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -410,6 +410,7 @@ describe('PR workflow parallelism', () => { 'root_directory_guard', 'typecheck', 'git_compatibility', + 'codex_index_heal_contract', 'xterm_patch_sync', 'shell_contracts', 'test', diff --git a/src/main/codex/codex-index-heal-binary-contract.test.ts b/src/main/codex/codex-index-heal-binary-contract.test.ts new file mode 100644 index 00000000000..8f6304b88e0 --- /dev/null +++ b/src/main/codex/codex-index-heal-binary-contract.test.ts @@ -0,0 +1,210 @@ +import { execFile } from 'node:child_process' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { promisify } from 'node:util' +import { afterEach, beforeAll, describe, expect, it } from 'vitest' +import SyncDatabase from '../sqlite/sync-database' +import { runCodexAppServerSession, type CodexAppServerRpc } from './codex-app-server-session' +import { findNewestCodexStateDbPath } from './codex-state-db' + +// Why this file exists: every other index-heal test drives a stub app-server and +// asserts "healed" as "the `thread/read` call did not error". That pins Orca's half +// of the contract and nothing about Codex's. The behavior Orca actually depends on +// lives in the Codex binary — a read of an unindexed rollout performs a read-repair +// that inserts the `threads` row. If Codex ever dropped that repair, the stub-driven +// tests would all stay green while the subsystem went silently inert. This is the +// real-binary backstop, built to the same shape as the Git binary compatibility +// contract in src/shared/git-binary-compatibility.test.ts. +// +// Keep it narrow. It pins the four arms that ablation established Orca relies on, +// and deliberately asserts nothing else about the app-server, so an unrelated Codex +// release does not redden it into being disabled. + +const execFileAsync = promisify(execFile) +const binary = process.env.ORCA_CODEX_CONTRACT_BINARY +const expectedVersion = process.env.ORCA_CODEX_CONTRACT_VERSION +const describeCodexContract = binary ? describe : describe.skip + +// Why this guard: skipping is the right local-dev default, but a CI job whose whole +// purpose is the real binary must not pass by reporting zero assertions. The job sets +// ORCA_CODEX_CONTRACT_REQUIRED=1, which turns a missing binary into a red test. +describe.runIf(process.env.ORCA_CODEX_CONTRACT_REQUIRED === '1' && !binary)( + 'codex binary index-heal contract prerequisites', + () => { + it('was given a Codex binary to run against', () => { + expect.fail( + 'ORCA_CODEX_CONTRACT_REQUIRED=1 but ORCA_CODEX_CONTRACT_BINARY is unset, so the contract would have silently skipped' + ) + }) + } +) + +// Why fixed: `thread/read` never reaches the network, and a whole session is +// spawn + initialize + one RPC. A generous ceiling still fails fast on a wedged child. +const SESSION_TIMEOUT_MS = 60_000 +// Why: each contract case can use three bounded app-server sessions; keep Vitest's +// watchdog longer than both child deadlines so cleanup cannot race a test timeout. +const CONTRACT_TEST_TIMEOUT_MS = SESSION_TIMEOUT_MS * 3 + 10_000 + +type ThreadRow = { id: string; archived: number } + +describeCodexContract( + 'codex binary index-heal contract', + { timeout: CONTRACT_TEST_TIMEOUT_MS }, + () => { + const disposableHomes: string[] = [] + + beforeAll(async () => { + // Why assert the version: the whole point of a real-binary check is that the + // binary drifts. A job that quietly ran some other Codex would report a + // contract this repo never verified. + const { stdout } = await execFileAsync(binary!, ['--version'], { timeout: SESSION_TIMEOUT_MS }) + expect(stdout.trim()).toBe(`codex-cli ${expectedVersion}`) + }) + + afterEach(() => { + while (disposableHomes.length > 0) { + rmSync(disposableHomes.pop() as string, { recursive: true, force: true }) + } + }) + + /** + * Builds a disposable CODEX_HOME in the state Orca actually heals from: Codex's + * own one-shot sqlite backfill has already run and stamped itself `complete`, so + * rollouts that appear afterwards are exactly the ones it will never index on its + * own. Never points at the user's real ~/.codex. + */ + async function createBackfilledCodexHome(): Promise { + const home = mkdtempSync(join(tmpdir(), 'orca-codex-heal-contract-')) + disposableHomes.push(home) + mkdirSync(join(home, 'sessions'), { recursive: true }) + // An app-server session over an empty sessions tree is what stamps the backfill complete. + await runAppServerSession(home, async () => undefined) + expect(readThreadRows(home)).toEqual([]) + return home + } + + function writeRollout(home: string, threadId: string, stamp: string): void { + const dayDir = join(home, 'sessions', '2026', '08', '29') + mkdirSync(dayDir, { recursive: true }) + const meta = { + timestamp: '2026-08-29T21:17:33.760Z', + ordinal: 0, + type: 'session_meta', + payload: { + session_id: threadId, + id: threadId, + timestamp: '2026-08-29T21:17:26.840Z', + cwd: tmpdir(), + originator: 'codex-tui', + cli_version: expectedVersion, + source: 'cli', + thread_source: 'user', + model_provider: 'openai' + } + } + const userMessage = { + timestamp: '2026-08-29T21:17:40.000Z', + ordinal: 1, + type: 'event_msg', + payload: { type: 'user_message', message: 'index-heal contract fixture' } + } + writeFileSync( + join(dayDir, `rollout-${stamp}-${threadId}.jsonl`), + `${JSON.stringify(meta)}\n${JSON.stringify(userMessage)}\n` + ) + } + + // Why reuse runCodexAppServerSession rather than a local JSON-RPC client: it is the + // transport the heal itself uses, so a framing or handshake regression that would + // break the heal breaks this check too. + async function runAppServerSession( + home: string, + body: (rpc: CodexAppServerRpc) => Promise + ): Promise { + return runCodexAppServerSession( + { + command: binary!, + args: ['app-server'], + cliPath: binary!, + env: { CODEX_HOME: home }, + timeoutMs: SESSION_TIMEOUT_MS + }, + body + ) + } + + function readThreadRows(home: string): ThreadRow[] { + const stateDbPath = findNewestCodexStateDbPath(home) + if (!stateDbPath) { + return [] + } + const db = new SyncDatabase(stateDbPath, { readonly: true, fileMustExist: true }) + try { + return db + .prepare('SELECT id, archived FROM threads ORDER BY id') + .all() as unknown as ThreadRow[] + } finally { + db.close() + } + } + + // Arms 1 and 2 are one test on purpose: the "no read" pass is the negative control + // that makes the insert causal rather than incidental. Split across two tests they + // would run against two different homes and prove nothing about each other. + it('inserts the state row because of the read, not because the server ran', async () => { + const home = await createBackfilledCodexHome() + const threadId = '01a04f62-715e-7830-9371-50db585caa71' + writeRollout(home, threadId, '2026-08-29T14-17-26') + + // Control: a complete app-server session that issues no `thread/read`. + await runAppServerSession(home, async () => undefined) + expect(readThreadRows(home)).toEqual([]) + + await runAppServerSession(home, async (rpc) => { + await rpc.request('thread/read', { threadId }) + }) + + expect(readThreadRows(home)).toEqual([{ id: threadId, archived: 0 }]) + }) + + it('leaves an already-indexed thread as a single row', async () => { + const home = await createBackfilledCodexHome() + const threadId = '01a04f62-715e-7830-9371-50db585caa72' + writeRollout(home, threadId, '2026-08-29T15-00-00') + + await runAppServerSession(home, async (rpc) => { + await rpc.request('thread/read', { threadId }) + }) + expect(readThreadRows(home)).toEqual([{ id: threadId, archived: 0 }]) + + await runAppServerSession(home, async (rpc) => { + await rpc.request('thread/read', { threadId }) + }) + + expect(readThreadRows(home)).toEqual([{ id: threadId, archived: 0 }]) + }) + + // Why this arm: the heal reads tens of thousands of rollouts. If a read cleared + // `archived`, the pass would silently resurrect every thread the user had archived. + it('stamps an archived thread archived rather than resurrecting it', async () => { + const home = await createBackfilledCodexHome() + const threadId = '01a04f62-715e-7830-9371-50db585caa73' + writeRollout(home, threadId, '2026-08-29T16-00-00') + + // The archived state is created here, never assumed: a real Codex home may + // never have had a thread archived, and this arm would then pass vacuously. + await runAppServerSession(home, async (rpc) => { + await rpc.request('thread/read', { threadId }) + await rpc.request('thread/archive', { threadId }) + }) + expect(readThreadRows(home)).toEqual([{ id: threadId, archived: 1 }]) + + await runAppServerSession(home, async (rpc) => { + await rpc.request('thread/read', { threadId }) + }) + + expect(readThreadRows(home)).toEqual([{ id: threadId, archived: 1 }]) + }) +})