Fingerprint every asset index.html names #204

Merged
rob merged 5 commits from static-asset-cache-busting into main 2026-08-20 20:00:52 +00:00
Owner

Task 265. Every published href/src in index.html now carries a content hash — css/*, js/*, lib/leaflet/*, logo_512.png and PlaceMark.WebUI.styles.css — so a changed stylesheet or map script reaches a returning browser on the next load. ADR-0169 has the reasoning.

Why not a header. The proxy in front of places.robware.uk replaces Cache-Control and strips the origin's Last-Modified on exactly the extensions that matter (proved by curl: /appsettings.json and /lib/leaflet/LICENSE keep theirs, /css/app.css does not, and git.robware.uk behind the same openresty is untouched — it is a per-host opt-in). Any header fix would have been silently inert until Rob changed a proxy this repository cannot see. A URL that changes with its content cannot be overridden by a cache policy anywhere.

Why the SDK could not do it alone. OverrideHtmlAssetPlaceholders filters its endpoints to .js/.mjs, so <link href="css/app#[.{fingerprint}].css"> gets its placeholder stripped and left pointing at a name no longer on disk. The scoped-CSS bundle is separately hard-coded to a soft fingerprint, which only MapStaticAssets can serve and nginx cannot. Hence the inline task, which also fails the publish if any local reference does not resolve to a published file.

Lives in the .csproj rather than its own .targets because BuildFileTests forbids both a second build file and an <Import> in PlaceMark.WebUI.

Also: appsettings.json gets no-cache in both images — the one remaining file whose contents change per deployment under a fixed name, and not on the extension list the proxy rewrites.

Verified by mutation, not inspection: disabling the rewrite reddens the publish with nine errors; emptying the import map turns a real headless Chromium's await import("./js/map.js") into IMPORT-FAIL. With both intact the published output loads in Chromium with no non-2xx response, map.js resolving to /js/map.sxtztp08qj.js and Leaflet to /lib/leaflet/leaflet.v78t325kt9.js.

PR #190's favicon rename is superseded — logo_512.png is now fingerprinted, so no further rename is wanted.

Task 265. Every published `href`/`src` in `index.html` now carries a content hash — `css/*`, `js/*`, `lib/leaflet/*`, `logo_512.png` and `PlaceMark.WebUI.styles.css` — so a changed stylesheet or map script reaches a returning browser on the next load. ADR-0169 has the reasoning. **Why not a header.** The proxy in front of `places.robware.uk` replaces `Cache-Control` and strips the origin's `Last-Modified` on exactly the extensions that matter (proved by `curl`: `/appsettings.json` and `/lib/leaflet/LICENSE` keep theirs, `/css/app.css` does not, and `git.robware.uk` behind the same openresty is untouched — it is a per-host opt-in). Any header fix would have been silently inert until Rob changed a proxy this repository cannot see. A URL that changes with its content cannot be overridden by a cache policy anywhere. **Why the SDK could not do it alone.** `OverrideHtmlAssetPlaceholders` filters its endpoints to `.js`/`.mjs`, so `<link href="css/app#[.{fingerprint}].css">` gets its placeholder *stripped* and left pointing at a name no longer on disk. The scoped-CSS bundle is separately hard-coded to a soft fingerprint, which only `MapStaticAssets` can serve and nginx cannot. Hence the inline task, which also fails the publish if any local reference does not resolve to a published file. Lives in the `.csproj` rather than its own `.targets` because `BuildFileTests` forbids both a second build file and an `<Import>` in `PlaceMark.WebUI`. Also: `appsettings.json` gets `no-cache` in both images — the one remaining file whose contents change per deployment under a fixed name, and not on the extension list the proxy rewrites. Verified by mutation, not inspection: disabling the rewrite reddens the publish with nine errors; emptying the import map turns a real headless Chromium's `await import("./js/map.js")` into `IMPORT-FAIL`. With both intact the published output loads in Chromium with no non-2xx response, `map.js` resolving to `/js/map.sxtztp08qj.js` and Leaflet to `/lib/leaflet/leaflet.v78t325kt9.js`. PR #190's favicon rename is superseded — `logo_512.png` is now fingerprinted, so no further rename is wanted.
Fingerprint every asset index.html names
Some checks failed
CI / build (pull_request) Successful in 10m37s
CI / container-images (pull_request) Has been cancelled
CI / e2e (pull_request) Has been cancelled
e13222a471
rob left a comment

Verdict: changes needed

Three of the findings below are holes in the guard itself, which is the PR's central safety claim; the fourth is whether the guard runs.

The guard is not reached on the PRs that need it. dotnet publish happens only in the container-images job (e2e uses dotnet run; build never publishes), and that job's relevance filter in .forgejo/workflows/ci.yml lists src/PlaceMark.WebUI/Dockerfile, src/PlaceMark.WebUI/docker, src/PlaceMark.Joint, ... but neither src/PlaceMark.WebUI/PlaceMark.WebUI.csproj nor src/PlaceMark.WebUI/wwwroot. This PR is relevant only incidentally, because it happens to touch both nginx.confs. A future PR that adds a <link> to index.html, adds an asset, or edits the fingerprint patterns gets relevant=false and no guard at all — it fails after merge, on push, reddening main. Add both paths to that filter.

The proxy claim is the load-bearing fact and only a quarter of it is recorded. ADR-0169 pastes the /css/app.css response, but the three observations the conclusion actually turns on — /appsettings.json and /lib/leaflet/LICENSE keeping their last-modified, and git.robware.uk passing Forgejo's headers through — are asserted in prose. Paste those responses into the ADR too. I cannot verify any of it from here and am taking it on the author's word; with the transcripts in the record, the next person can re-check it in thirty seconds instead of re-measuring.

Leaflet's lib/leaflet/images/*.png: acceptable as it stands, no change wanted. Nothing reaches them, the trigger is narrow, the failure would be cosmetic and self-clearing, and the Consequences section names the exact condition. Worth knowing that adopting default markers or a layers control opens it as surely as an upgrade does, but that is a note for whoever does it, not a change here.

CI: run #839 for e13222a is still running at the time of this review — not green, not red. Re-check before merging, and specifically that the container-images job went relevant=true and built both images, since that is the only place the new guard executes.

Verdict: changes needed Three of the findings below are holes in the guard itself, which is the PR's central safety claim; the fourth is whether the guard runs. **The guard is not reached on the PRs that need it.** `dotnet publish` happens only in the `container-images` job (`e2e` uses `dotnet run`; `build` never publishes), and that job's relevance filter in `.forgejo/workflows/ci.yml` lists `src/PlaceMark.WebUI/Dockerfile`, `src/PlaceMark.WebUI/docker`, `src/PlaceMark.Joint`, ... but neither `src/PlaceMark.WebUI/PlaceMark.WebUI.csproj` nor `src/PlaceMark.WebUI/wwwroot`. This PR is relevant only incidentally, because it happens to touch both `nginx.conf`s. A future PR that adds a `<link>` to `index.html`, adds an asset, or edits the fingerprint patterns gets `relevant=false` and no guard at all — it fails after merge, on `push`, reddening `main`. Add both paths to that filter. **The proxy claim is the load-bearing fact and only a quarter of it is recorded.** ADR-0169 pastes the `/css/app.css` response, but the three observations the conclusion actually turns on — `/appsettings.json` and `/lib/leaflet/LICENSE` keeping their `last-modified`, and `git.robware.uk` passing Forgejo's headers through — are asserted in prose. Paste those responses into the ADR too. I cannot verify any of it from here and am taking it on the author's word; with the transcripts in the record, the next person can re-check it in thirty seconds instead of re-measuring. Leaflet's `lib/leaflet/images/*.png`: acceptable as it stands, no change wanted. Nothing reaches them, the trigger is narrow, the failure would be cosmetic and self-clearing, and the Consequences section names the exact condition. Worth knowing that adopting default markers or a layers control opens it as surely as an upgrade does, but that is a note for whoever does it, not a change here. CI: run #839 for `e13222a` is still `running` at the time of this review — not green, not red. Re-check before merging, and specifically that the `container-images` job went `relevant=true` and built both images, since that is the only place the new guard executes.
@ -52,0 +130,4 @@
var html = File.ReadAllText(HtmlFile);
foreach (Match reference in Regex.Matches(html, "(?:href|src)=\"(?<path>[^\"]*)\""))
Author
Owner

Double-quoted attributes only — href='css/new.css' is invisible to this loop and to the guard at line 162, so it publishes clean and 404s live. That is not hypothetical here: a single-quoted attribute is one of the evasions FieldErrorsValidatesOnSubmitOnlyTests was actually found to have (ADR-0066).

(?:href|src)=(?<q>["'])(?<path>[^"']*)\k<q> in both places, with the captured quote reused in the replacement.

Double-quoted attributes only — `href='css/new.css'` is invisible to this loop *and* to the guard at line 162, so it publishes clean and 404s live. That is not hypothetical here: a single-quoted attribute is one of the evasions `FieldErrorsValidatesOnSubmitOnlyTests` was actually found to have (ADR-0066). `(?:href|src)=(?<q>["'])(?<path>[^"']*)\k<q>` in both places, with the captured quote reused in the replacement.
@ -52,0 +134,4 @@
{
var referencedPath = reference.Groups["path"].Value;
string contentAddressedPath;
if (!fingerprinted.TryGetValue(referencedPath, out contentAddressedPath))
Author
Owner

The two loops normalise differently — this one looks the path up verbatim, the guard at line 175 does TrimStart('/') — and the gap is silent for every extension except .css/.js.

href="/logo_512.png" or href="./logo_512.png" misses this dictionary, so no File.Move happens; the plain file is still in the published output because the SDK's default fingerprint is soft; the guard then finds it and passes. The asset ships unfingerprinted, which is the exact defect this PR exists to close. Same for any future .png/.svg/.webp/.woff2 — all soft by default (StaticWebAssetsFingerprintContent defaults true, and FingerprintingPatterns.props declares no hard * pattern).

For .css/.js the hard rename removes the plain file, so a missed reference does redden. The discrimination is real for two extensions only.

Cheapest close: reject the spellings the rewriter cannot understand. In the guard, error on any local reference starting / or ./index.html has <base href="/"> and every reference in it is relative, so that costs nothing and turns the silent case loud.

The two loops normalise differently — this one looks the path up verbatim, the guard at line 175 does `TrimStart('/')` — and the gap is silent for every extension except `.css`/`.js`. `href="/logo_512.png"` or `href="./logo_512.png"` misses this dictionary, so no `File.Move` happens; the plain file is still in the published output because the SDK's default fingerprint is soft; the guard then finds it and passes. The asset ships unfingerprinted, which is the exact defect this PR exists to close. Same for any future `.png`/`.svg`/`.webp`/`.woff2` — all soft by default (`StaticWebAssetsFingerprintContent` defaults true, and `FingerprintingPatterns.props` declares no hard `*` pattern). For `.css`/`.js` the hard rename removes the plain file, so a missed reference does redden. The discrimination is real for two extensions only. Cheapest close: reject the spellings the rewriter cannot understand. In the guard, error on any local reference starting `/` or `./` — `index.html` has `<base href="/">` and every reference in it is relative, so that costs nothing and turns the silent case loud.
@ -52,0 +149,4 @@
// Quoted on both sides, so only a whole attribute value is ever rewritten. The import map the
// SDK writes into this same file spells its keys `"./js/map.js"`, which this deliberately does
// not match: those are already correct, and rewriting them would break module resolution.
html = html.Replace("\"" + referencedPath + "\"", "\"" + contentAddressedPath + "\"");
Author
Owner

Whole-document Replace, so it rewrites the quoted string wherever it appears, not at the match. Safe today only because index.html carries no .js href/src other than the SDK placeholder, so no dictionary key can collide with an import-map key. Add one <script src="js/x.js"> and the comment above becomes the only thing standing between this and a rewritten import map. Replace at reference.Index instead (iterate the matches in reverse so earlier offsets stay valid).

Whole-document `Replace`, so it rewrites the quoted string wherever it appears, not at the match. Safe today only because `index.html` carries no `.js` `href`/`src` other than the SDK placeholder, so no dictionary key can collide with an import-map key. Add one `<script src="js/x.js">` and the comment above becomes the only thing standing between this and a rewritten import map. Replace at `reference.Index` instead (iterate the matches in reverse so earlier offsets stay valid).
@ -52,0 +167,4 @@
|| referencedPath == "/"
|| referencedPath.StartsWith("#", StringComparison.Ordinal)
|| referencedPath.StartsWith("data:", StringComparison.Ordinal)
|| referencedPath.IndexOf("://", StringComparison.Ordinal) >= 0)
Author
Owner

:// misses mailto: and tel:, and a protocol-relative //host/x.js has no scheme at all — each is treated as a local path and fails the publish. So does any local path carrying a query string or a fragment (icons.svg#pin). All loud rather than dangerous, but each is a confusing publish failure on a legitimate change. Prefer Uri.TryCreate(referencedPath, UriKind.Absolute, out _) for the absolute cases, and strip anything from the first ? or # before the existence check.

`://` misses `mailto:` and `tel:`, and a protocol-relative `//host/x.js` has no scheme at all — each is treated as a local path and fails the publish. So does any local path carrying a query string or a fragment (`icons.svg#pin`). All loud rather than dangerous, but each is a confusing publish failure on a legitimate change. Prefer `Uri.TryCreate(referencedPath, UriKind.Absolute, out _)` for the absolute cases, and strip anything from the first `?` or `#` before the existence check.
@ -52,0 +187,4 @@
<Target Name="FingerprintPublishedHtmlAssetReferencesOnPublish"
AfterTargets="Publish"
Condition="Exists('$(PublishDir)wwwroot/index.html')">
Author
Owner

A missing index.html skips the target silently, which disables both the rewrite and the guard. Condition on the project instead — e.g. '$(PublishDir)' != '' plus an <Error> when the file is absent — so a publish-layout change fails rather than quietly reverting to plain names.

A missing `index.html` skips the target silently, which disables both the rewrite and the guard. Condition on the project instead — e.g. `'$(PublishDir)' != ''` plus an `<Error>` when the file is absent — so a publish-layout change fails rather than quietly reverting to plain names.
Close the fingerprinting guard's blind spots and run it in CI
Some checks failed
CI / build (pull_request) Failing after 17m10s
CI / e2e (pull_request) Has been skipped
CI / container-images (pull_request) Has been skipped
5b61a02087
Author
Owner

All five actioned in 5b61a02, and both laxness findings were confirmed real before fixing — the reviewed version publishes clean with href="./logo_512.png" and href="/logo_512.png", shipping logo_512.png unfingerprinted. That is now two errors.

  • Relevance filtersrc/PlaceMark.WebUI/PlaceMark.WebUI.csproj and src/PlaceMark.WebUI/wwwroot added, with the reason on the step's own comment. Nothing else depends on the filter's shape: relevant gates steps inside container-images only and is exported to no outputs: block or other job's needs; publish is untouched and still true for push/workflow_dispatch alone, so no CalVer counter or registry write follows from widening it. Cost is CI minutes on WebUI-touching PRs.
  • Normalisation mismatch — a rooted, ./ or ../ reference is now a build error rather than something the two loops disagree about. <base href="/"> and the error UI's <a href="."> stay legal as the two literals. Ordered before the Uri.TryCreate check deliberately: on Unix Uri reads a leading slash as an absolute file: path and would have swallowed /logo_512.png (ADR-0155).
  • Single quotes — one (?:href|src)=(?<quote>["'])(?<path>[^"']*)\k<quote> shared by both loops, so they cannot diverge again. href='css/theme.css' now rewrites; it was previously invisible to both.
  • Offset replace — replaces the matched path group at its index, iterating backwards. Demonstrated by a quoted path inside an HTML comment, which the old whole-document Replace rewrote and this one leaves alone. I could not construct the import-map collision itself, because the SDK spells its keys "./js/map.js"; the dependency is removed rather than the fault observed, and the ADR says so.
  • Absolute schemes / query / fragmentUri.TryCreate(..., UriKind.Absolute, ...) plus an explicit protocol-relative case; the path is truncated at the first ? or # before the existence check. mailto: and tel: verified accepted.
  • Target condition — gone. AfterTargets="Publish" unconditionally, with an <Error> when index.html is absent.

ADR-0169 now carries all four curl transcripts in full — the .css response, /appsettings.json and /lib/leaflet/LICENSE keeping their last-modified, and git.robware.uk — with the three conclusions drawn from them explicitly rather than asserted. It also records what was found by attacking the first guard, what has no demonstration behind it, and that the filter is now the weakest link.

Leaflet images left exactly as they were; the note that default markers or a layers control open the same condition is worth carrying to whoever does that, and is not a change here.

CI: run #839 was cancelled by this push. Re-review against 5b61a02 / run #842.

All five actioned in `5b61a02`, and both laxness findings were confirmed real before fixing — the reviewed version publishes clean with `href="./logo_512.png"` and `href="/logo_512.png"`, shipping `logo_512.png` unfingerprinted. That is now two errors. - **Relevance filter** — `src/PlaceMark.WebUI/PlaceMark.WebUI.csproj` and `src/PlaceMark.WebUI/wwwroot` added, with the reason on the step's own comment. Nothing else depends on the filter's shape: `relevant` gates steps inside `container-images` only and is exported to no `outputs:` block or other job's `needs`; `publish` is untouched and still `true` for `push`/`workflow_dispatch` alone, so no CalVer counter or registry write follows from widening it. Cost is CI minutes on WebUI-touching PRs. - **Normalisation mismatch** — a rooted, `./` or `../` reference is now a build error rather than something the two loops disagree about. `<base href="/">` and the error UI's `<a href=".">` stay legal as the two literals. Ordered before the `Uri.TryCreate` check deliberately: on Unix `Uri` reads a leading slash as an absolute `file:` path and would have swallowed `/logo_512.png` (ADR-0155). - **Single quotes** — one `(?:href|src)=(?<quote>["'])(?<path>[^"']*)\k<quote>` shared by both loops, so they cannot diverge again. `href='css/theme.css'` now rewrites; it was previously invisible to both. - **Offset replace** — replaces the matched `path` group at its index, iterating backwards. Demonstrated by a quoted path inside an HTML comment, which the old whole-document `Replace` rewrote and this one leaves alone. I could not construct the import-map collision itself, because the SDK spells its keys `"./js/map.js"`; the dependency is removed rather than the fault observed, and the ADR says so. - **Absolute schemes / query / fragment** — `Uri.TryCreate(..., UriKind.Absolute, ...)` plus an explicit protocol-relative case; the path is truncated at the first `?` or `#` before the existence check. `mailto:` and `tel:` verified accepted. - **Target condition** — gone. `AfterTargets="Publish"` unconditionally, with an `<Error>` when `index.html` is absent. **ADR-0169** now carries all four `curl` transcripts in full — the `.css` response, `/appsettings.json` and `/lib/leaflet/LICENSE` keeping their `last-modified`, and `git.robware.uk` — with the three conclusions drawn from them explicitly rather than asserted. It also records what was found by attacking the first guard, what has no demonstration behind it, and that the filter is now the weakest link. Leaflet images left exactly as they were; the note that default markers or a layers control open the same condition is worth carrying to whoever does that, and is not a change here. CI: run #839 was cancelled by this push. Re-review against `5b61a02` / run #842.
Discard the unused Uri out parameter
All checks were successful
CI / build (pull_request) Successful in 3m18s
CI / container-images (pull_request) Successful in 2m20s
CI / e2e (pull_request) Successful in 4m23s
5f6f0aa509
Merge remote-tracking branch 'origin/main' into static-asset-cache-busting
All checks were successful
CI / build (pull_request) Successful in 5m10s
CI / container-images (pull_request) Successful in 3m51s
CI / e2e (pull_request) Successful in 5m21s
4c9ec69ce5
Author
Owner

Green on 4c9ec69 — run #845, all three jobs. container-images resolved relevant=true off src/PlaceMark.WebUI/PlaceMark.WebUI.csproj, and built all four images, so the guard actually executed in the WebUI and joint publishes.

origin/main (0ce99a7) merged in. That landed wwwroot/js/geolocation.js, which is a useful accident: it is fingerprinted to js/geolocation.w2f88ohs4f.js and mapped in the import map with no per-file work, and a real Chromium resolves await import("./js/geolocation.js") through it. Full suite green locally on the merged tree (2071 passed, 0 failed).

Run #842 on the previous head failed on Testcontainers timeouts shared with #840 and #841 in the same window — three concurrent runs, not this change.

Ready for re-review against 4c9ec69.

Green on `4c9ec69` — run #845, all three jobs. `container-images` resolved `relevant=true` off `src/PlaceMark.WebUI/PlaceMark.WebUI.csproj`, and built all four images, so the guard actually executed in the WebUI and joint publishes. `origin/main` (`0ce99a7`) merged in. That landed `wwwroot/js/geolocation.js`, which is a useful accident: it is fingerprinted to `js/geolocation.w2f88ohs4f.js` and mapped in the import map with no per-file work, and a real Chromium resolves `await import("./js/geolocation.js")` through it. Full suite green locally on the merged tree (2071 passed, 0 failed). Run #842 on the previous head failed on Testcontainers timeouts shared with #840 and #841 in the same window — three concurrent runs, not this change. Ready for re-review against `4c9ec69`.
rob left a comment

Verdict: changes needed

Re-reviewed at 4c9ec69. All five code findings are correctly closed and I have nothing further on the .csproj or ci.yml. What remains is three corrections in ADR-0169's evidence section — the section that opens by promising the four measurements "in full rather than summarised", and the reason I can hold nothing else against a design I could not otherwise verify. All prose, all one-liners.

The guard demonstrably ran. Not taken from the summary: the PR's own file list contains four of the filter's paths (PlaceMark.WebUI.csproj, src/PlaceMark.WebUI/docker, src/PlaceMark.Joint, .forgejo/workflows/ci.yml), so git diff --name-only origin/main HEAD -- <list> cannot come back empty and relevant=true is forced by the diff rather than reported. Every step after it is gated on that one output and nothing else, and run #845 concluded success — which for that job requires the four docker builds to have completed, each running dotnet publish, each running the target. A Log.LogError in a RoslynCodeTaskFactory fragment fails the task, so a miss could not have passed silently. I could not read the raw job log — Forgejo renders it client-side and it is not fetchable — but the claim does not rest on it.

I also confirmed the blast radius you checked: relevant appears only in step-level if:s, the job declares no outputs:, no job needs it, and publish still gates every registry and CalVer step. Widening the filter cannot reach the publish path.

geolocation.js is a fair incidental confirmation, of the SDK half specifically: CurrentLocationButton.razor.cs imports it as "./js/geolocation.js" and index.html never names it, so it is carried by the *.js pattern and the import map with no per-file work — which is the half that was already working. It exercises nothing in the task's own href/src rewrite. Worth having, worth not over-reading.

Honesty calibration in the ADR is right, not under-claimed. It separates the two demonstrated fixes from the two precautionary ones and says which is which; it names the CI filter as the weakest link; and the .csproj's "best-effort in the ADR-0066 sense" is paired with what the guard does guarantee — one pattern for both loops, and unlookupable spellings refused rather than waved through. That is stronger than a tripwire and the text says so without inflating it.

Verdict: changes needed Re-reviewed at `4c9ec69`. All five code findings are correctly closed and I have nothing further on the `.csproj` or `ci.yml`. What remains is three corrections in ADR-0169's evidence section — the section that opens by promising the four measurements "in full rather than summarised", and the reason I can hold nothing else against a design I could not otherwise verify. All prose, all one-liners. **The guard demonstrably ran.** Not taken from the summary: the PR's own file list contains four of the filter's paths (`PlaceMark.WebUI.csproj`, `src/PlaceMark.WebUI/docker`, `src/PlaceMark.Joint`, `.forgejo/workflows/ci.yml`), so `git diff --name-only origin/main HEAD -- <list>` cannot come back empty and `relevant=true` is forced by the diff rather than reported. Every step after it is gated on that one output and nothing else, and run #845 concluded `success` — which for that job requires the four `docker build`s to have completed, each running `dotnet publish`, each running the target. A `Log.LogError` in a `RoslynCodeTaskFactory` fragment fails the task, so a miss could not have passed silently. I could not read the raw job log — Forgejo renders it client-side and it is not fetchable — but the claim does not rest on it. I also confirmed the blast radius you checked: `relevant` appears only in step-level `if:`s, the job declares no `outputs:`, no job `needs` it, and `publish` still gates every registry and CalVer step. Widening the filter cannot reach the publish path. `geolocation.js` is a fair incidental confirmation, of the SDK half specifically: `CurrentLocationButton.razor.cs` imports it as `"./js/geolocation.js"` and `index.html` never names it, so it is carried by the `*.js` pattern and the import map with no per-file work — which is the half that was already working. It exercises nothing in the task's own `href`/`src` rewrite. Worth having, worth not over-reading. Honesty calibration in the ADR is right, not under-claimed. It separates the two demonstrated fixes from the two precautionary ones and says which is which; it names the CI filter as the weakest link; and the `.csproj`'s "best-effort in the ADR-0066 sense" is paired with what the guard does guarantee — one pattern for both loops, and unlookupable spellings refused rather than waved through. That is stronger than a tripwire and the text says so without inflating it.
@ -0,0 +62,4 @@
earlier in the day is reused with no request until that boundary passes: up to roughly twelve hours
of a browser cheerfully rendering last night's CSS. That is nginx's own `expires` directive, which
clears any `Cache-Control` already on the response before writing its own — the same reason the
origin's `Last-Modified` is gone.
Author
Owner

expires is not what removed Last-Modified, and this is the one causal claim in the record that is wrong. ngx_http_headers_module's expires sets Expires and Cache-Control and does not touch Last-Modified — the first half of the sentence is right and the second half does not follow from it.

The transcripts themselves argue against it: /css/app.css comes back with a strong etag: "6a86b78f-1b89", and nginx's mtime-size ETag shares its mtime word with /lib/leaflet/LICENSE's "6a86b78f-559", whose last-modified is present. So the origin computed both headers from the same mtime and something downstream removed one while keeping the other — selective header clearing (openresty bundles headers-more; proxy_hide_header does it too), not expires.

This strengthens the decision rather than weakening it: a rule that explicitly clears an origin header is even less deferential to one this repository sets than expires alone would be. Just drop the "the same reason" clause and say the removal is a separate, deliberate act whose directive is not observable from here.

`expires` is not what removed `Last-Modified`, and this is the one causal claim in the record that is wrong. `ngx_http_headers_module`'s `expires` sets `Expires` and `Cache-Control` and does not touch `Last-Modified` — the first half of the sentence is right and the second half does not follow from it. The transcripts themselves argue against it: `/css/app.css` comes back with a *strong* `etag: "6a86b78f-1b89"`, and nginx's `mtime-size` ETag shares its mtime word with `/lib/leaflet/LICENSE`'s `"6a86b78f-559"`, whose `last-modified` is present. So the origin computed both headers from the same mtime and something downstream removed one while keeping the other — selective header clearing (openresty bundles headers-more; `proxy_hide_header` does it too), not `expires`. This strengthens the decision rather than weakening it: a rule that explicitly clears an origin header is even less deferential to one this repository sets than `expires` alone would be. Just drop the "the same reason" clause and say the removal is a separate, deliberate act whose directive is not observable from here.
@ -0,0 +64,4 @@
clears any `Cache-Control` already on the response before writing its own — the same reason the
origin's `Last-Modified` is gone.
**The rule is per-host and matched by extension, not global.** `git.robware.uk` sits behind the
Author
Owner

Per-host is shown — git.robware.uk/assets/css/index.css is the right control, same extension, same openresty, headers through untouched. By-extension is inferred, and the four responses do not discriminate it from a path rule: /css/app.css is rewritten and /lib/leaflet/LICENSE and /appsettings.json are not, which a location /css/ block fits exactly as well as an extension list does.

It changes no decision — any of those rules defeats a header fix — but the heading states it as measured. One more curl -sSI https://places.robware.uk/lib/leaflet/leaflet.css closes it outright: same directory as the untouched LICENSE, so a rewrite there can only be the extension. Either add it or soften the wording to "selectively, by something the responses narrow to extension or path".

Per-host is shown — `git.robware.uk/assets/css/index.css` is the right control, same extension, same openresty, headers through untouched. By-extension is inferred, and the four responses do not discriminate it from a path rule: `/css/app.css` is rewritten and `/lib/leaflet/LICENSE` and `/appsettings.json` are not, which a `location /css/` block fits exactly as well as an extension list does. It changes no decision — any of those rules defeats a header fix — but the heading states it as measured. One more `curl -sSI https://places.robware.uk/lib/leaflet/leaflet.css` closes it outright: same directory as the untouched `LICENSE`, so a rewrite there can only be the extension. Either add it or soften the wording to "selectively, by something the responses narrow to extension or path".
@ -0,0 +184,4 @@
Two further changes have no such demonstration behind them, because neither was reachable from the
current `index.html`: the rewrite now replaces at the matched offset rather than every occurrence
of the same quoted string (proved to matter only by a quoted path in an HTML comment, which the old
Author
Owner

True of a bare quoted path in a comment, which is presumably what was tried; not true of a commented-out <link href="css/app.css">, which still matches AttributeReference, is still rewritten, and is still subject to the existence check — so commenting out a <link> whose target was also deleted now fails the publish. Harmless, and within the best-effort envelope the guard declares, but "leaves alone" reads wider than it is, and an over-read sentence in an ADR is the failure mode this record was written to correct in ADR-0104. Say "a quoted path outside an attribute".

True of a bare quoted path in a comment, which is presumably what was tried; not true of a commented-out `<link href="css/app.css">`, which still matches `AttributeReference`, is still rewritten, and is still subject to the existence check — so commenting out a `<link>` whose target was also deleted now fails the publish. Harmless, and within the best-effort envelope the guard declares, but "leaves alone" reads wider than it is, and an over-read sentence in an ADR is the failure mode this record was written to correct in ADR-0104. Say "a quoted path outside an attribute".
Name what the proxy evidence actually shows about Last-Modified
All checks were successful
CI / build (pull_request) Successful in 5m51s
CI / container-images (pull_request) Successful in 4m38s
CI / e2e (pull_request) Successful in 6m39s
a02d5f532e
Author
Owner

All three ADR findings actioned in a02d5f5. Documentation only — no code changed.

1. Mechanism corrected. You are right that expires does not touch Last-Modified, and the ADR no longer says it does. It now says plainly that something separate removes the header, lists the candidates (proxy_hide_header, headers-more, or the proxy's equivalent) and states that this record does not name which, because the configuration is not in this repository and nothing observable from outside distinguishes them. The expires paragraph is now confined to what expires actually does.

The ETag argument you spotted is now arithmetic rather than a shared prefix: nginx builds a static ETag as "<hex mtime>-<hex size>", and 0x6a86b78f decodes to Thu, 20 Aug 2026 08:15:11 GMT — to the second, the last-modified that /lib/leaflet/LICENSE still arrives with. Four responses share that word; one keeps the header.

2. By-extension now shown, not inferred. Your suggested curl, plus a third that makes the path hypothesis untenable rather than merely unlikely — all three pasted into the ADR:

$ curl -sSI https://places.robware.uk/lib/leaflet/leaflet.css
etag: "6a86b78f-3741"
expires: Thu, 20 Aug 2026 23:30:00 GMT
cache-control: max-age=13959

$ curl -sSI https://places.robware.uk/lib/leaflet/LICENSE
last-modified: Thu, 20 Aug 2026 08:15:11 GMT
etag: "6a86b78f-559"

$ curl -sSI https://places.robware.uk/lib/leaflet/images/marker-icon.png
etag: "6a86b78f-5ba"
expires: Thu, 20 Aug 2026 23:30:00 GMT
cache-control: max-age=13958

leaflet.css and images/marker-icon.png are rewritten; LICENSE, in the same directory as the first and a parent of the second, is untouched. Any path-prefix rule catches all three or none.

3. Wording narrowed. The claim now says the only reachable difference is a path quoted in HTML prose, and states explicitly that a commented-out <link href="..."> is still an href, still rewritten and still guarded.

Run #845 remains the last CI result on the code; this push is markdown only, so a fresh run is a formality — I will report its number when it lands.

All three ADR findings actioned in `a02d5f5`. Documentation only — no code changed. **1. Mechanism corrected.** You are right that `expires` does not touch `Last-Modified`, and the ADR no longer says it does. It now says plainly that something separate removes the header, lists the candidates (`proxy_hide_header`, `headers-more`, or the proxy's equivalent) and **states that this record does not name which**, because the configuration is not in this repository and nothing observable from outside distinguishes them. The `expires` paragraph is now confined to what `expires` actually does. The ETag argument you spotted is now arithmetic rather than a shared prefix: nginx builds a static ETag as `"<hex mtime>-<hex size>"`, and `0x6a86b78f` decodes to `Thu, 20 Aug 2026 08:15:11 GMT` — to the second, the `last-modified` that `/lib/leaflet/LICENSE` still arrives with. Four responses share that word; one keeps the header. **2. By-extension now shown, not inferred.** Your suggested `curl`, plus a third that makes the path hypothesis untenable rather than merely unlikely — all three pasted into the ADR: ``` $ curl -sSI https://places.robware.uk/lib/leaflet/leaflet.css etag: "6a86b78f-3741" expires: Thu, 20 Aug 2026 23:30:00 GMT cache-control: max-age=13959 $ curl -sSI https://places.robware.uk/lib/leaflet/LICENSE last-modified: Thu, 20 Aug 2026 08:15:11 GMT etag: "6a86b78f-559" $ curl -sSI https://places.robware.uk/lib/leaflet/images/marker-icon.png etag: "6a86b78f-5ba" expires: Thu, 20 Aug 2026 23:30:00 GMT cache-control: max-age=13958 ``` `leaflet.css` and `images/marker-icon.png` are rewritten; `LICENSE`, in the same directory as the first and a parent of the second, is untouched. Any path-prefix rule catches all three or none. **3. Wording narrowed.** The claim now says the only reachable difference is a path quoted in HTML *prose*, and states explicitly that a commented-out `<link href="...">` is still an `href`, still rewritten and still guarded. Run #845 remains the last CI result on the code; this push is markdown only, so a fresh run is a formality — I will report its number when it lands.
Author
Owner

Run #847 green on a02d5f5build, e2e and container-images all success. Ready for re-review.

Run **#847** green on `a02d5f5` — `build`, `e2e` and `container-images` all success. Ready for re-review.
rob left a comment

Verdict: mergeable

Re-reviewed at a02d5f5. All three ADR findings are closed and nothing new. No inline comments — there is nothing left to act on.

Both load-bearing steps check out independently:

  • The ETag decode is exact. 0x6a86b78f = 1787213711 = Thu, 20 Aug 2026 08:15:11 GMT, to the second the last-modified /lib/leaflet/LICENSE arrives with. The record under-sells it: /appsettings.json corroborates the same identity on a different mtime — 0x6a820c19 = Sun, 16 Aug 2026 19:14:33 GMT, again exact — so the "<hex mtime>-<hex size>" reading is confirmed twice on independent data rather than once. Sizes are consistent too (0x3741 = 14145 for leaflet.css, 0x5ba = 1466 for marker-icon.png, 0x559 = 1369 for LICENSE). The four max-age/expires pairs all land on 23:30:00 exactly, including the two new ones at 13959 and 13958 from 19:37:21 and 19:37:22.
  • The three-response argument does rule out a path rule, completely. Any prefix matching /lib/leaflet/leaflet.css also matches /lib/leaflet/LICENSE, which is untouched; the only prefix that reaches marker-icon.png while sparing LICENSE is /lib/leaflet/images/, which then misses leaflet.css. No prefix produces that split, so "would have caught all three or none" is right. The one alternative the data still cannot separate from extension is content-type — and on a static nginx origin content-type is derived from the extension, so it is a distinction without a consequence.

Declining to name the directive that removes Last-Modified is the right call, not a gap. The responses establish that the removal happens, that it is selective, and that it tracks the re-dating exactly; they cannot distinguish proxy_hide_header from headers-more from anything else, and a record that guessed would be a claim the next person could not check — which is the failure this ADR was written to correct in ADR-0104. Naming the candidates as candidates and stopping is the honest shape.

Merge without rebasing. Forgejo reports mergeable: true against the moved base (1c39408), the ADR index on main still ends at 0168 so the one added row applies cleanly, and there is no semantic conflict either: #202's ModalOverlayStylesheetTests and ShareInsecureContextReportedTests read source files, which fingerprinting does not touch — only the published output is renamed. Worth stating plainly that run #847 validated this head against a pre-#202 main, so the two have never run together; the interaction surface is nil, which is why that does not warrant a rebase.

Verdict: mergeable Re-reviewed at `a02d5f5`. All three ADR findings are closed and nothing new. No inline comments — there is nothing left to act on. Both load-bearing steps check out independently: - The ETag decode is exact. `0x6a86b78f` = 1787213711 = `Thu, 20 Aug 2026 08:15:11 GMT`, to the second the `last-modified` `/lib/leaflet/LICENSE` arrives with. The record under-sells it: `/appsettings.json` corroborates the same identity on a *different* mtime — `0x6a820c19` = `Sun, 16 Aug 2026 19:14:33 GMT`, again exact — so the `"<hex mtime>-<hex size>"` reading is confirmed twice on independent data rather than once. Sizes are consistent too (`0x3741` = 14145 for `leaflet.css`, `0x5ba` = 1466 for `marker-icon.png`, `0x559` = 1369 for `LICENSE`). The four `max-age`/`expires` pairs all land on 23:30:00 exactly, including the two new ones at 13959 and 13958 from 19:37:21 and 19:37:22. - The three-response argument does rule out a path rule, completely. Any prefix matching `/lib/leaflet/leaflet.css` also matches `/lib/leaflet/LICENSE`, which is untouched; the only prefix that reaches `marker-icon.png` while sparing `LICENSE` is `/lib/leaflet/images/`, which then misses `leaflet.css`. No prefix produces that split, so "would have caught all three or none" is right. The one alternative the data still cannot separate from extension is content-type — and on a static nginx origin content-type is derived from the extension, so it is a distinction without a consequence. Declining to name the directive that removes `Last-Modified` is the right call, not a gap. The responses establish that the removal happens, that it is selective, and that it tracks the re-dating exactly; they cannot distinguish `proxy_hide_header` from `headers-more` from anything else, and a record that guessed would be a claim the next person could not check — which is the failure this ADR was written to correct in ADR-0104. Naming the candidates as candidates and stopping is the honest shape. **Merge without rebasing.** Forgejo reports `mergeable: true` against the moved base (`1c39408`), the ADR index on `main` still ends at 0168 so the one added row applies cleanly, and there is no semantic conflict either: #202's `ModalOverlayStylesheetTests` and `ShareInsecureContextReportedTests` read *source* files, which fingerprinting does not touch — only the published output is renamed. Worth stating plainly that run #847 validated this head against a pre-#202 `main`, so the two have never run together; the interaction surface is nil, which is why that does not warrant a rebase.
rob merged commit 01c2a449cc into main 2026-08-20 20:00:52 +00:00
rob deleted branch static-asset-cache-busting 2026-08-20 20:00:52 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
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
rob/PlaceMark!204
No description provided.