Skip to content
Open
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
17 changes: 7 additions & 10 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
183 changes: 183 additions & 0 deletions config/scripts/cross-version-wire-lane-registration.test.mjs
Original file line number Diff line number Diff line change
@@ -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(
[]
)
})
})
17 changes: 16 additions & 1 deletion config/scripts/pr-code-change-scope.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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/',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orchestration-schema-version-skew.ts sits one directory up from this new prefix and is imported by the now-gated schema/migrate.ts; it owns resolveOrchestrationMigrationStartVersion, the exact cross-version decision the orchestration downgrade suite exercises. A change to it does not start the lane — classifyPrJobs(['src/main/runtime/orchestration/orchestration-schema-version-skew.ts'])['cross-version-wire'] is false. Consider adding it alongside db/schema/.

Technical details
# The orchestration version-skew resolver is ungated

## Affected sites
- `config/scripts/pr-code-change-scope.mjs:177` — `src/main/runtime/orchestration/db/schema/` was just added; the sibling resolver one directory up is not on the list.
- `src/main/runtime/orchestration/orchestration-schema-version-skew.ts:184` — `resolveOrchestrationMigrationStartVersion`, the function that decides whether a stored DB migrates or keeps its stamp.
- `src/main/runtime/orchestration/db/schema/migrate.ts:1` — imports the resolver; `migrate.ts` lives inside the newly gated directory.
- `tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts:5,25,43` — drives `OrchestrationDb` (which runs the resolver through `migrate`) against a pre-v41 checkout, so the lane can fail for a change here.

## Required outcome
- A change to `orchestration-schema-version-skew.ts` starts the `cross-version-wire` job, the same standard the `db/schema/` prefix was just added for.
- Add a representative case to the `pr-code-change-scope.test.mjs` classifier list so the on-disk path is exercised too.

## Open questions for the human
- The hand-gated transitive list also has no existence guard, and `db/orchestration-db.ts` / `db/attach-orchestration-db-methods.ts` are likewise ungated. Is `db/schema/` the intended coverage boundary, or the transitive closure around `db.ts`?
Suggested change
'src/main/runtime/orchestration/db/schema/',
'src/main/runtime/orchestration/db/schema/',
'src/main/runtime/orchestration/orchestration-schema-version-skew',

'mobile/src/worktree/agent-row-display.ts'
]

const MANAGED_HOOK_PREFIXES = [
Expand Down Expand Up @@ -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)),
Expand Down
14 changes: 13 additions & 1 deletion config/scripts/pr-code-change-scope.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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', () => {
Expand Down
19 changes: 15 additions & 4 deletions docs/reference/remote-wire-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
4 changes: 3 additions & 1 deletion tests/e2e/cross-version-wire/release-checkout-tree.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading