Skip to content

Verify cache seeds without gating deploy smoke jobs - #435

Open
prk-Jr wants to merge 8 commits into
mainfrom
fix/verify-cache-seeds
Open

prk-Jr wants to merge 8 commits into
mainfrom
fix/verify-cache-seeds

Conversation

@prk-Jr

@prk-Jr prk-Jr commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Verify that the cache-probe seed actually publishes its compiler, source and target caches before downstream probes run. Transient cache-service failures are retried within bounded limits; compilation errors and unreadable sccache stats fail immediately.
  • A compiler write is provisional. The seed accepts it only after a clean rebuild reads it back as an error-free Rust hit with zero misses. The pinned sccache backend can count a write whose upload finalization failed.
  • Require exact source/target-cache lookup hits even when the cache action returns early without applying fail-on-cache-miss.
  • Keep the action's public save-target: true path under hosted test, and keep the deployment release/smoke jobs independent of cache-probe availability. This extracts the unrelated hardening from Add pre-dispatch hooks and explicit request ingress fidelity #403 without its deploy-job coupling.

Changes

Crate / File Change
setup-rust-build-cache/tests/seed-compiler-cache.sh Per target: build fresh, accept an existing full hit, otherwise read back a new error-free write with a second clean build. Retry up to three attempts with 60s/120s backoff. Fail immediately on unreadable stats. Label log lines publish / read-back. Canonicalize the target root, validate the host path component, and reject symlinked target namespaces/debug directories before cleanup.
setup-rust-build-cache/tests/compiler-stats.sh Shared compiler_hit (Rust hits > 0, misses == 0, no cache errors) and compiler_wrote predicates, used by both the seed and assert-compiler-reuse.sh.
setup-rust-build-cache/tests/seed.sh Cover finalization failures (transient/permanent, native/WASM, last attempt), partial hits, unreadable/empty/error stats, compile and read-back failures, independent retry limits, symlink/traversal cleanup boundaries with outside sentinels, and the reuse probe's rejection of partial hits.
setup-rust-build-cache/tests/workflow.sh Workflow contracts moved out of fixture.sh: seed wiring, save-target: true, lookup output wiring, all 32 hit-output combinations, action paths/keys and retry conditions, independent release jobs.
setup-rust-build-cache/tests/{fixture,publication,review,run}.sh fixture.sh tests only the fixture. publication.sh applies 39 workflow mutations to workflow.sh alone. review.sh checks the writer step's save-target condition and target-cache-hit wiring.
.github/workflows/deploy-action.yml The seed keeps save-target: true, explicitly saves and lookup-verifies source and target seeds first, retries each once and enforces an exact lookup-hit assertion. The seed job has timeout-minutes: 20. Exhaustion fails only the cache-probe chain.

Closes

Closes #433

Test plan

Verified locally on 8e5e8522 with Rust 1.98.1:

  • cargo test --workspace --all-targets (1550 passed, 0 failed)
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo fmt --all -- --check
  • cargo check --workspace --all-targets --features "fastly cloudflare spin"
  • cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin
  • bash .github/actions/setup-rust-build-cache/tests/run.sh: action, revision, review, fixture, workflow, 39 workflow mutations and seed regressions pass.
  • ShellCheck (CI invocation), actionlint, repository action-pin gate, git diff --check.
  • Each new check was mutation-tested: removing the zero-miss clause, the stats validation, the read-back label, the target-cache-hit wiring or the writer's save-target condition makes the matching regression fail.
  • All 29 hosted checks pass on 8e5e8522, including the cache seed and probe jobs. The seed log shows a publish write followed by a read-back Rust hit for each target. It also shows the action's post-job save running and logging the expected duplicate-key warning.

Worst-case publication backoff is 60s + 120s per compiler target and 60s each for source and target archives (about 8 minutes). It affects only the cache-probe chain, which timeout-minutes: 20 now bounds as a whole.

Review decisions

  • Compiler publication uses a read-back hit, not write counters. sccache v0.16.0 uses OpenDAL 0.55.0, whose GHAC-v2 writer discards FinalizeCacheEntryUpload errors. A write can be counted while nothing readable is published. The seed therefore rebuilds from a clean target and requires a Rust hit. sccache awaits cache writes before replying to the compile (src/server.rs), so stats read right after cargo build are complete. An existing full hit proves publication without a second build.
  • A hit must have zero Rust misses. With more than one compilation unit, earlier units could hit while the newly written unit misses. The seed and the downstream reuse probe share one predicate, so they cannot drift apart.
  • The seed keeps save-target: true. It is the only hosted job that runs the action's cargo-target-save step (actions/cache@v5 with post-job save) and its target-cache-hit wiring. Explicit save and lookup steps still publish and verify first, so consumers never depend on the warning-only post-job hook. That hook then logs a harmless duplicate-key warning, as the source cache already does. The hook's own publication is not separately verified.
  • actions/cache v5 can return early. Restore can succeed with cache-hit=false before parsing fail-on-cache-miss when feature availability fails. The final shell gate therefore requires true from one of the actual lookup outputs.
  • Source archives are saved explicitly. Previously they were saved only in the warning-only post-job hook. A failed publication on a cold key could leave the seed green while the source-restore consumer failed.
  • Cleanup is confined. Validating only the cleanup root allowed an intermediate target symlink to redirect rm -rf. Cleanup now rejects namespace/debug symlinks and unsafe host components.

Local tests simulate publication failures without relying on live outages. Hosted happy-path checks do not prove recovery during an actual cache-service outage.

Checklist

  • Follows CLAUDE.md; changes limited to cache CI/tooling.
  • Behavior and failure handling have committed regression coverage.
  • No runtime dependencies, routing, store wiring or application behavior changes.
  • Bash/YAML only; no Python added.
  • No credentials, secrets or bylines added.

@prk-Jr prk-Jr self-assigned this Oct 22, 2026
@aram356 aram356 added this to the 202610 milestone Oct 22, 2026

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

😃 Approving the cache-seed hardening while keeping deployment smoke jobs independent of cache probes.

🤔 One non-blocking concern remains: compiler write counters can report success despite failed remote finalization. A bounded remote read-back hit and finalization-failure coverage would close that publication gap; details are inline.

@dhruv8sh dhruv8sh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review

Summary

This PR checks that the cache-probe seed actually publishes its compiler, source and target caches, with bounded retries, while the deploy and release jobs stay independent of it. The change is well-scoped and thoroughly covered by mutation tests. Approving.

😃 Praise

  • 😃 The final check doesn't trust fail-on-cache-miss alone: the "Require exact … seed publication" steps demand a literal true from one of the lookup outputs, and all 16 input combinations are exercised (fixture.sh:85-97). This covers actions/cache v5 returning early without applying fail-on-cache-miss.
  • Inline: workflow mutation guards (publication.sh) and confined directory cleanup (seed-compiler-cache.sh).

Findings

Blocking

None.

Non-blocking

  • 🤔 A cache-outage run can add up to about 8 minutes: retry waits are 60s then 120s per target (360s for native plus WASM), and the source and target saves can each add another 60s. That's acceptable because only the cache-probe jobs are affected, not the deploy jobs. Just confirming the bound is intended (seed-compiler-cache.sh:40).
  • 📝 Release jobs were already independent on main: fixture-release and store-free-release already had no needs. The new assertion in fixture.sh:44-47 locks that in as a regression guard.
  • 📝 A duplicate source-cache save is harmless: when there is no exact source-cache hit (including a partial match through restore-keys), the action's post-job hook tries to save the same key again and the cache rejects it with a warning only. The workflow comment already explains this.
  • Inline: ♻️ the source cache paths are copied four times; ⛏ the deploy workflow runs a script from a tests/ directory.

CI Status

  • fmt: PASS
  • clippy: PASS
  • tests: PASS
  • tests/seed.sh: PASS (fixture.sh and publication.sh were not run locally because yq isn't installed; the hosted checks pass)

Comment thread .github/workflows/deploy-action.yml Resolved
Comment thread .github/workflows/deploy-action.yml Resolved
@aram356 aram356 modified the milestones: 202610, 202611 Oct 29, 2026

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review

Summary

This verifies that the cache-probe seed actually publishes its compiler, source and target caches, with bounded retries, while the release and deploy jobs stay independent of it. The compiler read-back in 265076783 is sound. In sccache v0.16.0, src/server.rs:1520-1537 awaits the cache write before it sends the compile response, so stats read right after cargo build are complete. The hosted seed log shows one write and then one Rust hit on the clean rebuild for each target. One question needs an answer before merge: this change leaves no hosted job running the action's save-target: true path.

Findings

Blocking

  • ❓ No hosted job runs save-target: true any more: the seed was the only job using the action's save path, and the adoption guide still recommends it (.github/workflows/deploy-action.yml:141)

Non-blocking

  • 🤔 compiler_hit accepts a partial hit: it checks hits > 0 but not misses == 0. A dependency added to the fixture would silently skip the read-back, and assert-compiler-reuse.sh would pass without proving anything (seed-compiler-cache.sh:34)
  • 🤔 Unreadable stats are retried as if the cache service were down: a jq parse failure spends 180s on retries, then reports a publication failure (seed-compiler-cache.sh:29)
  • ♻️ fixture.sh is now mostly a workflow test: split out the workflow checks so the mutation suite targets only them (publication.sh:14)
  • ⛏ The read-back build is logged under the same label as the publication build (seed-compiler-cache.sh:20)
  • ⛏ The PR description is out of date: it was written before 265076783. It still says "All 28 hosted checks pass on 3373552e". The Changes table describes seed-compiler-cache.sh without the read-back. "Review decisions" presents the write-counter approach as the final compiler strategy. The description becomes the squash commit message, so please update it to describe the read-back and the current check run.
  • 🌱 cache-restore-seed has no timeout-minutes: the retries are bounded, but a hung cargo build, sccache request or cache call still runs to GitHub's 6-hour default. Something like timeout-minutes: 20 on the seed job would make "bounded" hold for the whole job. No other job in this workflow sets one, so this is optional.

📌 Out of Scope

  • None of the deploy-action.yml jobs are required status checks on main. The ruleset lists only the cargo, wasm and docs checks, so static-checks (which runs this cache suite) and the cache probe jobs block merges only by convention. That's a repository-settings decision, separate from this PR.

CI Status

  • fmt: PASS
  • clippy: PASS
  • tests: PASS (1550 passed, 0 failed)
  • setup-rust-build-cache/tests/run.sh (includes 38 workflow mutations and the seed regressions): PASS
  • ShellCheck, actionlint, action-pin gate, git diff --check: PASS
  • Hosted checks on 265076783: all passing

Comment thread .github/workflows/deploy-action.yml Resolved
Restore save-target: true on the cache seed so a hosted job still runs
the action's post-job save path and target-cache-hit wiring; the explicit
save and lookup verification still publish first. Require zero Rust
misses for a compiler hit in both the seed and the reuse probe via a
shared predicate, fail immediately on unreadable sccache stats, label
read-back builds, bound the seed job at 20 minutes, and move workflow
contracts from fixture.sh into workflow.sh so mutations target only them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@prk-Jr

prk-Jr commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@aram356 thanks for the review. The two summary-level items are handled in 8e5e852:

  • PR description: rewritten for the read-back approach, the current file layout and the current check run (all 29 checks pass on 8e5e8522).
  • timeout-minutes: the cache-restore-seed job now sets timeout-minutes: 20. The worst-case retry backoff is about 8 minutes, so a hung build or cache call can't run to the 6-hour default.

Each inline item has a reply in its own thread.

@prk-Jr
prk-Jr requested a review from aram356 November 4, 2026 13:54

This branch has not been deployed

No deployments
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.

Verify cache seed publication without coupling deploy smoke jobs

4 participants