Skip to content

fix(sse): keep Codex quota headers under the forwarding budget - #10306

Open
RaviTharuma wants to merge 7 commits into
diegosouzapw:release/v3.8.50from
RaviTharuma:fix/forward-codex-quota-headers
Open

fix(sse): keep Codex quota headers under the forwarding budget#10306
RaviTharuma wants to merge 7 commits into
diegosouzapw:release/v3.8.50from
RaviTharuma:fix/forward-codex-quota-headers

Conversation

@RaviTharuma

@RaviTharuma RaviTharuma commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Official 3.8.49 logs Dropped upstream response headers that exceeded forwarding budget on every SSE response (budgetBytes=768, droppedCount=12–26).
  • x-codex-primary-used-percent, reset-after-seconds, credits-* do not contain the substring ratelimit, so they were priority 3 and lost to date / cf-ray / content-security-policy.
  • x-codex-turn-state is 314 bytes and is not a client quota signal. It ate ~40% of the old budget.
  • Raise cap 768 → 2048. Treat Codex used/reset/window/credits/plan headers as priority 2. Do not forward x-codex-turn-state. Deprioritize date / csp / cf-* / org-id.
  • Unit test: quota headers survive a 300-byte turn-state + CSP + cf-ray + date.

Related Issues

Validation

  • Production-code changes include a new or updated automated test in this PR
node --import tsx --test tests/unit/middleware-header-strip-5849.test.ts
# tests 8, pass 8

Reviewer Notes

No secrets or private hostnames. Observed header names only.

diegosouzapw and others added 6 commits August 8, 2026 00:08
@diegosouzapw
fix(deps): bump nanoid, dompurify for 2 new Dependabot alerts (diegos…
b090b60 
…ouzapw#189, diegosouzapw#190)

Bumps: nanoid ^3.3.17 (was transitive, now overridden), dompurify ^3.4.13
(with monaco-editor scoped override). Closes Dependabot diegosouzapw#189, diegosouzapw#190.

Remaining diegosouzapw#182-diegosouzapw#188 (js-yaml + mermaid) already closed by diegosouzapw#9651 merge —
awaiting Dependabot re-scan.

npm audit → 0 vulnerabilities.
@diegosouzapw
fix(deps): bump nanoid, dompurify for Dependabot diegosouzapw#189, di…
026e1ca 
…egosouzapw#190

Closes Dependabot diegosouzapw#189 (dompurify 3.4.13) and diegosouzapw#190 (nanoid 3.3.17). npm audit → 0.
@diegosouzapw
fix(repo): harden .gitignore to also ignore a _tasks symlink (/_tasks)
918fba5 
_tasks is a SEPARATE nested git repo (gitignored). The pattern _tasks/ (trailing
slash) ignores only a directory, not a SYMLINK named _tasks. A self-referential
_tasks symlink can slip in via git add -A and, once pulled, checkout materializes
it over the real _tasks repo (destroying plans/specs/hands-off). Anchored /_tasks
ignores the symlink too, preventing re-capture.
@ritheshcn25
Hide health-check excluded models from /v1/models catalog (diegosouza…
5f0a394 
…pw#10026)

Mirror the request-time exclusion rule (provider_specific_data.excludedModels)
in the unified catalog builder: a model is hidden when its provider has
connections but none of them is eligible for it. Applied across the
PROVIDER_MODELS, synced, custom, alias-backed, and managed-fallback loops
so ghost models no longer appear as available.

Co-authored-by: ritheshcn25 <ritheshcn25@users.noreply.github.com>
@RaviTharuma @cursoragent
fix(models): memoize getModelsDevPricing (event loop / healthz) (dieg…
ca23eed 
…osouzapw#10055)

* fix(models): memoize getModelsDevPricing for /v1/models catalog

resolveCatalogPricing called getModelsDevPricing once per model while
building GET /v1/models. Each call re-scanned models_dev_pricing and
JSON.parsed every row (~10k SQL scans + multi-GB parse work), pegging
the event loop so even /healthz timed out (diegosouzapw#9685, diegosouzapw#10052).

Memoize the parsed map until saveModelsDevPricing / clearModelsDevPricing
and add a unit test for invalidation.

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>

* fix(db): invalidate modelsDevPricing cache on DB reset (diegosouzapw#10055)

Copilot review fixes:
1. Register invalidateModelsDevPricingCache() with DB state reset system
   so resetDbInstance() clears the process-local memo, preventing stale
   pricing data from surviving across DB reset/restore operations.
2. Add test assertion verifying DB reset bypasses the memo (Copilot diegosouzapw#10055).

The process-local memo at modelsDevSync.ts:204 caches getModelsDevPricing()
results until saveModelsDevPricing()/clearModelsDevPricing() to avoid
re-scanning all pricing rows on every /v1/models request. Without this hook,
backup restore and test DB resets would serve stale cached data from the
previous connection.

Tests: npm run test:unit:serial -- tests/unit/modelsDevSync-extended.test.ts

---------

Signed-off-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
@RaviTharuma
fix(sse): keep Codex quota headers under the forwarding budget
13dc860 
The 768-byte cap plus priority-3 for any name that does not contain
"ratelimit" dropped x-codex-*-used-percent / reset / credits on every
stream. x-codex-turn-state (314 bytes) ate the budget. Raise the cap,
treat Codex quota headers as rate-limit priority, and do not forward
turn-state.
@RaviTharuma

RaviTharuma commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Implements / pairs with #10310 (quota headers dropped) and reduces #10315 (warn storm) by not forwarding turn-state.

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.50 August 14, 2026 19:26
@diegosouzapw diegosouzapw mentioned this pull request Aug 14, 2026
Open
@diegosouzapw

diegosouzapw commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Thanks for chasing this down — the analysis is solid: x-codex-turn-state at ~314 bytes was eating roughly 40% of the old 768-byte budget, and the real quota signals (x-codex-*-used-percent, reset-after-seconds, credits-*) don't contain "ratelimit" so they fell to priority 3 and lost to date/cf-ray/CSP. The fix approach (deny x-codex-turn-state, bump those headers to priority 2, deprioritize date/CSP/cf-*/org-id) is right, and the new test in tests/unit/middleware-header-strip-5849.test.ts covers it well.

One blocker before this can land: the branch currently conflicts with release/v3.8.50's tip. #9492 already turned MAX_FORWARDED_UPSTREAM_RESPONSE_HEADER_BYTES into resolveForwardedHeaderBudget() (env-configurable via OMNIROUTE_FORWARDING_HEADER_BUDGET_BYTES, default still 768) after this branch was cut, so overwriting it with a hardcoded 2048 would silently drop that override capability. Could you rebase onto the current release/v3.8.50 tip, keep resolveForwardedHeaderBudget() intact, and layer the x-codex-turn-state denylist entry + the new getForwardingPriority() branches on top of it?

Also, the branch is currently carrying 4 commits that are out of scope for this PR — the getModelsDevPricing memoization and the health-check-excluded-models catalog filter (already merged via #10055/#10026, just not yet synced from main to this release branch), plus the .gitignore _tasks hardening and the dompurify/nanoid bumps (already present at the release tip). Dropping those (e.g. git rebase -i keeping only the "keep Codex quota headers under the forwarding budget" commit) will make the diff match the PR description.

Once it's rebased clean and the test still passes against the real tip, this looks good to merge.

@RaviTharuma @diegosouzapw @adevwithpurpose
fix(sse): keep Codex quota headers under the forwarding budget
5c0f9e4 
Merge origin/release/v3.8.50 to refresh 2280-commit-stale CI verdict.

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

diegosouzapw commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Babysit summary

Verdict: STALE — no code fix was needed. The PR was 2280 commits behind release/v3.8.50. The Build + Quality Ratchet failures on the PR list were artifacts of a frozen CI run against a pre-fix base, not defects in this PR.

  • Refresh-merge: merged origin/release/v3.8.50 (tip 774127be3) → commit 5c0f9e43. Resolved 5 conflicts:
    • open-sse/handlers/chatCore/responseHeaders.ts: kept the PR's substantive fix (drop x-codex-turn-state, Codex quota headers at rate-limit priority 2, diagnostics at priority 4) and the base's configurable resolveForwardedHeaderBudget() (default 768). The PR's own test + the base's forwarded-header-budget.test.ts both pass at the 768 default — 11/11 green.
    • .gitignore, src/lib/modelsDevSync.ts, tests/unit/modelsDevSync-extended.test.ts, package-lock.json: took base (theirs) — pure base drift, PR never touched them.
  • Gate: all green on 5c0f9e43 — Fast Quality Gates, Build (advisory), Unit Tests (4/4), Vitest, ESLint warnings, Merge integrity, docs, DAST, semgrep. No failures.
  • Net delta vs base: only the PR's responseHeaders.ts + middleware-header-strip-5849.test.ts + changelog (plus fork-lineage catalog.ts Hide health-check excluded models from /v1/models catalog #10026 content inherited from the contributor's base, not introduced here).
  • Tests: middleware-header-strip-5849.test.ts + forwarded-header-budget.test.ts run green locally; ESLint clean on changed files.
  • Ready for human review & merge.

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

Reviewers

@diegosouzapw diegosouzapw Awaiting requested review from diegosouzapw diegosouzapw is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RaviTharuma @diegosouzapw @ritheshcn25