Skip to content

Promote Three Rule and Runbook Corrections to Main - #630

Merged
ptr727 merged 3 commits into
mainfrom
develop
Aug 8, 2026
Merged

Promote Three Rule and Runbook Corrections to Main#630
ptr727 merged 3 commits into
mainfrom
develop

Conversation

@ptr727

@ptr727 ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Closes #578.

Promotion of develop at 6df14c0, three squashes since the last one. Merge commit only, no squash and no rebase, and no --delete-branch, since this pull request's head is develop itself.

What is being promoted

Squash Change
fdd1e25 (#627) Widens three carried GOVERNANCE.md rules to state the recurring case, which is #578, plus the WORKFLOW.md cross-reference the same change took.
92b9fc5 (#628) Corrects the Copilot runbook to read the reviewer bot's node id repo-wide rather than from the pull request under review, and records the disproof of the claim raised against it.
6df14c0 (#629) Records the backlog cluster for documenting a cross-owner write grant.

Four files, .github/copilot-instructions.md, GOVERNANCE.md, TODO.md and WORKFLOW.md, at 49 insertions against 73 deletions.

The closing keyword

Closes #578 is on this pull request rather than on #627, because GitHub fires a closing keyword only when it merges into the default branch, and #627 merged into develop. Merging this closes that issue. The work itself landed in fdd1e25.

What this owes downstream

Two of the three carry a rule rather than a hash refresh, so a repository left on the old copy is wrong on the rule and not only on the bytes. Both are recorded in TODO.md under "Re-vendor the changed verbatim content":

  • The #578 widening touches three verbatim GOVERNANCE.md sections, and "Operational Repositories" matters most on the two operational repositories that can act on it.
  • The runbook correction is intent fidelity, so no audit reports it. A repository on the old copy reads its own runbook as requiring a review on the pull request before the reviewer bot's node id can be read, and hands round 1 back to the maintainer whenever auto-review-on-open does not fire.

🤖 Generated with Claude Code

ptr727 and others added 3 commits August 8, 2026 11:49
#627)

Ships the `ready` cluster carrying
[#578](#578). No closing
keyword, since a feature to `develop` pull request never fires one.

Each of the three rules is correct about its common case and silent
about the case that keeps recurring. The issue's own diagnosis is that
downstream agents keep re-deriving them and reporting the result as a
discovery, which says the wording does not reach the case rather than
that the rule is missing.

All three `Checked` anchors were re-read against `develop` at `a706ddb`
before anything was written. All three gaps were still real.

## The three widenings

**`GOVERNANCE.md` "Branching Model"** now states that an issue closes
when its work is verifiably complete, and that the keyword automates the
case where completion and promotion coincide rather than adding a second
condition to it. Two cases fall outside it and both close by hand: a
promotion that merged without the keyword, and **work complete on
`develop` with no promotion imminent**, where waiting holds a finished
issue open for however long the next promotion takes. The closing
comment cites the squash SHA, re-read against the branch rather than
trusted from the claim that asked for the close.

The old wording licensed a hand-close **only** as a repair after a
promotion had already merged without the keyword, which is why the
second case had no route.

**`GOVERNANCE.md` "Communicating with the User"** now says the clickable
link goes in the message that comes **before** the prompt rather than
merely alongside it. The rule already got the hard part right, that a
prompt renders neither a Markdown link nor a bare URL. What it missed is
that a prompt blocks on an answer, so a message emitted after it is read
once the answer is already given, which is the one moment the link is no
longer any use. One word, `accompanies` to `before`, plus the reason.

**`GOVERNANCE.md` "Operational Repositories"** now states when to
decline the direct-commit grant. The grant exists for the one-line
config edit whose review round costs more than it protects, and that
reason stops applying well before a change gets large, so the test is
stated as a **shape rather than a line count**: restructuring rather
than adjusting a value, several files at once, a device, integration or
automation that did not exist before, anything whose failure shows up on
the live service rather than in a lint run, or a change the author
cannot state in one sentence.

This stays guidance by construction. Adding a `pull_request` rule to
`repo-config/operational/develop.json` would gate the direct push too
and withdraw the very allowance the model exists to give, so nothing
mechanical can apply it.

## What sweeping by term found

Item 3 swept by term rather than by the instance the issue named turned
up **`WORKFLOW.md` section 3** restating the same allowance with the
same silence, in the same words: "a change worth reviewing still takes
one". It now points at the section that owns the test instead of
repeating it, so the rule stays in one place and the second reader still
finds it.

Item 1's rule also sits in **"Branching Model"**, not the "Git and
Commit Rules" the cluster's `Touches` line named. That is where the
re-vendor has to look for it, so it is recorded rather than silently
corrected.

## Backlog bookkeeping

- The shipped cluster is retired from "Work Clusters".
- **#557 and #579 leave "Verified Complete, Awaiting Close"**, both
having been closed on 2026-08-08, along with four link definitions left
orphaned by their removal.
- **#578 takes their place** with its closing evidence quoted, so the
section is not empty and the issue stays visible while open, which step
1 of "How to Select the Next Item" requires.
- The "Fleet Sweeps" re-vendor entry records that this half **propagates
a rule rather than refreshing a hash**. Every other carried change
queued there is punctuation or capitalization, where a repo on the old
copy is correct on the rule and wrong on the bytes. Here it is wrong on
the rule, which makes it the half to carry first, and item 3 matters
most on the two `operational` repos that can act on it.

## Verification

| Gate | Result |
| --- | --- |
| `prose_lint.py --diff origin/develop .` | clean |
| markdownlint-cli2 over `**/*.md` | 0 issues in 44 files |
| editorconfig-checker | clean |
| CRLF preserved | 0 lines without CR in all three files |

Three carried `verbatim` `GOVERNANCE.md` sections change, so the fleet
is byte-mismatched until the re-vendor visit. `WORKFLOW.md` is `intent`,
so nothing reports its cross-reference at all, which is why it is
written down.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#628)

**Disposition** `Amends "Re-vendor the changed \`verbatim\` content,
which is one sweep covering seven files."` under "Fleet Sweeps". No
cluster under "Work Clusters" covered this, and a cluster created and
deleted in the same change records nothing, so the observation lands on
the entry that already reasons about what each downstream copy owes.

No closing keyword, since a feature to `develop` pull request never
fires one.

## What was wrong

"Triggering and Polling" in the runbook said the reviewer bot's node id
is read from an existing Copilot review **on the pull request being
reviewed**. Three statements asserted it:

- The round-1 paragraph: "The `requestReviews` mutation below is for
**re-requesting on later pushes** (a new head SHA). By then a prior
review exists, so its bot node id is readable."
- The step-1 code comment, whose query was `pullRequest(number: <N>) {
reviews }`.
- The paragraph after the recipe: "step 1 needs at least one prior
formal review on the PR".

A "Cold start" paragraph added later says the opposite in the same
section, that the id is the reviewer account's and is stable across
every pull request in the repo. The three statements above were never
swept, so the section contradicted itself and the strict reading is the
one a reader hits first. Acting on it, a pull request that
auto-review-on-open never fired on has no way to reach round 1 and gets
handed to the maintainer to seed through the UI, which is the hand-off
the mutation exists to remove.

## What the tree says

Read across the newest eight pull requests in this repo on 2026-08-08,
every Copilot review carries the same bot node id. It is an account id,
so nothing has to land on a pull request before it can be read.

## The change

- The round-1 paragraph drops the two claims above and says a round 1
carrying no review means wait, then request it yourself.
- Step 1 queries the 20 newest pull requests with the `first // empty`
guard and the `[ -z ]` stop, so it resolves on a pull request with no
review of its own.
- The per-pull-request claim and the cold-start exception collapse into
one statement of the id's scope, which drops the duplicated query block.
The pull-request-scoped read stays as a note that it buys nothing over
the repo-wide read and fails on exactly the round the repo-wide read
handles.
- `TODO.md` records the re-vendor. This propagates a procedure rather
than refreshing a hash, so a downstream repo left on the old copy is
wrong on the rule. The
[#578](#578) bullet
claimed to be the sole rule-propagating half of that sweep, which a
second one falsifies, so it is widened to two.

## Verification

Both corrected blocks were run verbatim against this repo before the
prose was written, reads only, and both returned the id.
`scripts/prose_lint.py` exit 0, `markdownlint-cli2` 0 issues,
`editorconfig-checker` exit 0, and both files were checked line by line
for CRLF, `.github/copilot-instructions.md` at 353 of 353 and `TODO.md`
at 489 of 489.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
**Disposition** `New entry`, as a new cluster under "Work Clusters".
Nothing in the file reasons about the write-safety kit's documentation,
so there was no entry to amend.

No closing keyword, since a feature to `develop` pull request never
fires one.

## The gap

The hook denies a `gh` write whose explicit target is under an owner
other than the checkout origin's, and the only way past it is a
maintainer grant in `GH_WRITE_GUARD_ALLOW`. Read against `develop` at
`92b9fc5`:

| Document | What it says about the variable |
| --- | --- |
| `docs/host-setup.md` "Agent Write-Safety Kit" | Nothing. This is the
section a developer follows to install the kit. |
| `host-setup/agent-safety/README.md` | Names it once, inside the bullet
describing what the hook *denies*. Gives the token format (`owner/repo`,
`owner/*` for a whole owner) and not how to set it. |
| `README.md` | No host-protection section, only a pointer to
`docs/host-setup.md`. |

So a reader who hits a denial is told a grant exists and never how to
make one.

## What the entry carries

- **The channel that works** is an `env` block in the checkout's
`.claude/settings.local.json`. It is per project rather than host wide,
so a grant made for one checkout does not follow the agent into another
repository's sessions.
- **The raising case is a fork**, where `origin` is under the
maintainer's own owner and `upstream` is the project it forked from.
Filing an issue or pull request against the upstream is the cross-owner
write and everything aimed at the fork is not, so the grant names the
upstream alone. That asymmetry is what a reader meets first, and it is
why the section wants a worked example rather than a definition.
- **The two forms a reader reaches for first cannot work**: an inline
`GH_WRITE_GUARD_ALLOW=owner/repo gh ...` prefix, and an `export` in a
shell call. The hook runs as its own process and reads only the
environment the session was launched with, which is exactly what makes a
grant a deliberate act taken outside the session rather than something
an agent can do for itself once blocked. `gh-write-guard.py` already
asserts the inline prefix denies, so the behavior is settled and only
the explanation is missing.
- **Two things to carry beside the example**: that a session restart
loads the grant, and a way to confirm one took, since inferring it from
a write that no longer denies means learning the answer by making the
write.

This change records the work. It does not do it, so both documents are
untouched here.

## Verification

`scripts/prose_lint.py` exit 0, `markdownlint-cli2` 0 issues,
`editorconfig-checker` exit 0, `TODO.md` at 507 of 507 CRLF lines. The
three new reference definitions sort by reference name, `[host-setup]`
after `[governance]`, `[readme]` before `[readme-structure]`, and
`[write-guard-readme]` after `[write-guard]`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 8, 2026 19:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Promotes the latest develop governance/runbook documentation updates to main, aligning the canonical rules and Copilot review runbook with the clarified “recurring case” guidance and updated operational-repo review expectations.

Changes:

  • Updates GOVERNANCE.md to explicitly cover recurring edge cases (hand-closing issues when keywords can’t fire; link placement before prompts; when to prefer PRs for operational repos).
  • Updates the Copilot runbook to source the reviewer bot node id repo-wide (including a guarded “no id found” path and a recorded disproof entry).
  • Updates WORKFLOW.md and TODO.md to cross-reference and record the above as rule/procedure-propagating changes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
WORKFLOW.md Points operational direct-commit guidance to the single authoritative “Operational Repositories” test in GOVERNANCE.md and adds the link reference.
TODO.md Removes the now-shipped cluster, records the completed #578 evidence, and adds a new ready cluster documenting cross-owner write grants.
GOVERNANCE.md Widens three rules to state the recurring cases explicitly (issue closure, prompt link ordering, operational PR preference).
.github/copilot-instructions.md Corrects the runbook to read the reviewer bot node id repo-wide and records the related disproved claim for future reference.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ptr727
ptr727 merged commit c40cd0a into main Aug 8, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Three carried rules state the common case and leave the recurring case unstated

2 participants