Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-06-29
Original file line number Diff line number Diff line change
@@ -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`).
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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/<your-name>/<branch-slug>`; 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/<your-name>/<branch-slug>`. 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/<your-name>/<branch-slug> --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/<your-name>/<branch-slug> --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).
43 changes: 43 additions & 0 deletions src/doctor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const {
OMX_SCAFFOLD_FILES,
AGENT_WORKTREE_RELATIVE_DIRS,
defaultAgentWorktreeRelativeDir,
envFlagIsTruthy,
} = require('../context');
const { runPackageAsset } = require('../core/runtime');

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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',
Expand All @@ -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();

Expand Down Expand Up @@ -1336,6 +1378,7 @@ module.exports = {
emitDoctorSandboxJsonOutput,
emitDoctorSandboxConsoleOutput,
autoFinishReadyAgentBranches,
runAutoShipGateForBranch,
pruneStaleAgentWorktrees,
runDoctorInSandbox,
};
73 changes: 73 additions & 0 deletions test/auto-finish-sweep-gate.test.js
Original file line number Diff line number Diff line change
@@ -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`);
}
});
Loading