diff --git a/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/.openspec.yaml b/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/.openspec.yaml new file mode 100644 index 0000000..34f9314 --- /dev/null +++ b/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-29 diff --git a/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/proposal.md b/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/proposal.md new file mode 100644 index 0000000..d4b649f --- /dev/null +++ b/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/proposal.md @@ -0,0 +1,35 @@ +## Why + +The doctor auto-finish sweep (`autoFinishReadyAgentBranches`) merges ready +`agent/*` branches into the base branch automatically and **without any review +gate** — it shells `agent-branch-finish.sh` directly, bypassing the merge gate +that interactive `gx finish` runs. The opt-in `GUARDEX_AUTO_SHIP` toggle makes +`gx finish` enforce that gate (clean AI review + green CI), but the unattended +sweep still merged ungated, so the two paths diverged: opting into safe shipping +did not make the background sweep safe. + +## What Changes + +- When `GUARDEX_AUTO_SHIP` is truthy, the auto-finish sweep runs the same + `runReviewGate` (review + green CI + GitHub-mergeable) before merging each + branch via the PR path. A blocked gate skips that branch (records a `[skip]` + detail) instead of merging. +- The gate applies only to the PR path. The direct/local fallbacks have no PR or + CI to gate and pass through unchanged. +- When `GUARDEX_AUTO_SHIP` is unset/falsy, sweep behavior is unchanged. +- Implementation extracted into an injectable `runAutoShipGateForBranch` helper + so the gate decision is unit-testable without a GitHub/`gh` fixture. +- `GUARDEX_AUTO_SHIP_REVIEW_PROVIDER` overrides the gate reviewer (default + `codex`) so an unattended sweep does not permanently block when the default + provider is unavailable. + +## Impact + +- Affected surface: `src/doctor/index.js` (auto-finish sweep). New test + `test/auto-finish-sweep-gate.test.js`. +- Behavior change is opt-in (gated behind `GUARDEX_AUTO_SHIP`); default off = + no change. +- Cost note: with the toggle on, `gx doctor`'s sweep runs an AI review + green-CI + wait per ready branch. That is the intended trade for closing the ungated + auto-merge-to-main hole; users who do not want it leave `GUARDEX_AUTO_SHIP` + unset (or set `GUARDEX_SKIP_AUTO_FINISH_READY_BRANCHES=1`). diff --git a/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/specs/gate-doctor-auto-finish-sweep-under-guardex-auto-ship/spec.md b/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/specs/gate-doctor-auto-finish-sweep-under-guardex-auto-ship/spec.md new file mode 100644 index 0000000..940dab8 --- /dev/null +++ b/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/specs/gate-doctor-auto-finish-sweep-under-guardex-auto-ship/spec.md @@ -0,0 +1,24 @@ +## ADDED Requirements + +### Requirement: Auto-finish sweep honors the GUARDEX_AUTO_SHIP merge gate +The doctor auto-finish sweep SHALL honor the `GUARDEX_AUTO_SHIP` merge gate when the toggle is enabled. The sweep SHALL enforce the same merge gate as interactive `gx finish` (clean AI review + green CI + GitHub-mergeable) before merging a ready `agent/*` branch into base via the PR path. The gate SHALL apply only to the PR path; direct and local fallback merges, which have no PR or CI to gate, SHALL pass through unchanged. When `GUARDEX_AUTO_SHIP` is unset or falsy, the sweep SHALL behave exactly as before this change. + +#### Scenario: Toggle off leaves the sweep ungated +- **WHEN** `GUARDEX_AUTO_SHIP` is unset or falsy and the sweep finishes a ready branch +- **THEN** the merge gate is not invoked +- **AND** the branch is merged using the prior sweep behavior. + +#### Scenario: Toggle on gates a PR-path merge +- **WHEN** `GUARDEX_AUTO_SHIP` is enabled and a ready branch would merge via the PR path +- **THEN** the review gate runs for that branch before any merge +- **AND** the branch merges only if the gate passes. + +#### Scenario: Gate failure skips the branch +- **WHEN** the review gate blocks a branch (failing review, red CI, or not mergeable) +- **THEN** the sweep records a skip for that branch with the gate reason +- **AND** the branch is not merged. + +#### Scenario: Non-PR fallback merges are not gated +- **WHEN** `GUARDEX_AUTO_SHIP` is enabled but the branch finishes via the direct or local fallback +- **THEN** the review gate is not invoked +- **AND** the branch merges through the fallback path unchanged. diff --git a/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/tasks.md b/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/tasks.md new file mode 100644 index 0000000..3f0bb1c --- /dev/null +++ b/openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/tasks.md @@ -0,0 +1,34 @@ +## Definition of Done + +This change is complete only when **all** of the following are true: + +- Every checkbox below is checked. +- The agent branch reaches `MERGED` state on `origin` and the PR URL + state are recorded in the completion handoff. +- If any step blocks (test failure, conflict, ambiguous result), append a `BLOCKED:` line under section 4 explaining the blocker and **STOP**. Do not tick remaining cleanup boxes; do not silently skip the cleanup pipeline. + +## Handoff + +- Handoff: change=`agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27`; branch=`agent//`; scope=`TODO`; action=`continue this sandbox or finish cleanup after a usage-limit/manual takeover`. +- Copy prompt: Continue `agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27` on branch `agent//`. Work inside the existing sandbox, review `openspec/changes/agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27/tasks.md`, continue from the current state instead of creating a new sandbox, and when the work is done run `gx branch finish --branch agent// --base dev --via-pr --wait-for-merge --cleanup`. + +## 1. Specification + +- [ ] 1.1 Finalize proposal scope and acceptance criteria for `agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27`. +- [ ] 1.2 Define normative requirements in `specs/gate-doctor-auto-finish-sweep-under-guardex-auto-ship/spec.md`. + +## 2. Implementation + +- [ ] 2.1 Implement scoped behavior changes. +- [ ] 2.2 Add/update focused regression coverage. + +## 3. Verification + +- [ ] 3.1 Run targeted project verification commands. +- [ ] 3.2 Run `openspec validate agent-claude-gate-doctor-auto-finish-sweep-under-guar-2026-06-29-09-27 --type change --strict`. +- [ ] 3.3 Run `openspec validate --specs`. + +## 4. Cleanup (mandatory; run before claiming completion) + +- [ ] 4.1 Run the cleanup pipeline: `gx branch finish --branch agent// --base dev --via-pr --wait-for-merge --cleanup`. This handles commit -> push -> PR create -> merge wait -> worktree prune in one invocation. +- [ ] 4.2 Record the PR URL and final merge state (`MERGED`) in the completion handoff. +- [ ] 4.3 Confirm the sandbox worktree is gone (`git worktree list` no longer shows the agent path; `git branch -a` shows no surviving local/remote refs for the branch). diff --git a/src/doctor/index.js b/src/doctor/index.js index 6d1e46e..22f327f 100644 --- a/src/doctor/index.js +++ b/src/doctor/index.js @@ -11,6 +11,7 @@ const { OMX_SCAFFOLD_FILES, AGENT_WORKTREE_RELATIVE_DIRS, defaultAgentWorktreeRelativeDir, + envFlagIsTruthy, } = require('../context'); const { runPackageAsset } = require('../core/runtime'); @@ -50,6 +51,7 @@ const { const { ensureOmxScaffold, configureHooks } = require('../scaffold'); const { detectRecoverableAutoFinishConflict, printAutoFinishSummary, isTerseMode } = require('../output'); const { autoCommitWorktreeForFinish } = require('../finish'); +const { runReviewGate } = require('../finish/review-gate'); /** * @typedef {Object} SandboxMetadata @@ -863,6 +865,34 @@ function syncDoctorLockRegistryAfterMerge(repoRoot, sandboxLockContent) { }; } +// When GUARDEX_AUTO_SHIP is on, the unattended auto-finish sweep must honor the +// same merge gate as `gx finish` (clean AI review + green CI + GitHub-mergeable) +// before merging an agent branch to base. The gate only applies to the PR path; +// the direct/local fallbacks have no PR or CI to gate, so they pass through +// unchanged. `runReviewGate` is injectable via `deps.runReviewGate` for tests. +// Returns `{ skip: true, reason }` when the gate blocks the merge. +function runAutoShipGateForBranch({ autoShip, fallbackMode, repoRoot, branch, baseBranch }, deps = {}) { + if (!autoShip || fallbackMode !== '') { + return { skip: false }; + } + const gate = deps.runReviewGate || runReviewGate; + // Override the reviewer for unattended runs: if the default provider is + // unavailable in the sweep environment, GUARDEX_AUTO_SHIP_REVIEW_PROVIDER + // avoids a permanent gate-block on every branch. + const reviewProvider = process.env.GUARDEX_AUTO_SHIP_REVIEW_PROVIDER || 'codex'; + try { + gate({ + repoRoot, + branch, + baseBranch, + options: { reviewProvider, allowNoChecks: false }, + }); + return { skip: false }; + } catch (error) { + return { skip: true, reason: `merge gate blocked (${error.message})` }; + } +} + function autoFinishReadyAgentBranches(repoRoot, options = {}) { const baseBranch = String(options.baseBranch || '').trim(); const dryRun = Boolean(options.dryRun); @@ -872,6 +902,7 @@ function autoFinishReadyAgentBranches(repoRoot, options = {}) { ? options.excludeBranches.map((branch) => String(branch || '').trim()).filter(Boolean) : [], ); + const autoShip = envFlagIsTruthy(process.env.GUARDEX_AUTO_SHIP); const summary = { enabled: true, @@ -984,6 +1015,16 @@ function autoFinishReadyAgentBranches(repoRoot, options = {}) { continue; } + const gateOutcome = runAutoShipGateForBranch( + { autoShip, fallbackMode, repoRoot, branch, baseBranch }, + { runReviewGate: options.runReviewGate }, + ); + if (gateOutcome.skip) { + summary.skipped += 1; + summary.details.push(`[skip] ${branch}: ${gateOutcome.reason}`); + continue; + } + summary.attempted += 1; const finishArgs = [ '--branch', @@ -1000,6 +1041,7 @@ function autoFinishReadyAgentBranches(repoRoot, options = {}) { } finishArgs.push(waitForMerge ? '--wait-for-merge' : '--no-wait-for-merge'); finishArgs.push('--cleanup'); + const finishResult = runPackageAsset('branchFinish', finishArgs, { cwd: repoRoot }); const combinedOutput = [finishResult.stdout || '', finishResult.stderr || ''].join('\n').trim(); @@ -1336,6 +1378,7 @@ module.exports = { emitDoctorSandboxJsonOutput, emitDoctorSandboxConsoleOutput, autoFinishReadyAgentBranches, + runAutoShipGateForBranch, pruneStaleAgentWorktrees, runDoctorInSandbox, }; diff --git a/test/auto-finish-sweep-gate.test.js b/test/auto-finish-sweep-gate.test.js new file mode 100644 index 0000000..9434ff6 --- /dev/null +++ b/test/auto-finish-sweep-gate.test.js @@ -0,0 +1,73 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { runAutoShipGateForBranch } = require('../src/doctor/index'); + +const BRANCH_INPUT = { + repoRoot: '/repo', + branch: 'agent/claude/x', + baseBranch: 'main', +}; + +test('auto-ship off: sweep does not gate (runReviewGate never called)', () => { + let called = 0; + const out = runAutoShipGateForBranch( + { autoShip: false, fallbackMode: '', ...BRANCH_INPUT }, + { runReviewGate: () => { called += 1; } }, + ); + assert.deepEqual(out, { skip: false }); + assert.equal(called, 0); +}); + +test('auto-ship on + PR path: gate runs and branch proceeds when it passes', () => { + let seen = null; + const out = runAutoShipGateForBranch( + { autoShip: true, fallbackMode: '', ...BRANCH_INPUT }, + { runReviewGate: (arg) => { seen = arg; } }, + ); + assert.deepEqual(out, { skip: false }); + assert.equal(seen.repoRoot, '/repo'); + assert.equal(seen.branch, 'agent/claude/x'); + assert.equal(seen.baseBranch, 'main'); + // The gate is invoked with the options runReviewGate actually reads. + assert.equal(seen.options.reviewProvider, 'codex'); + assert.equal(seen.options.allowNoChecks, false); +}); + +test('GUARDEX_AUTO_SHIP_REVIEW_PROVIDER overrides the gate reviewer', () => { + const prev = process.env.GUARDEX_AUTO_SHIP_REVIEW_PROVIDER; + process.env.GUARDEX_AUTO_SHIP_REVIEW_PROVIDER = 'claude'; + try { + let seen = null; + runAutoShipGateForBranch( + { autoShip: true, fallbackMode: '', ...BRANCH_INPUT }, + { runReviewGate: (arg) => { seen = arg; } }, + ); + assert.equal(seen.options.reviewProvider, 'claude'); + } finally { + if (prev === undefined) delete process.env.GUARDEX_AUTO_SHIP_REVIEW_PROVIDER; + else process.env.GUARDEX_AUTO_SHIP_REVIEW_PROVIDER = prev; + } +}); + +test('auto-ship on + PR path: gate failure skips the branch (no merge)', () => { + const out = runAutoShipGateForBranch( + { autoShip: true, fallbackMode: '', ...BRANCH_INPUT }, + { runReviewGate: () => { throw new Error('review found CRITICAL'); } }, + ); + assert.equal(out.skip, true); + assert.match(out.reason, /merge gate blocked/); + assert.match(out.reason, /review found CRITICAL/); +}); + +test('auto-ship on + non-PR fallback: no PR/CI to gate, so it passes through', () => { + for (const fallbackMode of ['direct', 'local']) { + let called = 0; + const out = runAutoShipGateForBranch( + { autoShip: true, fallbackMode, ...BRANCH_INPUT }, + { runReviewGate: () => { called += 1; } }, + ); + assert.deepEqual(out, { skip: false }, `fallback ${fallbackMode} should not gate`); + assert.equal(called, 0, `fallback ${fallbackMode} must not call the gate`); + } +});