Name who trips the ref gate, and stop a refused write from being re-shaped - #569
Conversation
…haped A ref gate is verified only by tripping it, and the agent that owns the deploy pipeline is refused that dispatch. The contract required the check, named nobody to perform it, and so produced the two outcomes it least wanted: the check quietly dropped and the deploy reported verified, or the refused call re-expressed as a raw gh api POST to get past the refusal. WORKFLOW.md 5C now names the dispatch as the maintainer's and states the evidence, which is the whole point of the check: the gate job's conclusion, its error text naming the expected and received ref, every downstream job recorded as skipped rather than passed, and the deployment count against the production environment unchanged. A gate that fails open and a gate nobody tripped produce the same empty run history, so the absence of a failure is not evidence. GOVERNANCE.md gains the rule the re-shaping needs. A harness refusal is not lifted by the maintainer authorizing the call in conversation, so the identical call is refused again and the second refusal is not a flake. Re-expressing the call through another API surface reaches the same endpoint with the same identity and the same blast radius, having defeated the one control that stopped it, and it is the more dangerous version because the agent believes it has permission. The two routes left are both the maintainer's, and writing oneself a permission rule is self-authorization whatever was said. The last clause is the general form: where a required verification can only be performed by a call the agent is refused, the document requiring it says so and names who runs it, since a mandatory unperformable check is dropped silently and then reported as done. The section is verbatim fleet-wide, so the re-vendor entry in TODO.md names it. Refs #567 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new WORKFLOW.md item cites S8 but the described procedure/evidence applies only to S13, and the new GOVERNANCE.md bullet should stay consistently scoped to “writes” within the write-safety section.
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 fleet governance documentation to explicitly separate maintainer-only verification steps from agent-runnable steps when an agent harness can refuse high-blast-radius mutations (notably production deploy dispatch), and to forbid “reshaping” a refused write through alternate API surfaces.
Changes:
- Add a live-probe note in
WORKFLOW.mdclarifying that verifying the production deploy ref gate requires the maintainer to dispatch a non-default ref, with specific evidence to capture. - Add a new write-safety rule in
GOVERNANCE.mdstating that harness refusals are reported (not retried/rephrased) and that conversational authorization does not override harness policy. - Update
TODO.mdto include the new write-safety rule in the fleet re-vendor sweep description.
File summaries
| File | Description |
|---|---|
| WORKFLOW.md | Documents maintainer-run live probe for tripping the production deploy ref gate and the evidence to capture. |
| TODO.md | Updates the carried-content re-vendor entry to mention the newly added refused-write rule. |
| GOVERNANCE.md | Adds an explicit write-safety rule forbidding re-shaping refused writes and clarifying harness vs maintainer permissions. |
Review details
- Files reviewed: 3/3 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.
Two scoping corrections from review, both right. The live probe cited S8 alongside S13, but the procedure and every item of evidence it lists are the production deploy's: the environment dispatched, the downstream jobs skipped, the deployment count against that environment. S8 is the publish dispatch's generic ref guard and records no deployment, so citing it there described a check that scenario does not have. The refusal rule opened on "a refused call" inside a section scoped to state-changing writes, which widened the section's frame in its lead sentence while the rest of the bullet already said write. Both sites now say write, which is also accurate for the case that prompted the rule, since a dispatch creates a run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
WORKFLOW.md introduces a mid-sentence semicolon splice that violates the documented prose rule banning semicolons in agent-authored prose.
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)
WORKFLOW.md:266
- This sentence uses a mid-sentence semicolon to join two independent clauses ("afterwards; it does not fire it"), which violates the documented prose rule (GOVERNANCE.md "Documentation Style Conventions": "No semicolon in agent-authored prose"). Recast as two sentences (or a comma) to keep the rule consistent across carried docs.
- **The deploy ref gate (S13) is verified only by tripping it, and the dispatch that trips it is the maintainer's to run.** Dispatch the production environment from a non-default ref and expect the run to fail at the gate. The evidence is four things, and each of them matters: the gate job's conclusion, its error text naming the expected and the received ref, every downstream job recorded as **skipped** rather than passed, and the deployment count against the production environment unchanged. Capture all four, because a gate that fails open and a gate nobody tripped produce the same empty run history, so "we have never seen it fail" is not evidence about the one control standing between a mis-dispatch and the live site. **The agent prepares the command and reads all four back afterwards; it does not fire it.** An agent harness may refuse to dispatch a production deploy, which is the harness working as intended, and the refusal is neither re-shaped into a raw API call nor talked around (GOVERNANCE.md "Repository Boundaries and Write Safety"). The same split applies to any probe that acts on the deploy host directly, an outbound SSH exercising a forced command among them.
- Files reviewed: 3/3 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.
GOVERNANCE.md "Documentation Style Conventions" bans a semicolon joining two
independent clauses, and this bullet carried one. Recast as two sentences.
Worth recording why the local gate passed it, because the gate is not the
check here. prose_lint exempts a span it reads as a list, via
count(';') > 1 or a colon before the first semicolon, and this bullet
enumerates the four evidence items after a colon. That marks the whole bullet
listish, so every semicolon later in it is exempt however plainly it joins two
clauses. The exemption decides what the rule does, which is the shape already
recorded against the markdown sweep, so the same manual read is what caught
this one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering round 2's suppressed finding here, since a suppressed finding has no thread to pair an answer to. Accepted, fixed in b1015b6.
Correct on both the rule and the recast. It now reads "... reads all four back afterwards. It does not fire it." The part worth recording is that the repo's own gate passed it, and The exemption is That is a gate defect rather than a one-line style slip: any bullet in any carried doc that introduces a list with a colon is silently exempt from the semicolon rule for its whole length, and the rule reports success while checking nothing there. It is the same shape as the Not caught by tooling, then. What caught it here was the review, and the recheck afterwards was a manual read of every line this PR adds for the same construction. The other PR in this pair, #568, is clean of it. |
There was a problem hiding this comment.
🟡 Changes recommended
TODO.md reintroduces a “refused call” phrasing that should match the new “refused write” terminology used in the write-safety rule.
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)
TODO.md:33
- This TODO entry says "a refused call" but the new rule in GOVERNANCE.md is explicitly scoped as a "refused write" (matching the write-safety section's terminology). Keeping this aligned avoids reintroducing the call/write ambiguity that was already fixed in GOVERNANCE.md.
- Re-vendor the changed `verbatim` content across the fleet, which is one sweep covering five files. `repo-config/configure.sh` is carried `verbatim` with `appliesTo: "*"` and the hub swept it to one sentence per line. In `AGENTS.md`, "Context and Delegation Discipline" carries the wait rule's failure clause and "Where the Rules Live" carries a row for the new section named below. In `GOVERNANCE.md`, "Verification Discipline" carries the rule that a launched process is not a result, "PR Review Etiquette" carries the five outcomes that close a finding, "Repository Boundaries and Write Safety" carries the rule that a refused call is reported rather than re-shaped, and "Representative Data in Agent-Authored Text" is an entirely new carried section that no downstream repo holds, which the audit reports as a missing section rather than as drift. Three further `GOVERNANCE.md` sections differ by a single word each, "Documentation Style Conventions", "Communicating with the User" and "Repository Details", where the format's name was capitalized to the convention `CODESTYLE.md` "Markdown and Spelling" now states, so they are byte-mismatched for a reason a reader of the diff would otherwise call cosmetic. Two comment lines in `.markdownlint-cli2.jsonc` took the same capitalization, and that file is `verbatim` and `whole`, so every downstream copy is byte-mismatched on a config nothing else changed about. `CODESTYLE.md` carries the new item and is the fifth file, at `intent` rather than `verbatim` fidelity, so it reaches the fleet as a rule each repo adopts in its own copy rather than as bytes to match, and the same mixed spelling is waiting in every downstream tree. Every repo already holding a copy of a changed section is byte-mismatched against the hub until it takes the new one, which the audit reports as stale rather than modified. This sweep is also the follow-through [#489][issue-489] and [#379][issue-379] were waiting on, and the `.editorconfig` line in [#353][issue-353] rides the same visit to each repo. Regenerate [reports/divergences.md][divergences-report] before using it as the work list, since the committed copy is dated 2026-07-22 and therefore predates the router split, which shows in it naming "Repository Boundaries and Write Safety", "Git and Commit Rules" and "Verification Discipline" as `AGENTS.md` sections when all three now live in `GOVERNANCE.md`. A stale ledger is the same hazard as a stale exemption, in that it hands out a work list measured against a tree that no longer exists.
- Files reviewed: 3/3 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 rename landed in GOVERNANCE.md and not in the re-vendor entry naming that same rule, so the two disagreed on the word the previous round had just settled. Grepped both spellings tree-wide rather than fixing the one line review pointed at. TODO.md line 33 is the only other site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Ready to approve
Documentation-only changes are coherent and consistent with existing contracts, with only a minor wording-precision nit captured in review comments.
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)
WORKFLOW.md:266
- In the S13 live-probe bullet, "error text naming the expected and the received ref" slightly overstates what the current gate emits: the workflow's error says "Deploy production from the default branch, got ", which names the expected default-branch requirement (not an explicit expected ref string) and the received ref. Tweaking this wording would keep the contract aligned with the actual message shape.
- **The deploy ref gate (S13) is verified only by tripping it, and the dispatch that trips it is the maintainer's to run.** Dispatch the production environment from a non-default ref and expect the run to fail at the gate. The evidence is four things, and each of them matters: the gate job's conclusion, its error text naming the expected and the received ref, every downstream job recorded as **skipped** rather than passed, and the deployment count against the production environment unchanged. Capture all four, because a gate that fails open and a gate nobody tripped produce the same empty run history, so "we have never seen it fail" is not evidence about the one control standing between a mis-dispatch and the live site. **The agent prepares the command and reads all four back afterwards. It does not fire it.** An agent harness may refuse to dispatch a production deploy, which is the harness working as intended, and the refusal is neither re-shaped into a raw API call nor talked around (GOVERNANCE.md "Repository Boundaries and Write Safety"). The same split applies to any probe that acts on the deploy host directly, an outbound SSH exercising a forced command among them.
- Files reviewed: 3/3 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.
|
Round 4's one new suppressed finding. Declined, on the measurement. The earlier two are answered above.
The message quoted in the finding is not the one the gate emits. Read from the implementing repo's ground-truth branch, echo "::error::Deploy production from main; got ${{ github.ref }}."Which produced, on the maintainer-dispatched run that tripped it (#567's evidence, run So the text names the expected ref as the literal Two things worth stating so the underlying point does not get lost, since it is a reasonable thing to have checked. The bullet deliberately does not quote a message. This file is contract-not-implementation, per its own opening and section 1, so the evidence item names what the text must identify (the expected ref and the received one) rather than a string to match. A second "The expected ref" and "the default branch" are the same thing here, not two alternatives. Section 1 already covers this: the guarantees say "default branch" portably, and it is implemented as the literal |
…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.
Resolves #567, raised from ptr727/Blog#33 after the deploy transport retest. Documentation only, no behavior change.
The problem the contract created
The deploy contract guarantees a ref gate: a production deploy is refused unless the dispatched ref is the default branch. A gate is shown to fail closed only by tripping it, so the verification is one
workflow_dispatchof the production environment from a non-default branch, expecting the run to fail at the gate job.The agent that owns the pipeline is refused that dispatch by its harness, and #567 records that conversational authorization did not lift it: the maintainer authorized the call explicitly, in session, and the identical call was refused again. The staging dispatch was allowed and the same text inside an
echowas allowed, so the refusal is a judgment about the action rather than the subcommand or the ref. That judgment is defensible and this change does not ask for it to be relaxed.What the contract produced was the worst arrangement of it. The check is required, the agent cannot perform it, and nothing said so. The two predictable outcomes are an agent that drops the check and reports the deploy verified, and an agent that re-expresses the refused call as a raw
gh api -X POST .../dispatches, which is a mutation fired as a workaround.What changes
WORKFLOW.mdsection 5C gains the ref gate as a live probe, named as the maintainer's step, with the evidence to capture. The evidence is the point: the gate job's conclusion, its error text naming the expected and the received ref, every downstream job recorded as skipped rather than passed, and the deployment count against the production environment unchanged. A gate that fails open and a gate nobody tripped produce the same empty run history, so "we have never seen it fail" is not evidence about the one control standing between a mis-dispatch and the live site. The agent prepares the command and reads all four back afterwards; it does not fire it. The same split covers a probe acting on the deploy host directly, an outbound SSH exercising a forced command among them, which #567 records failing the same way.GOVERNANCE.md"Repository Boundaries and Write Safety" gains the rule the re-shaping needs, and it is the natural home: that section already bans a write fired as a probe and a write consuming a fabricated id, and this is the third way a write goes somewhere nobody approved. It states that a harness refusal and the maintainer's permission are different things, that the second does not lift the first, that a refused write is never re-attempted through a different API surface or tool or rephrasing, and that writing oneself a permission rule is self-authorization whatever was said. Two routes remain and both are the maintainer's. The closing clause is the general form of #567's own complaint: where a required verification can only be performed by a call the agent is refused, the document requiring it says so and names who runs it, since a check that is mandatory and unperformable is dropped silently and then reported as done.Grepped
GOVERNANCE.md,AGENTS.md, and.github/copilot-instructions.mdfor both claims before adding either. Neither was stated anywhere, so this duplicates nothing and contradicts nothing.TODO.mdadds the section to the fleet re-vendor entry. "Repository Boundaries and Write Safety" isverbatimfidelity inspec/files.json, so every downstream repo now carries an older copy and an audit before the sweep reports it as stale. It joins the existing list rather than opening a new item.Verification
scripts/prose_lint.pyspec/validate.pyRefsrather thanCloses, since a closing keyword cannot fire from adevelop-targeted pull request. #567 is closed by hand with evidence once this merges.Filed by an agent in
ptr727/Blog, resolved here.🤖 Generated with Claude Code