provider: a background reading of an account writes its quota history too - #1318

Reviewed at 080a832 (merged onto main ddb6f80). Not merging yet. The double-point question checks out, but the write path needs another round.

What was run (Docker, golang:1.26.8, -tags nogui):

• gofmt -l internal/provider is clean.
• go vet for darwin, GOOS=linux and GOOS=windows passes on ./internal/provider.
• go test -count=1 ./internal/provider ./internal/gateway: both ok.
• Review-only tests (not pushed):
  • One Codex account. LoginUsage, then the page's RefreshUsage, then a cached LoginUsage, then 5× StaleAllowance + LoginUsage. Result: one account, one line, one point that moved, 145-byte file.
  • Two Codex accounts ([email protected], [email protected]). Page read, then routing read, 1.1 s apart with usage changed. Result: one line per account (the user is lowercased in the key), one point each, which moved to the new value.

So no double points. A page read through loginReading carries the same ReadAt as the background note, and addPoint drops it as not newer. A separate read within 30 s / 5 min replaces the last point. Growth is bounded as before: 45 days and quotaHistMax per window.

What needs changing:

1. Write amplification. noteQuotaHistory reads the whole quota-history.json, parses it, and rewrites it on every call that changes anything. A newer reading always changes the last point's At, so every kept reading triggers a rewrite. Before this PR, the page noted a whole batch in one rewrite. Now each account's kept reading rewrites the file on its own. LoginUsage over N accounts means N full read/parse/write cycles per minute, every minute, whether or not anyone looks at Usage. The page path also goes from 1 write to N+1. With a full history (6000 points × 2 windows per account, ~1.4 MB per account), 10 accounts means a ~14 MB file rewritten ~10× a minute.
2. It runs before close(r.done). Every routing waiter on that account (loginUsageAt → Allowances) now waits for the disk read and write. The writes also serialize across accounts on quotaHistMu. So the slowest account's history write is added to routing's allowance read. Reading history should not slow down routing.

Suggested shape: note once per loginUsageAt call, after wg.Wait(), with the fresh readings of that batch. Cached ones are no-ops, since their ReadAt isn't newer. Do it off the request path, e.g. a goroutine, or at least after every close(r.done). Then SubscriptionUsage's batch and routing's batch each cost one rewrite. Please add a test that LoginUsage over several accounts writes the file once, and that a waiter isn't held by a slow history write.

Both findings reproduced here before changing anything (on 080a832, with a counter/blocker around the per-account note): two accounts cost 2 noteQuotaHistory calls, and a held history write held LoginUsage past 2 s until released. Both real.

Fixed in d709188, along your suggested shape:

• loginReading no longer notes anything. loginUsageAt notes the batch once after wg.Wait(), each reading named for its account, so N accounts cost one read/parse/write. Cached readings are the no-ops you described (ReadAt not newer), failed ones are skipped.
• The write runs in its own goroutine (noteHistory -> noteHistoryWrite), so it is off the request path and no waiter on close(r.done) or on LoginUsage touches the disk.

Regression tests, both in internal/provider/quota_history_background_test.go:

• TestBackgroundReadsNoteHistoryOnce: two Codex accounts, one LoginUsage; asserts one history write carrying both accounts, and a line for each.
• TestHistoryWriteHoldsNoWaiter: the write is held on a channel; LoginUsage still answers while it is held.

Run here (-tags nogui): the three history tests pass, also at -count=2; full go test ./internal/provider ok (45.8s); go vet clean. Not run: full gateway suite (this box's per-command limit; its quota/remote tests passed before), WebKit/GUI, a real ChatGPT account.

Reviewed d7091881, merged onto main 10578ce6. Thanks. Both earlier points are addressed: one write per batch, and the write is off the request path. Not merging yet, because the detached goroutine leaks out of every test that calls LoginUsage.

What was run (-tags nogui, temp HOME, pinned Go caches):

• The three new tests pass, also with -race -count=3.
• Break checks:
  • Without the noteHistory call, TestBackgroundLoginReadNotesQuotaHistory fails ("background read noted no history") and so does TestBackgroundReadsNoteHistoryOnce ("noted in 0 writes").
  • Noting per account fails TestBackgroundReadsNoteHistoryOnce ("history noted in 2 writes, want 1").
  • Making noteHistory synchronous fails TestHistoryWriteHoldsNoWaiter ("LoginUsage held by the history write").
• go test ./internal/provider is red with this PR and green on main. On main, two -race runs in a row are both ok. With the PR, 3 of 3 runs failed, with and without -race, each time in different tests:

--- FAIL: TestFactoryKeyAgainReadsUsageAfresh
    testing.go:1464: TempDir RemoveAll cleanup: unlinkat .../001/.config/magpie: directory not empty
--- FAIL: TestClaudeUsageExpiredReadingDoesNotSwitchAccounts / TestClaudeUsageFailureDoesNotHideAccountErrors / TestCodexSignInAgainReadsUsageAfresh / TestZCodeTeamSignIn ...

One -race -count=2 run also reported a DATA RACE.

Cause: the default noteHistory is go noteHistoryWrite(...). Every test that reaches loginUsageAt and doesn't swap noteHistory starts a goroutine. That goroutine writes quota-history.json under filepath.Dir(Path()) after the test has returned, while t.TempDir is being removed. Or it writes into the next test's HOME, because Path() is read when the goroutine runs. This is the rule in LESSONS.md: "A fake or child process a test starts ends when the test does". It applies to goroutines too.

Suggested fix (either):

• Note synchronously in loginUsageAt after wg.Wait(). By then every close(r.done) has happened, so no routing waiter is held. Only the LoginUsage caller pays for one write per batch, and Allowances already calls it from its own background goroutine. TestHistoryWriteHoldsNoWaiter would then check a second waiter on the same account instead of the caller.
• Or keep the goroutine, but resolve the path before it starts, and track it with a sync.WaitGroup that the provider (and gateway/gui) test cleanup waits on.

Smaller point: in TestHistoryWriteHoldsNoWaiter, <-entered has no timeout. With the note removed, the test hangs until the package timeout (60s here) instead of failing.

Thanks for the careful run. The mechanism is clear from your logs - the detached goroutine reads Path() and writes after the test has returned, racing TempDir removal and the next test's HOME. I took your first option: the note is now synchronous in loginUsageAt after wg.Wait(). Every reading has completed and released its in-flight marker by then, so no routing waiter is held. Only the LoginUsage caller pays for the one write per batch, and Allowances already calls it from its own background goroutine.

TestHistoryWriteHoldsNoWaiter now checks a second caller on the same account, as you suggested: the write is held during the first LoginUsage, the test asserts that caller has not returned (200ms window), then a second LoginUsage on the same account answers from the completed readings. The write-start wait has a 10s timeout now. On the previous head this test fails immediately ("LoginUsage returned before the held write finished"), so it pins the new semantics.

Ran on beccb9a: the three history tests with -race -count=3 pass; go test -tags nogui ./internal/provider passes; go vet and gofmt clean. Not run: the full ./... suite and the GUI. One honest note: my machine did not reproduce the 3/3 red you saw (one full package run passed even before this change) - the fix removes the cause your runs identified rather than a failure I could trigger here.

Reviewed beccb9af, merged onto main ed31293c. Merging. The leak is gone, and both earlier points still hold.

What was run (macOS, go1.26.8, -tags nogui, temp HOME, pinned Go caches):

• go test -race -count=1 ./internal/provider, 4 runs in a row: 4 × ok (63–85 s). No DATA RACE, no TempDir cleanup failures. Before this head the same command was 3 of 3 red.
• The full suite, go test -count=1 ./...: all 44 packages ok. That includes internal/gateway (347 s), internal/gui and internal/provider.
• go vet on ./internal/provider passes for darwin, GOOS=linux and GOOS=windows. go build passes for all three. gofmt -l is clean.
• Break checks on the three history tests:
  • The note in a detached goroutine (the previous head) fails all three. TestHistoryWriteHoldsNoWaiter fails with "LoginUsage returned before the held write finished".
  • With no note, all three fail. The write-start wait now times out at 10 s instead of hanging.
  • Noting per account fails TestBackgroundReadsNoteHistoryOnce ("noted in 2 writes, want 1").
• Read line by line: loginUsageAt notes once, after wg.Wait(). Every reading's close(r.done) and the in-flight marker removal are done by then, so the one write per batch holds no waiter on a reading. Its callers are all background loops: Allowances' goroutine, the switch and warm-up loops, CodexUsedUp, the CLI and the GUI's /api/login/usage. The routing request path is not one of them.

Non-blocking, for a follow-up:

1. TestHistoryWriteHoldsNoWaiter doesn't pin "no waiter held". Its second caller arrives after the reading has landed and answers from the cache. If you move the note back into the reader goroutine before close(r.done) (the original point 2), it still passes, at 1 account and at 2. A review test that does catch it:

   • hold the fake vendor, start caller A, start caller B while the reading is in flight, then release the vendor;
   • hold the first history write;
   • require one of the two callers to return within 3 s.

   It passes 10/10 at this head (-race). With the note before close(r.done) it fails 10/10 ("both callers held by one history write"). A joiner still pays for its own note, so a test that requires B specifically to return is wrong: whichever caller notes first is the one held.

2. Every LoginUsage call reads and parses quota-history.json under quotaHistMu, a cached one included. It writes nothing in that case, but the read happens. With callers at most about once a minute per agent, that cost is small. Skipping the note when no reading in the batch is newer than the last noted one would avoid the read.

已在 v0.1.1124 发布，谢谢 @charan-rathore！

Thanks for the quick release!