fix(usage): cap usage.jsonl in scopes that never report (#788)

[SaulMoro]

Fixes #788
Fixes #803
Part of #752

Summary

 pull, report step (pendingUsageReport)
   for each reporting target
     N = readUsageEvents(scope).length
     reportUsageToTeam(...)
     if reported: truncateUsageAfterReport(N)   # deletes first N lines
+  for each active scope (project, user), reporting or not
+    capUsageEvents(scope)                     # ≤ 5,000 lines → file untouched

+ every writer of the usage file takes <usage file>.lock (acquireLock from update.ts, bounded retry)
+   holder first: fold <usage>.pending-<uuid>.jsonl side files into the file (skip one whose pendingId the file already holds), remove them
+   hook append      wait ≤ ~250 ms; else write own side file (O_EXCL, mode ≤ the usage file's, 0600 if none; line carries the file's uuid as `pendingId`)
+ readUsageEvents keeps skill, timestamp, tool only: pendingId never reaches stats, recommendations or stats/<user>.yaml
+   truncate / cap   wait ≤ ~5 s;   else leave the file as it is, log which lock to remove
+     rewrite = realpath(file); drop orphan temps; read; keep lines
+               write <file>.<pid>.<random>.tmp with the file's mode; rename over file
+ in-workspace .teamai/.gitignore (self, legacy project)
+   usage.jsonl
+   usage.jsonl.*            # lock, its link temp, rewrite temps
+   usage.pending-*.jsonl    # side files
+   existing self-mode file: added by migrateSelfModeGitignore on pull / push / contribute (writeFileAtomic)
+   existing project-scope file: added by ignoreUsageSideFiles before a side file or a rewrite temp is written (writeFileAtomic)

The cap runs after every truncate, never between a report's read and its truncate: there it would shift the deleted lines onto unsent events (#750). USAGE_EVENT_CAP = 5_000, the size of DASHBOARD_COMPACTION_THRESHOLD. No writer goes past the lock, so a rewrite never races an append or another rewrite (http scopes included, which hold no sync lock).

Evidence

Real CLI on head 0f920ef, sandbox HOME, user scope (probe-same-ms.sh). A live process holds the usage lock; two Claude Skill hooks record held at once with Date pinned to one millisecond (node --require fixed-date.cjs), so the two side files hold the same event; the lock goes away; the hook fires again; then teamai stats:

build	side files while held	held in the usage file after the fold	teamai stats
97a394b	2, byte-identical	1 (one event lost)	held 1
0f920ef	2, same event, own pendingId	2, no side file left	held 2; no pendingId in the output

Real CLI, sandbox HOME, user scope (probe-refold.sh). A live process holds the usage lock and the Claude Skill hook records held-1 in a side file; then the side file's line is appended to the usage file with the side file left behind (a holder that died between its append and its rm); then the hook fires again:

build	held-1 in the usage file after the next hook	side files left
064370e	2	0
97a394b	1	0
0f920ef	1 (matched by its pendingId)	0

Real CLI on head fc5f536, sandbox HOME, legacy project install (config and data in <workspace>/.teamai, committed .gitignore that lists only usage.jsonl). A live process holds the usage lock; the Claude Skill hook (hook-dispatch post-tool-use --tool claude --matcher Skill) fires and records into a side file (probe-legacy.sh):

build	git status --porcelain --untracked-files=all
8ab9be0 (previous head)	?? .teamai/usage.jsonl.lock, ?? .teamai/usage.pending-<uuid>.jsonl
fc5f536	M .teamai/.gitignore (+usage.jsonl.*, +usage.pending-*.jsonl); no usage file listed

Real CLI on head 583f908, sandbox HOME, user scope. A live process holds the usage lock; hook-dispatch prompt-submit --tool claude fires; the lock goes away; the hook fires again:

step	result
lock held, usage file 0600	user-usage.pending-<uuid>.jsonl is -rw------- (was -rw-r--r-- on 42fa895)
lock released, next hook	folded in: first, second, third in order; no side file left

The runs below are from before the merge and still describe the lock and cap.

Real CLI on head 42fa895, sandbox HOME. probe-pending.sh: a live process holds the user scope's usage lock, then the agent's hook-dispatch post-tool-use --tool <agent> Skill hook fires, then pull:

step	claude	codex
hook while the lock is held	428 ms; usage file unchanged; 1 side file	442 ms; unchanged; 1 side file
pull while the lock is held	5100 lines, untouched	same
holder exits, next hook	side file folded in (held-1 + after-1 present), no side file or lock left	same
pull	5000 lines, both events kept	same

Representative git cell (matrix.sh, git × claude, on 42fa895). A user scope is seeded with 100 legacy then 5,000 review events, mode 0600, and hooks keep firing during the pull (at most 4 in flight):

scenario	origin/main	this PR
usageReport: false	5101 → 5101	5101 → 5000, then the 7 hooks after the cap; hooks 31/31 kept; mode 600; nothing left beside the file
rejected push, then retry	5101 kept; retry reports legacy	5101 → 5000; retry reports review 4999 + by-claude 1, no legacy; file empty after

The full 12-cell matrix (git/gitlab/github × claude/codex/codebuddy/opencode) passed on fa67878, the same change before the pending side files: 15/15 … 24/24 hooks kept in every cell.

http scope, two pulls at once while the Claude hook keeps firing (probe-http.sh):

build	≤ 4 hooks in flight, 5 rounds	hundreds of concurrent hooks, 2 rounds
2cce4a8 (carry-over)	43/43 kept, mode 644	202/202 kept
fa67878 (unlocked fallback)	48/48, mode 600	1 of 164 lost; 73 of 657 lost in a heavier run
this PR	45/45, mode 600	287/287 kept, no side file or lock left

Tests, red on the previous head → green:

test	before
an append that gives up on the lock while a slow cap rewrites the file is kept	fail on b8e857d (5001 lines, late lost)
lock held: the hook records in a side file within 1 s; the next holder folds it in	fail on b8e857d (appended unlocked)
lock held: neither the cap nor the truncate rewrites the file	fail on b8e857d (rewrote after 5 s)
append issued while the cap holds the lock lands after it; same for the truncate	fail on 2cce4a8
two caps plus 5 appends at once: every append kept once	fail on 2cce4a8 (late0 twice)
mode 0600 kept; orphan rewrite temp removed; dead-owner lock reclaimed	fail on 2cce4a8
generated self-mode .gitignore ignores the lock, a rewrite temp and a side file (git check-ignore)	fail on 42fa895
an older self-mode .gitignore that lists only usage.jsonl gains those patterns	fail on 42fa895
side file gets the usage file's mode (0600); 0600 while there is no usage file	fail on 42fa895 (0644)
legacy project .teamai/.gitignore listing only usage.jsonl: a side file stays out of git status; after a cap the lock and a rewrite temp are ignored	fail on 8ab9be0
side file still being written (no newline) is left alone; failed temp write leaves the file byte-identical	pass (guards)
disk fills partway through healing the legacy .gitignore: it stays byte-identical, no temp left, the cap still lands	fail on fc5f536 (file truncated)
disk fills partway through healing a self-mode .gitignore: it stays byte-identical, no temp left	fail on 064370e (config.yam)
a side file that outlives its append (rm fails) is folded once and removed by the next holder	fail on 064370e (b twice)
a side-file event identical (skill, tool, ms) to one already in the file is kept	fail on 97a394b (['a','b'])
two identical events recorded in two side files at once are both kept	fail on 97a394b (['a','b'])
readUsageEvents and teamai stats output carry no pendingId; the reported stats/<user>.yaml neither	fail on 97a394b (no id written); report test is a guard

#803's own repro (an append injected between the truncate's readFile and its writeFile), run on 42fa895: the event is kept. On origin/main 8cee7ab: expected [ 'd' ] to deeply equal [ 'c', 'd' ]. The "same for the truncate" test above covers that interleaving.

Test plan

On head 0f920ef (origin/main ec56a67 merged in):

• npx tsc --noEmit: clean
• npx vitest run: 306 files, 4707 passed, 1 skipped
• npm run test:e2e: 46 files passed, 3 skipped; 242 tests passed
• npm run build + the same-millisecond real-CLI run above

CI review 5815927552 (on e80b19e, the tests-only push): P1 (the id was only in the tests; the fold still matched whole lines, so two identical concurrent events lost one) fixed in 0f920ef: side files carry pendingId, the fold matches that id, readers drop it. P3 (a side file whose rm keeps failing is appended again after a report truncates its line) not changed: telling it apart would need a record of folded ids outside the file the report truncates, for a side file that cannot be removed at all; stated in Merge Danger.

CI review 5815707212: P1 (fold deduped by the whole line, so identical concurrent events lost one) fixed in 0f920ef as above.

CI review 5815129038: P3 (self-mode .gitignore heal in migrateSelfModeGitignore rewrote the file in place) fixed in 97a394b with writeFileAtomic. P3 (non-idempotent fold) fixed in 97a394b (the fold skipped a line the usage file already held; by id since 0f920ef).

CI review on fc5f536: P3 (the legacy .gitignore heal rewrote the file in place, so ENOSPC or a kill could leave it empty and expose token/env) fixed in 064370e with writeFileAtomic. P3 (non-idempotent fold) fixed later in 97a394b.

CI review 5813925880: P1 (an existing legacy project .gitignore never gains the patterns) fixed in fc5f536. P3 (non-idempotent fold) fixed later in 97a394b.

CI review 5813562508: P1 (gitignore) and P2 (side-file mode) fixed above. P3 (a holder killed between appending a side file and removing it re-appends that event) fixed later in 97a394b.

Merge Danger

Door: one-way for the data it drops. The oldest events beyond 5,000 are deleted; a rollback stops the cap but does not restore them. In a reporting scope whose report does not complete while it holds more than 5,000 events, those events were never reported.

Blast Radius: local usage files and hook latency.

• A lock that is never released (one that names no owner is never reclaimed, [bug] acquireLock can hand one lock to two live processes #760) stops every rewrite: each cap is skipped after its wait, and a truncate that cannot run leaves the reported events in place, so the next report sends them again. The error names the lock to remove. Meanwhile hooks write side files, which the first holder after removal folds in.
• A side file is folded by the next lock holder: the next hook append, or the next pull's truncate or cap. Until then teamai stats and the report do not see that event.
• A side file whose pendingId the usage file already holds is removed without being appended (a holder died or could not remove it after its append). A side file that can never be removed is appended again once a report has truncated its line away.
• Each hook append does the lock's create, link and remove, plus a directory listing for side files: a few ms uncontended.
• A hook killed mid-acquire (its 4.5 s deadline under extreme load) leaves <usage file>.lock.<uuid>.tmp; a rewrite killed before its rename leaves <file>.<pid>.<random>.tmp (removed by the next rewrite). An in-workspace .teamai/.gitignore ignores both, the lock and the side files. An existing self-mode one gains those entries on the next pull or push. An existing project-scope one gains them just before the first side file or rewrite temp is written, silently (hooks stay quiet), so where it is committed it shows as modified until the member commits it.
• A pull whose report hangs (rather than fails) past the session-start exit is capped by the next pull that runs to completion.
• Windows: not run (CI is macOS/Ubuntu).

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:315 — The snapshot-and-rename sequence can delete newly appended usage events. If a hook calls appendUsageEvent() after the read at line 311 but before this rename, it appends to the old file, which is then replaced by the stale 5,000-line snapshot. HTTP and usageReport: false scopes have no synchronization preventing this, so the newest unreported event can be lost. Per-PID temp names avoid temp-file collisions but do not protect concurrent appends to the destination.
• [P2 non-blocking] src/pull.ts:2023 — The new mocked export was added only in pull-scope-isolation.test.ts. The usage-tracker mocks in pull-learnings-deletion.test.ts, pull-placement-reconcile.test.ts, pull-publishes-queue.test.ts, and pull-sync-truth.test.ts omit capUsageEvents. Accessing the missing mock export throws inside the broad auto-report catch, so those tests silently skip the entire reporting block instead of exercising it.

The PR description includes a test plan and real-CLI/e2e verification record, so no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:311 — The read-then-writeFile cap can delete events appended after the read. Scope sync locks do not cover hook writers, and HTTP scopes have no lock at all, so a concurrent hook or pull can append a fresh event that is then overwritten by the stale 5,000-line snapshot. The new slow-report guard only appends before truncation/capping and does not exercise this race.
• [P2 non-blocking] src/pull.ts:2023 — The full usage-tracker mocks in pull-learnings-deletion.test.ts, pull-placement-reconcile.test.ts, pull-publishes-queue.test.ts, and pull-sync-truth.test.ts still omit capUsageEvents. Its invocation therefore throws inside the broad auto-report catch, allowing those tests to pass without completing the new reporting/capping path.

The PR description includes both a test plan and real-CLI/e2e verification, so no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:311 — The read-then-writeFile cap still races with hook writers. An event appended after the read at line 308 can be erased when the stale 5,000-line snapshot truncates and overwrites the file. Scope locks do not cover hooks, and HTTP scopes have no lock.
• [P1 blocking] src/usage-tracker.ts:311 — In-place writeFile truncates the usage file before the replacement is durable. A process termination, disk-full error, or partial write can destroy or corrupt all retained events—especially serious for HTTP and usageReport: false scopes where this file is the only statistics source.

Resolved

• The previously reported missing capUsageEvents exports were added to all affected full-module mocks.
• The PR description contains a test plan and real-CLI/e2e record. Its record names older commit 0ea7884, which is a note rather than a blocking finding under the review rules.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:342 — The carry-over still loses hooks that opened the old inode before the rename but append after source.stat(). The copied range is fixed from that one size snapshot, so later bytes remain on the unlinked inode and disappear when the handle closes. The original concurrent-append data-loss problem therefore remains, albeit with a smaller window.
• [P1 blocking] src/pull.ts:2076 — Concurrent HTTP pulls can lose events despite the carry-over. If both caps open the original file, the first renames its replacement, a hook appends to that replacement, and the second then renames its stale temp over it, the event exists in neither second pull’s old-file handle nor its temp. HTTP scopes deliberately have no cross-process lock.
• [P1 blocking] src/usage-tracker.ts:327 — After the rename, failure or termination during copyAppended() cannot restore the old file. An ENOSPC, short/failed append, or process kill at this point can discard or partially copy the old inode’s concurrent tail, contradicting the claimed crash/full-disk safety.
• [P2 non-blocking] src/usage-tracker.ts:326 — Replacing the file with a newly created temp resets its permissions to the process umask. A user-restricted usage file such as mode 0600 can become 0644; preserve the source mode on the replacement.

Resolved

• The direct in-place truncation/durability issue is fixed for the initial capped snapshot.
• The missing capUsageEvents exports were added to the affected mocks.
• The PR description includes a test plan and real-CLI/e2e verification, so no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:342 — A hook that opens the old file before the rename but writes after source.stat() leaves its event on the unlinked inode; copyAppended() never sees it, so the event is permanently lost.
• [P1 blocking] src/pull.ts:2076 — Concurrent HTTP pulls can overwrite newly recorded events: both read the original file, the first renames its snapshot, a hook appends there, then the second renames its stale snapshot over it. HTTP scopes have no synchronization preventing this.
• [P1 blocking] src/usage-tracker.ts:329 — Once the rename succeeds, an ENOSPC, I/O failure, or process termination during copyAppended() cannot restore the old file path, permanently discarding some or all concurrently appended tail bytes.
• [P2 non-blocking] src/usage-tracker.ts:326 — Replacing the file with a newly created temporary file resets permissions. For example, a 0600 usage file becomes 0644 under a typical umask; preserve the source mode before renaming.

Resolved

• All affected full-module mocks now export capUsageEvents.
• The pre-rename write failure leaves the original file intact.
• The PR description includes a test plan and real-CLI/e2e verification, so no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:336 — withUsageLock() deliberately runs writers without the lock after timeout. If rewriting an oversized or slow usage file takes over 250 ms, a hook times out, appends to the old path, and the subsequent rename replaces that file, permanently losing the event. The 5-second rewrite fallback similarly allows concurrent caps to overwrite each other, so the original data-loss race remains.

Resolved

• The old-inode carry-over and post-rename copy failure paths were removed.
• Replacement files now preserve the original mode.
• All affected full-module mocks export capUsageEvents.
• The PR description includes a sufficient real-CLI/e2e record; its older commit reference is non-blocking.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:336 — Writers proceed without synchronization after the lock timeout. If capping an existing large file takes over 250 ms, a hook can time out, append to the current file, and then have that event erased when the cap renames its stale snapshot over the file. Likewise, a rewrite exceeding five seconds allows concurrent caps to overwrite each other. This preserves the original data-loss race for precisely the oversized files this change targets.

Resolved

• The old-inode carry-over and post-rename copy failure paths are removed.
• Replacement files now preserve the original mode, and all affected mocks export capUsageEvents.
• The PR description includes sufficient tests and real-CLI verification; the record referencing an older commit is non-blocking under the review rules.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:246 — The new pending files are not covered by the generated in-directory .teamai/.gitignore, which ignores only usage.jsonl. In a supported legacy/self in-workspace data home, a hook timing out on the lock leaves .teamai/usage.pending-<uuid>.jsonl; a subsequent git add . stages that machine-local usage event and can publish it. Add the pending and rewrite-temp patterns to every applicable generated/migrated gitignore.
• [P2 non-blocking] src/usage-tracker.ts:247 — Pending files are created with the process umask rather than the usage file’s permissions. A user who restricted usage.jsonl to 0600 can have the same event exposed through a typically 0644 pending file.
• [P3 nit] src/usage-tracker.ts:374 — Folding is not idempotent: termination or an rm failure after appendFile succeeds leaves the pending file in place, so the next lock holder appends the same event again and inflates usage statistics.

Resolved

• The usage lock and pending-file fallback resolve the previously reported concurrent append/cap races.
• Atomic replacement now preserves the original file mode and avoids in-place truncation corruption.
• All affected mocks export capUsageEvents.
• The PR description includes sufficient tests and real-CLI verification; its record naming older commit 2cce4a8 is non-blocking under the review rules.

[github-actions]

Findings

• [P1 blocking] src/init.ts:1725 — These patterns are added only when the project-scope .teamai/.gitignore is newly created. Existing legacy project installs retain an ignore file containing only usage.jsonl; a concurrent hook can therefore create a trackable usage.pending-<uuid>.jsonl, and git add . can commit machine-local usage data. Migrate existing project-scope ignore files as well.
• [P3 nit] src/usage-tracker.ts:377 — Pending-file folding remains non-idempotent. If the process exits or rm fails after appendFile succeeds, the next lock holder appends the same event again, inflating statistics.

Resolved

• The earlier append/cap races, unsafe in-place rewrite, permission regression, and incomplete mocks are fixed.
• The PR description includes sufficient testing and representative real-CLI verification; its older commit reference is non-blocking.

[github-actions]

Findings

• [P3 nit] src/usage-tracker.ts:440 — The legacy project .gitignore migration rewrites the file in place. An ENOSPC error or termination after truncation can leave it empty/partial, exposing entries such as token and env to a later git add .; use an atomic same-directory replacement.
• [P3 nit] src/usage-tracker.ts:378 — Pending-file folding remains non-idempotent. If termination or rm failure occurs after the append, the pending file is processed again by the next holder, duplicating the event and inflating statistics.

Resolved

• The previous append/cap races, unsafe usage-file rewrite, permission regression, legacy gitignore omission, and incomplete mocks are fixed.
• The PR description includes sufficient tests and representative real-CLI verification; the older commit references are non-blocking.

[github-actions]

Findings

• [P3 nit] src/init.ts:705 — Existing self-mode .teamai/.gitignore files missing the new patterns are still rewritten with the non-atomic writeFile at line 726. An ENOSPC error or termination after truncation can leave the file partial and expose entries such as token or env.local; use writeFileAtomic.
• [P3 nit] src/usage-tracker.ts:378 — Pending-file folding remains non-idempotent. If the process exits or rm fails after the append succeeds, the next lock holder appends the same event again, inflating usage statistics.

Resolved

• The earlier append/cap races, unsafe usage-file rewrite, permission regression, legacy project gitignore omission, and incomplete mocks are fixed.
• The PR description includes sufficient tests and representative real-CLI verification; no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:382 — Deduplicating by the complete JSON line drops legitimate events. If two concurrent hooks record the same skill/tool within the same millisecond while the lock is held, their pending files are identical; the first is appended, while the second is treated as already folded and deleted, silently undercounting usage. Use a unique event identifier rather than payload equality for idempotency.

Resolved

• The previously reported locking races, atomic-write and permission issues, gitignore migration gaps, and incomplete mocks are fixed.
• The PR description includes sufficient tests and representative real-CLI verification; its older tested commit is non-blocking under the review rules.

[github-actions]

Findings

• [P1 blocking] src/usage-tracker.ts:239 — The promised unique pending-event ID was added only to tests, not production code. Side files still contain JSON.stringify(event), and foldPendingEvents() still deduplicates by the complete line at src/usage-tracker.ts:382. Two identical concurrent events therefore cause the second side file to be deleted without recording its event. The new assertion expecting "pendingId" at src/__tests__/usage-tracking.test.ts:506 cannot pass against this commit.
• [P3 nit] src/usage-tracker.ts:386 — Folding is still not idempotent across reporting. If removal fails after a side file is appended, a successful report can truncate that line; the next lock holder then sees no match and appends the pending event again, duplicating its statistics.

Resolved

• The earlier append/cap races, unsafe rewrites, permission regression, gitignore migration gaps, and incomplete mocks are fixed.
• The PR description contains a test plan and representative real-CLI verification, so no missing-testing-description finding is needed.

[github-actions]

Findings

• [P3 nit] src/usage-tracker.ts:240 — Legacy project .gitignore migration occurs only after lock acquisition, and successful appends never invoke it. If the process exits while an ordinary append holds the first usage.jsonl.lock, that machine-local lock remains unignored and can be staged by git add .; heal the ignore file before creating the first usage lock.
• [P3 nit] src/usage-tracker.ts:393 — Folding is not fully idempotent if pending-file removal repeatedly fails. A successful report can subsequently truncate the ID-bearing line while the side file remains; once removal starts succeeding, a later holder appends that already-reported event again and inflates statistics.

Resolved

• The pending-event ID is now written by production code, so identical concurrent events are preserved.
• The previously reported append/cap races, unsafe rewrites, permission regression, and missing mocks are fixed.
• The PR description includes representative real-CLI verification. Its latest recorded test commit predates 0f920ef, but that is non-blocking under the review rules.