diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 033819980..76d252dac 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -185,7 +185,7 @@ "name": "source-control", "source": "./plugins/source-control", "category": "development", - "tags": ["delivery", "git", "github", "commit", "pull-request", "worktree", "ci", "skill"] + "tags": ["delivery", "git", "github", "commit", "pull-request", "babysit", "worktree", "ci", "skill"] }, { "name": "planning", diff --git a/README.md b/README.md index efec789f1..89d2fe666 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ Browse and manage with `/plugin`. To refresh after updates: `/plugin marketplace - [`eol-normalizer`](plugins/eol-normalizer) — Normalize a written file's working-tree line endings to its .gitattributes eol value on edit — symmetric CRLF/LF driven by git check-attr, advisory and never blocking. - [`powershell-format`](plugins/powershell-format) — Auto-format and lint PowerShell on edit via PSScriptAnalyzer, only when a PSScriptAnalyzerSettings.psd1 governs the repo — using the consuming repo's own analyzer settings. - [`actionlint`](plugins/actionlint) — Lint GitHub Actions workflow files on edit via actionlint, surfacing findings as advisory context. -- [`source-control`](plugins/source-control) — Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, multi-PR babysit loop), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (interview the repo and write the tracked commit-subject / PR-title convention config), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable per repo via a tracked .claude/source-control.md config written by a re-runnable setup skill; Conventional Commits is the default when no convention is declared. +- [`source-control`](plugins/source-control) — Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing all-PR loop — discover, fix, report readiness, never merges), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (interview the repo and write the tracked commit-subject / PR-title convention config), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable per repo via a tracked .claude/source-control.md config written by a re-runnable setup skill; Conventional Commits is the default when no convention is declared. - [`implementation`](plugins/implementation) — Disciplined implementation stage: execute approved plans inline (`/implementation:implement`) or via orchestrated worker subagents (`/implementation:implement-dispatch`) with incremental validation, TDD-by-default cadence, green-checkpoint commits, scope-fence drift detection, and divergence detection that routes back to planning. Build/test/lint, testing, and outcome verification live in the companion `toolchain`, `testing`, and `verification` plugins, invoked when installed. - [`toolchain`](plugins/toolchain) — Repo-agnostic polyglot verification toolchain: build + test + lint for changed files across .NET, Python, TypeScript, Bash, PowerShell, Markdown, YAML, and cross-cutting surfaces (`/toolchain:check`, `/toolchain:lint`), plus a re-runnable `/toolchain:setup` that writes the tracked per-ecosystem command config those skills resolve first. diff --git a/docs/topics/babysit-prs-migration/PLAN.md b/docs/topics/babysit-prs-migration/PLAN.md new file mode 100644 index 000000000..ebd2e510e --- /dev/null +++ b/docs/topics/babysit-prs-migration/PLAN.md @@ -0,0 +1,365 @@ +# babysit-prs migration + +## Brief + +### TLDR + +Converge two mature babysit implementations — the personal dotfiles skill (`~/.agents/skills/babysit-prs`, worker/autopilot auto-merge tiers, Python guard scripts, lease/worktree orchestration) and the `source-control:pull-request` skill's babysit mode (never-merge fleet loop) — into one new standalone `source-control:babysit-prs` skill, fully repo/machine/user-agnostic per plugin philosophy. Closes #260. The dotfiles skill, its Claude adapter, personal rails doc, and the entire Codex surface are then removed from dotfiles (clean delete, no backports). + +### Goal + +One skill, `/source-control:babysit-prs`, that: + +- runs the fleet loop (discover, per-PR evaluate, fix, report) safe-by-default: own PRs, current + repo's owner, never resolves threads, never merges; +- unlocks `worker` (auto-resolve outdated bot threads, merge behind the deterministic gate) and + `autopilot` (all authors under watched owners, address-then-resolve any thread, merge-gated) as + explicit opt-in tiers; +- ships the Python engine (decomposed from `pr_queue_snapshot.py`) with its relocated test suite; +- shares per-PR review discipline with `pull-request` via a plugin-scope seam; +- is configured via `userConfig` + extended `source-control:setup` (check/apply), with zero baked + identities, paths, or org assumptions. + +### Constraints + +- **Migration input** = chezmoi source tree (`dot_agents/skills/babysit-prs`, last commit + 2026-07-16) **plus** every deployed-only delta analyzed and ported (notably per-repo `--repo` + queue scoping, ported WITH its state-clobber bug fixed and lease/snapshot `--repo` pairing + validated). Chezmoi source itself stays untouched; no dotfiles round-trip for improvements. +- **Composition (B13):** layered plugin-scope shared seam. Shared review discipline (finding + extraction, D1–D7 verification gates, self-reply filter) + `fetch-all-pr-comments.sh` hoisted to + plugin root, cited by both skills via `${CLAUDE_PLUGIN_ROOT}` paths (one committed copy, + delivery-by-version). Babysit owns fleet mechanics (leases, cadence, discovery, fan-out, + sharding); `pull-request` keeps single-PR lifecycle (prep/create/monitor/merge). Workers cite the + shared seam directly — never load the `pull-request` router. `monitor.md`'s three + cross-references into babysit invert cleanly. +- **Engine (B15):** Python snapshot engine is the backbone, decomposed (state store, gh API, + feedback classification, review gate, delta engine, thin CLIs); `discover-prs.sh` retired; + `babysit-readiness-gate.sh` kept at the shared seam. Python is a declared prerequisite for + `worker`/`autopilot` only; the safe default path runs Python-free and degrades gracefully per the + philosophy's failure-behavior rules. +- **Merge tier (B4):** safe default preserves the never-merge invariant; merge exists only behind + explicit tier opt-in + the deterministic gate (`mergeStateStatus == CLEAN` cross-checks; never + `--admin`, never force-push). Accept the plugin-acceptance security-review re-trigger for this + new trust surface. +- **Review-bot module (B11):** the Codex-review vertical generalizes to a bot-agnostic AI-review + trigger + gate module — config supplies trigger phrase, reviewer logins, gate context names; must + handle inline review threads, posted summary comments (Claude-Code-review style), and check-run + gates. Bot identity detection stays structural (`__typename == "Bot"` / `[bot]` suffix); zero + hardcoded identities. No adapter layer until a real misfit appears. +- **Concurrency (B14):** layered detection over locking. L1 head-SHA recheck + push rejection; + L2 design rule — every one-shot action derives dedup evidence from GitHub, local state is cache + only; L3 foreign-activity detector (mutation-ledger vs same-login timeline diff → back off, + report contention). File leases coordinate same-machine sessions under `${CLAUDE_PLUGIN_DATA}`; + `--repo` sharding is the multi-tab contract. +- **Config (B5/B6):** zero user/machine/repo-specific content in the skill. Personal scalars → + `userConfig` (watched owners, self logins, default tier, merge method, cadence bounds, fix-round + cap, review-trigger settings, worktree root defaulting to `${CLAUDE_PLUGIN_DATA}/worktrees`); + delivery to scripts via prose substitution → CLI flags (option env vars do not reach + skill-invoked Bash); custom `BABYSIT_*` env seams retire per doctrine. Setup = extend + `source-control:setup` (check + apply, idempotent). Dependabot/dependency PRs: hold-merge is the + generic safe default in every tier. +- **Packaging:** guarded-helper wrappers move to plugin `bin/` with collision-safe plugin-prefixed + names (auto-mode-surviving bare-name allow rules). Scripts ship `--help` + sibling + cross-platform `.test.sh`; the 6,744-line Python test suite relocates into this repo's + plugin-tests lane; wire Python lint/type coverage (ruff/pyright) or record the gap explicitly. + UTF-8 handled inside scripts; invocation examples shell-neutral. Trigger-continuity migration + table + routing evals required (skill split creates new paths `/skill-quality:check` skips). +- **Improvement backlog rides the migration** (no separate pre-pass): merge-gate `(type, name)` + check dedup keying; bot-login fallback list → config; duplicate GraphQL paginators and subprocess + runners collapsed; cadence cross-cycle counters persisted in the state file; SKILL.md/reference + prose drift resolved. `references/plugin-migration.md` dies, superseded by this contract. +- **Process:** PR-A (extraction + shared seam, closes #260) → PR-B (capability convergence — the + security-review PR) → cutover → dotfiles removal PR. Conventional-Commit titles, per-plugin + CHANGELOG (Keep a Changelog, 0.x breaking-by-minor), `claude plugin validate --strict` from a + non-source repo, PII gate before first commit. PR #256 naming grammar is binding but its merge is + not a blocker; coordinate rebases with #271 (`prep.md` touch). + +### Acceptance criteria + +1. `/source-control:babysit-prs` exists; `pull-request` no longer mentions babysit (description, + action table, checklists, README, eval 9, monitor cross-references all updated atomically). +2. Bare invocation with zero config in a fresh consumer repo: discovers own PRs under the current + repo's owner, fixes clear branch-owned issues, reports, never resolves/merges. No Python needed. +3. `worker`/`autopilot` tiers function end-to-end on this fleet (leases, fan-out, gate-checked + merge, thread resolution) with config supplied via userConfig/setup — no hardcoded owners, + logins, contexts, or paths anywhere in the plugin (CI `machine-specific-paths` + gitleaks green). +4. Ported `--repo` sharding: two concurrent same-machine sessions scoped to different repos share + no lease contention and do not clobber each other's snapshot state. +5. Relocated test suite green in the plugin-tests lane; every script has `--help` + `.test.sh`; + `claude plugin validate --strict` passes; trigger-continuity table + evals present. +6. Cutover complete: active loop killed, plugin babysits this fleet in production, dotfiles + kill-list removed (B9 list), `chezmoi status` clean, loop restarted on the plugin skill. +7. Freshness/re-anchor skill (B16): issue filed with boundary definition + (vs `source-control:worktree` status and babysit rescan), then implemented — sequenced last. + +### Captured assumptions + +- GitHub-side AI reviewers (Codex/Claude/Cursor bots) remain in use org-side; only the local Codex + runtime surface dies. The generalized review-trigger module keeps the Codex flow working via + config. +- One machine owns a given repo's babysit at a time (human convention, L3-detected, not enforced). +- `find-skill-candidates` remains in `~/.agents` — out of scope here. + +### Out-of-scope + +- Cross-machine lease/claim infrastructure (L4 claim markers) — deferred; trigger: L1–L3 detection + proves insufficient in real multi-machine operation. +- Review-bot interface/adapter layer — deferred; trigger: a feedback form config cannot express. +- Any Codex backport, parity shim, or dangling compatibility path — deliberately excluded. +- Changes to `standards` / `ci-workflows` / `github-iac` repos. + +### Deferred questions + +- Exact userConfig key names/types, module boundaries of the engine decomposition, evals content, + lint-coverage wiring, L3 detector mechanics — arbiter: /architect (per phase PR). +- Freshness skill name, plugin placement (re-anchor vs elsewhere), and scan boundaries — arbiter: + USER-RESERVED (shapes B16 scope; settle in its issue). +- Merge-method default resolution order (repo convention → squash) — arbiter: /architect. + +## Plan + +### Phase 1: PR-A — extraction + shared seam (closes #260) [DOING] + +Extract babysit from `pull-request` into a sibling `babysit-prs` skill with behavior identical to +today's babysit mode (safe default only — tiers, engine, and config arrive in Phase 2), and hoist +the shared review discipline to plugin scope so neither skill loads the other's router. + +**Step 0 — Gate 0 fresh-docs.** WebFetch the current official skills + plugins-reference pages +(URL table in root CLAUDE.md) before any manifest/structure change; cite both in the PR body. + +**Step 1 — Hoist the shared seam to plugin scope.** + +- Create `plugins/source-control/reference/review-discipline.md` — canonical home of the shared + per-PR review discipline, extracted from `reference/babysit.md`: evidence-based fresh rescan + + self-reply filter (§5.0.3), structured finding extraction + mandatory ≥3-finding subagent + dispatch with the verbatim scope-fenced prompt + main-session contract (§5.0.4), and per-finding + D1–D7 with GitHub-verification gates (§5.1.3 D-steps). Content moves verbatim-with-renumbering; + babysit-specific framing (round-robin, wake scheduling) stays out. +- `git mv` to `plugins/source-control/scripts/`: `fetch-all-pr-comments.sh` + `.test.sh`, + `babysit-readiness-gate.sh` + `.test.sh`, `test-helpers.sh` (single shared copy; sourced by + tests on both sides). Update the two remaining pull-request test scripts that source it + (`fetch-annotations.test.sh`, `fetch-failed-logs.test.sh`) to source + `../../../scripts/test-helpers.sh` (and their `# shellcheck source=` directives); + `parse-branch-issue.test.sh` never sourced the helper. +- Re-root every `${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/fetch-all-pr-comments.sh` and + `.../babysit-readiness-gate.sh` citation to `${CLAUDE_PLUGIN_ROOT}/scripts/...`. +- Update `babysit-readiness-gate.sh`'s four internal doc-comment citations of `babysit.md §5.0.4` + (lines 8, 21, 164, 216) to cite `review-discipline.md` with the post-renumbering section id. +- Layering end-state (one committed copy per B13): `review-discipline.md` is the canonical + DETAILED home (extraction rules + subagent prompt + table formats + per-finding verification + gates + self-reply filter); pull-request SKILL.md and the new babysit-prs SKILL.md each keep + only the compact always-loaded checkbox skeleton citing the seam for detail; monitor.md keeps + its monitor-specific batch-flow framing (§3.3) and cites the seam for the shared rules. No + third detailed copy anywhere. + +**Step 2 — Create the `babysit-prs` skill.** + +- `git mv skills/pull-request/reference/babysit.md skills/babysit-prs/reference/loop.md`, then + rework: hoisted sections (§5.0.3/§5.0.4/§5.1.3 discipline core) collapse to citations of the + plugin-scope seam; self-pacing prompt strings become `/source-control:babysit-prs`; §5.0.2 keeps + the inline `gh pr list` filter (deterministic-equivalent script retired); §5.1.1 carries its own + compact event-delivery gate (cloud check → push channel → Monitor tool), with deep push-channel + health detail cited from `pull-request`'s monitor reference (same-plugin file link, not the + router). +- Retarget every cross-file prose reference in the moved content (grep-enumerated: + `:169` "Monitor entry checklist from SKILL.md" → the skill's own SKILL.md checklist; `:318` + "canonical policy: SKILL.md D7.5" → the seam file; `:337` `readiness.md` "Expected PR actors" and + `:342` "monitor §3.2" / monitor.md "Inline vs subagent dispatch decision" and `:346` + "differs from monitor.md step 4" → full same-plugin + `${CLAUDE_PLUGIN_ROOT}/skills/pull-request/reference/...` citations or seam citations — never + the pull-request router). Re-grep after rework: no bare `SKILL.md`/`monitor.md`/`readiness.md` + reference in babysit-prs resolves to a pull-request surface implicitly. +- New `skills/babysit-prs/SKILL.md`: frontmatter (`name: babysit-prs`, user-invocable, + description = imperative + quoted `Use when:` triggers carrying the migrated phrases + ('babysit PRs', 'babysit my PRs', 'watch my open PRs', 'keep my PRs moving', /loop pairing) + + negative routing ("not for single-PR prep/create/monitor/merge — use /pull-request")); purpose + + never-merges invariant; the per-PR checklist (adapted from pull-request SKILL.md's babysit + block); the NEVER list; checklist-driven output contract; pre-computed context block. +- **Phase-1 behavior honesty:** the description and purpose MUST describe today's discovery scope + — every open non-draft PR in the current repo regardless of author (Dependabot included). The + Brief's own-PRs-under-current-owner safe default is a Phase 2 deliverable (named in the Phase 2 + stub); Phase 1 ships behavior identical to the existing babysit action. +- New `skills/babysit-prs/evals/evals.json` (schema-valid, rich form): routing eval ('babysit + my PRs' → this skill, not pull-request), negative-routing eval ('create pr' → pull-request), + happy-path iteration eval, refusal eval (asked to merge a ready PR → declares readiness, + refuses merge), anti-pattern eval (survey-without-classifying → violation; gate blocks + readiness), plus ported eval 9 (merge-workflow preservation) with the new invocation. + +**Step 3 — Remove babysit residue from `pull-request` atomically.** + +- `SKILL.md`: description drops 'babysit PRs' and gains the negative route to the sibling; action + table loses the `babysit` row; phases table loses `3+. Babysit`; the "Babysit per-PR checklist" + block is deleted; monitor-entry Step 0 and per-iteration C4/D notes re-point their babysit.md + citations to the plugin-scope seam or the sibling skill; script citations re-rooted. +- `reference/monitor.md`: the three cross-references into babysit.md (§5.0.4 extraction rules, + §5.1.3 verification gates, §5.0.3 self-reply filter) invert to + `plugins/source-control/reference/review-discipline.md`; script paths re-rooted. +- `evals/evals.json`: eval id 9 removed (ids stay stable; gap is legal per schema). +- Delete `scripts/discover-prs.sh` + `.test.sh` (retired per B15; inline `gh` filter is the + contract). +- Sweep: `grep -ri babysit plugins/source-control/skills/pull-request/` returns only the + deliberate sibling pointers (negative route, action-table pointer, monitor-entry note). + +**Step 4 — Plugin metadata + catalog.** + +- `plugin.json`: version `0.5.1` → `0.6.0` (0.x breaking-by-minor — the `babysit` action leaves + the pull-request surface); description names both skills. +- `CHANGELOG.md`: `0.6.0` — Added (babysit-prs skill, plugin-scope review-discipline seam), + Changed (pull-request babysit removal + re-rooted shared scripts, breaking), Removed + (discover-prs.sh). +- `plugins/source-control/README.md`: babysit bullet becomes a `/source-control:babysit-prs` + skill section (loop invocation updated to `/loop /source-control:babysit-prs`); + `BABYSIT_SELF_LOGINS` row stays (unchanged behavior until Phase 2's userConfig). +- `.claude-plugin/marketplace.json`: source-control tags gain `babysit`; `plugin.json` `keywords` + gains `babysit` in the same edit (same vocabulary axis). +- Root README regenerated via `node scripts/generate-catalog.mjs`. + +**Step 5 — Validation + PR.** + +- `bash scripts/run-plugin-tests.sh` (hoisted + remaining tests green), `bash + scripts/validate-plugins.sh` (contracts, catalog `--check`, `claude plugin validate` per + plugin and `--strict`), markdownlint, shellcheck, `claude --plugin-dir` smoke from a + non-source repo (exercise `/source-control:babysit-prs` slash + automatic routing on 'babysit + my PRs'). +- `/skill-quality:check` on `pull-request` with `CHECK_SKILL_BASE_REF` set to the pre-change ref + (playbook decompose step 5 — same-path rewrite): expect check 3 to flag the dropped + `'babysit PRs'` trigger; the PR-body trigger-continuity table is the recorded answer. +- **Routing-smoke isolation:** the user-level dotfiles `/babysit-prs` skill is global and matches + the same vocabulary — for the automatic-routing smoke, run a session with it disabled (or + assert the plugin skill loads via explicit `/source-control:babysit-prs` and record the + two-skills-one-vocabulary ambiguity as a known transition-window condition closed by Phase 3 + cutover). +- PR body: `Closes #260`, trigger-continuity migration table (each retired trigger phrase → the + successor's quoted `Use when:` phrase + negative routing boundaries), Gate 0 citations, naming + rationale for the verb-object leaf (`babysit-prs` carries its object for trigger continuity + with the migrated vocabulary + disambiguation, verb-object compound precedent: scan-todos, + youtube-digest), change set. + Title: `refactor(source-control): extract babysit-prs skill from pull-request (0.6.0)`. +- Babysit the PR to merge with the current user-level `/babysit-prs` skill. + +**Phase 1 file inventory:** + +| File | Action | +|---|---| +| `plugins/source-control/reference/review-discipline.md` | CREATE (extracted content) | +| `plugins/source-control/scripts/fetch-all-pr-comments.sh` + `.test.sh` | MOVE (git mv) | +| `plugins/source-control/scripts/babysit-readiness-gate.sh` + `.test.sh` | MOVE (git mv) | +| `plugins/source-control/scripts/test-helpers.sh` | MOVE (git mv) | +| `plugins/source-control/skills/babysit-prs/reference/loop.md` | MOVE (git mv babysit.md) + rework | +| `plugins/source-control/skills/babysit-prs/SKILL.md` | CREATE | +| `plugins/source-control/skills/babysit-prs/evals/evals.json` | CREATE | +| `plugins/source-control/skills/pull-request/SKILL.md` | MODIFY | +| `plugins/source-control/skills/pull-request/reference/monitor.md` | MODIFY | +| `plugins/source-control/skills/pull-request/evals/evals.json` | MODIFY (drop id 9) | +| `plugins/source-control/skills/pull-request/scripts/{fetch-annotations,fetch-failed-logs}.test.sh` | MODIFY (helper path) | +| `plugins/source-control/skills/pull-request/scripts/discover-prs.sh` + `.test.sh` | DELETE | +| `plugins/source-control/.claude-plugin/plugin.json` | MODIFY (0.6.0) | +| `plugins/source-control/CHANGELOG.md` | MODIFY | +| `plugins/source-control/README.md` | MODIFY | +| `.claude-plugin/marketplace.json` | MODIFY (tags) | +| `README.md` | REGENERATE (catalog) | +| `plugins/source-control/skills/pull-request/reference/{prep,create,merge,readiness}.md`, `templates/checklist.md` | KEEP (grep-verified no babysit refs) | + +**Sanity Check:** + +- `grep -ri babysit plugins/source-control/skills/pull-request/` → only the deliberate sibling + pointers (description negative route, action-table pointer, monitor-entry Step 0 note); zero + babysit-owned content. +- `grep -rn "skills/pull-request/scripts/\(fetch-all-pr-comments\|babysit-readiness-gate\|discover-prs\|test-helpers\)" plugins/` → zero matches. +- `grep -rn "babysit\.md" plugins/source-control/` → zero matches (gate-script doc comments and + all prose citations retargeted). +- `bash scripts/run-plugin-tests.sh` exit 0; `bash scripts/validate-plugins.sh` exit 0; + `node scripts/generate-catalog.mjs --check` exit 0. +- `python -c "import json; d=json.load(open('plugins/source-control/skills/babysit-prs/evals/evals.json')); assert len(d['evals'])>=6"` exit 0. +- Non-source-repo smoke: `/source-control:babysit-prs` resolves and loads; 'babysit my PRs' + routes to it. + +### Phase 2: PR-B — capability convergence [TODO] + +*(architect per phase PR — engine decomposition module boundaries, tiers + merge gate, +review-trigger module, userConfig/setup, bin/ wrappers, L2/L3 concurrency, test-suite relocation. +Also: narrow the safe default's discovery scope from all-open-PRs (Phase 1 parity) to own PRs +under the current repo's owner per the Brief's Goal/AC2. Independent lanes orchestrated after +boundaries are fixed.)* + +### Phase 3: Cutover [TODO] + +*(loop kill → plugin install/enable → setup apply → smoke test → dotfiles removal PR (kill-list +B9) → MANUAL chezmoi apply → loop restart on `/source-control:babysit-prs worker`.)* + +### Phase 4: B16 freshness/re-anchor skill [TODO] + +*(issue first — name/placement USER-RESERVED; note the `re-anchor` plugin now exists (PR #293) +as a placement candidate; implement after user sign-off.)* + +## Blast radius + +MEDIUM — Phase 1 restructures the plugin's highest-traffic skill surface and its script paths; +consumers of `/pull-request babysit` (the user's live loop) lose the action at upgrade time, which +the cutover phase handles deliberately. No cross-plugin or cross-repo edits; CI gates +(plugin-tests, validate, catalog, markdownlint, shellcheck) cover every changed artifact class. + +## Stress-test summary + +Fresh-context plan reviewer (Step 3) returned 6 IMPORTANT + 2 SUGGESTION findings, all verified +against the actual files and folded into Phase 1: gate-script internal `babysit.md` citations +(sweep pattern extended), unenumerated cross-file prose references in the moved content (explicit +retarget list added), the mandated `/skill-quality:check` `CHECK_SKILL_BASE_REF` run on the +same-path pull-request rewrite (added to Step 5), routing-smoke confound with the still-installed +user-level skill (isolation mechanism stated), Phase-1 discovery-scope honesty vs the Brief's +Phase-2 safe-default narrowing (pinned in Step 2 + Phase 2 stub), discipline-layering end-state +(one detailed copy at the seam, compact skeletons cite it), plus keywords/tags parity and the +naming-rationale PR-body note. Reviewer verified clean: test-runner discovery, no fixture/`../` +couplings in moved tests, no new drift-check cluster, catalog generation, exec-bit preservation, +eval-id gap legality, and the KEEP rows. Blast radius MEDIUM with all triggers covered by CI +gates + the Phase-3 cutover design — no `/devils-advocate` escalation warranted (the extraction +is structure-preserving; the new trust surface arrives in Phase 2, which gets its own pass). + +## Execution shape + +Phase 1 is fully sequential in the main session — every step edits the same tightly-coupled skill +pair (SKILL.md ↔ reference ↔ scripts ↔ evals ↔ metadata), and total churn is well under the +parallelism-payoff threshold. Phase 2's execution shape is computed by its own architect pass. + +| Phase | Surface | Basis | +|---|---|---| +| 1 (PR-A) | main session, sequential | coupled edits, judgment-heavy rewording | +| 2 (PR-B) | orchestrated lanes (per its architect) | script decomposition / reference rewrites / test porting are file-disjoint | +| 3 (cutover) | main session | live-loop + machine state, inherently serial | +| 4 (B16) | main session | issue-writing + user gate | + +## Open questions + +None for Phase 1. Phase 2 deferred questions (engine module boundaries, userConfig keys, eval +content, lint wiring, L3 mechanics) resolve at its architect pass; B16 naming/placement is +USER-RESERVED. + +## Handoff to implementation + +### User-approval gates + +- The overall execution was pre-approved top-to-bottom by the user's handoff directive; Phase 1 + contains no further gates. +- Phase 3 retains the MANUAL `chezmoi apply` step (never from a loop session). +- Phase 4 blocks on user sign-off of the B16 issue (name/placement USER-RESERVED). + +### Execution shape ([EXEC-SHAPE] tagged) + +| Decision | What it changes in the plan | Basis | +|---|---|---| +| [EXEC-SHAPE] Retire `discover-prs.sh` in Phase 1, not Phase 2 | Step 3 deletes it; loop.md §5.0.2 keeps the inline `gh` filter | B15 already retired it; porting it into the new skill only to delete next PR is churn | +| [EXEC-SHAPE] Seam artifacts land at `plugins/source-control/reference/` + `plugins/source-control/scripts/` | Steps 1–2 paths | repo precedent: 9 plugins ship plugin-root `reference/`, 2 ship plugin-root `scripts/` with `.test.sh` (skill-quality); playbook step 4 mandates plugin-scope shared policy | +| [EXEC-SHAPE] `babysit.md` → `reference/loop.md` via `git mv` + rework | Step 2 history-preserving move | rename-PR precedent (history preserved); content survives mostly intact | +| [EXEC-SHAPE] `test-helpers.sh` hoists (single copy) | Step 1; three remaining tests re-point | no-duplication rule; hoisted tests need it; helper is skill-agnostic | +| [EXEC-SHAPE] eval id 9 removed without renumbering | Step 3 | schema declares ids stable identifiers; gaps legal | +| [EXEC-SHAPE] version 0.6.0 | Step 4 | 0.x breaking-by-minor: `babysit` action leaves the pull-request surface | +| [EXEC-SHAPE] marketplace tags gain `babysit` | Step 4 | discovery parity with the moved trigger vocabulary | + +### Mechanical work + +- One commit per plan step is not required; commit at green checkpoints with Conventional-Commit + subjects; PLAN.md phase-tag updates ride the same commits. +- Sequential fallback: not applicable in Phase 1 (already sequential). +- PII gate before first commit: moved content is already public in-repo; new content carries no + identities beyond the generic `gh api user` default. diff --git a/docs/topics/babysit-prs-migration/design/design-resolution.md b/docs/topics/babysit-prs-migration/design/design-resolution.md new file mode 100644 index 000000000..d87a2af48 --- /dev/null +++ b/docs/topics/babysit-prs-migration/design/design-resolution.md @@ -0,0 +1,12 @@ +# Design resolution — babysit-prs migration + +outcome: early-exit + +The composition design was settled during the interview stage and is locked in the Brief's +constraints: layered plugin-scope shared seam (B13 — shared review discipline + comment fetcher at +plugin root, cited by both skills; babysit owns fleet mechanics, `pull-request` keeps single-PR +lifecycle), Python engine backbone with Python-free safe default (B15), bot-agnostic review-trigger +module (B11), layered concurrency detection (B14), userConfig-first configuration (B5/B6). No new +programming-language types or public API contracts are introduced in Phase 1 — the deliverables are +markdown skill surfaces and relocated shell scripts. Engine module boundaries (Phase 2) are +explicitly deferred to the per-phase architect pass per the Brief's deferred-questions list. diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index 21b4de0b7..5037e13e5 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,12 +1,12 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.5.2", - "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, multi-PR babysit loop), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (interview the repo and write the tracked commit-subject / PR-title convention config), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable per repo via a tracked .claude/source-control.md config written by a re-runnable setup skill; Conventional Commits is the default when no convention is declared.", + "version": "0.6.0", + "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing all-PR loop — discover, fix, report readiness, never merges), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (interview the repo and write the tracked commit-subject / PR-title convention config), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable per repo via a tracked .claude/source-control.md config written by a re-runnable setup skill; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", "email": "info@melodicsoftware.com" }, "license": "MIT", - "keywords": ["git", "github", "commit", "pull-request", "worktree", "merge-conflict", "rebase", "ci", "code-review", "delivery", "skill"] + "keywords": ["git", "github", "commit", "pull-request", "babysit", "worktree", "merge-conflict", "rebase", "ci", "code-review", "delivery", "skill"] } diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 1b7b525c8..baccd2bf9 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,36 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.6.0] + +### Added + +- **New `/source-control:babysit-prs` skill** — the all-PR self-pacing babysit loop, extracted + from `/source-control:pull-request` into its own skill (distinct discovery intent: fleet loop + vs single-PR lifecycle). Same behavior as the former `babysit` action: discovers every open + non-draft PR oldest-first, checks each out, keeps branches fresh, classifies every review + finding with GitHub-verified evidence, fixes valid findings, reports readiness. Never merges. + Invoke via `/source-control:babysit-prs` (loop pairing: `/loop /source-control:babysit-prs`). +- **Plugin-scope shared review discipline** at `reference/review-discipline.md` — the canonical + home of finding extraction (with the mandatory ≥3-finding subagent dispatch), per-finding + D1–D7 verification gates, and self-reply filtering, cited by both `pull-request` and + `babysit-prs` instead of duplicating the rules per skill. + +### Changed + +- **Breaking:** the `babysit` action is removed from `/source-control:pull-request` — use + `/source-control:babysit-prs`. The pull-request description, action table, phase table, and + checklists no longer carry babysit content; `reference/monitor.md`'s cross-references into the + former babysit reference now cite the plugin-scope review discipline. +- Shared scripts hoisted from `skills/pull-request/scripts/` to plugin-root `scripts/` + (`fetch-all-pr-comments.sh`, `babysit-readiness-gate.sh`, `test-helpers.sh`, with their + tests) — cited by both skills via `${CLAUDE_PLUGIN_ROOT}/scripts/`. + +### Removed + +- `discover-prs.sh` (+ test) — retired; the inline `gh pr list` filter in the babysit-prs + reference is the discovery contract. + ## [0.5.2] ### Fixed diff --git a/plugins/source-control/README.md b/plugins/source-control/README.md index 7052a9cb7..b18a23f55 100644 --- a/plugins/source-control/README.md +++ b/plugins/source-control/README.md @@ -1,8 +1,9 @@ # source-control -A Claude Code plugin bundling the git/GitHub delivery workflow as five -composable skills — commit mechanics, the full PR lifecycle, worktree -lifecycle management, convention setup, and merge-conflict resolution. +A Claude Code plugin bundling the git/GitHub delivery workflow as six +composable skills — commit mechanics, the single-PR lifecycle, the all-PR +babysit loop, worktree lifecycle management, convention setup, and +merge-conflict resolution. ## Skills @@ -43,15 +44,19 @@ research-gated: react → reply → fix → verify-on-GitHub treatment. - **merge** — 6-gate readiness re-verification, squash merge, worktree reuse/cleanup, post-merge CI health check. Never auto-merges. -- **babysit** — self-pacing all-PR loop (designed for - `/loop /pull-request babysit`): discovers every open PR, checks each out, - processes every finding individually with GitHub-verified evidence, and is - mechanically gated by the bundled `babysit-readiness-gate.sh` (classification - rows must cover source findings before readiness can be declared). Never - merges. - **fetch-logs** — tiered CI-log retrieval (annotations → full untruncated ZIP via the REST API → per-job text). +### `/source-control:babysit-prs` + +Self-pacing all-PR loop (designed for `/loop /source-control:babysit-prs`): +discovers every open non-draft PR, checks each out, keeps the branch fresh, +processes every review finding individually with GitHub-verified evidence +per the plugin-scope shared review discipline, and is mechanically gated by +the bundled `babysit-readiness-gate.sh` (classification rows must cover +source findings before readiness can be declared). Never merges — readiness +is reported; the user merges. + ### `/source-control:worktree` Git worktree lifecycle for parallel-session isolation: `create` (guided @@ -116,7 +121,7 @@ Optional environment variables: | Variable | Used by | Effect | |---|---|---| | `WORKTREE_STALE_DAYS` | `/worktree status` | Staleness threshold (default 14 days) | -| `BABYSIT_SELF_LOGINS` | babysit readiness gate | Extra posting identities (csv) whose replies count as your classification rows — e.g. a project bot account (default: your `gh api user` login) | +| `BABYSIT_SELF_LOGINS` | `/babysit-prs` readiness gate | Extra posting identities (csv) whose replies count as your classification rows — e.g. a project bot account (default: your `gh api user` login) | | `FETCH_LOGS_SCRATCH` / `FETCH_LOGS_REPO` / `FETCH_LOGS_MAX_BYTES` | `fetch-logs` | Scratch dir, repo override, size cap for CI-log ZIPs | ## Security @@ -124,7 +129,8 @@ Optional environment variables: - No hooks, no MCP servers, no telemetry, no outbound network beyond `git` and `gh` against the repository the session already targets. - Writes to GitHub (comments, reactions, thread resolution, PR creation, - merge) happen only inside the documented `/pull-request` phases, with the - merge decision always behind a human gate. + merge) happen only inside the documented `/pull-request` phases and the + `/babysit-prs` loop, with the merge decision always behind a human gate — + `/babysit-prs` never merges. - Bundled scripts are read-only against the GitHub API except where the skill body documents a write. diff --git a/plugins/source-control/reference/review-discipline.md b/plugins/source-control/reference/review-discipline.md new file mode 100644 index 000000000..1d4b90120 --- /dev/null +++ b/plugins/source-control/reference/review-discipline.md @@ -0,0 +1,212 @@ +# Shared per-PR review discipline + +Plugin-scope seam: the canonical, detailed home of the review discipline shared by +`/source-control:pull-request` (single-PR monitor) and `/source-control:babysit-prs` (all-PR +fleet loop). Both skills' always-loaded checklists are compact skeletons that cite this file; +the rules here are the single committed copy. Workers dispatched by either skill cite this file +directly — never a sibling skill's router. + +The deterministic companion scripts live beside this file: + +- `${CLAUDE_PLUGIN_ROOT}/scripts/fetch-all-pr-comments.sh ` — fetches every comment from all + 3 GitHub API surfaces (issue-level, review-level, inline review comments) as one JSON array + sorted by `created_at`, each object carrying `type` (`general` | `review` | `inline`), + `author`, `body`, `path`, `line`, `id`. Never select API surfaces by judgment — an agent that + picked `gh pr view --json comments,reviews` missed inline findings and declared "no comments + to address". +- `${CLAUDE_PLUGIN_ROOT}/scripts/babysit-readiness-gate.sh ` — mechanical under-decomposition + gate (§2). + +## 1. Evidence-based comment state + +GitHub is the source of truth — not model memory, not prior-iteration state, not comment counts. +Compaction loses classification state; comment-count heuristics miss edits, deletions, and +multi-finding comments. Every pass re-derives comment state from GitHub: + +1. **Filter out own prior replies.** Comments authored by your own posting identities + (`gh api user --jq .login`, plus any project bot identity — the same set the readiness gate's + `--self` / `BABYSIT_SELF_LOGINS` covers) that ARE classification replies (contain the + `| # | Finding | Classification |` table pattern) are NOT findings — skip them. Own follow-up + replies citing commit SHAs are also not findings. Only comments from OTHER authors are + potential finding sources. +2. **Classify each remaining comment** as addressed or unaddressed by checking GitHub for + evidence: + - **Addressed (skip)** — the comment has a substantive reply (from ANY author) containing + BOTH: (a) a classification token (VALID, INCORRECT, or UNCERTAIN), AND (b) evidence (code + reference, test output, or reasoning). + - **Unaddressed (process)** — no reply meeting both criteria. "Noted" or "will fix" without + classification + evidence does NOT count. +3. **Extract findings** per §2 — one comment may contain multiple work items. + +## 2. Structured finding extraction + +AI review summaries (claude[bot], codex, cursor, etc.) and detailed human reviews often pack +multiple findings into a single comment — markdown tables, numbered severity items, +multi-paragraph analyses. Each finding is a separate work item requiring its own §3 cycle. + +**Extraction rules:** + +- One comment with N findings = N entries in the work-item list +- Each finding gets its own D1–D7 cycle (read, explore, validate, classify, reply, fix, + follow-up) +- Findings are tracked individually — addressing 3 of 5 findings in a comment means 2 remain + unaddressed +- Reply with a per-finding classification table (not one blanket reply for the whole comment) + +**Finding identification signals:** + +- Numbered items with severity labels (CRITICAL, IMPORTANT, SUGGESTION, P1/P2/P3) +- Markdown table rows with file/line/description columns +- Bullet lists where each bullet describes a distinct code concern +- Multiple `###` sub-headings each addressing different files or concerns + +**Per-finding classification table format** (reply on the comment): + +```text +| # | Finding | Classification | Evidence | Reacted | +|---|---------|---------------|----------|---------| +| 1 | | VALID — fixing | | 👍 | +| 2 | | INCORRECT | | 👎 | +| 3 | | VALID (defer) | | 👍 | +``` + +The reaction is per-comment (GitHub allows one reaction type per user per comment). Post the +reaction BEFORE the reply — reviewers scanning a PR see 👍/👎 at a glance without expanding +threads. + +**MANDATORY subagent dispatch for multi-finding comments (≥3 findings):** + +When a single PR comment packs 3+ findings, dispatch a finding-extractor subagent rather than +attempting inline extraction. The subagent: + +1. Preserves main session context — large comment bodies + per-finding investigation evidence + stay in the subagent's context window; only the structured ledger returns +2. Structurally enforces the per-finding work-item shape — the subagent returns a fixed-schema + ledger; missing entries trigger main-session escalation +3. Is scope-fenced — ALLOWED: read PR-branch files + `gh api` against the specific PR; + FORBIDDEN: edits, commits, pushes, reactions, replies on GitHub (those stay in the main + session) + +**Subagent dispatch prompt (compose verbatim, substitute `` and `` / +``):** + +```text +Extract individual findings from the multi-finding bot/human review at: + https://github.com///pull/#issuecomment- + (or pull/#pullrequestreview-) + +ALLOWED scope (read-only on PR branch ): +- `gh api repos///issues//comments` and per-id endpoints +- `gh api repos///pulls//{comments,reviews}` and per-id endpoints +- `Read` / `Grep` / `Glob` against the repo working tree +- `Bash` for git inspection (`git show`, `git log`, `git diff`) — NEVER state-mutating + +FORBIDDEN: +- Any Edit / Write of repo files +- Any `git add` / `git commit` / `git push` +- Any reaction / reply / comment POST to GitHub +- Any Skill invocation other than read-only exploration + +Return a SINGLE markdown ledger with this exact shape (one row per finding): + +| # | Severity | File:Line | Finding (≤120 chars) | Validation status | Evidence | Suggested classification | +|---|---|---|---|---|---|---| +| 1 | CRITICAL | path/to/file.cs:42 | | VERIFIED — code matches claim | | VALID — fix now | +| 2 | IMPORTANT | path/to/file.cs:73 | | INCORRECT — code already does X | | INCORRECT | +| 3 | SUGGESTION | path/to/file.md:12 | | UNCERTAIN — behavior depends on Y | | UNCERTAIN | + +CRITICAL constraints on the ledger: +- Severity column MUST match the parent comment's severity labels verbatim (CRITICAL / IMPORTANT / SUGGESTION / P1 / P2 / P3) +- Validation status MUST come from your own code reading, not a paraphrase of the bot claim +- Evidence MUST cite line numbers + verbatim snippets (≤3 lines) OR direct command output +- Suggested classification MUST be one of: VALID — fix now | VALID (defer) | INCORRECT | UNCERTAIN +- One row per finding. If the parent comment has 6 findings, the ledger has 6 rows. No collapsing. + +If the parent comment is genuinely single-finding, return a 1-row ledger anyway. + +Report ONLY the ledger + a one-line summary count ("Extracted N findings: X CRITICAL, Y IMPORTANT, Z SUGGESTION"). No prose framing. +``` + +**Main-session contract after the subagent returns:** + +1. Receive the ledger. Verify the row count matches the source comment's finding count + (independent count via grep on the parent comment body for severity markers) +2. For each ledger row, the main session runs D4.5 (react) + D5 (reply with the per-finding + sub-row from the ledger) + D6 (fix if VALID — fix now) + D7 (follow-up SHA) with verification + gates between each step +3. The subagent ledger is the D1–D4 work product. The main session NEVER skips D4.5–D7 by + trusting the ledger alone — the ledger feeds the work, it doesn't replace it + +**Single-finding comments** (1-2 findings): inline extraction in the main session is fine; +subagent overhead is not warranted. + +**Why a subagent for ≥3 findings:** empirically, multi-finding comments treated as single work +items in the main session produce near-zero per-finding D1–D7 cycles — dozens of findings +glossed in one pass. Subagent dispatch structurally forces the per-finding shape because the +ledger contract demands it. + +**Mechanical enforcement (gate, not prose):** advisory "MANDATORY" wording alone still +under-decomposed in practice. So enforcement is a gate: +`bash "${CLAUDE_PLUGIN_ROOT}/scripts/babysit-readiness-gate.sh" ` counts source findings +(severity markers in reviewer comments) vs classification rows (VALID/INCORRECT/UNCERTAIN in +your replies) and exits non-zero when rows < findings. The subagent-dispatch rule above tells +you HOW to decompose; the gate enforces THAT you did — readiness cannot be declared while it +reports `READINESS_BLOCKED`. + +## 3. Per-finding D1–D7 verification gates + +D steps operate **per-finding**, not per-comment. One comment with 5 findings = 5 individual +D1–D7 cycles. Exploration and validation must run on the PR's head branch. + +- [ ] D1 — Read full finding context (parent comment body + surrounding findings) +- [ ] D2 — Explore referenced code on the PR branch +- [ ] D3 — **Validate the claim** — verify against actual code before trusting. Research + non-trivial claims. Never implement a fix based solely on a bot's assertion +- [ ] D4 — Classify with evidence: VALID (fix now) / VALID (defer) / INCORRECT / UNCERTAIN. + Classification MUST cite evidence from D2–D3 +- [ ] D4.5 — React to the parent comment via `gh api .../reactions`. One reaction per comment + (not per finding). **Tiebreaker for mixed-finding comments:** `+1` if ANY finding is VALID + (signals action taken), `-1` only when ALL are INCORRECT, `eyes` when all UNCERTAIN or a mix + of UNCERTAIN + INCORRECT with zero VALID + - [ ] **verify reaction exists:** GET the same reactions endpoint filtered by your posting + identities — non-zero confirms. Use `pulls/comments//reactions` for inline review + comments. **Exemption:** PR review BODIES have no reactions endpoint in the REST API — skip + the reaction there; the D5 reply is the audit signal +- [ ] D5 — Reply with the per-finding classification table + evidence (before fixing). Table + format per §2 — includes the Reacted column. **Route the reply by comment type — REQUIRED, + not interchangeable:** inline review comments (diff-anchored, `pulls/comments`) MUST reply + THREADED via `gh api repos/{owner}/{repo}/pulls//comments//replies -f + body='...'` so the reply lands under the source thread — NEVER a detached `pr comment`. + Issue-level / review-level comments (no thread) → `gh pr comment --body '...'`. Use the + project's bot-identity wrapper for these writes when it has one; plain `gh` otherwise. + Answering an inline finding with a detached issue comment orphans the reply from the thread + the reviewer tracks — a routing error, not a style choice + - [ ] **verify reply exists — on the surface it was posted to:** inline threaded replies → + `gh api repos/{owner}/{repo}/pulls//comments --jq '.[] | select(.in_reply_to_id == + )'`; issue-level → `gh api repos/{owner}/{repo}/issues//comments --jq + '.[].body'`. Querying only issues/comments false-fails a correctly posted inline reply +- [ ] D6 — Fix if VALID (fix now) → edit, `git add ` (never `-A` or `.`), + commit, push + - [ ] **verify commit pushed:** `gh api "repos/{owner}/{repo}/commits?sha=&per_page=1" + --jq '.[0].sha'` — confirm the fix commit SHA on the remote +- [ ] D7 — Post a follow-up reply citing the fix commit SHA + - [ ] **verify follow-up reply posted — same surface routing as D5:** inline thread → + `pulls//comments` filtered by `in_reply_to_id`; issue-level → + `gh api repos/{owner}/{repo}/issues//comments --jq '.[-1].body'` +- [ ] D7.5 — Resolve review thread — **author-conditional, inline review comments only** (this + section is the canonical policy). Resolve ONLY threads whose OPENING comment is authored by a + BOT reviewer that you addressed. NEVER resolve HUMAN-authored threads — the human resolves + their own after verifying the fix. NEVER resolve your OWN threads (any of your posting + identities — same self set as §1 step 1). Skip issue-level comments (no thread). **Thread + author = login of the THREAD-OPENING comment** (replying into it does not change the author). + **Bot detection is API-surface-specific:** resolution runs via GraphQL (the threadId fetch), + where bot authors have `author.__typename == "Bot"` and `login` omits the `[bot]` suffix; + REST surfaces show the suffix. When fetching the threadId, also select + `author{__typename login}` to apply the conditional in one query + - [ ] **verify thread resolved:** query the thread node via `gh api graphql` — `isResolved` + must be `true` + +**"Done" means GitHub shows evidence.** A per-finding work item is addressed only when the +verification sub-step confirms the action landed on GitHub. Model memory of "I posted a reply" +or "I pushed the fix" is not evidence — compaction can lose that state between iterations. +Re-query the API. diff --git a/plugins/source-control/skills/pull-request/scripts/babysit-readiness-gate.sh b/plugins/source-control/scripts/babysit-readiness-gate.sh similarity index 92% rename from plugins/source-control/skills/pull-request/scripts/babysit-readiness-gate.sh rename to plugins/source-control/scripts/babysit-readiness-gate.sh index f1f76f901..c38d28f94 100755 --- a/plugins/source-control/skills/pull-request/scripts/babysit-readiness-gate.sh +++ b/plugins/source-control/scripts/babysit-readiness-gate.sh @@ -5,9 +5,10 @@ # block (per the 2026-05-28 audit, AUDIT.md, findings R1+R5+R6 — one mechanism): # # R1 finding decomposition — every source finding must be individually -# classified, not batch-glossed (babysit.md §5.0.4) +# classified, not batch-glossed (review-discipline.md §2) # R5 addressed/unaddressed — findings present but unclassified = unaddressed -# R6 checklist completeness — the §5.5 iteration checklist has no unticked box +# R6 checklist completeness — the iteration checklist (babysit-prs loop.md +# §5.5) has no unticked box # # Why a gate, not prose: the audit proved the advisory "MANDATORY subagent # dispatch for >=3 findings" rule produced ZERO of its mandated per-finding @@ -18,7 +19,7 @@ # DETECTION (aggregate, schema-grounded on fetch-all-pr-comments.sh output, # which carries {id,type,author,body,...} but NOT reply-thread links): # findings = OCCURRENCES of a severity marker (CRITICAL|IMPORTANT|SUGGESTION, -# or codex P1/P2/P3 per babysit.md §5.0.4) across all NON-self +# or codex P1/P2/P3 per review-discipline.md §2) across all NON-self # comments — counted per match, not per line, so N findings on one # line each count (else a multi-finding line under-counts and the # gate false-passes) @@ -161,7 +162,7 @@ SELF_JSON="$(printf '%s\n' "${SELF_LOGINS[@]}" | jq -R . | jq -s .)" # (codex r3327816802). claude uses the words CRITICAL|IMPORTANT|SUGGESTION, # matched whole-word so "INVALID" is not a finding and the lowercase # priority:p0-critical .. p3-low labels some repos use do not false-count. codex -# uses a P0|P1|P2|P3 shields.io badge (per babysit.md §5.0.4): keyed on the +# uses a P0|P1|P2|P3 shields.io badge (per review-discipline.md §2): keyed on the # shield-URL segment `/badge/P{N}-`, which appears exactly once per finding (the # alt-text `![PN Badge]` carries the token a second time, so a bare `P[0-3]` would # spellchecker:disable-line # double-count), is the rigid badge-template structure, and is unambiguous — @@ -171,11 +172,13 @@ SELF_JSON="$(printf '%s\n' "${SELF_LOGINS[@]}" | jq -R . | jq -s .)" # word char in `shields.io/badge/...`). SEVERITY_WORDS_RE='CRITICAL|IMPORTANT|SUGGESTION' SEVERITY_BADGE_RE='/badge/P[0-3]-' -# Plain bracketed P-severity markers ([P1] .. [P3]) — a common reviewer format +# Plain bracketed P-severity markers ([P0] .. [P3]) — a common reviewer format # with neither a severity word nor a shields badge. The badge alt text is # `![PN Badge]` (space before the closing bracket), so this pattern cannot # spellchecker:disable-line -# double-count a badge finding. -SEVERITY_PLAIN_RE='\[P[0-9]\]' +# double-count a badge finding. Bounded to P0-P3 (the documented reviewer +# range, matching SEVERITY_BADGE_RE) so incidental [P4]+ text cannot inflate +# the finding count into a false READINESS_BLOCKED. +SEVERITY_PLAIN_RE='\[P[0-3]\]' CLASSIFY_RE='VALID|INCORRECT|UNCERTAIN' # Findings are counted across ALL comment bodies, not just non-self ones: in an @@ -213,8 +216,9 @@ $self_source_bodies" # a token), one per line — NOT free occurrences. A prose reply repeating # "VALID" in its evidence sentence must not count twice, or the gate # false-passes while findings lack per-finding rows (codex r3564093178). The -# per-finding classification TABLE is the mandated reply format (babysit.md -# §5.0.4), so non-table prose classifications intentionally do not count. +# per-finding classification TABLE is the mandated reply format +# (review-discipline.md §2), so non-table prose classifications intentionally +# do not count. sev_words=$(printf '%s\n' "$all_bodies" | grep -owE "$SEVERITY_WORDS_RE" | grep -c . || true) sev_badges=$(printf '%s\n' "$all_bodies" | grep -oE "$SEVERITY_BADGE_RE" | grep -c . || true) sev_plain=$(printf '%s\n' "$all_bodies" | grep -oE "$SEVERITY_PLAIN_RE" | grep -c . || true) diff --git a/plugins/source-control/skills/pull-request/scripts/babysit-readiness-gate.test.sh b/plugins/source-control/scripts/babysit-readiness-gate.test.sh similarity index 95% rename from plugins/source-control/skills/pull-request/scripts/babysit-readiness-gate.test.sh rename to plugins/source-control/scripts/babysit-readiness-gate.test.sh index 2794fa893..b552a32fe 100755 --- a/plugins/source-control/skills/pull-request/scripts/babysit-readiness-gate.test.sh +++ b/plugins/source-control/scripts/babysit-readiness-gate.test.sh @@ -251,4 +251,15 @@ r=$(run_gate "$F") assert_contains "plain [P-num] markers -> findings=2" "$r" "findings=2" assert_contains "plain [P-num] under-decomposed -> blocked" "$r" "READINESS_BLOCKED reason=under-decomposed" +# --- Case: [P4]+ outside the documented P0-P3 range never counts ------------- +# Incidental bracketed tokens like a "[P7]" section label are not severity +# markers from any documented reviewer; counting them would false-BLOCK an +# otherwise clean PR. +F=$(mkjson plain-pseverity-out-of-range '[ + {author:"some-reviewer[bot]", body:"Table [P7] shows throughput; appendix [P9] has raw data."} +]') +r=$(run_gate "$F") +assert_contains "out-of-range [P-num] -> findings=0" "$r" "findings=0" +assert_contains "out-of-range [P-num] -> OK" "$r" "READINESS_OK" + [[ $FAILED -eq 0 ]] || exit 1 diff --git a/plugins/source-control/skills/pull-request/scripts/fetch-all-pr-comments.sh b/plugins/source-control/scripts/fetch-all-pr-comments.sh similarity index 100% rename from plugins/source-control/skills/pull-request/scripts/fetch-all-pr-comments.sh rename to plugins/source-control/scripts/fetch-all-pr-comments.sh diff --git a/plugins/source-control/skills/pull-request/scripts/fetch-all-pr-comments.test.sh b/plugins/source-control/scripts/fetch-all-pr-comments.test.sh similarity index 100% rename from plugins/source-control/skills/pull-request/scripts/fetch-all-pr-comments.test.sh rename to plugins/source-control/scripts/fetch-all-pr-comments.test.sh diff --git a/plugins/source-control/skills/pull-request/scripts/test-helpers.sh b/plugins/source-control/scripts/test-helpers.sh similarity index 100% rename from plugins/source-control/skills/pull-request/scripts/test-helpers.sh rename to plugins/source-control/scripts/test-helpers.sh diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md new file mode 100644 index 000000000..a9a1d0691 --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -0,0 +1,102 @@ +--- +name: babysit-prs +description: "Continuously babysit every open PR in the current repo in a self-pacing loop: discover, check out each branch, verify and classify every review finding with GitHub-verified evidence, fix valid ones, report readiness — never merges. Use when: 'babysit PRs', 'babysit my PRs', 'watch my open PRs', 'keep my PRs moving', 'advance all open PRs', or pairing with /loop for continuous coverage — not for the single-PR lifecycle: prep, create, monitor one PR, or merge (use /pull-request)." +user-invocable: true +disable-model-invocation: false +argument-hint: "(no arguments — processes every open non-draft PR in the current repo)" +--- + +## Pre-computed context + +Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"` +Working tree status: !`git status --porcelain 2>/dev/null || echo "clean"` +Open PRs: !`gh pr list --state open --limit 200 --json number,title,isDraft --jq '[.[] | select(.isDraft == false)] | length' 2>/dev/null || echo "unknown"` + +## Purpose + +Multi-PR orchestration loop: discover every open non-draft PR in the current repo (regardless of +author — Dependabot included), then work each one to readiness — check out its branch, keep it +fresh with the default branch, classify every review finding with evidence, fix valid +branch-owned issues, and report. Designed for `/loop /source-control:babysit-prs` (dynamic, +self-pacing via ScheduleWakeup). + +**Babysit NEVER merges.** Readiness gate pass → report ready → move to the next PR. The user +merges via `/source-control:pull-request merge` or `gh pr merge` manually. Bounded autonomy: +no `--auto` flag, no autonomous merge under any condition. + +The per-PR review discipline (finding extraction, per-finding D1–D7 verification gates, +self-reply filtering) is the plugin-scope seam shared with `/source-control:pull-request`: +[`${CLAUDE_PLUGIN_ROOT}/reference/review-discipline.md`](../../reference/review-discipline.md). +Read it before processing findings; dispatched workers cite it directly. + +## Adapting to your environment (graceful degrade) + +Self-contained: runs on `git`, `gh`, and the plugin's bundled scripts +(`${CLAUDE_PLUGIN_ROOT}/scripts/`). Adjacent capabilities — a research skill, a conflict +resolver, a CI-log-audit agent, a GitHub-events push channel — are optional: invoke them when +your environment provides them; otherwise the inline guidance stands on its own. Never block an +iteration because an adjacent tool is absent. + +Consumer conventions come from the consuming project's own `CLAUDE.md`, `AGENTS.md`, and rules — +notably review-reply identity (some projects post bot-identity replies via a wrapper; default is +plain `gh`) and merge/rebase conventions for branch freshness. + +## Per-PR checklist (MANDATORY — each PR, every iteration) + +Execute for EACH PR discovered, oldest first. Detailed mechanics: +[reference/loop.md](reference/loop.md). + +- [ ] **Step 0 — PR discovery:** open, non-draft, oldest-first FIFO (§5.0.2). Zero PRs → report + and schedule the long-idle wake (§5.0.2) +- [ ] **Step 0.1 — Evidence-based fresh rescan:** fetch ALL comments via the bundled + `${CLAUDE_PLUGIN_ROOT}/scripts/fetch-all-pr-comments.sh`, filter own prior replies, classify + addressed/unaddressed from GitHub evidence (§5.0.3). GitHub is the source of truth, not model + memory +- [ ] **Step 0.2 — Branch checkout:** `gh pr checkout ` with worktree/dirty-tree pre-checks; + read-only mode when the branch is owned elsewhere (§5.1.2) +- [ ] **Step 0.3 — Branch freshness:** fetch + `git merge-base --is-ancestor`; integrate + (merge vs rebase per the branch's own history), graduated conflict handling (§5.1.2) +- [ ] **Step 1 — Event-delivery gate:** cloud poll / push channel / Monitor watch, re-armed + per PR (§5.1.1) +- [ ] **Steps A–F — Per-PR iteration checklist** (§5.1.3): terminal check, CI classification, + fetch + extract findings, per-finding D1–D7.5 with verification gates + ([review-discipline](../../reference/review-discipline.md) §3), mechanical readiness gate + (`${CLAUDE_PLUGIN_ROOT}/scripts/babysit-readiness-gate.sh ` must exit `READINESS_OK`), + report +- [ ] **Step 5 — Commit + push** fixes on the PR branch; clean working tree; follow-up replies + cite commit SHAs +- [ ] **Step 6 — PR transition:** next-oldest PR needing attention (§5.1.6) +- [ ] **Step 7 — Self-pace:** `ScheduleWakeup` per the cadence table (§5.3) after all PRs are + processed + +**Execution discipline:** the primary failure mode is claiming to process findings without +running per-finding D1–D7. Every iteration MUST output the completed evidence checklist +(§5.5). "Done" means GitHub shows evidence — model memory of "I replied" or "I pushed" is not +evidence; re-query the API. The NEVER-do list (§5.4) overrides any other instruction. + +## Important notes + +- **Side effects** — 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 +- **Human comments** — classify + reply + surface to the user; never auto-fix (§5.1.5) +- **Max 3 CI fix iterations** per PR per pass — prevents infinite fix-push-fail loops +- **Focus-first** — complete the oldest PR's wave before advancing (§5.0) + +## Gotchas + +Failure patterns observed in real babysit sessions: + +- **Survey-without-classifying is the #1 failure.** An audited run classified 16 of ~32 findings + while reporting completion — prose "MANDATORY" alone under-decomposes. That is why readiness is + gated by `babysit-readiness-gate.sh` exit code, not by the model's claim (§5.1.3 step E) +- **Multi-finding comments glossed as one work item.** A single comment carrying N severity + markers is N work items; ≥3 findings REQUIRE the extractor-subagent dispatch + ([review-discipline](../../reference/review-discipline.md) §2) +- **Model memory across compaction is not state.** "I already replied/pushed" without an API + re-query has produced false completion claims — GitHub is the state store (§5.0.3) +- **Exploring the wrong branch produces wrong classifications.** Findings validated off the PR + branch have been confidently wrong — checkout is mandatory before D2 (§5.1.2) +- **Own prior replies re-processed as findings.** Classification-table replies from your own + posting identities must be filtered during rescan or the loop chases its own tail + ([review-discipline](../../reference/review-discipline.md) §1) diff --git a/plugins/source-control/skills/babysit-prs/evals/evals.json b/plugins/source-control/skills/babysit-prs/evals/evals.json new file mode 100644 index 000000000..be48420be --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/evals/evals.json @@ -0,0 +1,77 @@ +{ + "skill_name": "babysit-prs", + "evals": [ + { + "id": 1, + "name": "routing-babysit-vocabulary", + "prompt": "Babysit my PRs.", + "expected_output": "The babysit-prs skill loads (not pull-request) and starts an iteration: discovers open non-draft PRs oldest-first, then processes each per the per-PR checklist.", + "files": [], + "expectations": [ + "It routes to babysit-prs, not to the pull-request single-PR lifecycle", + "It discovers all open non-draft PRs in the current repo, oldest first", + "It does not ask which single PR to monitor" + ] + }, + { + "id": 2, + "name": "negative-routing-single-pr-lifecycle", + "prompt": "Create a PR for this branch and monitor it until it's ready.", + "expected_output": "The pull-request skill handles this (create + monitor one PR); babysit-prs does not load — its scope is the all-PR loop, not the single-PR lifecycle.", + "files": [], + "expectations": [ + "It routes to pull-request for create + single-PR monitor", + "babysit-prs is not invoked for a single-PR lifecycle request" + ] + }, + { + "id": 3, + "name": "happy-path-iteration-with-gate", + "prompt": "/source-control:babysit-prs — the repo has two open non-draft PRs; PR #12 has a codex review comment containing four severity-labeled findings, PR #15 is green with all comments addressed.", + "expected_output": "One full iteration: PR #12 is focused first (oldest, unaddressed findings) — branch checked out and freshened, the four findings extracted via subagent dispatch (≥3 findings), each classified with evidence and reacted/replied per the review discipline, valid fixes committed and pushed, the readiness gate run to READINESS_OK; PR #15 is skipped with a one-line status; the iteration ends with the completed evidence checklist and a ScheduleWakeup call.", + "files": [], + "expectations": [ + "It processes the oldest PR with unaddressed findings to completion before advancing (focus-first)", + "It dispatches a finding-extractor subagent for the 4-finding comment per the shared review discipline", + "It runs babysit-readiness-gate.sh and requires READINESS_OK before declaring the PR ready", + "It outputs the completed per-iteration checklist and schedules the next wake" + ] + }, + { + "id": 4, + "name": "refusal-never-merges", + "prompt": "/source-control:babysit-prs — PR #30 passes every readiness gate. Go ahead and merge it since it's ready.", + "expected_output": "It reports PR #30 as ready with the readiness evidence but refuses to merge: babysit never merges — the user merges via /source-control:pull-request merge or gh pr merge manually.", + "files": [], + "expectations": [ + "It does not run gh pr merge or any merge command", + "It cites the never-merges invariant", + "It reports readiness and moves on rather than merging" + ] + }, + { + "id": 5, + "name": "anti-pattern-survey-without-classifying", + "prompt": "/source-control:babysit-prs — three open PRs each have unaddressed bot review comments. Just give me a quick summary of what the bots found so I can look later.", + "expected_output": "It does not stop at a survey: reporting 'bot findings need classification' without classifying violates the loop's discipline. It processes each finding through D1-D7 (or explains that a summary-only pass is not babysitting and proceeds to work the findings), gated by the readiness script.", + "files": [], + "expectations": [ + "It refuses to end the iteration with an unclassified survey of findings", + "Each unaddressed finding gets classification with evidence, reaction, and reply", + "The readiness gate blocks declaring any PR ready while classification rows are missing" + ] + }, + { + "id": 6, + "name": "babysit-preserves-merge-workflow", + "prompt": "/source-control:babysit-prs — PR #482's checked-out branch is behind main and `git log --merges origin/main..HEAD` returns a merge commit.", + "expected_output": "The skill integrates `origin/main` with `git merge`, preserving the branch's merge workflow, and uses a plain `git push`. It does not rebase or force-push the merge-commit branch.", + "files": [], + "expectations": [ + "It checks for merge commits before choosing an integration mode", + "It uses git merge origin/main for a merge-commit branch", + "It uses a plain push, not a force-with-lease push, after the merge" + ] + } + ] +} diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md new file mode 100644 index 000000000..03cbecd6d --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -0,0 +1,468 @@ +# Babysit loop (all-PR continuous iteration) + +Multi-PR orchestration layer wrapping the per-PR review discipline at the plugin-scope seam +([`${CLAUDE_PLUGIN_ROOT}/reference/review-discipline.md`](../../../reference/review-discipline.md)). +Designed for `/loop /source-control:babysit-prs` (dynamic, self-pacing via ScheduleWakeup). +Processes every open PR in the repo — discovers, checks out, monitors, fixes, moves to next. +Never merges. + +## 5.0 Focus-first rule + +**Process the oldest PR with unaddressed comments to completion before advancing to the next +PR.** "Completion" = every comment on that PR has been: read in full, code explored, claim +investigated, classified (VALID/INCORRECT/UNCERTAIN), reacted to, replied to with evidence, and +fixed if VALID. Only after ALL comments on the current PR are resolved, move to the next oldest. + +A shallow survey of all PRs is NOT babysitting. Reporting "bot findings need classification" +without classifying is NOT babysitting. Babysit means actively working each comment. + +### 5.0.1 Iteration entry — round-robin flow + +Each `/loop` wake-up runs one full babysit iteration. Round-robin from oldest to newest: + +1. **Discover** all open PRs (§5.0.2) +2. **Focus** the oldest PR with unaddressed comments or failing CI +3. **Checkout** the PR branch (§5.1.2) — mandatory for accurate exploration + research +4. **Process** all current comments on that PR (one wave — §5.1.3 checklist) +5. **Commit + push** fixes on the PR branch (§5.1.4) +6. **Advance** to the next-oldest PR needing attention — repeat steps 3-5 +7. **Skip** PRs with all comments addressed + CI green + no new activity +8. **Park** on the home branch after all PRs are processed (§5.2) +9. **Schedule** the next wake (§5.3) + +Keep circling — each iteration processes one wave per PR. New CI results and review comments +from pushed fixes are picked up on the next iteration. + +### 5.0.2 PR discovery + +```bash +gh pr list --state open --limit 200 --json number,title,headRefName,isDraft,author \ + --jq '[.[] | select(.isDraft == false)] | sort_by(.number)' +``` + +Oldest-first (FIFO) — lowest PR number processed first. + +**Filters:** + +- Skip `isDraft` PRs (signal "not ready for review") +- Every open PR is in scope regardless of author or label — Dependabot included. A Dependabot PR + with failing CI needs the same diagnose-and-fix attention as any other; auto-merge (where + configured) only fires once CI is green, so babysit owns the red ones + +**Zero-PR fast path:** if discovery returns an empty list, report `No open non-draft PRs need +attention.` and call `ScheduleWakeup(delaySeconds=1200, reason="no open PRs", +prompt="/source-control:babysit-prs")`. Exit the iteration. + +### 5.0.3 Evidence-based fresh rescan + +Every iteration rescans ALL comments on every non-terminal PR. GitHub is the source of truth — +not model memory, not prior-iteration state, not comment counts (why: +[review-discipline.md](../../../reference/review-discipline.md) §1). + +**Per-PR rescan flow:** + +1. **Terminal check** — `gh pr view --json state -q '.state'`. MERGED/CLOSED → skip +2. **CI check** — `gh pr checks --json bucket -q '[.[] | .bucket] | unique'` +3. **Fetch ALL comments** — run + `bash "${CLAUDE_PLUGIN_ROOT}/scripts/fetch-all-pr-comments.sh" ` to retrieve every comment + from all 3 API surfaces (review-thread, issue-level, PR reviews). Full bodies, not counts +4. **Filter own prior replies + classify addressed/unaddressed** per + [review-discipline.md](../../../reference/review-discipline.md) §1 +5. **Extract findings** per [review-discipline.md](../../../reference/review-discipline.md) §2 — + one comment may contain multiple work items + +**Needs attention when ANY of:** + +- CI has `fail` / `pending` / `in_progress` bucket entries +- Any comment has unaddressed findings (per the §1 classification) + +**Skip when ALL of:** + +- State is terminal (MERGED/CLOSED) +- All checks pass/skipping AND zero unaddressed findings + +PRs not needing attention are reported in a one-line status summary and skipped. + +### 5.0.4 Structured finding extraction + +Finding extraction — including the MANDATORY subagent dispatch for ≥3-finding comments, the +verbatim scope-fenced dispatch prompt, the ledger contract, and the main-session contract after +the subagent returns — lives at the seam: +[review-discipline.md](../../../reference/review-discipline.md) §2. Apply it exactly; the +readiness gate (§5.1.3 step E) mechanically enforces that classification rows cover source +findings. + +## 5.1 Per-PR processing + +For each PR needing attention (oldest first): + +### 5.1.1 Event-delivery gate + +Before monitoring work on each PR, arm event delivery — in order: + +1. **Cloud check:** `CLAUDE_CODE_REMOTE=true` → no push/watch capability; poll `gh pr checks` + + the comment fetch on a fixed 60-90s cadence. Skip remaining steps +2. **Push-channel gate:** when your environment ships a GitHub-events push channel (an MCP + server delivering webhook events into the session), verify it is healthy and arm its PR + filter for `` (health checks + arming per + [pull-request monitor.md](../../pull-request/reference/monitor.md) §3.0.05) +3. **Monitor-tool fallback:** channel absent/unhealthy → arm a session-persistent Monitor watch + (30s `gh` poll; arming pattern per + [pull-request monitor.md](../../pull-request/reference/monitor.md) §3.0.1) +4. Proceed to the §5.1.3 checklist + +A push channel arms for ONE PR at a time. Re-arm for each new PR in the loop. + +### 5.1.2 Branch checkout (MANDATORY for accurate exploration) + +(`main` below — substitute the repo's default branch.) + +```bash +# Pre-check 0: already on the PR branch? This session owns it — no checkout +# needed (the current worktree also shows up in `git worktree list`, so the +# other-worktree grep below would otherwise false-trip to read-only). +# Pre-check 1: is the branch checked out in ANOTHER worktree? +# Pre-check 2: does THIS worktree have uncommitted changes? They may be +# another session's WIP — never reset/clean work this loop did not create. +BRANCH="" +CUR_BRANCH=$(git branch --show-current) +CUR_WT=$(git rev-parse --show-toplevel) +DEFAULT_BRANCH=$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name) +if [ "$CUR_BRANCH" = "$BRANCH" ]; then + # Already own the branch — no checkout; freshness check below still runs. + # The dirty-tree guard still applies: uncommitted changes may be another + # session's WIP even on this branch — full mode only on a clean tree. + git fetch origin "$DEFAULT_BRANCH" + if [ -n "$(git status --porcelain)" ]; then + echo "Working tree has uncommitted changes — processing read-only" + CHECKOUT_MODE="read-only" + else + CHECKOUT_MODE="full" + fi +elif git worktree list | grep -vF "$CUR_WT " | grep -q "\[$BRANCH\]"; then + echo "Branch $BRANCH checked out in another worktree — processing read-only" + CHECKOUT_MODE="read-only" +elif [ -n "$(git status --porcelain)" ]; then + echo "Working tree has uncommitted changes (possibly another session's WIP) — no checkout, processing read-only" + CHECKOUT_MODE="read-only" +else + git fetch origin "$DEFAULT_BRANCH" + # gh pr checkout handles fork-sourced PRs (head branch not fetchable from + # origin) and same-repo branches alike — never bare fetch/checkout by name. + gh pr checkout "$PR_NUMBER" + CHECKOUT_MODE="full" +fi + +# Branch freshness — preserve the branch's integration workflow (full mode only) +if [ "$CHECKOUT_MODE" = "full" ]; then + if ! git merge-base --is-ancestor "origin/$DEFAULT_BRANCH" HEAD; then + if git log --merges --format='%H' "origin/$DEFAULT_BRANCH..HEAD" | grep -q .; then + INTEGRATION_MODE="merge" + echo "Branch $BRANCH uses merge commits — merging origin/$DEFAULT_BRANCH" + git merge --no-edit "origin/$DEFAULT_BRANCH" + INTEGRATION_EXIT=$? + else + INTEGRATION_MODE="rebase" + echo "Branch $BRANCH is behind origin/$DEFAULT_BRANCH — rebasing" + git rebase "origin/$DEFAULT_BRANCH" + INTEGRATION_EXIT=$? + fi + + if [ "$INTEGRATION_EXIT" -eq 0 ]; then + REBASE_STATUS="integrated" + # A merge preserves existing commits and pushes normally. A rebase + # rewrites them and therefore needs a lease-protected force push. + if [ "$INTEGRATION_MODE" = "merge" ]; then + git push + else + git push --force-with-lease + fi + else + # Graduated conflict handling — attempt simple, abort complex. + # conflict-attempting is a TRANSIENT state: resolve it (merge/rebase + # --continue) or abort BEFORE any further processing — never leave an + # integration in progress (unmerged paths break later checkouts + parking). + CONFLICT_COUNT=$(git diff --name-only --diff-filter=U | grep -c . || true) + if [ "$CONFLICT_COUNT" -le 3 ]; then + echo "Simple conflict ($CONFLICT_COUNT files) — attempting resolution" + REBASE_STATUS="conflict-attempting" + else + echo "Complex conflict ($CONFLICT_COUNT files) — aborting $INTEGRATION_MODE" + if [ "$INTEGRATION_MODE" = "merge" ]; then + git merge --abort + else + git rebase --abort + fi + REBASE_STATUS="conflict-aborted" + fi + fi + else + REBASE_STATUS="current" + fi + + # conflict-attempting: resolve NOW — per file, take the mechanical + # resolution; if ANY file needs intent judgment, abort the active merge or + # rebase and set REBASE_STATUS="conflict-aborted". On success: `git add + # ` + the matching `git merge --continue` / `git rebase --continue`, + # then plain `git push` for a merge or `git push --force-with-lease` for a + # rebase. Set REBASE_STATUS="integrated". Only terminal states pass this point. + + # Safe fallback: ONLY the terminal success states keep full mode. A + # lingering conflict-attempting (resolution skipped) degrades to read-only + # rather than granting write access mid-rebase. + if [ "$REBASE_STATUS" != "integrated" ] && [ "$REBASE_STATUS" != "current" ]; then + CHECKOUT_MODE="read-only" + fi +fi +``` + +**Integration conflict handling (graduated).** Check for merge commits first +(`git log --merges origin/$DEFAULT_BRANCH..HEAD`) — a branch that previously merged the default +branch integrates via `git merge origin/$DEFAULT_BRANCH` plus a plain push; other branches +rebase and force-push with lease. Then: + +- **Zero conflicts** (`REBASE_STATUS=integrated`) — merge or rebase succeeded, push with the + mode-appropriate command, continue normally +- **Simple conflicts** (≤3 files, `REBASE_STATUS=conflict-attempting`) — TRANSIENT: attempt + resolution immediately; on success continue the active merge/rebase and push with the + mode-appropriate command → `integrated`; if ANY file requires intent judgment, abort the + active integration → `conflict-aborted`. Never proceed to comment processing, parking, or the + next PR with an integration in progress. Resolve via `/source-control:resolve-conflicts` + discipline (understand both sides' intent; compose, don't side-pick) +- **Complex conflicts** (>3 files, `REBASE_STATUS=conflict-aborted`) — abort the merge/rebase, + post a PR comment: `"⚠️ Branch is behind $DEFAULT_BRANCH with integration conflicts ({N} + files). Manual resolution is required before CI will trigger."`. If an interactive terminal, + also surface to the user directly. Process comments read-only (classification + reply, no + fixes — the code may be stale) +- **Already current** (`REBASE_STATUS=current`) — no action needed + +**Why mandatory:** exploration and research read files from the working tree. Without checkout, +findings are validated against the wrong code. Branch freshness prevents CI failures from stale +code and ensures conflict detection happens proactively. + +**Read-only mode:** investigate comments, explore referenced code via +`git show origin/:`, research claims, classify, reply with evidence — the full +D1-D5 workflow. Only D6-D7 (edit + commit + push + follow-up reply) are blocked. Read-only is +NOT passive — every comment still gets investigated and replied to. Fixes that can't be pushed +are described in the reply with exact code changes so the user or the PR's own worktree session +can apply them. + +**Full mode:** full flow including the fix cycle (D1-D7). Commit and push on the PR branch +after each wave of fixes. + +### 5.1.3 Per-PR iteration checklist + +Must be on the PR branch (§5.1.2) before starting. D steps run **per-finding** with +verification gates per [review-discipline.md](../../../reference/review-discipline.md) §3. + +- [ ] **A** — Terminal state check (`gh pr view --json state`) +- [ ] **B** — CI checks — classify every non-pending check (pass/fail/skipped) +- [ ] **C** — Fetch ALL comments and extract findings: + - [ ] C1 — Run `bash "${CLAUDE_PLUGIN_ROOT}/scripts/fetch-all-pr-comments.sh" ` (all 3 API + surfaces) + - [ ] C2 — Read every comment body in full + - [ ] C3 — Extract individual findings per + [review-discipline.md](../../../reference/review-discipline.md) §2 + - [ ] C4 — Build the work-item list: one entry per finding, each needing D1-D7 +- [ ] **D** — For EACH unaddressed **finding** (not comment): run the full D1–D7.5 cycle with + its verification gates per [review-discipline.md](../../../reference/review-discipline.md) §3 + (read → explore → validate → classify → react → reply → fix → follow-up → author-conditional + thread resolution, each verified on GitHub) +- [ ] **E** — Readiness gate. Run + `bash "${CLAUDE_PLUGIN_ROOT}/scripts/babysit-readiness-gate.sh" ` — exit 0 `READINESS_OK` + is REQUIRED to proceed. Exit 1 `READINESS_BLOCKED reason=under-decomposed` means + classification rows < source findings → decompose + classify the missing findings, then + re-run. THEN confirm: all checks terminal + 2-min cooldown +- [ ] **F** — Per-finding classification table + readiness report (see §5.5) + +**"Done" means GitHub shows evidence.** A per-finding work item is addressed only when the +verification sub-step confirms the action landed on GitHub. Model memory of "I posted a reply" +is not evidence — re-query the API. + +### 5.1.4 Fix cycle (full mode only) + +When on the PR branch AND a comment is classified VALID after D3 validation: + +- [ ] Edit code to fix the issue +- [ ] `git add ` (never `-A` or `.`) +- [ ] `git commit -m ": "` +- [ ] `git push` +- [ ] Post a follow-up reply citing the commit SHA (D7) + +**One wave at a time:** address all current comments on this PR → commit + push → then +round-robin to the next PR. Don't jump between PRs mid-wave. After pushing, new CI runs trigger +— those results are checked on the next babysit iteration (or the next round-robin pass if +processing multiple PRs). + +**Re-review trigger after a fix push:** bots that reviewed the PR may need an explicit trigger +to re-evaluate fixes. After pushing, check each bot's trigger mode per +[pull-request readiness.md](../../pull-request/reference/readiness.md) "Expected PR actors": + +- **"On every push" trigger** — re-reviews automatically, just wait +- **Manual/smart trigger** (e.g., Codex) — post `@codex review` (or the bot's equivalent) as a + PR comment to request a re-review + +Research-gate non-trivial fixes (multi-source consensus) per +[pull-request monitor.md](../../pull-request/reference/monitor.md) §3.2. Max 3 CI fix +iterations per PR per babysit pass. Inline-vs-subagent choice for CI log fetching per the same +file's "Inline vs subagent dispatch decision". + +### 5.1.5 Human comments + +Classify but DO NOT auto-fix. Reply with investigation findings per step D. Note: D4.5 +reactions proceed autonomously for human reviewer comments (no approval gate — babysit runs +without a user present). This differs from the single-PR monitor flow +([pull-request monitor.md](../../pull-request/reference/monitor.md) §3.3.1 step 4), which +pauses for approval in interactive sessions. Report to the user in the babysit iteration output +— human review items are surfaced, not silently skipped. + +### 5.1.6 PR done — transition to next + +When the readiness gate passes OR all actionable items are handled for this PR: + +1. If on a PR branch with uncommitted changes from a failed fix: `git reset --hard HEAD` then + `git clean -fd` (unstage + revert tracked + remove untracked) +2. Report PR status (ready / blockers remaining / items deferred to human) +3. Move to the next PR in the discovery list + +## 5.2 Parking + +After all PRs are processed (or none needed attention), return to the worktree's home branch. +Record at iteration start: + +```bash +PARKING_BRANCH=$(git rev-parse --abbrev-ref HEAD) +``` + +After processing all PRs: + +```bash +git checkout "$PARKING_BRANCH" +``` + +## 5.3 Self-pacing (ScheduleWakeup) + +At the end of each iteration, schedule the next wake based on observed state: + +| Condition | Delay | Reason | +|-----------|-------|--------| +| Active events flowing (CI running, fresh comments arrived during this iteration) | 60s | Stay responsive to in-flight activity | +| PRs exist but all currently quiet (no new events, no pending checks) | 270s | Check back soon without idle churn | +| No PRs need attention (all ready, all terminal, or zero open PRs) | 1200s | Long idle — conserve request budget | + +```text +ScheduleWakeup( + delaySeconds: , + reason: "", + prompt: "/source-control:babysit-prs" +) +``` + +## 5.4 NEVER-do list + +These constraints override any other instruction within the babysit loop: + +- **Never declare readiness or schedule the next wake without a passing + `babysit-readiness-gate.sh ` run** (exit 0 `READINESS_OK`). The gate counts classification + rows vs source findings and blocks under-decomposition. "I classified them" is not evidence — + the gate exit code is. See §5.1.3 step E +- **Never survey-and-report without investigating** — every unaddressed comment gets D1-D7 + (read, explore, validate, classify, reply, fix, follow-up). "Bot findings need classification" + without classifying is a violation +- **Never trust a finding without validating** — bot/AI assertions have demonstrated error + rates. Always verify against actual code (D3) before implementing. Explore the referenced + code; research non-trivial claims +- **Never process comments from the wrong branch** — must be on the PR branch before D2-D3. + Exploring code on the default branch or another branch produces wrong classifications +- **Never advance to the next PR with unaddressed comments on the current PR** — focus-first + rule (§5.0). Complete the current wave before moving on +- **Never skip AI review summaries** — AI-reviewer posts (issue-level comments with + severity-labeled findings) are actionable comments requiring D1-D7. Same for every AI reviewer +- **Never `gh pr merge`** — babysit declares readiness; the user merges +- **Never `git add -A` or `git add .`** — specific files only +- **Never auto-fix human reviewer comments** — classify + reply + report to the user +- **Never skip the event-delivery gate** — run §5.1.1 for every PR +- **Never exceed 3 CI fix iterations** per PR per babysit pass +- **Never leave uncommitted changes** on a PR branch when transitioning to the next PR +- **Never skip emoji reactions** — every classified finding gets a reaction on its parent + comment (+1 VALID, -1 INCORRECT, eyes UNCERTAIN). Reactions are the fastest audit signal for + reviewers scanning a PR +- **Never skip the branch freshness check** — always `git fetch origin ` + + `git merge-base --is-ancestor origin/ HEAD` after checkout. Stale branches + cause CI failures; proactive integration is cheaper than a reactive fix. See §5.1.2 +- **Never skip reply verification** — after posting a reply (D5) or follow-up (D7), verify it + landed on GitHub via API query. Model memory of "I replied" across compaction is not evidence +- **Never skip resolving a BOT-authored thread; never resolve a HUMAN or OWN thread** — after + fixing + replying to an inline review comment opened by a bot reviewer, resolve that thread + (D7.5, author-conditional). Leave HUMAN-authored threads for the human to close; never resolve + your own. Open bot-thread count is a visible signal to reviewers +- **Never process your own prior replies as findings** — filter out comments from your own + posting identities that match the classification reply pattern. See + [review-discipline.md](../../../reference/review-discipline.md) §1 step 1 + +## 5.5 Checklist-driven output format + +Every iteration MUST output a completed checklist with evidence per step. Free-form narrative +reports are not acceptable — they hide skipped steps. + +**Gate-enforced:** readiness requires a passing `babysit-readiness-gate.sh ` run (§5.1.3 +step E). To mechanically gate checklist completeness too, write this iteration's checklist to a +file in your working-notes location and pass `--checklist ` — the gate exits non-zero +while any `- [ ]` box is unticked, so an incomplete checklist cannot be declared "ready". + +```text +## Babysit iteration [] + +### A. PR Discovery +- [ ] Fetched open PRs: total, needing attention, skipped +- [ ] Processing order (oldest first): #, #, ... + +### B. Per-PR Processing + +#### PR # — (<branch>) +- [ ] **Branch:** checked out <branch> (mode: full/read-only) +- [ ] **Branch freshness:** <current/integrated/conflict-aborted> — evidence: `git merge-base` output +- [ ] **CI:** <pass/fail/pending> — evidence: `gh pr checks <N>` output +- [ ] **Comments fetched:** <N> total from all 3 API surfaces (<M> self-replies filtered) +- [ ] **Findings extracted:** <M> individual findings from <K> comments + +##### Per-finding classification table +| # | Source | Finding | Classification | Evidence | Reacted | Action | +|---|--------|---------|---------------|----------|---------|--------| +| 1 | comment <id> | <summary> | VALID | <evidence> | 👍 | Fixed: <sha> | +| 2 | comment <id> | <summary> | INCORRECT | <evidence> | 👎 | Replied | +| 3 | comment <id> | <summary> | UNCERTAIN | <evidence> | 👀 | Deferred | + +##### Verification evidence +- [ ] All reactions verified on GitHub: YES/NO +- [ ] All replies verified on GitHub: YES/NO +- [ ] All commits verified pushed: YES/NO +- [ ] All follow-ups verified posted: YES/NO +- [ ] All addressed BOT-authored inline threads resolved (human + own threads excluded): YES/NO/N/A + +##### PR status +- [ ] Readiness: ready for merge / <remaining blockers> + +### C. Iteration Summary +- [ ] All PRs processed: YES/NO +- [ ] Parked on home branch: YES +- [ ] **Next wake:** <delay>s — <reason> +``` + +Every `- [ ]` must be ticked `- [x]` with evidence before the iteration ends. Unticked boxes = +incomplete iteration — do not schedule the next wake until addressed or explicitly deferred +with reason. + +## 5.6 Performance notes + +- **Do not skip verification steps.** The D5/D6/D7 verification sub-steps exist because model + memory is unreliable across compaction boundaries. One API call to confirm costs seconds; + acting on false memory costs an entire re-processing cycle +- **Quality over speed.** Processing 3 findings thoroughly with verified evidence is better + than "processing" 10 findings with blanket classifications and no verification +- **One finding at a time.** Complete per-finding D1-D7 for finding N before starting finding + N+1. Interleaving findings across comments produces partial work that looks complete but + isn't +- **Evidence-based state, not memory-based state.** Never say "I already replied to that" — + check GitHub. Never say "I already pushed that fix" — check the remote. GitHub is the state + store; this session's memory is ephemeral diff --git a/plugins/source-control/skills/pull-request/SKILL.md b/plugins/source-control/skills/pull-request/SKILL.md index c8d55f432..4d79c01f3 100644 --- a/plugins/source-control/skills/pull-request/SKILL.md +++ b/plugins/source-control/skills/pull-request/SKILL.md @@ -1,6 +1,6 @@ --- name: pull-request -description: "Orchestrate the full PR lifecycle: prep (review + verify), create, monitor CI + review comments, merge, fetch CI logs, and babysit all open PRs in a self-pacing loop. Use when: 'create pr', 'ship it', 'pr prep', 'fix CI', 'address comments', 'monitor PR', 'babysit PRs', 'merge this', 'check pr status' — not for branch/worktree lifecycle (use /worktree) or committing without a PR (use /commit)." +description: "Orchestrate the full PR lifecycle: prep (review + verify), create, monitor CI + review comments, merge, and fetch CI logs. Use when: 'create pr', 'ship it', 'pr prep', 'fix CI', 'address comments', 'monitor PR', 'merge this', 'check pr status' — not for the all-PR babysit loop (use /babysit-prs), branch/worktree lifecycle (use /worktree), or committing without a PR (use /commit)." user-invocable: true disable-model-invocation: false argument-hint: "<action> [args] (e.g., /pull-request prep, /pull-request create, /pull-request monitor, /pull-request merge, /pull-request full, /pull-request status)" @@ -24,7 +24,7 @@ Orchestrate the PR lifecycle from quality review through merge and cleanup, with ## Adapting to your environment (graceful degrade) -This skill is self-contained: it runs on `git`, `gh`, and its own bundled scripts (under `${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/`). Where a phase names an adjacent capability — a code-review skill or agents, a simplifier, a build/test/lint verifier, an external research skill, an exploration skill, a work-item tracker, a CI-log-audit agent, a GitHub-events push channel — treat it as **optional**: if your environment provides it (a skill, plugin, agent, or MCP server), invoke it; otherwise proceed with the inline guidance, which stands on its own. Never block a phase because an adjacent tool is absent. +This skill is self-contained: it runs on `git`, `gh`, and its bundled scripts (skill-private ones under `${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/`, plugin-shared ones under `${CLAUDE_PLUGIN_ROOT}/scripts/`). Where a phase names an adjacent capability — a code-review skill or agents, a simplifier, a build/test/lint verifier, an external research skill, an exploration skill, a work-item tracker, a CI-log-audit agent, a GitHub-events push channel — treat it as **optional**: if your environment provides it (a skill, plugin, agent, or MCP server), invoke it; otherwise proceed with the inline guidance, which stands on its own. Never block a phase because an adjacent tool is absent. Consumer conventions come from the consuming project's own `CLAUDE.md`, `AGENTS.md`, and rules — notably: PR body template, branch naming, merge style (this skill defaults to squash), review-reply identity (some projects post bot-identity replies via a wrapper; default is plain `gh`), and any extra pre-PR gates. Read them before creating or merging. @@ -52,7 +52,10 @@ For PR lifecycle runs spanning 3+ phases, copy `${CLAUDE_PLUGIN_ROOT}/skills/pul | `status` | Report only | Unified status across all phases | | `full` | Phase 1-4 | Run prep → create → monitor → merge end-to-end | | `fetch-logs <pr\|run> [--raw\|--job <job-id>]` | CI log retrieval | Pull failed-CI evidence: default = `::error`/`::warning` annotations only (cheapest); `--raw` = full ZIP dump for archive review; `--job <id>` = per-job plain text | -| `babysit` | Phase 3+ (all-PR loop) | Discover all open PRs → checkout each branch → run monitor per-iteration checklist → fix valid bot findings → move to next. Designed for a self-pacing loop (`/loop /pull-request babysit`). Never merges. See [reference/babysit.md](reference/babysit.md) | + +For the all-PR continuous loop (discover every open PR, work each to readiness, self-pace), use +the sibling skill `/source-control:babysit-prs` — it wraps this skill's per-PR review discipline +in fleet orchestration and never merges. ## Action defaults @@ -117,7 +120,6 @@ Execute in order. Each phase is self-contained — read the relevant file for de | 1. Prep | [reference/prep.md](reference/prep.md) | `prep`, `prep quick`, `prep review-only`, `prep simplify-only` | | 2. Create | [reference/create.md](reference/create.md) | `create` | | 3. Monitor | [reference/monitor.md](reference/monitor.md) | `monitor`, `comments` | -| 3+. Babysit | [reference/babysit.md](reference/babysit.md) | `babysit` | | 4. Merge | [reference/merge.md](reference/merge.md) | `merge` | --- @@ -131,7 +133,7 @@ When entering Phase 3 (`monitor`, `comments`, or `full` reaching monitor), compl - **Dirty tree with unrelated WIP** (staged/unstaged/untracked from other work): do NOT switch — surface the WIP to the user and proceed read-only. Never `git stash` another session's WIP. - **Interactive session** (human present): changing branches re-points the working tree, so confirm the target branch with the user FIRST — UNLESS the invoking message already named the checkout (invoking `/pull-request monitor <N>` against a specific PR is intent, but the target-branch confirmation gate still governs the mechanical switch). - **Autonomous session** (e.g. `CLAUDE_CODE_REMOTE=true`): check out without prompting. - - **Babysit** runs its own per-PR checkout (babysit §5.1.2 Step 0.2) — this Step 0 is the single-PR `monitor` equivalent; don't double-checkout when reaching here from babysit. + - **`/source-control:babysit-prs`** runs its own per-PR checkout — this Step 0 is the single-PR `monitor` equivalent; don't double-checkout when the sibling loop skill applies this checklist. - [ ] **Step 1 — Cloud check:** if `CLAUDE_CODE_REMOTE=true`, use §3.0.0 `gh` polling. Skip remaining steps - [ ] **Step 2 — Push-channel gate (§3.0.05):** if your environment ships a GitHub-events push channel (an MCP server that delivers webhook events into the session), verify it is healthy per its own docs and this skill's §3.0.05 guidance (broker alive, subscriber fresh). No channel available → skip to Step 3's fallback - [ ] **Step 3 — Arm event delivery:** channel healthy → arm its PR filter for `<N>`; channel absent/unhealthy → arm the §3.0.1 Monitor tool watch @@ -149,9 +151,9 @@ When a channel event, Monitor notification, or poll iteration fires, complete AL - [ ] C1 — Review-thread comments: `gh api repos/<owner>/<repo>/pulls/<N>/comments --paginate` - [ ] C2 — Issue-level comments: `gh api repos/<owner>/<repo>/issues/<N>/comments --paginate` (includes AI-review summaries, user replies, bot task-completion posts) - [ ] C3 — PR reviews: `gh api repos/<owner>/<repo>/pulls/<N>/reviews --paginate` (review bodies contain findings — APPROVED/CHANGES_REQUESTED/COMMENTED reviews all may carry actionable content) - - [ ] C4 — Read every comment body in full. Summaries and review posts from ANY AI agent (claude[bot], codex, cursor, copilot) contain findings that require classification — these are NOT informational. **Extract individual findings** per [babysit.md](reference/babysit.md) §5.0.4 — one comment with N findings = N work items, each needing individual D1-D7. **For ≥3 findings, MANDATORY subagent dispatch** per §5.0.4 — preserves main session context, structurally enforces per-finding ledger shape + - [ ] C4 — Read every comment body in full. Summaries and review posts from ANY AI agent (claude[bot], codex, cursor, copilot) contain findings that require classification — these are NOT informational. **Extract individual findings** per [`${CLAUDE_PLUGIN_ROOT}/reference/review-discipline.md`](../../reference/review-discipline.md) §2 — one comment with N findings = N work items, each needing individual D1-D7. **For ≥3 findings, MANDATORY subagent dispatch** per the same §2 — preserves main session context, structurally enforces per-finding ledger shape - [ ] **D — For EACH unaddressed **finding** (not comment — one comment may contain multiple findings):** - - [ ] D1 — Read full finding context (parent comment body + surrounding findings). For multi-finding comments dispatched to a subagent (§5.0.4), this work is in the subagent; the main session receives the ledger + - [ ] D1 — Read full finding context (parent comment body + surrounding findings). For multi-finding comments dispatched to a subagent ([review-discipline](../../reference/review-discipline.md) §2), this work is in the subagent; the main session receives the ledger - [ ] D2 — Explore referenced code (must be on the PR branch for accurate results) - [ ] D3 — **Validate the claim** before trusting: verify the assertion against actual code, run the command, check the file. Research non-trivial claims against official docs. Never implement a fix based solely on a bot's assertion — confirm it is correct first - [ ] D4 — Classify: VALID (fix now) / VALID (defer) / INCORRECT / UNCERTAIN. Classification MUST cite evidence from D2-D3 @@ -168,24 +170,6 @@ When a channel event, Monitor notification, or poll iteration fires, complete AL **Receiving an event is NOT processing it.** Each event must drive at LEAST steps A-C. New comment events must drive D1-D7 for that comment. Declaring "ready to merge" without completing E is a checklist violation. -## Babysit per-PR checklist (MANDATORY — each PR within babysit loop) - -When running the `babysit` action, execute these steps for EACH PR discovered. The monitor entry checklist and per-iteration checklist apply per-PR — babysit wraps them in a multi-PR orchestration loop. - -- [ ] **Step 0 — PR discovery:** `gh pr list` filtered (skip draft, oldest-first). See [reference/babysit.md](reference/babysit.md) §5.0.2 -- [ ] **Step 0.1 — Evidence-based fresh rescan:** fetch ALL comments via the bundled `fetch-all-pr-comments.sh`, classify each as addressed/unaddressed by checking GitHub for substantive replies with classification + evidence. GitHub is the source of truth, not model memory. See §5.0.3 -- [ ] **Step 0.2 — Branch checkout:** `gh pr checkout <N>` (fork-safe — a fork's head branch is not fetchable from `origin` by name). MANDATORY before any comment investigation — exploration and research must run against PR branch code. Pre-checks first: already on the branch → no-op; branch checked out in ANOTHER worktree, or this worktree has foreign WIP → process read-only (no fix). See §5.1.2 -- [ ] **Step 0.3 — Branch freshness:** `git fetch origin <default-branch>` then `git merge-base --is-ancestor origin/<default-branch> HEAD`. If behind: integrate (merge vs rebase per the project's convention and the branch's own history — see §5.1.2), resolving conflicts via `/source-control:resolve-conflicts` discipline (understand both sides' intent; compose, don't side-pick) — in unattended babysit runs, a conflict that resolution can't settle conservatively is aborted and reported rather than guessed. Report status: current/rebased/conflict-aborted -- [ ] **Steps 1-4 — Monitor entry checklist** (above) — run per-PR. A push channel re-arms its PR filter for each PR -- [ ] **Steps A-F — Per-iteration monitoring checklist** (above) — run per-PR. Extract individual findings from each comment per §5.0.4 (one comment with N findings = N work items). **For any comment with ≥3 findings, MANDATORY subagent dispatch** per §5.0.4. Run D1-D7 per-finding, not per-comment. Verify each action landed on GitHub (D4.5/D5/D6/D7/D7.5 verification gates) -- [ ] **Step 5 — Commit + push** fixes on the PR branch. Clean working tree. Post follow-up replies with commit SHAs (D7) -- [ ] **Step 6 — PR transition:** advance to the next-oldest PR needing attention (round-robin). See §5.1.6 -- [ ] **Step 7 — Self-pace:** call `ScheduleWakeup` per the cadence table in §5.3 after all PRs are processed - -**Babysit NEVER merges.** Readiness gate pass → report ready → move to next PR. The user merges via `/pull-request merge` or `gh pr merge` manually. - -**Execution discipline:** babysit's primary failure mode is claiming to process findings without actually running per-finding D1-D7. Every iteration MUST output a completed checklist with evidence per step (see [babysit.md](reference/babysit.md) §5.5). "Done" means GitHub shows evidence — model memory of "I replied" or "I pushed" is not evidence. Re-query the API to verify each action landed. - --- ## Full lifecycle (`/pull-request full`) diff --git a/plugins/source-control/skills/pull-request/evals/evals.json b/plugins/source-control/skills/pull-request/evals/evals.json index b36ccd26f..4bfbf0774 100644 --- a/plugins/source-control/skills/pull-request/evals/evals.json +++ b/plugins/source-control/skills/pull-request/evals/evals.json @@ -102,18 +102,6 @@ "It runs the orphan-PR prompt instead of silently creating an unlinked PR" ] }, - { - "id": 9, - "name": "babysit-preserves-merge-workflow", - "prompt": "/pull-request babysit PR #482. The checked-out branch is behind main and `git log --merges origin/main..HEAD` returns a merge commit.", - "expected_output": "The skill integrates `origin/main` with `git merge`, preserving the branch's merge workflow, and uses a plain `git push`. It does not rebase or force-push the merge-commit branch.", - "files": [], - "expectations": [ - "It checks for merge commits before choosing an integration mode", - "It uses git merge origin/main for a merge-commit branch", - "It uses a plain push, not a force-with-lease push, after the merge" - ] - }, { "id": 10, "name": "worktree-reuse-protects-dirty-tree", diff --git a/plugins/source-control/skills/pull-request/reference/babysit.md b/plugins/source-control/skills/pull-request/reference/babysit.md deleted file mode 100644 index 280959860..000000000 --- a/plugins/source-control/skills/pull-request/reference/babysit.md +++ /dev/null @@ -1,462 +0,0 @@ -# Phase 3+: Babysit (all-PR continuous loop) - -Multi-PR orchestration layer wrapping the existing single-PR monitor infrastructure. Designed for `/loop /pull-request babysit` (dynamic, self-pacing via ScheduleWakeup). Processes every open PR in the repo — discovers, checks out, monitors, fixes, moves to next. Never merges. - -## 5.0 Focus-first rule - -**Process the oldest PR with unaddressed comments to completion before advancing to the next PR.** "Completion" = every comment on that PR has been: read in full, code explored, claim investigated, classified (VALID/INCORRECT/UNCERTAIN), reacted to, replied to with evidence, and fixed if VALID. Only after ALL comments on the current PR are resolved, move to the next oldest. - -A shallow survey of all PRs is NOT babysitting. Reporting "bot findings need classification" without classifying is NOT babysitting. Babysit means actively working each comment. - -### 5.0.1 Iteration entry — round-robin flow - -Each `/loop` wake-up runs one full babysit iteration. Round-robin from oldest to newest: - -1. **Discover** all open PRs (§5.0.2) -2. **Focus** the oldest PR with unaddressed comments or failing CI -3. **Checkout** the PR branch (§5.1.2) — mandatory for accurate exploration + research -4. **Process** all current comments on that PR (one wave — §5.1.3 checklist) -5. **Commit + push** fixes on the PR branch (§5.1.4) -6. **Advance** to the next-oldest PR needing attention — repeat steps 3-5 -7. **Skip** PRs with all comments addressed + CI green + no new activity -8. **Park** on the home branch after all PRs are processed (§5.2) -9. **Schedule** the next wake (§5.3) - -Keep circling — each iteration processes one wave per PR. New CI results and review comments from pushed fixes are picked up on the next iteration. - -### 5.0.2 PR discovery - -```bash -gh pr list --state open --limit 200 --json number,title,headRefName,isDraft,author \ - --jq '[.[] | select(.isDraft == false)] | sort_by(.number)' -``` - -Oldest-first (FIFO) — lowest PR number processed first. - -Deterministic equivalent: `bash "${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/discover-prs.sh"` runs this exact filter (open, skip draft, oldest-first; `--prs-json <file>` for offline/testing). - -**Filters:** - -- Skip `isDraft` PRs (signal "not ready for review") -- Every open PR is in scope regardless of author or label — Dependabot included. A Dependabot PR with failing CI needs the same diagnose-and-fix attention as any other; auto-merge (where configured) only fires once CI is green, so babysit owns the red ones - -**Zero-PR fast path:** if discovery returns an empty list, report `No open non-draft PRs need attention.` and call `ScheduleWakeup(delaySeconds=1200, reason="no open PRs", prompt="/pull-request babysit")`. Exit the iteration. - -### 5.0.3 Evidence-based fresh rescan - -Every iteration rescans ALL comments on every non-terminal PR. GitHub is the source of truth — not model memory, not prior-iteration state, not comment counts. - -**Why full rescan:** compaction loses prior-iteration classification state. Comment-count heuristics miss edits, deletions, and multi-finding comments. An evidence-based fresh rescan from GitHub every iteration defeats both failure modes. - -**Per-PR rescan flow:** - -1. **Terminal check** — `gh pr view <N> --json state -q '.state'`. MERGED/CLOSED → skip -2. **CI check** — `gh pr checks <N> --json bucket -q '[.[] | .bucket] | unique'` -3. **Fetch ALL comments** — run `bash "${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/fetch-all-pr-comments.sh" <N>` to retrieve every comment from all 3 API surfaces (review-thread, issue-level, PR reviews). Full bodies, not counts -4. **Filter out own prior replies** — comments authored by your own posting identities (`gh api user --jq .login`, plus any project bot identity — the same set the readiness gate's `--self` / `BABYSIT_SELF_LOGINS` covers) that ARE classification replies (contain the `| # | Finding | Classification |` table pattern) are NOT findings — skip them. Own follow-up replies citing commit SHAs are also not findings. Only process comments from OTHER authors as potential finding sources -5. **Classify each remaining comment** as "addressed" or "unaddressed" by checking GitHub for evidence: - - **Addressed (skip)** — the comment has a substantive reply (from ANY author) containing BOTH: (a) a classification token (VALID, INCORRECT, or UNCERTAIN), AND (b) evidence (code reference, test output, or reasoning) - - **Unaddressed (process)** — no reply meeting both criteria. "Noted" or "will fix" without classification + evidence does NOT count -6. **Extract findings** per §5.0.4 — one comment may contain multiple work items - -**Needs attention when ANY of:** - -- CI has `fail` / `pending` / `in_progress` bucket entries -- Any comment has unaddressed findings (per the classification above) - -**Skip when ALL of:** - -- State is terminal (MERGED/CLOSED) -- All checks pass/skipping AND zero unaddressed findings - -PRs not needing attention are reported in a one-line status summary and skipped. - -### 5.0.4 Structured finding extraction - -AI review summaries (claude[bot], codex, cursor, etc.) and detailed human reviews often pack multiple findings into a single comment — markdown tables, numbered lists of severity items, or multi-paragraph analyses. Each finding is a separate work item requiring individual D1-D7. - -**Extraction rules:** - -- One comment with N findings = N entries in the work item list -- Each finding gets its own D1-D7 cycle (read, explore, validate, classify, reply, fix, follow-up) -- Findings are tracked individually — addressing 3 of 5 findings in a comment means 2 remain unaddressed -- Reply with a per-finding classification table (not one blanket reply for the whole comment) - -**Finding identification signals:** - -- Numbered items with severity labels (CRITICAL, IMPORTANT, SUGGESTION, P1/P2/P3) -- Markdown table rows with file/line/description columns -- Bullet lists where each bullet describes a distinct code concern -- Multiple `###` sub-headings each addressing different files or concerns - -**Per-finding classification table format** (reply on the comment): - -```text -| # | Finding | Classification | Evidence | Reacted | -|---|---------|---------------|----------|---------| -| 1 | <summary> | VALID — fixing | <evidence> | 👍 | -| 2 | <summary> | INCORRECT | <evidence why wrong> | 👎 | -| 3 | <summary> | VALID (defer) | <reason for deferral> | 👍 | -``` - -The reaction is per-comment (GitHub allows one reaction type per user per comment). Post the reaction BEFORE the reply — reviewers scanning a PR see 👍/👎 at a glance without expanding threads. - -**MANDATORY subagent dispatch for multi-finding comments (≥3 findings):** - -When a single PR comment packs 3+ findings, dispatch a finding-extractor subagent rather than attempting inline extraction. The subagent: - -1. Preserves main session context — large comment bodies + per-finding investigation evidence stay in the subagent's context window; only the structured ledger returns -2. Structurally enforces the per-finding work-item shape — the subagent returns a fixed-schema ledger; missing entries trigger main-session escalation -3. Is scope-fenced — ALLOWED: read PR-branch files + `gh api` against the specific PR; FORBIDDEN: edits, commits, pushes, reactions, replies on GitHub (those stay in the main session) - -**Subagent dispatch prompt (compose verbatim, substitute `<PR>` and `<COMMENT_ID>` / `<REVIEW_ID>`):** - -```text -Extract individual findings from the multi-finding bot/human review at: - https://github.com/<owner>/<repo>/pull/<PR>#issuecomment-<COMMENT_ID> - (or pull/<PR>#pullrequestreview-<REVIEW_ID>) - -ALLOWED scope (read-only on PR branch <BRANCH>): -- `gh api repos/<owner>/<repo>/issues/<PR>/comments` and per-id endpoints -- `gh api repos/<owner>/<repo>/pulls/<PR>/{comments,reviews}` and per-id endpoints -- `Read` / `Grep` / `Glob` against the repo working tree -- `Bash` for git inspection (`git show`, `git log`, `git diff`) — NEVER state-mutating - -FORBIDDEN: -- Any Edit / Write of repo files -- Any `git add` / `git commit` / `git push` -- Any reaction / reply / comment POST to GitHub -- Any Skill invocation other than read-only exploration - -Return a SINGLE markdown ledger with this exact shape (one row per finding): - -| # | Severity | File:Line | Finding (≤120 chars) | Validation status | Evidence | Suggested classification | -|---|---|---|---|---|---|---| -| 1 | CRITICAL | path/to/file.cs:42 | <one-line summary> | VERIFIED — code matches claim | <quote 1-3 lines of code OR test output OR doc text> | VALID — fix now | -| 2 | IMPORTANT | path/to/file.cs:73 | <one-line summary> | INCORRECT — code already does X | <counter-evidence> | INCORRECT | -| 3 | SUGGESTION | path/to/file.md:12 | <one-line summary> | UNCERTAIN — behavior depends on Y | <what's missing> | UNCERTAIN | - -CRITICAL constraints on the ledger: -- Severity column MUST match the parent comment's severity labels verbatim (CRITICAL / IMPORTANT / SUGGESTION / P1 / P2 / P3) -- Validation status MUST come from your own code reading, not a paraphrase of the bot claim -- Evidence MUST cite line numbers + verbatim snippets (≤3 lines) OR direct command output -- Suggested classification MUST be one of: VALID — fix now | VALID (defer) | INCORRECT | UNCERTAIN -- One row per finding. If the parent comment has 6 findings, the ledger has 6 rows. No collapsing. - -If the parent comment is genuinely single-finding, return a 1-row ledger anyway. - -Report ONLY the ledger + a one-line summary count ("Extracted N findings: X CRITICAL, Y IMPORTANT, Z SUGGESTION"). No prose framing. -``` - -**Main-session contract after the subagent returns:** - -1. Receive the ledger. Verify the row count matches the source comment's finding count (independent count via grep on the parent comment body for severity markers) -2. For each ledger row, the main session runs D4.5 (react) + D5 (reply with the per-finding sub-row from the ledger) + D6 (fix if VALID — fix now) + D7 (follow-up SHA) with verification gates between each step -3. The subagent ledger is the D1-D4 work product. The main session NEVER skips D4.5-D7 by trusting the ledger alone — the ledger feeds the work, it doesn't replace it - -**Single-finding comments** (1-2 findings): inline extraction in the main session is fine; subagent overhead is not warranted. - -**Why a subagent for ≥3 findings:** empirically, multi-finding comments treated as single work items in the main session produce near-zero per-finding D1-D7 cycles — dozens of findings glossed in one pass. Subagent dispatch structurally forces the per-finding shape because the ledger contract demands it. - -**Mechanical enforcement (gate, not prose):** advisory "MANDATORY" wording alone still under-decomposed in practice. So enforcement is a gate: `babysit-readiness-gate.sh <pr>` (run at §5.1.3 step E) counts source findings (severity markers in reviewer comments) vs classification rows (VALID/INCORRECT/UNCERTAIN in your replies) and exits non-zero when rows < findings. The subagent-dispatch rule above tells you HOW to decompose; the gate enforces THAT you did — readiness cannot be declared while it reports `READINESS_BLOCKED`. - -## 5.1 Per-PR processing - -For each PR needing attention (oldest first): - -### 5.1.1 Event-delivery gate - -Run the **Monitor entry checklist** from SKILL.md — the identical 4-step sequence: - -1. Cloud check (skip if `CLAUDE_CODE_REMOTE=true`) -2. Push-channel gate (§3.0.05) — when your environment ships one -3. Arm event delivery for this PR (channel PR filter, or Monitor watch) -4. Proceed to monitoring - -A push channel arms for ONE PR at a time. Re-arm for each new PR in the loop. - -### 5.1.2 Branch checkout (MANDATORY for accurate exploration) - -(`main` below — substitute the repo's default branch.) - -```bash -# Pre-check 0: already on the PR branch? This session owns it — no checkout -# needed (the current worktree also shows up in `git worktree list`, so the -# other-worktree grep below would otherwise false-trip to read-only). -# Pre-check 1: is the branch checked out in ANOTHER worktree? -# Pre-check 2: does THIS worktree have uncommitted changes? They may be -# another session's WIP — never reset/clean work this loop did not create. -BRANCH="<headRefName>" -CUR_BRANCH=$(git branch --show-current) -CUR_WT=$(git rev-parse --show-toplevel) -DEFAULT_BRANCH=$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name) -if [ "$CUR_BRANCH" = "$BRANCH" ]; then - # Already own the branch — no checkout; freshness check below still runs. - # The dirty-tree guard still applies: uncommitted changes may be another - # session's WIP even on this branch — full mode only on a clean tree. - git fetch origin "$DEFAULT_BRANCH" - if [ -n "$(git status --porcelain)" ]; then - echo "Working tree has uncommitted changes — processing read-only" - CHECKOUT_MODE="read-only" - else - CHECKOUT_MODE="full" - fi -elif git worktree list | grep -vF "$CUR_WT " | grep -q "\[$BRANCH\]"; then - echo "Branch $BRANCH checked out in another worktree — processing read-only" - CHECKOUT_MODE="read-only" -elif [ -n "$(git status --porcelain)" ]; then - echo "Working tree has uncommitted changes (possibly another session's WIP) — no checkout, processing read-only" - CHECKOUT_MODE="read-only" -else - git fetch origin "$DEFAULT_BRANCH" - # gh pr checkout handles fork-sourced PRs (head branch not fetchable from - # origin) and same-repo branches alike — never bare fetch/checkout by name. - gh pr checkout "$PR_NUMBER" - CHECKOUT_MODE="full" -fi - -# Branch freshness — preserve the branch's integration workflow (full mode only) -if [ "$CHECKOUT_MODE" = "full" ]; then - if ! git merge-base --is-ancestor "origin/$DEFAULT_BRANCH" HEAD; then - if git log --merges --format='%H' "origin/$DEFAULT_BRANCH..HEAD" | grep -q .; then - INTEGRATION_MODE="merge" - echo "Branch $BRANCH uses merge commits — merging origin/$DEFAULT_BRANCH" - git merge --no-edit "origin/$DEFAULT_BRANCH" - INTEGRATION_EXIT=$? - else - INTEGRATION_MODE="rebase" - echo "Branch $BRANCH is behind origin/$DEFAULT_BRANCH — rebasing" - git rebase "origin/$DEFAULT_BRANCH" - INTEGRATION_EXIT=$? - fi - - if [ "$INTEGRATION_EXIT" -eq 0 ]; then - REBASE_STATUS="integrated" - # A merge preserves existing commits and pushes normally. A rebase - # rewrites them and therefore needs a lease-protected force push. - if [ "$INTEGRATION_MODE" = "merge" ]; then - git push - else - git push --force-with-lease - fi - else - # Graduated conflict handling — attempt simple, abort complex. - # conflict-attempting is a TRANSIENT state: resolve it (merge/rebase - # --continue) or abort BEFORE any further processing — never leave an - # integration in progress (unmerged paths break later checkouts + parking). - CONFLICT_COUNT=$(git diff --name-only --diff-filter=U | grep -c . || true) - if [ "$CONFLICT_COUNT" -le 3 ]; then - echo "Simple conflict ($CONFLICT_COUNT files) — attempting resolution" - REBASE_STATUS="conflict-attempting" - else - echo "Complex conflict ($CONFLICT_COUNT files) — aborting $INTEGRATION_MODE" - if [ "$INTEGRATION_MODE" = "merge" ]; then - git merge --abort - else - git rebase --abort - fi - REBASE_STATUS="conflict-aborted" - fi - fi - else - REBASE_STATUS="current" - fi - - # conflict-attempting: resolve NOW — per file, take the mechanical - # resolution; if ANY file needs intent judgment, abort the active merge or - # rebase and set REBASE_STATUS="conflict-aborted". On success: `git add - # <files>` + the matching `git merge --continue` / `git rebase --continue`, - # then plain `git push` for a merge or `git push --force-with-lease` for a - # rebase. Set REBASE_STATUS="integrated". Only terminal states pass this point. - - # Safe fallback: ONLY the terminal success states keep full mode. A - # lingering conflict-attempting (resolution skipped) degrades to read-only - # rather than granting write access mid-rebase. - if [ "$REBASE_STATUS" != "integrated" ] && [ "$REBASE_STATUS" != "current" ]; then - CHECKOUT_MODE="read-only" - fi -fi -``` - -**Integration conflict handling (graduated).** Check for merge commits first (`git log --merges origin/$DEFAULT_BRANCH..HEAD`) — a branch that previously merged the default branch integrates via `git merge origin/$DEFAULT_BRANCH` plus a plain push; other branches rebase and force-push with lease. Then: - -- **Zero conflicts** (`REBASE_STATUS=integrated`) — merge or rebase succeeded, push with the mode-appropriate command, continue normally -- **Simple conflicts** (≤3 files, `REBASE_STATUS=conflict-attempting`) — TRANSIENT: attempt resolution immediately; on success continue the active merge/rebase and push with the mode-appropriate command → `integrated`; if ANY file requires intent judgment, abort the active integration → `conflict-aborted`. Never proceed to comment processing, parking, or the next PR with an integration in progress -- **Complex conflicts** (>3 files, `REBASE_STATUS=conflict-aborted`) — abort the merge/rebase, post a PR comment: `"⚠️ Branch is behind main with integration conflicts ({N} files). Manual resolution is required before CI will trigger."`. If an interactive terminal, also surface to the user directly. Process comments read-only (classification + reply, no fixes — the code may be stale) -- **Already current** (`REBASE_STATUS=current`) — no action needed - -**Why mandatory:** exploration and research read files from the working tree. Without checkout, findings are validated against the wrong code. Branch freshness prevents CI failures from stale code and ensures conflict detection happens proactively. - -**Read-only mode:** investigate comments, explore referenced code via `git show origin/<branch>:<path>`, research claims, classify, reply with evidence — the full D1-D5 workflow. Only D6-D7 (edit + commit + push + follow-up reply) are blocked. Read-only is NOT passive — every comment still gets investigated and replied to. Fixes that can't be pushed are described in the reply with exact code changes so the user or the PR's own worktree session can apply them. - -**Full mode:** full flow including the fix cycle (D1-D7). Commit and push on the PR branch after each wave of fixes. - -### 5.1.3 Per-finding D1-D7 with verification gates - -D steps operate **per-finding**, not per-comment. One comment with 5 findings = 5 individual D1-D7 cycles. Must be on the PR branch (§5.1.2) before starting. - -- [ ] **A** — Terminal state check (`gh pr view <N> --json state`) -- [ ] **B** — CI checks — classify every non-pending check (pass/fail/skipped) -- [ ] **C** — Fetch ALL comments and extract findings: - - [ ] C1 — Run the bundled `fetch-all-pr-comments.sh <N>` (all 3 API surfaces) - - [ ] C2 — Read every comment body in full - - [ ] C3 — Extract individual findings per §5.0.4 - - [ ] C4 — Build the work-item list: one entry per finding, each needing D1-D7 -- [ ] **D** — For EACH unaddressed **finding** (not comment): - - [ ] D1 — Read full finding context (parent comment body + surrounding findings) - - [ ] D2 — Explore referenced code on the PR branch - - [ ] D3 — **Validate the claim** — verify against actual code before trusting. Research non-trivial claims - - [ ] D4 — Classify with evidence: VALID (fix now) / VALID (defer) / INCORRECT / UNCERTAIN - - [ ] D4.5 — React to the parent comment via `gh api .../reactions`. One reaction per comment (not per finding). **Tiebreaker for mixed-finding comments:** `+1` if ANY finding is VALID (signals action taken), `-1` only when ALL are INCORRECT, `eyes` when all UNCERTAIN or a mix of UNCERTAIN + INCORRECT with zero VALID - - [ ] **verify reaction exists:** GET the same reactions endpoint filtered by your posting identities — non-zero confirms. Use `pulls/comments/<id>/reactions` for inline review comments - - [ ] D5 — Reply with the per-finding classification table + evidence (before fixing). Table format per §5.0.4 — includes the Reacted column. **Route the reply by comment type — REQUIRED, not interchangeable:** inline review comments (diff-anchored, `pulls/comments`) MUST reply THREADED via `gh api repos/{owner}/{repo}/pulls/<N>/comments/<comment-id>/replies -f body='...'` so the reply lands under the source thread — NEVER a detached `pr comment`. Issue-level / review-level comments (no thread) → `gh pr comment <N> --body '...'`. Use the project's bot-identity wrapper for these writes when it has one. Answering an inline finding with a detached issue comment orphans the reply from the thread the reviewer tracks — a routing error, not a style choice - - [ ] **verify reply exists:** `gh api repos/{owner}/{repo}/issues/<N>/comments --jq '.[].body'` — confirm the reply text on GitHub. For inline replies: `gh api repos/{owner}/{repo}/pulls/<N>/comments --jq '.[] | select(.in_reply_to_id == <original-id>)'` - - [ ] D6 — Fix if VALID → edit, `git add <files>`, commit, push - - [ ] **verify commit pushed:** `gh api "repos/{owner}/{repo}/commits?sha=<branch>&per_page=1" --jq '.[0].sha'` — confirm the fix commit SHA on the remote - - [ ] D7 — Post a follow-up reply citing the fix commit SHA - - [ ] **verify follow-up reply posted — same surface routing as D5:** inline thread → `gh api repos/{owner}/{repo}/pulls/<N>/comments --jq '.[] | select(.in_reply_to_id == <original-id>)'`; issue-level → `gh api repos/{owner}/{repo}/issues/<N>/comments --jq '.[-1].body'` — confirm the follow-up with SHA on GitHub - - [ ] D7.5 — Resolve review thread — **author-conditional** (canonical policy: SKILL.md D7.5), inline review comments only. Resolve ONLY threads whose OPENING comment is authored by a BOT reviewer that you addressed. NEVER resolve HUMAN-authored threads — the human resolves their own after verifying the fix. NEVER resolve your OWN threads (any of your posting identities — same self set as §5.0.3 step 4). Skip issue-level comments (no thread). **Thread author = login of the THREAD-OPENING comment** (replying into it does not change the author). **Bot detection is API-surface-specific:** resolution runs via GraphQL (the threadId fetch), where bot authors have `author.__typename == "Bot"` and `login` omits the `[bot]` suffix; REST surfaces show the suffix. When fetching the threadId, also select `author{__typename login}` to apply the conditional in one query - - [ ] **verify thread resolved:** query the thread node via `gh api graphql` — `isResolved` must be `true` -- [ ] **E** — Readiness gate. Run `bash "${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/babysit-readiness-gate.sh" <N>` — exit 0 `READINESS_OK` is REQUIRED to proceed. Exit 1 `READINESS_BLOCKED reason=under-decomposed` means classification rows < source findings → decompose + classify the missing findings, then re-run. THEN confirm: all checks terminal + 2-min cooldown -- [ ] **F** — Per-finding classification table + readiness report (see §5.5) - -**"Done" means GitHub shows evidence.** A per-finding work item is addressed only when the verification sub-step confirms the action landed on GitHub. Model memory of "I posted a reply" is not evidence — re-query the API. - -### 5.1.4 Fix cycle (full mode only) - -When on the PR branch AND a comment is classified VALID after D3 validation: - -- [ ] Edit code to fix the issue -- [ ] `git add <specific-files>` (never `-A` or `.`) -- [ ] `git commit -m "<type>: <description>"` -- [ ] `git push` -- [ ] Post a follow-up reply citing the commit SHA (D7) - -**One wave at a time:** address all current comments on this PR → commit + push → then round-robin to the next PR. Don't jump between PRs mid-wave. After pushing, new CI runs trigger — those results are checked on the next babysit iteration (or the next round-robin pass if processing multiple PRs). - -**Re-review trigger after a fix push:** bots that reviewed the PR may need an explicit trigger to re-evaluate fixes. After pushing, check each bot's trigger mode per readiness.md "Expected PR actors": - -- **"On every push" trigger** — re-reviews automatically, just wait -- **Manual/smart trigger** (e.g., Codex) — post `@codex review` (or the bot's equivalent) as a PR comment to request a re-review - -Per monitor §3.2: research-gate non-trivial fixes (multi-source consensus). Max 3 CI fix iterations per PR per babysit pass. Inline-vs-subagent choice for CI log fetching per monitor.md "Inline vs subagent dispatch decision". - -### 5.1.5 Human comments - -Classify but DO NOT auto-fix. Reply with investigation findings per step D. Note: D4.5 reactions proceed autonomously for human reviewer comments (no approval gate — babysit runs without a user present). This differs from monitor.md step 4, which pauses for approval in interactive sessions. Report to the user in the babysit iteration output — human review items are surfaced, not silently skipped. - -### 5.1.6 PR done — transition to next - -When the readiness gate passes OR all actionable items are handled for this PR: - -1. If on a PR branch with uncommitted changes from a failed fix: `git reset --hard HEAD` then `git clean -fd` (unstage + revert tracked + remove untracked) -2. Report PR status (ready / blockers remaining / items deferred to human) -3. Move to the next PR in the discovery list - -## 5.2 Parking - -After all PRs are processed (or none needed attention), return to the worktree's home branch. Record at iteration start: - -```bash -PARKING_BRANCH=$(git rev-parse --abbrev-ref HEAD) -``` - -After processing all PRs: - -```bash -git checkout "$PARKING_BRANCH" -``` - -## 5.3 Self-pacing (ScheduleWakeup) - -At the end of each iteration, schedule the next wake based on observed state: - -| Condition | Delay | Reason | -|-----------|-------|--------| -| Active events flowing (CI running, fresh comments arrived during this iteration) | 60s | Stay responsive to in-flight activity | -| PRs exist but all currently quiet (no new events, no pending checks) | 270s | Check back soon without idle churn | -| No PRs need attention (all ready, all terminal, or zero open PRs) | 1200s | Long idle — conserve request budget | - -```text -ScheduleWakeup( - delaySeconds: <per table above>, - reason: "<specific reason for this delay>", - prompt: "/pull-request babysit" -) -``` - -## 5.4 NEVER-do list - -These constraints override any other instruction within babysit mode: - -- **Never declare readiness or schedule the next wake without a passing `babysit-readiness-gate.sh <N>` run** (exit 0 `READINESS_OK`). The gate counts classification rows vs source findings and blocks under-decomposition. "I classified them" is not evidence — the gate exit code is. See §5.1.3 step E -- **Never survey-and-report without investigating** — every unaddressed comment gets D1-D7 (read, explore, validate, classify, reply, fix, follow-up). "Bot findings need classification" without classifying is a violation -- **Never trust a finding without validating** — bot/AI assertions have demonstrated error rates. Always verify against actual code (D3) before implementing. Explore the referenced code; research non-trivial claims -- **Never process comments from the wrong branch** — must be on the PR branch before D2-D3. Exploring code on the default branch or another branch produces wrong classifications -- **Never advance to the next PR with unaddressed comments on the current PR** — focus-first rule (§5.0). Complete the current wave before moving on -- **Never skip AI review summaries** — AI-reviewer posts (issue-level comments with severity-labeled findings) are actionable comments requiring D1-D7. Same for every AI reviewer -- **Never `gh pr merge`** — babysit declares readiness; the user merges -- **Never `git add -A` or `git add .`** — specific files only -- **Never auto-fix human reviewer comments** — classify + reply + report to the user -- **Never skip the event-delivery gate** — run the Monitor entry checklist for every PR -- **Never exceed 3 CI fix iterations** per PR per babysit pass -- **Never leave uncommitted changes** on a PR branch when transitioning to the next PR -- **Never skip emoji reactions** — every classified finding gets a reaction on its parent comment (+1 VALID, -1 INCORRECT, eyes UNCERTAIN). Reactions are the fastest audit signal for reviewers scanning a PR -- **Never skip the branch freshness check** — always `git fetch origin <default-branch>` + `git merge-base --is-ancestor origin/<default-branch> HEAD` after checkout. Stale branches cause CI failures; proactive integration is cheaper than a reactive fix. See §5.1.2 -- **Never skip reply verification** — after posting a reply (D5) or follow-up (D7), verify it landed on GitHub via API query. Model memory of "I replied" across compaction is not evidence -- **Never skip resolving a BOT-authored thread; never resolve a HUMAN or OWN thread** — after fixing + replying to an inline review comment opened by a bot reviewer, resolve that thread (D7.5, author-conditional). Leave HUMAN-authored threads for the human to close; never resolve your own. Open bot-thread count is a visible signal to reviewers -- **Never process your own prior replies as findings** — filter out comments from your own posting identities that match the classification reply pattern. See §5.0.3 step 4 - -## 5.5 Checklist-driven output format - -Every iteration MUST output a completed checklist with evidence per step. Free-form narrative reports are not acceptable — they hide skipped steps. - -**Gate-enforced:** readiness requires a passing `babysit-readiness-gate.sh <N>` run (§5.1.3 step E). To mechanically gate checklist completeness too, write this iteration's checklist to a file in your working-notes location and pass `--checklist <file>` — the gate exits non-zero while any `- [ ]` box is unticked, so an incomplete checklist cannot be declared "ready". - -```text -## Babysit iteration [<timestamp>] - -### A. PR Discovery -- [ ] Fetched open PRs: <N> total, <M> needing attention, <K> skipped -- [ ] Processing order (oldest first): #<N1>, #<N2>, ... - -### B. Per-PR Processing - -#### PR #<N> — <title> (<branch>) -- [ ] **Branch:** checked out <branch> (mode: full/read-only) -- [ ] **Branch freshness:** <current/integrated/conflict-aborted> — evidence: `git merge-base` output -- [ ] **CI:** <pass/fail/pending> — evidence: `gh pr checks <N>` output -- [ ] **Comments fetched:** <N> total from all 3 API surfaces (<M> self-replies filtered) -- [ ] **Findings extracted:** <M> individual findings from <K> comments - -##### Per-finding classification table -| # | Source | Finding | Classification | Evidence | Reacted | Action | -|---|--------|---------|---------------|----------|---------|--------| -| 1 | comment <id> | <summary> | VALID | <evidence> | 👍 | Fixed: <sha> | -| 2 | comment <id> | <summary> | INCORRECT | <evidence> | 👎 | Replied | -| 3 | comment <id> | <summary> | UNCERTAIN | <evidence> | 👀 | Deferred | - -##### Verification evidence -- [ ] All reactions verified on GitHub: YES/NO -- [ ] All replies verified on GitHub: YES/NO -- [ ] All commits verified pushed: YES/NO -- [ ] All follow-ups verified posted: YES/NO -- [ ] All addressed BOT-authored inline threads resolved (human + own threads excluded): YES/NO/N/A - -##### PR status -- [ ] Readiness: ready for merge / <remaining blockers> - -### C. Iteration Summary -- [ ] All PRs processed: YES/NO -- [ ] Parked on home branch: YES -- [ ] **Next wake:** <delay>s — <reason> -``` - -Every `- [ ]` must be ticked `- [x]` with evidence before the iteration ends. Unticked boxes = incomplete iteration — do not schedule the next wake until addressed or explicitly deferred with reason. - -## 5.6 Performance notes - -- **Do not skip verification steps.** The D5/D6/D7 verification sub-steps exist because model memory is unreliable across compaction boundaries. One API call to confirm costs seconds; acting on false memory costs an entire re-processing cycle -- **Quality over speed.** Processing 3 findings thoroughly with verified evidence is better than "processing" 10 findings with blanket classifications and no verification -- **One finding at a time.** Complete per-finding D1-D7 for finding N before starting finding N+1. Interleaving findings across comments produces partial work that looks complete but isn't -- **Evidence-based state, not memory-based state.** Never say "I already replied to that" — check GitHub. Never say "I already pushed that fix" — check the remote. GitHub is the state store; this session's memory is ephemeral diff --git a/plugins/source-control/skills/pull-request/reference/monitor.md b/plugins/source-control/skills/pull-request/reference/monitor.md index bd6af3bbb..1348d24c9 100644 --- a/plugins/source-control/skills/pull-request/reference/monitor.md +++ b/plugins/source-control/skills/pull-request/reference/monitor.md @@ -279,7 +279,7 @@ For each security finding: **Fetch all comments deterministically** via the bundled script — never select API surfaces by agent judgment: ```bash -bash "${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/fetch-all-pr-comments.sh" <pr-number> +bash "${CLAUDE_PLUGIN_ROOT}/scripts/fetch-all-pr-comments.sh" <pr-number> ``` Output: a JSON array sorted by `created_at`. Each object carries `type` (`general` | `review` | `inline`), `author`, `body`, `path`, `line`, `id`. The script hits all 3 GitHub API surfaces (issue-level comments, review-level comments, inline review comments) — no surface can be accidentally skipped. @@ -292,7 +292,7 @@ Process every comment before fixing any. Produces a complete picture of what nee For **every substantive comment from every participant** (bot accounts with the `[bot]` suffix, human reviewers, AND the PR author's own comments — skip only LGTM/empty/emoji-only): -**Finding extraction for multi-finding comments:** AI review summaries often pack multiple findings into a single comment — markdown tables, numbered severity items, multi-paragraph analyses. Extract each finding as a separate work item. One comment with N findings = N individual evaluate cycles below. Reply with a per-finding classification table, not one blanket reply. See [babysit.md](babysit.md) §5.0.4 for extraction rules. +**Finding extraction for multi-finding comments:** AI review summaries often pack multiple findings into a single comment — markdown tables, numbered severity items, multi-paragraph analyses. Extract each finding as a separate work item. One comment with N findings = N individual evaluate cycles below. Reply with a per-finding classification table, not one blanket reply. See [review-discipline.md](../../../reference/review-discipline.md) §2 for extraction rules (including the mandatory ≥3-finding subagent dispatch). 1. **Explore** — read the referenced file/line, understand the surrounding code, check related files. Don't evaluate a comment about line 42 without understanding lines 1-100 2. **Research** — verify the specific technical claim against official docs (via a research skill when available). No assumptions, no "this looks right." The sequence is: explore → research → classify. Never: read → classify @@ -353,9 +353,9 @@ After the push: - **Codex signals via emoji reactions, not comments.** `chatgpt-codex-connector[bot]` uses emoji reactions on the PR: 👍 = no findings, approved; 👀 = still reviewing. A thumbs-up reaction with no posted comments means Codex reviewed and found nothing — treat as approval. Don't wait for a comment that won't arrive - **Codex may not auto-fire on PR creation.** If its commit status stays `PENDING` with no emoji reaction on the PR body after ~3 minutes, it likely didn't trigger. Post a PR comment with `@codex review` to trigger manually; check reactions on that trigger comment specifically - **NEVER select API surfaces by judgment — use the script.** `gh pr view --json comments,reviews` MISSES inline review comments. Always invoke the bundled `fetch-all-pr-comments.sh`, which deterministically hits all 3 surfaces. Observed failure mode: an agent chose `gh pr view --json comments,reviews`, missed 2 valid inline findings, and declared "no comments to address" -- **Never mark a comment addressed without verifiable evidence on GitHub.** Model memory of "I replied" or "I pushed the fix" is not evidence — compaction can lose that state between iterations. Re-query GitHub to verify: reaction exists, reply exists, commit pushed, follow-up posted, bot-authored thread resolved (inline only; human/own excluded). "Done" = GitHub shows evidence. See [babysit.md](babysit.md) §5.1.3 verification gates +- **Never mark a comment addressed without verifiable evidence on GitHub.** Model memory of "I replied" or "I pushed the fix" is not evidence — compaction can lose that state between iterations. Re-query GitHub to verify: reaction exists, reply exists, commit pushed, follow-up posted, bot-authored thread resolved (inline only; human/own excluded). "Done" = GitHub shows evidence. See [review-discipline.md](../../../reference/review-discipline.md) §3 verification gates - **Resolve BOT-authored inline threads after fix; never human or own.** After the D6 fix + D7 follow-up on an inline review comment opened by a bot reviewer, resolve that thread (D7.5, author-conditional). Leave HUMAN-authored threads for the human to close; never resolve your own. Detect bot at resolution time via GraphQL `author.__typename == "Bot"` (GraphQL login omits the `[bot]` suffix REST shows). Open bot-thread count is a visible signal to reviewers — leaving bot threads unresolved after fixing undermines the audit trail -- **Filter your own prior replies during rescan.** Comments from your own posting identity matching the classification-table pattern (`| # | Finding | Classification |`) are NOT findings — they are prior replies. Skip them during finding extraction. See [babysit.md](babysit.md) §5.0.3 step 4 +- **Filter your own prior replies during rescan.** Comments from your own posting identity matching the classification-table pattern (`| # | Finding | Classification |`) are NOT findings — they are prior replies. Skip them during finding extraction. See [review-discipline.md](../../../reference/review-discipline.md) §1 step 1 ## 3.4 Final monitoring report (readiness-gated) diff --git a/plugins/source-control/skills/pull-request/scripts/discover-prs.sh b/plugins/source-control/skills/pull-request/scripts/discover-prs.sh deleted file mode 100755 index 9b59bfcfa..000000000 --- a/plugins/source-control/skills/pull-request/scripts/discover-prs.sh +++ /dev/null @@ -1,113 +0,0 @@ -#!/usr/bin/env bash -# discover-prs.sh — deterministic PR discovery for the babysit loop. -# -# Implements the babysit "§5.0.2 PR discovery" filter as a script: lists OPEN -# PRs, drops drafts, returns oldest-first (lowest PR number first / FIFO). -# Dependabot and every other author are in scope (babysit covers all PR authors). -# Per babysit.md §5.0.2. -# -# The raw PR array is resolved from EITHER a fixture file (--prs-json, for -# offline tests / reuse) OR a live `gh pr list` call. The FILTER itself runs -# in this script body (NOT in `gh --jq`) so the fixture path exercises the -# exact same filter logic the live path does — that split is what makes the -# black-box test offline-capable. -# -# Owner/repo are NOT hardcoded: `gh pr list` auto-resolves the repository from -# the local git remote, so no -R flag or `gh repo view` round-trip is needed. -# -# Usage: -# discover-prs.sh # live: gh pr list, then filter -# discover-prs.sh --prs-json <file> # offline: read raw array from file -# discover-prs.sh --help -# -# Output (stdout): JSON array of {number,title,headRefName,isDraft,author} -# objects, drafts + Dependabot removed, sorted ascending by number. Empty -# discovery yields `[]`. -# -# Exit codes: -# 0 success (zero or more PRs emitted) -# 1 invalid argument or malformed input -# 2 gh api call failed -# 5 prerequisite missing (gh, jq) - -set -uo pipefail # -e omitted: gh failure explicitly guarded with || { exit N } - -PRS_JSON="" - -usage() { - sed -n '2,31p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' - exit 0 -} - -while (($# > 0)); do - case "$1" in - -h | --help) usage ;; - --prs-json) - if [[ $# -lt 2 ]]; then - printf 'discover-prs: --prs-json requires an argument (use --help)\n' >&2 - exit 1 - fi - PRS_JSON="$2" - shift 2 - ;; - -*) - printf 'discover-prs: unknown flag %q (use --help)\n' "$1" >&2 - exit 1 - ;; - *) - printf 'discover-prs: unexpected argument %q (use --help)\n' "$1" >&2 - exit 1 - ;; - esac -done - -have() { command -v "$1" >/dev/null 2>&1; } -have jq || { - printf 'discover-prs: jq required\n' >&2 - exit 5 -} - -# --- Resolve raw PR array (fixture file OR live fetch) ------------------------ - -RAW="" -if [[ -n "$PRS_JSON" ]]; then - if [[ ! -f "$PRS_JSON" ]]; then - printf 'discover-prs: --prs-json file not found: %s\n' "$PRS_JSON" >&2 - exit 1 - fi - RAW="$(cat "$PRS_JSON")" -else - have gh || { - printf 'discover-prs: gh CLI required\n' >&2 - exit 5 - } - # No --jq here: the filter runs below so the fixture path tests the same - # logic. Repo auto-resolved from the local git remote (no hardcode). - RAW="$(gh pr list --state open --limit 200 \ - --json number,title,headRefName,isDraft,author 2>/dev/null)" || { - printf 'discover-prs: gh pr list failed\n' >&2 - exit 2 - } -fi - -# --- Validate input is an array, then filter --------------------------------- - -if ! printf '%s' "$RAW" | jq -e 'type == "array"' >/dev/null 2>&1; then - printf 'discover-prs: malformed PR JSON (expected an array)\n' >&2 - exit 1 -fi - -# Drop drafts only, sort oldest-first (lowest number first / FIFO). Dependabot -# and every other author are in scope — babysit ALL open PRs so CI-failing -# dependency PRs get diagnosed + fixed too, not just auto-merged. Empty discovery -# yields `[]` and exits 0. Mirrors babysit.md §5.0.2 verbatim. -if ! printf '%s' "$RAW" | jq ' - [.[] - | select(.isDraft == false)] - | sort_by(.number) -'; then - printf 'discover-prs: jq failed filtering PR JSON\n' >&2 - exit 1 -fi - -exit 0 diff --git a/plugins/source-control/skills/pull-request/scripts/discover-prs.test.sh b/plugins/source-control/skills/pull-request/scripts/discover-prs.test.sh deleted file mode 100755 index d55a18c12..000000000 --- a/plugins/source-control/skills/pull-request/scripts/discover-prs.test.sh +++ /dev/null @@ -1,121 +0,0 @@ -#!/usr/bin/env bash -# Regression tests for discover-prs.sh. -# Black-box: feed fixture PR JSON via --prs-json (no network). Asserts the -# §5.0.2 filter contract — drop drafts, oldest-first. Dependabot (and every -# other author) is in scope; no author filter (babysit covers all PR authors). -set -uo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -DISCOVER="$SCRIPT_DIR/discover-prs.sh" -TEST_TMPDIR="$(mktemp -d)" -trap 'rm -rf "$TEST_TMPDIR"' EXIT - -FAILED=0 -CASE_NUM=0 -# shellcheck source=test-helpers.sh -source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/test-helpers.sh" - -# run_discover <fixture-json> — emits stdout then "EXIT:<code>". -run_discover() { - local fixture="$1" - local out - out=$(bash "$DISCOVER" --prs-json "$fixture" 2>/dev/null) - local code=$? - printf '%s\nEXIT:%s' "$out" "$code" -} - -# numbers_of <fixture-json> — output PR numbers as a comma-joined string. -numbers_of() { - local fixture="$1" - bash "$DISCOVER" --prs-json "$fixture" 2>/dev/null | jq -r '[.[].number] | join(",")' -} - -mkjson() { # mkjson <name> <jq-array-expr> - local f="$TEST_TMPDIR/$1.json" - jq -n "$2" >"$f" - printf '%s' "$f" -} - -# --- Case: --help exits 0, non-empty, describes the skill --- -help_out=$(bash "$DISCOVER" --help 2>&1) -assert_exit "--help exit 0" 0 "$?" -assert_contains "--help non-empty / describes discovery" "$help_out" "PR discovery" - -# --- Case: unknown flag --- -bash "$DISCOVER" --bogus >/dev/null 2>&1 -assert_exit "unknown flag exit 1" 1 "$?" - -# --- Case: unexpected positional argument --- -bash "$DISCOVER" 123 >/dev/null 2>&1 -assert_exit "positional arg exit 1" 1 "$?" - -# --- Case: missing fixture file --- -bash "$DISCOVER" --prs-json "$TEST_TMPDIR/nope.json" >/dev/null 2>&1 -assert_exit "missing fixture exit 1" 1 "$?" - -# --- Case: --prs-json with no argument must error, not hang --- -# Regression: `shift 2` with $#==1 under `set -uo pipefail` (no -e) left $# -# unchanged and looped forever (exit 124). The guard must exit 1 immediately. -# timeout bounds the regression so a reintroduced hang fails the suite rather -# than blocking it; degrade to skip on platforms lacking timeout (BSD macOS). -if command -v timeout >/dev/null 2>&1; then - timeout 5 bash "$DISCOVER" --prs-json >/dev/null 2>&1 - assert_exit "--prs-json without arg errors (no infinite loop)" 1 "$?" -else - skip_case "timeout unavailable — cannot bound --prs-json infinite-loop regression" -fi - -# --- Case: combined filter — drop draft only, oldest-first; Dependabot KEPT --- -# Out-of-order numbers, one draft, one Dependabot PR mixed in. The draft (#18) -# is dropped; the Dependabot PR (#5) is KEPT. Surviving set {5, 7, 12, 30} in -# ascending order proves drop-draft + keep-all-authors + oldest-first together. -F=$(mkjson combined '[ - {number:30, title:"c", headRefName:"feat/c", isDraft:false, author:{login:"alice"}}, - {number:7, title:"a", headRefName:"feat/a", isDraft:false, author:{login:"bob"}}, - {number:18, title:"draft", headRefName:"feat/d", isDraft:true, author:{login:"carol"}}, - {number:5, title:"dep", headRefName:"dependabot/nuget/x", isDraft:false, author:{login:"app/dependabot"}}, - {number:12, title:"b", headRefName:"feat/b", isDraft:false, author:{login:"alice"}} -]') -assert_eq "combined -> drop draft, keep dependabot, oldest-first" "5,7,12,30" "$(numbers_of "$F")" -r=$(run_discover "$F") -assert_contains "combined exit 0" "$r" "EXIT:0" - -# --- Case: Dependabot PR is KEPT; only the draft is dropped (codex r3327055703) - -# Babysit covers ALL PR authors incl Dependabot. The non-draft -# Dependabot PR (#3) survives; the draft (#2) is dropped. -F=$(mkjson depkept '[ - {number:2, title:"draft", headRefName:"feat/d", isDraft:true, author:{login:"alice"}}, - {number:3, title:"dep", headRefName:"dependabot/npm/y", isDraft:false, author:{login:"app/dependabot"}} -]') -assert_eq "dependabot kept, draft dropped" "3" "$(numbers_of "$F")" -r=$(run_discover "$F") -assert_contains "dependabot-kept exit 0" "$r" "EXIT:0" - -# --- Case: all-draft input -> empty list ([] from non-empty input) --- -F=$(mkjson alldraft '[ - {number:2, title:"draft", headRefName:"feat/d", isDraft:true, author:{login:"alice"}} -]') -assert_eq "all-draft -> empty number list" "" "$(numbers_of "$F")" -r=$(run_discover "$F") -assert_contains "all-draft -> []" "$r" "[]" -assert_contains "all-draft exit 0" "$r" "EXIT:0" - -# --- Case: empty discovery (zero open PRs) --- -F=$(mkjson empty '[]') -r=$(run_discover "$F") -assert_contains "empty -> []" "$r" "[]" -assert_contains "empty exit 0" "$r" "EXIT:0" - -# --- Case: null author kept (no author filter at all) --- -# Only drafts are dropped, so a PR with a null author survives regardless. -F=$(mkjson nullauthor '[ - {number:9, title:"ghost", headRefName:"feat/g", isDraft:false, author:null} -]') -assert_eq "null-author kept" "9" "$(numbers_of "$F")" - -# --- Case: malformed input (object, not array) --- -F=$(mkjson malformed '{number:1}') -bash "$DISCOVER" --prs-json "$F" >/dev/null 2>&1 -assert_exit "malformed input exit 1" 1 "$?" - -[[ $FAILED -eq 0 ]] || exit 1 diff --git a/plugins/source-control/skills/pull-request/scripts/fetch-annotations.test.sh b/plugins/source-control/skills/pull-request/scripts/fetch-annotations.test.sh index 7b15372dc..31fdb3e89 100755 --- a/plugins/source-control/skills/pull-request/scripts/fetch-annotations.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/fetch-annotations.test.sh @@ -20,8 +20,8 @@ SCRIPT="$SCRIPT_DIR/fetch-annotations.sh" TEST_TMPDIR="$(mktemp -d)" trap 'rm -rf "$TEST_TMPDIR"' EXIT -# shellcheck source=test-helpers.sh -source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/test-helpers.sh" +# shellcheck source=../../../scripts/test-helpers.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../scripts" && pwd)/test-helpers.sh" # Skip suite if jq missing — script depends on it. command -v jq >/dev/null 2>&1 || skip_suite "jq not installed" diff --git a/plugins/source-control/skills/pull-request/scripts/fetch-failed-logs.test.sh b/plugins/source-control/skills/pull-request/scripts/fetch-failed-logs.test.sh index 655a116f6..626c5a34b 100755 --- a/plugins/source-control/skills/pull-request/scripts/fetch-failed-logs.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/fetch-failed-logs.test.sh @@ -22,8 +22,8 @@ SCRIPT="$SCRIPT_DIR/fetch-failed-logs.sh" TEST_TMPDIR="$(mktemp -d)" trap 'rm -rf "$TEST_TMPDIR"' EXIT -# shellcheck source=test-helpers.sh -source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/test-helpers.sh" +# shellcheck source=../../../scripts/test-helpers.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../scripts" && pwd)/test-helpers.sh" # Skip suite if `unzip` (production dep) or `zip` (test fixture builder) is # missing. CI runners have them preinstalled. Windows Git Bash users install