Stop crediting #65 with a fix that never ran

[ptr727]

Found by Copilot review on #77, the promotion. Documentation only.

The stale mechanism

The item described what #75 replaced — installing the Ubuntu package and calling git restore-mtime static. CI uses a SHA-pinned action now, and make-release.sh requires v2025.08 or newer by either invocation form.

The worse half, which was mine

The entry claimed:

Release 20260809-030521 is the first production deploy to exercise it

It exercised nothing. That release ran v2022.12, which current git refuses to run and which exits 0 regardless. The host measured the result as 0 of 3,275 shared inodes.

I wrote that sentence earlier the same day, on the strength of a merge rather than a measurement — the exact distinction this repo keeps having to relearn, and the one I had told the VPS agent I would stop getting wrong two rounds before writing it.

What the item says now

• Credits Use the maintained git-restore-mtime, and assert it actually restored #75 and Restore mtimes in the local release too, so both paths fail the same way #76, and says plainly that Restore file mtimes in CI so the deploy can hard-link static files #65 shipped broken and why: git whatchanged, refused by current git, no way to pass --i-still-use-this through the tool.
• Carries what was actually measured — 1052 of 3269 linked on the local production mirror, the release stamped 2026-08-01 rather than at build time (proving the restore reached the release, not just the working tree), and 584 MB then 18 MB across two releases.
• Records the thing that will otherwise read as a regression: the first restored release cannot link, because its predecessor was built with unrestored mtimes. So the first deploy after the promotion is expected to link zero, and the one after it is the real test.

The retained #65 diagnosis

Kept verbatim, with a lead-in saying what it is. It contains a "today" that means 2026-08-08 and names a fix that did not run — both read as current state without the framing.

markdownlint clean, both checks/ gates pass.

[Copilot]

Pull request overview

Updates the project’s TODO/state record to accurately describe the git-restore-mtime rollout and measurements, correcting a previously overstated claim about which release actually exercised the fix.

Changes:

• Rewrites the “Restore file mtimes so --link-dest links” entry to credit #75/#76 (and explicitly note #65 shipped broken).
• Records measured outcomes (local mirror hard-link counts, release timestamp provenance, and expected “first restored deploy links 0” transition behavior).
• Adds link references for PRs #75 and #76.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ptr727]

Fixed in 67eaa9e. Recording it here rather than on the thread, which was already resolved by the time I went to reply — so the fixing SHA would otherwise not be written down anywhere.

Use US English spelling for consistency with the rest of this file. "behaviour" should be "behavior".

Correct, and worse than a stray inconsistency: I wrote "behaviour" two words away from the "behavior" I was introducing, in the same sentence.

Swept the rest of the diff for the class — colour, favour, organis, recognis — and that was the only one.

Worth recording why nothing caught it: cspell runs over README.md and HISTORY.md only, so spelling in TODO.md is convention rather than an enforced rule. That is a deliberate scope choice rather than a gap — content/ is an imported archive and cspell.json ignores it on purpose — but it does mean this file depends on a reader.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

TODO.md:45

• The TODO entry says both CI and the local release path assert that nothing under static/ is newer than HEAD’s commit. In deploy/make-release.sh, the assertion explicitly excludes locally modified/untracked static/ paths (so the check remains usable during an edit loop). The wording here is slightly stronger than what the script actually enforces.

- **~~Restore file mtimes so `--link-dest` links.~~ Done, but by [#75][pr-75] and [#76][pr-76] rather than by [#65][pr-65], which shipped it broken.** CI restores with `chetan/git-restore-mtime-action`, pinned by SHA, and [`deploy/make-release.sh`](./deploy/make-release.sh) requires the tool on `PATH` or as a git subcommand at **v2025.08 or newer**. Both then assert the outcome: nothing under `static/` may be newer than HEAD's own commit, since a checkout always happens after the commit it checks out.

[ptr727]

Fixed in 110faee.

The TODO entry says both CI and the local release path assert that nothing under static/ is newer than HEAD's commit. In deploy/make-release.sh, the assertion explicitly excludes locally modified/untracked static/ paths.

Correct. CI does assert it over the whole tree; the local path does not, and cannot — a working tree can legitimately hold a static file newer than any commit, and skipping the check whenever the tree is dirty would make it useless during an edit loop.

So the entry claimed a stronger rule than the code runs, which is the same defect this file keeps producing: prose describing the rule someone meant rather than the one that executes. Third instance in this PR alone.

It now states the shared invariant as tracked, unmodified files, and names why the two paths differ rather than eliding it:

CI compares the whole tree, because a fresh checkout has nothing uncommitted in it; the local path excludes modified and untracked paths … so the check stays usable during an edit loop rather than being skipped whenever the tree is dirty.

The guarantee is the same in both; only the exclusion CI does not need differs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

TODO.md:45

• The text says “CI compares the whole tree”, but the CI assertion in .github/workflows/deploy-site-task.yml checks static/ only (it runs find static ...). As written, this reads like CI scans the entire repo, which is inaccurate and could mislead future edits to the guard.

- **~~Restore file mtimes so `--link-dest` links.~~ Done, but by [#75][pr-75] and [#76][pr-76] rather than by [#65][pr-65], which shipped it broken.** CI restores with `chetan/git-restore-mtime-action`, pinned by SHA, and [`deploy/make-release.sh`](./deploy/make-release.sh) requires the tool on `PATH` or as a git subcommand at **v2025.08 or newer**. Both then assert the outcome, since a checkout always happens after the commit it checks out: **no tracked, unmodified file under `static/` may be newer than HEAD's own commit.** CI compares the whole tree, because a fresh checkout has nothing uncommitted in it; the local path excludes modified and untracked paths, which a working tree can legitimately hold newer than any commit, so the check stays usable during an edit loop rather than being skipped whenever the tree is dirty.

[ptr727]

Fixed in f88dba8.

The text says "CI compares the whole tree", but the CI assertion checks static/ only (it runs find static ...).

Correct. I meant "the whole of static/" and wrote something that reads as the repository. Verified against both implementations rather than from memory:

.github/workflows/deploy-site-task.yml:133   find static -type f
deploy/make-release.sh:186                   find static -type f

Nothing outside that directory is examined by either. The difference between the two paths is the exclusion, not the scope, and the sentence now says that.

Your reason for flagging it is the right one — this sentence is what a future edit to the guard gets read against, so "whole tree" would invite someone to widen the walk and wonder why generated pages fail it. They always would: public/ is rebuilt every run and can never be older than HEAD.

That is four wording defects in this one documentation PR, all the same shape — prose asserting something the code does not do. The file is the one place in the repo where nothing mechanical checks a claim, which is now recorded in it as an item of its own.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

TODO.md:51

• This line again uses the non-comma form 1052 of 3266 adjacent to comma-separated values elsewhere in the same section. Using a consistent thousands separator improves readability.

  - **The original diagnosis, kept as written on 2026-08-08 because it is what the next change to the deploy's linking behavior will need.** Read it as the record of why the change was made, not as current state: the "today" in it is that date, and the fix it names is #65's, which turned out not to run. The host side measured zero shared inodes across every release the pipeline has delivered, against 1052 of 3266 on a release built here, and the cause is neither the call site nor the confined rsync: both were tested there and link correctly through a relative symlink. Git stores no mtimes, so a CI checkout writes all 3,272 files inside a 23-second window and the `static/` tree that would otherwise match arrives freshly stamped with everything else. `git-restore-mtime` is the fix and needs no checkout change, since `deploy-site-task.yml` already uses `fetch-depth: 0`, and it is deterministic across runs in exactly the place that matters, because `static/` has stable last-commit times. **The ordering is the part worth writing down.** Today every file arrives as a fresh inode, so `--no-g --chmod=D2755,F644` re-establishes the mode contract on every deploy; make the mtimes honest and about a third of the tree starts arriving as links carrying whatever mode its chain began with, which is the trap above. Harmless as things stand, since every inode in the current chain was made by that same rsync line, and it means the live media check should exist first. Nothing is broken meanwhile: the cost is ~585 MB a release, which the host's prune timer reclaims.

TODO.md:49

• Within this block, numeric counts alternate between comma-separated and non-comma-separated forms (e.g., 0 of 3,275 vs 1052 of 3269 / 1052 of 3275). Standardizing the formatting makes the measurements easier to compare at a glance and reduces the chance of misreading values.

This issue also appears on line 51 of the same file.

  - **Verified end to end on the local production mirror, 2026-08-09**, with the tool installed by hand: `1052 of 3269 files hard-linked`, and the release carrying `site/media` and `site/external` stamped `2026-08-01`, the commit date rather than the build date, which is what proves the restore reached the release and not merely the working tree. Two releases cost 584 MB then 18 MB.
  - **The first restored release cannot link, by construction**, because it is compared against a predecessor built with unrestored mtimes. `make-release.sh`'s zero-shared-files guard refuses that one, so the changeover takes a single `NO_LINK_DEST=1` release to seed a restored generation. The deploy has no such guard and will simply link nothing for one deploy. **So the first deploy after the promotion is expected to link 0**, and the one after it is the real test.
  - **The shared-inode count on the VPS is still the open half** and is the host's to measure, since nothing here can read an inode there. Told to them in the channel as §S, which corrects §R for predicting the jump one deploy too early. Roughly 1052 of 3275 is what the change predicts, at the *second* restored deploy.

TODO.md:50

• “badly moded file” is ambiguous phrasing in this context (it can read as “fashionable” rather than “incorrect permissions”). Using “incorrect permissions” (or similar) makes the operational risk clearer.

  - Making `--link-dest` effective is also what makes a badly moded file able to ride a link into every later release, which is the trap below. The live media check landed first, deliberately, and is what watches for exactly that.

[ptr727]

All three fixed in beccfb0.

Thousands separators

Both comments are the same finding and both are right — 1052 of 3269 sat next to 0 of 3,275 in the same block, which makes two measurements of the same quantity look like different quantities at a glance.

Measured the file's actual convention rather than guessing: comma form leads 10 instances to 6, and 1,052 already appeared with one. So the new text follows it.

One deliberate exception: line 51 is left alone. Its lead-in says it is kept as written on 2026-08-08, and reformatting inside it would make that claim false in the same PR that added it. A preserved record that gets tidied is no longer a preserved record. Flagging it rather than quietly leaving it, since it will keep showing up as an inconsistency.

"badly moded"

it can read as "fashionable" rather than "incorrect permissions"

Fair, and I would not have seen it. Now:

Making --link-dest effective is also what makes a file that acquires the wrong permissions able to ride a link into every later release

Which is what the sentence is actually about: a hard link carries its inode's mode, so a wrong one propagates forward. The trap further down still says "mode" in **A hard link keeps its inode's mode and ownership** — that one is precise rather than ambiguous, since it names the inode attribute directly, so it stays.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.