feat(dev): babysit-prs — worker briefing + strict return contract - #14
Conversation
Give workers their own rulebook: the worker-side half of SKILL.md (axis ordering + routing, the confidence rule, all nine safety bullets) moves verbatim into references/worker-briefing.md, joined by an explicit worktree-hygiene section and the canonical RESULT/DETAIL return contract with one-line verdict definitions. Workers now read the briefing instead of the whole SKILL.md, so they never load the orchestrator half. The worker prompt keeps only the two belt-and-braces rails (head-branch-only pushes with the lease carve-out, worktree-scoped git clean) plus the literal RESULT/DETAIL block; the inline no-Agent fallback and both procedure preambles point at the briefing too. SKILL.md's bottom half becomes a short stub. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe babysit-prs skill now uses ChangesBabysit-prs worker workflow
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/dev/skills/babysit-prs/references/worker-briefing.md`:
- Around line 88-92: Update the repository-level Git scoping guidance in the
briefing and referenced ci-procedure.md commands: explicitly allow setup and
removal to run before the worktree exists, using git -C "$REPO_ROOT" for
repository-level fetch, worktree add, and worktree remove operations, and
require git -C "$WORKTREE_DIR" for commands executed inside the created
worktree.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d0e6fa7-3bf4-4cbe-91e0-a7bd6aceffe8
📒 Files selected for processing (4)
plugins/dev/skills/babysit-prs/SKILL.mdplugins/dev/skills/babysit-prs/references/ci-procedure.mdplugins/dev/skills/babysit-prs/references/conflict-procedure.mdplugins/dev/skills/babysit-prs/references/worker-briefing.md
Address CodeRabbit review on PR #14: the briefing demanded git -C "$WORKTREE_DIR" on every command, but fetch / worktree add / worktree remove necessarily run before the worktree exists (or after it's gone). State the two-scope rule explicitly in the briefing and scope those lifecycle commands with git -C "$REPO_ROOT" in both procedures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the babysit-prs skill documentation so worker subagents have a standalone, canonical “worker rulebook” (references/worker-briefing.md) and no longer need to load the orchestrator half of SKILL.md. It also updates the per-axis procedure docs to point at that briefing and trims the worker-side content out of SKILL.md.
Changes:
- Added
references/worker-briefing.mdcontaining worker scope, axis routing, confidence rule, safety rails, worktree hygiene, and the strict return contract. - Updated the worker prompt template and reference tables in
SKILL.mdto point workers to the briefing (and reduced the inlined worker-side text to a stub). - Updated
conflict-procedure.mdandci-procedure.mdpreambles and lifecycle commands to route through the briefing and use repo-root scopedgit -Cfor lifecycle operations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| plugins/dev/skills/babysit-prs/SKILL.md | Points workers at the new briefing, updates reference checks/table, and removes the embedded worker-side rulebook content. |
| plugins/dev/skills/babysit-prs/references/worker-briefing.md | New canonical worker rulebook (routing + safety + hygiene + return contract). |
| plugins/dev/skills/babysit-prs/references/conflict-procedure.md | Repoints prerequisites to the briefing and scopes lifecycle git commands to repo root. |
| plugins/dev/skills/babysit-prs/references/ci-procedure.md | Repoints prerequisites to the briefing and scopes lifecycle git commands to repo root. |
Suppressed comments (1)
plugins/dev/skills/babysit-prs/references/ci-procedure.md:99
- Same as step 4: this HEAD branch lookup should be repo-scoped to avoid
ghrunning against the wrong repo when not executed from inside a checkout. Usegh -R <OWNER_REPO>(the prompt provides it).
REPO_ROOT="<your repo root>"
WORKTREE_DIR="<your assigned CI worktree dir>"
HEAD_BRANCH=$(gh pr view <pr_number> --json headRefName --jq .headRefName)
git -C "$WORKTREE_DIR" add -A # not `commit -am`: that skips files the fix added
git -C "$WORKTREE_DIR" commit -m "fix: <what you fixed>"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rktree Copilot review on PR #14: worker blocks run in fresh shells, so a bare gh infers the repo from cwd and the safety-rail examples showed an unscoped git push. Scope every worker-side gh call with -R <owner_repo> (or gh repo view's positional arg), scope the example pushes with git -C "<worktree dir>", and state the gh scoping rule in the briefing's hygiene section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/dev/skills/babysit-prs/references/worker-briefing.md (2)
122-130: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake mixed-axis verdicts mutually exclusive.
A pass that fixes conflicts and re-queues CI matches both
PARTIALandDEFERRED: one axis was pushed, and another axis is waiting. A pass that fixes one axis but finds a human decision also overlapsPARTIALandNEEDS-HUMAN.Define precedence or rewrite the definitions so each final result has one valid verdict.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/dev/skills/babysit-prs/references/worker-briefing.md` around lines 122 - 130, The verdict definitions in the “One verdict word, defined” section overlap for mixed-axis outcomes. Rewrite them or add explicit precedence so each pass maps to exactly one verdict, including combinations of fixed work with deferred checks or human decisions, while preserving the required one-line explanation for PARTIAL and NEEDS-HUMAN.
66-72: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCorrect the
--force-with-leasesafety statement.
--force-with-lease="$HEAD_BRANCH:$SAVED_SHA"is force-capable. If the remote branch still equalsSAVED_SHA, it can accept a non-fast-forward update and rewrite history. The lease protects against concurrent updates but does not enforce fast-forward pushes. Use a normal push when history must remain immutable. The conflict procedure's ancestry guard must provide the separate fast-forward protection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/dev/skills/babysit-prs/references/worker-briefing.md` around lines 66 - 72, Correct the force-push safety guidance in the worker briefing: state that conflict procedure’s --force-with-lease="$HEAD_BRANCH:$SAVED_SHA" can rewrite history when the lease matches, so it only protects against concurrent updates. Instruct users to use a normal push when history must remain immutable, while relying on the conflict procedure’s ancestry guard for fast-forward protection; keep bare --force and bare --force-with-lease forbidden.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/dev/skills/babysit-prs/references/ci-procedure.md`:
- Around line 90-103: Add fail-fast shell options (`set -euo pipefail`) at the
start of the “Verify, push, clean up” command block so failures from git commit,
show, or push stop execution before cleanup. Keep the existing non-zero handling
for gh pr checks confined to its separate query block.
---
Outside diff comments:
In `@plugins/dev/skills/babysit-prs/references/worker-briefing.md`:
- Around line 122-130: The verdict definitions in the “One verdict word,
defined” section overlap for mixed-axis outcomes. Rewrite them or add explicit
precedence so each pass maps to exactly one verdict, including combinations of
fixed work with deferred checks or human decisions, while preserving the
required one-line explanation for PARTIAL and NEEDS-HUMAN.
- Around line 66-72: Correct the force-push safety guidance in the worker
briefing: state that conflict procedure’s
--force-with-lease="$HEAD_BRANCH:$SAVED_SHA" can rewrite history when the lease
matches, so it only protects against concurrent updates. Instruct users to use a
normal push when history must remain immutable, while relying on the conflict
procedure’s ancestry guard for fast-forward protection; keep bare --force and
bare --force-with-lease forbidden.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c26db0d-04f3-41c5-bf9f-1176854446f5
📒 Files selected for processing (4)
plugins/dev/skills/babysit-prs/SKILL.mdplugins/dev/skills/babysit-prs/references/ci-procedure.mdplugins/dev/skills/babysit-prs/references/conflict-procedure.mdplugins/dev/skills/babysit-prs/references/worker-briefing.md
… failed fix A failed commit or push previously fell through to clean -fd / worktree remove, deleting the local fix and letting the worker report a push that never happened. Add set -euo pipefail to the block and spell out the failure-path reporting rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/dev/skills/babysit-prs/references/ci-procedure.md`:
- Around line 107-114: Update the commit/push/cleanup procedure so it records
push success immediately after a successful push, then reports cleanup failures
separately without retracting the pushed status. Keep “not pushed” only for
failures in the commit or push commands, while preserving fail-fast behavior and
including command output for whichever operation fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8e7320f-9327-4b2f-bfba-65bd35303a57
📒 Files selected for processing (1)
plugins/dev/skills/babysit-prs/references/ci-procedure.md
A failure after the push line (clean/worktree remove) leaves the fix on the remote; suppress "pushed" only when commit or push itself failed. Resolves a coderabbitai comment on PR #14. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
plugins/dev/skills/babysit-prs/references/worker-briefing.md:110
- The briefing says workers must remove every worktree even on failure, but the CI procedure’s step 5 explicitly keeps the worktree intact on mid-block failure (to avoid discarding unpushed changes). These two instructions conflict; the briefing should allow the explicit “leave intact to preserve state” exception and require reporting the leftover worktree so it can be cleaned up.
**Remove every worktree you created before finishing, even when you failed.** A
leftover worktree pins its branch checkout and blocks the next pass's
`worktree add` for this PR.
plugins/dev/skills/babysit-prs/SKILL.md:296
- The worker prompt template’s safety-rail bullet has a line-break that leaves a dangling “Never” at the end of a line, which reads like an incomplete sentence and can be confusing in a safety-critical instruction block.
then `git -C "<worktree dir>" push origin "HEAD:$HEAD_BRANCH"` — that head
branch, nothing else. Never
paste the branch name itself into a command: Git allows `$(…)`, backticks and
quotes in ref names, and double quotes do not stop the shell from evaluating
them. The one sanctioned lease is the conflict procedure's
…, line wrap Address the remaining PR #14 review findings: - worker-briefing: the sanctioned --force-with-lease is a lease-guarded force update, not a push that can't rewrite history — the ancestry guard, not the flag, keeps it a fast-forward (matches SKILL.md and conflict-procedure.md) - worker-briefing: worktree-removal rule now carries the CI procedure's leave-intact-on-failed-push exception, with the leftover reported - worker-briefing: verdict words get explicit precedence (FAILED > NEEDS-HUMAN > PARTIAL > DEFERRED > FIXED) so mixed-axis passes map to one verdict - SKILL.md: rewrap the dangling 'Never' in the safety-rail bullet Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the four remaining review findings in 74167b6:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/dev/skills/babysit-prs/references/worker-briefing.md`:
- Around line 69-76: Update the push procedure in the worker briefing to check
at execution time whether HEAD_BRANCH equals DEFAULT_BRANCH and stop before any
push when they match. Keep fork-backed PR skipping unchanged, and apply this
guard specifically to same-repository PRs so a default-branch head cannot be
pushed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a240c105-64c8-476e-b3b8-22728dcb84b8
📒 Files selected for processing (3)
plugins/dev/skills/babysit-prs/SKILL.mdplugins/dev/skills/babysit-prs/references/ci-procedure.mdplugins/dev/skills/babysit-prs/references/worker-briefing.md
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/dev/skills/babysit-prs/references/ci-procedure.md
A same-repo PR can use the default branch as its head (e.g. a back-merge into a release branch), so "never push to the default branch" needs an execution-time check, not just a rule. Both procedures' push blocks now resolve DEFAULT_BRANCH alongside HEAD_BRANCH and stop before pushing when they match; the briefing and the worker prompt's restated rail carry the rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
plugins/dev/skills/babysit-prs/references/ci-procedure.md:100
- In the push block, the default-branch guard is implemented as a bare test (
[ "$HEAD_BRANCH" != "$DEFAULT_BRANCH" ]). When it fails underset -e, it exits with no explanatory output, which makes it harder for a worker to understand why the block stopped and what to report. Prefer an explicit check that prints a STOP message and exits non-zero.
[ "$HEAD_BRANCH" != "$DEFAULT_BRANCH" ] # never push to the default branch — a matching head stops the block here
Item 3 of the babysit-prs improvement plan (follows #10–#12): give workers their own canonical rulebook so they never load the orchestrator half of SKILL.md.
What changed
references/worker-briefing.md— the worker-side half of SKILL.md moved verbatim (axis ordering + procedure routing, the confidence rule, all nine safety bullets), plus two new sections made explicit:git -C "<worktree dir>"for everything; remove every worktree you created before finishing, even on failure.RESULT #<pr>: …/DETAIL: …block, with a one-line definition of each verdict word (FIXED / NEEDS-HUMAN / PARTIAL / DEFERRED / FAILED).git clean), keeping the literal RESULT/DETAIL block as a self-contained fallback; the inline no-Agent path reads the briefing per PR; the moved bottom half becomes a short stub. Step 1's existence check and reference table gain the briefing. 414 → 352 lines.conflict-procedure.mdandci-procedure.mdnow say "read the worker briefing first" instead of pointing back into SKILL.md.Verification
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit