Skip to content

Check every closing keyword in a PR body, not only the intended ones - #171

Merged
dmccoystephenson merged 2 commits into
mainfrom
fix/negated-closing-keywords
Nov 1, 2026
Merged

dmccoystephenson merged 2 commits into
mainfrom
fix/negated-closing-keywords

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

  • A paragraph is added to the generated template's Phase 4, directly after "Before pushing, verify each Closes #N". It makes that check run in the other direction too. GitHub's closing-keyword parser ignores negation, so a sentence that says an issue is not being closed still closes it on merge if a keyword sits directly before the reference. The paragraph lists the keyword family, says a keyword may only come before a reference this PR really closes, gives non-keyword wording for mentioning an issue, and requires every keyword-plus-reference pair to be read before gh pr create and after any later body edit.
  • A Closing keywords item is added to the Phase 4 universal self-review rubric, directly after Issue resolution. It is scored from gh pr view <number> --json body, and the self-review has to quote each pair it finds.
  • The failure loses work. A wrongly closed issue drops out of the next cycle's triage, and nothing in a later self-audit flags it, because the PR body reads as though leaving the issue open was deliberate. Three generated skills reported it independently (fiefs-dev-loop#19, graphik-dev-loop#5, professions-dev-loop#9).

No placeholder was added. No Step was added or removed, and no phase was renumbered. The Step 4 table and README's "What it does" list therefore stay the same.

Closes #125

Research grounding

No RESEARCH.md finding applies directly. The behavior being guarded against is a property of GitHub's keyword parser, not something the agent-research literature measures. The rubric item follows §1's framing (score from a command output, not judgment). The motivating harm is close to §7's implication that triage decisions should never be filtered silently: a wrongly auto-closed issue is filtered out of triage with no record. This PR does not implement either finding, so no Implementations entry was added.

Doc sync check

  • Every {{placeholder}} added or changed has a corresponding Step 4 substitution-table row (none added; check_docs.py passes)

Test plan

  • python3 scripts/check_docs.py: "Doc consistency check passed."
  • python3 -m unittest discover -s tests: Ran 42 tests, OK
  • UNVERIFIED (behavioral): no /create-dev-loop validation run was performed. This PR came from a headless Gardener dispatch. Its sandbox has no second repository to generate against, and Step 6 would create a GitHub repository, which the dispatch may not do (see [self-audit] The repo's own validation anchor is structurally unrunnable in a headless dispatch, so template changes can never merge autonomously #109). None of the five checklist items in CLAUDE.md's "Testing changes" section were exercised. Under the loop's rules the PR is handed to a human, not auto-merged. The change is one prose paragraph and one rubric bullet with no placeholders or fences, so it can be checked by reading it.

Retrofit

Per CLAUDE.md's "Promoting a rule into the template", the skills that reported this gap likely need a retrofit pass: fiefs-dev-loop, graphik-dev-loop, and professions-dev-loop. More generally, every generated skill that predates this change writes Closes #N and is exposed to the same failure.

gardener interface

Steps 3, 5, and 6 are not touched, so this is not a breaking change for gardener.

Deferred backlog

The other open issues were deferred this cycle for these reasons:

#125 was picked because three skills reported it independently and the change is small.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code


drafted by Claude on behalf of Daniel Stephenson

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review rubric (Phase 4, one intrinsic pass, anchored on doc-consistency green on head ec84115):

Universal

  • Scope: PASS. Only create-dev-loop.md is modified. There are 4 added lines and 1 reworded clause, all in service of Negated closing keywords in PR bodies still auto-close issues #125.
  • Tests-new: N/A. No function was added. The only executable code (scripts/check_docs.py) is untouched.
  • Tests-fix: N/A for automated tests. This is a template-behavior change that no test can reach. See Behavioral validation below.
  • Tests-mutation: N/A, because the PR is not Stage B.
  • Sibling structure: PASS. No new file. The new paragraph copies the bold-lead-sentence form of the "Before pushing, verify each Closes #N" paragraph above it. The new rubric bullet copies the **Name:** condition form of its neighbors.
  • Sibling renames: N/A, because nothing was renamed.
  • Docs: PASS. The Phase 7 table was walked row by row.
    • README.md: the Step list and generated-skill structure do not enumerate the universal rubric items, so neither needed a change.
    • RESEARCH.md: no finding is implemented, and the body says so.
    • CONTRIBUTING.md, the PR template, and the issue templates restate no Phase 4 wording.
    • SECURITY.md: Step 2's read list is unchanged.
    • scripts/README.md: unaffected.
    • CLAUDE.md: its rules still hold.
  • Visuals: N/A, because nothing visible changed.
  • Issue resolution: PASS. Negated closing keywords in PR bodies still auto-close issues #125 asks for two things, and both are present:
    • the reverse-direction check after "verify each Closes #N" (create-dev-loop.md:322)
    • a matching rubric row (create-dev-loop.md:375)
  • CI: PASS. doc-consistency is green on ec84115. Its scope is mechanical parity only and does not verify behavior.

Repo-specific

  • Placeholder parity: PASS. No placeholder was added, and check_docs.py printed "Doc consistency check passed."
  • README Step parity: PASS. No Step changed.
  • Fence escaping: PASS. No fence was added.
  • Phase numbering: PASS. Phases were not renumbered.
  • Version comments: N/A, because the header was not touched.
  • Research grounding: PASS. The body states that no finding applies directly and adds no Implementations entry.
  • No back-ported specifics: PASS. The text names no repo-specific command, reviewer, or prefix.
  • gardener interface: PASS. Steps 3, 5, and 6 were not touched, and the body says so.
  • Retrofit note: PASS. The body names fiefs-dev-loop, graphik-dev-loop, and professions-dev-loop, and notes that every older skill is exposed.
  • Behavioral validation: FAIL (UNVERIFIED). No /create-dev-loop run was possible in this headless dispatch ([self-audit] The repo's own validation anchor is structurally unrunnable in a headless dispatch, so template changes can never merge autonomously #109). Under the rules, auto-merge is withheld and the PR is handed to a human.
  • Closing keywords (the new item, applied to this PR): PASS. A case-insensitive grep of the published body finds exactly one keyword-plus-reference pair, Closes #125, and that close is intended.

Fixed during review

  • "in any case" could be read as "in any situation", so it was reworded to "matched case-insensitively" in ec84115.

Judgment call left for the reviewer

Summary: the change is mechanically consistent and CI is green. It is behaviorally UNVERIFIED and is handed off for a maintainer merge.

This review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).


drafted by Claude on behalf of Daniel Stephenson

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Required real-repository /create-dev-loop checks were not run, leaving generated Phase 4 behavior unverified by documentation-consistency CI and requiring maintainer validation.

No open issues 🎉

What changed in this PR

Updates the generated dev-loop skill’s Phase 4 guidance to prevent unintended issue closures from negated closing keywords, addressing #125.

Changes:

  • Requires checking every closing-keyword/reference pair before PR creation and after later body edits.
  • Documents safe wording for mentioning issues without closing them.
  • Adds a self-review rubric item requiring inspection of the published PR body and quotation of each closing pair.
File Description
create-dev-loop.md Adds closing-keyword guidance and a matching Phase 4 self-review check.

Review effort: Balanced

@dmccoystephenson
dmccoystephenson merged commit a0b80a4 into main Nov 1, 2026
1 check 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.

Negated closing keywords in PR bodies still auto-close issues

2 participants