Skip to content

fix(codex): tolerate malformed account plan values - #1373

Merged
Wibias merged 1 commit into
lidge-jun:devfrom
luvs01:agent/fix-codex-plan-normalization
Aug 10, 2026
Merged

fix(codex): tolerate malformed account plan values#1373
Wibias merged 1 commit into
lidge-jun:devfrom
luvs01:agent/fix-codex-plan-normalization

Conversation

@luvs01

@luvs01 luvs01 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • centralize Codex plan normalization so persisted or upstream non-string values degrade to an unknown weekly plan instead of throwing
  • apply the boundary across auth DTO/import, collision checks, quota windows and recovery, routing scores, provider capacity aggregation, and provider quota cache signatures
  • omit malformed plan metadata from public account/capacity responses while preserving exact case-insensitive go / free monthly behavior
  • add regression coverage through the public provider quota report path, including its cache key and aggregation result

Verification

  • bun run typecheck
  • bun run privacy:scan
  • focused malformed-plan tests across auth API, collision, quota recovery, routing, capacity, and provider quota reports
  • bun test tests/codex-auth-collision.test.ts (7 passed)
  • bun test tests/provider-quota.test.ts -t "pool reports tolerate a malformed persisted plan" (1 passed)
  • git diff --check
  • attempted the full root suite; it was not green because unrelated Windows user-identity/catalog fixtures and existing shared-state/timing tests failed, so the run was stopped after the changed paths' tests passed

Checklist

  • The change is rebased onto the latest dev
  • Regression tests cover malformed persisted and upstream plan values
  • No GUI files or credential values are changed
  • Full local CI is green

Review readiness checklist

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of invalid or unexpected Codex plan values.
    • Normalized plan names consistently across account imports, quota calculations, routing, and capacity selection.
    • Preserved quota information while excluding malformed plans from aggregation and account metadata.
    • Prevented invalid plan data from causing incorrect account collisions or quota-window classification.
  • Tests

    • Added regression coverage for malformed plans across account management, quota recovery, routing, and provider capacity behavior.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4e9808c7-b4b7-4c8c-a99f-4d47a213ce33

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
📝 Walkthrough

Walkthrough

Changes

Codex plan normalization

Layer / File(s) Summary
Shared plan contract
src/codex/plan.ts
Adds codexPlanValue, codexPlanKey, and isThirtyDayOnlyCodexPlan for validating, normalizing, and classifying plan values.
Authentication and quota classification
src/codex/auth-api.ts, src/codex/auth-collision.ts, src/codex/quota.ts, src/codex/routing.ts, tests/codex-auth-api.test.ts, tests/codex-auth-collision.test.ts, tests/codex-cooldown-recovery.test.ts, tests/codex-routing.test.ts
Authentication and quota paths accept unknown plan inputs, normalize valid values, omit malformed plans, and classify invalid plans as weekly or unknown.
Provider aggregation and cache normalization
src/providers/codex-capacity.ts, src/providers/quota.ts, tests/provider-capacity.test.ts, tests/provider-quota.test.ts
Provider aggregation and cache signatures use normalized plan keys. Invalid plans are excluded from weighted aggregation and omitted from account metadata while raw quota remains available.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: lidge-jun, wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: handling malformed Codex account plan values.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/codex/auth-api.ts, src/codex/auth-collision.ts.

@github-actions github-actions Bot added the bug Something isn't working label Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

UI screenshot waived by the gui-screenshot-waived label.
This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently. If no CodeRabbit review appears, comment @coderabbitai review to request one.
Maintainers: @lidge-jun @Ingwannu @Wibias

Hygiene

Deterministic PR hygiene checks passed.

Wibias commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Please rebase this PR onto the latest dev (e8ce2b93dd3ef8074d96dee407704c5dc06e2699) before merge, then rerun the exact-head CI checks.

@luvs01
luvs01 force-pushed the agent/fix-codex-plan-normalization branch from eefa41d to 095a9a5 Compare August 9, 2026 23:25

luvs01 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the current dev@d517161a; the rewritten head is 095a9a5b. Focused exact-head checks passed (auth-collision 7/7, malformed-plan provider-quota 1/1, typecheck, privacy scan, and diff check).

The only remaining failures are maintainer-owned gates: unsponsored_surface requires maintainer-sponsored, and the PR gate asks for a UI screenshot. This PR changes no GUI files, so could you apply gui-screenshot-waived as well if that is appropriate? I have kept the PR in Draft.

@Wibias
Wibias marked this pull request as ready for review August 9, 2026 23:45
@Wibias Wibias added the gui-screenshot-waived Maintainer waiver for false-positive GUI screenshot requirements label Aug 9, 2026
@luvs01

luvs01 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

@github-actions[bot] The PR gate reports that pull request #1373 is ready for review at head 095a9a5b7de0d651c4e7bca656ef9fb235d0fdae. I will review the current changes.

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@github-actions[bot] have exceeded the limit for the number of chat messages per hour. Please wait 48 minutes and 30 seconds before sending another message.

@Wibias Wibias removed the intake: hygiene-blocked Deterministic PR hygiene checks failed label Aug 10, 2026
@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@github-actions[bot] have exceeded the limit for the number of chat messages per hour. Please wait 25 minutes and 8 seconds before sending another message.

@Wibias Wibias added the maintainer-sponsored Maintainer sponsors this change to an auth, workflow, release, or dependency surface label Aug 10, 2026
@github-actions github-actions Bot removed the intake: hygiene-blocked Deterministic PR hygiene checks failed label Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@github-actions[bot] have exceeded the limit for the number of chat messages per hour. Please wait 16 minutes and 34 seconds before sending another message.

@Wibias
Wibias merged commit 4ef5350 into lidge-jun:dev Aug 10, 2026
38 of 50 checks passed

Wibias commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Thanks @luvs01 — nice hardening here. The centralized plan normalization keeps malformed persisted/upstream values from leaking into quota, routing, collision, and capacity behavior, and the regression coverage across those paths is solid. Merged — appreciate the careful work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working gui-screenshot-waived Maintainer waiver for false-positive GUI screenshot requirements maintainer-sponsored Maintainer sponsors this change to an auth, workflow, release, or dependency surface review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants