Repository navigation
fix: harden quota system — code review fixes + build fix - #214
Merged
diegosouzapw merged 2 commits intoMar 6, 2026
Merged
diegosouzapw merged 2 commits into
diegosouzapw merged 2 commits into
Conversation
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the robustness and code quality of the quota-aware account selection system. It addresses several bugs, enhances concurrency handling, and introduces better coding practices. Additionally, it fixes a build issue related to Tailwind CSS. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
- Move setModelUnavailable from per-account loop to all-accounts-exhausted path - Clear model unavailability on successful fallback - Add in-memory quota cache with background refresh (5min active, 20min exhausted) - Integrate quota cache in account selection to skip exhausted accounts - Mark accounts as exhausted from 429 when no cached quota data exists - Populate quota cache from dashboard usage endpoint
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No New Issues Found | Recommendation: Merge OverviewThe PR implements a comprehensive quota-aware account selection system with background refresh. Most issues from the initial review have been addressed:
Issues Previously Raised & Resolution
Key Changes Reviewed
Files Reviewed (11 files)
The implementation is solid. The quota-aware routing should significantly improve reliability by avoiding exhausted accounts. Background refresh ensures quota data stays current. The code follows existing patterns and is well-documented. |
DavyMassoneto
force-pushed
the
fix/claude-oauth-usage-endpoint
branch
from
March 5, 2026 21:51
d7c4b59 to
93029c6
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces significant hardening and fixes for the quota management system. The changes are well-structured, including centralizing constants, adding a robust in-memory quota cache with background refresh, and making the account selection process quota-aware. The build fix for Tailwind and other code quality improvements are also valuable. I found one potential issue in the handling of 429 responses that could lead to using stale cache data.
DavyMassoneto
force-pushed
the
fix/claude-oauth-usage-endpoint
branch
2 times, most recently
from
March 5, 2026 22:30
53c25b2 to
45a961c
Compare
…remature model unavailability - Fix inverted Claude OAuth utilization (remaining, not used) - Add hasUtilization() guard to prevent false exhaustion from empty responses - Centralize anthropic-version into CLAUDE_CONFIG.apiVersion - Add parseDate() for safe date validation in quota cache - Batch background refresh with MAX_CONCURRENT_REFRESHES=5 - Move setModelUnavailable to after all accounts exhausted, not first 429 - Extract safePercentage() to shared utils (dedup) - Use isRecord() type guard in usage API route - Exclude binary files from Tailwind v4 source scanning
DavyMassoneto
force-pushed
the
fix/claude-oauth-usage-endpoint
branch
from
March 5, 2026 22:40
45a961c to
8eb14e0
Compare
Collaborator
|
Thanks @DavyMassoneto for this excellent hardening PR! 🎉 The quota system is now significantly more robust — false exhaustion from empty responses, concurrent refresh protection, and the Tailwind v4 build fix are all high-impact improvements. Great code quality across the board. We really appreciate your effort! |
diegosouzapw
added a commit
that referenced
this pull request
Mar 7, 2026
…point fix: harden quota system — code review fixes + build fix
diegosouzapw
added a commit
that referenced
this pull request
Sep 10, 2026
Closes #221 (smol-toml, HIGH, DoS via malformed TOML, vulnerable <= 1.7.0). @openai/codex-security pins smol-toml at 1.6.1 as an EXACT version, so no `npm update` reaches it. This repo already uses `overrides` as its standard tool for exactly that situation — the block carries 20+ entries, including the scoped-by-parent form and the `qs`/`fast-uri`/`ip-address` entries that back earlier security bumps — so a scoped override is the idiomatic fix here, not a new mechanism: "@openai/codex-security": { "smol-toml": "^1.8.0" } The nested copy deduplicates to the root's existing 1.8.0, which two other consumers (the root itself and knip) already run, so the version is proven in this tree. The whole lockfile diff is the 14 lines of the removed 1.6.1 entry. Also raised the `@yarnpkg/parsers` js-yaml floor from ^4.3.1 to ^4.3.2, so the override documents the patched version rather than permitting the vulnerable one it was written against. Not fixed, and not fixable by version — verified against the npm registry rather than trusting the advisory metadata: #218 extract-zip — latest published IS 2.0.1, the vulnerable version. Dev scope, via @openai/codex-security. No release to move to. #214 adm-zip — latest published IS 0.6.0, the top of the vulnerable range (>= 0.5.9, <= 0.6.0). RUNTIME scope, via onnxruntime-node's ^0.5.16, and the repo already overrides adm-zip to ^0.6.0. No release to move to. Both need an upstream fix or a decision to replace the dependency; neither is a lockfile change. adm-zip being runtime rather than dev makes it the one worth tracking. #210 esbuild stays open too. A flat `overrides: { esbuild: ^0.28.2 }` in opencode-plugin-v2 does close it — npm then reports 0 vulnerabilities — but it requires regenerating that lockfile from scratch: 823 lines, 96 packages moved, for a LOW dev-only alert, and a major esbuild bump inside tsup cannot be validated here without a real install of that package. Tried, measured, reverted. Left for an upstream tsup release. check:lockfile OK on all lockfiles including the workspace consistency check; check:tracked-artifacts OK; prettier clean.
diegosouzapw
added a commit
that referenced
this pull request
Sep 10, 2026
* chore(deps): drain the Dependabot queue — 7 of 10 alerts Lockfile-only bumps; no manifest touched, so nothing changes for consumers. Root package-lock.json: hono 4.13.0 -> 4.13.7 (#215 #216 #217, medium, patched 4.13.5) csv-parse 7.0.1 -> 7.0.2 (#213, medium) joi 18.2.3 -> 18.2.8 (#211 #212, low, patched 18.2.4/18.2.5) @omniroute/opencode-plugin: toml 4.1.1 -> 4.3.0 (#209, HIGH, patched 4.1.2) @omniroute/opencode-plugin-v2: esbuild 0.28.1 -> 0.28.2 (#210, low) — the direct copy only; see below. The plugin-v2 diff looks large but is one package: esbuild ships 27 platform binaries, each carrying version + resolved + integrity. Three alerts stay open, deliberately: #218 extract-zip (HIGH) and #214 adm-zip (medium) have NO published patch. Both are dev-scope. Closing them needs an upstream release or a decision to replace the dependency — neither belongs in a lockfile bump. #210 esbuild is only half-closed. `node_modules/esbuild` is on 0.28.2, but `tsup` pins `esbuild: ^0.27.0`, so its nested copy stays at 0.27.7 — inside the vulnerable range (>= 0.27.3, < 0.28.1). Updating tsup does not move it (8.5.1 is already current). Forcing it would take an `overrides` entry pushing a major of esbuild inside the bundler, which is exactly the change that breaks a build silently, for a LOW dev-only alert. Left for an upstream tsup release. check:lockfile passes on all three, including the workspace lock/manifest consistency check. check:tracked-artifacts OK. * chore(deps): bump js-yaml to 4.3.2 (root + electron) Two more HIGH alerts arrived after the first sweep: #220 js-yaml (root package-lock.json) >= 4.0.0, < 4.3.2 #219 js-yaml (electron/package-lock.json) >= 4.0.0, < 4.3.2 The root's own js-yaml was already on 5.4.1; the vulnerable copies were the ones nested under @yarnpkg/parsers, lockfile-lint, xmlbuilder2 (root) and the direct dependency in electron. All now 4.3.2. Four version lines, nothing else. #221 smol-toml (HIGH, <= 1.7.0) is NOT closed here. The root is on 1.8.0; the vulnerable 1.6.1 sits under @openai/codex-security, which pins it as an EXACT version rather than a range, so `npm update` cannot move it. Bumping codex-security itself (0.1.24 -> 0.1.26) does not help — 0.1.26 pins the same 1.6.1 — so that bump was reverted rather than carried along for no benefit. Closing #221 needs an upstream codex-security release or an `overrides` entry, the same trade already declined for #210/tsup: forcing a transitive pin from outside is how a build breaks silently. Note that @openai/codex-security is also the package carrying the unpatched extract-zip (#218), so one upstream release would likely clear both. * chore(deps): override smol-toml to 1.8.0 and raise the js-yaml floor Closes #221 (smol-toml, HIGH, DoS via malformed TOML, vulnerable <= 1.7.0). @openai/codex-security pins smol-toml at 1.6.1 as an EXACT version, so no `npm update` reaches it. This repo already uses `overrides` as its standard tool for exactly that situation — the block carries 20+ entries, including the scoped-by-parent form and the `qs`/`fast-uri`/`ip-address` entries that back earlier security bumps — so a scoped override is the idiomatic fix here, not a new mechanism: "@openai/codex-security": { "smol-toml": "^1.8.0" } The nested copy deduplicates to the root's existing 1.8.0, which two other consumers (the root itself and knip) already run, so the version is proven in this tree. The whole lockfile diff is the 14 lines of the removed 1.6.1 entry. Also raised the `@yarnpkg/parsers` js-yaml floor from ^4.3.1 to ^4.3.2, so the override documents the patched version rather than permitting the vulnerable one it was written against. Not fixed, and not fixable by version — verified against the npm registry rather than trusting the advisory metadata: #218 extract-zip — latest published IS 2.0.1, the vulnerable version. Dev scope, via @openai/codex-security. No release to move to. #214 adm-zip — latest published IS 0.6.0, the top of the vulnerable range (>= 0.5.9, <= 0.6.0). RUNTIME scope, via onnxruntime-node's ^0.5.16, and the repo already overrides adm-zip to ^0.6.0. No release to move to. Both need an upstream fix or a decision to replace the dependency; neither is a lockfile change. adm-zip being runtime rather than dev makes it the one worth tracking. #210 esbuild stays open too. A flat `overrides: { esbuild: ^0.28.2 }` in opencode-plugin-v2 does close it — npm then reports 0 vulnerabilities — but it requires regenerating that lockfile from scratch: 823 lines, 96 packages moved, for a LOW dev-only alert, and a major esbuild bump inside tsup cannot be validated here without a real install of that package. Tried, measured, reverted. Left for an upstream tsup release. check:lockfile OK on all lockfiles including the workspace consistency check; check:tracked-artifacts OK; prettier clean.
diegosouzapw
added a commit
that referenced
this pull request
Sep 15, 2026
Dependabot #214 (GHSA-vwc7-r8mq-g2x9 / CVE-2026-76845, moderate): adm-zip 0.5.9–0.6.0 follows a symlink that already exists inside the extraction root and writes through it, outside the root. The advisory still reports `first_patched_version: null`, but 0.6.1 (published after the advisory) is the fix — `util/utils.js` gains `assertPathSafe`, which walks every path component below the root with `lstat` and throws on a symlink; `extractAllTo` calls it before every write. Verified by diffing the two tarballs. Reach in this repo: adm-zip is pulled only by `onnxruntime-node` (an optionalDependency, itself pinned by override) and used only in its install script to unpack the vendor's own runtime binary. No request path touches it. The override already existed at ^0.6.0 (PR #7732, the previous adm-zip CVE); this just raises the floor. Lockfile moves 0.6.0 → 0.6.1, nothing else.
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…e-oauth-usage-endpoint fix: harden quota system — code review fixes + build fix
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…ference#13213) * chore(deps): drain the Dependabot queue — 7 of 10 alerts Lockfile-only bumps; no manifest touched, so nothing changes for consumers. Root package-lock.json: hono 4.13.0 -> 4.13.7 (cheaper-inference#215 cheaper-inference#216 cheaper-inference#217, medium, patched 4.13.5) csv-parse 7.0.1 -> 7.0.2 (cheaper-inference#213, medium) joi 18.2.3 -> 18.2.8 (cheaper-inference#211 cheaper-inference#212, low, patched 18.2.4/18.2.5) @omniroute/opencode-plugin: toml 4.1.1 -> 4.3.0 (cheaper-inference#209, HIGH, patched 4.1.2) @omniroute/opencode-plugin-v2: esbuild 0.28.1 -> 0.28.2 (cheaper-inference#210, low) — the direct copy only; see below. The plugin-v2 diff looks large but is one package: esbuild ships 27 platform binaries, each carrying version + resolved + integrity. Three alerts stay open, deliberately: cheaper-inference#218 extract-zip (HIGH) and cheaper-inference#214 adm-zip (medium) have NO published patch. Both are dev-scope. Closing them needs an upstream release or a decision to replace the dependency — neither belongs in a lockfile bump. cheaper-inference#210 esbuild is only half-closed. `node_modules/esbuild` is on 0.28.2, but `tsup` pins `esbuild: ^0.27.0`, so its nested copy stays at 0.27.7 — inside the vulnerable range (>= 0.27.3, < 0.28.1). Updating tsup does not move it (8.5.1 is already current). Forcing it would take an `overrides` entry pushing a major of esbuild inside the bundler, which is exactly the change that breaks a build silently, for a LOW dev-only alert. Left for an upstream tsup release. check:lockfile passes on all three, including the workspace lock/manifest consistency check. check:tracked-artifacts OK. * chore(deps): bump js-yaml to 4.3.2 (root + electron) Two more HIGH alerts arrived after the first sweep: cheaper-inference#220 js-yaml (root package-lock.json) >= 4.0.0, < 4.3.2 cheaper-inference#219 js-yaml (electron/package-lock.json) >= 4.0.0, < 4.3.2 The root's own js-yaml was already on 5.4.1; the vulnerable copies were the ones nested under @yarnpkg/parsers, lockfile-lint, xmlbuilder2 (root) and the direct dependency in electron. All now 4.3.2. Four version lines, nothing else. cheaper-inference#221 smol-toml (HIGH, <= 1.7.0) is NOT closed here. The root is on 1.8.0; the vulnerable 1.6.1 sits under @openai/codex-security, which pins it as an EXACT version rather than a range, so `npm update` cannot move it. Bumping codex-security itself (0.1.24 -> 0.1.26) does not help — 0.1.26 pins the same 1.6.1 — so that bump was reverted rather than carried along for no benefit. Closing cheaper-inference#221 needs an upstream codex-security release or an `overrides` entry, the same trade already declined for cheaper-inference#210/tsup: forcing a transitive pin from outside is how a build breaks silently. Note that @openai/codex-security is also the package carrying the unpatched extract-zip (cheaper-inference#218), so one upstream release would likely clear both. * chore(deps): override smol-toml to 1.8.0 and raise the js-yaml floor Closes cheaper-inference#221 (smol-toml, HIGH, DoS via malformed TOML, vulnerable <= 1.7.0). @openai/codex-security pins smol-toml at 1.6.1 as an EXACT version, so no `npm update` reaches it. This repo already uses `overrides` as its standard tool for exactly that situation — the block carries 20+ entries, including the scoped-by-parent form and the `qs`/`fast-uri`/`ip-address` entries that back earlier security bumps — so a scoped override is the idiomatic fix here, not a new mechanism: "@openai/codex-security": { "smol-toml": "^1.8.0" } The nested copy deduplicates to the root's existing 1.8.0, which two other consumers (the root itself and knip) already run, so the version is proven in this tree. The whole lockfile diff is the 14 lines of the removed 1.6.1 entry. Also raised the `@yarnpkg/parsers` js-yaml floor from ^4.3.1 to ^4.3.2, so the override documents the patched version rather than permitting the vulnerable one it was written against. Not fixed, and not fixable by version — verified against the npm registry rather than trusting the advisory metadata: cheaper-inference#218 extract-zip — latest published IS 2.0.1, the vulnerable version. Dev scope, via @openai/codex-security. No release to move to. cheaper-inference#214 adm-zip — latest published IS 0.6.0, the top of the vulnerable range (>= 0.5.9, <= 0.6.0). RUNTIME scope, via onnxruntime-node's ^0.5.16, and the repo already overrides adm-zip to ^0.6.0. No release to move to. Both need an upstream fix or a decision to replace the dependency; neither is a lockfile change. adm-zip being runtime rather than dev makes it the one worth tracking. cheaper-inference#210 esbuild stays open too. A flat `overrides: { esbuild: ^0.28.2 }` in opencode-plugin-v2 does close it — npm then reports 0 vulnerabilities — but it requires regenerating that lockfile from scratch: 823 lines, 96 packages moved, for a LOW dev-only alert, and a major esbuild bump inside tsup cannot be validated here without a real install of that package. Tried, measured, reverted. Left for an upstream tsup release. check:lockfile OK on all lockfiles including the workspace consistency check; check:tracked-artifacts OK; prettier clean.
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…3737) Dependabot cheaper-inference#214 (GHSA-vwc7-r8mq-g2x9 / CVE-2026-76845, moderate): adm-zip 0.5.9–0.6.0 follows a symlink that already exists inside the extraction root and writes through it, outside the root. The advisory still reports `first_patched_version: null`, but 0.6.1 (published after the advisory) is the fix — `util/utils.js` gains `assertPathSafe`, which walks every path component below the root with `lstat` and throws on a symlink; `extractAllTo` calls it before every write. Verified by diffing the two tarballs. Reach in this repo: adm-zip is pulled only by `onnxruntime-node` (an optionalDependency, itself pinned by override) and used only in its install script to unpack the vendor's own runtime binary. No request path touches it. The override already existed at ^0.6.0 (PR cheaper-inference#7732, the previous adm-zip CVE); this just raises the floor. Lockfile moves 0.6.0 → 0.6.1, nothing else.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #202
Code review fixes for the quota-aware account selection system introduced in #203, plus a build fix for Tailwind v4 scanning binary files.
Bug Fixes
hasUtilization()guard prevents creating quota objects when API returns{}(was defaultingutilization ?? 0= 0% remaining)parseDate()validates dates before comparison (handlesInvalid Datefrom malformedresetAt)total=0false infinite quota:normalizeQuotasnow defaults to0%remaining whentotalis zero (was100%)*.sqlite-shm/.sqlite-walbinary files, triggering "Invalid code point" error — added@source notexclusionsHardening
tickRunningflag prevents overlapping background ticks;refreshingSetdeduplicates per-connection refreshesMAX_CONCURRENT_REFRESHES=5clearModelUnavailabilityon every success, not only fallback pathsCode Quality
anthropic-versionintoCLAUDE_CONFIG.apiVersion(was hardcoded 3x)safePercentage()tosrc/shared/utils/formatting.ts(was duplicated inquotaCache.tsandutils.tsx)isRecord()type guard in usage API route (replaces inlinetypeofchain)ProviderLimitCardusesremainingPercentagedirectly (was recalculating from used/total)Test Plan
npm run buildpasses