Skip to content

fix(usage): guard malformed Z.AI usage payloads#110741

Merged
altaywtf merged 3 commits into
openclaw:mainfrom
Leon-SK668:fix/zai-usage-null-guard-20260718
Jul 20, 2026
Merged

fix(usage): guard malformed Z.AI usage payloads#110741
altaywtf merged 3 commits into
openclaw:mainfrom
Leon-SK668:fix/zai-usage-null-guard-20260718

Conversation

@Leon-SK668

@Leon-SK668 Leon-SK668 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where users viewing Z.AI usage can hit an uncaught TypeError when the provider returns a successful HTTP response whose JSON payload is null or 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 error snapshot. 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)
  • Submission base: main at a115af277410a91fb039d2ed699eafad706f5c73
  • Core tsgo was attempted; the run is currently blocked by three unrelated existing errors in packages/net-policy/src/ip.ts.

Exact-Head Runtime Proof

  • Exact PR head: c5073e3f81e4139e14ac801a4fbce49d1277373f.
  • Secretless exact-head Actions proof passed on Node 24.15.0. The workflow grants only contents: read, both checkouts use persist-credentials: false, and the runtime uses a synthetic token rather than repository or provider secrets.
  • The proof imported the real src/infra/provider-usage.fetch.zai.ts#fetchZaiUsage, confirmed its fixed upstream target, and redirected that request through real fetch to a disposable loopback HTTP server. This is controlled-server runtime proof, not a credentialed call to the external Z.AI service.
  • For both HTTP 200 malformed top-level payloads (null and []), the refresh did not reject. Each result was { provider: "zai", error: "API error", windows: [] }; the observed requests were GET, requested JSON, and carried only the redacted synthetic authorization header.
  • The redacted proof artifact is zai-usage-loopback-proof-c5073e3f81e4139e14ac801a4fbce49d1277373f (GitHub artifact digest sha256:1b4d0585fba98e7728fdb335552784b222bbcb9afefc7c1355321073804aabb9; contained JSON SHA-256 de08065516cbf7aa5bfead80a83879b6a3b8576b9e0028ac86ecef0df5ecb3e8).

AI-assisted contribution; the change and its behavior are understood and covered by the focused regression tests and exact-head runtime proof.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 18, 2026
@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 20, 2026, 2:48 AM ET / 06:48 UTC.

Summary
The branch validates that a successful Z.AI usage JSON payload is a record before reading usage fields, with regression tests for null and array payloads.

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 null or []; the unguarded successful-response field access follows directly. The PR’s exact-head loopback run reports the corrected result for both shapes.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase or update the branch against current main, then re-run the focused test and exact-head loopback proof on the resulting head.

Risk before merge

  • [P1] The branch is behind current main; refresh it against current main and re-run the focused regression and exact-head proof before merge so the reviewed result matches the merge result.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the shared record guard at the Z.AI response boundary, rebase onto current main, and retain the focused malformed-payload regression coverage and controlled-runtime proof.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] No repair lane is needed: the patch is correct and proof-positive; it needs only the ordinary current-main refresh before merge.

Security
Cleared: The two-file TypeScript and test change adds no dependency, permission, secret, workflow, network-target, or supply-chain surface.

Review details

Best possible solution:

Keep the shared record guard at the Z.AI response boundary, rebase onto current main, and retain the focused malformed-payload regression coverage and controlled-runtime proof.

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 null or []; the unguarded successful-response field access follows directly. The PR’s exact-head loopback run reports the corrected result for both shapes.

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 changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR now includes exact-head controlled-runtime evidence using the real fetcher and a loopback server, with observed corrected results for both malformed payload shapes; the linked proof is appropriately secretless and redacted.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR now includes exact-head controlled-runtime evidence using the real fetcher and a loopback server, with observed corrected results for both malformed payload shapes; the linked proof is appropriately secretless and redacted.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a bounded provider-usage robustness fix that prevents an optional usage refresh from failing on malformed upstream JSON.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR now includes exact-head controlled-runtime evidence using the real fetcher and a loopback server, with observed corrected results for both malformed payload shapes; the linked proof is appropriately secretless and redacted.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR now includes exact-head controlled-runtime evidence using the real fetcher and a loopback server, with observed corrected results for both malformed payload shapes; the linked proof is appropriately secretless and redacted.
Evidence reviewed

PR surface:

Source +1, Tests +11. Total +12 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 4 3 +1
Tests 1 11 0 +11
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 15 3 +12

What I checked:

  • Current failure surface: The Z.AI usage fetcher reads success, code, and msg from a successful JSON result; a top-level null or array is not a valid response record and can cause the reported failure before a stable snapshot is returned. (src/infra/provider-usage.fetch.zai.ts:56, e2e394e1ab80)
  • Narrow implementation: The PR adds the repository’s shared isRecord guard at the response boundary, preserves the existing success/code checks for valid records, and returns the existing API error fallback for non-record payloads. (src/infra/provider-usage.fetch.zai.ts:57, c5073e3f81e4)
  • Regression coverage: The focused test adds successful HTTP 200 cases for both null and [], asserting the stable error and empty usage-window result. (src/infra/provider-usage.fetch.zai.test.ts:23, c5073e3f81e4)
  • After-fix runtime proof: The PR body links exact-head secretless Actions proof using the real fetcher against a loopback HTTP server; both malformed payloads reportedly return { provider: "zai", error: "API error", windows: [] } without rejecting. The supplied PR checks also show the real-behavior-proof job and changed-path checks succeeding. (c5073e3f81e4)

Likely related people:

  • Leon-SK668: Authored the exact-head guard and regression coverage; no stronger current-main feature-history owner was established in the available review evidence. (role: current patch author; confidence: low; commits: c5073e3f81e4; files: src/infra/provider-usage.fetch.zai.ts, src/infra/provider-usage.fetch.zai.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (2 earlier review cycles)
  • reviewed 2026-07-18T14:50:30.965Z sha 2c4e2c4 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-20T03:16:32.221Z sha c5073e3 :: needs real behavior proof before merge. :: none

@Leon-SK668
Leon-SK668 force-pushed the fix/zai-usage-null-guard-20260718 branch from 2c4e2c4 to c5073e3 Compare July 20, 2026 03:13
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 20, 2026
@Yigtwxx

Yigtwxx commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

The data guard is wider than the crash it fixes, and it converts a currently-graceful response into a provider error.

// src/infra/provider-usage.fetch.zai.ts:71 (before this PR, at 0ac7b24d56c)
const limits = data.data?.limits || [];

Optional chaining plus || [] meant a 200 with success: true, code: 200 and no data key at all resolved to zero windows and a clean snapshot. The normalizer returns undefined for that same payload, which the caller turns into error: "API error". Four shapes, only one of which is the bug:

payload (HTTP 200, success: true) before after
data key absent z.ai: no data z.ai: API error
data: null z.ai: no data z.ai: API error
limits: null z.ai: no data z.ai: API error
limits: {} TypeError: limits is not iterable z.ai: API error

Only the last row is the crash. The visible surface is formatUsageReportLines (src/infra/provider-usage.format.ts:139-142), where entry.error short-circuits window rendering, reached from src/commands/status.command.ts:300-301.

The first row is the one I'd push back on: an account with no quotas configured is a legitimate 200, and openclaw status now calls it an API error. Narrowing keeps the crash fixed without that:

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 📊 Usage: line filters error snapshots and empty-window snapshots alike (provider-usage.format.ts:105-108), so only the /status report line differs. And I can't tell from here whether Z.AI ever omits data; if you've observed it always present, this is moot and worth one line in the body saying so.

Worth noting the sibling picked the opposite policy for the same case — normalizeClaudeUsage falls back rather than erroring, and skips malformed entries individually (src/infra/provider-usage.fetch.claude.ts:88-92). Your entry-level !isRecord(rawLimit) skip already matches that shape; it's only the container-level guard that diverges.

Second thing, on coverage. The three limit.number !== undefined guards are a real fix — without them a limit with a recognized unit and no number rendered as Tokens (undefinedh) through the template literals at :77-83. But none of the three is pinned. The closest existing fixture is { type: "TOKENS_LIMIT", percentage: -5, unit: 99 }, and unit: 99 matches no branch; the new { type: "TIME_LIMIT", percentage: "40" } has no unit at all. Both land on the default "Limit" label with or without the guards, so reverting all three leaves the suite green. A single { type: "TOKENS_LIMIT", unit: 3, percentage: 32 } asserting Tokens (Limit) would lock it. Same for the data-absent branch — only data: null and data: [] are asserted.

One procedural note, not a finding: the ClawSweeper review here is against c5073e3f81e4 and describes the surface as "Source +1, Tests +11". Head is ad766bfd at +130/−28, so its "no metrics identified" doesn't cover the normalizer rewrite or the number guards. Worth a re-review before anyone reads that rating as coverage of this diff.

Happy to send the two missing fixtures against your branch if that's easier than folding them in.

@altaywtf
altaywtf merged commit 8d4d02a into openclaw:main Jul 20, 2026
91 checks passed
@altaywtf

Copy link
Copy Markdown
Member

Merged via squash.

@altaywtf

Copy link
Copy Markdown
Member

Post-merge validation note:

  • Reproduced and validated on a fresh Cloudflare Crabbox lease bound to the prepared head; changed-file SHA-256 checks matched the reviewed sources.
  • The focused Z.AI usage suite passed 15/15, including malformed entries, missing/null/non-record data, non-array limits, and recognized units with no duration.
  • Targeted formatting and typed lint checks passed with zero warnings or errors.
  • A live request using the scoped machine Z.AI credential reached the real endpoint and returned HTTP 200 with a typed unsuccessful API envelope. The production fetchZaiUsage path converted that into a coherent stable error snapshot without throwing or exposing credential/provider payload values.
  • The available account did not return a successful coding-plan payload, so successful empty-data normalization is covered by focused regression fixtures rather than claimed as live proof.
  • Exact-head CI, including openclaw/ci-gate, passed on 92ad28b71f9ab696e02165a6c280d119bdf31600; two post-change automated reviews reported no actionable findings.

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.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 21, 2026
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants