Skip to content

refactor(source-control): extract babysit-prs skill from pull-request (0.6.0) - #300

Merged
kyle-sexton merged 6 commits into
mainfrom
refactor/extract-babysit-prs
Jul 17, 2026
Merged

refactor(source-control): extract babysit-prs skill from pull-request (0.6.0)#300
kyle-sexton merged 6 commits into
mainfrom
refactor/extract-babysit-prs

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Extracts the all-PR babysit loop from /source-control:pull-request into a standalone
/source-control:babysit-prs skill (distinct discovery intent: fleet loop vs single-PR
lifecycle) and hoists the shared review discipline to plugin scope. Phase 1 of the babysit-prs
migration contract (docs/topics/babysit-prs-migration/PLAN.md on this branch); capability
convergence with the dotfiles skill (tiers, Python engine, userConfig) is the follow-up PR.

Closes #260

Change set

  • New skill plugins/source-control/skills/babysit-prs/ — SKILL.md (compact per-PR
    checklist, NEVER-merges invariant, gotchas from observed failure history),
    reference/loop.md (the former babysit.md, reworked), routing/refusal/anti-pattern evals.
    Phase-1 behavior is identical to the former babysit action (all open non-draft PRs,
    Dependabot included); the own-PRs safe-default narrowing is a Phase 2 deliverable.
  • Plugin-scope seamreference/review-discipline.md is the single committed copy of the
    shared discipline (finding extraction + mandatory ≥3-finding subagent dispatch, per-finding
    D1–D7 verification gates, self-reply filter); both skills' checklists cite it.
    monitor.md's three cross-references into the former babysit reference invert to the seam.
  • Shared scripts hoisted to plugin-root scripts/ via git mv:
    fetch-all-pr-comments.sh, babysit-readiness-gate.sh, test-helpers.sh (+ tests); all
    citations re-rooted to ${CLAUDE_PLUGIN_ROOT}/scripts/. discover-prs.sh retired (inline
    gh pr list filter is the discovery contract).
  • Breaking (0.5.1 → 0.6.0, 0.x breaking-by-minor): the babysit action leaves the
    pull-request surface; pull-request keeps only negative-routing pointers. Eval 9 moved to the
    new skill's eval set (ids stay stable; gap is legal per the evals schema).
  • Metadata: plugin description/keywords, marketplace tags, plugin README skill section,
    CHANGELOG 0.6.0, root README catalog regenerated.

Trigger continuity (skill split — decompose step 5 evidence)

Retired trigger (old pull-request surface) Successor
'babysit PRs' (description trigger) babysit-prs: "Use when: 'babysit PRs', 'babysit my PRs', 'watch my open PRs', 'keep my PRs moving', 'advance all open PRs', or pairing with /loop"
/pull-request babysit (action) /source-control:babysit-prs
/loop /pull-request babysit (loop pairing) /loop /source-control:babysit-prs

Negative routing boundaries: pull-request description now carries "not for the all-PR babysit
loop (use /babysit-prs)"; babysit-prs carries "not for the single-PR lifecycle: prep, create,
monitor one PR, or merge (use /pull-request)". Routing evals: babysit-prs evals 1–2
(vocabulary routes to the loop skill; single-PR lifecycle stays with pull-request).

/skill-quality:check on the same-path pull-request rewrite with
CHECK_SKILL_BASE_REF=origin/main reports the expected check-3 finding — dropped trigger
'babysit PRs' — which this table answers (the checker deliberately skips new paths; this
table + the routing evals are the required split evidence). New-path check on babysit-prs:
PASS, 0 errors, 0 warnings.

Fresh-docs citations (Gate 0)

Naming rationale

babysit-prs carries its object in the leaf name for trigger continuity with the migrated
vocabulary ("babysit PRs" is the phrase users reach with) and to disambiguate from babysitting
other source-control objects; verb-object compound precedent: scan-todos, youtube-digest.

Validation

  • scripts/run-plugin-tests.sh affected tests green (hoisted fetch-all-pr-comments,
    babysit-readiness-gate; re-pointed fetch-annotations, fetch-failed-logs,
    parse-branch-issue); full lane re-verified by CI
  • scripts/validate-plugins.sh exit 0 (contracts, catalog --check, claude plugin validate
    per plugin + --strict)
  • markdownlint + shellcheck clean; exec bits preserved via git mv
  • Non-source-repo smoke (claude --plugin-dir … -p from a clean consumer repo):
    /source-control:babysit-prs discovered with the new description. Known transition-window
    condition: the user-level dotfiles babysit-prs skill shares the vocabulary until the
    migration's cutover phase retires it
  • Residue sweep: grep -ri babysit in the pull-request skill returns only the three deliberate
    sibling pointers; no stale babysit.md or old script-path citations anywhere in the plugin

Contract note

docs/topics/babysit-prs-migration/PLAN.md (Brief + phased Plan) rides this branch and stays
for the follow-up convergence PR; the slice is pruned at the migration's final PR per the
topic-docs close-out.

Related

🤖 Generated with Claude Code

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 31s —— View job


PR Review: refactor(source-control): extract babysit-prs skill from pull-request (0.6.0)

  • Gather context and read changed files
  • Review new babysit-prs skill structure and content
  • Review plugin-scope seam (review-discipline.md)
  • Review hoisted scripts and path re-rooting
  • Review pull-request skill modifications (removals, negative routing)
  • Review metadata, manifests, and changelog
  • Check for security issues and CLAUDE.md compliance
  • Post findings

Summary

Well-executed extraction. The skill split is architecturally sound — the discovery intent distinction (fleet loop vs single-PR lifecycle) is clearly articulated in the routing boundaries, and the plugin-scope review-discipline.md seam is the right call rather than duplicating the D1–D7 gate content in each skill. The trigger continuity table is thorough. Script hoisting is clean and the test re-rooting is correct. One medium finding, two low/informational notes.


Finding 1 — Medium: Hardcoded "main" in conflict PR comment template

File: plugins/source-control/skills/babysit-prs/reference/loop.md

The complex-conflict path in §5.1.2 tells the skill to post this PR comment:

"⚠️ Branch is behind main with integration conflicts ({N} files). Manual resolution is required before CI will trigger."

DEFAULT_BRANCH is already resolved in the same script block (line ~131: DEFAULT_BRANCH=$(gh repo view ...)). The hardcoded "main" here will produce an inaccurate message on any repo whose default branch is master, trunk, develop, or anything else.

Fix: Fix this →

- post a PR comment: `"⚠️ Branch is behind main with integration conflicts ({N} files).`
+ post a PR comment: `"⚠️ Branch is behind $DEFAULT_BRANCH with integration conflicts ({N} files).`

Finding 2 — Low: SEVERITY_PLAIN_RE is over-broad

File: plugins/source-control/scripts/babysit-readiness-gate.sh, line 179

SEVERITY_PLAIN_RE='\[P[0-9]\]'

The pattern matches [P4][P9] which are not part of any documented reviewer severity vocabulary (CRITICAL/IMPORTANT/SUGGESTION for claude[bot]; P0–P3 badge for codex; P1–P3 bracket for common reviewer templates). A comment incidentally containing [P5] or [P8] (e.g. a section heading "Table [P7] — …") would inflate the finding count, causing a false READINESS_BLOCKED.

Suggested fix: constrain to \[P[0-3]\] to match only the documented range (aligning with the existing SEVERITY_BADGE_RE /badge/P[0-3]-). P0 inclusion matches codex's highest severity; P4+ have no known emitter in the review discipline doc.


Finding 3 — Informational: Cross-skill reference coupling in loop.md

File: plugins/source-control/skills/babysit-prs/reference/loop.md, lines 298–307

babysit-prs/reference/loop.md directly references pull-request/reference/readiness.md and pull-request/reference/monitor.md for the re-review trigger logic and CI fix dispatch guidance (§5.1.4). These are intra-plugin relative paths (valid), but they create a maintenance coupling: restructuring or renaming those pull-request reference docs silently breaks the babysit-prs cross-references.

The guidance in §5.1.4 is operational detail (which bots auto-trigger vs need manual @codex review, 3-iteration limit) that isn't obviously owned by the plugin-scope seam or the babysit skill itself. If monitor.md or readiness.md move in a future refactor, this will surface as a quiet broken link rather than a build error. Not a blocker for this PR — just flagging as a future maintenance risk. Inline duplication of the 3-iteration cap and re-review trigger heuristic into loop.md would decouple them.


Other checks

  • Residue sweep — confirmed: grep -ri babysit inside the pull-request skill returns only the three deliberate negative-routing pointers in SKILL.md. No stale babysit.md or old pull-request/scripts/ citations.
  • Script re-rootingfetch-annotations.test.sh and fetch-failed-logs.test.sh both source test-helpers.sh via $(cd "$(dirname "${BASH_SOURCE[0]}")/../../../scripts" && pwd)/test-helpers.sh, correctly resolving to the hoisted plugin-root location.
  • review-discipline.md paths — verified: ../../reference/review-discipline.md from babysit-prs/SKILL.md, ../../../reference/review-discipline.md from babysit-prs/reference/loop.md, and ../../reference/review-discipline.md from pull-request/SKILL.md all resolve to plugins/source-control/reference/review-discipline.md. Correct.
  • Eval gap — the "ids stay stable; gap is legal" claim is consistent with what's visible: pull-request evals lose the former eval 9 (12 lines, one object), babysit-prs ships with IDs 1–6 scoped to its own eval set. No concern.
  • Security — no shell injection risk: PR_NUMBER is embedded in a URL string for gh api, not a shell expansion. BABYSIT_SELF_LOGINS passes through jq, not eval. SELF_CSV processed via IFS=',' read -r -a — safe.
  • Plugin-form compliance — all script citations use ${CLAUDE_PLUGIN_ROOT}/scripts/; no ../ reach-outs; no hardcoded repo names or project-specific values.
  • Versionplugin.json bumped to 0.6.0; marketplace.json updated to match. CHANGELOG entry covers added/changed/removed correctly.
  • set -uo pipefail (no -e) in both scripts — intentional and documented in comments; || guards on every gh API call are present and correct.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author
# Finding Classification Evidence Reacted
1 Hardcoded main in conflict PR comment template (loop.md §5.1.2) VALID — fixing $DEFAULT_BRANCH is resolved in the same §5.1.2 block; the template should use it. Pre-existing text carried over from babysit.md, but in-scope and small 👍
2 SEVERITY_PLAIN_RE='\[P[0-9]\]' over-broad in babysit-readiness-gate.sh:179 VALID — fixing No documented emitter for [P4][P9]; SEVERITY_BADGE_RE already constrains to P[0-3]. Gate tests only assert [P1]/[P2] counting (babysit-readiness-gate.test.sh:242-252), so narrowing to \[P[0-3]\] keeps them green while removing the false-READINESS_BLOCKED path 👍
3 Cross-skill reference coupling in loop.md §5.1.4 → "quiet broken link" risk INCORRECT The repo runs a deterministic offline lychee lane in CI ("Deterministic on-disk link integrity is covered separately by the offline lychee check" — link-check.yml header; lychee.toml:2), so a future rename of monitor.md/readiness.md fails the build rather than breaking silently. Inline duplication of the 3-iteration cap / re-review heuristic would violate the one-committed-copy seam rule (B13) this PR implements; the coupling is deliberate and CI-guarded — (one reaction per comment; 👍 posted for the comment)

Fixes for 1–2 incoming in a follow-up commit.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Findings 1–2 fixed in be8db00: conflict-comment template now uses $DEFAULT_BRANCH; SEVERITY_PLAIN_RE bounded to \[P0\]\[P3\] with a regression test for out-of-range tokens. Finding 3 answered above (INCORRECT — offline lychee lane makes cross-reference breakage a build error; one-copy seam rule bars inline duplication).

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude Code is working…

I'll analyze this and get back to you.

View job run

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f14cec3700

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/source-control/skills/babysit-prs/SKILL.md Outdated
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 19s —— View job


Follow-up Review: refactor(source-control): extract babysit-prs skill from pull-request (0.6.0)

  • Verify fix for Finding 1 ($DEFAULT_BRANCH in loop.md §5.1.2)
  • Verify fix for Finding 2 (SEVERITY_PLAIN_RE bounded to \[P0\]-\[P3\] + regression test)
  • Assess Codex P2 finding re: author filtering in babysit-prs/SKILL.md:80
  • Post findings

Fix verification

Finding 1 (loop.md:233) — confirmed fixed. The conflict-aborted PR comment template now reads "⚠️ Branch is behind $DEFAULT_BRANCH with integration conflicts…". $DEFAULT_BRANCH is resolved at line 130 in the same block; the substitution is correct.

Finding 2 (babysit-readiness-gate.sh:181) — confirmed fixed. SEVERITY_PLAIN_RE='\[P[0-3]\]' is bounded to P0–P3, matching SEVERITY_BADGE_RE. The regression tests at babysit-readiness-gate.test.sh:254-263 cover both the valid in-range case ([P1]/[P2]findings=2) and the out-of-range case ([P7]/[P9]findings=0, READINESS_OK). Clean.


Codex P2 finding — SKILL.md:79 side-effects note

The Codex finding is VALID as a documentation precision issue but Codex's proposed fix direction (add author filtering before checkout) would change Phase 1 behavior that is explicitly designed to be all-PRs.

The specific inaccuracy: SKILL.md:79-80 reads:

Side effects — commits, pushes, replies, reactions, and bot-thread resolution happen autonomously on YOUR OWN open PRs' branches.

But Phase 1's stated purpose (lines 17–18 of the same file) is discovery of "every open non-draft PR in the current repo (regardless of author — Dependabot included)". The checkout block at loop.md:131-153 confirms: gh pr checkout "$PR_NUMBER" runs unconditionally when the tree is clean and the branch isn't in another worktree, setting CHECKOUT_MODE="full" — no author check. A colleague's or Dependabot's same-repo PR branch gets full-mode commits and pushes in Phase 1.

The PR description documents this intentionally ("own-PRs safe-default narrowing is a Phase 2 deliverable"), but the SKILL.md side-effects note contradicts it. An operator reading only SKILL.md receives a false safety guarantee.

Classification: VALID — documentation contradicts actual Phase 1 behavior; a user relying on the note may be surprised when the loop commits to a colleague's PR branch.

Suggested fix for this PR (minimal — preserves the Phase 2 intent without scope-expanding):

- - **Side effects** — commits, pushes, replies, reactions, and bot-thread resolution happen
-   autonomously on YOUR OWN open PRs' branches. Merging never does
+ - **Side effects** — commits, pushes, replies, reactions, and bot-thread resolution happen
+   autonomously on every open PR branch you have write access to (Phase 1 scope: all
+   non-draft PRs regardless of author; own-PRs narrowing is Phase 2). Merging never does

This is the only new finding. The three original findings are fully resolved and the Codex structural concern (broken-link risk) remains correctly rebutted by the offline lychee lane.


@kyle-sexton

Copy link
Copy Markdown
Contributor Author

The side-effects note fix landed in 04ca210 (pushed while this follow-up review was running) — wording now states the all-authors Phase 1 write scope explicitly, matching the suggested direction. All findings across both reviewers are resolved.

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 5s —— View job


  • Verify fix for side-effects scope note in SKILL.md (04ca210)
  • Scan full diff for any remaining issues not caught in prior rounds
  • Post findings

Follow-up Review: fix(source-control): state babysit-prs write scope accurately

Fix verification — 04ca210

Side-effects scope note (SKILL.md:79-81) — confirmed fixed. The note now reads:

commits, pushes, replies, reactions, and bot-thread resolution happen autonomously on the branches of the open PRs the loop processes — every open non-draft PR in the repo regardless of author, wherever the branch is writable. Merging never happens

This accurately matches the Phase 1 discovery contract (all non-draft PRs, any author, full-checkout path in loop.md) and drops the false "YOUR OWN" safety guarantee.

f14cec3 — trivial test-comment hygiene (drops a PR citation from the [P7]/[P9] out-of-range test); no impact on behavior or coverage.

All open findings resolved

Finding Status
1 — Hardcoded main in conflict PR comment template (loop.md §5.1.2) ✅ Fixed in be8db00
2 — SEVERITY_PLAIN_RE='\[P[0-9]\]' over-broad ✅ Fixed in be8db00 (bounded to P[0-3] + regression test)
3 — Cross-skill reference coupling in loop.md §5.1.4 ✅ Rebutted (offline lychee lane in CI makes breakage a build error; one-copy seam rule bars inline duplication)
Codex P2 — side-effects note contradicts Phase 1 write scope ✅ Fixed in 04ca210

No new findings. All security checks, plugin-form compliance, script re-rooting, eval gap, and residue sweep findings from the earlier review round remain clean. This PR is ready to merge.

kyle-sexton and others added 6 commits July 17, 2026 17:50
Interview contract for converging the personal dotfiles babysit-prs skill
with the pull-request babysit mode into a standalone source-control skill
(refs #260): composition seams, tier/merge policy, engine choice, config
surface, concurrency model, PR sequencing, and dotfiles cutover.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fill the Plan section for PR-A (extraction + shared seam): plugin-scope
review-discipline seam, script hoists, babysit-prs skill creation, atomic
residue removal, metadata/catalog, validation gates. Stress-test findings
folded in; later phases stubbed for per-PR architecting.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (0.6.0)

Move the all-PR babysit loop into its own skill (distinct discovery
intent: fleet loop vs single-PR lifecycle) and hoist the shared review
discipline to plugin scope: reference/review-discipline.md (finding
extraction + mandatory >=3-finding subagent dispatch, per-finding D1-D7
verification gates, self-reply filter) and plugin-root scripts/
(fetch-all-pr-comments.sh, babysit-readiness-gate.sh, test-helpers.sh).
monitor.md's three cross-references into the former babysit reference
invert to the seam; pull-request keeps only negative-routing pointers.
discover-prs.sh retired (inline gh filter is the discovery contract).
Breaking: the `babysit` action leaves the pull-request surface.

Closes #260

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Parameterize the conflict-comment template on $DEFAULT_BRANCH (already in
scope in the same block) instead of hardcoding "main", and bound the
readiness gate's plain P-severity pattern to the documented [P0]-[P3]
range so incidental [P4]+ tokens cannot inflate the finding count into a
false READINESS_BLOCKED. Regression test added for the out-of-range case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
comment-hygiene flags tracker references in code comments; the rationale
stands without the citation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The side-effects note claimed autonomous writes are limited to the user's
own PR branches, but discovery covers every open non-draft PR regardless
of author and the fix path pushes to any writable PR branch. Reword to
match actual scope; the own-PRs safe-default narrowing is the convergence
PR's deliverable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton force-pushed the refactor/extract-babysit-prs branch from 04ca210 to 6d91e05 Compare July 17, 2026 22:31
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 58s —— View job


Follow-up Review: refactor(source-control): extract babysit-prs skill from pull-request (0.6.0)

  • Review current branch state and diff from origin/main
  • Verify all prior findings are addressed in HEAD (6d91e05)
  • Scan for any new issues introduced by fix commits
  • Post final findings

Fix verification — HEAD (6d91e05)

All three fix commits are minimal, correct, and scoped to exactly what was reported.

Finding 1 — hardcoded "main" in conflict PR comment template
loop.md:233confirmed fixed (cfafa43). Template now reads "⚠️ Branch is behind $DEFAULT_BRANCH with integration conflicts ({N} files)." $DEFAULT_BRANCH is resolved at the top of the same block.

Finding 2 — SEVERITY_PLAIN_RE over-broad
babysit-readiness-gate.sh:181confirmed fixed (cfafa43). Pattern bounded to '\[P[0-3]\]', now aligned with SEVERITY_BADGE_RE. Regression tests at babysit-readiness-gate.test.sh:250–263 cover both the valid in-range case ([P1]/[P2]findings=2, READINESS_BLOCKED) and the out-of-range case ([P7]/[P9]findings=0, READINESS_OK). Clean.

Codex P2 — side-effects note contradicts Phase 1 write scope
SKILL.md:79–82confirmed fixed (6d91e05). Note now accurately states writes occur on every open non-draft PR branch regardless of author, wherever writable; drops the false "YOUR OWN" safety guarantee. Matches the Phase 1 discovery contract in the same file (lines 17–18) and the checkout path in loop.md.

Finding 3 — cross-skill reference coupling
Correctly rebutted in the prior round — offline lychee CI lane makes a broken reference a build error, not a silent failure; the one-committed-copy seam rule bars inline duplication.


No new findings

The fix commits (cfafa43, 9cc24f0, 6d91e05) are all narrowly scoped: a template parameterization, a regex bound + regression test, and a prose correction. No new code paths, no new shell execution, no new plugin-form concerns. Previous security, compliance, residue, and routing checks remain clean.

All findings resolved. This PR is ready to merge.

@kyle-sexton
kyle-sexton merged commit 9105d3d into main Jul 17, 2026
20 checks passed
@kyle-sexton
kyle-sexton deleted the refactor/extract-babysit-prs branch July 17, 2026 22:40
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.

refactor(source-control): extract babysit from pull-request into its own skill

1 participant