Skip to content

fix(codex): isolate Spark quota scope - #4293

Merged
diegosouzapw merged 13 commits into
diegosouzapw:release/v3.8.30from
xz-dev:fix/codex-spark-quota-scope
Jun 19, 2026
Merged

diegosouzapw merged 13 commits into
diegosouzapw:release/v3.8.30from
xz-dev:fix/codex-spark-quota-scope

Conversation

@xz-dev

@xz-dev xz-dev commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • split Codex quota/rate-limit scope handling between normal Codex models and GPT-5.3-Codex-Spark
  • parse Codex WHAM additional_rate_limits for Spark-specific quota windows while preserving normal primary/secondary window handling
  • keep Codex 429 failover cooldowns scoped so a Spark 429 does not write connection-wide rateLimitedUntil and block normal Codex models like gpt-5.5
  • surface separate Spark quota rows/labels in the dashboard UI
  • keep existing helper imports from open-sse/executors/codex.ts stable via re-exports

Details

This adds a shared Codex quota scope helper module and uses it from the executor, quota fetcher, auth selection, account fallback/model lockout, usage parsing, and UI label formatting.

Spark detection covers:

  • gpt-5.3-codex-spark
  • codex-spark
  • spark
  • bengalfox
  • metered_feature = gpt_5_3_codex_spark

Normal Codex requests continue to use the regular WHAM rate_limit.primary_window / secondary_window values. Spark requests use Spark-specific windows from additional_rate_limits and do not fall back to normal Codex windows when Spark data is absent.

Validation

  • node node_modules/prettier/bin/prettier.cjs --check ...changed files...
  • DISABLE_SQLITE_AUTO_BACKUP=true node --import tsx --import ./open-sse/utils/setupPolyfill.ts --test tests/unit/executor-codex.test.ts tests/unit/codex-quota-fetcher.test.ts tests/unit/account-fallback-service.test.ts tests/unit/sse-auth.test.ts tests/unit/usage-service-hardening.test.ts
    • tests 209
    • pass 209
    • fail 0
  • node node_modules/typescript/bin/tsc --pretty false -p tsconfig.typecheck-core.json
  • git diff --cached --check

@xz-dev
xz-dev requested a review from diegosouzapw as a code owner June 19, 2026 17:09

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request isolates the standard 'codex' and premium 'spark' rate-limiting and quota pools for Codex, ensuring that exhausting one does not block requests to the other. It introduces model-scoping helpers, updates connection selection and rate-limiting logic to handle these scopes independently, and updates the usage service and dashboard to display Spark quotas. Feedback was provided to prevent potential data loss in chatCore.ts when updating connection data on transient database errors, to avoid modifying the cache map during iteration in codexQuotaFetcher.ts, and to use bracket notation instead of dot notation in usage.ts to prevent TypeScript compilation errors.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread open-sse/handlers/chatCore.ts Outdated
Comment on lines 3126 to 3153
const connection = await getProviderConnectionById(
String(failedConnectionId)
).catch(() => null);
const existingProviderData =
connection?.providerSpecificData &&
typeof connection.providerSpecificData === "object"
? connection.providerSpecificData
: credentials?.providerSpecificData &&
typeof credentials.providerSpecificData === "object"
? credentials.providerSpecificData
: {};
const existingScopeMap =
existingProviderData.codexScopeRateLimitedUntil &&
typeof existingProviderData.codexScopeRateLimitedUntil === "object"
? (existingProviderData.codexScopeRateLimitedUntil as Record<string, unknown>)
: {};
const nextProviderData = {
...existingProviderData,
codexScopeRateLimitedUntil: {
...existingScopeMap,
[failedScope]: rateLimitedUntil,
},
};
updateProviderConnection(String(failedConnectionId), {
rateLimitedUntil,
testStatus: "unavailable",
providerSpecificData: nextProviderData,
lastError: "429 rate limited — codex account rotation",
errorCode: 429,
}).catch(() => {});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If getProviderConnectionById fails or returns null (due to a transient database error or connection issue), existingProviderData will fall back to credentials?.providerSpecificData or {}. Calling updateProviderConnection with providerSpecificData: nextProviderData in this case would overwrite the entire providerSpecificData column in the database with a nearly empty object, potentially wiping out critical credentials, OAuth tokens, or settings. To prevent accidental data loss, only include providerSpecificData in the database update if the connection was successfully retrieved.

                  const connection = await getProviderConnectionById(
                    String(failedConnectionId)
                  ).catch(() => null);
                  const existingProviderData =
                    connection?.providerSpecificData &&
                    typeof connection.providerSpecificData === "object"
                      ? connection.providerSpecificData
                      : credentials?.providerSpecificData &&
                          typeof credentials.providerSpecificData === "object"
                        ? credentials.providerSpecificData
                        : {};
                  const existingScopeMap =
                    existingProviderData.codexScopeRateLimitedUntil &&
                    typeof existingProviderData.codexScopeRateLimitedUntil === "object"
                      ? (existingProviderData.codexScopeRateLimitedUntil as Record<string, unknown>)
                      : {};
                  const nextProviderData = {
                    ...existingProviderData,
                    codexScopeRateLimitedUntil: {
                      ...existingScopeMap,
                      [failedScope]: rateLimitedUntil,
                    },
                  };
                  updateProviderConnection(String(failedConnectionId), {
                    ...(connection ? { providerSpecificData: nextProviderData } : {}),
                    lastError: "429 rate limited — codex account rotation",
                    errorCode: 429,
                  }).catch(() => {});

Comment on lines +111 to +116
function deleteQuotaCacheForConnection(connectionId: string): void {
quotaCache.delete(connectionId);
for (const key of quotaCache.keys()) {
if (key === connectionId || key.startsWith(`${connectionId}:`)) quotaCache.delete(key);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Modifying a Map or Cache during iteration over its keys can lead to unpredictable behavior or runtime errors depending on the underlying implementation (especially if quotaCache is a custom cache or an LRU cache). It is safer and cleaner to collect the keys to delete first, and then perform the deletion. Additionally, key === connectionId is redundant since quotaCache.delete(connectionId) is already called at the start of the function.

Suggested change
function deleteQuotaCacheForConnection(connectionId: string): void {
quotaCache.delete(connectionId);
for (const key of quotaCache.keys()) {
if (key === connectionId || key.startsWith(`${connectionId}:`)) quotaCache.delete(key);
}
}
function deleteQuotaCacheForConnection(connectionId: string): void {
quotaCache.delete(connectionId);
const keysToDelete: string[] = [];
for (const key of quotaCache.keys()) {
if (key.startsWith(connectionId + ":")) {
keysToDelete.push(key);
}
}
for (const key of keysToDelete) {
quotaCache.delete(key);
}
}

Comment thread open-sse/services/usage.ts Outdated
Comment on lines +2963 to +2974
if (
!isCodexSparkLimitDescriptor(
getFieldValue(entry, "limit_name", "limitName"),
getFieldValue(entry, "metered_feature", "meteredFeature"),
getFieldValue(entry, "limit_id", "limitId"),
entry.id,
entry.name,
entry.title,
entry.model,
getFieldValue(entry, "model_id", "modelId")
)
) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using dot notation like entry.id, entry.name, etc., on a record of type Record<string, unknown> will cause TypeScript compilation errors under strict mode. For consistency with open-sse/services/codexQuotaFetcher.ts and other fields in this file, use bracket notation to access these properties safely.

        if (
          !isCodexSparkLimitDescriptor(
            getFieldValue(entry, "limit_name", "limitName"),
            getFieldValue(entry, "metered_feature", "meteredFeature"),
            getFieldValue(entry, "limit_id", "limitId"),
            entry["id"],
            entry["name"],
            entry["title"],
            entry["model"],
            getFieldValue(entry, "model_id", "modelId")
          )
        )

xz-dev and others added 12 commits June 20, 2026 01:40
# Conflicts:
#	config/quality/complexity-baseline.json
#	config/quality/file-size-baseline.json
#	open-sse/handlers/chatCore.ts
#	tests/integration/integration-wiring.test.ts
#	tests/integration/search-providers-catalog.test.ts
#	tests/unit/tproxy-transparent-socket.test.ts
…ase/v3.8.30 merge (diegosouzapw#4293)

Measured on the actual merged tree (not the PR's main-based estimate):
complexity 1885->1887 (+2); file-size auth.ts 2219->2279, chatCore.ts 5116->5125,
accountFallback.ts 1727->1731, + the 4 Codex test files. Drift test-file conflicts
(search-providers-catalog, tproxy-transparent-socket, integration-wiring) resolved
to the already-merged release versions (diegosouzapw#4276).

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.30 June 19, 2026 20:57
@diegosouzapw
diegosouzapw merged commit 915991c into diegosouzapw:release/v3.8.30 Jun 19, 2026
3 checks passed
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @xz-dev! 🙏 Merged into release/v3.8.30. This is a solid, well-tested fix — isolating the Codex Spark (gpt-5.3-codex-spark) quota pool from the standard Codex pool across the whole selection path (quota-policy / headroom / preflight / P2C scoring), the 429 failover (scoped codexScopeRateLimitedUntil instead of a connection-wide rateLimitedUntil), and the model-lock keys, plus surfacing the Spark windows in the usage dashboard. The extraction to leaf helpers (codexQuotaScopes.ts, codexUsageQuotas.ts, codexFailover.ts) kept it clean.

What I reconciled on merge (the branch was based on main): I rebased onto release/v3.8.30, took the already-merged release versions of three unrelated drift test files (search-providers-catalog / tproxy-transparent-socket / integration-wiring — fixed differently by #4276), and re-measured the baselines on the actual merged tree (complexity 1885→1887; file-size auth.ts→2279, chatCore.ts→5125, accountFallback.ts→1731 + the 4 Codex test files). Full local validation green: 209/209 Codex/resilience unit tests, typecheck, file-size/complexity/dead-code gates. Will ship in the next release. Great work!

@diegosouzapw diegosouzapw mentioned this pull request Jun 20, 2026
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
* fix(codex): isolate Spark quota scope

* fix(codex): address Spark quota review feedback

* fix(ci): update Electron undici override

* fix(ci): update root undici overrides

* test(integration): sync stale expectations

* test(tproxy): tolerate available native addon

* test(tproxy): avoid environment-specific skips

* test(tproxy): keep assertion count stable

* fix(ci): stabilize quality and tproxy checks

* chore(ci): rebaseline auth file size

* fix(ci): extend node compatibility budget

* chore(quality): reconcile complexity + file-size baselines after release/v3.8.30 merge (diegosouzapw#4293)

Measured on the actual merged tree (not the PR's main-based estimate):
complexity 1885->1887 (+2); file-size auth.ts 2219->2279, chatCore.ts 5116->5125,
accountFallback.ts 1727->1731, + the 4 Codex test files. Drift test-file conflicts
(search-providers-catalog, tproxy-transparent-socket, integration-wiring) resolved
to the already-merged release versions (diegosouzapw#4276).

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>

---------

Co-authored-by: ci <ci@local>
Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
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.

2 participants