refactor: extract report command#153
refactor: extract report command#153ndycode wants to merge 4 commits intorefactor/pr1-check-commandfrom
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
Summary
codex auth reportimplementation into a dedicated command moduleWhat Changed
lib/codex-manager/commands/report.tswith report argument parsing, output generation, and file-write handlinglib/codex-manager.tsso thereportpath delegates to the extracted command moduletest/codex-manager-report-command.test.tsfor help, invalid-option, and JSON/file-output coverageValidation
npm run lintnpm run typechecknpm run test -- test/codex-manager-report-command.test.ts test/codex-manager-check-command.test.ts test/codex-manager-status-command.test.ts test/codex-manager-switch-command.test.ts test/codex-manager-cli.test.tsnpm run buildRisk and Rollback
8368528to restore the inline report implementationAdditional Notes
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
extracts the
codex auth reportimplementation from the monolithiccodex-manager.tsintolib/codex-manager/commands/report.ts, following the same command-split pattern used forcheckandstatus. the dep-injection approach (ReportCommandDeps) makes the new module fully unit-testable without touching the real filesystem or network.key changes:
defaultWriteFilenow uses a temp-file + atomic rename pattern with EBUSY/EPERM retry on the rename — resolving both prior review concerns (bare write and temp-file leak)serializeForecastResultsis duplicated betweencodex-manager.ts(still used byrunForecast) and the newreport.ts; a shared utility would be cleaner but is out of scope for this PRRETRYABLE_WRITE_CODEScoversEBUSY/EPERMbut omitsEAGAIN, which is inconsistent with theunified-settings.tsconventiondefaultWriteFileretry logic has no vitest coverage — all tests inject a mockwriteFile, leaving the windows-safety path unexercisedConfidence Score: 4/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant CLI as codex-manager.ts participant CMD as runReportCommand participant DEPS as injected deps participant FS as defaultWriteFile CLI->>CMD: runReportCommand(rest, deps) CMD->>DEPS: setStoragePath(null) CMD->>DEPS: getStoragePath() CMD->>DEPS: loadAccounts() opt --live flag loop each enabled account CMD->>DEPS: queuedRefresh(refreshToken) CMD->>DEPS: fetchCodexQuotaSnapshot(...) end end CMD->>CMD: evaluateForecastAccounts(...) CMD->>CMD: build report object opt --out flag alt writeFile dep provided CMD->>DEPS: writeFile(outputPath, json) else default CMD->>FS: defaultWriteFile(outputPath, json) FS->>FS: fs.mkdir(dirname) FS->>FS: fs.writeFile(tempPath) loop up to 5 rename attempts (EBUSY/EPERM retry) FS->>FS: fs.rename(tempPath → path) end end end CMD-->>CLI: exit code (0 or 1)Prompt To Fix All With AI
Reviews (4): Last reviewed commit: "test: harden report command path asserti..." | Re-trigger Greptile
Context used: