fix(usage): render the quota panel again and restore reset countdowns - #4656
Conversation
46525ad to
337c46e
Compare
|
Rebased onto dev Conflict: CHANGELOG Interaction with #4635 — checked, and it strengthens this PR: Validation on
Pre-existing failure, not from this branch: |
|
Correction to my earlier comment on this PR, body updated to match: I described the pre-existing It is test hermeticity — the hooks are declared on the exported Nothing here changes this PR: its files, tests, and results are unaffected — the note only clarified why an unrelated suite is red on developer machines while green in CI. Head remains |
The multi-account usage panel dropped the `resets in …` line entirely: it was gated on a window having a single limit, so the moment a second credential appeared the only reset signal left was a parenthetical that competed with the account label for column width. Coarse single-unit durations made it worse -- a weekly window read `7d` whether 6.6 or 7.4 days remained -- and per-account labels right-truncated into identical stubs, so the panel could not answer either question it exists for: when does my quota come back, and for which account. Always resolve the reset range, render it with hour precision plus an absolute local reset time, and fit account labels as a set so the columns stay mutually distinguishable. Lore-id: 7c1a9f3e Constraint: keep formatDuration untouched -- job/elapsed rendering shares it Rejected: widen columns until labels fit | wraps or overflows narrow terminals Rejected: drop the reset suffix from the header row | loses per-account skew Confidence: high Scope-risk: narrow Reversibility: safe Tested: multi-account reset line, divergent/identical reset windows, 48h+ precision, label uniqueness at 80 cols Not-tested: live provider fetch against real credentials
Canonicalizing multi-account management (364f140) rewired the interactive /usage handler from the graphical panel to the account inventory text view. That view prints only `label: N% used (M% left)`, so bars, per-account columns, and every reset signal disappeared from the command whose entire job is answering "how much is left, and when does it come back" -- and handleUsageCommand/renderUsageReports became unreachable code that no surface could reach. Plain /usage renders the panel again, fed by the same cache-only inventory snapshot the text view reads, so the cache-only contract that motivated the rewire is preserved and no fetch or probe returns. `/usage check` stays on the text path, where the per-credential health verdict is the point. Account rows on every surface now carry the reset countdown and its absolute time. Lore-id: 7c1a9f3e Constraint: plain /usage must remain cache-only -- no fetch, no probe Constraint: keep formatDuration untouched -- job/elapsed rendering shares it Rejected: add resets to the text rows only | leaves the panel dead and the bars gone Rejected: revive the panel via session.fetchUsageReports | reintroduces the network call 364f140 removed Confidence: high Scope-risk: narrow Reversibility: safe Tested: panel reset lines, divergent/identical windows, 48h+ precision, label uniqueness at 80 cols, text-row reset detail incl. expired/absent windows Not-tested: live provider fetch against real credentials
337c46e to
55469da
Compare
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Adversarial exact-head review — APPROVE on 55469dae4c8295d8bd4f2ae7ff96586e93dedb36
Reviewer: Yeachan-Heo (maintainer; independent of PR author kook-oh). Base 2bd7b4a48c (immutable event base, current origin/dev). Canonical source digest e3819dca7d508b2bf9818387c49f4417176c6a9be2946e92d1352687e72eef1f — recomputed locally with git diff --binary --full-index --no-ext-diff 2bd7b4a48c...55469dae4c, byte-identical to the verdict line in the PR body.
Authorship and reconstruction
- Both commits authored and committed by
오승국 <kook@oseung-gug-ui-MacBookAir-2.local>(kook-oh), original author dates 2026-08-18 preserved. The contributor performed their own rebase onto9f29ee590→337c46e72→55469dae4c; no maintainer force-push was needed on this branch. - PR code/test paths are byte-identical to the original head
46525ad98a(path-scoped diff overcommand-controller.ts,builtin-registry.ts,usage-report.ts,usage-report-columns.test.tsis empty); the only per-head delta vs27afb732b3is CHANGELOG[Unreleased]sibling ordering, both entries kept.
Semantics — checked against merged #4635 and in-flight #4663
- Cache-only contract preserved.
collectCachedUsageReports()readsrow.usage.reportout ofbuildAccountInventorySnapshot()— the same redacted snapshot the text view reads. No fetch, no probe, nosession.fetchUsageReports.redactUsageWindow()ataccount-inventory.ts:238preservesresetsAt, andredactUsageReport()allowlistsemail, so both the per-account labels and the reset lines survive the inventory path intact. Interaction with #4635 is complementary: #4635 taught the snapshot to resolve the provider base URL (without it this panel would render a subset of accounts); this PR renders resets. /usage checkstays text. Plain/usage→ panel when cached reports exist, text fallback otherwise;checkalways goes throughbuildUsageReportText({check:true})with the health verdicts. Both surfaces now carryresets in <countdown> (<absolute time>)viaformatLimitDetail().- Reset range fix is real. The old gate
sortedLimits.length <= 1 ? resolveResetRange(...) : nullsuppressed the only reset signal exactly when multiple credentials existed. Now always resolved; divergent resets rendermin–max (first <abs>), identical resets one value. Expired/window-less limits emit no reset text (filtered byvalue > nowMs/ early return). - Two-unit countdowns are local.
formatResetCountdown/formatLimitReset/formatResetAtare new local helpers;formatDurationis untouched, so job/elapsed rendering keeps its coarse label.formatResetShort(status-line path) unchanged. - Label set-fitting is monotone and terminating.
truncateAccountLabelstries full → local-part → head-truncated → middle-squeezed, keeping the first mutually-unique variant; the#Nfallback guarantees uniqueness even when all representations collide. Padding usesMath.max(0, ...)so narrow terminals can't produce negative repeats (the pre-existing code would have). - Panel is reachable again, dead code is not accumulated.
handleUsageCommand/renderUsageReportsregain their only interactive caller; the fallback text path remains for empty snapshots.
Pre-existing red, correctly out of scope
account-inventory-usage.test.ts (from #4635) fails 2/2 on this machine and on clean origin/dev — test hermeticity against developer-shell credential resolution, tracked as #4661 with the fix in #4663. This PR does not touch account-inventory.ts and correctly declines to reach into that feature's files.
Verification on this exact head (hermetic, local)
bun test ./packages/coding-agent/test/usage-report-columns.test.ts ./packages/coding-agent/test/status-line-usage.test.ts— 15 pass / 0 fail; addingsession-manager/usage-statistics→ 18 pass / 0 failbun --cwd=packages/coding-agent run check— clean (biome 2837 files,tsc --noEmit)bun scripts/verify-gjc-state-writers.ts --fail --root .— 0 write sites outside the sanctioned writerbun scripts/changelog-history-guard.ts— no released sections removedgit diff --check 2bd7b4a48c 55469dae4c— clean- CLI smoke from source:
--version→gjc/0.14.0;accounts list --json→ valid inventory JSON - Canonical binaries: dev
2bd7b4a48c→94653606…, head55469dae4c→5dcb965a6a2ed3476fdc9e5d31f6ef842fae35be449831318df000809ee027a1; head binary--version/--help/--smoke-test(smoke-test: ok) all green - Product CI on this exact head: 15 success / 6 skipped — including
native-build,check:@gajae-code/coding-agent,cli-smoke,ts-build, targetedusage-report-columnsshard, all fivegjc-state-gatesshards, evidence producer, Virtual integration, Public site sync. The only reds are the two PR-contract jobs, whose sole error isVerdict needs-human intentionally blocks merge— the designed gate.
No blockers. Approving this exact head.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Terminal disposition — MERGED to devPR #4656 Exact-head merge basis (head
|
…Yeachan-Heo#4656) * fix(tui): restore reset countdown and account identity in /usage The multi-account usage panel dropped the `resets in …` line entirely: it was gated on a window having a single limit, so the moment a second credential appeared the only reset signal left was a parenthetical that competed with the account label for column width. Coarse single-unit durations made it worse -- a weekly window read `7d` whether 6.6 or 7.4 days remained -- and per-account labels right-truncated into identical stubs, so the panel could not answer either question it exists for: when does my quota come back, and for which account. Always resolve the reset range, render it with hour precision plus an absolute local reset time, and fit account labels as a set so the columns stay mutually distinguishable. Lore-id: 7c1a9f3e Constraint: keep formatDuration untouched -- job/elapsed rendering shares it Rejected: widen columns until labels fit | wraps or overflows narrow terminals Rejected: drop the reset suffix from the header row | loses per-account skew Confidence: high Scope-risk: narrow Reversibility: safe Tested: multi-account reset line, divergent/identical reset windows, 48h+ precision, label uniqueness at 80 cols Not-tested: live provider fetch against real credentials * fix(usage): render the quota panel again and restore reset countdowns Canonicalizing multi-account management (364f140) rewired the interactive /usage handler from the graphical panel to the account inventory text view. That view prints only `label: N% used (M% left)`, so bars, per-account columns, and every reset signal disappeared from the command whose entire job is answering "how much is left, and when does it come back" -- and handleUsageCommand/renderUsageReports became unreachable code that no surface could reach. Plain /usage renders the panel again, fed by the same cache-only inventory snapshot the text view reads, so the cache-only contract that motivated the rewire is preserved and no fetch or probe returns. `/usage check` stays on the text path, where the per-credential health verdict is the point. Account rows on every surface now carry the reset countdown and its absolute time. Lore-id: 7c1a9f3e Constraint: plain /usage must remain cache-only -- no fetch, no probe Constraint: keep formatDuration untouched -- job/elapsed rendering shares it Rejected: add resets to the text rows only | leaves the panel dead and the bars gone Rejected: revive the panel via session.fetchUsageReports | reintroduces the network call 364f140 removed Confidence: high Scope-risk: narrow Reversibility: safe Tested: panel reset lines, divergent/identical windows, 48h+ precision, label uniqueness at 80 cols, text-row reset detail incl. expired/absent windows Not-tested: live provider fetch against real credentials --------- Co-authored-by: 오승국 <kook@oseung-gug-ui-MacBookAir-2.local> (cherry picked from commit b01f345)
Fixes #4661.
What
Makes
packages/coding-agent/test/account-inventory-usage.test.tspass regardless of what credentials exist on the machine running it. Test-only change; no product file touched.peekCachedCredentialHealthForSource,recordCredentialHealthForSource,peekApiKey,checkApiKeyCredential.source === "stored"+ provider) instead ofrows[0].Why
The suite fails 2/2 on dev tip
2bd7b4a48with no changes applied:providerSet()adds every provider fromlistProvidersWithEnvKey()whose key resolves — 62 providers are scanned — and each one produces a synthetic row whosesourceHealth()call the minimal stub cannot answer. Once the crash is out of the way, a second failure surfaces:rows[0]is the synthetic row, not the stored credential, sorows[0]?.usageisundefined.Two details make this broader than an exported-variable problem, and correct the reproduction note on #4635:
env -idoes not avoid it.$credentialEnv()resolves through$inheritedEnv → live credential store → ~/.gjc/agent/.env → piEnv → ~/.env → shell-rc parsing, so a cleared process environment still sees file-backed credentials.amazon-bedrockfalls throughAWS_BEARER_TOKEN_BEDROCKtohasResolvableAwsProfileSource(), so a plain~/.aws/config+~/.aws/credentialsis enough. That is what triggers it on the machine where this was found — measured withlistProvidersWithEnvKey().filter(getEnvApiKey)returning["amazon-bedrock"]under both a normal shell andenv -i.CI stays green because runners carry no credentials and therefore build no synthetic rows, so the failure only ever reaches developer machines.
I first filed #4661 as a product defect and then, in a follow-up, attributed the trigger to a specific API-key variable. Both were wrong and are corrected on the issue. A type probe confirms
peekCachedCredentialHealthForSourceandcheckApiKeyCredentialare declared on the exportedAuthStoragetype, so the runtime contract is intact and the fault is the stub lying throughas unknown as AuthStorage. Guarding the call sites inaccount-inventory.tswould mask genuinely missing methods on a real storage, so the fix belongs in the test.Testing
bun test packages/coding-agent/test/account-inventory-usage.test.ts— 2 pass / 0 fail in three configurations:~/.awsprofile present (the original failing case)env -i(cleared process environment)OPENAI_API_KEYandANTHROPIC_API_KEYexportedbun --cwd=packages/coding-agent run check— clean (biome 2836 files +tsc --noEmit).GJC verdict
devbun checkpasses (coding-agent package check on exact head6b92e97c8)