fix(tick): reliably emit Closes #NN so a merged bot PR auto-closes its issue #373

Merged

joshtronic merged 1 commit from feat/372-ensure-closes-keyword into master

2026-07-10 18:39:47 +00:00

igor commented

2026-07-10 04:36:59 +00:00

Collaborator

What

Teach the harness to reliably emit a Closes #NN keyword so a merged bot PR auto-closes the issue it resolves (#372).

The bug

bin/tick.sh already appended a bare Closes #<issue> to PR bodies, but with two holes that let an issue survive the merge of its own fix:

Unconditional append — when Claude had already written a closing keyword, the body ended up with it twice.
Non-WIP existing PR left untouched — the append only ran on the new-PR and WIP-finalize paths. When an issue-work PR already existed and was not a WIP checkpoint (i.e. a reworked PR), the finalize block did nothing and the body was left exactly as Claude wrote it. If Claude omitted the keyword, the PR merged without closing its issue. That is exactly #371's bug — it left #369 open after merge.

The fix

New pure helper pr_body_ensure_closes <body> <issue> in lib/checkpoint.sh: idempotent, no-dup, empty-issue-safe, and exact-issue-matched (a body mentioning #3690 does not satisfy #369; a keyword for a different issue still appends). Recognizes close/fix/resolve in any inflection, case-insensitive.
All three PR-body sites (new PR, checkpoint create, WIP finalize) now route through the helper instead of a raw append.
Added the missing else branch: a non-WIP existing PR now gets the keyword guaranteed, editing the body only when it's actually absent. Reuses the PR JSON already fetched for the title (no extra API call).

Tests

bin/test-checkpoint.sh gains 10 cases for pr_body_ensure_closes: empty-guard, append + body-preservation, dedup across Closes/Fixes/resolved inflections, case-insensitivity, #3690≠#369, different-issue-still-appends, and idempotence.

make test green; shellcheck clean on the changed lines (the one remaining SC2015 is pre-existing).

Closes #372

## What Teach the harness to reliably emit a `Closes #NN` keyword so a merged bot PR auto-closes the issue it resolves (#372). ## The bug `bin/tick.sh` already appended a bare `Closes #<issue>` to PR bodies, but with two holes that let an issue survive the merge of its own fix: 1. **Unconditional append** — when Claude had already written a closing keyword, the body ended up with it twice. 2. **Non-WIP existing PR left untouched** — the append only ran on the new-PR and WIP-finalize paths. When an issue-work PR *already existed* and was **not** a WIP checkpoint (i.e. a reworked PR), the finalize block did nothing and the body was left exactly as Claude wrote it. If Claude omitted the keyword, the PR merged **without closing its issue**. That is exactly #371's bug — it left **#369 open** after merge. ## The fix - New pure helper `pr_body_ensure_closes <body> <issue>` in `lib/checkpoint.sh`: idempotent, no-dup, empty-issue-safe, and **exact-issue-matched** (a body mentioning `#3690` does not satisfy `#369`; a keyword for a *different* issue still appends). Recognizes `close/fix/resolve` in any inflection, case-insensitive. - All three PR-body sites (new PR, checkpoint create, WIP finalize) now route through the helper instead of a raw append. - Added the missing `else` branch: a **non-WIP existing PR** now gets the keyword guaranteed, editing the body only when it's actually absent. Reuses the PR JSON already fetched for the title (no extra API call). ## Tests `bin/test-checkpoint.sh` gains 10 cases for `pr_body_ensure_closes`: empty-guard, append + body-preservation, dedup across `Closes`/`Fixes`/`resolved` inflections, case-insensitivity, `#3690`≠`#369`, different-issue-still-appends, and idempotence. `make test` green; `shellcheck` clean on the changed lines (the one remaining SC2015 is pre-existing). Closes #372

igor added 1 commit

2026-07-10 04:36:59 +00:00

fix(tick): reliably emit Closes #NN so a merged bot PR auto-closes its issue

Lint / check-sync (push) Successful in 6s

Details

Lint / check-sync (pull_request) Successful in 5s

Details

983a5567a0

The harness appended a bare `Closes #<issue>` to every PR body, but that
guarantee had two holes that let issues survive their own fix's merge (#372):

  1. The append was unconditional, so when Claude had already written a
     closing keyword the body carried it twice.
  2. Only the NEW-PR and WIP-finalize paths appended. When an issue-work PR
     already existed AND was NOT a WIP checkpoint (a reworked PR), the code
     did nothing -- the body was left exactly as Claude wrote it. If Claude
     omitted the keyword, the PR merged without closing its issue. That is
     #371's bug: it left #369 open after merge.

Route all three PR-body sites (new PR, checkpoint create, WIP finalize)
through a new `pr_body_ensure_closes` helper (lib/checkpoint.sh): idempotent,
no-dup, empty-issue-safe, and exact-issue-matched (so `#3690` never satisfies
`#369`). Add the missing else branch that guarantees the keyword on a non-WIP
existing PR, editing only when it's actually absent.

Pure string helper -> unit-tested in bin/test-checkpoint.sh (10 cases:
empty-guard, append, dedup across close/fix/resolve inflections, exact-match,
idempotence).

Closes #372

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YUVfbfUWdUwifGJ2TvkP

igor commented

2026-07-10 04:39:01 +00:00

Author

Collaborator

🤖 Review — `APPROVE` (automated)

CI for 983a5567: success

Change: Adds a pure pr_body_ensure_closes helper in lib/checkpoint.sh and routes all PR-body construction paths (WIP draft, WIP finalize, new PR) through it, plus a new else branch that guarantees a Closes #NN keyword on a non-WIP existing (reworked) PR. Fixes the double-append and the missing-keyword-on-rework holes behind #372. CI green.

Verdict: APPROVE — correct, in scope (~40 lines), honestly described, well-tested.

Findings

Regex (close[sd]?|fix(e[sd])?|resolve[sd]?)[[:space:]]+#${issue}([^0-9]|$) correctly covers the GitHub/Forgejo keyword inflections, is case-insensitive, and the ([^0-9]|$) guard makes #3690 not satisfy #369. Idempotent because the appended Closes #NN re-matches on a second pass — verified by the round-trip test.
EX_JSON is fetched once and both .title and .body are extracted from it — no extra API call, as claimed. || echo '{}' keeps jq safe on a failed fetch.
The else branch edits the body only when the keyword is actually absent ([ "$EX_NEW" != "$EX_BODY" ]), so it won't churn PRs that already close their issue, and it preserves the existing body (including any count tag) since it appends to the JSON body rather than rebuilding it.
Checklist is accurate: 10 new test cases present, three existing append sites converted, new else branch added.

Minor notes (non-blocking, no action required)

The '#3690' does NOT satisfy #369 -> appends test uses see #3690, which has no closing keyword at all — so it would append regardless and doesn't actually exercise the ([^0-9]|$) boundary. Using Closes #3690 with issue 369 would validate the exact-match claim directly. The regex is correct either way; just a coverage gap.
A body written as Closes: #369 (colon before the space) wouldn't match the [[:space:]]+ requirement and would get a second Closes #369 appended. Harmless (still auto-closes), and rare, but worth knowing.

_{Independent review by the harness on claude-opus-4-8 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `APPROVE` _(automated)_ CI for `983a5567`: **success** **Change:** Adds a pure `pr_body_ensure_closes` helper in `lib/checkpoint.sh` and routes all PR-body construction paths (WIP draft, WIP finalize, new PR) through it, plus a new `else` branch that guarantees a `Closes #NN` keyword on a non-WIP existing (reworked) PR. Fixes the double-append and the missing-keyword-on-rework holes behind #372. CI green. Verdict: **APPROVE** — correct, in scope (~40 lines), honestly described, well-tested. **Findings** - Regex `(close[sd]?|fix(e[sd])?|resolve[sd]?)[[:space:]]+#${issue}([^0-9]|$)` correctly covers the GitHub/Forgejo keyword inflections, is case-insensitive, and the `([^0-9]|$)` guard makes `#3690` not satisfy `#369`. Idempotent because the appended `Closes #NN` re-matches on a second pass — verified by the round-trip test. - `EX_JSON` is fetched once and both `.title` and `.body` are extracted from it — no extra API call, as claimed. `|| echo '{}'` keeps `jq` safe on a failed fetch. - The `else` branch edits the body only when the keyword is actually absent (`[ "$EX_NEW" != "$EX_BODY" ]`), so it won't churn PRs that already close their issue, and it preserves the existing body (including any count tag) since it appends to the JSON body rather than rebuilding it. - Checklist is accurate: 10 new test cases present, three existing append sites converted, new else branch added. **Minor notes (non-blocking, no action required)** - The `'#3690' does NOT satisfy #369 -> appends` test uses `see #3690`, which has no closing keyword at all — so it would append regardless and doesn't actually exercise the `([^0-9]|$)` boundary. Using `Closes #3690` with issue `369` would validate the exact-match claim directly. The regex is correct either way; just a coverage gap. - A body written as `Closes: #369` (colon before the space) wouldn't match the `[[:space:]]+` requirement and would get a second `Closes #369` appended. Harmless (still auto-closes), and rare, but worth knowing. --- <sub>Independent review by the harness on `claude-opus-4-8` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-10 04:39:01 +00:00

1 minute

igor requested review from joshtronic

2026-07-10 04:39:02 +00:00