Record the container appdata path and re-derive the redirect table - #6
Conversation
Two gaps, both surfaced by asking why `secrets/.env` carried only two values. **The appdata path was recorded nowhere.** The container reads three host paths and a release writes only one. `$DEPLOY_ROOT` holds the bundle, while the bootstrap Caddyfile and Caddy's persistent state live under a separate appdata root that no script touches and nothing documented. The bootstrap is outside the bundle deliberately, since rules held there would leave a rolled-back site served by the current release's redirects, but that also means no release refreshes it and a rebuild depended on someone remembering where it goes. `CADDY_APPDATA` now records it in `secrets/.env`, and OPERATIONS.md carries the install command and the mount table. The value stays out of committed files, because this repo names no host path. The deployed bootstrap had drifted from the repo's copy, carrying the pre-rewrite comments and a stale rule count. Functionally identical, the `import` line matched exactly, so nothing was broken. Now byte-identical, and the site was re-verified after the restart at 1245 URLs honored. **The redirect table was wrong in more than its count.** It claimed 11 regex rules where the Caddyfile carries 13 `redir` directives reading 5 map files through 3 `map` blocks. Three destinations were also stale: the date archives, their pagination, and the Blogger monthly archives were all documented as redirecting to `/` when the config sends them to `/all/`. `labels.map` and `terms.map` appeared nowhere. The table is rebuilt against the config and keyed on the Caddy matcher names rather than invented R numbers, so a reader can grep the Caddyfile instead of trusting the doc. Every row was checked: the classes sum to 917 exactly, matching `redirect-urls.txt`, with the 778 legacy image URLs called out as gated separately. `@label` is documented as deliberately outside the contract, since that class is a soft 404 rather than a redirect. Clears the backlog item added when #5 dropped the count rather than guessing at it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Not ready to approve
The updated documentation contains a couple of statements/table conventions that can mislead operators about how releases affect active Caddy redirect/map configuration without an explicit reload/restart.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Updates the operational and deploy documentation for the blog’s Caddy-based deployment by recording the container appdata (CADDY_APPDATA) bootstrap location and aligning the documented redirect table with the actual Caddy matcher/directive structure.
Changes:
- Documented the bootstrap Caddyfile placement and the rationale for keeping it outside the release bundle, including how
CADDY_APPDATArelates to/configand/data. - Reworked
deploy/README.md’s redirect documentation to reflect matcher names / directive structure and map usage. - Extended
deploy/env.exampleto includeCADDY_APPDATAand clarify environment variable intent.
File summaries
| File | Description |
|---|---|
| TODO.md | Removes a completed backlog item related to re-deriving redirect rule counts. |
| OPERATIONS.md | Adds a section documenting the bootstrap Caddyfile and records why CADDY_APPDATA must be tracked out-of-band. |
| deploy/README.md | Updates the redirect table and surrounding explanation to match the current Caddy config structure (matchers, maps, directives). |
| deploy/env.example | Documents and adds CADDY_APPDATA to the env template. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Review caught a claim that was not just wrong but inverted, and testing it against the running mirror found a second instance that was worse. Caddy expands `import` at config-parse time, for the site config and for the `map` blocks reading `maps/*.map`, and does not watch those files. Only static file requests follow the `current` symlink per request. Proven on the mirror: a map entry present in the live release on disk answered 404 until the container restarted, then 301. That makes the verification loop unsound as it shipped. Change a redirect, release, check without reloading, and the check exercises the previous rules, so a broken redirect reports PASS while the artifact is broken. That is exactly the failure the loop exists to catch, so the restart is now a step in it rather than a footnote. The rollback procedure was the worse instance. It said "no restart and no reload", which would revert the content while leaving the previous release served by the current release's redirects, the precise mismatch that shipping config inside the bundle exists to prevent. Also proven: rolling back to a release containing the probe still answered 404 until the restart, then 301. The container name moves to `CADDY_CONTAINER` rather than being hardcoded in the docs, since this repo names no host value in a committed file. `deploy/env.example` now states the naming convention, which is that the prefix names whatever owns the value rather than whatever reads it. HUGO_ is fixed by Hugo's own env mapping, DEPLOY_ is the release tooling that writes the root and is also the CI variable for the SSH deploy, and CADDY_ is container state a release never touches. The set was already consistent under that rule and nothing said so. Also clarifies the redirect table per review: the thirteen classes sum to 917 and `@uploads` is excluded deliberately, with each repeated `@mapped` row naming its map file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Not ready to approve
OPERATIONS.md’s Redirects intro still states “eleven regular-expression rules,” which is now inconsistent with the verified 13 redir directives in deploy/Caddyfile.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
OPERATIONS.md:138
- The Redirects intro still says redirects are “satisfied by eleven regular-expression rules”, but
deploy/Caddyfilenow contains 13redirdirectives (and not all are regex-based). This makes OPERATIONS.md internally inconsistent with the current config and with the updated deploy/README.md.
`CADDY_APPDATA` is recorded in `secrets/.env` for exactly this reason. No script reads it, so a rebuild would otherwise depend on someone remembering where the bootstrap goes.
## Redirects
The site answers roughly a thousand addresses it does not render. They are satisfied by eleven regular-expression rules and five map files, all inside the bundle.
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Re-deriving the count in deploy/README.md fixed one instance of it and left two, because the number was never grepped for across the repo. Both found by review. OPERATIONS.md still opened its Redirects section with "eleven regular-expression rules and five map files". It now states the verified figures and points at deploy/README.md for the per-class breakdown, so the two files stop restating each other and there is one place for a count to go stale. The blog post carried the same wrong figure, and a worse claim beside it: that rolling back "cannot leave yesterday's site being served by today's rules". That is exactly what a rollback does without a reload, as proven on the mirror earlier in this branch. A published post giving that advice would be actively misleading, so it now carries the catch, the way it was found, and the general lesson that an atomic swap is only atomic for whatever reads through it per request. Verified after the change with the full documented loop, release then restart then check: 1245 URLs honored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 1, 2026
Answering the suppressed comment on d15e55b, which has no thread. Accepted, and it found a third instance. The root cause is mine: when I re-derived the count I fixed the file I was editing and never grepped for the number elsewhere. Grepping afterwards found it in two more places, not one. The second was the blog post, which carried the same figure plus a worse claim beside it: that rolling back "cannot leave yesterday's site being served by today's rules". That is precisely what a rollback does without a reload, as proven on the mirror earlier in this branch. It would have shipped as public advice, so the post now carries the catch, how it was found, and the general lesson.
Verified after the change with the full documented loop, release then restart then check: |
There was a problem hiding this comment.
🟡 Not ready to approve
There’s at least one documentation statement whose “false pass” wording is currently unconditional but only accurate when redirect/config inputs changed, and it should be qualified to avoid misleading operational guidance.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
OPERATIONS.md:55
- The wording “omitting it produces a false pass” reads as unconditional, but it’s only false when the release changed Caddy config or map files. If you’re deploying content-only changes, skipping the restart still exercises the same redirect rules, so the pass is not inherently false. Consider qualifying the statement while still keeping the restart as the recommended default.
**The restart is not optional, and omitting it produces a false pass.** Caddy expands `import` at config-parse time, both for the site config and for the `map` blocks that read `maps/*.map`. It does not watch those files. Swapping the `current` symlink therefore changes what a *static file* request resolves to, per request, but the redirect rules and map tables stay exactly as they were when Caddy last loaded. Verified against the running mirror: a new map entry present in the live release on disk returned 404 until the container was restarted, then 301.
That is the failure this whole loop exists to catch, so it is worth being blunt about. Change a redirect, release, and check without reloading, and the check exercises the **previous** rules. A broken redirect reports `PASS` while the shipped artifact is broken.
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The warning read as unconditional. It is not: a content-only release leaves the Caddyfile and the maps untouched, so the rules Caddy already holds are still the right ones and a check without a restart is honest. The false pass is specific to a release that changed `deploy/Caddyfile` or anything under `deploy/maps/`. The step stays unconditional regardless, and now says why rather than overstating the risk. Deciding correctly means knowing whether anything reached the config, which is easy to get wrong when a change spans several paths or a map was regenerated as a side effect, and getting it wrong is silent because the wrong answer is a green check rather than an error. A few seconds of restart is cheaper than that reasoning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 1, 2026
Answering the suppressed comment on 4c22267. Accepted, the wording was unconditional and the risk is not. Fixed in 31d5e87. You are right that a content-only release leaves The step stays unconditional, but now says why rather than overstating the risk: deciding correctly means knowing whether anything reached the config, which is easy to get wrong when a change spans several paths or a map was regenerated as a side effect, and getting it wrong is silent because the wrong answer is a green check rather than an error. A few seconds of restart is cheaper than that reasoning each time. |
There was a problem hiding this comment.
🟢 Ready to approve
The updated documentation and env template match the repository’s actual Caddy configuration and checked contract counts, and no inconsistencies were found in the changed regions.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Two gaps, both surfaced by asking why
secrets/.envcarried only two values.The appdata path was recorded nowhere
The container reads three host paths and a release writes only one:
$DEPLOY_ROOT/srv/blog, read-only$CADDY_APPDATA/config/config$CADDY_APPDATA/data/dataThe bootstrap is outside the bundle deliberately, since rules held there would leave a rolled-back site served by the current release's redirects. But that also means no release refreshes it, and a rebuild depended on someone remembering where it goes.
CADDY_APPDATAnow records it insecrets/.env, with the install command and mount table inOPERATIONS.md. The value stays out of committed files, because this repo names no host path.The deployed bootstrap had drifted from the repo's copy, carrying the pre-rewrite comments and a stale rule count. Functionally identical (the
importline matched exactly), so nothing was broken. Now byte-identical, and the site was re-verified after the restart:PASS - 1245 URLs honored.The redirect table was wrong in more than its count
It claimed 11 regex rules. The Caddyfile carries 13
redirdirectives reading 5 map files through 3mapblocks. Three destinations were also stale:-> /, config sends-> /all/-> /all/labels.mapandterms.mapappeared nowhere.Rebuilt against the config and keyed on the Caddy matcher names rather than invented R numbers, so a reader can grep the Caddyfile instead of trusting the doc. Every row checked: the classes sum to 917 exactly, matching
redirect-urls.txt, with the 778 legacy image URLs called out as gated separately bygolden-media-legacy.txt.@labelis documented as deliberately outside the contract, since that class is a soft 404 rather than a redirect.Clears the backlog item added when #5 dropped the count rather than guessing at it.