Conversation
* Let CI assemble a release, and run the script that assembles it make-release.sh has two callers and the last change only considered one. It began requiring git-restore-mtime on PATH, which CI cannot satisfy: the workflows restore with a pinned action that runs the tool from the action's own directory. The staging deploy failed at the assemble step with the tool genuinely absent, before any transport, so nothing was uploaded and nothing flipped. MTIME_RESTORED lets a caller that restored already skip the restore. It skips the work and never the assertion, so a caller setting it without having restored fails the same check as one that never tried. Verified all three ways, with a PATH carrying hugo and no mtime tool: knob set, tool absent, tree restored installs, exit 0 no knob, tool absent the failure that broke the deploy knob set, tree NOT restored assertion fails, exit 1 The second half is why this reached a deploy at all. validate-task.yml linted make-release.sh and never ran it, so the only thing that executed it was a dispatch-only workflow, and a broken caller contract could not be seen by any pull request. shellcheck cannot see that class: the defect was in what the script demanded of its caller, not in its syntax. Validation now assembles a release into a scratch root, which exercises the deploy's path short of the transport: the mtime contract, the build, the URL gate, the precompression, the staging tree and the release stamp. The root is empty, so no previous release exists and the hard-link guard does not apply. That step reproduces case A above, so this specific defect now fails on the pull request that introduces it. Its checkout takes fetch-depth: 0, since the restore has no commit to date a file from without history. Comments in the touched files are cut back to the house style: one sentence per line, no wrapped prose, no incident narrative. Several had grown across the day's edits, which is the creep the style rule names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Name what MTIME_RESTORED skips, and what it does not The comment, the log line and the reference row all said the knob skips the work and not the assertion, which reads as a distinction without a stated boundary. Each now says the two effects plainly: no restore is run and the tool is not required, while the assertion runs regardless. The log line said the mtimes were restored by the caller, which describes a state rather than the decision being taken. It now says the restore is being skipped and the assertion still follows. Found by Copilot review on #79. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Make the release assembly the build gate, not a second one The assemble step re-ran the two most expensive things the job had already done: make-release.sh does rm -rf public, rebuilds with the same --panicOnWarning, and re-runs check-url-parity. Measured on this branch, the job went from 38s to 48s for no additional coverage. The standalone build and URL-contract steps are removed and the script is the gate. It runs the identical commands, so nothing is lost, and the reasoning that belonged to those steps moves to where the work now happens. The alternative was a knob letting the script reuse an existing public/ in CI. That was rejected: it makes the script behave differently under CI than locally, and a CI-only divergence in this script is precisely what broke the deploy this pull request is fixing. Found by Copilot review on #79, as a suppressed comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR promotes the already-reviewed develop changes (from #79) to main to restore the broken deploy path by fixing the make-release.sh caller contract around git-restore-mtime, and by exercising that contract during validation (so regressions fail in PR CI rather than during deploy).
Changes:
- Add and document
MTIME_RESTORED=1as an explicit caller contract so workflows that already restoredstatic/mtimes can skipmake-release.sh’s internal restore while still running the mtime assertion. - Update validation to restore mtimes and run
deploy/make-release.sh(including Hugo build + URL contract checks) to prevent deploy-only failures from slipping past CI. - Update the deploy workflow to set
MTIME_RESTORED=1when callingmake-release.sh, aligning it with the pinned restore action behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
ENVIRONMENT.md |
Documents the new MTIME_RESTORED knob and its exact behavior/guarantees. |
deploy/make-release.sh |
Implements MTIME_RESTORED handling to skip only the restore step while keeping the assertion gate. |
checks/check-env-docs.py |
Adds MTIME_RESTORED to the KNOBS set so env-doc validation stays complete. |
.github/workflows/validate-task.yml |
Restores mtimes and runs make-release.sh during validation to catch caller-contract regressions. |
.github/workflows/deploy-site-task.yml |
Sets MTIME_RESTORED=1 when invoking make-release.sh after restoring mtimes via the pinned action. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes
developtomain. One commit, already reviewed and squash-merged todevelopas #79.maincurrently cannot deploy. The previous promotion carried a regression that fails the deploy at the assemble step, so this is the promotion that makes a deploy possible again rather than an incremental one.The regression this fixes
deploy/make-release.shhas two callers, and the change that madegit-restore-mtimemandatory only considered one. CI restores with a pinned action that runs the tool from the action's own directory, so it is genuinely absent fromPATHthere:Nothing was uploaded and nothing flipped. The failure landed before the deploy key was installed, which is the two-phase design working.
MTIME_RESTOREDlets a caller that has already restored skip the restore only. The assertion always runs, so a caller that sets it without having restored fails the same check as one that never tried. Demonstrated three ways against aPATHcarrying hugo and no mtime tool:Why it reached a deploy, which is the part worth carrying
validate-task.ymllintedmake-release.shand never ran it. The only thing that executed it was a dispatch-only workflow, so a broken caller contract was invisible to every pull request.shellcheckcannot see that class: the defect was in what the script demanded of its caller rather than in its syntax.Validation now assembles a release into a scratch root, and that step is the build and URL-contract gate rather than a second one beside it, since the script runs
hugo --panicOnWarningandcheck-url-parityitself. Confirmed on #79's own CI run: the assemble step took theMTIME_RESTOREDpath, built, passed the contract, and installed a release into the scratch root.A knob letting the script reuse an existing
public/under CI was rejected. It would make the script behave differently in CI than locally, and a CI-only divergence in this exact script is what caused the failure being fixed.Cost, measured rather than asserted
Validation is about 15 seconds slower. Per-step timings, since a first attempt at attributing this was wrong and corrected on #79:
The increase is the price of executing the release script rather than only linting it, not of building twice.
Merge
Plain
--merge, never--delete-branch: on adevelop -> mainpromotion that flag deletesdevelop.Nothing publishes on this merge. After it, a deploy from
mainbecomes possible again, and the first restored deploy is still expected to link zero for the reason recorded inTODO.md.