[mdsmithaustin]

Why

Issue #301 left three gaps in the PR recap rules. Nothing tied the recap start marker in review-bodies.ts to the marker the pr-recap template writes. The skill said "never link" a bot review, which a code span or bare URL slips past. Two refresh edges and the Bun minimum were unwritten.

What changed

• Add a review-bodies.test.ts test that builds a recap comment from skills/pr-recap/references/block.md, adds a bot review URL, and requires the review to stay unacknowledged. It does not export RECAP_START.
• Reword pr-recap to "never include a bot review's URL anywhere in the comment, code spans included". Settle two edges in task-triggers.md and babysit.md: a threads-only fix push refreshes the recap, and an operator's hold or zero-writes order wins, so the refresh is reported pending.
• Name Bun 1.4.0 as the minimum for the sketch, with npx --allow-scripts=bun [email protected] ... as the fallback for an older Bun, and replace the "untested on Hermes" line. The skip-the-sketch rule now applies only after that fallback fails, and PORTING.md lists the new threads-only sentence in Babysit step 1.

Scope

Refs #301. The issue stays open because Hermes work remains. Bun releases before 1.4.0 cannot read bun.lock: most fail with Unknown lockfile version (checked on 1.1.39, 1.1.45, 1.2.0, and 1.3.14) and 1.1.38 ignores it. Bun 1.4.0 is the first version that reads it, found by testing 1.3.1 through 1.4.2.

pr-recap ran end to end through Hermes v0.22.4 in a throwaway container with a container-local HERMES_HOME. The sketch exited 0, the PNG was viewed with vision_analyze, and the recap follows block.md. The post steps did not run because the container had no gh or token.

Tradeoffs

This PR leaves out the renderer lever for the Codex plain-word gap. The lever made the render fail while a drawn label shared words with a phrase the reader flagged. I measured it with two codex exec runs per arm on PR 293. The lever never fired, because Codex rewrote the flagged glosses on its own. An independent reader flagged 6 phrases in both arms. I dropped it. Add it back only after a larger measurement.

Blast Radius

Agents that refresh recaps or draw sketches read these edits. The test is the only code change, and it reads a template file. A wrong reading of the hold rule could leave a recap stale.

Verification

• Full poteto-mode suite on the final head: 558 tests, 557 pass, 1 skip (needs the Origin CLI), 0 fail. tsc --strict passes.
• Changing the marker in block.md makes the new test fail. Restoring it makes the file pass, 53 of 53.
• tools/gate-check.sh lint and skill-ci check pass. CI is green on the head.
• The fallback npx --allow-scripts=bun [email protected] "<skill>/scripts/sketch/sketch.mjs" scene.json sketch.png renders flow.json with exit 0 and no problems on npm 11.19.0 and 12.2.1. The old npx [email protected] wording exits 1 on npm 12.2.1 with "Bun's postinstall script was not run".
• Not proven live: an agent omitting bot review URLs, a refresh after a threads-only push, and a deferred refresh during a hold. The gate's live validation was inconclusive, 4 of 6 scenarios, and its Test finding was approved.

✅ Low: The change is wording edits plus one behavioral test. I checked all five intent items against the diff and found no contradictions or defects.

Updates from git push no-mistakes

[mdsmithaustin]

Verdict: FAIL
Head: 5a983b7
Patch-id: 0ac0dfedef12f6b16eadbe6fbe29cd2631ae5974
Docs: needs changes

Root verdict from a full swarm at this head. Two lanes ran things (sonnet@high), and one audit lane (opus@xhigh) read the code. None of them took the PR body as fact.

Gate and live lane: PASS.

• Lint passes. The test tier passes except the 18 statechart failures, which match trunk by name (statechart: 18 TLC-backed tests fail on macOS and never run in CI #308).
• The marker test fails when either block.md or RECAP_START drifts. It goes through the real buildReviewState path.
• [email protected] fails the frozen install and [email protected] passes it.
• I wrote a no-post recap for merged PR 162 by following the head SKILL.md. It contains none of the Copilot review URLs. Fed through head's acknowledgment logic, the flagged review stayed open, including when a review URL was appended to the recap. A plain comment with that URL acknowledged it.

Agent-behavior lane: PASS. I ran 16 fresh sonnet and codex samples on three scenarios: bot review URL, a threads-only fix push, an operator hold. Every head sample answered as designed. Trunk scored the same, so the PR states these rules outright without changing any outcome.

Findings sent back. Each is in scope, and a real caller reaches it.

1. The npx [email protected] fallback fails on npm 12 (skills/pr-recap/references/sketch.md:97). npm 12 blocks install scripts by default, so Bun's postinstall never runs. npm exec --yes [email protected] -- --version exits 1 with "Bun's postinstall script was not run". The PR's own Hermes run hit this on npm 12.0.2. The fix is npx --allow-scripts=bun [email protected], verified on npm 11.19.0 and 12.2.1. Spell out the full command (npx --allow-scripts=bun [email protected] "<skill>/scripts/sketch/sketch.mjs" scene.json sketch.png) so "the same command" can't be read as bun install.
2. sketch.md:97 says every Bun before 1.4.0 fails. Bun 1.1.38 ignores the text bun.lock, installs unpinned, and renders with exit 0. Only 1.2.0 through 1.3.14 fail with "Unknown lockfile version". State the minimum as 1.4.0 without claiming all older versions fail.
3. sketch.md:103 tells the agent to skip when a package can't install. It makes no exception for the line-97 retry. The older-Bun failure prints "could not install the sketch packages", so an agent can match line 103 and skip without trying the fallback. Make line 103 apply after the fallback fails.
4. PORTING.md:49 doesn't list the new port sentence in skills/poteto-mode/playbooks/babysit.md:9. That step came from upstream. A sync can drop it silently. Add it to PORTING.md.

Notes, not blocking:

• The pre-existing ledger claim for babysit-body-only-finding-acknowledged-in-pr-comment omits fix(poteto-mode): exclude PR recaps from review acknowledgments #305's recap exception. Filed as promises: the acknowledgment ledger claim omits the pr-recap exception from #305 #315.
• Babysit's "touches nothing else" sits next to the new exception at babysit.md:9. Several agents had to reason past it, and none misread it.
• The sketch rules say "use real names", but the identifier check rejects names like create_file. This is already on trunk.

[mdsmithaustin]

A recap that linked a bot review could clear that review's blocker, and no test tied the recap marker in review-bodies.ts to the one the template writes. The new review-bodies.test.ts test builds a comment from block.md, adds a bot review URL, and requires the review to stay unacknowledged.

Not covered. The gate's live validation was inconclusive. No run shows an agent leaving out a bot review URL, refreshing after a threads-only push, or deferring the refresh during a hold.

Details for reviewers

Risk. Low. One new test reads a template file, and the rest is wording in skill text that agents read.

Base ee59a17a on main, head a464a2f0. 6 files, +31 -7.

Start here. review-bodies.test.ts line 331 and SKILL.md line 67.

Then.

• Refresh edges: task-triggers.md, babysit.md. A threads-only fix push refreshes the recap, and an operator hold wins, so the refresh is reported pending.
• Sketch setup: skills/pr-recap/references/sketch.md. Bun 1.4.0 is the minimum, with npx --allow-scripts=bun [email protected] as the fallback because npm 12 skips Bun's install script, and Hermes v0.22.4 is named as having run the tool.
• Port record: PORTING.md. It lists the threads-only sentence in babysit.md step 1 next to the refresh trigger line, so a sync keeps both.

Proof. lint run and skill-checks run on the head. Changing the marker in block.md makes the new test fail. The npx fallback renders flow.json with exit 0 and no problems on npm 11.19.0 and 12.2.1, and the old wording exits 1 on npm 12.2.1.

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]

Verdict: PASS+NOTES
Head: a464a2f
Patch-id: 4fdbf256eb3e7180320e01008931371687529b2b
Docs: pass

Root verdict for round 2. A single trail reviewer (opus@xhigh) ran it, because the round changed only wording: 5a983b74..a464a2f0, two files, +3 -3. It replaces the round-1 FAIL at 5a983b74.

• Gates at head: lint passes. The sketch tests pass, 24 of 24. review-bodies.test.ts passes, 53 of 53, including the block.md marker test.
• Live lane: this ran in a fresh copy with empty caches and only Bun 1.3.14 on PATH. The main command exits 2, and line 103 sends the agent to the fallback. The exact sketch.md:97 npx --allow-scripts=bun [email protected] … command exits 0 with problems: [] and writes the PNG on npm 10.9.2, 11.19.0, 12.0.0, 12.0.2 and 12.2.1. As a control, npm 12.2.1 without --allow-scripts=bun exits 1.
• Round-1 findings:
  1. The fallback works on npm 12, and the full command is spelled out.
  2. The Bun text is accurate. 1.4.0 passes. 1.0.0, 1.1.0, 1.1.45, 1.2.0 and 1.3.14 fail. 1.1.38 installs unpinned. CI pins 1.4.0.
  3. The skip rule now applies only after the fallback fails.
  4. PORTING.md lists the babysit.md step-1 sentence.

Notes, filed as #317:

• With Bun 1.1.38, the main command renders unpinned and exits 0 unless the agent checks its Bun version first.
• SKILL.md:60 still skips on hosts with Node but no Bun, though the npx route would work there. That predates this PR.