Skip to content

ci: let domain-unit restore the rust-cache it was forbidden from matching - #857

Merged
jason931225 merged 1 commit into
mainfrom
ci/domain-unit-cache-key
Aug 21, 2026
Merged

ci: let domain-unit restore the rust-cache it was forbidden from matching#857
jason931225 merged 1 commit into
mainfrom
ci/domain-unit-cache-key

Conversation

@jason931225

Copy link
Copy Markdown
Collaborator

"Domain crates — unit tests" is CI's critical path at 12m — one step worth 714s. It was never a slow test suite. On merge_group run 32434156418 that step is 684.8s of rustc (1434 Compiling, 0 Fresh) and ~29s of actual test execution: the 1156-test lib sweep in 1.1s, 18 doctests in 14.3s, everything else sub-second.

It was a cold build on every run, masquerading as a slow job.

The cause

rust-cache logged No cache found. on all 14 recent main runs. The key it asked for was …-b587c171-; the writer and all five PostgreSQL shards restore …-f3304cdf- with an identical lockfile half. The env half differed by exactly two variables — CARGO_PROFILE_DEV_DEBUG=0 and CARGO_PROFILE_TEST_DEBUG=0 — which rust-cache folds into its hash, which 03e7292 added to the shards on 2026-08-18 to fix this same miss there, and which check-ci-preflight.mjs forbade on this job:

if (/^    (?:env|defaults):/m.test(domainUnit) ...)
  failures.push("domain-unit must use the default shell with no job or step env/defaults overrides");

A blanket "no env" ban is what kept the miss in place. The contract was protecting the shell; the side effect was a 685s build per run.

The fix

Two lines of YAML and a narrower ban. The job gains the same env block the shards carry, with the same rationale. The contract no longer forbids job env — it requires it to be exactly those two variables, and still forbids step-level env and any defaults:. Failure text unchanged, so existing tests keep matching.

I went in believing the lever was nextest

Measured locally on a warm build, the lib sweep is 57.4s under cargo vs 2.08s under nextest — a 27× ratio that turned out to be a macOS process-spawn artifact applied to what is, on the runner, about one second of execution. Honest nextest gain here: 10–20s. It would also have opened .config/nextest.toml as a second, unpinned selection surface for 141 binaries — a profile default-filter narrows what runs while the verbatim-pinned ci.yml text is unchanged and check-executed-tests keeps reporting everything executed. An adversarial review found both. No nextest, no new run step, no change to the pinned cargo invocations, ratchets unchanged at 130/390.

Proven

Preflight exit 0 · contract suite 61/61 · gate sweep 13/13 · executed-tests 0.

The env requirement can fail: dropping CARGO_PROFILE_DEV_DEBUG, appending RUSTC_WRAPPER, or re-adding defaults: each turns preflight red with the pinned text; restored, exit 0. The envelope digest was recomputed with the preflight's own serializer, not copied.

Predicted, not measured

A warm restore should take this step from ~714s to near the ~30s of real execution plus restore time. That is the first run's job to confirm. If it holds, the 12m critical path drops out entirely and wall clock is set by buck-app (11m) and the platform shard (9m).

🤖 Generated with Claude Code

…hing

"Domain crates — unit tests" is the critical path of CI at 12m, one step
worth 714s. It was never a slow test suite. On merge_group run 32434156418
that step is 684.8s of rustc -- 1434 `Compiling` lines, 0 `Fresh` -- and
about 29s of actual test execution: the 1156-test lib sweep in 1.1s, the 18
doctests in 14.3s, one 8.7s gate suite, everything else sub-second.

It was a cold build on every run, masquerading as a slow job. rust-cache
logged "No cache found." on all 14 recent main runs (577-749s each). The
key it asked for was ...-b587c171-; the writer and all five PostgreSQL
shards restore ...-f3304cdf- with an identical lockfile half. The env half
differed by exactly two variables -- CARGO_PROFILE_DEV_DEBUG=0 and
CARGO_PROFILE_TEST_DEBUG=0 -- which rust-cache folds into its hash, which
03e7292 added to the shards on 2026-08-18 to fix THIS SAME MISS there,
and which scripts/check-ci-preflight.mjs FORBADE on this job:

    if (/^    (?:env|defaults):/m.test(domainUnit) ...)
      failures.push("domain-unit must use the default shell with no job
                     or step env/defaults overrides");

A blanket "no env" ban is what kept the cache miss in place. The contract
was protecting the shell, and the side effect was a 685s build per run.
Even on a key hit, a target dir built WITH debuginfo cannot be reused by a
build that wants it off, so matching the writer's profile is also what
makes a restored cache usable at all.

THE FIX IS TWO LINES OF YAML AND A NARROWER BAN. The job gains the same
env block the shards carry, with the same MUST-match-the-writer rationale.
The contract no longer forbids job env; it REQUIRES job env to be exactly
those two variables, and still forbids step-level env and any `defaults:`.
The failure text is unchanged, so the existing tests keep matching.

I went in believing the lever was nextest. Measured locally on a warm
build, the lib sweep is 57.4s under cargo and 2.08s under nextest -- a
27x ratio that turned out to be a macOS process-spawn artifact applied to
what is, on the runner, about one second of execution. The honest nextest
gain here is 10-20s, and it would have opened .config/nextest.toml as a
second, unpinned selection surface for 141 binaries (a profile
default-filter narrows what runs while the verbatim-pinned ci.yml text is
unchanged and check-executed-tests keeps reporting everything executed).
An adversarial review found both. No nextest, no new run step, no change
to the pinned cargo invocations, ratchets unchanged at 130/390.

Proven: preflight exit 0; contract suite 61/61; gate sweep 13/13;
check-executed-tests 0. The env requirement can FAIL -- dropping
CARGO_PROFILE_DEV_DEBUG, appending RUSTC_WRAPPER to the block, or
re-adding a `defaults:` each turns the preflight red with the pinned text;
restored, exit 0. The envelope digest was recomputed with the preflight's
own serializer, not copied.

PREDICTED, NOT MEASURED: a warm restore takes this step from ~714s to
roughly the shards' warm numbers -- the ~1434 crates become `Fresh` and the
step should land near the 30s of real execution plus restore time. That
is the first run's job to confirm. If it holds, the 12m critical path
drops out entirely and CI wall clock is set by buck-app (11m) and the
platform shard (9m).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jason931225

Copy link
Copy Markdown
Collaborator Author

Anvil Canonical Code Review & Adversarial Quality Report

PR: #857ci: let domain-unit restore the rust-cache it was forbidden from matching
Repository: oyatie/console
Verdict: APPROVE


Executive Summary

PR #857 identifies and resolves the root cause of CI critical-path latency in domain-unit (which spent ~685s of a 714s job on cold rustc compilation).

  1. Root Cause Confirmation: Swatinem/rust-cache incorporates all CARGO_* environment variables into its cache key generation. The cache writer (backend) and all five PostgreSQL shards set CARGO_PROFILE_DEV_DEBUG: "0" and CARGO_PROFILE_TEST_DEBUG: "0" (cache key hash ...-f3304cdf-), while domain-unit set neither (cache key hash ...-b587c171-).
  2. Chesterton's Fence & Policy Alignment: The previous preflight script (scripts/check-ci-preflight.mjs) had an overly broad regex forbidding any job-level env:, originally intended to prevent environment pollution and unvetted overrides. This change narrows the contract: it strictly mandates the exact two cache-matching variables while preserving bans on step-level env and defaults:.
  3. Pragmatism Over Speculation: The author investigated cargo-nextest and identified that apparent local speedups were macOS process spawning artifacts that offered minimal runner benefit (~10–20s) while introducing unpinned test filtering risks via .config/nextest.toml. Rejecting nextest in favor of cache parity is an exemplary application of Essentialism and Cartesian Doubt.

Canonical 16-Lens Adversarial Review Matrix

Lens Evaluation Status
1. Cartesian Doubt Verified that slow test execution was a symptom, not root cause. Profiling confirmed 684.8s rustc build vs 29s real test execution. PASS
2. Essentialism / YAGNI Minimal 2-line workflow addition + exact regex/SHA256 preflight pin. Avoided speculative test runners or restructuring. PASS
3. Chesterton's Fence Explains why the blanket env ban existed (shell hygiene) and refines the invariant to permit only identical cache keys. PASS
4. Contrarian / 10x Solves a 12-minute bottleneck at near-zero maintenance complexity without architectural churn. PASS
5. Socratic Contracts strictly assert exact equality (JSON.stringify(domainEnvLines) === JSON.stringify(expectedDomainEnv)). PASS
6. Pragmatism Restores parity with existing PostgreSQL shards (postgres-reachability-*) established on 2026-08-18. PASS
7. Red Team Job-level env variables are strictly pinned to constant boolean flags ("0"), eliminating injection or untrusted override surfaces. PASS
8. Systems Thinking Traced cross-job cache key computation dependencies across GHA workflow boundaries. PASS
9. Operability / Day-2 Preserves existing failure messages and diagnostics in preflight checks; zero operational drift. PASS
10. Opportunity Cost Negligible maintenance burden; unblocks CI throughput across all active branches and merge queues. PASS
11. Blast-radius / Cell-based Isolated to domain-unit job configuration and preflight validation checks. PASS
12. Constant-work / Anti-fragility Eliminates ~11 minutes of redundant CPU compilation on CI runners per workflow run. PASS
13. Shared-nothing Read-only cache restore against writer cache artifacts; idempotent across parallel and replay executions. PASS
14. FinOps / Unit-cost Dramatic reduction in GitHub Actions runner-minutes (~11 runner-minutes saved per merge group / PR run). PASS
15. Telemetry-first Backed by timing metrics (1434 Compiling, 0 Fresh -> target ~30s execution). PASS
16. Zero-trust / Defense-in-depth Enforces triple-lock integrity: AST metadata, SHA-256 job hash, and regex token checking in preflight. PASS

Critical Risks & Findings

  • Cache Key Stability: The preflight contract enforces exact line-by-line equality of CARGO_PROFILE_DEV_DEBUG: "0" and CARGO_PROFILE_TEST_DEBUG: "0". Any deviation, extra variable, or step-level override immediately fails preflight with a clear assertion message.
  • No Security Regressions: No execution metadata or unpinned execution vectors are introduced.

Decision: APPROVE without reservations.

Findings not addressable in the diff

  • .github/workflows/ci.yml:263 [RIGHT] -- diff unavailable or unparseable: Explicitly declaring CARGO_PROFILE_DEV_DEBUG: "0" and CARGO_PROFILE_TEST_DEBUG: "0" aligns domain-unit's environment hash with the backend cache writer and existing Postgres shards, resolving the persistent No cache found. cache key divergence.
  • scripts/check-ci-preflight.mjs:2294 [RIGHT] -- diff unavailable or unparseable: The regex extraction and expectedDomainEnv array equality check strictly enforce that only the two required CARGO_* environment variables are permitted on domain-unit, maintaining defense-in-depth while preventing environment creep or step-level overrides.

🤖 [Reviewed] by Oyatie Anvil

@jason931225

Copy link
Copy Markdown
Collaborator Author

❌ Blocked — 13 finding(s) across 72 gates; 10 gate(s) produced no measurement.

  • clean-arch — not measured: no core/ports/adapters/facade layering found: 0 of 3 file(s) examined belong to a recognised layer
  • coverage — not measured: the synchronous entry point cannot run a coverage tool; call CoverageGuard::measure_diff_coverage
    • fix: add tests covering the lines this PR adds
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • slo — not measured: no Prometheus or OpenTelemetry endpoint is configured, so error budget consumption over any window was never queried
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • cluster-audit — not measured: no Kubernetes API or ArgoCD cluster access is configured, so no live state was read back and no comparison against Git was performed
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • ci-wallclock — not measured: no GitHub Actions workflow-run timing API access is configured, so neither this PR's CI duration nor its billable compute was read
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • remote-cache — not measured: no sccache or Buck2 CAS statistics endpoint is configured, so no cache hit rate was read and no lockfile was hashed
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • shadow-traffic — not measured: no traffic mirror and no replay target are configured, so no production requests were sampled and no responses were compared
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • brand-absence — failed: 12 name(s) or PR-visible string(s) stamp an aspiration instead of naming what the code verifies
  • shape — warning: no shape spec adopted (.anvil/shape.json absent); see anvil shape validate-spec
    • fix: run anvil shape plan --repo-dir <clone> for the move plan; a regression on a blocking rule needs an entry in .anvil/baselines/shape.signoff.json
    • note: this gate is partial fidelity and does not fully measure what its name implies
  • automated-canary — not measured: no canary deployment and no Prometheus or OpenTelemetry metrics endpoint are configured, so no baseline or canary latency samples were ever read
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • stacked-diffs — not measured: no pull request DAG was read from the forge, so this PR's parent branch and any children stacked on it are unknown and no stack was evaluated
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • microbench — not measured: no criterion benchmark harness or published baseline exists for this repository, so neither the base nor the head ns/op figure was ever measured
    • note: this gate is aspirational fidelity and does not fully measure what its name implies
  • schema-evolution — failed: Detected 1 breaking wire schema changes or tag renumberings.

🤖 [Blocked] by Oyatie Anvil

@jason931225
jason931225 added this pull request to the merge queue Aug 21, 2026
Merged via the queue into main with commit d6d1c73 Aug 21, 2026
34 checks passed
@jason931225
jason931225 deleted the ci/domain-unit-cache-key branch August 21, 2026 02:37
@jason931225

Copy link
Copy Markdown
Collaborator Author

Measured outcome — closing the "predicted, not measured" section above.

Prediction was ~714s → "near the 30s of real execution plus restore time".

Measured on this PR's run (32437098619, job 96640754902):

  • The cache-key fix works as diagnosed. Cache restored successfully, Restored from cache key "v0-rust-backend-cargo-Linux-x64-f3304cdf-…" — the writer's exact key, where every prior run logged No cache found.
  • Step: 714s → 356s. Job: 11–12m → 6m29s.
  • But 628 Compiling lines remain (down from 1434), so the prediction of ~30s was wrong by an order of magnitude.

Why: the restored cache is built by the writer's clippy --all-targets, which produces check metadata (.rmeta), not linked test binaries. cargo test still has to codegen roughly half the graph. Matching the key was necessary and sufficient to halve the step; it was never going to make the artifacts reusable end-to-end.

That gives a further lever, if this job ever matters again: a writer whose cache also carries built test artifacts would cut most of the remaining 356s.

Wall clock is 14m (from ~19m). This job is off the critical path; buck-app (11–12m) and the platform shard now bind. Note for whoever looks next: the platform shard is noisy at 8m / 9m / 12m / 15m across runs — measure several before calling any single number a regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant