fork-ci: one cargo invocation per step; Cargo.lock selects only affected packages

[thedancingdeveloper]

What

fork-ci ran cargo clippy -p, cargo nextest run -p and cargo fmt -p once per crate, and treated any codex-rs/Cargo.lock change as every package. On the 30-vCPU rust-heavy runner that was ~160 serial cargo invocations, each re-resolving features, with the CPU 3–13% busy.

Change 1 — one invocation per step

fmt, clippy and nextest now take every selected package as repeated -p flags in a single invocation. Clippy still emits JSON and the changed-file gate is unchanged: the one JSON stream is split back into per-crate files by package_id afterwards. --no-fail-fast, the known-failures filterset, the upstream nextest profile and the workspace-bins build (because cargo test -p will not build another package's cargo_bin()) are all kept. Test selection is otherwise unchanged.

Change 2 — Cargo.lock selects only what moved

A lock-only change now diffs cargo metadata --format-version 1 resolve graphs between the base lock and HEAD, and selects only workspace members whose reachable dependency set differs. A workspace Cargo.toml change still selects everything; a member manifest selects that member plus its workspace reverse-dependencies. Covered by .github/scripts/test_fork_changed_packages.py.

Change 3 — CI-scoped build settings (job env only, not Cargo.toml)

• CARGO_PROFILE_DEV_DEBUG=line-tables-only and CARGO_PROFILE_TEST_DEBUG=line-tables-only — keeps file/line in backtraces, which tests read, and drops variable debuginfo.
• CARGO_INCREMENTAL=0 — CI builds each crate version once.
• No linker flag. Stable rustc 1.95 on x86_64-unknown-linux-gnu already links with its bundled rust-lld, so -fuse-ld=lld would switch to a different linker for no gain. Nightly-only options (-Zthreads, cranelift) are skipped.
• --timings on clippy and nextest, uploaded as the cargo-timings artifact.

Timings

Same runner class (self-hosted rust-heavy, 30 vCPU) for both.

	before, run 38041043937 (PR #13, per-crate, full debuginfo, ~160 invocations)	after, run 38048461775 (workflow_dispatch, codex-core,codex-tui,codex-app-server)
cargo check	2m 34s	1m 41s
cargo fmt	39s	10s
cargo clippy	55m 34s	3m 49s
cargo test	23m 11s	35m 21s (12,661 tests, all passed)
job	82m 19s	41m 20s

What each change is worth

• One invocation (change 1) is the clippy win. 55m 34s → 3m 49s. The old step paid feature resolution and a shared-dependency rebuild once per crate, serially, which is why a 30-vCPU box sat at 3–13% CPU. One cargo clippy -p a -p b -p c shares that work. fmt drops for the same reason (39s → 10s).
• Debuginfo and incremental (change 3) show up in cargo check, which was already a single invocation before this PR: 2m 34s → 1m 41s, about a third off, with no other change to that step. The same settings apply to clippy and the test build, so part of the clippy drop belongs here too, but it cannot be separated from change 1 in this run.
• Tests got slower, and that is change 1 as well. 23m 11s → 35m 21s. nextest was already saturating the cores within a crate; the per-crate loop's only extra cost was process startup, and running three big crates in one nextest process cuts the parallelism nextest had across crate boundaries. The build-settings saving is smaller than that loss. Worth it only because clippy dominates the job: 82m 19s → 41m 20s overall.
• The lockfile selection (change 2) has no runtime in these numbers. Neither run changed Cargo.lock; it saves a future lock-only PR from rebuilding the whole workspace. The pull_request run on this PR (38047945544) selected no packages and finished in 7m 23s, almost all of it the unconditional cargo check.

Head: c86cbfb2e7c4ebfe11691a77b4b8abcce3f12015.

[thedancingdeveloper]

Blockers (head c86cbfb)

Epic lead. Checked as asked:

• Test selection: unchanged. One cargo nextest run with the same -p set, and --no-fail-fast is kept.
• Known-failures filterset: kept. Single disjunction (lines 326–330).
• cargo_bin workspace build: kept. cargo build --workspace --bins --exclude codex-voice-host (line 315).
• fmt: one invocation, fine.

Two blockers, both of which fail open.

1. The Cargo.lock resolve diff selects nothing for a real lock-only dependency change. The member-manifest reverse-closure is empty too.
   • What the code reads: fork-lock-affected.py (reachable) and fork-changed-packages.py (reverse_closure) both walk packages[].dependencies[] via dep.get("pkg") or dep.get("id").
   • What real cargo metadata contains: packages[].dependencies[] is the declared requirement list, with keys features, kind, name, optional, path, registry, rename, req, source, target, uses_default_features and no pkg or id. I verified this with cargo metadata --format-version 1 on a two-crate workspace. Resolved edges exist only in resolve.nodes[].deps[].pkg (with dep_kinds).
   • So in CI every member's "reachable set" is just its own id. A lock-only bump, e.g. serde 1.0.x → 1.0.y, leaves every set equal and selects zero packages, so clippy and tests are skipped.
   • The tests pass only because their fixtures put {"pkg": …} into packages[].dependencies, a shape cargo never emits.
   • Fixes:
     • (a) In fork-lock-affected.py, build the graph from resolve.nodes[].deps[].pkg. Keep dev and build kinds; dep_kinds tells you which.
     • (b) In reverse_closure: the selection step runs cargo metadata --no-deps, which has resolve: null. So match declared dependencies[] entries that have a path (or whose name is a workspace member) to members by name.
     • (c) Make the fixtures real. Generate them from cargo metadata on a tiny scratch workspace, or copy the real JSON shape exactly.
     • (d) Add a fail-safe: if codex-rs/Cargo.lock changed and the lock diff selects nothing, select every member (or fail loudly). A lock change that "affects nothing" is far more likely a bug than a fact.
2. The clippy JSON split drops every diagnostic from six crates into a file the gate never reads.
   • What attribute() does: it matches #<crate>@ in package_id, or a leading name token. Otherwise the line goes to /tmp/clippy-unattributed.json, and the gate loop only reads /tmp/clippy-${crate}.json.
   • The gap: cargo's package-id spec omits the name when it equals the directory's last segment. In my scratch run the member a in dir a had id path+file:///…/a#0.1.0, and clippy messages use the same spec. Here that's codex-api, codex-backend-openapi-models, codex-client, codex-experimental-api-macros, codex-home and codex-mcp: every lint in those crates passes ungated. Add a config-gated Claude Code global-instructions provider #12/Fork features: ci-lines, skills roots, instructions dir/global, fail-closed, output schema #22 change codex-home.
   • Fix: don't split for gating. The gate is by changed file path, so run the jq gate once over the whole /tmp/clippy.json (keep the split only for the report if you want it). At minimum, gate the unattributed lines too.
   • Test: a lint in a changed file of codex-home must fail the step.

Non-blocking:

• The timings table shows tests 23m → 35m. That's acceptable given the clippy win, as the PR body says.
• The --timings artifact upload is fine.

[thedancingdeveloper]

Fixed both blockers at e7817b0.

Blocker 1 — the lock diff and reverse closure selected nothing.

• fork-lock-affected.py now builds its graph from resolve.nodes[].deps[].pkg, keeping normal, dev and build edges by dep_kinds (cargo's normal kind is null).
• reverse_closure no longer looks for pkg/id on declared dependencies. The selection step runs cargo metadata --no-deps, where resolve is null, so it matches declared dependencies[] entries that have a path or whose name is a workspace member.
• The hand-written fixtures are replaced with real cargo metadata --format-version 1 from a three-crate scratch workspace: testdata/nodeps.json (--no-deps) and testdata/lock-base.json / lock-head.json (full resolves differing only by a pinned serde 1.0.200 → 1.0.210).
• Fail-safe: if codex-rs/Cargo.lock changed and the resolve diff selects nothing, the step logs a warning and selects every workspace member instead of skipping the checks.

Blocker 2 — the clippy gate dropped six crates.
The per-crate split is gone. The gate runs once over the whole /tmp/clippy.json stream, matching changed file paths, so a lint is gated whether or not its package_id contains #<name>@. That covers codex-home, codex-api, codex-backend-openapi-models, codex-client, codex-experimental-api-macros and codex-mcp.

Tests: a lock-only serde bump selects codex-core and its reverse dependent codex-cli; a codex-protocol manifest change selects the member plus both reverse dependents; identical resolves select nothing (the empty-selection fail-safe lives in the workflow); a codex-home lint whose package id has no #codex-home@ fails the gate filter. python3 .github/scripts/test_fork_changed_packages.py — 7 tests, OK.

[thedancingdeveloper]

no blockers (head e7817b0)

Epic lead. Both blockers from c86cbfb are resolved.

1. Lock selection.
   • fork-lock-affected.py builds edges from resolve.nodes[].deps[].pkg, keeping normal, dev and build kinds through dep_kinds.
   • reverse_closure matches declared deps by path, or by a name that is a workspace member. That works on the --no-deps metadata the selection step uses, where resolve is null.
   • A Cargo.lock change with an empty diff now selects every member, with a ::warning::, so it no longer fails open.
   • The fixtures in .github/scripts/testdata/ have cargo's real shape: resolve.nodes, declared deps without pkg, and resolve: null for --no-deps.
2. Clippy gate. It runs once over the whole /tmp/clippy.json, and the per-crate split is gone. Crates whose package_id omits the name (codex-home, codex-api, codex-mcp, codex-client, codex-backend-openapi-models, codex-experimental-api-macros) are gated again.

Verified locally: python3 test_fork_changed_packages.py runs 7 tests and all pass, including test_nameless_package_id_in_a_changed_file_fails_the_gate and test_lock_only_change_selects_the_affected_subset. fork-ci run 38056461664 is green on this head.

Still as reviewed before: test selection is unchanged (one nextest run over the same -p set, --no-fail-fast), and the known-failures filterset and the cargo build --workspace --bins step are kept.

Non-blocking, for a follow-up: reachable() memoizes inside a DFS that skips nodes already on the stack. In a dev-dependency cycle (a crate dev-depends on a test-support crate that depends back on it), the inner member's set is cached partial and can miss a change reached only through the cycle. Memoize only completed roots, or compute strongly connected components first. The fail-safe doesn't cover this case, because something else is usually selected.