Restore z.ai rate-window pace - #2802
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 894a6ee431
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex review: needs changes before merge. Reviewed August 9, 2026, 8:21 PM ET / August 10, 2026, 00:21 UTC. ClawSweeper reviewWhat this changesThe PR restores z.ai time-limit durations, adds tightly scoped pace rules for five-hour, weekly, and MCP-monthly windows, and expands regression coverage. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessThis PR fixes the verified five-hour and weekly z.ai pace path, but it does not actually render the claimed MCP-monthly pace in the menu because the extra-window renderer still rejects z.ai. A small code-and-test follow-up is needed before merge. Priority: P2 Review scores
Verification
How this fits togetherThe z.ai plugin converts quota API limits into normalized primary, secondary, and MCP extra usage windows. Descriptor pace rules feed CLI output and menu-card rendering, where the MCP window follows the extra-window display path. flowchart LR
A[z.ai quota API] --> B[JavaScript adapter]
B --> C[Primary, weekly, and MCP windows]
C --> D[z.ai pace descriptor]
D --> E[CLI pace output]
C --> F[Menu extra-window renderer]
F --> G[Menu card pace detail]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Route the z.ai MCP extra window through descriptor-backed reset-window pace rendering, then add a menu-card regression that proves the monthly pace appears while rolling 30-day token and credit windows remain excluded. Do we have a high-confidence way to reproduce the issue? Yes, from source: a z.ai MCP extra window reaches the menu extra-window renderer, whose current provider gate excludes z.ai before its descriptor pace rule can run. Is this the best way to solve the issue? No; the adapter and descriptor repair is sound for five-hour and weekly windows, but it must also connect the MCP extra-window menu path to reset-window pace rendering. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 648a948b7ab4. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
3ab4720 to
a6f15b9
Compare
|
Post-merge proof:
Scope caveat: this PR fixes the remaining z.ai regression only. #2431 tracks multiple providers and remains open. |
Addresses the remaining z.ai regression in #2431.
The JavaScript provider migration kept computing
TIME_LIMITdurations but stopped exporting them. That dropped the established monthly MCP marker and left the z.ai descriptor without its verified 5-hour and weekly pace lanes.This restores the one-minute MCP marker as the existing calendar-month sentinel, preserves explicit
TIME_LIMITdurations, leaves unknown cadence unset, and enables pace only for the verified 5-hour and weekly slots. Monthly inference also requires the adapter's MCP marker, so an explicit rolling 30-day token or credit window retains its duration without being rewritten as a calendar month.Verification:
CI=1 CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --disable-keychain --disable-netrc --filter "ZaiProviderTests|ProviderPaceCapabilityTests|CLISnapshotTests|ProviderPresentationPolicyCharacterizationTests|UsagePaceTextTests|ZaiMenuCardTests|MenuBarPaceTextTests"— 98 tests passedCI=1 CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 ./Scripts/test-plugin-engines.sh— 76 tests passed under QuickJS and 76 under JavaScriptCoremake check— passedCODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 make test— all 833 selections passed in 70 groups; no retries or timeoutsgit diff --check origin/main...HEAD— passedswift build -c release --product CodexBarCLI— passedReal provider proof
A maintainer-owned z.ai QA credential was injected at runtime through the approved 1Password service-account path; the key value, account identity, and usage percentages were never printed or added to the PR. The same command was run before and after the patch.
api; real primary window5-hour;windowMinutes: 300; reset present; no pace object.api; the same5-hour/ 300-minute/reset-bearing window;pace.primarypresent; no provider error.This directly exercises the reported real-provider regression while confirming that authentication and window selection remain unchanged.
Compatibility risk is low: token and credit parsing is unchanged, explicit rolling cadences are preserved, and unknown MCP cadence remains fail-closed.
Review focus: the one-minute marker mapping, the MCP-only monthly discriminator, and the exact z.ai descriptor lane rules.