Skip to content

fix(poteto-mode): exclude PR recaps from review acknowledgments - #305

Merged
mdsmithaustin merged 2 commits into
mainfrom
fix/watch-pr-recap-ack
Oct 24, 2026
Merged

mdsmithaustin merged 2 commits into
mainfrom
fix/watch-pr-recap-ack

Conversation

@mdsmithaustin

Copy link
Copy Markdown
Owner

Why

A PR recap that linked a bot review could accidentally acknowledge its findings and clear a review blocker. Recap comments now leave those findings pending for separate triage.

What changed

  • Exclude comments starting with the recap marker, after leading whitespace, from review acknowledgments.
  • Add regression coverage for review findings and merge-gate handling of unread bot reviews, while preserving ordinary trusted acknowledgments.
  • Align the babysit, pr-recap, and porting documentation with this rule.

Scope

Covers recap comments in the shared acknowledgment logic used by watch-pr and merge-gate. Existing author and repository-association trust requirements remain unchanged.

Blast Radius

Touches GitHub bot-review readiness checks. PRs whose findings were acknowledged only through a recap will remain blocked until a separate trusted comment acknowledges the review.

Verification

Inspected the full branch diff. Tests were not run in this drafting phase; the diff adds regression tests in review-bodies.test.ts and merge-gate-cli.test.ts.

✅ Low: The change adds one predicate to acknowledgment(). That function is the only acknowledgment path used by both watch-pr's flagReviewBodies and merge-gate's unread-review gate. The predicate rejects only comments whose trimStart'd body begins with the recap start marker, matching the intent exactly and leaving every other acknowledgment rule unchanged.

Updates from git push no-mistakes

acknowledgment() accepted any trusted non-bot comment that linked a bot
review, so a recap comment with that link cleared the review-findings
blocker in watch-pr and merge-gate without triage. A comment whose
trimmed body starts with <!-- pstack-recap:start --> now never
acknowledges a review. Babysit steps 6 and 8 and pr-recap step 4 state
the exception.
@mdsmithaustin

Copy link
Copy Markdown
Owner Author

The PR 303 verifier found that a pr-recap comment containing a bot review's URL counted as acknowledging that review, so watch-pr and merge-gate would clear its findings with no one triaging them. Now acknowledgment() skips any comment whose trimmed body starts with the recap start marker, so only a separate trusted comment can clear those findings.

Before: a pr-recap comment that links a bot review goes through acknowledgment, which clears the bot findings unread. After: the same pr-recap comment goes through acknowledgment, which now has a recap marker check, and the findings stay open.

Not covered. Both new tests use fixture comments, so no run has checked a live bot review, and a PR whose findings only a recap acknowledged now stays blocked until a separate trusted comment links the review.

Details for reviewers

Risk. Low. The change adds one condition to acknowledgment() that can only refuse an acknowledgment, and tests in review-bodies.test.ts and merge-gate-cli.test.ts pin both the refused recap and an ordinary acknowledgment that still passes.

Base 0bc7cf93 on main, head 42484cc1. 6 files, +43 -4.

Start here. acknowledgment() in review-bodies.ts, where RECAP_START and the new trimStart().startsWith condition sit. It serves watch-pr's flagReviewBodies and merge-gate's unread-review gate.

Then.

  • Tests: review-bodies.test.ts expects an owner's recap that links the review, after a leading newline, to leave it open. merge-gate-cli.test.ts expects exit 10 with review-bodies failed for a recap, and exit 0 for a plain comment that links the same review.
  • Docs: skills/poteto-mode/playbooks/babysit.md steps 6 and 8, skills/pr-recap/SKILL.md step 4, and PORTING.md. They state the recap exception, and PORTING.md now points to the Babysit playbook's rules instead of repeating them.

Proof. On head 42484cc1, the lint run passed, and its skills job passed both new tests. The skill-checks run and CodeQL also passed. No verdict comment is posted on this PR yet. The why comes from the verifier note above and the issue #301 status.

These pointers are a start, not the review. Read the whole diff, because a defect can sit in a file this recap ranks low.

@mdsmithaustin

Copy link
Copy Markdown
Owner Author

Verdict: PASS+NOTES
Head: 42484cc
Patch-id: 12687a7502e171985d0733337b9a424ddac927e1
Docs: pass

Independent verdict (trail reviewer, opus@xhigh), base 0bc7cf9.

  1. One acknowledgment path. acknowledgment() is the only code that counts a comment as acknowledging a review. All watch-pr modes (single, --stack, --queued-stack) and merge-gate build snapshots through readSnapshot → resolveReviewState → flagReviewBodies → acknowledgment(). merge-gate also calls it directly for unread bodies. linksReview is private to review-bodies.ts. verdict.ts uses isTrustedComment only for lines that start with Verdict:, Fail-count reset:, or Stopped:, and a recap's first line is the marker, so it can't match.
  2. Edge cases, run against acknowledgment(). Excluded: leading spaces, tabs, CR/LF, NBSP, and a BOM (trimStart strips U+FEFF). CRLF bodies are also excluded, because the prefix check ends before the first line break. A real acknowledgment that carries the recap start marker later in its body still counts. An otherwise empty recap is excluded, but it has no review link, so it never counted anyway. These are not excluded, which matches the docs' "trimmed body starts with" rule: a leading zero-width space, a heading before the marker, or a marker with different spacing or case. They matter only if a recap departs from skills/pr-recap/references/block.md.
  3. Tests. In skills/poteto-mode/scripts, bun install --frozen-lockfile && bun run test && bun run typecheck gave 543 pass, 1 skip (origin-live), 0 fail, and typecheck exited 0. I swapped the base review-bodies.ts into a scratch copy and both new tests failed, so they catch the regression.
  4. CLI fakes, scratch copy.
    • merge-gate --check with an unread body: a recap alone exits 10 and fails review-bodies. A real acknowledgment exits 0. A recap plus a separate acknowledgment from another collaborator exits 0 in either order, and without --check the same setup calls merge once.
    • watch-pr on the real chore(pstack): remove cloud spawn wording and ship astra trail reviewer #108 fixtures: a recap alone exits 8 with review-findings. A recap followed by the real acknowledgment exits 0 and reports by=mdsmithaustin with the real comment's URL (issuecomment-5945280704), in both the text and JSON output. When the real body comes from another login, by= names that login.
    • At base, the recap-first case reported the recap as the acknowledgment.
  5. Lint and CI. tools/gate-check.sh lint passes all ten checks. Every CI check on the head is green.
  6. Docs (documentation-impact, review mode). babysit.md steps 6 and 8, pr-recap SKILL.md step 4, and the PORTING watcher bullet match the code. The PORTING link resolves. Shipping step 5, references/bugbot-triage.md, and the user guide defer to Babysit step 8 and stay correct.
  7. Optional. The recap start marker is now written in three places that nothing ties together: RECAP_START in review-bodies.ts, the first line of references/block.md, and the cleanup jq in pr-recap SKILL.md. If the marker changes in pr-recap, the hole reopens without warning. A test that compares the constant to block.md's first line would catch that.
  8. Optional. The claim for babysit-body-only-finding-acknowledged-in-pr-comment in evals/promises/ledger.json still says any author or maintainer comment that contains the review URL clears the blocker. It could mention the recap exception. The ledger lint passes as written.

@mdsmithaustin
mdsmithaustin merged commit 8846421 into main Oct 24, 2026
7 checks passed
@mdsmithaustin
mdsmithaustin deleted the fix/watch-pr-recap-ack branch October 24, 2026 17:52
mdsmithaustin added a commit that referenced this pull request Oct 26, 2026
…nt claim (#321)

The ledger claim for `babysit-body-only-finding-acknowledged-in-pr-comment` said any PR comment with the review's URL acknowledges a bot review. Since #305, the watcher skips a comment that opens with the pr-recap start marker, so the claim and the code disagreed.

- The claim now says the acknowledging comment must not open with the pr-recap start marker.
- The claim's script check adds an author pr-recap comment that the watcher must reject.

Closes #315
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.

1 participant