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
43 changes: 42 additions & 1 deletion bin/multiagent-safety.js
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,36 @@ function compactAutoFinishPathSegments(message) {
});
}

function detectRecoverableAutoFinishConflict(message) {
const text = String(message || '').trim();
if (!text) {
return null;
}

if (/rebase --continue/i.test(text) && /rebase --abort/i.test(text)) {
return {
rawLabel: 'auto-finish requires manual rebase.',
summary: 'manual rebase required in the source-probe worktree; run rebase --continue or rebase --abort',
};
}

if (/Rebase\/merge '.+' into '.+' and resolve conflicts before finishing\./i.test(text)) {
return {
rawLabel: 'auto-finish requires manual rebase or merge.',
summary: 'manual rebase or merge required before auto-finish can continue',
};
}

if (/Merge conflict detected while merging/i.test(text)) {
return {
rawLabel: 'auto-finish requires manual merge resolution.',
summary: 'manual merge resolution required before auto-finish can continue',
};
}

return null;
}

function summarizeAutoFinishDetail(detail) {
const trimmed = String(detail || '').trim();
const match = trimmed.match(/^\[(\w+)\]\s+([^:]+):\s*(.*)$/);
Expand All @@ -764,8 +794,11 @@ function summarizeAutoFinishDetail(detail) {
const [, status, rawBranch, rawMessage] = match;
const branch = truncateMiddle(rawBranch, DOCTOR_AUTO_FINISH_BRANCH_LABEL_MAX);
let message = String(rawMessage || '').trim();
const recoverableConflict = status === 'skip' ? detectRecoverableAutoFinishConflict(message) : null;

if (status === 'fail') {
if (recoverableConflict) {
message = recoverableConflict.summary;
} else if (status === 'fail') {
message = message.replace(/^auto-finish failed\.?\s*/i, '');
if (/\[agent-sync-guard\]/.test(message) && /Resolve conflicts/i.test(message)) {
message = 'rebase conflict in finish flow; run rebase --continue or rebase --abort in the source-probe worktree';
Expand Down Expand Up @@ -3563,6 +3596,14 @@ function autoFinishReadyAgentBranches(repoRoot, options = {}) {
continue;
}

const recoverableConflict = detectRecoverableAutoFinishConflict(combinedOutput);
if (recoverableConflict) {
summary.skipped += 1;
const tail = combinedOutput ? ` ${combinedOutput.split('\n').slice(-2).join(' | ')}` : '';
summary.details.push(`[skip] ${branch}: ${recoverableConflict.rawLabel}${tail}`);
continue;
}

summary.failed += 1;
const tail = combinedOutput ? ` ${combinedOutput.split('\n').slice(-2).join(' | ')}` : '';
summary.details.push(`[fail] ${branch}: auto-finish failed.${tail}`);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## Why

- `gx doctor` currently counts recoverable auto-finish rebase conflicts as hard failures even when the repo itself is safe and the only remaining work is a manual conflict resolution step.
- That makes long doctor sweeps look broken or unsafe when the real state is narrower: the branch cannot be auto-finished yet and needs a human to rebase or merge it.

## What Changes

- Reclassify recoverable auto-finish conflict states during `gx doctor` from `[fail]` to a manual-action `[skip]` status.
- Keep the compact default summary actionable and keep `--verbose-auto-finish` useful by preserving the raw tail text behind the skip line.
- Add install-test coverage for the new summary counts and color behavior.

## Impact

- Affects only doctor auto-finish reporting for branches that hit recoverable rebase or merge conflicts.
- Keeps true auto-finish failures red and failed; only manual-resolution conflict cases move to the skip/pending bucket.
- Main risk: conflict detection could miss a new finish-script wording, so the pattern matching should stay narrow and test-backed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## ADDED Requirements

### Requirement: doctor sweep classifies manual conflict work as actionable skips
The human-readable `gx doctor` auto-finish sweep SHALL classify recoverable manual conflict states as skip/manual-action rows instead of hard failures.

#### Scenario: auto-finish rebase conflict becomes a skip/manual-action row
- **GIVEN** a ready local `agent/*` branch exists during `gx doctor`
- **AND** `scripts/agent-branch-finish.sh` stops because it needs a human to continue or abort a source-probe rebase
- **WHEN** doctor prints the auto-finish summary
- **THEN** the summary SHALL not count that branch as failed
- **AND** the branch detail SHALL be emitted as a skip/manual-action row with the rebase instructions preserved in verbose mode

#### Scenario: true auto-finish failures remain failures
- **GIVEN** a ready local `agent/*` branch exists during `gx doctor`
- **AND** `scripts/agent-branch-finish.sh` fails for a reason other than a recoverable manual conflict
- **WHEN** doctor prints the auto-finish summary
- **THEN** the summary SHALL still count that branch as failed
- **AND** the branch detail SHALL remain a failed row
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## 1. Specification

- [x] 1.1 Finalize proposal scope and acceptance criteria for `doctor-auto-finish-manual-conflict`.
- [x] 1.2 Define normative requirements in `specs/doctor-workflow/spec.md`.

## 2. Implementation

- [x] 2.1 Reclassify recoverable doctor auto-finish rebase/merge conflicts from failed rows to manual-action skip rows.
- [x] 2.2 Keep compact default output actionable and preserve verbose raw tail text for manual conflict rows.
- [x] 2.3 Update focused doctor/install regressions for counts, detail text, and ANSI colors.

## 3. Verification

- [x] 3.1 Run focused doctor/install verification (`node --test --test-name-pattern "doctor" test/install.test.js`, `node --check bin/multiagent-safety.js`).
- [x] 3.2 Run `openspec validate agent-codex-doctor-auto-finish-manual-conflict-2026-04-22-10-42 --type change --strict`.
- [x] 3.3 Run `openspec validate --specs`.

Verification note: `node --check bin/multiagent-safety.js` passed. `node --test --test-name-pattern "doctor" test/install.test.js` passed with `18/18` doctor-focused tests, including the new skip/manual-conflict regressions. `openspec validate agent-codex-doctor-auto-finish-manual-conflict-2026-04-22-10-42 --type change --strict` passed, and `openspec validate --specs` returned `No items found to validate.` Extra check: `npm test` still fails on the pre-existing metadata parity assertion that `scripts/agent-branch-start.sh` diverges from `templates/scripts/agent-branch-start.sh`; this branch did not modify either file.

## 4. Completion

- [ ] 4.1 Finish the agent branch via PR merge + cleanup (`gx finish --via-pr --wait-for-merge --cleanup` or `bash scripts/agent-branch-finish.sh --branch <agent-branch> --base <base-branch> --via-pr --wait-for-merge --cleanup`).
- [ ] 4.2 Record PR URL + final `MERGED` state in the completion handoff.
- [ ] 4.3 Confirm sandbox cleanup (`git worktree list`, `git branch -a`) or capture a `BLOCKED:` handoff if merge/cleanup is pending.
13 changes: 7 additions & 6 deletions test/install.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,7 @@ exit 1
assert.match(combinedOutput, /Auto-finish sweep \(base=main\): attempted=1, completed=1, skipped=\d+, failed=0/);
});

test('doctor compacts auto-finish failures by default and expands them with --verbose-auto-finish', () => {
test('doctor treats recoverable auto-finish rebase conflicts as actionable skips', () => {
const repoDir = initRepoOnBranch('main');
seedCommit(repoDir);
attachOriginRemoteForBranch(repoDir, 'main');
Expand Down Expand Up @@ -1706,10 +1706,11 @@ exit 1
);
assert.equal(result.status, 0, result.stderr || result.stdout);
const compactOutput = `${result.stdout}\n${result.stderr}`;
assert.match(compactOutput, /Auto-finish sweep \(base=main\): attempted=1, completed=0, skipped=\d+, failed=0/);
assert.match(
compactOutput,
new RegExp(
`\\[fail\\] ${escapeRegexLiteral(readyBranch)}: rebase conflict in finish flow; run rebase --continue or rebase --abort in the source-probe worktree`,
`\\[skip\\] ${escapeRegexLiteral(readyBranch)}: manual rebase required in the source-probe worktree; run rebase --continue or rebase --abort`,
),
);
assert.doesNotMatch(compactOutput, /git -C "\/tmp\/very\/long\/path\/for\/source-probe-agent-worktree/);
Expand All @@ -1721,11 +1722,11 @@ exit 1
);
assert.equal(result.status, 0, result.stderr || result.stdout);
const verboseOutput = `${result.stdout}\n${result.stderr}`;
assert.match(verboseOutput, new RegExp(`\\[fail\\] ${escapeRegexLiteral(readyBranch)}: auto-finish failed\\.`));
assert.match(verboseOutput, new RegExp(`\\[skip\\] ${escapeRegexLiteral(readyBranch)}: auto-finish requires manual rebase\\.`));
assert.match(verboseOutput, /git -C ".+rebase --continue/);
});

test('doctor colors failure and success status lines when color output is enabled', () => {
test('doctor colors manual conflict skips yellow and success status lines green', () => {
const repoDir = initRepoOnBranch('main');
seedCommit(repoDir);
attachOriginRemoteForBranch(repoDir, 'main');
Expand Down Expand Up @@ -1767,12 +1768,12 @@ exit 1
assert.match(ansiOutput, /\u001B\[32m\[gitguardex\] ✅ No safety issues detected\.\u001B\[0m/);
assert.match(
ansiOutput,
/\u001B\[31m\[gitguardex\] Auto-finish sweep \(base=main\): attempted=1, completed=0, skipped=\d+, failed=1\u001B\[0m/,
/\u001B\[33m\[gitguardex\] Auto-finish sweep \(base=main\): attempted=1, completed=0, skipped=\d+, failed=0\u001B\[0m/,
);
assert.match(
ansiOutput,
new RegExp(
`\\u001B\\[31m\\[gitguardex\\]\\s+\\[fail\\] ${escapeRegexLiteral(readyBranch)}: rebase conflict in finish flow; run rebase --continue or rebase --abort in the source-probe worktree\\u001B\\[0m`,
`\\u001B\\[33m\\[gitguardex\\]\\s+\\[skip\\] ${escapeRegexLiteral(readyBranch)}: manual rebase required in the source-probe worktree; run rebase --continue or rebase --abort\\u001B\\[0m`,
),
);
assert.match(ansiOutput, /\u001B\[32m\[gitguardex\] ✅ Repo is fully safe\.\u001B\[0m/);
Expand Down