Standardize the static-site-deploy repo type and its destination - #560
Conversation
Blog has been running a Hugo build with an rsync-over-SSH deploy to a host we own, governed by nothing: the CI contract had no destination for a filesystem on our own host, the registry target enum had no member for it, and Blog stood cataloged as source-only behind two interim driftNotes. This closes#456's intake and #558's spec change. Add the hugo type with nine checks. Only hugo.build.strict names the generator, where the flag is the letter. The rest are phrased so that promoting them to a shared type when a second generator arrives is a registry edit, per the new type-model "Generators" section. What a repo builds and where the result lands stay separate axes, so the transport is the publish target rather than part of the type. Add the self-hosted target and the deploy-ssh mechanism. Both requires and stores are empty deliberately: the credentials are per-environment GitHub Environment secrets, which validate.py and audit.py cannot enumerate, and listing them would make the audit demand them in the repository actions store and report a correctly configured repo as missing them. The optional environments block records the names as operator documentation, and says plainly that it is not a gate. Extending the stores enum instead would raise KeyError in audit.py for every repo using the mechanism. Add D4.6, which asserts which release and which environment answered rather than that the transport exited zero, and D5.6, which requires the prune of a durable destination to be asserted on the host. D4.3 is unchanged: the deploy is a separate dispatch that touches no release, so it adds no fourth route to the tag-only shape. Reclassify Blog to hugo plus source-only with both publish targets. Three checks fail against it today and are recorded as driftNotes naming their check id: nothing prunes the remote release tree, the vendored theme records no upstream ref, and the generator pin is duplicated across two workflows. The reference leaf carries the prune the repo lacks, so it leads its only member by one step. Correct TODO.md rather than deleting it, since the intake predicted a submodule theme, an unpinned generator, and a tag cut after the deploy, and all three are wrong against what runs. Drive-by: scope-model.md was missing cpp from the token table, and STANDUP's new-type procedure never mentioned the registry target enum, which is what the first repo declaring a new destination fails validate.py on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first draft of hugo.deploy.retention and D5.6 required the deploy to prune the destination and assert the count on the host. Blog cannot, and should not be made to: its deploy credential is a forced rsync command confined write-only, so the server never acts as sender and the key can neither delete nor read the destination back. Blog's own ownership table already assigns release-prune timers to the host, which is the correct resolution rather than a gap. As written the check was unsatisfiable for exactly the repos that confine their credentials properly, and the only way to pass it was to widen a deliberately narrow key. That trades a real confinement boundary for a check, so the check was wrong rather than the design. Retention is now bounded by a declared count with one side recorded as owning the prune: the deploy asserts it where its credential can observe the destination, and the host owns it where the credential cannot. What the guarantee still rejects is a prune against a local scratch tree, a best-effort prune, and neither side owning it, since each then assumes the other prunes. The reference leaf keeps the assert-in-pipeline shape and says when to delete the step. Blog passes the corrected check, so its retention driftNote is dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Standardizes governance and spec support for deploying static-site artifacts to a filesystem on a project-owned host, including a first-class hugo generator type, a self-hosted publish target mapped to an SSH-based deploy mechanism, and reference workflow snippets that implement deploy verification and on-host retention guarantees.
Changes:
- Add
self-hostedas a registry publish target and map it to a newdeploy-sshmechanism (with environment-scoped secrets documented but not audited). - Introduce the
hugoproject type plus accompanying model/spec updates (scope/type model, conformance matrix, audit/type detection notes). - Add reference deploy workflows (
deploy-site.yml,deploy-site-task.yml) and extend WORKFLOW/GOVERNANCE contracts with deploy verification (D4.6) and destination pruning (D5.6).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| WORKFLOW.md | Adds the “filesystem on own host” destination row plus D4.6/D5.6 guarantees and hugo/static-site walkthrough/scenarios. |
| TODO.md | Updates backlog entries/record-keeping around secrets modeling and the new hugo/self-hosted work. |
| STANDUP.md | Extends the new-type procedure to include the registry target enum and naming constraints for build-*-task.yml. |
| spec/type-model.md | Adds a “Generators” section describing generator-type naming and promotion strategy. |
| spec/secrets.schema.json | Adds an environments documentation block shape for environment-scoped secrets/vars. |
| spec/secrets.json | Adds deploy-ssh and maps self-hosted to it; documents why env secrets can’t be audited yet. |
| spec/scope-model.md | Updates the project-type token table (adds cpp and hugo). |
| spec/project-types.json | Introduces the hugo type and its checks. |
| spec/files.json | Adds an interface baseline entry for .github/workflows/deploy-site-task.yml for hugo repos. |
| reports/conformance-matrix.md | Updates matrix rows for Blog and adds a self-hosted target row. |
| registry/repos.schema.json | Extends target enum with self-hosted. |
| registry/repos.json | Reclassifies Blog to ["hugo","source-only"] and adds self-hosted publish target + updated drift notes. |
| GOVERNANCE.md | Adds the deploy-to-own-host output seam description to the release model section. |
| catalog/snippets/workflows/README.md | Documents the new deploy entrypoint/task snippets. |
| catalog/snippets/workflows/deploy-site.yml | Adds dispatch entrypoint workflow with env choice, ref gate, shared validation, and env-keyed concurrency. |
| catalog/snippets/workflows/deploy-site-task.yml | Adds reusable deploy task implementing SSH+rsync deploy, on-host pruning, and live verification. |
| AUDIT.md | Updates type detection and applicability description to include hugo. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (1)
catalog/snippets/workflows/deploy-site-task.yml:155
- The "Flip current" step claims rsync replaces the symlink via temp+rename so it is never absent, but rsync does not guarantee atomic symlink replacement. This can leave a brief window where
currentis removed/recreated in-place, which risks transient 404s or serving a mixed release during a deploy.
Consider flipping current with an explicit remote atomic rename (create a new symlink, then mv -T it into place) instead of syncing the symlink with rsync.
# A separate step from the upload, so a failed transfer cannot half-publish a site.
# The pointer is relative, so one bundle works at any remote root.
# The transport replaces a symlink through a temporary and a rename, so it is never absent to a request in flight.
- name: Flip current step
env:
Copilot raised a collision between two deploys starting in the same second. The scenario as stated is not reachable: a multi-minute generator install precedes this step, and the environment-keyed concurrency group serializes deploys to one environment, so two ids a second apart cannot both be derived. The finding is still worth taking, for a reason it did not give. Both of those properties are incidental to where the step sits, and neither is stated anywhere. Moving the id derivation to the top of the job is an obvious refactor that would silently reintroduce the collision, and a colliding id is the one value the live check cannot tell apart, since it is simultaneously the directory name, the stamp in the shipped config, and the expectation. The run id and attempt make it unique by construction. The timestamp stays the prefix, so the id still reads chronologically and a name-sorted prune keeps working. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 5, 2026
Answering the suppressed review commentIt has no thread to reply in, so quoting it here. From the collapsed block on the current review, against
Declining: I traced it, and rsync does exactly the temp-plus-rename the comment says it does not guarantee. $ ln -sfn releases/old dst/current # before
$ ln -sfn releases/new src/current # what we are shipping
$ strace -f -e trace=symlink,rename,unlink rsync -a --no-recursive src/current dst/symlink("releases/new", ".current.0") = 0rename(".current.0", "current") = 0
$ readlink dst/currentreleases/newThere is no The suggested fix is also the same operation. Worth saying which part of the concern is real, though, because a neighbouring one is. Content and configuration do not go live together: a static file server resolves Low-confidence findings on this repo have historically been actionable, so this one got the same treatment as the two threaded ones rather than being taken on faith in either direction. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
catalog/snippets/workflows/deploy-site-task.yml:159
- Same issue as the upload step:
rsync -aincludes--owner/--groupand can fail under a non-root, forced-command deploy account. Disabling owner/group preservation makes the pointer-flip step robust on tightly confined keys.
rsync -a --no-recursive \
-e "ssh -i ~/.ssh/deploy -o IdentitiesOnly=yes" \
"${RUNNER_TEMP}/bundle/current" \
"${DEPLOY_SSH_USER}@${DEPLOY_SSH_HOST}:/${ENVIRONMENT}/"
WORKFLOW.md:135
- The Output Seam row implies the deploy always "prunes what it wrote to", but D5.6 explicitly allows retention to be host-owned when the deploy credential is deliberately write-only (no in-pipeline prune). This sentence should be phrased in terms of bounded/owned retention rather than an always-prune step.
- **Filesystem on a host the project owns** (a static site, a config tree): the leaf builds the tree, ships it to the host, and contributes no `release-asset-*`. The transport is the repo's own. What the contract fixes is that the deploy is a **separate `workflow_dispatch`** from the release, so a redeploy of an unchanged commit mints no tag and a host rebuild, a rollback, or proving a branch on a non-production environment costs nothing; that its credentials come from a **per-environment GitHub Environment** rather than the repository secret store; and that the deploy ends by asserting **what the host serves** rather than the transport's exit status (D4.6), then prunes what it wrote to (D5.6).
spec/secrets.json:3
- This note says feature workflow mechanisms are "detected by the mechanism's workflowNeeds action", but neither spec/audit.py nor spec/validate.py implements any workflow-content detection for featureMechanisms/workflowNeeds. As written, it overstates what the audit cross-checks today and could mislead operators relying on that detection.
"note": "Secrets the audit cross-checks. `baseline` applies to every fleet repo (the App-signed merge-bot runs everywhere). `mechanisms` are per-target/per-feature additions: a repo requires the baseline plus the mechanisms whose publish target (`targetMechanisms`) or feature workflow (`featureMechanisms`, detected by the mechanism's `workflowNeeds` action) it uses. Baseline secrets are implicit and are NOT repeated in a repo's registry `requiredSecrets`, which lists only the domain-specific additions. `typeMechanisms` are per-language requirements: a `csharp` or `python` repo must carry the mapped mechanism (codecov) regardless of opt-in. A configured secret that no applicable mechanism claims is a stale-secret finding; a present `forbids` secret is a defect. `environments`, where a repo carries it, lists the per-environment GitHub Environment secrets and variables its deploy needs. It is operator documentation rather than part of the mechanism audit: no tool reads it, because neither `spec/validate.py` nor `spec/audit.py` can enumerate an environment-scoped store, so a clean audit is not evidence that an environment is configured. `environmentSecrets` names what one environment carries and another does not, so a name audit does not read a single-environment credential as missing everywhere else.",
Uh oh!
There was an error while loading. Please reload this page.
…tector Two review findings, both accurate. The Output Seam row still ended "then prunes what it wrote to", which is what D5.6 said before the previous commit rewrote it. It now allows the prune to be host-owned where the deploy credential is confined write-only, so the row asserted a step the guarantee no longer requires. This is my own inconsistency from correcting one and not the other, and the row now states the bounded-and-owned shape. The secrets.json note claimed featureMechanisms is "detected by the mechanism's workflowNeeds action". Nothing implements that. workflowNeeds appears in neither audit.py nor validate.py, featureMechanisms is only shape-validated at validate.py:89-95, and the one feature it names is claimed through typeMechanisms at build profile instead (audit.py:596, audit.py:678). The note now says all three mappings resolve from the registry entry rather than from workflow content, and that workflowNeeds is documentation for a reader rather than a detector. That text predates this change, but the note is the region this change already edits, so it is fixed here rather than stepped around. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 5, 2026
Answering the three suppressed comments from the latest roundNone carry a thread, so quoting each. Two of the three are right and are fixed in 1. The seam row contradicts D5.6 — correct, fixed
Right, and this is my own inconsistency rather than a subtlety. D5.6 originally did require an in-pipeline prune. A later commit rewrote it, because Blog's deploy credential is a forced The row now reads:
Good catch on a cross-file inconsistency, which is the kind that survives review most easily. 2. The secrets note claims a detector that does not exist — correct, fixed
Also right, and I checked rather than took it:
So the note described a mechanism that has never existed, and your phrase for the risk is the right one: an operator could rely on detection that is not there. The note now says all three mappings resolve from the registry entry rather than from workflow content, that nothing reads a repo's Actions files to infer a mechanism, and that That text predates this pull request. Fixing it here rather than filing it, since the note is a region this change already edits. 3. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (4)
spec/files.json:33
deploy-site.ymlis part of the new standardized deploy surface (catalog snippet + WORKFLOW.md walkthrough), but it is not included in the baseline manifest. As a result,spec/audit.pywill not check that Hugo repos actually carry the dispatch entrypoint (ref gate + shared validation + call intodeploy-site-task.yml). Add.github/workflows/deploy-site.ymltospec/files.json(likely asfidelity: interface) alongsidedeploy-site-task.yml.
{ "path": ".github/workflows/deploy-site-task.yml", "fidelity": "interface", "contract": { "requiredJobKeys": ["assert-environment", "deploy"], "requireTokensInJob": { "deploy": ["environment:"] } }, "reference": "catalog/snippets/workflows/deploy-site-task.yml", "intentRef": "WORKFLOW.md#d4---release--publish", "appliesTo": ["hugo"] },
owntest/src/a.txt:1
- This looks like an ad-hoc test artifact ("hi") and is not referenced anywhere in the repo. Please delete it to avoid shipping stray files in the template and accidentally seeding them into downstream repos.
hi
owntest/dest/a.txt:1
- This looks like an ad-hoc test artifact ("hi") and is not referenced anywhere in the repo. Please delete it to avoid shipping stray files in the template and accidentally seeding them into downstream repos.
hi
owntest/dest2/a.txt:1
- This looks like an ad-hoc test artifact ("hi") and is not referenced anywhere in the repo. Please delete it to avoid shipping stray files in the template and accidentally seeding them into downstream repos.
hi
… mistake Two review findings, both right. The dispatch entry point was left out of the manifest on the reasoning that its caller shape has one member. That was wrong, and inconsistent with declaring the task leaf next to it. The entry point is where the ref gate lives, which is what stops production being deployed from an arbitrary branch, and it was the one security-relevant property of this surface with nothing mechanically checking it. It is now an interface entry requiring the assert-ref, validate and deploy job keys, and requiring the deploy job to call the task leaf with secrets: inherit. Blog satisfies it as it stands, so this records the shape rather than demanding a change. The three owntest/ files were rsync test fixtures. I ran the test without changing directory into the scratchpad first, so it built the tree in the repository root, and a later git add -A swept it into the commit. They are removed. This is exactly the failure the queued agent-isolation rule describes, where a command's working directory is implied rather than written into the command, and it is worth noting the mechanical fix is the same: the absolute path belongs inside the command, not in a preceding cd. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 5, 2026
Answering the four suppressed comments from round 4All four taken, in 1. The dispatch entry point is not in the manifest, so nothing checks the ref gate
Right, and my reasoning for leaving it out does not survive being written down. I excluded it because "the caller shape has one member", which is equally true of the task leaf sitting immediately below it in the same manifest, and I declared that one. The inconsistency is the tell. The substantive point is the one you make: the entry point is where the ref gate lives, and that gate is what stops production being deployed from an arbitrary branch. It was the only security-relevant property of this surface with nothing mechanically checking it, while the merely structural properties next to it were checked. Added at { "path": ".github/workflows/deploy-site.yml", "fidelity": "interface", "contract": { "requiredJobKeys": ["assert-ref", "validate", "deploy"], "requireTokensInJob": { "deploy": ["deploy-site-task.yml", "secrets: inherit"] } }, "reference": "catalog/snippets/workflows/deploy-site.yml", "intentRef": "WORKFLOW.md#d4---release--publish", "appliesTo": ["hugo"] }
2, 3 and 4. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
catalog/snippets/workflows/deploy-site-task.yml:159
- Same as the upload step: make the SSH options explicit for the pointer-flip rsync as well (StrictHostKeyChecking/UserKnownHostsFile/BatchMode) so the snippet enforces the intended security behavior regardless of runner SSH defaults.
rsync -a --no-recursive \
-e "ssh -i ~/.ssh/deploy -o IdentitiesOnly=yes" \
"${RUNNER_TEMP}/bundle/current" \
"${DEPLOY_SSH_USER}@${DEPLOY_SSH_HOST}:/${ENVIRONMENT}/"
catalog/snippets/workflows/deploy-site-task.yml:144
- The rsync SSH transport relies on OpenSSH defaults for host key checking and known-hosts location. Since this is a reference deploy snippet and the comments call out host key integrity as critical, explicitly pin
StrictHostKeyChecking=yes,UserKnownHostsFile, andBatchMode=yesin the-e ssh ...options to avoid any runner-default drift and to ensure failures are non-interactive.
This issue also appears on line 156 of the same file.
rsync -az --mkpath --no-g --chmod=D2755,F644 \
--link-dest="/${ENVIRONMENT}/current/" \
-e "ssh -i ~/.ssh/deploy -o IdentitiesOnly=yes" \
"${RUNNER_TEMP}/bundle/releases/${RELEASE_ID}/" \
Uh oh!
There was an error while loading. Please reload this page.
Two review findings from the hub's copy of this pipeline (ptr727/ProjectTemplate#560), both of which apply here identically. The production gate compared github.ref_name against "main". Tags and branches are separate namespaces that share a short name, so a tag named main would satisfy that comparison while pointing at an arbitrary commit, bypassing the one gate protecting production. It now compares github.ref against refs/heads/main, which is unambiguous. The rsync transport left host key checking and the known-hosts location to the runner's OpenSSH defaults. StrictHostKeyChecking defaults to ask, which a non-interactive runner resolves ambiguously, and the known-hosts path was the default rather than the file the deploy key step writes. Both are now pinned, along with BatchMode=yes so a credential problem fails the step instead of hanging the job to its timeout. The option string is declared once at workflow level, so the upload and the pointer flip cannot drift apart, which is the same single-declaration reasoning as the generator pin. Verified: actionlint clean at exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two review findings on the reference leaves, both right. The production gate compared github.ref_name against the default branch name. Tags and branches are separate namespaces that share a short name, so a tag named for the default branch would satisfy that comparison while pointing at an arbitrary commit. That is a bypass of the one gate protecting production, in the file this change just added to the manifest for carrying exactly that gate. It now compares github.ref against the full refs/heads/<default> form. The rsync transport left host key checking and the known-hosts location to the runner's OpenSSH defaults, in a snippet whose own comments call host key integrity load-bearing. StrictHostKeyChecking, UserKnownHostsFile and BatchMode are now pinned explicitly, and the option string is declared once at workflow level so the upload and the pointer flip cannot drift apart. BatchMode in particular turns a credential problem into a failed step rather than a job that hangs to its timeout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (2)
catalog/snippets/workflows/deploy-site-task.yml:47
- The
SSH_TRANSPORTcommand uses-o UserKnownHostsFile=~/.ssh/known_hosts. Because~is not expanded by the shell when it appears after=, this relies on SSH performing tilde expansion itself and can lead to StrictHostKeyChecking reading the wrong known_hosts path. Use$HOME(or an absolute path) for both the identity file and known_hosts to avoid ambiguity.
SSH_TRANSPORT: >-
ssh -i ~/.ssh/deploy
-o IdentitiesOnly=yes
-o StrictHostKeyChecking=yes
-o UserKnownHostsFile=~/.ssh/known_hosts
TODO.md:48
- This TODO entry says the type landed with an
environmentsblock inspec/secrets.json, but this PR adds theenvironmentsschema tospec/secrets.schema.jsonand updatesspec/secrets.jsonprose; there is noenvironmentsblock inspec/secrets.jsontoday. Adjust the wording so it matches the actual implementation.
- Declare locally-required secrets the way GitHub-stored ones are already declared, and make a gitignored `secrets/` directory the fleet standard that holds them. [`spec/secrets.json`][secrets] covers only the Actions and Dependabot stores, so a repo that deploys somewhere has no declared way to say what it needs at runtime, and the required set is discoverable only by reading the deploy. The pattern already runs in the fleet in two shapes: HomeAutomation-Config keeps a gitignored secrets directory of env files and Docker secret files, and ESPHome-Config keeps a gitignored `secrets.yaml` beside a committed `_secrets.yaml`. The committed file carries the required names with dummy values, so the shape of the requirement is in git while the values never are, which is the same split the GitHub side already gets from `requiredSecrets[]`. Blog needs it immediately, since it deploys on the proxmox host through HomeAutomation-Config's Docker Compose stack and carries the copy destinations and the internal URI. The hub carries neither the directory nor a `.gitignore` entry for one today, so adopting it here comes first. Note that the **GitHub** side has the same missing axis, surfaced by the `hugo` type: a deploy's credentials are per-environment GitHub Environment secrets and variables, `stores` is a closed enum of `actions` and `dependabot`, and `spec/audit.py` seeds `required_by_store` with those two keys and indexes it unguarded, so adding an `environments` value to the enum raises `KeyError` for every repo whose `publish[]` maps to that mechanism. The type landed with an optional, unread `environments` block in `spec/secrets.json` documenting the names, which is honest but is not a gate, so a clean audit says nothing about whether an environment is configured. Solving it properly means a store vocabulary that does not crash on an unknown key and a per-environment secrets and variables fetch, and it is the same axis as the local half above rather than a separate problem.
Copilot's point, and it is right: the TODO entry said the type landed with an environments block in spec/secrets.json. It did not. What landed is the optional block in spec/secrets.schema.json plus a description in the note, so a repo that has environments may declare its per-environment names and the shape is legal rather than invented. The hub's own secrets.json carries no such block, having no environments of its own. The distinction matters because the entry is a record of what is still owed. Reading it as "the block exists in the hub's file" would make the remaining work look smaller than it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 5, 2026
Answering the two suppressed comments from round 8One taken, one declined with a measurement, and the declined one is worth reading because the suggested fix would have broken the deploy. 1. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (3)
catalog/snippets/workflows/deploy-site-task.yml:192
- This step runs deploy/prune-releases.sh directly, which depends on the script having the executable bit set in git. Calling it via bash avoids a brittle Permission denied failure mode while keeping behavior the same.
run: |
set -Eeuo pipefail
deploy/prune-releases.sh "${DEPLOY_SSH_USER}@${DEPLOY_SSH_HOST}" "${ENVIRONMENT}" "${KEEP_RELEASES}"
catalog/snippets/workflows/deploy-site-task.yml:203
- This step executes checks/check-live-urls.sh directly, which requires the executable bit to be set. Using bash here makes the reference workflow less fragile when copied into new repos.
run: |
set -Eeuo pipefail
checks/check-live-urls.sh "${{ vars.SITE_BASE_URL }}"
catalog/snippets/workflows/deploy-site-task.yml:123
- This step executes deploy/make-release.sh directly. That requires the script to be committed with the executable bit set; if it is not (common when scripts are created/edited on Windows), the deploy will fail with a Permission denied error. Invoking it via bash makes the snippet more robust without changing its inputs/outputs.
This issue also appears in the following locations of the same file:
- line 189
- line 201
run: |
set -Eeuo pipefail
deploy/make-release.sh "${RUNNER_TEMP}/bundle" "${{ steps.release.outputs.id }}"
Copilot's point across three call sites, and right for a snippet whose whole purpose is being copied: calling a script by path requires the executable bit to have survived into the copying repo's git index. A repo authoring those scripts fresh, particularly on Windows where the fleet's default line endings already point, gets Permission denied at deploy time rather than at review time. It also matches the convention the hub already uses on itself, where every script runs through its interpreter (python3 spec/validate.py, python3 scripts/test_prose_lint.py). Shell scripts were the inconsistent case. Left unchanged in the one downstream member, whose scripts are committed 100755 and are proven by a real deploy. The leaf is doctrine and the repo is working code, so this is not worth a round of review there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 5, 2026
Answering the three suppressed comments from round 9One finding across three call sites, and taken in
Right, and the reasoning is specifically strong for this file rather than in general. A workflow in a working repo can rely on its own scripts being Checking the house convention rather than taking it on the argument alone, it turns out this was the inconsistent case already. The hub runs every one of its own scripts through an interpreter: python3 spec/validate.pypython3 scripts/test_prose_lint.pypython3 scripts/test_repo_gate.pypython3 spec/audit.py --selftestPython scripts got interpreter-prefixed invocation and shell scripts did not, for no reason anyone wrote down. So this makes the snippet consistent with the repo it ships from, which is a better justification than robustness alone. All three call sites changed: the bundle assembly, the prune, and the live check. Not changed in the one downstream member. Its scripts are committed That is 19 findings across nine rounds, all answered. Seven taken, twelve declined with a measurement or a citation. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (1)
catalog/snippets/workflows/deploy-site-task.yml:47
- SSH_TRANSPORT uses
~/.ssh/...paths inside a string passed to rsync's-e. Tilde expansion is not guaranteed there (it may be exec'd without a shell, or via/bin/shwhich doesn't expand~), which can make the SSH key/known_hosts paths fail to resolve at runtime.
SSH_TRANSPORT: >-
ssh -i ~/.ssh/deploy
-o IdentitiesOnly=yes
-o StrictHostKeyChecking=yes
-o UserKnownHostsFile=~/.ssh/known_hosts
ptr727
commented
Aug 5, 2026
Answering the suppressed comment from round 10Same finding as round 8, restated with a different mechanism. Quoting it since it carries no thread:
Still declining, and the new mechanism is the exact one the earlier measurement was designed to rule out. I single-quoted the options specifically so no shell touched them, which is the no-shell exec case: $ ssh -G -o 'UserKnownHostsFile=~/.ssh/known_hosts' -o 'IdentityFile=~/.ssh/deploy' example.invalididentityfile ~/.ssh/deployuserknownhostsfile /home/pieter/.ssh/known_hosts
Repeating the other half too, because it is the part that would cause harm: the suggested No change. If there is a concrete rsync or OpenSSH version where |
Uh oh!
There was an error while loading. Please reload this page.
…#30) * Declare the generator pin once and record what the theme is a copy of Three conformance gaps against the fleet hugo type (ptr727/ProjectTemplate#560), plus two smaller items found alongside them. Closes#28. Closes#29. The Hugo version and checksum were declared in both validate-task.yml and deploy-site-task.yml, each with an instruction to update both and nothing enforcing it. A one-sided bump was silent and produced the failure the pin exists to prevent: validation building the site with one generator while the deploy shipped a tree built by another, each verifying its own checksum against its own version and both passing. No Dependabot ecosystem tracks Hugo, so there was no bot to catch the skew either. Both installs now call a composite action that owns the pin, so the two cannot diverge, and it asserts the extended build from the binary rather than inferring it from the file name. The vendored theme recorded no upstream ref, so nothing could be diffed, updated, or audited against it. themes/README.md now records the commit, recovered by matching all 125 tracked blobs against upstream history rather than guessed: 154d006e0182dfc7da38008323976b02e6bfab4a, describing as v8.0-138-g154d006. Every file matches it exactly except two, both additions in extension points the theme documents for the purpose, and both are listed with the note that Hugo would resolve them from the project root instead, which would make the next update a clean directory replace. The record sits outside PaperMod/ so replacing that directory does not take it with it. That exposed a scoping bug: the markdown glob excluded all of themes/, so a file we author about a vendored tree would not have been linted. It now excludes themes/*/** instead, reaching inside a theme rather than over the directory that holds them. Also: assert-ref and assert-environment ran without a permissions block, so two jobs that only echo and case-match inherited the repository default, and both are now permissions: {}. And deploy-site-task.yml exposed no outputs, so no caller could record what shipped; it now returns release-id and site-url, and the live check already proves that id is the one answering, which makes it the value a rollback names. Verified: actionlint clean, markdownlint 0 issues across 16 files with the provenance file now in scope and the vendored tree still out, editorconfig-checker clean on everything tracked, and the site builds under --panicOnWarning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Hardcode the Hugo pin so a caller cannot override it Copilot's point, and it is right: exposing version and sha256 as inputs with defaults left the divergence this action exists to remove, one level up. Two callers could pass different values and reintroduce the silent skew, and the only thing preventing it was that neither caller passes the arguments today. That is correctness by convention, which is what the original two-file pin also was. The pin is now hardcoded in the action, so callers cannot override it and every caller moves together or none does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Gate production on the full ref, and pin the transport's SSH options Two review findings from the hub's copy of this pipeline (ptr727/ProjectTemplate#560), both of which apply here identically. The production gate compared github.ref_name against "main". Tags and branches are separate namespaces that share a short name, so a tag named main would satisfy that comparison while pointing at an arbitrary commit, bypassing the one gate protecting production. It now compares github.ref against refs/heads/main, which is unambiguous. The rsync transport left host key checking and the known-hosts location to the runner's OpenSSH defaults. StrictHostKeyChecking defaults to ask, which a non-interactive runner resolves ambiguously, and the known-hosts path was the default rather than the file the deploy key step writes. Both are now pinned, along with BatchMode=yes so a credential problem fails the step instead of hanging the job to its timeout. The option string is declared once at workflow level, so the upload and the pointer flip cannot drift apart, which is the same single-declaration reasoning as the generator pin. Verified: actionlint clean at exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ate (#568) Resolves the audit-tooling half of #563, filed from `ptr727/Blog` after its re-integration. All four of that issue's asks, plus a defect in one of the fixes it proposes. ## The gate that suppressed the check `spec/audit.py` wrapped the whole freshness check in `if not findings:`. The rationale was sound in isolation, in that a clean repo has no outstanding work for a pending-marker note to describe, but the consequence is that **any** repo carrying one finding it cannot clear has its entire `driftNotes` list exempted. Blog carries exactly that, the `carried: AGENTS.md references the template repo` finding tracked in #552, so its notes were never checked however clean the rest of the audit ran. The repo with open findings is where a stale note is most likely, which is the inverse of what the gate produced. Both shapes are now evaluated on every run, worded by context rather than suppressed: | Note | Audit clean | Findings open | | --- | --- | --- | | prose marker (`pending`, `still`, ...) | contradicted outright, as before | raised as which of the open findings it means | | names a check id | surfaced for a hand decision | surfaced for a hand decision | Measured live, this is one added advisory across the fleet. Of 53 notes on 22 repos, one carries a marker (MediaTools, `pending fleet-wide ratification`), and it now reads `while 25 finding(s) are open - confirm it describes one of them rather than closed work`. Under the old gate it was silent. ## The check-id matcher, and why it is not anchored #563 proposes `\(([a-z]+\.[a-z.]+)\)$`. That pattern matches **neither of the two notes it was written for**, because both end the sentence after the paren: > ... so it cannot be moved or diffed against upstream (hugo.vendored.provenance)**.** Run over the whole registry it matches zero notes on zero repos, which is indistinguishable from a fleet carrying no such note. That is the silent-narrowing shape `GOVERNANCE.md` "Verification Discipline" names: a pattern that matches less still exits zero. `CHECK_ID_RE` is therefore unanchored, and the self-test covers the trailing-period case, the mid-sentence case, an id absent from the catalog, an id whose type the repo does not declare, and a parenthesized version string as the false positive the shape has to exclude. **Watched failing**: re-anchoring the pattern to `$` turns five cases red and the suite reports `SELFTEST FAIL`. ## What the audit will not claim Resolving the id is as far as a tool can honestly go here, because `spec/audit.py` never reads `spec/project-types.json` at all. So the audit checks that the id exists and that the repo declares its type, then hands the check itself to the auditor. The finding clears when the note is deleted, which is the retirement path #560 promised, made visible rather than left to a matcher that could never fire. That same fact is now stated where an agent reads a run rather than only in the issue: `AUDIT.md` section 4, the console line printed for a clean repo, the `--issue` body preamble, and `OPERATIONS.md`. Adding a check to `project-types.json` changes what an auditor must judge and changes no tool's output, and silence from a tool that was never looking reads exactly like a pass. ## Blog's two notes are dropped Both deviations closed in ptr727/Blog#30 and are on Blog's ground-truth `main` (`2b132e4`), verified by reading that branch rather than trusting the issue: - `hugo.vendored.provenance` - `themes/README.md` records the upstream repository, commit `154d006e`, its upstream date, `git describe`, the license location, and both local edits. - `hugo.generator.pinned` - the version and SHA256 are declared once at `.github/actions/install-hugo/action.yml:26-27`, and `validate-task.yml:76` and `deploy-site-task.yml:79` both consume that composite action. ## Verification | Gate | Result | | --- | --- | | `spec/audit.py --selftest` | PASS, and FAIL on the anchored matcher | | `spec/validate.py` | 22 cataloged, 0 backlog, clean | | `spec/audit.py Blog MediaTools` (live) | behaves as described above | | markdownlint-cli2 | 44 files, 0 issues | | cspell (README, HISTORY) | 0 issues | | editorconfig-checker | clean | | `scripts/prose_lint.py` | no new violations on any touched file, net -2 | `Refs` rather than `Closes`, since a closing keyword cannot fire from a `develop`-targeted pull request. #563 is closed by hand with evidence once this merges. Filed by an agent in `ptr727/Blog`, resolved here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…574) Promotes the nine commits `develop` has carried since the previous promotion (#555) to `main`. Merge commit only, no squash, and `develop` is not deleted. ## What lands - **#560** Standardize the static-site-deploy repo type and its destination, including the deploy-verification guarantee and a retention rule that records which side owns the prune. - **#561** Ask for a blocked decision instead of reporting it, and scope the clickable-link rule to a surface that renders markdown. - **#564** Read a suppressed section nested inside the review-details wrapper, so `scripts/pr_review.py` stops reporting a clean round over findings that reach no thread. - **#566** Settle Markdown capitalization in `CODESTYLE.md`, and fix the two `TODO.md` prose defects the digest defect had hidden. - **#568** Check a `driftNote` on every run rather than only on an otherwise clean audit, and state what the audit does not evaluate. - **#569** Name who trips the production ref gate, and state that a harness refusal is a different thing from the maintainer's permission and is not lifted by it. - **#571** Re-audit `ptr727/Blog` against the hugo type and bump the conformance matrix. - **#573** Scope the semicolon exemption to the sentence its list lives in, with the 44 newly reported occurrences recorded on #519 rather than swept here. - **#572** Exempt a verbatim section from the coordination-reference scan. ## Issues closed Fixes#562. Closes#565. The closing keywords sit on this promotion rather than on the feature pull requests, because GitHub fires them only on a merge into the default branch. #456, #558, #563, #567, and #570 are already closed against their merged fixes. ## Verification `git merge-tree` reports no conflict between `origin/main` and `origin/develop`, so this promotion needs no throwaway resolution branch. Every constituent pull request merged green with its review loop closed.
Closes#456. Closes#558.
Blog has been running a Hugo build with an rsync-over-SSH deploy to a host we own, and nothing governed it.
WORKFLOW.md'sOutput Seam by Destinationhad no row for a filesystem on our own host,registry/repos.schema.json'stargetenum had no member for it, and Blog stood cataloged assource-onlybehind two interim driftNotes recording the deferral.What lands
The
hugotype (spec/project-types.json), nine checks. Onlyhugo.build.strictnames the generator, where a generator-specific flag is the letter. The rest are phrased generator-agnostically, so promoting them to a shared type when a second generator arrives is a registry edit rather than a rewrite.spec/type-model.mdgrows a Generators section stating that rule and why there is nostatic-sitetohugohierarchy at one member.The
self-hostedtarget and thedeploy-sshmechanism. What a repo builds and where the result lands stay separate axes, so an SMB or S3 publisher later is a new mechanism, not a new type.D4.6 and D5.6. D4.6 requires the deploy to assert which release and which environment answered, polling to a bounded timeout, rather than trusting the transport's exit status. D5.6 requires the prune of a durable destination to be asserted on the host that was written to.
A reference leaf pair in
catalog/snippets/workflows/, plus the 5A addendum, scenarios S12/S13, and a section 6 walkthrough.Blog reclassified to
["hugo", "source-only"]with both publish targets.Three questions a reviewer will ask
Does D4.3 need extending? No. It enumerates how a repo reaches the tag-only release shape, and this destination adds no fourth route: the deploy is a separate dispatch that touches no release, and Blog reaches tag-only through the source-only route already listed.
Why are
requiresandstoresempty ondeploy-ssh? Not for want of credentials. They are per-environment GitHub Environment secrets, which neithervalidate.pynoraudit.pycan enumerate.validate.py:200would force anyrequiresname into the repo'srequiredSecrets, andaudit.py:615unions that into the actions store expectation, so listing them guarantees a false DEFECT on a correctly configured repo. The new optionalenvironmentsblock records the names as operator documentation and says plainly that it is not a gate, so a clean audit is not evidence an environment is configured. Extending thestoresenum instead was measured and rejected:audit.py:606seedsrequired_by_storewith two keys and:610indexes it unguarded, so an unknown store raisesKeyErrorfor every repo whosepublish[]maps to that mechanism. Follow-up folded intoTODO.md's locally-required-secrets entry, since it is the same missing axis.Why is the leaf concrete rather than parameterized? An eleven-input generalized transport leaf was proposed and rejected. The other twelve leaves take
ref/branch/smokeand nothing else; reuse is by vendored copy, so a copier edits arun:line for free; the hub never executes these files, so eleven parameters would be untested surface presented as canonical; and naming it for the transport is the mistake the type name avoids. The two outputs were kept, since Blog currently has none and no caller can record what shipped.Two checks Blog fails today
Recorded as driftNotes naming their check id, so the next audit retires them mechanically.
hugo.vendored.provenance.gitmodules, no recorded upstream ref, and no Dependabot ecosystem covering it.hugo.generator.pinnedBoth are fixed in ptr727/Blog#30.
A third check was wrong, and the second commit fixes it
hugo.deploy.retentionand D5.6 originally required the deploy to prune the destination and assert the count on the host. Blog cannot, and should not be made to. Its deploy credential is a forcedrsynccommand confined write-only, so the server never acts as sender and the key can neither delete a release nor read the destination back to count one. Blog's own ownership table already assigns release-prune timers to the host, which is the correct resolution rather than a gap.As first written the check was unsatisfiable for exactly the repos that confine their credentials properly, and the only way to pass it was to widen a deliberately narrow key. That trades a real confinement boundary for a green check, so the check was wrong rather than the design.
Retention is now bounded by a declared count with one side recorded as owning the prune: the deploy asserts it where its credential can observe the destination, the host owns it where the credential cannot. What the guarantee still rejects is a prune against a local scratch tree, a best-effort prune, and neither side owning it, since each then assumes the other prunes. The reference leaf keeps the assert-in-pipeline shape and says when to delete the step. Blog passes the corrected version, so its retention driftNote is dropped and ptr727/Blog#27 is closed as invalid.
Worth noting as evidence for the type: a check written from the hub's side alone, against a real repo, was wrong on first contact in a way only the repo could reveal.
The record was wrong, and is corrected rather than deleted
TODO.md's intake entry predicted three things that are wrong against what Blog actually runs, and writing the type from the prediction would have encoded requirements the repo does not meet:latest, not pinnedDrive-by fixes
spec/scope-model.md's project-type token table was missingcpp, pre-existing.STANDUP.md's new-type procedure never mentioned the registrytargetenum, which is exactly what the first repo declaring a new destination failsvalidate.pyon. It also now warns that a leaf must not be namedbuild-*-task.yml, sincesource-only.detectis literally that string.Verification
spec/validate.pyspec/audit.py --selftestscripts/repo_gate.pyscripts/prose_lint.py --diff develop.artifacts/andTests/obj/build outputSelector resolution verified directly: Blog resolves to
{hugo, source-only, release, dispatch-only, pull}, the deploy leaf is selected,build-release-task.ymlis correctly not selected, thereleasedevelop payload is selected over the operational one, andself-hostedroutes todeploy-sshwith an emptyrequiresso no secret finding is manufactured.Not in this PR
reports/blog/audit.md; the current one predates the deploy and its staleness is noted in the conformance matrix.audit.py.🤖 Generated with Claude Code