diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index a909d99a7..5ff5f9d3f 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.15.0", + "version": "0.15.1", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply — interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index d68786509..d71393ec1 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,31 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.15.1] + +### Changed + +- **`babysit-prs` autopilot merge tier (#476) — completed the gate-off flip precondition (#675), + still shipped DISABLED.** Three coherence gaps that had to close before the tier can ever be + flipped on are now resolved, all as prose/contract changes with no behavioral shift to the + merge gate. (1) **Merge-surface wiring:** every autopilot merge surface is swept so an ENABLED + config can no longer merge via the flagless base path — autopilot's step 3 in `SKILL.md` and the + zero-blocker direct-gate path both point at `reference/safety.md`, now the single home for both + the base and the enabled-tier merge paths, and the Pinned-Command Degradation operator handoff + reproduces the tier-flagged command when the tier is enabled. (2) **Second-account approve mechanic:** the concrete + out-of-band approval the gate's distinct-bot criterion requires is specified — `gh pr review + … --approve` submitted under a distinct `` identity (`GH_TOKEN` or `gh + auth switch`, never the PR author or a lane identity), only after a genuine clean review pass, + on the live head so the `--expected-head` pin holds. (3) **Review-workflow requiredness + precondition:** enabling the tier now carries a documented operator precondition — the base + branch's ruleset must make the review workflow a **required** status context *and* that workflow + must always run to a non-skipped conclusion on every PR to the base (requiredness is necessary + but not sufficient: a required-but-skipped review still reads `mergeStateStatus == CLEAN` without + having gated anything). Where the review workflow is not required, or can conditionally skip on + the tier's PRs, the tier must not be enabled. Chosen over a merge-gate review-context config + (rejected option b) to keep the gate deterministic with nothing new to wire. The skill-contract + tests are extended to pin all three contracts against drift. + ## [0.15.0] ### Added diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index 1e54beecb..8d0c9b9d9 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -151,12 +151,12 @@ a human. Per PR, in its own fresh worker, autopilot: `"action": "resolved"` before treating it as cleared — never the exit code alone. 3. After the worker's final push, takes a fresh post-push snapshot (or uses the exact pushed - commit after vetting it), then merges through `source-control-babysit-merge owner/repo#N - --allowed-owners --self-logins @me, --merge --expected-head - ` once the - deterministic gate proves the PR ready. Never reuse the pre-worker snapshot pin after a - push. The gate is never bypassed; if a PR cannot be made ready, autopilot reports that one - PR and moves on. + commit after vetting it), then merges on that post-push head through the pinned + `source-control-babysit-merge` gate once it proves the PR ready. The exact command — and the + `--autopilot-merge-tier` flags the enabled tier layers on so an enabled config never merges + via the base path — is the single home in [reference/safety.md](reference/safety.md). Never + reuse the pre-worker snapshot pin after a push. The gate is never bypassed; if a PR cannot be + made ready, autopilot reports that one PR and moves on. "Every PR" means every PR: the orchestrator's own priority judgment is never grounds to leave a queue member untouched. The only permitted exclusions are the deterministic ones — lease @@ -178,7 +178,7 @@ settings powers — those still escalate. Run it looped: ## Autopilot merge tier (#476) -A config-gated escalation of autopilot's merge authority, **shipped DISABLED** and active only while the operator sets `babysit_autopilot_merge_tier` (enabling it, and the later gate-off flip, are separate announced steps; without it every merge decision is exactly today's). When enabled, per candidate PR autopilot runs a **genuine review pass** under a **second bot account** (author ≠ approver) that submits an approving review **only when clean**, then runs the pinned merge gate with the tier flags (`--autopilot-merge-tier --lane-logins --approver-bot-logins --block-labels `) added to `--merge --expected-head `. +A config-gated escalation of autopilot's merge authority, **shipped DISABLED** and active only while the operator sets `babysit_autopilot_merge_tier` (enabling it, and the later gate-off flip, are separate announced steps; without it every merge decision is exactly today's). When enabled, per candidate PR autopilot runs a **genuine review pass** under a **second bot account** (author ≠ approver) that submits an approving review **only when clean**, then runs the pinned merge gate with the `--autopilot-merge-tier` flags layered onto `--merge --expected-head `. The concrete enabled-path merge command, the second-account approve mechanic, and the review-workflow requiredness precondition for enabling the tier are the single home in [reference/safety.md](reference/safety.md). That gate merges **only when every criterion holds** — the criteria and the safety-contract rationale are codified in [reference/safety.md](reference/safety.md). It is **fail-closed** (the umbrella flag refuses unless all three parameter sets are supplied; predicates reused from the shared `babysit_classify` module), and any criterion failing falls back to the human merge-ready list — the tier never routes around the gate. ## Guarded mutations: deterministic gates, agent judgment @@ -251,14 +251,14 @@ template (untrusted PR fields fenced as data) are in A PR that is merely unchanged since the last cycle — even one still reporting blockers it was already escalated for — does not get a fresh worker. A non-draft PR with zero blockers **and no untriaged material feedback** also gets no worker, only a direct mode-appropriate -`source-control-babysit-merge` gate check; that is coverage, not a skip — the gate does not -triage bot feedback, so a PR still carrying untriaged material findings defers to the -snapshot's `needs_worker` signal instead of going straight to the gate. In default (safe) mode, -run the gate without `--merge` and report readiness without merging. Pass +`source-control-babysit-merge` gate check; that is coverage, not a skip — a PR still carrying +untriaged material findings defers to the snapshot's `needs_worker` signal instead. In default +(safe) mode, run the gate without `--merge` and report readiness without merging. Pass `--merge --expected-head ` only in `worker` or `autopilot` mode, or under -an explicit user order to merge that PR. Use the exact head SHA from the snapshot; a missing or -stale pin must refuse the merge and send the PR back through snapshot and assessment, never an -unattended unpinned override. +an explicit user order to merge that PR — but an enabled autopilot merge tier adds the tier flags +([reference/safety.md](reference/safety.md)), never the flagless base command. Use the exact head +SHA from the snapshot; a missing or stale pin must refuse the merge and send the PR back through +snapshot and assessment, never an unattended unpinned override. **Zero-blocker drafts are the exception:** always route them through a worker, never directly to the merge gate. In autopilot, that worker assesses whether the draft is complete: a diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 55c4723af..cf76a9e12 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -160,6 +160,63 @@ auto-mode safety classifier and blocks the call before the wrapper runs. Treat a thread as cleared only when its own entry shows `"action": "resolved"`, and a merge as performed only when the merge output's `action` field says so. +## Autopilot Merge Tier: Enabled-Path Mechanics + +Reachable only while `babysit_autopilot_merge_tier` is enabled; absent that flag none of this +section applies and autopilot's merge path is byte-for-byte its prior self. This is the single +home for the enabled-path merge command that autopilot's step 3 in `SKILL.md` points at, so the +base and enabled-tier merge paths never drift apart. The tier still ships **DISABLED**; enabling +it, and any later gate-off flip, is a separate announced operator step. + +- **Enabled-path merge command.** After the worker's final push and a fresh post-push snapshot + (or the exact pushed commit, vetted), merge on that post-push head by layering the tier flags + onto the base gate command — this is the *only* autopilot merge path once the tier is enabled, + never the four-flagless base command, which would ignore every tier criterion: + + ```text + source-control-babysit-merge owner/repo#N --allowed-owners --self-logins @me, --merge --expected-head --autopilot-merge-tier --lane-logins --approver-bot-logins --block-labels + ``` + + The umbrella `--autopilot-merge-tier` is fail-closed: it refuses (exit `3`) unless + `--lane-logins`, `--approver-bot-logins`, and `--block-labels` are all supplied, and any of + those three without the umbrella is a usage error (exit `2`). Add `--method ` + when configured, exactly as for the base merge readiness gate above. + +- **Second-account approve mechanic.** The approving review the gate's distinct-bot criterion + requires is submitted out-of-band by the agent — the gate only verifies one exists on the live + head, it never creates it. Bind a **distinct** identity (one of the `` + accounts, never the PR author or a lane identity), run a **genuine** review pass — through a + review skill/plugin when one is installed, otherwise an equivalent thorough manual review (this + skill declares no review-plugin dependency; the gate requires only that the resulting approval + exists on the live head, not that a particular tool produced it) — and only when that pass is + clean submit the approval under that identity: + + ```text + GH_TOKEN= gh pr review owner/repo#N --approve --body "" + ``` + + `gh auth switch --user ` before a plain `gh pr review … --approve` is the + equivalent when the approver is a persisted gh account rather than a bound token. Submit on the + live head so the gate's head-unchanged-since-review pin (`--expected-head`) still holds; any + push after the approval invalidates it and the review pass must be re-run against the new head. + Never approve on an unclean pass, and never under the author or a lane identity — either + collapses author ≠ approver and the gate refuses the merge fail-closed. + +- **Review-workflow requiredness precondition (enabling).** Enable the tier ONLY on a base branch + whose ruleset makes the review workflow a **required status context** *and* whose review workflow + always runs to a non-skipped conclusion on every PR to that base. The gate proves the review ran + solely through `mergeStateStatus == CLEAN`, which guarantees only that *required* contexts passed; + a review workflow that is present but not required can be absent, skipped, or failing while the PR + still reads CLEAN, so the gate could green-light a merge the review never actually gated. + Requiredness is necessary but not sufficient: a conditionally-skipped review job can report a + `SKIPPED` conclusion that is counted as a passing state, so a required-but-skipped review still + reads CLEAN without having run. Requiring the review workflow therefore closes that hole + deterministically *only when* it cannot conditionally skip on the paths or conditions the tier's + PRs hit — it must always execute and produce a non-skipped result on the pinned head. Where the + review workflow is not a required context, or can skip on those PRs, do not enable the tier: this + is an operator enabling precondition, verified before the flip, not something the merge gate can + self-enforce. + ## Harness Permission Layer A permission denial can come from two different layers. Tell them apart before deciding how to @@ -199,6 +256,11 @@ For a merge: source-control-babysit-merge owner/repo#42 --allowed-owners --merge --expected-head --method ``` +When the autopilot merge tier is enabled, this degraded handoff carries the tier flags too: +surface the enabled-path command from Autopilot Merge Tier: Enabled-Path Mechanics above, not this +flagless base form, so the operator's manual merge is held to the same tier criteria the blocked +gate would have enforced. + For a thread resolve, never surface a bare `--autonomous` or `--include-human` resolve: both re-fetch the live thread list and re-evaluate every eligible thread at execution time, so an unpinned command could resolve a thread this run never vetted — one opened or changed after its diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_skill_contract.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_skill_contract.py index ea08c26f1..98d020fcf 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_skill_contract.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_skill_contract.py @@ -80,21 +80,43 @@ def test_unchanged_zero_blocker_gate_uses_the_snapshot_head(self) -> None: self.assertIn("exact head SHA from the snapshot", paragraph) self.assertNotIn("", paragraph) - def test_worker_push_paths_require_the_post_push_head(self) -> None: - pinned_merge = "--merge --expected-head " - markers = ( - "After the worker's final push", - "In worker mode, after a worker's fix", + def test_direct_gate_path_wires_the_tier_when_enabled(self) -> None: + # #675: the zero-blocker direct-gate autopilot path must also carry the + # tier flags when the tier is enabled, or an already-clean PR merges via + # the flagless base gate — the same fail-open §3 closed. + paragraph = _paragraph_containing( + self.skill_text, "A non-draft PR with zero blockers" ) + self.assertIn("an enabled autopilot merge tier adds the tier flags", paragraph) + self.assertIn("reference/safety.md", paragraph) + self.assertIn("never the flagless base command", paragraph) - for marker in markers: - with self.subTest(marker=marker): - paragraph = _paragraph_containing(self.skill_text, marker) - self.assertIn(pinned_merge, paragraph) - self.assertIn("fresh post-push snapshot", paragraph) - self.assertIn("exact pushed commit", paragraph) - self.assertIn("Never reuse the pre-worker snapshot pin", paragraph) - self.assertNotIn("", paragraph) + def test_worker_push_path_pins_the_post_push_head_command(self) -> None: + # Worker tier has no merge tier, so its push paragraph still spells the + # full pinned merge command inline. + paragraph = _paragraph_containing( + self.skill_text, "In worker mode, after a worker's fix" + ) + self.assertIn("--merge --expected-head ", paragraph) + self.assertIn("fresh post-push snapshot", paragraph) + self.assertIn("exact pushed commit", paragraph) + self.assertIn("Never reuse the pre-worker snapshot pin", paragraph) + self.assertNotIn("", paragraph) + + def test_autopilot_step3_points_at_safety_for_the_tier_wired_command(self) -> None: + # Autopilot §3 no longer inlines a base-only merge command (the coherence + # gap #675 closed): it points at safety.md, which holds both the base and + # enabled-tier merge paths as one home so an enabled config cannot merge + # via the flagless base path. The push discipline stays in the paragraph. + paragraph = _paragraph_containing( + self.skill_text, "After the worker's final push" + ) + self.assertIn("fresh post-push snapshot", paragraph) + self.assertIn("exact pushed commit", paragraph) + self.assertIn("Never reuse the pre-worker snapshot pin", paragraph) + self.assertIn("--autopilot-merge-tier", paragraph) + self.assertIn("reference/safety.md", paragraph) + self.assertNotIn("", paragraph) def test_generic_merge_gate_requires_the_vetted_head(self) -> None: paragraph = _paragraph_containing(self.skill_text, "**Merge readiness**") @@ -156,6 +178,44 @@ def test_safety_md_codifies_the_tier_criteria(self) -> None: with self.subTest(criterion=criterion): self.assertIn(criterion, safety) + def test_safety_md_specifies_the_enabled_path_mechanics(self) -> None: + # #675 flip-precondition prose: the tier-wired merge command, the + # second-account approve mechanic, and the review-context enabling + # precondition are pinned so they cannot silently drift. Fenced command + # lines land in their own paragraphs, so assert against the whole file. + safety = (SKILL.parent / "reference" / "safety.md").read_text(encoding="utf-8") + + for header in ( + "Enabled-path merge command", + "Second-account approve mechanic", + "Review-workflow requiredness precondition", + ): + with self.subTest(header=header): + self.assertIn(header, safety) + + # Enabled-path merge command — the four-flag tier layering is the single home. + self.assertIn( + "--autopilot-merge-tier --lane-logins " + "--approver-bot-logins " + "--block-labels ", + safety, + ) + self.assertIn( + "*only* autopilot merge path once the tier is enabled", safety + ) + + # Second-account approve mechanic — distinct approver identity, clean pass only. + self.assertIn("gh pr review owner/repo#N --approve", safety) + self.assertIn("GH_TOKEN=", safety) + self.assertIn("gh auth switch --user ", safety) + self.assertIn("never the PR author or a lane identity", safety) + + # Review-workflow requiredness enabling precondition (fork 3a). + self.assertIn("required status context", safety) + self.assertIn("mergeStateStatus == CLEAN", safety) + self.assertIn("operator enabling precondition", safety) + self.assertIn("do not enable the tier", safety) + def test_full_queue_and_draft_contract_remains_explicit(self) -> None: autopilot = _paragraph_containing(self.skill_text, '"Every PR" means every PR') drafts = _paragraph_containing(self.skill_text, "**Draft PRs** are in scope")