fix(usage): guard malformed Z.AI usage payloads#110741
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 20, 2026, 2:48 AM ET / 06:48 UTC. Summary PR surface: Source +1, Tests +11. Total +12 across 2 files. Reproducibility: yes. from source: make the Z.AI usage endpoint return HTTP 200 with top-level JSON Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Keep the shared record guard at the Z.AI response boundary, rebase onto current Do we have a high-confidence way to reproduce the issue? Yes, from source: make the Z.AI usage endpoint return HTTP 200 with top-level JSON Is this the best way to solve the issue? Yes. Guarding the top-level response with the existing shared record helper at the parser boundary is the narrowest maintainable fix; it preserves valid record behavior and uses the existing error snapshot for malformed successful payloads. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e2e394e1ab80. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1, Tests +11. Total +12 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
2c4e2c4 to
c5073e3
Compare
|
The // src/infra/provider-usage.fetch.zai.ts:71 (before this PR, at 0ac7b24d56c)
const limits = data.data?.limits || [];Optional chaining plus
Only the last row is the crash. The visible surface is The first row is the one I'd push back on: an account with no quotas configured is a legitimate if (value.data !== undefined && !isRecord(value.data)) {
return undefined;
}Scope-limiting, because it's smaller than it first looks: this is a reporting-wording regression, not a data one — the compact Worth noting the sibling picked the opposite policy for the same case — Second thing, on coverage. The three One procedural note, not a finding: the ClawSweeper review here is against Happy to send the two missing fixtures against your branch if that's easier than folding them in. |
|
Merged via squash.
|
|
Post-merge validation note:
Thank you @Leon-SK668 for the contribution, and thank you @Yigtwxx for the detailed review that caught the graceful empty-response regression and the missing boundary fixtures. Those points materially improved the landed fix. |
* fix(usage): guard malformed Z.ai usage payloads * fix(usage): normalize Z.ai usage payloads * fix(usage): preserve empty Z.ai snapshots --------- Co-authored-by: Leon-SK668 <[email protected]> Co-authored-by: Altay <[email protected]>
What Problem This Solves
Fixes an issue where users viewing Z.AI usage can hit an uncaught
TypeErrorwhen the provider returns a successful HTTP response whose JSON payload isnullor an array.Why This Change Was Made
The usage endpoint now treats non-record successful payloads like the existing unsuccessful-payload path and returns the stable
API errorsnapshot. Valid Z.AI usage records keep the existing window and plan parsing behavior.User Impact
Z.AI usage checks degrade to a stable provider error instead of rejecting the usage refresh when the upstream sends a malformed top-level JSON shape.
Evidence
node scripts/run-vitest.mjs src/infra/provider-usage.fetch.zai.test.ts(9 tests passed)oxfmt --check src/infra/provider-usage.fetch.zai.ts src/infra/provider-usage.fetch.zai.test.ts(passed)git diff --check(passed)mainata115af277410a91fb039d2ed699eafad706f5c73tsgowas attempted; the run is currently blocked by three unrelated existing errors inpackages/net-policy/src/ip.ts.Exact-Head Runtime Proof
c5073e3f81e4139e14ac801a4fbce49d1277373f.contents: read, both checkouts usepersist-credentials: false, and the runtime uses a synthetic token rather than repository or provider secrets.src/infra/provider-usage.fetch.zai.ts#fetchZaiUsage, confirmed its fixed upstream target, and redirected that request through realfetchto a disposable loopback HTTP server. This is controlled-server runtime proof, not a credentialed call to the external Z.AI service.nulland[]), the refresh did not reject. Each result was{ provider: "zai", error: "API error", windows: [] }; the observed requests wereGET, requested JSON, and carried only the redacted synthetic authorization header.zai-usage-loopback-proof-c5073e3f81e4139e14ac801a4fbce49d1277373f(GitHub artifact digestsha256:1b4d0585fba98e7728fdb335552784b222bbcb9afefc7c1355321073804aabb9; contained JSON SHA-256de08065516cbf7aa5bfead80a83879b6a3b8576b9e0028ac86ecef0df5ecb3e8).AI-assisted contribution; the change and its behavior are understood and covered by the focused regression tests and exact-head runtime proof.