fix(source-control): invoke babysit mutation wrappers by bin/ path, not bare name (#484)#840
Conversation
…ot bare name (#484) The babysit-prs worker/autopilot contract told workers to invoke the guarded mutation wrappers (source-control-babysit-merge, source-control-babysit-resolve-thread) by their bare command names, but those names are not on the Bash tool's PATH — every bare invocation failed command not found (exit 127), forcing workers to hand-roll raw gh api graphql resolveReviewThread calls and lose the wrapper's --allowed-owners guardrail and JSON action receipt. Invoke each wrapper as bash "${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>" — the same form the read-only sibling scripts under ${CLAUDE_PLUGIN_ROOT}/scripts/ already use — across SKILL.md, reference/orchestration.md (incl. the worker prompt template), and reference/safety.md. Refine safety.md's Guarded Mutation Wrappers posture accordingly: launching a wrapper by path runs the wrapper with every guard intact, so the only forbidden re-spelling is the raw Python behind them (which bypasses the guards) plus piping a wrapper into an interpreter. Add a one-line pointer in reference/review-discipline.md noting the babysit tiers resolve through the wrapper, while D7.5 keeps its general raw-GraphQL policy for /pull-request. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
… -> 0.15.9) A sibling source-control PR (the #475 readiness-gate fix) is open concurrently and already claims 0.15.8. Per the coordinate-and-serialize protocol for same-plugin PRs, move this PR one increment past it to 0.15.9 so the two never collide on version, and keep this PR do-not-merge until the sibling lands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 4m 56s —— View job PR Review: fix(source-control): invoke babysit mutation wrappers by bin/ path (#484)
Overall: The fix is correct, minimal, and well-reasoned. The CorrectnessCore fix is sound. Posture refinement in
IssuesModerate — SKILL.md autopilot step 2 uses bare-name shorthand before the convention is declared
The convention that covers this shorthand — "every command written as The worker contract in Minor — Pinned-Command Degradation handoffs use
|
|
Claude finished @kyle-sexton's task in 3m 50s —— View job Security Review
Scope: Documentation-only change ( No CRITICAL or IMPORTANT findings. Two SUGGESTION-level observations follow. Finding 1 — SUGGESTION · PLAUSIBLEAutopilot step 2 in
The risk is that an LLM agent following the numbered autopilot steps sequentially may not apply the general statement retroactively to override the bare-name form at step 2, reproducing the exact runtime failure ( The "single home" approach chosen here is a valid design tradeoff (avoids merge conflicts with concurrent PRs), but the asymmetry between step 2 (still bare-name) and step 3 (explicit safety.md reference) leaves a gap. Finding 2 — SUGGESTION · PLAUSIBLE
All updated commands now take the form: The PR itself confirms that The failure is safe (no such file in
This risk is theoretical given the naming convention, but a guard such as No findings related to:
|
…quality cap The guarded-mutations invocation correction pushed SKILL.md to 502 lines, over the skill-quality-gate hard cap of 500. Trim the note to defer the full invocation rationale to reference/safety.md (its single home), restoring 499. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 3m 48s —— View job PR Review: fix(source-control): invoke babysit mutation wrappers by bin/ path (#484)
Overall: The core fix is correct and safe. Both wrapper files are confirmed present in CorrectnessCore fix verified. Both All three executable invocation points in
IssuesModerate — SKILL.md autopilot step 2 still describes bare-name invocation without a path-form pointer
The convention that covers this — "every command written as By contrast, step 3 at line 155 — also bare-name — explicitly defers to Simplest fix: mirror step 3's treatment — add Minor — Pinned-Command Degradation handoffs require
|
|
Claude finished @kyle-sexton's task in 3m 53s —— View job Security Review — PR #840 (fix/source-control: invoke babysit mutation wrappers by bin/ path)
Scope: Documentation-only change ( IMPORTANT — Confirmed: Autopilot step 2 retains bare-name invocation before the bin/-path convention is declaredFile: Autopilot step 2 spells the resolve command as a bare name: The bin/-path convention covering this is declared at line 186, 40 lines later. An autopilot agent following the numbered steps (1 → 2 → 3) encounters the bare name at step 2 before it reaches the convention. When the bare invocation fails
This is the exact failure mode the PR is fixing in Confidence: Confirmed. Line 145 is unchanged (bare name), line 186 declares the convention after it, and the PR description itself describes the fallback path as a first-hand reproduction. Remediation: One of: (a) add SUGGESTION — Plausible:
|
|
Post-green review classification (work lane):
No inline review threads to resolve (all comments are PR-level). Holding |
…step 2 (#484) Autopilot step 2 spelled the resolve command as a bare wrapper name with args, 40 lines before the bin/-path invocation convention is declared. An autopilot agent reading steps top-down hit the bare name first and reproduced the exact command not found (exit 127) -> raw-GraphQL fallback this PR exists to prevent, bypassing --allowed-owners enforcement and the thread-pin pair rule. Make step 2 point at reference/safety.md for the exact command, symmetric with step 3's merge-gate treatment, so no runnable bare command precedes the convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
|
Claude finished @kyle-sexton's task in 2m 35s —— View job Security Review — PR #840 (HEAD: b34208c)
Scope: Documentation-only change ( Prior IMPORTANT finding: RESOLVED by latest commit (b34208c)The three prior automated security reviews each flagged — one at IMPORTANT/Confirmed severity — that The latest commit resolves this: step 2 now reads:
No runnable bare name precedes the convention. Step 2 and step 3 are now symmetric — both defer to No CRITICAL or IMPORTANT findingsWrapper guard verification (unchanged scripts): Both wrapper scripts self-locate via No shell injection surface introduced: The documented commands use Worker Prompt Template ( No GitHub Actions changes: No workflow files modified. No No credential/token handling changes: SUGGESTION (pre-existing, not a regression) —
|
|
Claude finished @kyle-sexton's task in 1m 31s —— View job PR Review: post-fix pass (commit
|
| Item | Status |
|---|---|
do-not-merge label |
Correctly held — serialized behind #839 which has not yet landed. Drop after #839 merges and version is confirmed still leading main. |
Pinned-Command Degradation handoffs embed ${CLAUDE_PLUGIN_ROOT} (operator shell may not have it set) |
Non-blocking residual, documented in CHANGELOG |
Worker Prompt Template command in orchestration.md in prose not a code block |
Pre-existing, not a regression |
Root cause — plugin bin/ not on Bash tool's PATH |
Upstream/harness gap, tracked as #843 |
No new concerns
The fix is strictly subtractive at the risk surface: it removes a runnable bare command and replaces it with a documentation pointer. No new execution path is introduced.
Summary: The IMPORTANT/Confirmed finding is resolved. The PR is clean and consistent. The do-not-merge hold should be dropped once sibling #839 lands and the version is reconfirmed.
|
Confirmed: commit `b34208c` resolves the IMPORTANT/DEFER-FORBIDDEN finding — post-fix review pass explicitly verifies step 2/step 3 symmetry is now complete and the bare-name invocation is gone. Line count unaffected (499/500, 3-for-3 line swap). Security-review re-run still in progress on this commit; will do a final check once it lands before considering this PR's review pass fully closed. `do-not-merge` remains correctly held pending #839. |
|
Post-fix review pass — confirmed closed. Both the re-run PR review and security-review on commit `b34208c` independently verify the IMPORTANT/DEFER-FORBIDDEN finding is resolved (step 2/step 3 now symmetric, no bare-name invocation precedes the convention, no new concerns introduced). No further findings. This PR's review pass is done. Only the intentional `do-not-merge` hold remains — pending sibling #839 (still open, not yet merged). Will rebase + re-bump past #839's version once it lands, then flip to ready. Handing to babysit lane for ongoing watch in the meantime. |
…de-ops, work-items Re-checked open PRs touching these plugins' plugin.json right before the version-bump collision protocol requires it: source-control carries #839 (0.15.8) and #840 (0.15.9), claude-ops carries #844 (0.17.2), work-items carries #826 (0.18.2) — all still open. Bumps this PR's claims one past each plugin's current highest open-PR claim (source-control 0.15.10, claude-ops 0.17.3, work-items 0.18.3) so this PR does not collide at merge time regardless of which sibling lands first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
Resolves conflicts on source-control and work-items plugin.json/CHANGELOG.md: main advanced source-control to 0.15.8 (#839, merged) and work-items to 0.18.2 (#826, merged) since this branch was last rebased. Also re-checked live open PRs at merge time and found work-items now carries a new open claim, #857 at 0.19.0 (Jira adapter) — re-bumped this branch's work-items claim from 0.18.3 to 0.19.1 to stay ahead of it. source-control's 0.15.10 and claude-ops's 0.17.3 remain valid (still one past #840/#860's 0.15.9 and #844/#860's 0.17.2 open claims, respectively). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
Integrate #839 (source-control 0.15.8, babysit-readiness-gate diagnostic). Resolved plugin.json to 0.15.9 (one past main) and stacked the 0.15.9 CHANGELOG entry above the merged-in 0.15.8 entry. SKILL.md auto-merged (distinct regions); babysit-prs skill-quality gate green at 499/500 lines.
|
Claude finished @kyle-sexton's task in 2m 46s —— View job PR Review: fix(source-control): invoke babysit mutation wrappers by bin/ path (#484)
Trigger: PR synchronize — reviewing HEAD Verdict: Clean pass. The What changed in
|
| Finding | Prior classification | Status |
|---|---|---|
SKILL.md step 2 bare-name before convention (line 146 vs line 186) |
IMPORTANT / Confirmed (3× reviews) | Resolved in commit b34208c: step 2 now defers to safety.md for the exact command, symmetric with step 3. Verified at HEAD. |
CLAUDE_PLUGIN_ROOT empty-string → /bin/<wrapper> in degradation handoffs |
SUGGESTION / Plausible | Pre-existing residual; documented in CHANGELOG. Not blocking. |
| Worker Prompt Template command in prose, not a code block | Observation | Pre-existing; not a regression. Not blocking. |
Correctness at HEAD
SKILL.md step 2/3 symmetry (lines 145–159): both steps now defer to safety.md for the exact command. No runnable bare name appears before the convention declaration.
safety.md convention coverage (lines 123–136): declares the bin/-path form above all inline bare-name shorthand references in the file, so the "every command spelled below as source-control-babysit-<x> …" convention covers them correctly in reading order.
orchestration.md invocation points (line 36, line 510, line 576): all three runnable invocation points use bash "${CLAUDE_PLUGIN_ROOT}/bin/...". Confirmed at HEAD.
SKILL.md line count: 499 (under the 500-line cap). ✓
review-discipline.md D7.5 (lines 202–206): the parenthetical correctly notes babysit tiers route through the guarded wrapper without altering the D7.5 raw-GraphQL policy for /pull-request. Scoping is accurate.
Security
No security concerns. The path-form change is confirmed not a guard-bypass re-spelling: the wrapper still self-locates via readlink -f "$0" and every guard (fail-closed without --allowed-owners, rejection of --allow-unpinned-head, thread-pin pair rule) is intact. The narrowed prohibition in safety.md (raw Python invocation and interpreter-piping, not bash "...") correctly captures only the forms that dodge the guards.
Summary
The merge with main is clean, the version serialization condition is satisfied (0.15.9 leads main's 0.15.8, #839 has landed), all prior IMPORTANT findings are resolved, and no new concerns are introduced. Ready to drop do-not-merge and proceed to merge.
|
Claude finished @kyle-sexton's task in 1m 56s —— View job Security Review — PR #840 (HEAD: dc6c1de)
Scope: Documentation-only change ( Prior IMPORTANT finding: CONFIRMED RESOLVEDThe four prior automated security reviews each flagged — one at IMPORTANT/Confirmed severity — that
No runnable bare name precedes the convention declaration. Steps 2 and 3 are now symmetric — both defer to No CRITICAL or IMPORTANT findingsWrapper guards are intact. Both Path-form invocation is not a guard bypass. The merge wrapper's No shell injection surface introduced. Arguments to the wrapper arrive as No GitHub Actions changes. No workflow files modified. No credential or token handling changes. Permission effect is a net narrowing. The path form does not match the pre-approved SUGGESTION (pre-existing, not a regression) —
|
|
Claude finished @kyle-sexton's task in 1m 29s —— View job PR Review: fix(source-control): invoke babysit mutation wrappers by bin/ path (#484)
Trigger: PR marked ready_for_review — final review pass at HEAD Verdict: Clean. Ready to merge. What was verifiedCore fix — All three runnable invocation points in
Prior IMPORTANT finding — resolved.
Version & CHANGELOG — SecurityNo concerns. The path-form invocation is confirmed not a guard bypass: the merge wrapper still self-locates via Remaining non-blocking items (unchanged from prior reviews)
Summary: All five prior automated review passes (2× PR review, 2× security review, 1× final-pass re-run) are consistent. The core fix is sound, the IMPORTANT/Confirmed finding is resolved, and the serialization condition is met. This PR is clean. |
|
Claude finished @kyle-sexton's task in 1m 47s —— View job Security Review — PR #840 (HEAD: dc6c1de)
Scope: Documentation-only change ( No CRITICAL or IMPORTANT findingsPrior IMPORTANT finding: CONFIRMED RESOLVEDAll prior automated security reviews flagged — one at IMPORTANT/Confirmed severity — that Commit
No runnable bare name precedes the convention. Steps 2 and 3 are now symmetric — both defer to Path-form invocation: not a guard bypassBoth No shell injection surface introducedAll updated invocations in No other security surfaces touched
SUGGESTION (pre-existing, not a regression) —
|
Re-syncs onto main after #860 (fleet-wide `shell: bash` declaration sweep) merged, which bumped source-control to 0.15.9, claude-ops to 0.17.2, and repo-hygiene to 0.4.5. Resolved conflicts on those three plugins' plugin.json and CHANGELOG.md; re-derived each held version against the new main plus a fresh live-open-PR check rather than assuming the prior numbers still held: - source-control 0.15.10: still one past main's 0.15.9 and #840's live 0.15.9 claim. - claude-ops 0.17.3: still one past main's 0.17.2 and #844's live 0.17.2 claim. - repo-hygiene 0.4.6: still one past main's new 0.4.5 (#860, now merged); no open-PR collision remains for this plugin. - work-items 0.19.1 and guardrails 0.9.6 were untouched by this merge (no conflict) and remain correct against #857/#861 (both still 0.19.0, open) and no open collision, respectively. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
Integrate #860 (fleet-wide `shell: bash` skill-frontmatter chore, which merged source-control to 0.15.9). Re-derived this PR's bump to 0.15.10 (one past current main) and re-labeled the 0.15.9 CHANGELOG entry to 0.15.10, stacked above the merged-in 0.15.9 (shell: bash) entry. SKILL.md auto-merged (my body edits + #860's frontmatter, distinct regions); babysit-prs skill-quality gate green at 499/500 lines.
|
Claude finished @kyle-sexton's task in 1m 54s —— View job PR Review: fix(source-control): invoke babysit mutation wrappers by bin/ path (#484)
Trigger: PR synchronize — reviewing HEAD Verdict: Clean pass. No regressions. What came in via
|
| Commit | Description | Source-control plugin impact |
|---|---|---|
7688ae7 |
chore: declare shell: bash on every skill with dynamic-context injections |
Adds shell: bash to babysit-prs/SKILL.md frontmatter; bumps version 0.15.9 → 0.15.10; adds ## [0.15.9] CHANGELOG entry |
560d7ac |
docs(topics): boris-video-absorption Phase 8 flip | None |
21b1b86 |
feat(naming): retro tournament lessons into name-it-better | None |
The only material change to the source-control plugin is the shell: bash addition, which is a safe cross-cutting change from main with no interaction with this PR's fix.
Version & CHANGELOG — serialization re-satisfied
origin/main is now at 0.15.9 (for the shell: bash chore). This branch is correctly at 0.15.10. CHANGELOG ordering is:
## [0.15.10] ← this PR's fix (babysit bin/-path invocation)
## [0.15.9] ← shell: bash on all skills (came from main)
## [0.15.8] ← #839 babysit-readiness-gate diagnostic
The version leads main (0.15.10 > 0.15.9). The serialization condition from the prior hold is re-satisfied after this merge.
Core fix — no regressions
All five prior review passes verified the core fix; confirmed again at this HEAD:
SKILL.md step 2/3 symmetry — both steps defer to safety.md for the exact command. No runnable bare name appears before the convention declaration. The command not found → raw-GraphQL fallback path remains closed. ✓
orchestration.md invocation points — all three use the bash "${CLAUDE_PLUGIN_ROOT}/bin/..." path form:
- Fan-out gate (line 36) ✓
- Worker Contract auto-resolve (line 510) ✓
- Worker Prompt Template (line 576) ✓
safety.md posture — bin/-path convention declared above all inline bare-name shorthand references; prohibition correctly narrowed to raw-Python invocation and interpreter-piping only. ✓
review-discipline.md D7.5 — parenthetical intact; D7.5 raw-GraphQL policy for /pull-request unchanged. ✓
Security
No new concerns. The shell: bash frontmatter addition is inert from a security standpoint — it constrains the precompute host, it does not widen any execution surface. All wrapper guards confirmed intact (unchanged scripts throughout).
Remaining non-blocking items (unchanged from prior reviews)
| Item | Status |
|---|---|
CLAUDE_PLUGIN_ROOT empty-string expands degradation-handoff commands to /bin/<wrapper> (safe failure, misleading error) |
Pre-existing residual; documented in CHANGELOG. |
Worker Prompt Template command in orchestration.md in prose, not a fenced block |
Pre-existing; not a regression. |
| Bare-name allow-rule mismatch (per-call prompts now replace broad allow-rule match) | Documented in CHANGELOG. Resolves when upstream PATH gap is fixed (#843). |
Summary: The merge with origin/main is clean. The version correctly re-serializes to 0.15.10 (leading main's 0.15.9). The core fix is intact with no regressions across all six changed files. All prior IMPORTANT findings remain resolved. This PR is ready to merge once the do-not-merge label is dropped.
|
Claude finished @kyle-sexton's task in 2m 14s —— View job Security Review — PR #840 (HEAD: 93a0b82)
Scope: Documentation-only change ( No CRITICAL or IMPORTANT findingsPrior IMPORTANT finding: CONFIRMED RESOLVEDAll prior automated security reviews flagged — one at IMPORTANT/Confirmed severity — that
No runnable bare name precedes the convention declaration. Steps 2 and 3 are symmetric. Path-form invocation is not a guard bypassBoth Worker Prompt Template injection fence intactThe Permission effect is a net tighteningThe No GitHub Actions, credential-handling, or permission-widening changesNo workflow files modified. No changes to Merge commit (93a0b82) is cleanThe latest commit brings in SUGGESTION (pre-existing, not a regression) —
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93a0b82cf2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
#840 rebased since this branch's last check and re-derived its own claim from 0.15.9 to 0.15.10, so this PR's prior 0.15.10 now collides exactly. Bump to 0.15.11 (one past #840's current live claim). claude-ops (0.17.3), work-items (0.19.1), repo-hygiene (0.4.6), and guardrails (0.9.6) were independently re-verified against current main and all live open PRs and remain correct — no change needed for those four. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
#857 (Jira Cloud adapter) merged into main since this branch's last rebase, bumping work-items to 0.19.0. Resolved the resulting conflict, keeping this branch's 0.19.1 (already one past both the new main and #861's still-open 0.19.0 claim, so no further bump needed). Re-verified all 5 plugins fresh against current main and every live open PR (#840 0.15.10, #844 0.17.2, #861 0.19.0): source-control 0.15.11, claude-ops 0.17.3, repo-hygiene 0.4.6, and guardrails 0.9.6 all remain correct with no change required. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
An unrelated claude-ops docs PR (#877, closing #864) merged since this branch's last rebase and landed at exactly 0.17.3 — the same version this branch was claiming. Re-bumped claude-ops to 0.17.4 to clear the exact collision (plugin.json auto-merged to an identical-looking "0.17.3" string without flagging a conflict, since both sides happened to write the same literal text for unrelated reasons; caught by diffing main's version history directly, not by trusting a clean auto-merge). CHANGELOG.md split into two sections (0.17.3 for #877's actual change, 0.17.4 for this PR's). Also swept for new open PRs against all 5 plugins post-merge: #870 (feat/800-audit-instructions) newly touches repo-hygiene (claims 0.4.5) and claude-ops (claims 0.17.2) — both below this branch's current claims (0.4.6 / 0.17.4), no collision. source-control (0.15.11), work-items (0.19.1), and guardrails (0.9.6) re-confirmed against current main and every live open PR (#840 0.15.10, #844 0.17.2, #861 0.19.0) — unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
claude-ops: #844 rebased and re-derived its claim from 0.17.2 to 0.17.4, colliding exactly with this branch's prior 0.17.4. Bump to 0.17.5. source-control: a new PR, #882 (fix/511-babysit-self-identity-decouple), claims 0.16.0 — a minor bump above this branch's prior 0.15.11 and above #840's 0.15.10. Bump to 0.16.1 to stay ahead of both. repo-hygiene (0.4.6), guardrails (0.9.6), and work-items (0.19.1) were re-verified fresh against current main and every live open PR (main: repo-hygiene 0.4.5, guardrails 0.9.5, work-items 0.19.0; #861 0.19.0) and remain correct, no change needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
#844 (claude-ops) merged into main at exactly 0.17.4, which this branch's prior 0.17.5 was already one past — kept as-is, no re-bump needed for the number itself, just resolved the resulting plugin.json/CHANGELOG.md conflict. Full fresh collision sweep after the merge found claude-ops now has NO open-PR collision at all. It also found #861 (work-items) re-derived its own claim from 0.19.0 to 0.20.0 since the last check (following #857's Jira-adapter minor bump into main) — colliding with this branch's prior 0.19.1. Re-bumped work-items to 0.20.1. source-control (0.16.1, held behind #882's 0.16.0 and #840's 0.15.10), repo-hygiene (0.4.6, no collision), and guardrails (0.9.6, no collision) re-verified against current main and all live open PRs — unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
…lve scope The parenthetical added to D7.5 claimed the bot-only, never-human author-conditional rule applied as the babysit "tier discipline" for both the worker and autopilot tiers. The autopilot tier deliberately resolves addressed human threads (`--resolve --include-human`; SKILL.md, safety.md), so that blanket claim contradicts the autopilot contract — a worker taking it literally would leave addressed human threads unresolved and keep the PR blocked on conversation resolution. Keep the wrapper-vs-raw-GraphQL pointer, drop the over-generalization, and state each tier's own resolve author-scope: worker matches this bot-only rule; autopilot extends to addressed human threads, per that skill's reference/safety.md. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 0s —— View job PR Review: fix(source-control): invoke babysit mutation wrappers by bin/ path (#484)
Trigger: PR synchronize — reviewing HEAD Verdict: Clean. No new concerns. This PR is ready to merge. What changed in
|
| Finding | Status |
|---|---|
| SKILL.md step 2 bare-name before convention (IMPORTANT, Confirmed — 3× reviews) | Resolved — commit b34208c. Verified at HEAD. |
| D7.5 parenthetical over-generalizes bot-only rule to autopilot tier (Codex P2) | Resolved — commit 716915c. Verified at HEAD. |
CLAUDE_PLUGIN_ROOT empty-string in degradation-handoff commands (SUGGESTION) |
Pre-existing residual; documented in CHANGELOG. Not blocking. |
Worker Prompt Template command in orchestration.md in prose, not a fenced block |
Pre-existing; not a regression. Not blocking. |
| Bare-name allow-rule mismatch (per-call prompts replace broad allow-rule match) | Documented in CHANGELOG. Resolves with upstream PATH gap fix (#843). |
Security
No security concerns. The D7.5 correction is documentation-only; no execution surface changed. All wrapper guards confirmed intact through prior passes (unchanged scripts). No GitHub Actions, credential handling, or permission-widening changes.
Version & CHANGELOG
Plugin at 0.15.10, correctly leading origin/main's 0.15.9. The 0.15.10 CHANGELOG entry at CHANGELOG.md lines 7–30 accurately describes both the primary fix and its known residuals. The D7.5 per-tier correction is an intra-PR accuracy fix and does not require an additional version increment.
Summary: Both blocking findings (SKILL.md step 2 bare-name, Codex P2 D7.5 over-generalization) are resolved. No new concerns introduced by 716915c. This PR is clean and ready to merge.
| Branch
|
Claude finished @kyle-sexton's task in 2m 21s —— View job Security Review — PR #840 (HEAD: 716915c)
Scope: Documentation-only change ( New commit:
|
#882 (source-control) merged into main at 0.16.0, which this branch's prior 0.16.1 was already one past — resolved the resulting plugin.json/CHANGELOG.md conflict, kept as-is. Full fresh collision sweep after the merge found source-control now carries TWO additional open PRs beyond #840: #895 (fix/548-babysit-worktree-head-safety, claims 0.15.10) and #898 (feat/399-shared-worktree-helper, claims 0.17.0 — a minor bump above this branch's prior 0.16.1). Re-bumped source-control to 0.17.1 to stay ahead of all three (#898's 0.17.0, #895's and #840's 0.15.10). claude-ops (0.17.5, no collision), work-items (0.20.1, held behind #861's 0.20.0), repo-hygiene (0.4.6, no collision), and guardrails (0.9.6, no collision) re-verified against current main and all live open PRs — unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1V3gkrfSf75isB8MiDy3o
Integrate the latest main (source-control advanced to 0.16.0 via #511's pr_queue_snapshot self-identity work, plus #882). Re-derived this PR's bump to 0.16.1 (one past current main) and re-labeled the CHANGELOG entry from 0.15.10 to 0.16.1, stacked above the merged-in 0.16.0 entry. safety.md, orchestration.md, review-discipline.md, and SKILL.md auto-merged (my bin/-path edits and the upstream changes touch distinct regions); babysit-prs skill-quality gate green at 499/500 lines.
…ath' into fix/484-babysit-resolve-thread-path
|
Claude finished @kyle-sexton's task in 2m 4s —— View job PR Review: fix(source-control): invoke babysit mutation wrappers by bin/ path (#484)
Trigger: PR synchronize — reviewing HEAD Verdict: Clean pass. No regressions. What came in via
|
| Commit | Description | Source-control plugin impact |
|---|---|---|
51770af |
Merge origin/main (source-control advanced to 0.16.0 via #511 pr_queue_snapshot self-identity work) |
Re-derives this PR's bump to 0.16.1 (one past current main); CHANGELOG re-labeled from 0.15.10 → 0.16.1 |
78fb5e2 |
Re-merge remote branch into local (picking up 716915c D7.5 fix) |
review-discipline.md — the per-tier scope correction (+4/-3 lines) |
The ce1c50c test commit from main (test(claude-config): add step-list-culling eval case) arrived in 51770af with no source-control plugin impact.
Version & CHANGELOG — serialization satisfied
origin/main is now at 0.16.0. This branch is at 0.16.1. The CHANGELOG orders ## [0.16.1] above ## [0.16.0]. The version leads main. ✓
Core fix — no regressions
SKILL.md step 2/3 symmetry (lines 147–163) — both steps defer to safety.md for the exact command. No runnable bare name appears before the convention declaration. The command not found → raw-GraphQL fallback path (bypassing --allowed-owners and the thread-pin TOCTOU rule) remains closed. ✓
orchestration.md invocation points — all three runnable references use the bash "${CLAUDE_PLUGIN_ROOT}/bin/..." path form:
- Fan-out gate (line 36) ✓
- Worker Contract auto-resolve (line 510) ✓
- Worker Prompt Template (line 576) ✓
safety.md posture — bin/-path convention declared above all inline bare-name shorthand references; prohibition correctly narrowed to raw-Python invocation and interpreter-piping only. ✓
review-discipline.md D7.5 (lines 202–211) — the parenthetical correctly states each tier's own resolve author-scope (worker = bot-only matching D7.5; autopilot = deliberately extends to addressed human threads via --include-human), pointing at safety.md as authority. The Codex P2 over-generalization is resolved. ✓
SKILL.md line count: 499/500 (under the quality cap). ✓
Security
No concerns. Both wrapper scripts (bin/source-control-babysit-merge, bin/source-control-babysit-resolve-thread) are confirmed unchanged throughout this PR. All guards survive the path-form invocation — the merge wrapper still self-locates via readlink -f "$0", still rejects --allow-unpinned-head (exit 2), and both wrappers still fail closed without --allowed-owners (exit 3).
Remaining non-blocking items (unchanged from prior reviews)
| Item | Status |
|---|---|
CLAUDE_PLUGIN_ROOT empty-string expands degradation-handoff commands to /bin/<wrapper> (safe failure, misleading error) |
Pre-existing residual; documented in CHANGELOG. Tracked as #843. |
Worker Prompt Template command in orchestration.md in prose, not a fenced block |
Pre-existing; not a regression. |
| Bare-name allow-rule mismatch (per-call prompts now replace broad allow-rule match) | Documented in CHANGELOG. Resolves when upstream PATH gap is fixed (#843). |
Summary: The re-merge is clean. Version correctly advances to 0.16.1 (leading main's 0.16.0). All prior IMPORTANT/Confirmed findings — the SKILL.md step 2 bare-name ordering issue and the Codex P2 D7.5 over-generalization — are resolved and unaffected by the merge. No new concerns. This PR is ready to merge.
| Branch
|
Claude finished @kyle-sexton's task in 1m 50s —— View job Security Review — PR #840 (HEAD: 78fb5e2)
Scope: Documentation-only change ( No CRITICAL or IMPORTANT findingsPrior IMPORTANT finding: CONFIRMED STILL RESOLVEDAll prior automated security reviews flagged — one at IMPORTANT/Confirmed severity — that
No runnable bare name precedes the convention declaration. Steps 2 and 3 are symmetric. What came in via 78fb5e2 — D7.5 per-tier scope correction is safeThe merge brought in Path-form invocation: not a guard bypassBoth Worker Prompt Template injection fence intactThe No shell injection surface introducedAll updated invocations use Permission effect is a net tighteningThe No GitHub Actions, credential-handling, or permission-widening changesNo workflow files modified. No changes to SUGGESTION (pre-existing, not a regression) —
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78fb5e2373
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…mutation section The Guarded mutations declaration prohibited the bare command name, yet the bullets immediately below spelled each wrapper bare, so a reader copying them hits the command-not-found path this change exists to close. State once that each `source-control-babysit-<x>` spelling in those bullets is the wrapper launched by its `bin/`-path form, mirroring the convention safety.md already declares over its own shorthand — no per-bullet command duplication.
|
Claude finished @kyle-sexton's task in 2m 34s —— View job PR Review: fix(source-control): invoke babysit mutation wrappers by bin/ path (#484)
Trigger: PR synchronize — reviewing HEAD Verdict: Clean. The Codex P2 finding is resolved. This PR is ready to merge. What changed in
|
| Finding | Prior classification | Status |
|---|---|---|
SKILL.md step 2 bare-name before convention (3× PR + security reviews, IMPORTANT/Confirmed) |
Blocking | Resolved in b34208c. Intact at HEAD. |
Codex P2 — SKILL.md guarded-mutations bullets use bare names before convention is declared |
Codex P2 | Resolved in 01c6fb2. ✓ |
| Codex P2 — D7.5 parenthetical over-generalizes bot-only rule to autopilot tier | Codex P2 | Resolved in 716915c. Intact at HEAD. |
CLAUDE_PLUGIN_ROOT empty-string expands degradation-handoff commands to /bin/<wrapper> |
SUGGESTION / pre-existing | Documented in CHANGELOG. Not blocking. |
Worker Prompt Template command in orchestration.md in prose, not a fenced block |
Observation / pre-existing | Not a regression. |
| Bare-name allow-rule mismatch (per-call prompts replace broad allow-rule match) | Documented in CHANGELOG | Resolves with upstream PATH gap fix (#843). |
Security
No concerns. The change is additive prose in the section preamble. The bullets themselves are unchanged; the wrappers are unchanged; every guard survives. No new execution surface introduced.
Version & CHANGELOG
plugin.json at 0.16.1, correctly leading origin/main's 0.16.0. The 0.16.1 CHANGELOG entry accurately describes both the primary fix and the known residuals. No version bump is needed for this intra-PR accuracy fix. ✓
Summary: The Codex P2 finding (guarded-mutations bullets using bare names before the convention is declared) is resolved by 01c6fb2 using the same section-level convention pattern already established in safety.md. All prior blocking and Codex findings are now resolved. The PR is clean and ready to merge.
| Branch
|
Claude finished @kyle-sexton's task in 2m 20s —— View job Security Review — PR #840 (HEAD: 01c6fb2)
Scope: Documentation-only change ( New commit:
|
Summary
The
source-control:babysit-prsworker/autopilot contract told workers to run the merge gate and resolve review threads by invoking the guarded mutation wrappers as bare command names (source-control-babysit-merge …,source-control-babysit-resolve-thread …). Those bare names are not on the Bash tool'sPATH, so every such invocation failedcommand not found(exit 127) — reproduced first-hand in this environment. Workers fell back to rawgh api graphql resolveReviewThreadcalls, losing the wrapper's--allowed-ownersguardrail and the JSONactionreceipt the contract tells them to parse, and re-implementing the safety logic ad hoc per worker.Fix
Invoke each wrapper by its bundled path —
bash "${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>" …— exactly the form the read-only sibling scripts under${CLAUDE_PLUGIN_ROOT}/scripts/(fetch-all-pr-comments.sh,babysit-readiness-gate.sh) already use. Updated surfaces:skills/babysit-prs/reference/safety.md— the single home for the exact commands. The prior Guarded Mutation Wrappers posture said the wrappers are invoked "only by their pinned bare wrapper names … never through an interpreter-prefixed path," which the bare-name PATH gap made unfollowable. It now prescribes thebin/-path form and narrows the prohibition to what actually protects the guards: raw-Python invocation (python … babysit_merge.py, which bypasses the wrapper's own guards such as the merge wrapper's--allow-unpinned-headrejection) and piping a wrapper into an interpreter. All fenced command blocks and the Pinned-Command Degradation operator handoffs use the path form.skills/babysit-prs/SKILL.md— corrected the "invoked ONLY by their bare wrapper names" statement; points atsafety.mdas the single home and states that everysource-control-babysit-<x> …command in this skill and its references is launched by thebin/-path form.skills/babysit-prs/reference/orchestration.md— the worker prompt template (the literal contract handed to a dispatched worker) and the file's two other runnable references now use the path form, so the file is self-consistent for a standalone worker.reference/review-discipline.md— a one-line pointer in D7.5 noting the babysit tiers resolve through the guarded wrapper (which adds--allowed-owners, bot-vs-human classification, and a JSON receipt). D7.5's general raw-GraphQL policy is unchanged — it stays the mechanism for/pull-request, which is not a babysit-tier consumer; repointing it at the babysit-tier wrapper would wrongly couple/pull-requestto a babysit tool.Why the path form and not shipping the bare name onto PATH: the wrappers are designed to be bare commands on PATH, and Claude Code's plugin reference documents a plugin's
bin/as added to the Bash tool's PATH "while the plugin is enabled." Empirically that is not happening in this environment — an upstream/harness matter this repo cannot fix. The${CLAUDE_PLUGIN_ROOT}/bin/-path form is the deterministic, repo-level invocation that works today and matches the established sibling-script convention.Verification
which source-control-babysit-resolve-thread→ not found (exit 1);$CLAUDE_PLUGIN_ROOTis empty in the shell; no pluginbin/onPATH.bash "…/bin/source-control-babysit-merge" owner/repo#1 --allow-unpinned-head→ still rejected, exit 2 (--allow-unpinned-head is not permitted through the wrapper).bash "…/bin/source-control-babysit-resolve-thread" owner/repo#1 --resolve(no--allowed-owners) → still fail-closed, exit 3, with the JSON refusal receipt.markdownlint-cli2clean (0 errors) on all four edited docs..md+plugin.jsonversion); no scripts or executables modified.Closes #484
Related
babysit-readiness-gate.showner/repo-derivation diagnostic), landing concurrently inplugins/source-control/. Per the coordinate-and-serialize protocol this PR is opened DRAFT +do-not-merge, with its version set one increment past fix(source-control): actionable owner/repo diagnostic on readiness-gate fetch failure #839 (0.15.9vs fix(source-control): actionable owner/repo diagnostic on readiness-gate fetch failure #839's0.15.8); once fix(source-control): actionable owner/repo diagnostic on readiness-gate fetch failure #839 lands, dropdo-not-mergeand confirm the version still leadsmain.source-control:babysit-prsissue (resolve-threadhumanThreadsActedmislabel), noted as concurrent source-control work.${CLAUDE_PLUGIN_ROOT}/bin/…form does not match a pre-approved bare-nameBash(source-control-babysit-resolve-thread:*)allow rule, so an operator's narrow allowlist entries no longer auto-approve these calls (per-call prompts return). The wrappers' own header comments and this repo'sbin/design still assume the bare-name form; restoring it is deferred to the root-cause fix below.bin/as on the Bash tool's PATH while enabled, but it is empirically absent in this environment. Only closing that upstream/harness gap makes the bare names — and the allow-rule match above — work as intended. (Not fixed here.)