Correct two claims the previous commit stated too broadly - #470
Merged
Conversation
Both were written from inference rather than from checking, in the commit whose subject was recording mechanics accurately. A closing keyword does not "never fire under this model". It fires whenever the pull request itself merges into the default branch, which here is `main`, so a promotion or a Dependabot security update opened against `main` closes what it references. Only a feature pull request, which targets `develop`, cannot. The rule now states when the keyword works, so the absolute has nowhere to hide, and it notes that a promotion body can carry one deliberately. `--arg` is not a `gh api graphql` flag. It belongs to `jq`, and `gh api graphql` takes `-f` and `-F`. The `accepts 1 arg(s), received 4` error came from `gh` treating the extra tokens as positional arguments, which is what I reasoned from instead of reading `gh api --help`. The paragraph now leads with the general failure mode, that a failed command leaves stdout empty and the surrounding `$(...)` yields the empty string the test reads as success, and keeps the error string as a symptom rather than a claim about which tool owns the flag. Reported by Copilot on promotion #469. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens and corrects governance/runbook documentation so future agents don’t rely on over-broad claims when executing the promotion/review loop mechanics in this repository fleet.
Changes:
- Refines the “closing keyword” guidance to reflect that keywords only take effect when the PR merges into the default branch (
main) and calls out promotion/Dependabot-to-maincases. - Updates the “empty stdout reads as success” guidance to remove an incorrect attribution about
--argand instead emphasize the durable stderr/stdout failure-mode pattern and its common symptom.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
GOVERNANCE.md | Rewords the closing-keyword rule under the branching model/promotion mechanics to avoid an absolute “never” claim and to name the main-merge cases explicitly. |
.github/copilot-instructions.md | Rephrases the “empty result reads as success” warning to remove an incorrect flag-ownership claim and focus on the reliable diagnostic pattern. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Fixing an over-broad claim by writing a differently over-broad one is not a fix. The closing-keyword rule lost the words "in a pull request body" while gaining the branch condition, which left it reading as a statement about closing keywords generally. A keyword in a commit message is a separate mechanism, and nothing here has tested it, so the rule says what was checked and no more. The polling note claimed any failed command leaves stdout empty. A command can print part of its output and then fail. The sentence is about a `gh` call that does not run, which is the case the guard exists for. Reported by Copilot on #470. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 31, 2026
ptr727 added a commit
that referenced
this pull request
Jul 31, 2026
From the low-confidence block of Copilot's second review of promotion #469. It flagged one duplicate. Checking the other two additions from #468 found a second, and the second is worse than a duplicate. ## What #468 got wrong That pull request said it was recording three mechanics "none of them written down before". I did not grep the files before writing to them. **`gh pr edit`** was already covered at `.github/copilot-instructions.md` "PR Edits and Merge-State Gotchas", which gives **both** the GraphQL and the REST form and says to verify the edit took. The existing entry is better than the one I added beside it, so the addition goes and the original stays untouched. **Issue-closing keywords** were already covered in `GOVERNANCE.md` under the release model, and that entry is not merely earlier but **correct where mine was not**: > Issue-closing keywords (`Closes #N`, `Fixes #N`) go in the `develop -> main` promotion PR, not the feature -> develop PR. That is a working mechanism. My branching-model bullet said to close the issue by hand instead. Two bullets, one topic, **different procedures**, which is a contradiction in fleet law rather than a repetition, and exactly the drift Copilot warned the duplicate would cause. It also means my reply on #470, that the promotion mechanism was untested and so the rule should stay silent on it, was answering a question this repository had already answered. #462 could have been closed by putting the keyword on promotion #460. ## What survives The release-model rule gains the one thing mine had that it lacked: the fallback for a develop pull request that already merged with a keyword on it. That is the case #462 actually hit, and without it a reader who has already made the mistake finds no instruction. The polling guard from #468 **stays**. Guarding an empty bot node id before a mutation is documented in two places already, but the **exit test of a poll loop** is not, and the numeric comparison is what stops an empty result reading as a landed review. Kept as the one genuinely new thing in that commit. ## Why this happened, and what would prevent it The closing-keyword rule lives under "Release Model" while the question I was answering was a branching one, so reading the Branching Model section did not surface it. That is a findability problem in a 400-line governance file rather than an excuse: a `grep` for the topic would have found it in either section, and adding to a rules file without grepping it first is the actual failure. Not proposing a reorganization here. Moving committed rule text between sections changes two verbatim sections at once and every downstream copy with them, which is not something to ride along with a fix. ## Verification ``` gh pr edit mentions -> 1 (copilot-instructions.md:209) closing-keyword bullets -> 1 (GOVERNANCE.md:83) ``` 71 self-tests and 19 repo_gate tests pass, `repo_gate` is clean, `charset` and `dupword` exit 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727 added a commit
that referenced
this pull request
Jul 31, 2026
Promotes `f35e8c7` (#468), `a5e12a2` (#470), and `7477c42` (#472). Conflict-free, three commits ahead. The net change against `main` is **three lines in two files**. Reviewing this promotion is what reduced it to that, so the history and the result are described separately below. ## What actually lands **A polling guard, in the Copilot review runbook.** A poll that captures a `gh api --jq` result and exits on `[ "$found" != "0" ]` treats an **empty** string as a landed review, and an empty string is what a mis-written filter returns. Counting matches and testing `-gt 0` makes a query that finds nothing and a query that ran wrong read alike. One such poll during #460 reported a review that had not landed. **A fallback on the existing issue-closing rule, in `GOVERNANCE.md`.** The rule already said to put `Closes #N` on the promotion pull request rather than the feature one. It did not say what to do once a develop pull request has already merged carrying the keyword. It now does: move the keyword to the promotion body, and close by hand only when the promotion has merged without it. That is the case #462 hit. ## What was reverted, and why that matters more #468 opened claiming three mechanics "none of them written down before". Two of them **were**, in the files it edited. `gh pr edit` being broken by the classic-Projects sunset was already documented under "PR Edits and Merge-State Gotchas", with both the GraphQL and the REST form. The addition was a plain duplicate and is gone. Issue-closing keywords were already documented under the release model, and that entry is **correct where the addition was wrong**. It gives a working mechanism, the keyword on the promotion pull request. The added branching-model bullet said to close the issue by hand instead. Same topic, two sections, different procedures, which is a contradiction in fleet law rather than a repetition, and is the drift a duplicate is supposed to risk only later. That bullet is gone and the surviving rule absorbed the one thing it lacked. Two further corrections landed on the way. The closing-keyword bullet first said a keyword "never fires under this model", which is false because a pull request merging **into** `main` closes what it references. The polling note called `--arg` a `gh api graphql` flag, which is false because `--arg` belongs to `jq` and `gh api graphql` takes `-f` and `-F`. Both files are vendored verbatim across the fleet, so either statement would have propagated on the next re-vendor. ## Why promote now What survives is small and true. The polling guard is the only new mechanic in the set, and it is the one that produced a wrong report during #460 rather than a hypothetical. The `GOVERNANCE.md` sentence completes a rule that was already right by covering the state a reader reaches only after getting it wrong. `main` is what a newly scaffolded or realigning repository carries, and what the fleet audit reads as ground truth, so a rule that is complete on `develop` and partial on `main` is a rule that argues with itself across the fleet. ## Fidelity note `GOVERNANCE.md` and `.github/copilot-instructions.md` both carry verbatim sections, so downstream copies stay **stale** until re-vendored. They were already stale from #460 and this does not change that state, only its size, which is now three lines rather than the eleven #468 first proposed. Documentation only, no behavior change. 71 self-tests and 19 repo_gate tests pass, `repo_gate` is clean, `charset` and `dupword` exit 0, and the warn-only backlog is unchanged at dash 962, comment-wrap 454, semicolon 388, comment-case 56. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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 freeto 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.
Both findings from Copilot's review of promotion #469, against the mechanics #468 had just recorded. Both were written from inference rather than from checking, in the commit whose subject was recording mechanics accurately.
The closing keyword is not "never"
Verified rather than reasoned:
A keyword fires whenever the pull request itself merges into the default branch. A promotion, or a Dependabot security update opened against
main, therefore closes what it references. Only a feature pull request, which targetsdevelop, cannot.The rule is inverted to state when the keyword does work, so the absolute has nowhere to hide, and it names the promotion and bot cases explicitly. It also adds something the original missed: a promotion body can carry a keyword deliberately, which is a real option rather than a trap.
--argis jq's flag, not gh'sCopilot filed this as "likely to go stale or be incorrect across GitHub CLI versions". It was incorrect on arrival, which is the more useful finding:
gh api graphqltakes-fand-F. Theaccepts 1 arg(s), received 4error came fromghtreating the extra tokens as positional arguments, and I reasoned the flag's ownership from that error instead of readinggh api --help.The paragraph now leads with the durable general rule, that a failed command writes to stderr and leaves stdout empty so the surrounding
$(...)yields the empty string the test reads as success. The error string stays as a recognizable symptom, without the claim about which tool owns the flag.Why this matters more than its size
GOVERNANCE.mdand.github/copilot-instructions.mdare vendored verbatim across the fleet. A false statement in either propagates to every repository that re-vendors, and it arrived in the commit that exists specifically to stop the next agent reasoning from a symptom. Both corrections came from a review round rather than from me re-reading my own work.Verification
Documentation only, no behavior change, so no test accompanies it. 71 self-tests and 19 repo_gate tests pass,
repo_gateis clean,charsetanddupwordexit 0, and the warn-only backlog is unchanged at dash 962, comment-wrap 454, semicolon 388, comment-case 56.🤖 Generated with Claude Code