diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 795beca131e1..e6490fa3943f 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -818,19 +818,16 @@ jobs: # A path filter that matches nothing exits 1 ("No test files found"), so this # lane cannot report success while running zero tests. + # + # The whole directory, not a curated file list. The unit shards exclude this directory + # wholesale, so this is the only lane that runs it, and a curated list made "in the + # directory" and "runs somewhere" two different facts: suites kept drifting into the gap + # and executing on no machine. The directory is the same boundary the shard exclusion + # uses, so a file cannot land between them. - name: Old/new client and server compatibility journeys run: >- pnpm exec vitest run --config config/vitest.config.ts - tests/e2e/cross-version-wire/release-checkout.unit.test.ts - tests/e2e/cross-version-wire/cross-version-browser-placement.unit.test.ts - tests/e2e/cross-version-wire/cross-version-terminal-wire.unit.test.ts - tests/e2e/cross-version-wire/reported-lossy-initial-snapshot.unit.test.ts - tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts - tests/e2e/cross-version-wire/cross-version-worktree-identity-downgrade.unit.test.ts - tests/e2e/cross-version-wire/cross-version-session-tabs-retirement-proof.unit.test.ts - tests/e2e/cross-version-wire/agent-session-resume-marker-downgrade.unit.test.ts - tests/e2e/cross-version-wire/agent-session-unproven-release-downgrade.unit.test.ts - tests/e2e/cross-version-wire/cross-version-worktree-ps-verdict.unit.test.ts + tests/e2e/cross-version-wire/ managed_hook_node18: name: managed hooks on Node 18 diff --git a/config/scripts/cross-version-wire-lane-registration.test.mjs b/config/scripts/cross-version-wire-lane-registration.test.mjs new file mode 100644 index 000000000000..817cf903ce54 --- /dev/null +++ b/config/scripts/cross-version-wire-lane-registration.test.mjs @@ -0,0 +1,183 @@ +import { existsSync, readFileSync, readdirSync } from 'node:fs' +import { join, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { parse } from 'yaml' +import { UNIT_EXCLUDE } from './ci-unit-files.mjs' +import { classifyPrJobs } from './pr-code-change-scope.mjs' + +/** + * Every cross-version wire suite must run on some machine. + * + * PR CI runs `tests/e2e/**` in the sharded unit job, EXCEPT this directory: the suites here + * skew current code against an extracted release, which needs the full history and tags the + * shard's shallow clone does not have. So the shard excludes the directory wholesale and the + * `cross-version-wire` job is the only lane that runs it. + * + * That made the job's vitest argv the single thing deciding whether a file executes. While it + * was a curated list, `orchestration-delivery-downgrade`, `agent-session-death-evidence-downgrade` + * and `published-field-shape` landed in the directory without joining it and ran on no machine. + * + * Both halves matter and being in one is not enough: `CROSS_VERSION_WIRE_PREFIXES` in + * pr-code-change-scope.mjs decides whether the job RUNS for a diff, and the argv decides + * whether the FILE runs once the job started. So every module a suite pairs has to start the + * job too, or a change to it breaks the pairing on a PR that never runs the lane. + */ + +const projectDir = resolve(import.meta.dirname, '../..') +const LANE_JOB = 'cross-version-wire' +const LANE_STEP = 'Old/new client and server compatibility journeys' +const LANE_DIRECTORY = 'tests/e2e/cross-version-wire' +const UNIT_TEST_WORKFLOW = '.github/workflows/unit-tests.yml' + +/** What `config/vitest.config.ts` collects from `tests/e2e/**`. */ +const SUITE_PATTERN = /\.unit\.test\.ts$/ + +/** + * Floor for the suite population, so a broken directory read cannot make every assertion + * below vacuous by finding nothing. Only ever lowered when a suite is genuinely deleted. + */ +const SUITE_FLOOR = 9 + +/** + * The release-checkout machinery extracts a tree and pairs nothing; `release-checkout.unit.test.ts` + * feeds it made-up paths on purpose. + */ +const CHECKOUT_MACHINERY = new Set([ + 'release-checkout.ts', + 'release-checkout-tree.ts', + 'release-checkout.unit.test.ts' +]) + +/** A working-tree import, static or dynamic, from a suite or a wire helper it loads through. */ +const WORKING_TREE_IMPORT = /['"]\.\.\/\.\.\/\.\.\/((?:src|mobile)\/[^'"]+)['"]/g + +/** A repo path handed to `importReleaseCheckoutModule`, inline or through a constant. */ +const CHECKOUT_MODULE_PATH = /['"]\/?((?:src|mobile)\/[^'"\s]+\.tsx?)['"]/g + +/** Floor for the paired modules found, so a scanner that stops matching cannot pass empty. */ +const PAIRED_MODULE_FLOOR = 25 + +function readWorkflow(relativePath) { + return parse(readFileSync(join(projectDir, relativePath), 'utf8')) +} + +/** The vitest argv of the lane's one test step. */ +function readLaneArgv() { + const workflow = readWorkflow('.github/workflows/pr.yml') + const job = workflow.jobs?.[LANE_JOB] + if (!job) { + throw new Error( + `No ${LANE_JOB} job in .github/workflows/pr.yml. If it was renamed, update LANE_JOB here -- do not delete this guard.` + ) + } + const step = (job.steps ?? []).find((candidate) => candidate?.name === LANE_STEP) + if (!step) { + throw new Error( + `No "${LANE_STEP}" step in the ${LANE_JOB} job. If it was renamed, update LANE_STEP here -- do not delete this guard.` + ) + } + const run = String(step.run ?? '') + if (!run.includes('vitest run')) { + throw new Error(`The "${LANE_STEP}" step no longer invokes vitest; this guard is stale.`) + } + return run.split(/\s+/).filter((token) => token.startsWith(`${LANE_DIRECTORY}`)) +} + +const laneArgv = readLaneArgv() +const suites = readdirSync(join(projectDir, LANE_DIRECTORY)) + .filter((name) => SUITE_PATTERN.test(name)) + .map((name) => `${LANE_DIRECTORY}/${name}`) + .sort() + +function resolveModule(specifier) { + if (/\.tsx?$/.test(specifier)) { + return specifier + } + const candidates = [`${specifier}.ts`, `${specifier}.tsx`, `${specifier}/index.ts`] + return candidates.find((path) => existsSync(join(projectDir, path))) ?? specifier +} + +/** + * Each module the suites load for pairing, with the first file that loads it. Only direct + * loads: a module reached through one (the orchestration schema, via db.ts) is gated by hand. + */ +function readPairedModules() { + const modules = new Map() + for (const name of readdirSync(join(projectDir, LANE_DIRECTORY)).sort()) { + if (!name.endsWith('.ts') || CHECKOUT_MACHINERY.has(name)) { + continue + } + const source = readFileSync(join(projectDir, LANE_DIRECTORY, name), 'utf8') + for (const pattern of [WORKING_TREE_IMPORT, CHECKOUT_MODULE_PATH]) { + for (const [, specifier] of source.matchAll(pattern)) { + const path = resolveModule(specifier) + if (!modules.has(path)) { + modules.set(path, name) + } + } + } + } + return modules +} + +const pairedModules = readPairedModules() + +/** A vitest positional selects a file when it names the file or a directory above it. */ +function selectedByLane(path) { + return laneArgv.some((token) => path === token || path.startsWith(token)) +} + +describe('cross-version wire suites run somewhere', () => { + it('reads a plausible number of suites', () => { + expect(suites.length).toBeGreaterThanOrEqual(SUITE_FLOOR) + }) + + it('selects every suite in the directory from the lane argv', () => { + expect(laneArgv.length).toBeGreaterThan(0) + expect(suites.filter((path) => !selectedByLane(path))).toEqual([]) + }) + + it('keeps the unit shards excluding the directory, which is what makes this lane the only one', () => { + const workflow = readWorkflow(UNIT_TEST_WORKFLOW) + const shardStep = (workflow.jobs?.test?.steps ?? []).find( + (step) => typeof step?.run === 'string' && step.run.includes('--shard=') + ) + expect(shardStep, `no sharded vitest step in ${UNIT_TEST_WORKFLOW}`).toBeDefined() + // If this exclusion is ever dropped the suites gain a second lane, and the argv stops + // being the only thing that decides whether a file runs. Revisit this guard, do not + // delete it: the shallow shard clone has no tags for an extracted release to skew against. + // The shard applies UNIT_EXCLUDE only under this env flag; the lane's own run does not set it. + expect(shardStep.env?.ORCA_BALANCE_UNIT_SHARDS).toBe('1') + expect(UNIT_EXCLUDE).toContain(`${LANE_DIRECTORY}/**`) + }) + + it('runs the lane for a change to any suite it owns', () => { + for (const path of suites) { + expect(classifyPrJobs([path])[LANE_JOB], path).toBe(true) + } + }) + + it('finds a plausible number of paired modules', () => { + expect( + pairedModules.size, + `Only ${pairedModules.size} paired modules were recognized; the floor is ` + + `${PAIRED_MODULE_FLOOR}. The scanner has probably stopped matching a load shape.` + ).toBeGreaterThanOrEqual(PAIRED_MODULE_FLOOR) + }) + + it('pairs only modules that exist', () => { + const missing = [...pairedModules] + .filter(([path]) => !existsSync(join(projectDir, path))) + .map(([path, loader]) => `${path} (loaded by ${loader})`) + expect(missing).toEqual([]) + }) + + it('runs the lane for a change to any module a suite pairs', () => { + const ungated = [...pairedModules] + .filter(([path]) => !classifyPrJobs([path])[LANE_JOB]) + .map(([path, loader]) => `${path} (loaded by ${loader})`) + expect(ungated, 'add each to CROSS_VERSION_WIRE_PREFIXES in pr-code-change-scope.mjs').toEqual( + [] + ) + }) +}) diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index 98dc914052af..f247d731159e 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -164,7 +164,18 @@ const CROSS_VERSION_WIRE_PREFIXES = [ 'src/main/runtime/runtime-worktree-agent-', 'src/main/runtime/runtime-worktree-pty-agent-sources', 'src/shared/runtime-worktree-contracts', - 'src/renderer/src/runtime/remote-runtime-terminal-multiplexer' + 'src/renderer/src/runtime/remote-runtime-terminal-multiplexer', + 'src/main/runtime/rpc/methods/index.ts', + 'src/main/runtime/runtime-subscription-registry.ts', + 'src/shared/terminal-retirement-proof-ledger.ts', + 'src/main/runtime/mobile-session-terminal-retirement.ts', + 'src/main/runtime/mobile-session-terminal-retirement-proof.ts', + 'src/main/persistence/tracking-repos/worktree-identity-migration.ts', + 'src/main/runtime/orchestration/db.ts', + 'src/main/runtime/orchestration/db/contract-constants.ts', + // Why: the orchestration downgrade suite opens a current-schema database with old code; db.ts reaches this transitively. + 'src/main/runtime/orchestration/db/schema/', + 'mobile/src/worktree/agent-row-display.ts' ] const MANAGED_HOOK_PREFIXES = [ @@ -406,6 +417,10 @@ export function classifyPrJobs(changedFiles) { // that is exactly the diff that changes the page this job builds. Gated on should_run it would // skip on every PR that can break it and run on none. jobs.mobile_web_app = jobs.mobile_web_app || changesMobileWebApp(changedFiles) + // Why outside should_run, for the same reason: the phone's `worktree ps` row reader is paired + // against the host, and a change to it is a mobile-only diff. + jobs['cross-version-wire'] = + jobs['cross-version-wire'] || jobDetector('cross-version-wire')(changedFiles) return { should_run: shouldRun, native_cache_changed: shouldRun && (emptyDiff || changedFiles.some(isNativeCacheInputPath)), diff --git a/config/scripts/pr-code-change-scope.test.mjs b/config/scripts/pr-code-change-scope.test.mjs index 1981623d2816..23df99865594 100644 --- a/config/scripts/pr-code-change-scope.test.mjs +++ b/config/scripts/pr-code-change-scope.test.mjs @@ -353,7 +353,16 @@ describe('per-job path classification', () => { 'src/main/runtime/runtime-worktree-agent-rows.ts', 'src/main/runtime/runtime-worktree-pty-agent-sources.ts', 'src/shared/runtime-worktree-contracts.ts', - 'src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts' + 'src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts', + 'src/main/runtime/rpc/methods/index.ts', + 'src/main/runtime/runtime-subscription-registry.ts', + 'src/shared/terminal-retirement-proof-ledger.ts', + 'src/main/runtime/mobile-session-terminal-retirement.ts', + 'src/main/runtime/mobile-session-terminal-retirement-proof.ts', + 'src/main/persistence/tracking-repos/worktree-identity-migration.ts', + 'src/main/runtime/orchestration/db.ts', + 'src/main/runtime/orchestration/db/contract-constants.ts', + 'src/main/runtime/orchestration/db/schema/migrate-v42.ts' ]) { expectClassification([file], { 'cross-version-wire': true, @@ -365,6 +374,9 @@ describe('per-job path classification', () => { ['tests/e2e/cross-version-wire/cross-version-terminal-wire.unit.test.ts'], { 'cross-version-wire': true } ) + const phoneRowReader = classifyPrJobs(['mobile/src/worktree/agent-row-display.ts']) + expect(phoneRowReader.should_run).toBe(false) + expect(phoneRowReader['cross-version-wire']).toBe(true) }) it('runs workflow-self-change and lockfile diffs as force-all', () => { diff --git a/docs/reference/remote-wire-compatibility.md b/docs/reference/remote-wire-compatibility.md index b2bcb3c40f4e..bd6eb5fcf3ae 100644 --- a/docs/reference/remote-wire-compatibility.md +++ b/docs/reference/remote-wire-compatibility.md @@ -192,10 +192,21 @@ Run it with: pnpm exec vitest run --config config/vitest.config.ts tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts ``` -The harness covers the terminal stream and the structured agent-session surface. It does -**not** cover the session-tab sync channel, legacy agent-session publications, file or Git -RPCs, mobile/E2EE framing, or the relay transport. A change on those paths still needs its -own reasoning against the three rules above. +The harness covers the terminal stream and the structured agent-session surface. The one +agent-status path it pairs is the `worktree ps` row (`cross-version-worktree-ps-verdict.unit.test.ts`). +It does **not** cover the session-tab sync channel, including the `agentStatus` each terminal +tab carries to paired desktops and phones, legacy agent-session publications, file or Git RPCs, +mobile/E2EE framing, or the relay transport. A change on those paths still needs its own +reasoning against the three rules above. + +Every suite in `tests/e2e/cross-version-wire/` runs in the `cross-version-wire` PR job and +nowhere else — the unit shards exclude the directory, because a shallow shard clone has no tags +for an extracted release to skew against. The job therefore names the directory rather than a +list of files. Which diffs start the job is a separate list, `CROSS_VERSION_WIRE_PREFIXES` in +`config/scripts/pr-code-change-scope.mjs`, and every module a suite pairs must be on it. +`config/scripts/cross-version-wire-lane-registration.test.mjs` holds both, but it sees only the +modules a suite loads directly; one reached through them, such as the orchestration schema +migrations behind `db.ts`, has to be added to the list by hand. ## Worked example: `agentWait` on terminal and worker reads diff --git a/tests/e2e/cross-version-wire/agent-session-death-evidence-downgrade.unit.test.ts b/tests/e2e/cross-version-wire/agent-session-death-evidence-downgrade.unit.test.ts index 96780a185176..001575587bd7 100644 --- a/tests/e2e/cross-version-wire/agent-session-death-evidence-downgrade.unit.test.ts +++ b/tests/e2e/cross-version-wire/agent-session-death-evidence-downgrade.unit.test.ts @@ -14,6 +14,7 @@ import { importReleaseCheckoutModule, materializeReleaseCheckout } from './relea const BASELINE_REF = 'v1.4.211' const SESSION = 'session-alpha-1' +// Why the explicit budget: a cold run may extract the pinned release inside this test. test('an older build loads a proof of death that names its owner and its last proof of life', async () => { const directory = mkdtempSync(join(tmpdir(), 'orca-death-evidence-downgrade-')) try { @@ -58,4 +59,4 @@ test('an older build loads a proof of death that names its owner and its last pr } finally { rmSync(directory, { recursive: true, force: true }) } -}) +}, 180_000) 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 index 8126d33d6010..0398b4085855 100644 --- a/tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts +++ b/tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts @@ -9,6 +9,7 @@ import { importReleaseCheckoutModule, materializeReleaseCheckout } from './relea // Pin the last pre-v41 implementation: this contract specifically exercises status-only readers. const PRE_V41 = 'aac38d698ff75ac4c8658addab48ef5a83617619' +// Why the explicit budget: this pins a commit no sibling shares, so a cold run extracts its own checkout. test('pre-v41 code opens, acknowledges and writes a current-schema database, then current code reopens it', async () => { const checkout = await materializeReleaseCheckout(PRE_V41) const baseline = await importReleaseCheckoutModule( @@ -67,4 +68,4 @@ test('pre-v41 code opens, acknowledges and writes a current-schema database, the db?.close() rmSync(directory, { recursive: true, force: true }) } -}) +}, 180_000) diff --git a/tests/e2e/cross-version-wire/release-checkout-tree.ts b/tests/e2e/cross-version-wire/release-checkout-tree.ts index b5f97e78b242..73438b1eceaf 100644 --- a/tests/e2e/cross-version-wire/release-checkout-tree.ts +++ b/tests/e2e/cross-version-wire/release-checkout-tree.ts @@ -1,7 +1,9 @@ import { readFile, readdir, rm, writeFile } from 'node:fs/promises' import { dirname, join, relative } from 'node:path' -const CHECKOUT_PROCESS_TIMEOUT_MS = 45_000 +// Why not 45s: the lane runs the whole directory at once, so several pinned checkouts extract +// concurrently beside running suites; this bounds a hung `git archive`, not a solo cold fill. +const CHECKOUT_PROCESS_TIMEOUT_MS = 120_000 const CHECKOUT_MAX_OUTPUT_BYTES = 1024 * 1024 // Why: the wire endpoints only need the runtime RPC host, the renderer client, and