Skip to content

fix(pr-recap): refresh the recap after any push and skip major pin bumps - #303

Merged
mdsmithaustin merged 8 commits into
mainfrom
fix/pr-recap-301
Oct 24, 2026
Merged

mdsmithaustin merged 8 commits into
mainfrom
fix/pr-recap-301

Conversation

@mdsmithaustin

@mdsmithaustin mdsmithaustin commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

Why

PR recaps need to stay current after pushes from any workflow. Refresh recaps after the new head is published and any gate run reaches its outcome.

What changed

  • Add a shared post-push recap trigger covering user pushes, Babysit and Shipping waves, and gate fix rounds.
  • Point the opening-a-PR playbook and the README at that trigger. pr-recap states the same timing itself, because it can run without poteto-mode.
  • Tell recap writers never to link a bot review, because watch-pr and merge-gate count a trusted comment that links one as acknowledging it. Record the trigger line in PORTING as a port addition to keep on sync.
  • Clarify that version-pin-only changes skip recaps even for major upgrades.

Scope

Covers recap invocation timing and skip guidance. The recap format and generation tooling are unchanged.

Blast Radius

Affects agents following the PR publication and refresh workflows. Incorrect timing could leave a recap stale or refresh it before a gate finishes.

Verification

Reviewed the full diff between the supplied base and target commits. No tests were run in this drafting phase.

✅ Low: These are wording-only instruction edits. The shared post-push refresh trigger is in task-triggers.md, it waits for the gate outcome, and the links from opening-a-pr.md, the README and PORTING.md resolve. The user already chose to ignore the two earlier concerns (the reworded pin-skip reason and the bot-review link rule), and nothing new has turned up since.

Updates from git push no-mistakes

@mdsmithaustin mdsmithaustin changed the title fix: refresh PR recaps after published pushes and gate outcomes fix(pr-recap): refresh the recap after any push and skip major pin bumps Oct 24, 2026
@mdsmithaustin

Copy link
Copy Markdown
Owner Author

Verdict: PASS+NOTES
Head: ea831a5
Patch-id: 366f222a72869e2e4364fd45193680ba8028d278
Docs: needs changes

Independent review on the trail reviewer role. This reviewer did not write the change. The change is instruction text only. I read the four changed files at the head against babysit.md, shipping.md, both autopilot playbooks, opening-a-pr.md, PORTING.md, and the watcher source. I ran the checks below in a detached temporary worktree at the head and removed it afterwards. No agent run exercised the new trigger, so the code verdict stops at PASS+NOTES.

Checks at ea831a5:

  • tools/gate-check.sh lint exits 0, with all 8 lint checks PASS.
  • python3 evals/promises/ledger.py audit exits 0 with 0 problems (945 units, 254 promises, 24 deferred).
  • skill-ci check (v1.4.0) exits 0. Frontmatter has 63 skills and 0 errors, content has 157 files and 0 findings, and 4 manifest checks have 0 failures.
  • The new links resolve: README to opening-a-pr.md and task-triggers.md, and opening-a-pr.md to ../references/task-triggers.md.
  • CI on the head is green (skills, skill-checks, worktree-audit, CodeQL).

Notes:

  1. Coverage of pr-recap: test Hermes, the skip path, and comment replacement live; close the Codex sketch gap #301 N3. The trigger names Babysit, Shipping, and no-mistakes fix rounds. It does not name autopilot. Autopilot owners are covered only through "yours", because they push their own branches and run Babysit waves, and poteto-mode reads task triggers on every invocation. Consider naming autopilot so the line matches N3. In an ungated repository, autopilot owners push after every verifiable unit. Each of those pushes now asks for a full recap rebuild, and each rebuild posts a new comment that notifies subscribers. Consider waiting for the owner's code-ready head. The wait for the gate outcome also keeps an agent inside a gate step (NO_MISTAKES_GATE set) from refreshing, because its own run has not reached an outcome.

  2. No topology conflict. Posting a recap and deleting the older one does not retarget, rebase, or force-push. It does not conflict with Babysit step 4, Shipping step 8, or Autopilot-stack step 8. Two edges stay open. Babysit threads-only "touches nothing else" but can push a fix wave, and the trigger does not say whether that wave refreshes the recap. An operator's zero-writes hold (Autopilot-full step 7) should override a pending refresh, and the trigger does not say so.

  3. The recap is a comment the merge gates read. watch-pr and merge-gate count any trusted non-bot comment that contains a bot review's URL as that review's acknowledgment (acknowledgment() and linksReview() in skills/poteto-mode/scripts/watch-pr/review-bodies.ts, used by merge-gate.ts). pr-recap allows review links as proof. The new timing waits for the gate outcome, so in a gated repository with Copilot review the head's bot review often exists when the refresh posts. A recap that links that review clears its review-findings blocker with no triage. The exposure predates this PR, but this PR extends refreshes into Babysit waves. Suggested follow-up: forbid bot review links in the recap, or make the watcher skip comments that carry the recap start marker.

  4. Timing is consistent. task-triggers.md, pr-recap SKILL.md, opening-a-pr.md, and the README row agree. Refresh after a push that changes the diff, once the new head is published and any gate run has reached its outcome. "any gate run" and "any gate run on it" differ in wording only. The first recap after a PR opens has no such wait. In a gated repository the PR opens in the gate's pr step, before its ci step, so a CI fix round can replace the head that the first recap describes. The trigger then refreshes it, so the cost is one extra recap, not a stale one.

  5. Major-version skip. Skipping is sound, because a version pin has no mechanism to sketch. The stated reason holds for PR 302. Its skills job installed [email protected] from the frozen lockfile, and the pr-recap sketch tests passed. Those tests render each example and check glyph coverage and text overlap through opentype.parse, getAdvanceWidth, and charToGlyphIndex. As general guidance, "because CI ... covers a dependency's behavior" is a claim about the host repository's CI. It is false where no test exercises the dependency. Suggested wording: "because a sketch cannot show a dependency's behavior. Tests on the head must cover it." Also have the skip line name the major bump.

  6. Docs: needs changes. PORTING.md line 49 lists what a sync keeps for pr-recap: "the skill, its trigger row, the Recap paragraph in Opening a PR, ...". This PR moves post-push refresh out of that Recap paragraph into a port-only line in task-triggers.md, and PORTING.md does not name that line. Upstream has no task-triggers.md, because the port split it out of upstream's SKILL.md. A sync that rebuilds the trigger list can drop the line and keep a Recap paragraph that points to it. The deslop and documentation-impact entries name their task triggers for this reason. Add the post-push recap trigger to the pr-recap keep list. README, opening-a-pr.md, task-triggers.md, SKILL.md, docs/guide/06-verify-and-ship.md, and poteto-help agree with each other and need no change.

  7. One stale PR body claim. "Link the opening-a-PR playbook, pr-recap skill, and README to that shared timing rule" is wrong at the head. Commit ea831a5 makes pr-recap SKILL.md state the timing itself, with no link to task triggers. Rewrite that bullet before merge-ready, per the Descriptions rule in opening-a-pr.md.

Add a poteto-mode task trigger that refreshes a PR's recap after any push
that changes its diff, including Babysit and Shipping waves and gate fix
rounds, once the new head is published and the gate run has an outcome.
The rule used to live only in the Opening a PR playbook, so recaps went
stale after later pushes. Say that a version pin counts as a skip even
across a major version.
pr-recap runs without poteto-mode on some hosts, so state the refresh
timing in the skill itself instead of linking into poteto-mode.
pr-recap runs without poteto-mode on some hosts, so state the refresh
timing in the skill itself instead of linking into poteto-mode.
…bot reviews

Name Autopilot owners' pushes in the refresh trigger. Tell recap writers
never to link a bot review, because watch-pr and merge-gate read a
trusted comment that links one as acknowledging it. Ground the major
version skip in what a sketch can show, and record the trigger line as a
port addition to keep on sync.
@mdsmithaustin

Copy link
Copy Markdown
Owner Author

Verdict: PASS+NOTES
Head: 1cbc13d
Patch-id: b8fd8d9638727e5786cbbad86719757943019128
Docs: pass

Re-verification on the trail reviewer role after the rebase onto main at 0d4f8e8 (#297). This reviewer did not write the change. It replaces the verdict for ea831a5. The change is still instruction text only. I checked it by reading and by the checks below, so the code verdict stops at PASS+NOTES.

Checks at 1cbc13d, run in a detached temporary worktree that I removed afterwards:

  • tools/gate-check.sh lint exits 0, with all 8 lint checks PASS.
  • python3 evals/promises/ledger.py audit exits 0 with 0 problems (960 units, 263 promises, 24 deferred).
  • skill-ci check (v1.4.0) exits 0. Frontmatter has 64 skills and 0 errors, content has 158 files and 0 findings, and 4 manifest checks have 0 failures.
  • The README links to opening-a-pr.md and task-triggers.md resolve, and so does opening-a-pr.md's link to ../references/task-triggers.md. The task-triggers path that PORTING.md names exists.
  • CI on the head is green (skills, skill-checks, worktree-audit, CodeQL).

Notes:

  1. Earlier notes are resolved. The trigger now names an Autopilot owner's pushes (N1). SKILL.md step 4 forbids linking a bot's review, and its reason matches acknowledgment() and linksReview() in skills/poteto-mode/scripts/watch-pr/review-bodies.ts (N3). The major-version reason is now "a pin has no mechanism to sketch", which holds in any repository (N5). PORTING.md records the trigger line as a port addition to keep on sync (N6). The PR body now describes pr-recap stating the timing itself (N7).

  2. The rebase kept main's text. PORTING.md differs from main only at line 49, the pr-recap bullet. The statechart bullet on line 50 is byte-identical to main (same SHA-256). "It needs Bun and Node." is still at PORTING.md line 250.

  3. The link rule is narrower than the check. linksReview() matches a review URL anywhere in the comment body, including plain text and code spans, not only Markdown links. An agent can read "Never link a bot's review" as covering Markdown links only. "Never include a bot review's URL" matches the check exactly. I found no open issue for the structural watcher fix. File one so the follow-up has a record.

  4. The N2 edges stand as agreed. The trigger does not say whether a Babysit threads-only fix wave refreshes the recap, or that an operator's zero-writes hold overrides a pending refresh.

  5. Docs pass. task-triggers.md, pr-recap SKILL.md, opening-a-pr.md, the README row, and PORTING.md agree on who refreshes the recap, when, and what a sync keeps. docs/guide/06-verify-and-ship.md and poteto-help are unchanged and still accurate. Two minor body gaps remain. ## Scope says "Covers recap invocation timing and skip guidance" but leaves out the bot-review link rule and the PORTING line that the diff now adds. The first bullet of What changed leaves out Autopilot owner pushes. Consider updating both before merge-ready.

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