镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(quota): enforce overall timeout for stalled fetch and body reads - #152

Merged
patlux merged 3 commits into
patlux:mainfrom
karaaslanz:fix/quota-timeout-upstream-20261013
Oct 15, 2026
Merged

patlux merged 3 commits into
patlux:mainfrom
karaaslanz:fix/quota-timeout-upstream-20261013

Conversation

@karaaslanz

@karaaslanz karaaslanz commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The quota command enforces its overall timeout only by calling AbortController.abort(). A custom fetch implementation that ignores AbortSignal, or a response whose json()/text() never settles, leaves /commandcode-quota hanging past the configured deadline.

Fix

  • Race fetch(), JSON parsing and HTTP error-body reads against one shared quota deadline.
  • Preserve the existing AbortController, error type, timeout semantics and partial quota fallback.
  • Add 3 hermetic tests for non-cooperative fetch, stuck JSON and stuck error-body reads.
  • Keep the change limited to src/quota.ts and tests/test-quota.ts.

Verification

  • Fork work prepared on a branch forked from upstream main (2 commits ahead / 0 behind; exactly 2 changed files).
  • Fork CI: formatting, TypeScript check, security scans, Oh My Pi compatibility and benchmark checks passed.
  • Upstream GitHub Actions CI: SUCCESS, including format, typecheck/npm test, dependency and security scans, and Oh My Pi compatibility. Earlier stale-pricing failure occurred only on the older fork baseline; no pricing overrides were made.
  • Targeted quota tests: 26/26 PASS on exact commit 7812552 (Node 24.21, Windows). This includes all 3 new non-cooperative I/O cases.

No changes to release/pricing, credentials or Control Center.

Comment thread src/quota.ts Resolved
@karaaslanz
karaaslanz marked this pull request as ready for review October 13, 2026 19:01

@pierreraby pierreraby left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified exact head 781255228eaf6c8b5cbdc00cc1b234eb70b5f398 (fork karaaslanz:fix/quota-timeout-upstream-20261013, base b61093d) as a scoped quota-timeout hardening change.

Independent checks on a clean detached checkout of this exact head:

  • npm run typecheck PASS; full npm run test:unit green (17 files, 0 failures, exit 0); quota suite 26/26 including the 3 new non-cooperative-I/O cases (~26 ms each, deterministic — no wall-clock upper bound asserted).
  • RED proven on base b61093d with the PR tests applied: the runner hangs on the first new test (killed by external timeout, exit 124) — the cooperative abort alone never settles a fetch that ignores the signal.
  • prettier --check and git diff --check PASS on both changed files.
  • Upstream checks on this head: 13/13 success (typecheck, format, CodeQL x2, Semgrep x2, OMP compat, npm audit, Gitleaks, lifecycle, dependency).
  • Scope confirmed: only src/quota.ts (+23/-8) and tests/test-quota.ts (+43); workflow files byte-identical to main.

Read of the fix: one shared deadline promise races fetchImpl(), error-body text(), and json(); the timer aborts before rejecting so signal.aborted normalizes every path to kind: "timeout" with the existing partial-quota fallback intact. All quota I/O (including the OMP usage provider via src/usage.ts) funnels through request()/safeRequest() — no uncovered I/O found.

Notes (non-blocking): no CHANGELOG entry (consistent with release-time collection per RELEASE.md — please credit #152 at release); the shared deadline promise's losing side stays pending for a broken custom fetch (real fetch honors the signal); no dedicated test for a stalled summaryRaw (same helper, covered by structure).

Approving. Leaving merge timing and 0.7.8 scoping to the maintainers.

@patlux
patlux merged commit 5820781 into patlux:main Oct 15, 2026
12 checks passed
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.

4 participants