Skip to content

feat: add live probe budgets and warnings#376

Closed
ndycode wants to merge 1 commit intomainfrom
feat/live-probe-budgets-and-warnings
Closed

feat: add live probe budgets and warnings#376
ndycode wants to merge 1 commit intomainfrom
feat/live-probe-budgets-and-warnings

Conversation

@ndycode
Copy link
Copy Markdown
Owner

@ndycode ndycode commented Apr 6, 2026

Summary

  • add --max-accounts, --max-probes, and --cached-only controls to codex auth report --live
  • surface live probe budgets and actual considered/executed counts in both human-readable and JSON output
  • add focused report-command coverage for invalid budget values, enforced limits, and cached-only behavior

Validation

  • node_modules/.bin/vitest.cmd run test/codex-manager-report-command.test.ts
  • node_modules/.bin/tsc.cmd --noEmit

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

adds --max-accounts, --max-probes, and --cached-only flags to codex auth report --live, surfacing budget counters in both human-readable and JSON output with focused vitest coverage for budget enforcement, invalid-value rejection, and cached-only behaviour. the implementation is clean and the new tests are well-scoped; one ordering detail means token refresh can fire for an account that immediately hits the probe budget on the very next check.

Confidence Score: 5/5

safe to merge; all findings are P2 style/efficiency suggestions with no correctness or data-integrity impact

the budget logic produces correct output, the new tests cover the critical paths, and the one ordering concern (refresh before probe budget check) is a minor efficiency issue that does not affect report accuracy or cause data loss

lib/codex-manager/commands/report.ts — review the maxProbes check placement relative to queuedRefresh to avoid unnecessary refreshes on windows

Important Files Changed

Filename Overview
lib/codex-manager/commands/report.ts adds --max-accounts/--max-probes/--cached-only parsing and enforcement; probe budget check fires after queuedRefresh, creating an orphaned refresh+persist when maxProbes is set
test/codex-manager-report-command.test.ts adds budget-enforcement, cached-only, and liveProbeBudget field tests; missing coverage for --max-accounts invalid value and the = inline parsing form

Sequence Diagram

sequenceDiagram
    participant CLI as CLI args
    participant Parser as parseReportArgs
    participant Loop as Account Loop
    participant Refresh as queuedRefresh
    participant Probe as fetchCodexQuotaSnapshot

    CLI->>Parser: --live --max-accounts N --max-probes M --cached-only
    Parser-->>Loop: options{maxAccounts, maxProbes, cachedOnly}

    loop for each account in storage
        Loop->>Loop: check maxAccounts budget (break if hit)
        Loop->>Loop: skip disabled accounts
        Loop->>Loop: consideredLiveAccounts += 1
        alt token not usable AND NOT cachedOnly
            Loop->>Refresh: queuedRefresh(refreshToken)
            Note right of Refresh: network call + temp file write (windows risk)
            Refresh-->>Loop: TokenResult
            Loop->>Loop: persist refreshed token to disk
        else cachedOnly AND token not usable
            Loop->>Loop: push skip error, continue
        end
        Loop->>Loop: check missing accountId (continue if missing)
        Loop->>Loop: check maxProbes budget (break if hit)
        Note over Loop: ⚠ refresh already ran for this account
        Loop->>Probe: fetchCodexQuotaSnapshot(accountId, accessToken, model)
        Probe-->>Loop: CodexQuotaSnapshot
        Loop->>Loop: executedLiveProbes += 1
    end

    Loop-->>CLI: report{liveProbeBudget{consideredAccounts, executedProbes}}
Loading

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: lib/codex-manager/commands/report.ts
Line: 433-523

Comment:
**Refresh fires before `--max-probes` budget check**

when `--max-probes=N` and accounts need refreshing, account N+1 executes `queuedRefresh` (network call + temp-file write on windows) before the probe-budget guard fires and breaks the loop. on windows, an antivirus-held temp handle can trigger EPERM on the next write — per project convention, unnecessary temp files are a real risk.

moving the `maxProbes` check above the `hasUsableAccessToken` block prevents the orphaned refresh:

```typescript
// check probe budget before attempting any refresh
if (
	typeof options.maxProbes === "number" &&
	executedLiveProbes >= options.maxProbes
) {
	probeErrors.push(`live probe request budget reached (${options.maxProbes})`);
	break;
}

let probeAccessToken = account.accessToken;
let probeAccountId = account.accountId ?? extractAccountId(account.accessToken);
if (!deps.hasUsableAccessToken(account, now)) {
	// ... refresh logic
}
```

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-report-command.test.ts
Line: 84-93

Comment:
**Missing coverage for `--max-accounts` invalid value**

only `--max-probes 0` is tested for budget rejection; `--max-accounts 0` (and the `=`-inline form `--max-accounts=0`) have no parallel test. the two parsing paths are symmetrical — add a companion case to hit that validation branch and confirm it surfaces the right error message.

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/report.ts
Line: 583-590

Comment:
**`liveProbeBudget` emitted unconditionally in non-live reports**

the field is always serialized even when `options.live` is `false`, so every non-live JSON report carries `{ cachedOnly: false, maxAccounts: null, maxProbes: null, consideredAccounts: 0, executedProbes: 0 }`. gating it avoids schema noise for consumers that never use live mode:

```suggestion
		liveProbeBudget: options.live
			? {
					cachedOnly: options.cachedOnly,
					maxAccounts: options.maxAccounts ?? null,
					maxProbes: options.maxProbes ?? null,
					consideredAccounts: consideredLiveAccounts,
					executedProbes: executedLiveProbes,
			  }
			: undefined,
```

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

Reviews (1): Last reviewed commit: "feat: add live probe budgets and warning..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 6, 2026

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 28 minutes and 22 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 22 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 389f4bb0-c25a-4eb0-a7c8-a578c7dc3395

📥 Commits

Reviewing files that changed from the base of the PR and between 478f44c and 26cbb41.

📒 Files selected for processing (2)
  • lib/codex-manager/commands/report.ts
  • test/codex-manager-report-command.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/live-probe-budgets-and-warnings
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/live-probe-budgets-and-warnings

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

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

@chatgpt-codex-connector
Copy link
Copy Markdown

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.

Comment on lines 433 to +523
@@ -444,8 +511,16 @@ export async function runReportCommand(
);
continue;
}
if (
typeof options.maxProbes === "number" &&
executedLiveProbes >= options.maxProbes
) {
probeErrors.push(`live probe request budget reached (${options.maxProbes})`);
break;
}

try {
executedLiveProbes += 1;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Refresh fires before --max-probes budget check

when --max-probes=N and accounts need refreshing, account N+1 executes queuedRefresh (network call + temp-file write on windows) before the probe-budget guard fires and breaks the loop. on windows, an antivirus-held temp handle can trigger EPERM on the next write — per project convention, unnecessary temp files are a real risk.

moving the maxProbes check above the hasUsableAccessToken block prevents the orphaned refresh:

// check probe budget before attempting any refresh
if (
	typeof options.maxProbes === "number" &&
	executedLiveProbes >= options.maxProbes
) {
	probeErrors.push(`live probe request budget reached (${options.maxProbes})`);
	break;
}

let probeAccessToken = account.accessToken;
let probeAccountId = account.accountId ?? extractAccountId(account.accessToken);
if (!deps.hasUsableAccessToken(account, now)) {
	// ... refresh logic
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/codex-manager/commands/report.ts
Line: 433-523

Comment:
**Refresh fires before `--max-probes` budget check**

when `--max-probes=N` and accounts need refreshing, account N+1 executes `queuedRefresh` (network call + temp-file write on windows) before the probe-budget guard fires and breaks the loop. on windows, an antivirus-held temp handle can trigger EPERM on the next write — per project convention, unnecessary temp files are a real risk.

moving the `maxProbes` check above the `hasUsableAccessToken` block prevents the orphaned refresh:

```typescript
// check probe budget before attempting any refresh
if (
	typeof options.maxProbes === "number" &&
	executedLiveProbes >= options.maxProbes
) {
	probeErrors.push(`live probe request budget reached (${options.maxProbes})`);
	break;
}

let probeAccessToken = account.accessToken;
let probeAccountId = account.accountId ?? extractAccountId(account.accessToken);
if (!deps.hasUsableAccessToken(account, now)) {
	// ... refresh logic
}
```

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

Fix in Codex

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Apr 6, 2026

Superseded by #387, which rebuilds the full open PR stack onto one reviewed integration branch.

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Apr 6, 2026

Closing in favor of #387.

@ndycode ndycode closed this Apr 6, 2026
@ndycode ndycode deleted the feat/live-probe-budgets-and-warnings branch April 12, 2026 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant