refactor: extract doctor command

[ndycode]

Summary

• extract the codex auth doctor implementation into a dedicated command module
• add direct unit coverage for help, invalid-option handling, and JSON diagnostics while preserving the existing doctor CLI behavior

What Changed

• added lib/codex-manager/commands/doctor.ts for doctor diagnostics, file checks, summary generation, and fix-mode reporting
• updated lib/codex-manager.ts so runDoctor(...) delegates to the extracted doctor command module through injected dependencies
• added test/codex-manager-doctor-command.test.ts for command-level coverage

Validation

• npm run test -- test/codex-manager-doctor-command.test.ts test/codex-manager-cli.test.ts
• npm run lint
• npm run typecheck
• npm run build

Risk and Rollback

• Risk level: medium-low
• Rollback plan: revert 81fd616 to restore the inline doctor implementation

Additional Notes

• this keeps the manager split progressing while preserving the existing doctor regression coverage in the CLI suite

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this PR extracts the inline runDoctor implementation from lib/codex-manager.ts into a dedicated lib/codex-manager/commands/doctor.ts command module, continuing the manager split pattern established by best, check, forecast, and report. the two regressions flagged in the previous round (fix-mode token refresh+sync being dropped, and the null guard on auth-file parse) are both correctly implemented in the new module.

• runDoctorCommand correctly carries forward all fix-mode behavior: applyDoctorFixes, queuedRefresh, setCodexCliActiveSelection, and the single-call saveAccounts guard (storageNeedsSave && !options.dryRun)
• DoctorFixAction interface was not cleaned up from codex-manager.ts — a local non-exported copy remains alongside the canonical exported one in doctor.ts, creating a structural duplicate that could silently diverge
• test cleanup uses bare rmSync which violates the project's windows filesystem safety anti-pattern; test/helpers/remove-with-retry.ts should be used instead
• vitest coverage is missing for the dry-run fix path and the queuedRefresh failure path — both touch windows token-file write branches

Confidence Score: 4/5

• safe to merge after addressing the duplicate DoctorFixAction and bare rmSync; no behavioral regressions introduced
• the two previously-flagged behavioral regressions are resolved. remaining issues are a maintenance-hazard type duplicate, a windows-safety anti-pattern in tests, and missing coverage for two branches — none are runtime blockers, but the rmSync issue can cause flaky CI on windows
• lib/codex-manager.ts (DoctorFixAction cleanup), test/codex-manager-doctor-command.test.ts (rmSync + missing dry-run / refresh-failure tests)

Important Files Changed

Filename	Overview
lib/codex-manager/commands/doctor.ts	new doctor command module — fix-mode refresh+sync and null-guard are both correctly implemented; minor: DoctorFixAction is duplicated with codex-manager.ts rather than being the single source of truth
lib/codex-manager.ts	delegation to runDoctorCommand is clean; DoctorSeverity/DoctorCheck/DoctorCliOptions removed correctly, but local DoctorFixAction interface not cleaned up — still duplicated alongside the exported version in doctor.ts
test/codex-manager-doctor-command.test.ts	good coverage of help, invalid-option, json output, invalid auth shape, and refresh+sync happy path; bare rmSync violates AGENTS.md windows safety policy; dry-run and queuedRefresh-failure branches are untested

Sequence Diagram

sequenceDiagram
    participant CLI as codex-manager.ts<br/>runDoctor()
    participant CMD as doctor.ts<br/>runDoctorCommand()
    participant FS as node:fs<br/>(existsSync / stat / readFile)
    participant DEPS as injected deps<br/>(loadAccounts / queuedRefresh / etc.)

    CLI->>CMD: runDoctorCommand(args, deps)
    CMD->>CMD: parse args / help check
    CMD->>FS: existsSync(storagePath)
    CMD->>FS: existsSync(codexAuthPath)
    CMD->>FS: readFile(codexAuthPath) → null-guard parse
    CMD->>DEPS: loadCodexCliState()
    CMD->>DEPS: loadAccounts()
    alt options.fix && accounts exist
        CMD->>DEPS: applyDoctorFixes(storage)
        alt !hasUsableAccessToken && !dryRun
            CMD->>DEPS: queuedRefresh(refreshToken)
            DEPS-->>CMD: TokenResult
        end
        alt !dryRun
            CMD->>DEPS: setCodexCliActiveSelection(...)
        end
    end
    alt storageNeedsSave && !dryRun
        CMD->>DEPS: saveAccounts(storage)
    end
    CMD-->>CLI: exit code (0 / 1)

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/codex-manager.ts
Line: 3103-3106

Comment:
**Duplicate `DoctorFixAction` not cleaned up**

`interface DoctorFixAction` is now defined in both `lib/codex-manager.ts` (local, unexported) and `lib/codex-manager/commands/doctor.ts` (exported). the `applyDoctorFixes` function here uses the local copy, not the canonical one. they're structurally identical today, but this creates a maintenance hazard — if `DoctorFixAction` gains a new field in `doctor.ts` (e.g. a `detail` property), `applyDoctorFixes` here won't be updated automatically and the type mismatch will only surface at the structural boundary when wired into `DoctorCommandDeps`.

import `DoctorFixAction` from `./codex-manager/commands/doctor.js` and remove the local definition, the same way `DoctorSeverity`, `DoctorCheck`, and `DoctorCliOptions` were cleaned up in this PR.

```suggestion
import {
	type DoctorCliOptions,
	type DoctorFixAction,
	runDoctorCommand,
} from "./codex-manager/commands/doctor.js";
```

then delete the local `interface DoctorFixAction` block at line 3103–3106.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/codex-manager-doctor-command.test.ts
Line: 86

Comment:
**Bare `rmSync` — windows antivirus / EBUSY risk**

the project's anti-pattern list (`AGENTS.md`, `test/AGENTS.md`) explicitly bans bare `fs.rm` / `rmSync` in test cleanup because windows antivirus and AV scan-on-close can hold the directory open, causing `EBUSY` / `EPERM` / `ENOTEMPTY` on CI. `test/helpers/remove-with-retry.ts` already exists and is used in `codex-cli-sync.test.ts`, `config-save.test.ts`, `dashboard-settings.test.ts`, and `rotation-integration.test.ts` for this exact reason.

```suggestion
		cleanup: () => { void removeWithRetry(root); },
```

add the import at the top of the file:

```ts
import { removeWithRetry } from "./helpers/remove-with-retry.js";
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/codex-manager-doctor-command.test.ts
Line: 149-237

Comment:
**Missing vitest coverage: dry-run path and refresh-failure path**

the new test suite covers help, invalid-option, json output, invalid auth JSON shape, and the happy-path refresh+sync. two critical branches in `runDoctorCommand` are still uncovered:

1. **dry-run fix mode** (`options.fix && options.dryRun`): lines 427–432 and 491–497 of `doctor.ts` push synthetic fix actions without calling `queuedRefresh` or `setCodexCliActiveSelection`. there's no test that asserts those deps are NOT called under `--dry-run`, nor that `saveAccounts` is skipped.

2. **`queuedRefresh` failure path** (`refreshResult.type !== "success"`): lines 456–465 of `doctor.ts` add a `"doctor-refresh"` check with severity `"warn"` and skip the codex sync. the default `createDeps()` mock already returns `{ type: "failed" }` from `queuedRefresh`, but no test exercises that path with `hasUsableAccessToken: () => false` + `fix: true` to assert the warn check is emitted and `setCodexCliActiveSelection` is not called.

both paths involve windows token-file writes, so skipping them leaves the EBUSY / partial-write edge-cases untested on this platform.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "fix: sync doctor act..."}

Context used (4)

• Context used - AGENTS.md (source)
• Context used - test/AGENTS.md (source)
• Context used - lib/AGENTS.md (source)
• Context used - speak in lowercase, concise sentences. act like th... (source)

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

• skip-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fdc1ff85-7c40-473c-9e7d-18cb75138a0c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/pr1-doctor-command

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch refactor/pr1-doctor-command

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ndycode]

Closing because this work is now included in main via #318 and #319.