CI: one SWORD module cache that main keeps; fetch gives up on a stall #133

Merged
claude-bot merged 2 commits from ci/module-cache into main 2026-10-30 13:20:18 +00:00
Collaborator

What

CI keeps one cache entry with all the SWORD modules (key sword-modules-all-<hash of tools/golden/fetch.sh>). The rust job saves it and the app and e2e jobs only read it, so CrossWire is asked again only when fetch.sh changes or the entry expires. A weekly modules.yml on main saves the entry again if it's missing. fetch.sh skips zips it already has, and a stalled download now gives up and retries instead of taking 15 minutes. The rust job and modules.yml install zstd, because actions/cache only finds an entry compressed the same way.

Blast radius

Tier: trunk
Trunk paths touched: .forgejo/workflows/ci.yml, .forgejo/workflows/modules.yml
Testing tier: maximum (CI) (also: ship on sight: docs/09; standard: tools/golden/fetch.sh)
Gate / rollback: revert this merge. Each job then fetches its own modules again, as before.

Proof

  • fetch.sh, locally: a fresh KJV fetch takes 6 s, and a second run reuses the zip. NoSuchModuleX returns 404 four times (three retries), exits 22 and leaves no .part file.
  • The defect the targeted check caught, on 9b51e63 (run 254): rust saved the entry with gzip (cache.tgz), and app and e2e, which have zstd (cache.tzst), missed it in the same run (Cache not found for input keys: sword-modules-all-8d5d…). The zstd commit c87abea fixes this.
  • CI run 256 on c87abea is green. In the rust job, Cache restored from key: sword-modules-all-8d5d…, the fetch step only unzips and Save SWORD modules is skipped. app and e2e also log Cache restored from key: sword-modules-all-8d5d…. cargo-registry and sccache missed once because of the zstd change and were saved again.
  • modules.yml, run on the branch: run 257 missed, fetched and saved (Cache saved with key: sword-modules-all-8d5d…). Run 258 logged Lookup only - skipping download and Cache found and can be restored from key: …, and skipped the save.
  • After the merge (to be posted here): main's rust job saves the entry in main's scope, and the next PR's jobs read it.

Review

Reviewer: fresh-context code-review at high (review 352): 3 low findings. 1 agreed and verified after the merge, 2 declined (comment 2609). The targeted check found the zstd mismatch, fixed in c87abea. Delta review at medium (353): 0 findings. 0 left open.
Review-Confidence: 85/100. Reviewed at the right depth and CI green on c87abea; the hand-off from main to later PRs is not shown yet (to be checked after the merge).

Confidence by part

  • One entry shared by rust, app, e2e and modules.yml: high. Every hit is shown in the logs above.
  • main scope and expiry: medium. A save on main is checked only after the merge, and how long the runner keeps an entry is its default setting, not something this PR sets.
  • fetch.sh stall limit and retries: high. Tested locally against a 404. A real stall wasn't reproduced.

🤖 Generated with Claude Code

## What CI keeps one cache entry with all the SWORD modules (key `sword-modules-all-<hash of tools/golden/fetch.sh>`). The rust job saves it and the app and e2e jobs only read it, so CrossWire is asked again only when `fetch.sh` changes or the entry expires. A weekly `modules.yml` on main saves the entry again if it's missing. `fetch.sh` skips zips it already has, and a stalled download now gives up and retries instead of taking 15 minutes. The rust job and `modules.yml` install zstd, because actions/cache only finds an entry compressed the same way. ## Blast radius **Tier:** trunk **Trunk paths touched:** `.forgejo/workflows/ci.yml`, `.forgejo/workflows/modules.yml` **Testing tier:** maximum (CI) (also: ship on sight: docs/09; standard: `tools/golden/fetch.sh`) **Gate / rollback:** revert this merge. Each job then fetches its own modules again, as before. ## Proof - `fetch.sh`, locally: a fresh KJV fetch takes 6 s, and a second run reuses the zip. `NoSuchModuleX` returns 404 four times (three retries), exits 22 and leaves no `.part` file. - The defect the targeted check caught, on 9b51e63 (run 254): rust saved the entry with gzip (`cache.tgz`), and app and e2e, which have zstd (`cache.tzst`), missed it in the same run (`Cache not found for input keys: sword-modules-all-8d5d…`). The zstd commit c87abea fixes this. - CI run 256 on c87abea is green. In the rust job, `Cache restored from key: sword-modules-all-8d5d…`, the fetch step only unzips and `Save SWORD modules` is skipped. app and e2e also log `Cache restored from key: sword-modules-all-8d5d…`. cargo-registry and sccache missed once because of the zstd change and were saved again. - `modules.yml`, run on the branch: run 257 missed, fetched and saved (`Cache saved with key: sword-modules-all-8d5d…`). Run 258 logged `Lookup only - skipping download` and `Cache found and can be restored from key: …`, and skipped the save. - After the merge (to be posted here): main's rust job saves the entry in main's scope, and the next PR's jobs read it. ## Review **Reviewer:** fresh-context `code-review` at high (review 352): 3 low findings. 1 agreed and verified after the merge, 2 declined (comment 2609). The targeted check found the zstd mismatch, fixed in c87abea. Delta review at medium (353): 0 findings. 0 left open. **Review-Confidence:** 85/100. Reviewed at the right depth and CI green on c87abea; the hand-off from main to later PRs is not shown yet (to be checked after the merge). ## Confidence by part - One entry shared by rust, app, e2e and modules.yml: high. Every hit is shown in the logs above. - main scope and expiry: medium. A save on main is checked only after the merge, and how long the runner keeps an entry is its default setting, not something this PR sets. - `fetch.sh` stall limit and retries: high. Tested locally against a 404. A real stall wasn't reproduced. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
ci: one SWORD module cache that main keeps, and a fetch that gives up on a stall
All checks were successful
CI / changes (pull_request) Successful in 7s
CI / rust (pull_request) Successful in 9m7s
CI / audit (pull_request) Successful in 11s
CI / app (pull_request) Successful in 3m20s
CI / e2e (pull_request) Successful in 10m23s
CI / click (pull_request) Successful in 2m7s
9b51e63d25
Each new PR fetched the e2e job's modules from CrossWire (its own cache
key, seen only by that PR; 15 min for 3 modules in run 249). Now one
entry, sword-modules-all, holds every module: the rust job saves it
right after the fetch, the app and e2e jobs only restore it, and
modules.yml looks it up weekly on main and fetches it only when missing.
fetch.sh's curl gets a connect timeout, a stall limit and three retries,
and downloads into a .part file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review left a comment

Review level: high. Commit: 9b51e63d25. Range: origin/main...HEAD.

Files reviewed: .forgejo/workflows/ci.yml, .forgejo/workflows/modules.yml, .forgejo/workflows/presets.yml, tools/golden/fetch.sh, tools/ci/main-skip.sh, docs/09-quality-gates.md, CLAUDE.md, Makefile.toml.

No high or medium findings. The restore/save split, cache-hit != 'true' guards, lookup-only, and the needs: rust -> app -> e2e ordering hold up: the rust job saves in PR scope before app/e2e restore, and each fetch.sh call with a module subset reads from the full zip set.

Low findings:

  1. (outside the diff) Targeted check gap. A workflow_dispatch of modules.yml on the branch saves under the branch's scope, not main's. It proves the miss -> fetch+save -> hit logic but not the scheduled run on main, and it doesn't prove that a later PR's rust job hits an entry saved on main. Suggest a follow-up observation after merge: main's full run (ci.yml changed, so main-skip.sh runs everything) saves the entry, and the next PR's rust job should log a hit with no CrossWire download. The planned check covers the logic; it does not cover the main-scope read.
  2. modules.yml:12 · if Forgejo's cache GC evicts entries unused for about 7 days and lookup-only does not refresh the entry's use time, the weekly cadence is no margin: the entry can be evicted just before Monday and every PR in between re-downloads from CrossWire. Check the retention setting, or run the lookup more often (cheap when it hits).
  3. modules.yml (whole job) · unlike presets.yml:56, a failed scheduled fetch (CrossWire down for the whole 3-retry window) notifies no one, so the entry stays missing until someone notices PR runs getting slow.
Review level: high. Commit: 9b51e63d259c2e5e4e6269444eca1f26b11b14a8. Range: origin/main...HEAD. Files reviewed: .forgejo/workflows/ci.yml, .forgejo/workflows/modules.yml, .forgejo/workflows/presets.yml, tools/golden/fetch.sh, tools/ci/main-skip.sh, docs/09-quality-gates.md, CLAUDE.md, Makefile.toml. No high or medium findings. The restore/save split, `cache-hit != 'true'` guards, lookup-only, and the `needs: rust -> app -> e2e` ordering hold up: the rust job saves in PR scope before app/e2e restore, and each fetch.sh call with a module subset reads from the full zip set. Low findings: 1. (outside the diff) Targeted check gap. A workflow_dispatch of modules.yml on the branch saves under the branch's scope, not main's. It proves the miss -> fetch+save -> hit logic but not the scheduled run on main, and it doesn't prove that a later PR's rust job hits an entry saved on main. Suggest a follow-up observation after merge: main's full run (ci.yml changed, so main-skip.sh runs everything) saves the entry, and the next PR's rust job should log a hit with no CrossWire download. The planned check covers the logic; it does not cover the main-scope read. 2. modules.yml:12 · if Forgejo's cache GC evicts entries unused for about 7 days and lookup-only does not refresh the entry's use time, the weekly cadence is no margin: the entry can be evicted just before Monday and every PR in between re-downloads from CrossWire. Check the retention setting, or run the lookup more often (cheap when it hits). 3. modules.yml (whole job) · unlike presets.yml:56, a failed scheduled fetch (CrossWire down for the whole 3-retry window) notifies no one, so the entry stays missing until someone notices PR runs getting slow.
@ -0,0 +9,4 @@
on:
schedule:
- cron: "0 4 * * 1" # Mondays, 04:00 UTC
Collaborator

Low: weekly equals a likely 7-day cache retention; if lookup-only doesn't refresh the entry's last-use time, it can expire just before the run and PRs in between re-download. Also no failure notice like presets.yml has for a failed scheduled run.

Low: weekly equals a likely 7-day cache retention; if lookup-only doesn't refresh the entry's last-use time, it can expire just before the run and PRs in between re-download. Also no failure notice like presets.yml has for a failed scheduled run.
Author
Collaborator

Answers to review 352's lows:

  1. Main-scope read not covered by the planned check: agreed. A PR can't show it before the merge. After the merge, I'll read main's run (the rust job should save sword-modules-all) and the next PR's rust job (it should restore it, with no CrossWire download). I'll post both on this PR.
  2. Eviction before Monday: the runner uses the default cache settings (no cache: section in its config; ~/dev/infrastructure/modules/ci-runner). It evicts stale entries when it starts. The cache server of act/forgejo-runner keeps an unused entry for about 30 days by default, as far as I know; I haven't checked it in the runner's source. Every PR's rust restore uses the entry too, so a weekly lookup leaves a wide margin. No change.
  3. No notice for a failed weekly run: declined. A failure costs only speed: the next main full run, or the next weekly run, saves the entry again, and PR runs still fetch on their own. An issue opened automatically for that would be noise.
Answers to review 352's lows: 1. **Main-scope read not covered by the planned check:** agreed. A PR can't show it before the merge. After the merge, I'll read main's run (the `rust` job should save `sword-modules-all`) and the next PR's `rust` job (it should restore it, with no CrossWire download). I'll post both on this PR. 2. **Eviction before Monday:** the runner uses the default cache settings (no `cache:` section in its config; `~/dev/infrastructure/modules/ci-runner`). It evicts stale entries when it starts. The cache server of act/forgejo-runner keeps an unused entry for about 30 days by default, as far as I know; I haven't checked it in the runner's source. Every PR's `rust` restore uses the entry too, so a weekly lookup leaves a wide margin. No change. 3. **No notice for a failed weekly run:** declined. A failure costs only speed: the next main full run, or the next weekly run, saves the entry again, and PR runs still fetch on their own. An issue opened automatically for that would be noise.
ci: compress the module entry with zstd, so the app and e2e jobs find it
All checks were successful
CI / changes (pull_request) Successful in 8s
CI / rust (pull_request) Successful in 8m39s
CI / audit (pull_request) Successful in 10s
CI / app (pull_request) Successful in 3m14s
CI / e2e (pull_request) Successful in 10m16s
CI / click (pull_request) Successful in 2m21s
c87abea2ac
actions/cache picks zstd when it is on PATH, else gzip, and an entry is
found only by a job that compresses the same way. The rust-ci and node
images have no zstd and the app image has: on 9b51e63 the rust job
saved the entry and the app and e2e jobs of the same run missed it. The
rust job and modules.yml now install zstd (with unzip, in one step).
The rust job's cargo and sccache entries miss once for the same reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review left a comment

Level: medium (delta). Reviewed commit: c87abea2ac. Range: 9b51e63d259c2e5e4e6269444eca1f26b11b14a8..HEAD.

Files reviewed: .forgejo/workflows/ci.yml, .forgejo/workflows/modules.yml, tools/golden/fetch.sh, CLAUDE.md.

Targeted check (one module cache entry found by the rust, app and e2e jobs and by modules.yml, no download on a hit): no defects found. Both the rust job and modules.yml now install zstd and unzip before any cache step, so both save and read with zstd, as the app and e2e images do. Keys and paths match across all four consumers. On a hit, fetch.sh skips the download when the zip exists and only unzips.

Not verified (no finding): that the ut-app-ci image has zstd. The comment in ci.yml says it does, and docker/ was not in this worktree to confirm. If it lacks zstd, the app and e2e jobs would miss the entry and download again. That would cost time only, not break the build.

Findings outside the diff: none.

Level: medium (delta). Reviewed commit: c87abea2acb5a14c32f9f0ae5a90a132fc413dfb. Range: 9b51e63d259c2e5e4e6269444eca1f26b11b14a8..HEAD. Files reviewed: .forgejo/workflows/ci.yml, .forgejo/workflows/modules.yml, tools/golden/fetch.sh, CLAUDE.md. Targeted check (one module cache entry found by the rust, app and e2e jobs and by modules.yml, no download on a hit): no defects found. Both the rust job and modules.yml now install zstd and unzip before any cache step, so both save and read with zstd, as the app and e2e images do. Keys and paths match across all four consumers. On a hit, fetch.sh skips the download when the zip exists and only unzips. Not verified (no finding): that the ut-app-ci image has zstd. The comment in ci.yml says it does, and docker/ was not in this worktree to confirm. If it lacks zstd, the app and e2e jobs would miss the entry and download again. That would cost time only, not break the build. Findings outside the diff: none.
claude-bot changed title from WIP: CI: one SWORD module cache that main keeps; fetch gives up on a stall to CI: one SWORD module cache that main keeps; fetch gives up on a stall 2026-10-30 13:19:20 +00:00
claude-review left a comment

Gate approval on c87abea. Reviews 352 and 353 have no high or medium findings; the 3 lows were answered in comment 2609. No commits after c87abea. CI is green on head (success). Proof lists no unrun elevated, maximum or AI-behavior gate: runs 256, 257 and 258 are shown. Open item: the main-to-PR cache hand-off is checked after the merge, as the PR body states.

Gate approval on c87abea. Reviews 352 and 353 have no high or medium findings; the 3 lows were answered in comment 2609. No commits after c87abea. CI is green on head (success). Proof lists no unrun elevated, maximum or AI-behavior gate: runs 256, 257 and 258 are shown. Open item: the main-to-PR cache hand-off is checked after the merge, as the PR body states.
Author
Collaborator

After the merge:

  • main's run 259 (23a11fd) is green. Its rust job logged Cache restored from key: sword-modules-all-8d5d…, so it skipped the download and the save. The entry the branch runs saved was visible to main, so this runner's cache is looser than GitHub's per-branch scopes.
  • The next PR's run (260, #134) also restored that entry, in both rust (13:34:11) and app (13:50:58). Its e2e was cancelled by a new push. The current run (261) covers it.

Nothing asked CrossWire again on either run.

After the merge: - main's run 259 (23a11fd) is green. Its rust job logged `Cache restored from key: sword-modules-all-8d5d…`, so it skipped the download and the save. The entry the branch runs saved was visible to main, so this runner's cache is looser than GitHub's per-branch scopes. - The next PR's run (260, #134) also restored that entry, in both rust (`13:34:11`) and app (`13:50:58`). Its e2e was cancelled by a new push. The current run (261) covers it. Nothing asked CrossWire again on either run.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
schaefera/utStudyBIble!133
No description provided.