Skip to content

Remove two rules this repository already had - #472

Merged
ptr727 merged 2 commits into
developfrom
fix/undo-duplicated-governance-and-runbook-rules
Jul 31, 2026
Merged

Remove two rules this repository already had#472
ptr727 merged 2 commits into
developfrom
fix/undo-duplicated-governance-and-runbook-rules

Conversation

@ptr727

@ptr727 ptr727 commented Jul 31, 2026

Copy link
Copy Markdown
Owner

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

#468 said it was recording three mechanics "none of them written down before".
Two of them were, in the files it edited, and I did not look before writing.

`gh pr edit` is covered by "PR Edits and Merge-State Gotchas", which gives both
the GraphQL and the REST form and says to verify the edit took. That entry is
better than the one added beside it, so the addition goes.

Issue-closing keywords are covered under the release model, and that entry is
not merely a duplicate but the better answer: put the keyword on the promotion
pull request, where it does fire. The bullet added to the branching model said
to close the issue by hand instead, so the two gave different procedures for the
same situation, which is a contradiction in fleet law rather than a repetition.

The surviving rule gains the one thing the new one had, the fallback for a
develop pull request that already merged with a keyword on it, since that is the
case #462 actually hit.

The polling guard from #468 stays. Guarding an empty bot node id before a
mutation is documented already, but the exit test of a poll loop is not, and a
numeric comparison there is what stops an empty result reading as a landed
review.

Reported by Copilot on #469, in the low-confidence block.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 31, 2026 02:15

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

Removes two documentation rules that were duplicated (or contradictory) with existing fleet governance, consolidating the canonical guidance so promotion/branching procedures don’t drift across sections.

Changes:

  • Removes the branching-model bullet about issue-closing keywords (duplicate/contradictory with Release Model guidance).
  • Removes the duplicate gh pr edit warning block from the “Known non-working request paths” section, keeping the existing canonical “PR Edits and Merge-State Gotchas” section.
  • Extends the Release Model issue-closing-keyword rule with a fallback for when a develop PR already merged with a closing keyword.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
GOVERNANCE.md Deletes the duplicate/contradictory closing-keyword guidance and updates the canonical release-model rule with a fallback case.
.github/copilot-instructions.md Removes a duplicate gh pr edit warning block, preserving the existing canonical guidance elsewhere in the runbook.

Comment thread GOVERNANCE.md Outdated
The sentence said to close by hand "rather than waiting for a promotion that
will not close it either", which reads as a claim that promotions cannot close
issues and contradicts the clause above it.

It also gave the weaker remedy. A develop pull request that merged with the
keyword on it is recoverable: put the keyword on the promotion body, which is
what the rule says to do in the first place. Closing by hand is only for the
case where the promotion has already merged without it.

Reported by Copilot on #472.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 31, 2026 02:18

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

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

@ptr727
ptr727 merged commit 7477c42 into develop Jul 31, 2026
7 checks passed
@ptr727
ptr727 deleted the fix/undo-duplicated-governance-and-runbook-rules branch July 31, 2026 02:21
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)
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.

2 participants