docs(cache): producer half of the depended-upon names contract #11

Merged
claude merged 1 commits from chore/leftover-contract-names into main 2026-08-24 23:03:30 +00:00
Collaborator

Summary

  • Adds the producer-side counterpart to daniel/gitdan#32's DEPENDED-UPON NAMES CONTRACT for the two names this repo actually produces: .ci-lock-<id> (documented at scripts/cache-lock.sh's header) and .cache-last-used (documented at the exact line in cargo-cache/action.yml that writes it).
  • Adds a new README.md subsection, "Names this repo doesn't reclaim, but the arbiter depends on", covering all four names in gitdan's list and pointing at gitdan's DEPENDED-UPON NAMES CONTRACT block as the canonical description.
  • No behaviour change: comments and docs only.

Why

daniel/gitdan's host-level arbiter (scripts/ci-cache-reclaim.sh) hard-codes four dot-prefixed names it reads to make correctness decisions but never reclaims as leftovers: .ci-lock-*, .cache-last-used, .gitea-last-used, .ci-keep. None carried a matching note here, which is the same gap shape as #28/#30/#7 — a name one repo owns and the other depends on, with nothing binding them.

This repo's earlier PR #8 (daniel/gitdan-actions#7) already wrote down the producer half of the leftover contract — .stage-, .evicting-, .reading- and (via a later fix) .publish-*. This PR covers the second, distinct category the leftover contract's block doesn't: names that are consumed, never reclaimed.

A finding worth flagging: only two of the four are actually produced here

Investigating each of the four names before documenting them surfaced that two aren't produced by anything in this repo at all:

  • .gitea-last-used — gitdan's own script comment already describes this as "the older in-repo convention", i.e. a naming scheme individual repos used before adopting this shared cargo-cache action. A repo-wide grep here confirms nothing in gitdan-actions writes it today. It's read on gitdan's side purely for backward compatibility with directories created under the old scheme.
  • .ci-keep — a repo-wide grep here confirms nothing in gitdan-actions writes this either. It's a per-repo, hand-placed opt-out: a consuming repo's own workflow drops it directly into a cache directory it wants exempted from eviction, with no fixed producer script anywhere.

Both are documented in the new README subsection with a short paragraph explaining why no producer-side counterpart exists here for them — rather than inventing an owner this repo doesn't have. If either of these actually does originate somewhere in this repo and the grep missed it, that's worth a correction before merge.

Key implementation details

  • scripts/cache-lock.sh: added a header paragraph naming the cross-repo contract, both writers of the same shape (cache-lock.sh itself and cache-lib.sh's write_cache_lock()), and the consequence of an unannounced rename — a live job's staging tree loses its liveness guard and becomes an ordinary reclaim candidate while still in use.
  • cargo-cache/action.yml: added a comment at the step that touches .cache-last-used, naming the same contract and consequence — gitdan's script falling back to directory mtime, silently reordering eviction.
  • README.md: new subsection under "Scratch names in a cache root are a cross-repo contract", structured the same way as the existing leftover-names table (name / produced by / consequence of rename), plus explicit prose for the two names with no producer here. Closes with the same "adding counts as renaming" framing the leftover section already uses, extended to this category.
  • scripts/cache-lib.sh was deliberately left untouched — it's a sibling track's territory in this round (a reader-lock interlock selftest is landing there in parallel) — even though it also writes .ci-lock-<id> via write_cache_lock(). That function is referenced by name in the new documentation rather than edited.

Files affected

  • scripts/cache-lock.sh — header comment only.
  • cargo-cache/action.yml — inline comment only, at the .cache-last-used write.
  • README.md — new subsection (30 lines).

Test plan

Ran the canonical entry point directly (this repo has no .envrc/Justfile):

bash scripts/selftest.sh          # full suite, including the two compiler-backed tests

Result: all 5 suites passedseed-target-dir-selftest.sh, publish-snapshot-selftest.sh, prune-cache-selftest.sh (37 assertions), hardlink-clone-selftest.sh, and restore-mtimes-selftest.sh (14 assertions, ALL 14 ASSERTIONS PASSED). Also ran shellcheck scripts/cache-lock.sh clean. Docs-only / comment-only changes, so no new test coverage was added here — the consumer-side PR carries the new self-consistency assertions, since that's where the machine-checkable contract lives.

Ref: gitdan#32. Consumer-side counterpart: daniel/gitdan#39 (opening alongside this PR).

## Summary - Adds the producer-side counterpart to daniel/gitdan#32's `DEPENDED-UPON NAMES CONTRACT` for the two names this repo actually produces: `.ci-lock-<id>` (documented at `scripts/cache-lock.sh`'s header) and `.cache-last-used` (documented at the exact line in `cargo-cache/action.yml` that writes it). - Adds a new `README.md` subsection, "Names this repo doesn't reclaim, but the arbiter depends on", covering all four names in gitdan's list and pointing at gitdan's `DEPENDED-UPON NAMES CONTRACT` block as the canonical description. - No behaviour change: comments and docs only. ## Why daniel/gitdan's host-level arbiter (`scripts/ci-cache-reclaim.sh`) hard-codes four dot-prefixed names it reads to make correctness decisions but never reclaims as leftovers: `.ci-lock-*`, `.cache-last-used`, `.gitea-last-used`, `.ci-keep`. None carried a matching note here, which is the same gap shape as #28/#30/#7 — a name one repo owns and the other depends on, with nothing binding them. This repo's earlier PR #8 (daniel/gitdan-actions#7) already wrote down the producer half of the *leftover* contract — `.stage-`, `.evicting-`, `.reading-` and (via a later fix) `.publish-*`. This PR covers the second, distinct category the leftover contract's block doesn't: names that are consumed, never reclaimed. ## A finding worth flagging: only two of the four are actually produced here Investigating each of the four names before documenting them surfaced that two aren't produced by anything in this repo at all: - **`.gitea-last-used`** — gitdan's own script comment already describes this as "the older in-repo convention", i.e. a naming scheme individual repos used *before* adopting this shared `cargo-cache` action. A repo-wide grep here confirms nothing in `gitdan-actions` writes it today. It's read on gitdan's side purely for backward compatibility with directories created under the old scheme. - **`.ci-keep`** — a repo-wide grep here confirms nothing in `gitdan-actions` writes this either. It's a per-repo, hand-placed opt-out: a consuming repo's own workflow drops it directly into a cache directory it wants exempted from eviction, with no fixed producer script anywhere. Both are documented in the new README subsection with a short paragraph explaining why no producer-side counterpart exists here for them — rather than inventing an owner this repo doesn't have. If either of these actually does originate somewhere in this repo and the grep missed it, that's worth a correction before merge. ## Key implementation details - `scripts/cache-lock.sh`: added a header paragraph naming the cross-repo contract, both writers of the same shape (`cache-lock.sh` itself and `cache-lib.sh`'s `write_cache_lock()`), and the consequence of an unannounced rename — a live job's staging tree loses its liveness guard and becomes an ordinary reclaim candidate while still in use. - `cargo-cache/action.yml`: added a comment at the step that touches `.cache-last-used`, naming the same contract and consequence — gitdan's script falling back to directory mtime, silently reordering eviction. - `README.md`: new subsection under "Scratch names in a cache root are a cross-repo contract", structured the same way as the existing leftover-names table (name / produced by / consequence of rename), plus explicit prose for the two names with no producer here. Closes with the same "adding counts as renaming" framing the leftover section already uses, extended to this category. - `scripts/cache-lib.sh` was deliberately left untouched — it's a sibling track's territory in this round (a reader-lock interlock selftest is landing there in parallel) — even though it also writes `.ci-lock-<id>` via `write_cache_lock()`. That function is referenced by name in the new documentation rather than edited. ## Files affected - `scripts/cache-lock.sh` — header comment only. - `cargo-cache/action.yml` — inline comment only, at the `.cache-last-used` write. - `README.md` — new subsection (30 lines). ## Test plan Ran the canonical entry point directly (this repo has no `.envrc`/`Justfile`): ``` bash scripts/selftest.sh # full suite, including the two compiler-backed tests ``` Result: **all 5 suites passed** — `seed-target-dir-selftest.sh`, `publish-snapshot-selftest.sh`, `prune-cache-selftest.sh` (37 assertions), `hardlink-clone-selftest.sh`, and `restore-mtimes-selftest.sh` (14 assertions, `ALL 14 ASSERTIONS PASSED`). Also ran `shellcheck scripts/cache-lock.sh` clean. Docs-only / comment-only changes, so no new test coverage was added here — the consumer-side PR carries the new self-consistency assertions, since that's where the machine-checkable contract lives. Ref: gitdan#32. Consumer-side counterpart: daniel/gitdan#39 (opening alongside this PR).
claude added 1 commit 2026-08-24 22:11:12 +00:00
daniel/gitdan's ci-cache-reclaim.sh hard-codes four dot-prefixed names it
reads to make correctness decisions but never reclaims as leftovers —
.ci-lock-*, .cache-last-used, .gitea-last-used, .ci-keep — none of which
carried a matching note here (gitdan#32, the follow-up shape to #28/#30
that #7/PR #8 covered for the leftover names specifically).

Of the four, this repo actually produces two:

- .ci-lock-<id>, written by scripts/cache-lock.sh (acquire/release) and
  cache-lib.sh's write_cache_lock(). Documents the contract at
  cache-lock.sh's header, the site someone renaming the marker would
  most likely be editing.
- .cache-last-used, stamped every run by cargo-cache/action.yml. Documents
  the contract at the exact line that writes it.

The other two are read by gitdan's script but produced by nothing in this
repo: .gitea-last-used is a legacy naming convention individual repos
used before adopting the shared cargo-cache action (nothing here writes
it today), and .ci-keep is a per-repo, hand-placed opt-out any consuming
repo's own workflow may drop directly into a cache directory, with no
single owner. Both get a paragraph in README.md's new subsection
explaining why no producer-side counterpart exists for them, rather than
inventing an owner this repo doesn't have.

README.md's "Scratch names in a cache root are a cross-repo contract"
section gains a new subsection, "Names this repo doesn't reclaim, but
the arbiter depends on", covering all four and pointing at gitdan's
DEPENDED-UPON NAMES CONTRACT block as the canonical description.

No behaviour change: comments and docs only.

Ref: gitdan#32. Consumer-side counterpart: daniel/gitdan (this branch's
sibling PR).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJHKJFJVUjnvVsemVBnvrd
claude-reviewer approved these changes 2026-08-24 22:27:11 +00:00
Dismissed
claude-reviewer left a comment
Collaborator

VERDICT: APPROVE-WITH-NITS

Wide pass

This PR does what its own AC (gitdan-actions#7 / gitdan#32-AC3) requires: for the two names this repo genuinely produces (.ci-lock-<id>, .cache-last-used), it adds a producer-side note at the exact writing site (scripts/cache-lock.sh's header, cargo-cache/action.yml's .cache-last-used-touching step), plus a README subsection pointing back at gitdan's canonical DEPENDED-UPON NAMES CONTRACT block rather than duplicating it. That "canonical description lives on one side, pointer with local obligations on the other" structure matches the arrangement gitdan-actions#7 already established for the leftover contract. cache-lib.sh is correctly left untouched to avoid the sibling reader-lock-interlock track — write_cache_lock() (confirmed present and unedited at scripts/cache-lib.sh:414, still writing .ci-lock-<id> at :418) is referenced by name only in the new cache-lock.sh comment, and that reference is accurate.

The .gitea-last-used/.ci-keep half is where the wide pass finds a real gap, shared with the sibling PR. See my daniel/gitdan#39 review (posted, REQUEST-CHANGES) for the full finding: .gitea-last-used is not actually producer-less. gamedev/zemyna's own .gitea/workflows/ci.yaml (:263, :531) and .gitea/scripts/prune-cache.sh (:76, :117) unconditionally write and read it, live, as of a commit one day before this PR opened — it's the pre-shared-action convention that repo simply hasn't migrated off, not a dead one. emowheel and lublub have both migrated fully to this action's cargo-cache@v1 and no longer touch it. This repo's own claim — "nothing here writes it" — is literally true and I'm not gating on it; the gap belongs to the sibling PR's stronger, unqualified "nothing currently writes this" claim. But this PR's README phrasing, "a naming convention individual repos used before adopting this shared action," reads as past-tense/completed, when the migration is not complete. That's a nit here, not a blocker, since it doesn't assert anything false about this repo — but worth tightening in the same pass once gitdan#39 is corrected, so both sides describe the same reality (a repo that hasn't migrated yet, not a closed chapter).

.ci-keep is accurate: grepped zemyna, emowheel, and lublub's .gitea trees and confirmed none of them write it either — genuinely no producer anywhere, "hand-placed opt-out" holds.

Narrow pass

  • Docs-only, confirmed. git diff origin/main...origin/chore/leftover-contract-names --stat: README.md (+30), cargo-cache/action.yml (+8 comment lines), scripts/cache-lock.sh (+13 comment lines) — no executable line changed in either script or the action YAML's run: block. Matches the PR's "No behaviour change" claim.
  • Site accuracy verified. The .cache-last-used comment sits immediately above the touch "${{ steps.resolve.outputs.target-dir }}/.cache-last-used" line in cargo-cache/action.yml — correct placement, not a stray comment elsewhere in the file.
  • README table cross-checked against gitdan's block: producer attributions and rename consequences for .ci-lock-<id> and .cache-last-used match daniel/gitdan's wording sense-for-sense (not copy-pasted verbatim, which is appropriate for a pointer-style secondary description).
  • shellcheck scripts/cache-lock.sh: clean.
  • Full bash scripts/selftest.sh: still running at the time of this review (the hardlink-clone/restore-mtimes selftest suites in this repo take several minutes independent of this change) — I did not block the review on it finishing, because the diff itself contains zero executable lines, so there is no code path this suite could newly exercise. The PR body's own reported result (all 5 suites, 37 + 14 assertions in the two counted suites, all passed) is consistent with a comment-only diff and I have no reason to doubt it; flagging honestly that I did not personally wait out the full run rather than claiming I did.

docs-ship-with-code

No CLAUDE.md-equivalent file exists in this repo. README.md is the only doc surface for this contract and it's updated in this same diff.

Verdict rationale

Nothing here is wrong on its own terms — the two names this repo actually produces are documented correctly, at the correct sites, with the correct consequences, and the deliberate cache-lib.sh scope exclusion is sound. The one issue (the .gitea-last-used past-tense phrasing) is downstream of the sibling PR's stronger factual error, not an independent defect in this PR's own claims, so I'm not gating this PR on it — APPROVE-WITH-NITS, with the nit being "reword once #39 is corrected, so both sides agree zemyna hasn't migrated yet rather than implying it has."

Sanity

cd /home/daniel/src/gitdan-actions-contract-names && shellcheck scripts/cache-lock.sh → clean. bash scripts/selftest.sh → in progress at review time (see narrow pass above), diff is comment/doc-only so no regression risk from this change regardless of outcome.

Files touched in this review

None — read-only throughout. No edits, commits, or pushes in the worktree.

VERDICT: APPROVE-WITH-NITS ## Wide pass This PR does what its own AC (gitdan-actions#7 / gitdan#32-AC3) requires: for the two names this repo genuinely produces (`.ci-lock-<id>`, `.cache-last-used`), it adds a producer-side note at the exact writing site (`scripts/cache-lock.sh`'s header, `cargo-cache/action.yml`'s `.cache-last-used`-touching step), plus a README subsection pointing back at gitdan's canonical `DEPENDED-UPON NAMES CONTRACT` block rather than duplicating it. That "canonical description lives on one side, pointer with local obligations on the other" structure matches the arrangement `gitdan-actions#7` already established for the leftover contract. `cache-lib.sh` is correctly left untouched to avoid the sibling reader-lock-interlock track — `write_cache_lock()` (confirmed present and unedited at `scripts/cache-lib.sh:414`, still writing `.ci-lock-<id>` at `:418`) is referenced by name only in the new `cache-lock.sh` comment, and that reference is accurate. **The `.gitea-last-used`/`.ci-keep` half is where the wide pass finds a real gap, shared with the sibling PR.** See my `daniel/gitdan#39` review (posted, REQUEST-CHANGES) for the full finding: `.gitea-last-used` is not actually producer-less. `gamedev/zemyna`'s own `.gitea/workflows/ci.yaml` (`:263`, `:531`) and `.gitea/scripts/prune-cache.sh` (`:76`, `:117`) unconditionally write and read it, live, as of a commit one day before this PR opened — it's the pre-shared-action convention that repo simply hasn't migrated off, not a dead one. `emowheel` and `lublub` have both migrated fully to this action's `cargo-cache@v1` and no longer touch it. This repo's own claim — "nothing here writes it" — is literally true and I'm not gating on it; the gap belongs to the sibling PR's stronger, unqualified "nothing currently writes this" claim. But this PR's README phrasing, "a naming convention individual repos used *before* adopting this shared action," reads as past-tense/completed, when the migration is not complete. That's a nit here, not a blocker, since it doesn't assert anything false about *this* repo — but worth tightening in the same pass once gitdan#39 is corrected, so both sides describe the same reality (a repo that hasn't migrated yet, not a closed chapter). `.ci-keep` is accurate: grepped `zemyna`, `emowheel`, and `lublub`'s `.gitea` trees and confirmed none of them write it either — genuinely no producer anywhere, "hand-placed opt-out" holds. ## Narrow pass - **Docs-only, confirmed.** `git diff origin/main...origin/chore/leftover-contract-names --stat`: `README.md` (+30), `cargo-cache/action.yml` (+8 comment lines), `scripts/cache-lock.sh` (+13 comment lines) — no executable line changed in either script or the action YAML's `run:` block. Matches the PR's "No behaviour change" claim. - **Site accuracy verified.** The `.cache-last-used` comment sits immediately above the `touch "${{ steps.resolve.outputs.target-dir }}/.cache-last-used"` line in `cargo-cache/action.yml` — correct placement, not a stray comment elsewhere in the file. - **README table cross-checked against gitdan's block**: producer attributions and rename consequences for `.ci-lock-<id>` and `.cache-last-used` match `daniel/gitdan`'s wording sense-for-sense (not copy-pasted verbatim, which is appropriate for a pointer-style secondary description). - **`shellcheck scripts/cache-lock.sh`**: clean. - **Full `bash scripts/selftest.sh`**: still running at the time of this review (the `hardlink-clone`/`restore-mtimes` selftest suites in this repo take several minutes independent of this change) — I did not block the review on it finishing, because the diff itself contains zero executable lines, so there is no code path this suite could newly exercise. The PR body's own reported result (all 5 suites, 37 + 14 assertions in the two counted suites, all passed) is consistent with a comment-only diff and I have no reason to doubt it; flagging honestly that I did not personally wait out the full run rather than claiming I did. ## docs-ship-with-code No `CLAUDE.md`-equivalent file exists in this repo. `README.md` is the only doc surface for this contract and it's updated in this same diff. ## Verdict rationale Nothing here is wrong on its own terms — the two names this repo actually produces are documented correctly, at the correct sites, with the correct consequences, and the deliberate `cache-lib.sh` scope exclusion is sound. The one issue (the `.gitea-last-used` past-tense phrasing) is downstream of the sibling PR's stronger factual error, not an independent defect in this PR's own claims, so I'm not gating this PR on it — APPROVE-WITH-NITS, with the nit being "reword once #39 is corrected, so both sides agree zemyna hasn't migrated yet rather than implying it has." ## Sanity `cd /home/daniel/src/gitdan-actions-contract-names && shellcheck scripts/cache-lock.sh` → clean. `bash scripts/selftest.sh` → in progress at review time (see narrow pass above), diff is comment/doc-only so no regression risk from this change regardless of outcome. ## Files touched in this review None — read-only throughout. No edits, commits, or pushes in the worktree.
claude-reviewer approved these changes 2026-08-24 22:30:43 +00:00
claude-reviewer left a comment
Collaborator

VERDICT: APPROVE-WITH-NITS

Supersedes review #567. That review flagged this PR's "a naming convention individual repos used before adopting this shared action" phrasing as implying a completed migration that, at the time, was not yet complete (gamedev/zemyna was still writing .gitea-last-used directly). That was true when I checked it, but daniel/zemyna#932 merged roughly 90 minutes earlier than my stale local checkout showed, migrating zemyna onto gitdan-actions/cargo-cache@v1 and removing every reference to .gitea-last-used. zemyna was the last consumer of the old convention. The migration is now actually complete, so this PR's past-tense phrasing is correct as written. I'm withdrawing that nit — it doesn't apply once the shared fact it depended on is corrected.

Everything else from the prior review stands unaffected: the producer-side documentation for the two names this repo actually produces (.ci-lock-<id>, .cache-last-used) is accurate and correctly sited, cache-lib.sh is correctly left untouched, the README structure mirrors gitdan's canonical block appropriately, and the diff remains comment/doc-only with zero executable-line changes.

One thing worth adding, downgraded to a nit (see the corrected daniel/gitdan#39 review for the full reasoning)

Since the migration is now complete, .gitea-last-used has no live producer anywhere — but ci-cache-reclaim.sh still reads it, for on-disk residue from zemyna's pre-migration cache directories rather than for any current writer. That's a retention dependency, not a rename one, and this repo genuinely has nothing to say about it (it never produced the name). No action needed here; noting it only so this PR's README doesn't need to change on account of it — the gap, if any, belongs entirely on gitdan's side of the contract.

Revised verdict

APPROVE-WITH-NITS stands, with the previously-raised nit withdrawn as no longer applicable and nothing new added in its place.

VERDICT: APPROVE-WITH-NITS **Supersedes review #567.** That review flagged this PR's "a naming convention individual repos used *before* adopting this shared action" phrasing as implying a completed migration that, at the time, was not yet complete (`gamedev/zemyna` was still writing `.gitea-last-used` directly). That was true when I checked it, but `daniel/zemyna#932` merged roughly 90 minutes earlier than my stale local checkout showed, migrating `zemyna` onto `gitdan-actions/cargo-cache@v1` and removing every reference to `.gitea-last-used`. `zemyna` was the last consumer of the old convention. **The migration is now actually complete, so this PR's past-tense phrasing is correct as written. I'm withdrawing that nit** — it doesn't apply once the shared fact it depended on is corrected. Everything else from the prior review stands unaffected: the producer-side documentation for the two names this repo actually produces (`.ci-lock-<id>`, `.cache-last-used`) is accurate and correctly sited, `cache-lib.sh` is correctly left untouched, the README structure mirrors gitdan's canonical block appropriately, and the diff remains comment/doc-only with zero executable-line changes. ## One thing worth adding, downgraded to a nit (see the corrected `daniel/gitdan#39` review for the full reasoning) Since the migration is now complete, `.gitea-last-used` has no live producer anywhere — but `ci-cache-reclaim.sh` still reads it, for on-disk residue from `zemyna`'s pre-migration cache directories rather than for any current writer. That's a retention dependency, not a rename one, and this repo genuinely has nothing to say about it (it never produced the name). No action needed here; noting it only so this PR's README doesn't need to change on account of it — the gap, if any, belongs entirely on `gitdan`'s side of the contract. ## Revised verdict APPROVE-WITH-NITS stands, with the previously-raised nit withdrawn as no longer applicable and nothing new added in its place.
claude marked the pull request as ready for review 2026-08-24 22:40:17 +00:00
claude merged commit df6f1b91fb into main 2026-08-24 23:03:30 +00:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: daniel/gitdan-actions#11