From bc39a5a43e05b7a330ae83a2e337aad8eb21798e Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Tue, 21 Apr 2026 16:11:34 +0200 Subject: [PATCH] Make doctor failures visually obvious in terminal output Doctor already prints compact actionable status lines, but fail and success output blend together during long recursive runs. This colors semantic doctor lines only and supports FORCE_COLOR so ANSI coverage can be verified deterministically. Constraint: Human-readable doctor output must remain plain in non-color terminals Rejected: Add a dedicated doctor color flag | FORCE_COLOR already covers explicit ANSI opt-in without expanding the CLI surface Confidence: high Scope-risk: narrow Directive: Keep doctor status coloring scoped to semantic outcome lines; do not color every repair bullet without rechecking readability Tested: node --check bin/multiagent-safety.js; node --test --test-name-pattern "doctor" test/install.test.js; openspec validate agent-codex-doctor-status-colors-2026-04-21-15-58 --type change --strict; openspec validate --specs Not-tested: Full repo non-doctor test suite --- bin/multiagent-safety.js | 105 ++++++++++++++++-- .../.openspec.yaml | 2 + .../notes.md | 5 + .../proposal.md | 17 +++ .../specs/doctor-workflow/spec.md | 21 ++++ .../tasks.md | 23 ++++ test/install.test.js | 53 +++++++++ 7 files changed, 215 insertions(+), 11 deletions(-) create mode 100644 openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/.openspec.yaml create mode 100644 openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/notes.md create mode 100644 openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/proposal.md create mode 100644 openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/specs/doctor-workflow/spec.md create mode 100644 openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/tasks.md diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index 8a33b4d..b291bee 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -357,7 +357,17 @@ function runtimeVersion() { } function supportsAnsiColors() { - return Boolean(process.stdout.isTTY) && !process.env.NO_COLOR && process.env.TERM !== 'dumb'; + const forced = String(process.env.FORCE_COLOR || '').trim().toLowerCase(); + if (['0', 'false', 'no', 'off'].includes(forced)) { + return false; + } + if (forced.length > 0) { + return true; + } + if (process.env.NO_COLOR) { + return false; + } + return Boolean(process.stdout.isTTY) && process.env.TERM !== 'dumb'; } function colorize(text, colorCode) { @@ -367,6 +377,56 @@ function colorize(text, colorCode) { return `\u001B[${colorCode}m${text}\u001B[0m`; } +function doctorOutputColorCode(status) { + const normalized = String(status || '').trim().toLowerCase(); + if (['active', 'done', 'ok', 'safe', 'success'].includes(normalized)) { + return '32'; + } + if (normalized === 'disabled') { + return '36'; + } + if (['degraded', 'pending', 'skip', 'warn', 'warning'].includes(normalized)) { + return '33'; + } + if (['error', 'fail', 'inactive', 'unsafe'].includes(normalized)) { + return '31'; + } + return null; +} + +function colorizeDoctorOutput(text, status) { + const colorCode = doctorOutputColorCode(status); + return colorCode ? colorize(text, colorCode) : text; +} + +function detectAutoFinishDetailStatus(detail) { + const trimmed = String(detail || '').trim(); + const match = trimmed.match(/^\[(\w+)\]/); + if (match) { + return match[1].toLowerCase(); + } + if (/^Skipped\b/i.test(trimmed) || /^No local agent branches found\b/i.test(trimmed)) { + return 'skip'; + } + return null; +} + +function detectAutoFinishSummaryStatus(summary) { + if (!summary || summary.enabled === false) { + return detectAutoFinishDetailStatus(summary?.details?.[0]); + } + if ((summary.failed || 0) > 0) { + return 'fail'; + } + if ((summary.completed || 0) > 0) { + return 'done'; + } + if ((summary.skipped || 0) > 0) { + return 'skip'; + } + return null; +} + function statusDot(status) { if (status === 'active') { return colorize('●', '32'); // green @@ -604,22 +664,29 @@ function printAutoFinishSummary(summary, options = {}) { if (enabled) { console.log( - `[${TOOL_NAME}] Auto-finish sweep (base=${baseBranch}): attempted=${summary.attempted}, completed=${summary.completed}, skipped=${summary.skipped}, failed=${summary.failed}`, + colorizeDoctorOutput( + `[${TOOL_NAME}] Auto-finish sweep (base=${baseBranch}): attempted=${summary.attempted}, completed=${summary.completed}, skipped=${summary.skipped}, failed=${summary.failed}`, + detectAutoFinishSummaryStatus(summary), + ), ); const visibleDetails = verbose ? details : details.slice(0, detailLimit).map(summarizeAutoFinishDetail); for (const detail of visibleDetails) { - console.log(`[${TOOL_NAME}] ${detail}`); + console.log(colorizeDoctorOutput(`[${TOOL_NAME}] ${detail}`, detectAutoFinishDetailStatus(detail))); } if (!verbose && details.length > detailLimit) { console.log( - `[${TOOL_NAME}] … ${details.length - detailLimit} more branch result(s). Re-run with --verbose-auto-finish for full details.`, + colorizeDoctorOutput( + `[${TOOL_NAME}] … ${details.length - detailLimit} more branch result(s). Re-run with --verbose-auto-finish for full details.`, + 'warn', + ), ); } return; } if (details.length > 0) { - console.log(`[${TOOL_NAME}] ${verbose ? details[0] : summarizeAutoFinishDetail(details[0])}`); + const detail = verbose ? details[0] : summarizeAutoFinishDetail(details[0]); + console.log(colorizeDoctorOutput(`[${TOOL_NAME}] ${detail}`, detectAutoFinishDetailStatus(detail))); } } @@ -5043,21 +5110,34 @@ function printScanResult(scan, json = false) { if (scan.guardexEnabled === false) { console.log( - `[${TOOL_NAME}] Guardex is disabled for this repo (${describeGuardexRepoToggle(scan.guardexToggle)}).`, + colorizeDoctorOutput( + `[${TOOL_NAME}] Guardex is disabled for this repo (${describeGuardexRepoToggle(scan.guardexToggle)}).`, + 'disabled', + ), ); return; } if (scan.findings.length === 0) { - console.log(`[${TOOL_NAME}] ✅ No safety issues detected.`); + console.log(colorizeDoctorOutput(`[${TOOL_NAME}] ✅ No safety issues detected.`, 'safe')); return; } for (const item of scan.findings) { const target = item.path ? ` (${item.path})` : ''; - console.log(`[${item.level.toUpperCase()}] ${item.code}${target}: ${item.message}`); + console.log( + colorizeDoctorOutput( + `[${item.level.toUpperCase()}] ${item.code}${target}: ${item.message}`, + item.level, + ), + ); } - console.log(`[${TOOL_NAME}] Summary: ${scan.errors} error(s), ${scan.warnings} warning(s).`); + console.log( + colorizeDoctorOutput( + `[${TOOL_NAME}] Summary: ${scan.errors} error(s), ${scan.warnings} warning(s).`, + scan.errors > 0 ? 'error' : 'warn', + ), + ); } function setExitCodeFromScan(scan) { @@ -5498,10 +5578,13 @@ function doctor(rawArgs) { verbose: singleRepoOptions.verboseAutoFinish, }); if (safe) { - console.log(`[${TOOL_NAME}] ✅ Repo is fully safe.`); + console.log(colorizeDoctorOutput(`[${TOOL_NAME}] ✅ Repo is fully safe.`, 'safe')); } else { console.log( - `[${TOOL_NAME}] ⚠️ Repo is not fully safe yet (${scanResult.errors} error(s), ${scanResult.warnings} warning(s)).`, + colorizeDoctorOutput( + `[${TOOL_NAME}] ⚠️ Repo is not fully safe yet (${scanResult.errors} error(s), ${scanResult.warnings} warning(s)).`, + scanResult.errors > 0 ? 'unsafe' : 'warn', + ), ); } setExitCodeFromScan(scanResult); diff --git a/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/.openspec.yaml b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/.openspec.yaml new file mode 100644 index 0000000..4b8c565 --- /dev/null +++ b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-21 diff --git a/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/notes.md b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/notes.md new file mode 100644 index 0000000..f85e383 --- /dev/null +++ b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/notes.md @@ -0,0 +1,5 @@ +# T1 Notes + +- Color `gx doctor` failure lines red so blocked auto-finish rows are visible in long recursive runs. +- Color doctor success lines green, including `No safety issues detected.` and `Repo is fully safe.`, while keeping non-TTY output unchanged. +- Add a regression that forces ANSI output and proves doctor renders colors for both failure and success status lines. diff --git a/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/proposal.md b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/proposal.md new file mode 100644 index 0000000..3005336 --- /dev/null +++ b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/proposal.md @@ -0,0 +1,17 @@ +## Why + +- `gx doctor` already prints compact actionable statuses, but the success and failure lines all use the same default terminal color and are easy to miss in long recursive runs. +- Auto-finish failures are the most actionable doctor output, yet they visually blend into the surrounding safe scan output. +- The CLI needs a deterministic way to emit ANSI colors during automated verification so status-color regressions can be tested. + +## What Changes + +- Color human-readable `gx doctor` success lines green. +- Color doctor failure lines red and skip/pending lines yellow. +- Honor the standard `FORCE_COLOR` environment variable so ANSI output can be verified in tests without changing non-color output defaults. + +## Impact + +- Affects only the human-readable doctor/status CLI output when ANSI colors are enabled. +- JSON output and non-color terminals remain unchanged. +- Main risk: over-coloring could reduce readability, so the change stays scoped to doctor scan/final status lines and auto-finish summary/detail rows. diff --git a/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/specs/doctor-workflow/spec.md b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/specs/doctor-workflow/spec.md new file mode 100644 index 0000000..ee1405f --- /dev/null +++ b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/specs/doctor-workflow/spec.md @@ -0,0 +1,21 @@ +## ADDED Requirements + +### Requirement: `gx doctor` uses semantic status colors +When ANSI color output is enabled, the human-readable `gx doctor` workflow SHALL color success lines green, failure lines red, and skip or pending lines yellow. + +#### Scenario: safe doctor lines render green +- **GIVEN** `gx doctor` runs in human-readable mode with ANSI color output enabled +- **WHEN** the repo scan reports `No safety issues detected.` and doctor reaches `Repo is fully safe.` +- **THEN** both success lines SHALL be emitted in green + +#### Scenario: doctor auto-finish failures render red +- **GIVEN** `gx doctor` runs in human-readable mode with ANSI color output enabled +- **AND** the auto-finish sweep reports at least one failed branch result +- **WHEN** doctor prints the auto-finish summary and failed branch detail +- **THEN** the failure summary line SHALL be emitted in red +- **AND** the failed branch detail line SHALL be emitted in red + +#### Scenario: doctor skip or pending lines render yellow +- **GIVEN** `gx doctor` runs in human-readable mode with ANSI color output enabled +- **WHEN** doctor prints a skipped or pending auto-finish line +- **THEN** that line SHALL be emitted in yellow diff --git a/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/tasks.md b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/tasks.md new file mode 100644 index 0000000..87cbc48 --- /dev/null +++ b/openspec/changes/agent-codex-doctor-status-colors-2026-04-21-15-58/tasks.md @@ -0,0 +1,23 @@ +## 1. Specification + +- [x] 1.1 Define the doctor status-color requirement for human-readable output. + +## 2. Implementation + +- [x] 2.1 Color doctor success/failure/pending lines with semantic ANSI colors when color output is enabled. +- [x] 2.2 Add a regression that forces ANSI output and checks both red failure lines and green success lines. + +## 3. Verification + +- [x] 3.1 Run `node --check bin/multiagent-safety.js`. +- [x] 3.2 Run `node --test --test-name-pattern "doctor" test/install.test.js`. +- [x] 3.3 Run `openspec validate agent-codex-doctor-status-colors-2026-04-21-15-58 --type change --strict`. +- [x] 3.4 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 doctor-focused tests, including the new forced-color regression. `openspec validate agent-codex-doctor-status-colors-2026-04-21-15-58 --type change --strict` passed, and `openspec validate --specs` returned `No items found to validate.` + +## 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 --base --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. diff --git a/test/install.test.js b/test/install.test.js index 9717db2..96f96e7 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -1526,6 +1526,59 @@ exit 1 assert.match(verboseOutput, /git -C ".+rebase --continue/); }); +test('doctor colors failure and success status lines when color output is enabled', () => { + const repoDir = initRepoOnBranch('main'); + seedCommit(repoDir); + attachOriginRemoteForBranch(repoDir, 'main'); + const { readyBranch, readyWorktree, fileName } = prepareDoctorAutoFinishReadyBranch(repoDir, { + taskName: 'doctor-color-status', + fileName: 'doctor-color-status.txt', + }); + + let result = runCmd('git', ['worktree', 'remove', readyWorktree, '--force'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + fs.writeFileSync(path.join(repoDir, fileName), 'main branch conflicting color change\n', 'utf8'); + result = runCmd('git', ['add', fileName], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'main branch conflicting color change'], repoDir, { + ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1', + }); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['push', 'origin', 'main'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const { fakePath: fakeGhPath } = createFakeGhScript(` +if [[ "$1" == "--version" ]]; then + echo "gh version 2.0.0" + exit 0 +fi +echo "unexpected gh args: $*" >&2 +exit 1 +`); + + result = runNodeWithEnv( + ['doctor', '--target', repoDir, '--allow-protected-base-write'], + repoDir, + { GUARDEX_GH_BIN: fakeGhPath, FORCE_COLOR: '1' }, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const ansiOutput = `${result.stdout}\n${result.stderr}`; + 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/, + ); + 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`, + ), + ); + assert.match(ansiOutput, /\u001B\[32m\[gitguardex\] ✅ Repo is fully safe\.\u001B\[0m/); +}); + test('setup pre-commit blocks codex session commits on non-agent branches by default', () => { const repoDir = initRepo();