feat(source-control): converge babysit-prs tiers + engine (0.8.0) - #322
Conversation
Fill PLAN.md Phase 2 (PR-B capability convergence): engine module boundaries (7 modules + lease library + thin CLIs), userConfig contract with key-to-flag delivery matrix, tier semantics, lane structure (E engine / D surfaces / C integration) with scope fences. Fold both review passes: fresh-context plan reviewer (22 findings — lease --repo pairing, delivery-matrix gaps, draft/cadence contradictions) and formal devils-advocate (16 findings — guard-CLI allowlist fail-closed flags, tier-keyword requirement, unprotected-repo merge refusal, scope-aware staleness guard, schema_version, corrupt- state quarantine, prompt-injection surface). Design record at design/phase-2-engine-modules.md. Phase 1 tag flipped to DONE (PR #300 merged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Decompose pr_queue_snapshot's mega-module into flat stdlib siblings: babysit_util (single subprocess core + SHA-pin constant), babysit_gh (single parameterized discovery + single reviewThreads paginator), babysit_state (flag-only state dir, scope-aware staleness guard, scoped-run clobber fix, schema_version, corrupt-state quarantine), babysit_lease (--repo scoping + snapshot pairing validation), babysit_checks ((type,name,workflow) identity keying), babysit_feedback (structural bot typing, config-fed logins, dependency-author detection), babysit_review_trigger (config-driven, dormant when unconfigured), babysit_delta (classify + 12 delta arms + L3 foreign-activity). Thin pr_queue_snapshot + manage_babysit_lease CLIs. Zero env reads, zero hardcoded identities; all config via CLI flags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Port the two privileged mutation helpers onto the decomposed engine. Both import only util/gh/checks (+ the pure is_dependency_author) and route every gh call through the shared runner and the single reviewThreads paginator. Merge gate hardening beyond the source: --allowed-owners replaces the retired BABYSIT_OWNERS channel and fails closed (exit 3) when absent; dependency-manager-authored PRs are held absent --allow-dependency; a non-self PR on an unprotected base (0 required reviews AND 0 required contexts) is held absent --allow-unprotected; check dedup keys by (type,name,workflow) via babysit_checks. Resolve helper keeps its bot-only bright line, isOutdated autonomous guard, and comment-count + last-updated TOCTOU pins. Extend the shared paginator with per-comment updatedAt (the edit-detection pin's signal). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the two guarded-mutation wrappers as the fleet's first plugin bin/ executables: source-control-babysit-merge and source-control-babysit-resolve-thread. Bare names let a non-interpreter allow rule survive auto mode; the merge wrapper refuses --allow-unpinned-head so no allow-rule-covered invocation merges an unvetted head. A shared sourced babysit-python.sh resolves a working Python >= 3.11 by functionality probe (not PATH presence, which Windows Store aliases defeat). Drop the typing.TypeIs annotations from babysit_util (a 3.13-only import with zero runtime value and no type-checker in CI) so the real floor is 3.11 (datetime.UTC), widening consumer compatibility. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Port the four non-guard mutation CLIs onto the decomposed engine: manage_feedback_ledger (dispose / advisory-round / worker-checkin), prune_babysit_worktrees (--root required, no default path, dotfiles special-case removed), refresh_pr_branch (guarded behind-base refresh, BLOCKED compare fallback), and request_review (renamed from the codex helper, config-driven via --trigger-phrase / --review-bot-logins, review_request_history / review_attempt_history ledger keys). All route subprocess through babysit_util and take state via --state-dir. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rework the skill's documentation and config for the tier system. SKILL.md gains the safe/worker/autopilot grammar, the AC2-narrowed safe default (own PRs under the current owner), the substituted effective-config block, the explicit-keyword rule for merge-capable tiers, and per-tier draft policy — with every test_skill_contract prose literal preserved. Seven new reference files (orchestration, cadence, freshness, review-trigger, worktrees, safety, feedback) carry the ported fleet mechanics with untrusted PR fields fenced as data and script invocations citing the bin/ wrapper names. plugin.json declares 13 non-sensitive userConfig keys; setup gains a babysit check/apply section (effective config, branch-protection posture, Windows long-path); README documents the tiers + config table. evals.json adds tier/gate/ dependency/dormant/honesty cases and revises the Phase-1 cases for the narrowed scope. Multi-value substitution smoke recorded (csv). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bump to 0.7.0 (0.x breaking-by-minor: the safe default narrows to own PRs under the current owner). Wire the two review-downgrade userConfig keys (approval_downgrade_logins, skip_downgrade_logins) that the engine already accepts as flags, so all 15 keys are reachable and the design record, manifest, SKILL effective-config block, and README table agree. Refresh the plugin description for the tier system and regenerate the root catalog. CHANGELOG documents the convergence, the breaking safe-default narrowing, the state-root move, and the retired BABYSIT_* Python env seams. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Relocate and reorganize the engine test coverage onto the decomposed modules: per-module behavioral tests (checks, feedback, gh, lease, delta, state), the review-trigger race suite retargeted to the config-driven module, a near-verbatim test_skill_contract port passing against the current SKILL.md, cross-module integration tests (classify_pr to save_state round-trip, scoped-clobber, corrupt-state quarantine), and guard-CLI security tests (allowlist fail-closed, head/thread pin refusals). engine.test.sh drives the unittest suite, an optional ruff pass, and the bash-level wrapper checks; it self-SKIPs when Python 3.11+ is absent. 174 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(PR #322) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a58cb3e1fb
ℹ️ 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".
… scripts executable Discovery forwarded a multi-value `self_logins` as a single `gh --author` argument, but `--author` accepts one login, so a comma-joined value matched no one and multi-login users silently lost their own PRs from the queue. Resolve the author filter to a deduped, order-preserving list (new `resolve_authors`: comma-split, per-login `@me` resolution), query `gh` once per login, and union the results; errors dedupe across passes and `self_logins` becomes the full set. The `--self` readiness path already comma-split, so this closes the discovery-vs-gate asymmetry. Reported by Codex on #322. Also stamp the executable bit on the 18 shebang-carrying engine scripts: they were committed on Windows without +x and tripped the repo hygiene gate, which requires a shebang file to be mode 100755 (matching the sibling scripts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 975118d11e
ℹ️ 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".
The multi-author union test left `list_repos_for_owner` unmocked, so the
owner-mode listing sweep shelled out to a real `gh`. That passed where `gh` is
on PATH but failed on CI ("gh executable not found on PATH") by appending a
repository-list error. Mock it to [] like the sibling owner-mode tests; the
search path already exercises the per-author union deterministically.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc594c260a
ℹ️ 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".
|
Claude finished @kyle-sexton's task in 4m 33s —— View job PR Review: feat(source-control): converge babysit-prs tiers + engine (0.7.0)Tasks
SummaryThis is a large, high-quality convergence PR. The security model is solid: the subprocess runner is allowlist-only with no That said, I found four issues that warrant action before merge: two runtime bugs, one documentation error that causes a real behavioral gap, and one code/doc divergence in the autonomous thread-resolve path. Confirmed Bugs1.
|
|
Claude finished @kyle-sexton's task in 2m 4s —— View job PR Review: feat(source-control): converge babysit-prs tiers + engine (0.7.0) — Updated for dc594c2Tasks
What Changed Since Last ReviewThe latest push (975118d) fixed the multi-login discovery bug: Bugs — Still Unresolved1.
|
| Finding | Fix commit |
|---|---|
Multi-value self_logins delivered as single gh --author, silently dropping owned PRs for multi-login users |
975118d |
Tracked (Not Blockers)
babysit_resolve_thread.py:319— Bulk--autonomouspath can't enforce pre-push-snapshot requirement: The TOCTOU pin check only runs when--thread-idis supplied;--autonomous --resolvein bulk resolves allisOutdatedbot threads without a per-thread pin. SKILL.md documents the pre-push requirement as a prose constraint on the agent. This is a bounded risk (requires a worker's own push to incidentally satisfyisOutdatedon an unaddressed finding) but not a runtime crash. Tracked here alongside fix(source-control): reconcile review-trigger 'pending or failing' gate contract with pending-only code #324.- Failing gate never becomes trigger candidate (
pendingonly, contract sayspending or failing): Deferred to fix(source-control): reconcile review-trigger 'pending or failing' gate contract with pending-only code #324 per kyle-sexton's analysis. ✓
Positive Observations (Unchanged)
The core of this PR remains well-engineered: run_command uses allowlist + shutil.which + no shell=True; the merge gate's --match-head-commit gives server-side atomic head pinning; bot identity is __typename == "Bot" OR [bot]-suffix (no hardcoded list, fail-closed on truncated pages); parse_repo_number URL validation is correctly paranoid; the GraphQL paginator fails closed on malformed pageInfo and raises on partial thread-connection pages. The architecture decomposition (8 modules + 8 CLIs + bin wrappers) is clean. The test_skill_contract.py SKILL.md prose-assertion pattern is a nice safeguard against doc/code drift.
The three open bugs are all straightforward one-liners or template additions — no design changes required.
|
Claude finished @kyle-sexton's task in 2m 19s —— View job Claude finished reviewing PR #322 — Updated for dc594c2 Tasks
What Changed in dc594c2
The Bugs — Still Unresolved (Three Open from Prior Reviews)1.
|
| Finding | Fix commit |
|---|---|
Multi-value self_logins delivered as single gh --author, silently dropping owned PRs for multi-login users |
975118d |
test_multiple_authors_are_queried_separately_and_unioned missing list_repos_for_owner stub (non-hermetic) |
dc594c2 |
Tracked (Not Blockers)
babysit_resolve_thread.py:319— Bulk--autonomouspath can't enforce pre-push-snapshot requirement: The TOCTOU pin check runs only when--thread-idis supplied.--autonomous --resolvewithout--thread-idresolves allisOutdatedbot threads with no per-thread pin; a worker's own push can satisfyisOutdatedon a finding the worker never addressed. SKILL.md documents the pre-push constraint as a prose obligation on the agent, not enforced in code. Bounded risk, tracked alongside fix(source-control): reconcile review-trigger 'pending or failing' gate contract with pending-only code #324.- Failing gate never becomes trigger candidate (
pendingonly, contract sayspending or failing): Deferred to fix(source-control): reconcile review-trigger 'pending or failing' gate contract with pending-only code #324 per prior analysis. ✓
Positive Observations (Unchanged from Prior Review)
The core of this PR is well-engineered. The dc594c2 test fix is small, correct, and exactly right — it closes the only non-hermetic gap in the multi-author union test coverage. The three open bugs above are all one-liners or template additions; no design changes are required.
…ave) Resolve the plugin.json / CHANGELOG / README conflicts from main's #325 (skill-channel userConfig migration); rebump source-control to 0.8.0 (main shipped 0.7.0). - Prefix all 15 babysit userConfig keys `babysit_*` to match #325's feature-prefixed convention; `self_logins` unifies with #325's `babysit_self_logins`. Keep main's `worktree_stale_days` and `fetch_logs_max_bytes`. - Additive self-identity across every consumer: `--extra-self` (gate, from #325), `--author @me,<extras>` (discovery), `--self-logins @me,<extras>` (merge gate) each fold `babysit_self_logins` onto your gh login. - Fix F4: review-trigger candidacy no longer stalls when no CI gateway context is configured (ci_gateway_green true when unset). - Fix F5: pr_queue_snapshot stores the resolved --state-dir so a `~`-prefixed dir is shared with the other state CLIs. - Fix F6: babysit_merge resolves `@me` in --self-logins (after the argument-shape refusals; fail-closed if unresolvable) so own PRs are exempt from the unprotected-base hold. Reported by Codex on #322. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
markdownlint MD022/MD032: the merge-reconciled CHANGELOG ran the 0.8.0 "Removed" list straight into the "## [0.7.0]" heading with no blank line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c006d65365
ℹ️ 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".
|
Claude finished @kyle-sexton's task in 4m 22s —— View job PR Review: feat(source-control): converge babysit-prs tiers + engine (0.7.0) — Updated for af32fabTasks
What Changed in af32fab
Bug fixes shipped:
Breaking rename (correctly documented): All 15 babysit userConfig keys now carry the Consistency cross-check: The 15 All Fixed ✓
Tracked (Not Blockers)
Overall AssessmentAll confirmed bugs from the two prior reviews have been fixed in this branch. The af32fab merge is clean: the The four tracked deferrals (#324, #326, #327) are correctly scoped to an opt-in dormant-by-default module and do not affect the safe default path. The PR is ready to merge from a correctness and security standpoint. |
|
Claude finished @kyle-sexton's task in 2m 47s —— View job PR Review: feat(source-control): converge babysit-prs tiers + engine (0.7.0) — Updated for c006d65Tasks
What Changed Since Last Reviewc006d65 is a pure style fix — blank line before the af32fab (the reconciliation commit reviewed here) fixed all three previously-confirmed bugs. Verified against the current HEAD: All Prior Bugs — Fixed ✓1.
|
| Finding | Status | Commit |
|---|---|---|
resolve_state_dir return value discarded |
✅ Fixed | af32fab |
| CI gateway blocks trigger when unconfigured | ✅ Fixed | af32fab |
SKILL.md wrong flag name + missing --self-logins |
✅ Fixed | af32fab |
Multi-value self_logins silently dropped |
✅ Fixed | 975118d |
Bulk --autonomous TOCTOU pin gap |
Tracked → #326 | — |
| Failing gate never becomes trigger candidate | Tracked → #324 | — |
| Prior-head reactions suppress new head candidacy (F7) | Tracked → #327 | — |
| BLOCKED-but-BEHIND PRs bypass stale-head guard (F8) | Tracked → #327 | — |
…-behind guard, stale-head reactions) (#452) Closes #327 ## Summary Two head-staleness defects in the dormant-by-default AI review-trigger flow (both found by Codex on #322, P2). The module only activates when `babysit_review_trigger_phrase` + `babysit_review_bot_logins` + `babysit_review_gate_context` are configured, so neither affects the default babysit tiers. ### F7 — reject compare-behind BLOCKED heads before requesting review `request_review.py`'s pre-POST freshness guard rejected only the literal `BEHIND` merge state. GitHub reports `mergeStateStatus=BLOCKED` for a PR that is *also* behind its base (BLOCKED masks BEHIND), so the guard passed and posted the one-shot trigger on a **stale head**. The guard now reuses `compute_branch_freshness(current)` — off the `_blocked_base_compare` enrichment `view_pr` already computes — so a compare-confirmed behind head is rejected and the branch-refresh flow runs first. ### F8 — ignore stale-head reactions for new review windows The candidate predicate's unconditional `and not reactions` blocked candidacy whenever *any* reaction existed. Reactions carry no commit SHA, so a reviewer-bot reaction left on an earlier head persisted onto later heads; the new head never accrued `missing_observations` and the trigger never posted for the updated head. The check is now scoped to `associated_reactions` (reactions associated with, or newly observed for, the current head — already computed nearby). ## Scope note The live post-time guard at `request_review.py` (`if reaction_signals: raise`) and the state-string branch at `babysit_review_trigger.py` remain conservative by design — they still refuse to post while any unproven reaction lingers. F8 restores the **observation window** (candidacy / `missing_observations` accrual) without weakening a fail-safe post guard. Fully unblocking end-to-end posting through a persistent stale reaction is a larger call related to #324 and is intentionally out of scope here. ## Verification - `engine.test.sh` locally: 184 unittest OK, **ruff clean**, guarded-wrapper checks pass. - New regression tests in `test_review_trigger_race.py`: F7 (BLOCKED-masked-BEHIND rejected; BLOCKED-but-fresh still passes), F8 (stale earlier-head reaction no longer suppresses the window; current-head reaction still suppresses). - source-control `0.9.0 → 0.9.1` + CHANGELOG. ## Related - #322 (PR-B), #324 (sibling review-trigger reconciliation — not pulled in; one PR per issue). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rule, snapshot exit-code taxonomy, cross-PR dependency channel (#456) Closes #373. Implements the four enhancements plus one gotcha from issue #373 in the `babysit-prs` skill. The source retrospective the issue cites (`.work/handoffs/babysit-autopilot-retro.md`) is not present in the checkout (`.work/` is gitignored), so every edit was driven by the issue body plus the live skill files; the retro is provenance only. All doc edits are pointer-style (they point at the existing rule/clause, never restate it). ## (a) No-monitor hard STOP at the merge / gate-completion step Added in both `SKILL.md` (Guarded mutations) and `reference/safety.md` (Pinned-Command Degradation): once a PR is proven ready, or its merge is deferred to a human, report and stop — never arm a watch/monitor. Both point at the existing no-background-monitor clause in the Worker Contract (`reference/orchestration.md`) rather than restating it. ## (b) Bare-wrapper pipe rule `reference/safety.md` Guarded Mutation Wrappers now states that the wrapper's JSON must be parsed in a separate step — never `wrapper | python` / `| jq` — because an interpreter-in-pipeline trips the auto-mode safety classifier and blocks the call before the wrapper runs. ## (c) Snapshot exit-code taxonomy `scripts/pr_queue_snapshot.py` previously collapsed every non-fatal error into exit `1`, so a purely-advisory head-ref alias-check error was indistinguishable from a substantive per-PR hydration failure. The split: | code | meaning | state written | | --- | --- | --- | | `0` | valid snapshot, no errors | yes | | `1` | valid snapshot, ≥1 substantive error (per-PR hydration or discovery failure) | yes | | `2` | fatal — exception before a snapshot existed | no | | `3` | valid snapshot, advisory-only (head-ref alias cross-check) errors | yes | Substantive errors take precedence over advisory (a run with both returns `1`). The advisory marker is a single-source constant (`babysit_delta.HEAD_REF_ALIAS_ERROR_MARKER`) used at both message-construction sites (queue path in `babysit_delta.py`, single-PR path in `pr_queue_snapshot.py`) so they cannot drift. Taxonomy documented in the module docstring; new `scripts/tests/test_pr_queue_snapshot.py` covers 0/1/2/3 including the advisory-vs-substantive distinction and precedence. **Consumers checked** (exit codes are a caller contract): every `*.md`, `*.sh`, `*.py`, `engine.test.sh`, orchestration doc, and `bin/` wrapper referencing the snapshot. No live consumer keys on the script's `$?` — the docs (`orchestration.md`, `SKILL.md`, `loop.md`) invoke it and parse the JSON snapshot; `babysit-python.sh` uses `exec` (child exit code passes through unchanged); no `bin/` wrapper or sibling script branches on it. Introducing `3` therefore breaks no caller that treated `1` as "any error". ## (d) Worker→main cross-PR dependency channel New `Cross-PR Dependency Signalling` section in `reference/orchestration.md` formalizing a worker signalling a discovered cross-PR coupling (e.g. medley#1567 ↔ #1597, #224 ↔ #356) back to the main agent, which owns cross-PR ordering. Points at the existing `SendMessage` mechanism (documented there for main→worker), explicitly reversed. ## Gotcha: self-blocking CI check Recorded in the `SKILL.md` Gotchas list: a newly required check whose own fix PR carries that same check cannot be gate-merged and needs a one-time human admin-merge bootstrap. The concurrent-same-identity yield rationale is already covered by the Checkout/Push invariants (head-SHA recheck + worker lease + `foreign_activity` suppressor) and is deliberately not duplicated. ## Verification - `bash engine.test.sh`: `Ran 186 tests … OK`; ruff `All checks passed!`; guarded-wrapper behavior checks all PASS. - markdownlint-cli2 on the four changed `.md` files: 0 issues. - Plugin bumped `0.9.0` → `0.9.1` (`jq -e .version` confirms) with a matching `## [0.9.1]` CHANGELOG entry. - No trailing whitespace / final-newline issues in any changed file. ## Related - #322 (PR-B) — retro source for these enhancements. - #324 — sibling review-trigger reconciliation (F8 posting-guard follow-on territory). - #326, #327 — same babysit-prs safety-hardening cluster. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sit thread resolves (#561) ## Summary `babysit_resolve_thread.py`'s bulk resolve path (`--resolve` with no `--thread-id`) did not require the per-thread TOCTOU pins the single-thread path already enforces. Because an autonomous worker's own push marks a review thread `isOutdated`, the bulk `--autonomous --resolve` path could clear threads that changed since they were vetted with no proof the finding was addressed — the actor signing its own permission slip. This implements the maintainer-confirmed Option 1: reject bulk autonomous resolves and require a per-thread vetted loop. ## Fix - **Guard (root cause):** In `babysit_resolve_thread.py`, `--autonomous --resolve` without `--thread-id` is now refused (exit 2, before any network fetch), forcing the unattended-worker path through a per-thread loop where each thread reuses the existing pin guard (`--expected-comment-count` + `--expected-last-updated`). No second pinning model is introduced. - **Closed bypass (deliberate extension beyond AC1's literal "without --thread-id"):** `--allow-unpinned-thread` is also refused in `--autonomous` mode — otherwise `--autonomous --resolve --thread-id X --allow-unpinned-thread` would reopen the exact "resolve with no pins" hole. It stays an interactive-only override. Flagged here explicitly for maintainer visibility; it is restriction-direction (auto-ratified) and fail-closed, consistent with the Option 1 rationale. - **Docs:** Updated the module docstring and `SKILL.md` Autopilot step 2 from a bulk call to the per-thread vetted loop, aligning them with the pinned form already documented in `reference/orchestration.md`, `reference/safety.md`, and the worker prompt template (those were already correct; the code and step 2 were the stragglers). - **Test:** Two regression tests in `test_guards.py` prove the bulk autonomous resolve is refused and that `--allow-unpinned-thread` cannot bypass pins in autonomous mode. - Version bump `0.9.3 → 0.10.0` (behavior change to the autonomous-worker contract) + CHANGELOG entry. ## Verification Full plugin engine suite (stdlib unittest + ruff + guarded-wrapper behavior) via `bash engine.test.sh`: ``` == unittest suite == Ran 219 tests in 1.293s OK == ruff == All checks passed! == guarded-wrapper behavior == PASS: merge wrapper rejects --allow-unpinned-head PASS: merge wrapper reaches fail-closed CLI (no allowlist) PASS: resolve wrapper reaches fail-closed CLI (no allowlist) === EXIT: 0 === ``` New regression tests (`python -m unittest tests.test_guards -v`): ``` test_autonomous_allow_unpinned_thread_is_refused ... ok test_autonomous_bulk_resolve_without_thread_id_is_refused ... ok ... (11 tests) ... OK ``` markdownlint-cli2 on the two changed Markdown files: `Summary: 0 error(s)`. Plugin + catalog manifests: `All plugin manifests and the catalog validated.` Closes #326 ## Related - #326 — the issue this closes (maintainer decision: Option 1, reject bulk autonomous resolves). - #322 — the PR whose review pass originated this finding. - #324 — sibling issue from the same review pass. - #571 — machine-enforced fix for the displacement bypass (autonomous worker clearing a bot thread its own push merely displaced); the per-thread pins in this PR do not close it. 🤖 Generated with a claude-code-plugins work-2 lane implementation subagent (model: opus) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…oop.md §5.3 (#1307) *This was generated by AI during an autonomous work-loop execution session.* Closes #653 ## Summary `babysit-prs`'s SKILL.md described a cadence-ownership split that no longer exists. #652 added the engine-backed `recommended_cadence` → `ScheduleWakeup.delaySeconds` mapping table to `reference/loop.md` §5.3, alongside the static Python-free degrade ladder already living there. `reference/cadence.md` owns only the cadence *states and thresholds* — and has said so since #322 ("`loop.md` owns the wake mechanics, this file owns the states and thresholds behind the recommendation"). SKILL.md still routed readers the old way: | Surface | Before | After | |---|---|---| | Runbook step 9 | `recommended_cadence` per `cadence.md`; `loop.md` §5.3 named only as the "static ladder … Python-free degrade" | `Schedule the next wake per the cadence contract in reference/loop.md §5.3.` | | Reporting closing line | "Recommend the exact next interval per `cadence.md`" | same sentence, now citing `loop.md` §5.3 | | References entry for `loop.md` | "…checklist, static cadence ladder" | "…checklist, and the §5.3 cadence contract" | | Step-5 progressive-disclosure trigger | load `cadence.md` "only before **recommending cadence**" | load `cadence.md` "only before **interpreting a cadence state**" | The filed issue named step 9. The other three are the same defect: leaving them would have made the file internally inconsistent about which section owns the seconds. **The Reporting line was a live wrong-number risk, not just imprecision.** `cadence.md` states `idle` = **daily**; §5.3 documents `ScheduleWakeup` clamping `delaySeconds` to `[60, 3600]`, so inside `/loop` `idle` and `quiet` both wake hourly. "Recommend the exact next interval per `cadence.md`" could therefore surface an interval the loop will never schedule. The `cadence.md` link is dropped from step 9 rather than kept alongside `loop.md`: §5.3 already hops to `cadence.md` for the states in one line, so a second pointer here is the gratuitous cross-reference `CLAUDE.md`'s pointer-not-copy rule forbids. `cadence.md` keeps its own References entry and its step-5 disclosure trigger. The new step-9 wording matches vocabulary the file already uses at its Step 7 checklist line ("schedule the next wake per the cadence contract (§5.3)") and in the sibling `babysit-loop` SKILL.md ("that mapping owns the seconds"). Docs-only. No behavior change, no script or engine change. `source-control` `0.26.1` → `0.26.2` with the matching CHANGELOG entry. ## Test plan Every command below was run from the branch worktree; all passed. - `plugins/skill-quality/scripts/check-skill.sh babysit-prs` (`CHECK_SKILL_BASE_REF=origin/main`) — **exit 0**, `PASS — 0 errors`. This is the gate CI runs via `scripts/check-changed-skills.sh`. Includes the 500-line hard cap, all 9 base-ref trigger phrases preserved, broken-internal-ref check, and `scripts/engine.test.sh`. - **SKILL.md is now 497 lines** (base `origin/main`: 499). An intermediate revision of this branch landed at exactly 500 and *failed* the hard cap (`LINE_COUNT >= LINE_HARD_CAP`, cap 500) — caught by an independent reviewer before the PR was opened. Collapsing step 9 to a single pointer and the References bullet back to two lines both fixed the cap and made the diff better follow pointer-not-copy. The one remaining `WARN` is the pre-existing 200-line soft target, untouched by this change. - `scripts/check-changelog-parity.sh --check` — exit 0. - `scripts/check-changelog-parity.sh --check-bump origin/main` — exit 0 (the manifest bump has a matching `## [0.26.2]` entry). - `markdownlint-cli2` over both changed markdown files, repo config auto-discovered — exit 0, 0 errors. - `jq empty` on `plugins/source-control/.claude-plugin/plugin.json` — exit 0. - `typos` over both changed markdown files — exit 0. - Every relative link in the edited regions resolved by hand (`reference/loop.md`, `cadence.md`, `safety.md`, `orchestration.md`, `freshness.md`, `stuck-checks.md`, `feedback.md`, `review-trigger.md`); the `§5.3` anchor exists at `reference/loop.md:426`. No external URLs introduced, so lychee's absence changes nothing. - Repo-wide `grep` for any remaining reference sending a reader to `cadence.md` for wake seconds, or describing §5.3 as holding only the static ladder — none remain. The four surviving `cadence.md` citations in `plugins/source-control/` were each checked and are correct. Claims verified against the actual files rather than the issue text: §5.3 does contain both the mapping table (`loop.md:435-440`) and the degrade ladder (`loop.md:456-462`); `cadence.md:4-5` does disclaim the wake mechanics; `cadence.md:27` does state `idle` = daily against `loop.md:449-452`'s `[60, 3600]` clamp. The CHANGELOG entry's causal account was corrected after `git show --stat e9cef6e` showed #652 never touched `cadence.md`, and `git log -L4,5` dated that file's disclaimer to `fe78acb6` (#322) — so the references were always imprecise; #652 only made the correct target concrete. Per this repo's fresh-docs mandate, <https://code.claude.com/docs/en/skills> was fetched this session. It confirms the progressive-disclosure model this diff operates on — SKILL.md body plus bundled supporting files loaded on demand — and imposes no constraint on how a SKILL.md cites its own `reference/` files, so the choice of pointer is a repo-convention question, decided above by `CLAUDE.md`'s pointer-not-copy rule. ## Related - Refs #652 / #504 — the PR and issue that added the §5.3 mapping table this diff points at. - Refs #322 — where `reference/cadence.md` first disclaimed ownership of the wake mechanics. - Version note: open PRs #1285 and #1264 both claim `source-control` `0.27.0`. If either merges first this branch's `0.26.2` becomes a regression — it self-surfaces as a git conflict on both the manifest line and the CHANGELOG insert point, so rebase rather than force-merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…1392) *This was generated by AI during autonomous /work-items:work-loop execution.* Closes #324 ## Summary `reference/review-trigger.md` documented the review-trigger signal as `<review-gate-context>` "pending **or failing**", while every `gate_state` comparison in the engine accepts only `pending` — the candidate predicate and `request_signal_pending` in `babysit_review_trigger.py`, and the "awaiting requested review" material signal in `babysit_delta.py`. #324 called this a genuine design fork and explicitly forbade resolving it by inference. **Decision: honor the code, narrow the doc.** Documentation and test only; no behavior change. ### The empirical tiebreaker #324 asked for #324 named the deciding fact: *does the `<review-gate-context>` the cutover config wires report FAILING or PENDING when no reviewer has engaged?* That fact was located, and it settles the fork in favor of the code: - The wired context name is `codex-review` — `babysit_review_gate_context` under `pluginConfigs["source-control@melodic-software"]` in the dotfiles chezmoi data (`melodic-software/dotfiles`, `.chezmoidata/claude.json`). - The workflow that posts that context is `melodic-software/medley`, `.github/workflows/comment-review-gate.yml`. Every status it writes is either `state: "success"` (reviewer engaged) or `state: "pending"` (not engaged). It **never** posts `failure` or `error`: the file's only four `state:` assignments are three `success` and one `pending`. - Its header states the contract directly — *"If engaged → 'success' (review received, covers this push) … If not → 'pending'"*, and *"its status stays 'pending' until the next push re-checks."* So "no reviewer engaged yet" is **PENDING**, never FAILING. The honor-doc branch — widening the code so a failing gate becomes a trigger candidate — would have been correct only if the gate reported FAILING for that state, and it does not. `review-trigger.md`'s "or failing" was the erroneous restatement; the code was right. Scope note for the reviewer: the gate is deployed in `medley`, not in this repository — recent `claude-code-plugins` PRs carry no `StatusContext` at all, so the evidence above comes from the config that names the context plus the workflow that posts it, not from a live status on this PR. ### Corroborating repo-internal evidence Three signals inside this repository point the same way, independently of the gate observation: 1. `review-trigger.md`'s own Engagement Gate Semantics section — the mechanism-level definition of the gate's vocabulary — defines exactly two states (`PENDING` = no qualifying reviewer activity after the polling window, `SUCCESS` = may reflect activity from an earlier head) and gives `FAILING` no engagement meaning anywhere in the file. The "pending or failing" phrasing appears only in the downstream restatement of the helper's preconditions, one paragraph below a separate and correct rule about never discarding "a pending or failed StatusContext" — the likelier provenance of the phrase than authored intent to include failing gates. 2. A failing analogue of `request_signal_pending` (`effective_state == "PENDING" and not target_url`) would require knowing what `target_url` a failing gate carries. Nothing records that, so honoring the doc would have meant inventing a signal shape inside logic that posts comments to GitHub. 3. The motivating harm does not exist. `classify_checks` buckets the gate `StatusContext` like any other check, so a failing gate already raises `babysit_delta`'s "N failing check(s)" blocker. A failing gate is escalated to a human, not silently dropped — the coherent reading of the split: an unengaged reviewer is nudged automatically, a broken gate is not. ### Changes - **`reference/review-trigger.md`** — the trigger precondition now reads `pending`, and Engagement Gate Semantics states the failing case once (a failing gate is not an engagement signal, is never a trigger candidate, and reaches the operator through the ordinary failing-check blocker), so the contract is complete rather than merely non-contradictory. - **`test_review_trigger_race.py`** — a regression class pinning the chosen behavior. The rollup makes the gate `StatusContext` the only non-success check and asserts `ci_gateway_green` and `non_review_checks_green` are both true, so the failing gate is the isolated variable rather than one of several reasons `request_eligible` could be false. It also pins the `else`-branch pass-through (`state == "failing"`) and the failing-check bucketing from point 3 above. - **No code change.** All three sites named in #324, plus `pr_queue_snapshot.py`, already read `pending` and are correct under this decision; a grep of every `gate_state` comparison confirms the enumeration is complete. - Plugin version bump to `0.26.9` with the matching CHANGELOG entry. ### If you want to override This is the reversible branch. Should the deployed gate ever be rewired to report FAILING (not PENDING) for "no reviewer engaged yet", the fix is to widen the candidate predicate and define a failing analogue of `request_signal_pending` with the observed `target_url` shape in hand — and the regression class added here is what would need inverting. ## Test plan Run locally in the worktree: - `python -m unittest discover` over `plugins/source-control/skills/babysit-prs/scripts/tests` — **375 tests, OK** after integrating `main` (includes the 3 new regression tests). - `scripts/validate-plugins.sh` — all plugin manifests and the catalog validated. - `scripts/check-changelog-parity.sh --check` and `--check-bump origin/main` — both pass. - `markdownlint-cli2` on both changed markdown files — 0 errors. Left to required CI: `scripts/check-changed-skills.sh` and `scripts/check-skill-portability.sh`, which run in the `skill-quality-gate` and `portability-lint` jobs respectively. ## Related Refs #322 — PR-B (capability convergence); Codex flagged this doc/code fork inline there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
What
Converges the two mature babysit implementations — the personal dotfiles skill (worker/autopilot
auto-merge tiers, Python guard scripts, lease/worktree orchestration) and the extracted
source-control:babysit-prssafe-default loop — into one repo/machine/user-agnostic skill at0.8.0. This is the capability-convergence PR (PR-B) of the babysit-prs migration; the
extraction PR (#300) shipped the safe default.
Stacked on
mainafter #300, then merged up to reconcile with #325. Contract:docs/topics/babysit-prs-migration/PLAN.md(Phase 2), design recorddocs/topics/babysit-prs-migration/design/phase-2-engine-modules.md.Reconciliation with main (#325) — please sanity-check
#325 (the #315 userConfig wave) merged to main while this PR was in review, bumping source-control
to 0.7.0 and adding
babysit_self_logins(+worktree_stale_days,fetch_logs_max_bytes).This PR was rebumped to 0.8.0 and reconciled per two choices confirmed with the maintainer:
babysit_-prefixed (matching feat: migrate skill-channel personal scalars to native userConfig (4 plugins) #325's feature-prefixedconvention);
self_loginsunifies with feat: migrate skill-channel personal scalars to native userConfig (4 plugins) #325'sbabysit_self_logins.worktree_stale_daysandfetch_logs_max_bytesare kept as-is.gh api userlogin is always included andbabysit_self_loginsadds extras — folded onto@mefor discovery (--author), the readinessgate (
--extra-self, from feat: migrate skill-channel personal scalars to native userConfig (4 plugins) #325), and the merge gate (--self-logins).babysit_self_loginswas changed from #325's plainstring(default"") tomultiple: truewith no default, for consistency with the othermulti-value babysit keys (comma-join substitution is unchanged). If you'd rather keep #325's exact
shape, say so and I'll revert just that key.
Codex review dispositions
Six findings across four Codex passes, all triaged with GitHub-verified evidence:
--authorsingle-valued)pending-vs-failingcontractci_gateway_greenfalse when gateway unset--state-dir ~/…not normalized across CLIs--self-logins@meresolution)Highlights
workerauto-resolves outdated bot threads and merges gate-proven PRs;autopilotwidens author/thread scope under watched owners. The safe default is unchanged inspirit but narrows discovery to your own PRs under the current repo's owner (breaking, by
minor per 0.x).
parameterized discovery function, one reviewThreads paginator, one subprocess runner,
scope-aware + schema-versioned state with corrupt-file quarantine,
--reposharding with thedeployed state-clobber bug fixed, bot-agnostic review-trigger module (dormant when
unconfigured), L3 foreign-activity detection.
bin/wrappers for the two guarded mutations (bare-name allow rules surviveauto mode; the merge wrapper refuses
--allow-unpinned-head).userConfigkeys + a/source-control:setupbabysit check/apply section. Zero bakedidentities, owners, contexts, or paths.
engine.test.sh, self-SKIPwhen Python is absent).
Gate 0 — fresh docs
Fetched the official plugins reference on 2026-07-17
(
https://code.claude.com/docs/en/plugins-reference): confirmeduserConfigtypes(
string|number|boolean|directory|file,multiple: truearrays),${CLAUDE_PLUGIN_DATA}semantics (resolves to
~/.claude/plugins/data/{id}/, substitutes in skill content, env-exportedto hooks only — never skill-invoked Bash), and
bin/PATH-join behavior (collision-safe names).These shaped the flag-delivery contract (config → SKILL.md effective-config block → CLI flags) and
the wrapper naming.
Security review (plugin-acceptance re-trigger — new trust surface)
The
worker/autopilotmerge tier is a new trust surface, re-triggering the review across allsurfaces:
goes through one runner (
babysit_util.run_command, argv lists, noshell=True); guard-CLIargv is built only from validated refs/SHAs.
${CLAUDE_PLUGIN_DATA};--state-dir/--rootare flag-only and hard-error on empty/filesystem-root paths. No../.git/ghonly.--repodelta (with itsstate-clobber bug fixed); no hand-copied opaque blobs.
bin/PATH surface. Firstbin/executables in the fleet; plugin-prefixed collision-safenames; each wrapper self-locates its CLI and adds no capability of its own.
needs_worker_reasonsflowinto worker prompts fenced as quoted data, never instructions.
Merge-gate trust chain. Readiness is gate-verified end-to-end:
mergeStateStatus == CLEANcross-checked against the live effective branch rules, head-SHA pinned (
--match-head-commit),unresolved-thread and required-check reconciliation, never
--admin, never force-push.Dependency-manager PRs are held in every tier absent
--allow-dependency; a non-self PR on anunprotected base is held absent
--allow-unprotected.Named residual risk. The gate's scope inputs (
--allowed-owners, the allow-* overrides) areagent-supplied per call. Readiness stays gate-verified; scope relies on the wrapper's
rejected-flag set and the bare-name allow-rule surface. An out-of-band policy file the wrapper
reads is deferred (trigger: a real scope-confusion incident).
Trigger continuity (dotfiles skill → this skill)
Phase 3 retires the user-level dotfiles
/babysit-prsskill; its vocabulary maps here:/source-control:babysit-prs/source-control:babysit-prs worker/source-control:babysit-prs autopilotLint / gap notes
engine.test.shrunsruff checkopportunistically(self-SKIP when absent); pyright is not wired — recorded gap, deferred to a future toolchain
change (out of scope here per the Brief;
ci-workflowsis not this repo).worker/autopilottiers only; the safe defaultruns Python-free and degrades gracefully.
Related
capability convergence it scoped.
by this PR.
No linked issue (migration tracked by the PLAN contract, not a dedicated GitHub issue).