Repository navigation
fix(usage): key-mask collision, double countdown timer, and OpenCode Go quota (#3640, #3470, #3334) - #3882
Open
ntdatt812 wants to merge 2 commits into
Open
fix(usage): key-mask collision, double countdown timer, and OpenCode Go quota (#3640, #3470, #3334)#3882ntdatt812 wants to merge 2 commits into
ntdatt812 wants to merge 2 commits into
Conversation
…olua#3470) Two places created the quota tracker's intervals and each cleared only what its own ref pointed at: // auto-refresh effect intervalRef.current = setInterval(refreshAll, REFRESH_INTERVAL_MS); countdownRef.current = setInterval(tick, 1000); // visibilitychange handler, on becoming visible intervalRef.current = setInterval(refreshAll, REFRESH_INTERVAL_MS); countdownRef.current = setInterval(tick, 1000); The second assignment overwrites the refs, so whatever they held is orphaned and ticks forever. Two paths reach that: - the tab is hidden (handler clears both, nulls the refs), `refreshAll` changes identity while hidden, and the effect re-runs: its cleanup finds null refs and does nothing, then it creates a pair while the tab is still hidden. Becoming visible creates a second pair on top. - two visible transitions in a row - which split-view and window focus changes can produce - run the resume branch twice. Either way the countdown drops two seconds per second, which is the ~2x speed in the report. Move ownership into createRefreshTimers(): `start()` stops first, so a second start replaces the timers instead of stacking them, and both callers go through it. The effect no longer starts anything while the tab is hidden, and the visibility effect stops the timers on unmount - a tab hidden at mount takes the effect through its early return, which leaves no cleanup behind. `refreshAll` is now read through a ref, so a new identity no longer tears the interval down and restarts the 60s window mid-count. COUNTDOWN_SECONDS is derived from REFRESH_INTERVAL_MS rather than written as a literal 60 in three places.
Keys are minted as sk-{machineId}-{keyId}-{crc}, so every key issued by one
install shares its first 8 characters -- exactly what maskApiKey() returns.
The 24h/today branch of getUsageStats grouped on that mask, so all of an
install's keys collapsed onto one row: the first key seen kept its name and
absorbed everyone else's requests, tokens and cost.
Grouping on the raw key instead, as the report suggests, would regress
AUDIT-002: these bucket names are returned to the dashboard. Probing that
guard showed the daily-summary branch (7d / 30d / all) already breaks it --
aggregateEntryToDay names its stored buckets after the raw key, and those
names were copied into the response verbatim. The source-text assertion only
covered the 24h template, so it never saw it.
Derive both branches from a shared non-secret identity: the key's own id, or
a digest for a key that is no longer registered. Distinct keys get distinct
buckets, and no branch puts the secret in the payload.
Fixes decolua#3640
ntdatt812
force-pushed
the
bundle/usage-quota
branch
from
September 24, 2026 02:45
1ceb806 to
8922b9e
Compare
Contributor
Author
|
Rebased onto One commit came out of the bundle: Worth flagging how that showed up, because it is quiet: the conflict was only over neighbouring lines, so the merge produced a Two commits remain:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three usage/quota fixes that were sitting in separate PRs (#3337, #3537, #3650), rebased onto current
master. Independent commits.feat(opencode-go)(#3334)fix(quota)(#3470)fix(usage)(#3640)The
#3640one is the load-bearing fix: bucketing by the mask both mis-attributed usage and, in the obvious "just group by the raw key" alternative, would have put the key itself on the dashboard. It buckets by key identity instead, which is whytests/unit/security-audit.test.jsis touched — that suite pins that no raw key reaches the response.Verification
Run with vitest 3 and the repo config — both parts matter:
Without
--configthe@/alias does not resolve (Failed to load url @/lib/dataDir.js). And vitest 2.x cannot run anything that reachesopen-sse/translator/index.js: it throwsTypeError: register is not a functionat the top-levelregister(...)calls, becauseindex.jsdeliberately relies on function-declaration hoisting to survive the import cycle while Vite 5's SSR transform turns the named import into a call-time property access. I confirmed that failure on cleanmasterand onmasteras of 15 Aug, so it is not caused by these changes.Every fix was checked by reverting it and confirming which test dies. No claim here rests on a test that passes for another reason.