refactor: extract forecast command

[ndycode]

Summary

• extract the codex auth forecast implementation into a dedicated command module
• add direct unit coverage for forecast argument parsing and JSON output while preserving existing CLI behavior

What Changed

• added lib/codex-manager/commands/forecast.ts for forecast argument parsing, live-probe orchestration, JSON output, and display rendering
• updated lib/codex-manager.ts so runForecast(...) delegates to the extracted forecast command module through injected helpers and services
• added test/codex-manager-forecast-command.test.ts for help, invalid-option, and JSON-output coverage

Validation

• npm run test -- test/codex-manager-forecast-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 cae16c2 to restore the inline forecast implementation

Additional Notes

• this is intentionally stacked on the manager command-split train from the same worktree

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 codex auth forecast implementation from the monolithic codex-manager.ts into lib/codex-manager/commands/forecast.ts, consistent with the ongoing command-split refactor train. the delegation in runForecast is a clean passthrough and all injected helpers map 1-to-1 to the original call sites, so behavioral parity is preserved. the duplicate if (!arg) continue; guard from the original parser is also quietly fixed in the extracted version.

key points:

• formatQuotaSnapshotLine is a required dep for serializeForecastResults but is absent from the exported ForecastCommandDeps interface — it's bolted on as an intersection type at the runForecastCommand call site, which makes the public contract incomplete for any future implementors
• the per-account live-probe loop remains sequential (await inside for), meaning N-account runs are O(N) latency; the module's own concurrency test confirms no shared state exists, so a Promise.allSettled refactor would be safe and aligns with the existing parallel-probe.ts pattern
• vitest coverage is missing for the --live non-JSON display path (quota-detail inline rows) and the probeErrors rendering branch; given the 80% project-wide coverage threshold these should be added
• no windows filesystem or token safety regressions introduced — saveQuotaCache/loadQuotaCache remain injected deps, so EBUSY/EPERM retry responsibility stays with the caller as before; access tokens are scoped locally and not logged

Confidence Score: 4/5

• safe to merge with minor follow-ups; no behavioral regressions, no token or filesystem safety risks introduced
• the refactor is a clean extraction with 1-to-1 dep mapping and the original inline bugs (duplicate guard) are fixed. deducting one point for the incomplete ForecastCommandDeps interface contract and the missing vitest coverage on the live-display and probe-error branches, both of which are straightforward to fix before or after merge
• lib/codex-manager/commands/forecast.ts — formatQuotaSnapshotLine interface gap and sequential probe loop; test/codex-manager-forecast-command.test.ts — missing coverage for live non-JSON display and probeErrors paths

Important Files Changed

Filename	Overview
lib/codex-manager/commands/forecast.ts	new 469-line command module extracting forecast arg parsing, live-probe orchestration, serialization, and display rendering via injected deps; clean DI pattern but formatQuotaSnapshotLine is missing from the exported ForecastCommandDeps interface and the probe loop remains sequential
lib/codex-manager.ts	pure delegation refactor — removes ~270 lines of inline forecast logic and replaces runForecast body with a single runForecastCommand(args, { ...deps }) call; no behavioral change, all injected helpers match the original call sites exactly
test/codex-manager-forecast-command.test.ts	new 199-line vitest suite covering help, invalid-option, JSON output, concurrent-run isolation, and styled row segments; missing coverage for live non-JSON display path and the probeErrors rendering branch

Sequence Diagram

sequenceDiagram
    participant CLI as codex-manager.ts<br/>runForecast()
    participant CMD as forecast.ts<br/>runForecastCommand()
    participant Store as loadAccounts / setStoragePath
    participant Cache as loadQuotaCache / saveQuotaCache
    participant Refresh as queuedRefresh
    participant Probe as fetchCodexQuotaSnapshot
    participant Out as logInfo / logError

    CLI->>CMD: runForecastCommand(args, deps)
    CMD->>CMD: parseForecastArgs(args)
    alt --help
        CMD->>Out: printForecastUsage()
        CMD-->>CLI: return 0
    else parse error
        CMD->>Out: logError(message)
        CMD-->>CLI: return 1
    end
    CMD->>Store: setStoragePath(null)
    CMD->>Store: loadAccounts()
    Store-->>CMD: AccountStorageV3
    opt --live
        CMD->>Cache: loadQuotaCache()
        Cache-->>CMD: QuotaCacheData
    end
    loop each account (sequential)
        opt token expired
            CMD->>Refresh: queuedRefresh(refreshToken)
            Refresh-->>CMD: TokenResult
        end
        CMD->>Probe: fetchCodexQuotaSnapshot(...)
        Probe-->>CMD: CodexQuotaSnapshot
        CMD->>Cache: updateQuotaCacheForAccount(...)
    end
    CMD->>CMD: evaluateForecastAccounts / summarizeForecast / recommendForecastAccount
    alt --json
        CMD->>Cache: saveQuotaCache (if changed)
        CMD->>Out: logInfo(JSON.stringify(...))
    else display
        CMD->>Out: render per-account rows + recommendation
        CMD->>Cache: saveQuotaCache (if changed)
    end
    CMD-->>CLI: return 0

Loading

Comments Outside Diff (2)

1. lib/codex-manager/commands/forecast.ts, line 637-645 (link)

   module-level mutable dep is a concurrency bug

   deps_formatQuotaSnapshotLine is assigned at the top of every runForecastCommand call, then used later inside serializeForecastResults which is only invoked after several await points. in a concurrent scenario (e.g. two overlapping calls from integration tests, a future parallel-probe path, or the existing codex-manager-cli.test.ts suite running cases in the same process), the second call overwrites the module-level variable while the first is suspended at an await, so serializeForecastResults picks up the wrong formatQuotaSnapshotLine dep.

   serializeForecastResults should receive the formatter as a direct argument instead:

   function serializeForecastResults(
   	results: ForecastAccountResult[],
   	liveQuotaByIndex: Map<number, CodexQuotaSnapshot>,
   	refreshFailures: Map<number, TokenFailure>,
   	formatQuotaSnapshotLine: (snapshot: CodexQuotaSnapshot) => string,
   ): Array<...> {
   

   then pass deps.formatQuotaSnapshotLine at the call-site and drop the module-level variable entirely.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: lib/codex-manager/commands/forecast.ts
   Line: 637-645
   
   Comment:
   **module-level mutable dep is a concurrency bug**
   
   `deps_formatQuotaSnapshotLine` is assigned at the top of every `runForecastCommand` call, then used later inside `serializeForecastResults` which is only invoked after several `await` points. in a concurrent scenario (e.g. two overlapping calls from integration tests, a future parallel-probe path, or the existing `codex-manager-cli.test.ts` suite running cases in the same process), the second call overwrites the module-level variable while the first is suspended at an `await`, so `serializeForecastResults` picks up the wrong `formatQuotaSnapshotLine` dep.
   
   `serializeForecastResults` should receive the formatter as a direct argument instead:
   
   ```
   function serializeForecastResults(
   	results: ForecastAccountResult[],
   	liveQuotaByIndex: Map<number, CodexQuotaSnapshot>,
   	refreshFailures: Map<number, TokenFailure>,
   	formatQuotaSnapshotLine: (snapshot: CodexQuotaSnapshot) => string,
   ): Array<...> {
   ```
   
   then pass `deps.formatQuotaSnapshotLine` at the call-site and drop the module-level variable entirely.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

2. lib/codex-manager/commands/forecast.ts, line 830-832 (link)

   joinStyledSegments silently replaced with different join behavior

   the original inline implementation called joinStyledSegments(rowParts) (a dedicated ANSI-aware helper), then emitted the leading styled | separately. the new code removes joinStyledSegments from deps entirely and substitutes rowParts.join(deps.stylePromptText(" | ", "muted")). if joinStyledSegments joined with a plain space, or applied ANSI resets between segments, the rendered row will differ in terminal output. this is a silent behavior change that isn't covered by any of the new tests — they mock stylePromptText as an identity function so the difference would never surface.

   either add joinStyledSegments to ForecastCommandDeps and pass it through, or add a test that captures the full rendered row string so the exact separator can be verified.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: lib/codex-manager/commands/forecast.ts
   Line: 830-832
   
   Comment:
   **`joinStyledSegments` silently replaced with different join behavior**
   
   the original inline implementation called `joinStyledSegments(rowParts)` (a dedicated ANSI-aware helper), then emitted the leading styled `|` separately. the new code removes `joinStyledSegments` from deps entirely and substitutes `rowParts.join(deps.stylePromptText(" | ", "muted"))`. if `joinStyledSegments` joined with a plain space, or applied ANSI resets between segments, the rendered row will differ in terminal output. this is a silent behavior change that isn't covered by any of the new tests — they mock `stylePromptText` as an identity function so the difference would never surface.
   
   either add `joinStyledSegments` to `ForecastCommandDeps` and pass it through, or add a test that captures the full rendered row string so the exact separator can be verified.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/codex-manager/commands/forecast.ts
Line: 209-214

Comment:
**`formatQuotaSnapshotLine` belongs in `ForecastCommandDeps`**

`formatQuotaSnapshotLine` is a required dep for `serializeForecastResults` but is hung off the call-site as an intersection type rather than declared in the exported `ForecastCommandDeps` interface. any future implementor who types their deps object as `ForecastCommandDeps` will get a compile error at call time with no hint about the missing field. move it into the interface body alongside the other snapshot helpers.

```suggestion
export async function runForecastCommand(
	args: string[],
	deps: ForecastCommandDeps,
): Promise<number> {
```

and add to `ForecastCommandDeps`:

```ts
formatQuotaSnapshotLine: (snapshot: CodexQuotaSnapshot) => string;
```

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

---

This is a comment left during a code review.
Path: lib/codex-manager/commands/forecast.ts
Line: 253-256

Comment:
**Sequential live-probe loop — concurrency issue**

each account's `queuedRefresh` and `fetchCodexQuotaSnapshot` are `await`ed one-by-one inside a `for` loop. with N accounts this is O(N) wall-clock latency. the PR's own concurrent-run test demonstrates the module is free of shared state, so `Promise.all` / `Promise.allSettled` across accounts would be safe and is worth noting as a follow-up — especially given that the existing `parallel-probe.ts` module already has a pattern for this. no regression introduced here (same as the original inline code), but since this extraction is the right moment to improve it, calling it out explicitly.

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-forecast-command.test.ts
Line: 134-143

Comment:
**missing vitest coverage: live non-JSON display path and `probeErrors` rendering**

current suite covers `--help`, invalid option, `--json`, concurrent-run isolation, and styled row segments. two important branches have no coverage:

1. `--live` without `--json` — the `showQuotaDetails` rendering path (lines 846-850 of forecast.ts) where `liveQuotaByIndex` entries are printed inline per row. this is the path most users hit.
2. `probeErrors` display (lines 887-900 of forecast.ts) — reached when `fetchCodexQuotaSnapshot` throws. this controls whether error text leaks unformatted to the terminal, so it's worth a dedicated test that overrides `fetchCodexQuotaSnapshot` to throw and asserts `logInfo` received the `"Live check notes"` header.

80% coverage threshold is enforced project-wide; adding these two cases keeps the new module from pulling the per-file number below the bar.

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

_{Last reviewed commit: "fix: isolate forecas..."}

[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: 00ceb1e8-c60c-4a63-9c04-dde0a1ee959c

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-forecast-command

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch refactor/pr1-forecast-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.}

[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.

[ndycode]

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