From b8e857deb89904c6b92754b74be1d357d2936a95 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 24 Sep 2026 13:36:34 +0200 Subject: [PATCH 1/8] fix(usage): cap usage.jsonl in scopes that never report (#788) A scope that never reports (an http source, `usageReport: false`, a remote rejecting every push) never truncated its usage file, so it grew without bound. After the report step, pull now keeps each active scope's newest 5,000 events; a file at or below the cap is not rewritten. The cap runs after the report's truncate, never between its read and truncate, where it would shift the deleted lines onto unsent events (#750). Every writer of the usage file takes one lock beside it (acquireLock from update.ts with a bounded retry): hook appends wait at most ~250 ms and then write anyway, the truncate and the cap wait up to ~5 s. A rewrite therefore cannot drop an append made while it runs, and two pulls (http scopes included, which hold no sync lock) cannot interleave their rewrites. Both rewrites go through a uniquely named temp file beside the realpath'd file, created with its mode and renamed over it, so a kill or a full disk leaves the old file whole; the next locked rewrite removes temps a killed one left behind. --- CHANGELOG.md | 1 + docs/designs/team-intelligence-platform.md | 8 +- docs/usage-guide.md | 11 +- docs/usage-guide.zh-CN.md | 8 +- src/__tests__/e2e/report-timeout.test.ts | 81 ++++++- src/__tests__/pull-learnings-deletion.test.ts | 1 + .../pull-placement-reconcile.test.ts | 1 + src/__tests__/pull-publishes-queue.test.ts | 1 + src/__tests__/pull-scope-isolation.test.ts | 27 ++- src/__tests__/pull-sync-truth.test.ts | 1 + src/__tests__/usage-tracking.test.ts | 209 ++++++++++++++++++ src/pull.ts | 12 +- src/usage-tracker.ts | 114 +++++++++- 13 files changed, 454 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71e9a5a77..cff670c7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes +- A scope's usage file no longer grows without bound where it is never reported: an http source, a team with `usageReport: false`, or a remote that rejects every push. `teamai pull` keeps its newest 5,000 events, so `teamai stats` still shows recent usage there, the local file being its only source. In a reporting scope whose report does not complete while it holds more than 5,000 events, the dropped events were never reported. The cap runs after the report has removed the events it sent, so it cannot shift that removal onto events not yet reported, and a file at or below the cap is not rewritten. Hook appends, the report's truncate and the cap now share one lock beside the usage file, and a rewrite goes through a temp file that keeps the file's mode, so a rewrite no longer loses an event recorded while it runs, nor the file on a kill or a full disk (for [#788](https://github.com/Tencent/teamai-cli/issues/788)). - `teamai stats` no longer counts a session twice, and no longer counts another project's sessions. Its dashboard section added the whole machine's local `events.jsonl` metrics to the scope's already-reported totals from the team repo, so every session that a `pull` had reported — and that stays in the event log until compaction — was counted once by the team total and once again locally, and sessions whose `cwd` belonged to a different project were added to this scope's as well. It now filters the event log the way `teamai pull` reports it (a project scope keeps only the sessions under its own root; the user scope resolves no root, so nothing is excluded and the log passes through unfiltered) and adds only what that scope has not reported yet, derived from the same per-session `reported-*` snapshots the report path advances — so the local figure agrees with the team's instead of exceeding it. When the reported totals could not be read at all (no stats file, an unreadable one, a reports worktree that is not there), or when they exist but hold nothing yet, nothing is subtracted — the local `reported-*` snapshots are machine-global while the team file is per-scope, so a non-empty team total is what licenses trusting them, and a session the member can see happening is never hidden. The per-repo and by-hour breakdowns read that same filtered log, so they stay inside the scope; they answer a different question from the headline — what this machine's retained event log holds, per repo — and both the headings and the `--by-repo` / `--by-time` flag descriptions now name that source instead of presenting them as a split of the headline (for [#768](https://github.com/Tencent/teamai-cli/issues/768)). - The data-partition migration no longer retires a legacy `/.teamai/` while the partition's `config.yaml` cannot be read. `teamai init`, `pull` and `push` took a partition config that merely existed as a finished migration and renamed the legacy directory to `.teamai.bak`, although it held the only config that still loaded. A partition `config.yaml` that is empty or cannot be opened, does not parse, does not validate, or is not `scope: project` now leaves the legacy directory in place and names the file to fix; once it is fixed, the next of those commands retires it as before. A partition directory whose `config.yaml` was moved aside is no longer replaced by a fresh copy of the legacy directory, which deleted what the partition held; the migration waits and says to restore the file or move the directory aside (for [#797](https://github.com/Tencent/teamai-cli/issues/797)). - The legacy `teamai dashboard-report` command no longer records dashboard events in a directory that never set up teamai. A current install writes only `teamai hook-dispatch`, whose dashboard-report handler declares `requiresConfig` and is dropped when no config resolves for the hook's `cwd`; the old subcommand stayed ungated, so a hook left behind by an earlier install kept recording events for every project it fired in, and those sessions were then reported by whichever scope pulled next. It now applies the same gate `teamai contribute-check` was given, asked about the session's `cwd` — or, for a host that sends none, the directory the hook runs in (for [#768](https://github.com/Tencent/teamai-cli/issues/768)). diff --git a/docs/designs/team-intelligence-platform.md b/docs/designs/team-intelligence-platform.md index 53771acf7..644d7f2c6 100644 --- a/docs/designs/team-intelligence-platform.md +++ b/docs/designs/team-intelligence-platform.md @@ -22,7 +22,7 @@ Transform TeamAI from a simple skill-sharing CLI into a **Team Intelligence Plat | 7 | Session 内容来源 | AI 对话和工具使用（非 git diff） | 核心目的是记录 AI 用错工具的情况 | | 8 | Session 过滤 | 有价值才推送（包含工具错误/重试→有价值） | 避免团队仓库充斥无用信息 | | 9 | Hook 追踪范围 | 只追踪 Skill 工具调用 | 追踪所有工具会产生大量噪音数据，核心需求是 skill 使用统计 | -| 10 | JSONL 生命周期 | 全部选中目标确认成功后截断已上报数据 | 失败保留事件；推送期间新增事件保留到下一批 | +| 10 | JSONL 生命周期 | 全部选中目标确认成功后截断已上报数据 | 失败保留事件（最多最新 5,000 条）；推送期间新增事件保留到下一批 | | 11 | 错误处理策略 | 所有 I/O 操作 try-catch + graceful degrade | 零静默失败原则 | | 12 | Feature 3 推送方式 | 删除 `push --stats`，只用 auto-report | Decision 5 已决定自动上报，手动命令冗余 | | 13 | Hook 命令实现 | `teamai track` TypeScript CLI 命令 | 类型安全 + 输入验证，比 bash one-liner 稳健 | @@ -108,6 +108,12 @@ reports worktree → git add → git commit → git push (teamai-reports) 推送和本地确认，并在完成后释放相关同步锁。失败重试即使重建出与上次提交 相同的文件，也会重新推送已有提交，无需创建空提交。 +所有目标处理完后，Pull 把每个活跃 scope 的 usage 文件截为最新 5,000 条（含 +HTTP 与 `usageReport: false` 这些从不上报的 scope，#788）。截断必须在清理之后： +清理按上报读到的条数删除文件开头的行，中间插入截断会把删除挪到未上报的事件上。 +Hook 追加、清理与截断共用 usage 文件旁的一把锁（`.lock`），改写经临时文件 +再 rename，保留原文件权限。 + 此修复不提供持久化批次或远端去重：进程在推送与确认之间终止、多仓库部分 成功后的重试仍可能产生重复统计。等待超时不等于取消 Git 或强制退出进程。 diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 82e12adb6..b42b9fa3a 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -1777,7 +1777,7 @@ reported, so a session that moved into another project mid-session reaches both teams with the part recorded in each; the first report after upgrading starts from the snapshot every scope used to share, so nothing is reported twice. A target removes its usage events only after it confirms success; failed pushes -preserve them. The affected sync locks remain +preserve them, up to the newest 5,000 (see below). The affected sync locks remain held until reporting finishes, preventing another pull from racing the report. This is best-effort reporting, not crash-safe delivery: termination between a @@ -1793,6 +1793,15 @@ can turn this off in `teamai.yaml`: usageReport: false ``` +Pull keeps each scope's usage file to its newest 5,000 events, dropping the +oldest after the report step. For an http source or a `usageReport: false` +team, that file is the only record `teamai stats` has, so it stays bounded +without going empty; a reporting scope whose report does not complete while +it holds more than 5,000 drops its oldest unreported events the same way. The +cap runs only after a report has removed the events it sent. Hook appends, the +report's truncate and the cap take one lock beside the usage file, so a rewrite +does not lose an event recorded while it runs. + ### Git submodules If your team distributes skills as git submodules, opt in with `submodules: true` diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 35156bdcd..93ef1877b 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -1708,7 +1708,7 @@ Copilot 会话以及从软链接路径启动的会话。旧版本记录的事件 位于其根目录下的那些，user scope 一条也不上报。每个 scope 还各自保存已上报快照， 因此会话中途切换到另一个项目时，两个团队各自收到在其中记录的那部分；升级后的首次上报 从原先各 scope 共用的快照开始，不会重复上报。目标确认成功后才清理自己的使用事件， -推送失败会保留事件。上报完成前继续持有相关同步锁， +推送失败会保留事件（最多保留最新 5,000 条，见下文）。上报完成前继续持有相关同步锁， 避免另一次 Pull 与尚未完成的上报竞争。 这仍是尽力上报，不提供崩溃恢复保证：远端推送成功与本地确认之间如果进程 @@ -1722,6 +1722,12 @@ Copilot 会话以及从软链接路径启动的会话。旧版本记录的事件 usageReport: false ``` +Pull 在上报步骤之后把每个 scope 的使用文件限制为最新 5,000 条事件，丢弃更早的 +事件。对 HTTP 源或 `usageReport: false` 的团队，该文件是 `teamai stats` 唯一的 +数据来源，因此文件保持有界而不会被清空；上报未完成且事件超过 5,000 条的上报 +scope 也以同样方式丢弃最早的未上报事件。该上限只在上报清理完已发送事件之后执行。Hook 追加、上报后的清理与该上限共用使用文件旁的一把锁， +因此改写文件时不会丢失期间记录的事件。 + ### Git 子模块 若团队以 git submodule 形式分发 skill，在 `teamai.yaml` 中开启 `submodules: true`： diff --git a/src/__tests__/e2e/report-timeout.test.ts b/src/__tests__/e2e/report-timeout.test.ts index 66ea3c450..95d7fc72b 100644 --- a/src/__tests__/e2e/report-timeout.test.ts +++ b/src/__tests__/e2e/report-timeout.test.ts @@ -15,7 +15,7 @@ function git(args: string[], cwd: string, env: NodeJS.ProcessEnv): string { return execFileSync('git', args, { cwd, env, encoding: 'utf8', windowsHide: true }); } -function fixture(agent: keyof typeof agents, provider: string) { +function fixture(agent: keyof typeof agents, provider: string, team: Record = {}) { sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-report-timeout-')); const home = path.join(sandbox, 'home'); const seed = path.join(sandbox, 'seed'); @@ -23,11 +23,11 @@ function fixture(agent: keyof typeof agents, provider: string) { const clone = path.join(home, '.teamai/team-repo'); fs.mkdirSync(seed, { recursive: true }); fs.mkdirSync(path.join(home, agents[agent]), { recursive: true }); - const env = { ...process.env, HOME: home, USERPROFILE: home, XDG_CONFIG_HOME: path.join(home, '.config'), + const env: NodeJS.ProcessEnv = { ...process.env, HOME: home, USERPROFILE: home, XDG_CONFIG_HOME: path.join(home, '.config'), GIT_CONFIG_GLOBAL: path.join(home, '.gitconfig'), GIT_CONFIG_NOSYSTEM: '1', GIT_AUTHOR_NAME: 'Report Test', GIT_AUTHOR_EMAIL: 'test@example.invalid', GIT_COMMITTER_NAME: 'Report Test', GIT_COMMITTER_EMAIL: 'test@example.invalid', FORCE_COLOR: '0' }; - fs.writeFileSync(path.join(seed, 'teamai.yaml'), YAML.stringify({ team: 'report-test', repo: remote, provider })); + fs.writeFileSync(path.join(seed, 'teamai.yaml'), YAML.stringify({ team: 'report-test', repo: remote, provider, ...team })); git(['init', '-q', '-b', 'main'], seed, env); git(['add', '.'], seed, env); git(['commit', '-q', '-m', 'fixture'], seed, env); @@ -91,7 +91,10 @@ function fixture(agent: keyof typeof agents, provider: string) { const p = path.join(dashboard, `user-reported-${name}.json`); return fs.existsSync(p) ? JSON.parse(fs.readFileSync(p, 'utf8')) : {}; } - return { home, usage, usageLine, seedEvents, receiver, pull, stats, snapshot }; + function run(args: string[]) { + return execFileSync(process.execPath, [cli, ...args], { cwd: sandbox, env, encoding: 'utf8', windowsHide: true }); + } + return { home, env, clone, usage, usageLine, dashboard, seedEvents, receiver, pull, stats, snapshot, run }; } afterEach(() => { @@ -153,3 +156,73 @@ describe('real CLI report completion', () => { expect(fs.readFileSync(f.usage, 'utf8')).toBe(''); }, 60_000); }); + +describe('real CLI usage cap (#788)', () => { + const cap = 5_000; + const line = (skill: string) => JSON.stringify({ skill, tool: 'claude', timestamp: new Date().toISOString() }); + /** `legacy` oldest, `review` newest: only the newest survive a cap. */ + function seedUsage(file: string, legacy: number, review: number) { + fs.writeFileSync(file, [...Array(legacy).fill(line('legacy')), ...Array(review).fill(line('review'))].join('\n') + '\n'); + } + const lines = (file: string) => fs.readFileSync(file, 'utf8').split('\n').filter(Boolean); + + it('caps a usageReport: false scope and stats still shows its newest usage', async () => { + const f = fixture('claude', 'git', { usageReport: false }); + seedUsage(f.usage, 100, cap); + await f.pull(); + expect(lines(f.usage)).toEqual(Array(cap).fill(expect.stringContaining('"review"'))); + const stats = f.run(['stats']); + expect(stats).toMatch(/review\s+5000 uses/); + expect(stats).not.toContain('legacy'); + }, 60_000); + + it('caps an http scope and stats still shows its newest usage', async () => { + const f = fixture('claude', 'git'); + fs.writeFileSync(path.join(f.home, '.teamai/config.yaml'), YAML.stringify({ + repo: { localPath: f.clone, remote: 'http://127.0.0.1:9/team', kind: 'http', url: 'http://127.0.0.1:9' }, + username: 'alice', scope: 'user', updatePolicy: 'skip', enabledAgents: ['claude'], additionalRoles: [], + })); + f.env.TEAMAI_API_TOKEN = 'unused'; + seedUsage(f.usage, 100, cap); + await f.pull(); + expect(lines(f.usage)).toEqual(Array(cap).fill(expect.stringContaining('"review"'))); + const stats = f.run(['stats']); + expect(stats).toMatch(/review\s+5000 uses/); + expect(stats).not.toContain('legacy'); + }, 60_000); + + it('does not rewrite a usage file below the cap', async () => { + const f = fixture('claude', 'git', { usageReport: false }); + seedUsage(f.usage, 0, 10); + const past = new Date('2026-01-01T00:00:00Z'); + fs.utimesSync(f.usage, past, past); + await f.pull(); + expect(fs.statSync(f.usage).mtimeMs).toBe(past.getTime()); + expect(lines(f.usage)).toHaveLength(10); + }, 60_000); + + it('caps a reporting scope whose push is rejected and reports its newest events on retry', async () => { + const f = fixture('claude', 'git'); + await f.pull(); // Warm reports worktree without any session data. + seedUsage(f.usage, 100, cap); + f.receiver('reject'); + await f.pull(); + expect(lines(f.usage)).toEqual(Array(cap).fill(expect.stringContaining('"review"'))); + f.receiver('normal'); + await f.pull(); + expect(f.stats().skills.review.count).toBe(cap); + expect(f.stats().skills.legacy).toBeUndefined(); + expect(lines(f.usage)).toEqual([]); + }, 60_000); + + it('a reporting scope over the cap keeps every event written after the report read it', async () => { + const f = fixture('claude', 'git'); + await f.pull(); // Warm reports worktree without any session data. + seedUsage(f.usage, 100, cap); + f.receiver('slow'); + await f.pull(() => fs.appendFileSync(f.usage, line('late') + '\n' + line('late') + '\n')); + expect(lines(f.usage)).toEqual([expect.stringContaining('"late"'), expect.stringContaining('"late"')]); + expect(f.stats().skills.legacy.count).toBe(100); + expect(f.stats().skills.review.count).toBe(cap); + }, 60_000); +}); diff --git a/src/__tests__/pull-learnings-deletion.test.ts b/src/__tests__/pull-learnings-deletion.test.ts index e3707d536..c2705cc97 100644 --- a/src/__tests__/pull-learnings-deletion.test.ts +++ b/src/__tests__/pull-learnings-deletion.test.ts @@ -48,6 +48,7 @@ vi.mock('../team-push.js', () => ({ reportUsageToTeam: vi.fn().mockResolvedValue vi.mock('../usage-tracker.js', () => ({ readUsageEvents: vi.fn().mockResolvedValue([]), truncateUsageAfterReport: vi.fn().mockResolvedValue(undefined), + capUsageEvents: vi.fn().mockResolvedValue(undefined), })); vi.mock('../roles.js', () => ({ loadRolesManifest: vi.fn().mockResolvedValue({ diff --git a/src/__tests__/pull-placement-reconcile.test.ts b/src/__tests__/pull-placement-reconcile.test.ts index 052c323b8..cb311a7f2 100644 --- a/src/__tests__/pull-placement-reconcile.test.ts +++ b/src/__tests__/pull-placement-reconcile.test.ts @@ -56,6 +56,7 @@ vi.mock('../team-push.js', () => ({ reportUsageToTeam: vi.fn().mockResolvedValue vi.mock('../usage-tracker.js', () => ({ readUsageEvents: vi.fn().mockResolvedValue([]), truncateUsageAfterReport: vi.fn().mockResolvedValue(undefined), + capUsageEvents: vi.fn().mockResolvedValue(undefined), })); vi.mock('../roles.js', () => ({ loadRolesManifest: vi.fn().mockResolvedValue({ version: 1, roles: [], defaults: { shareTarget: 'primary-role' } }), diff --git a/src/__tests__/pull-publishes-queue.test.ts b/src/__tests__/pull-publishes-queue.test.ts index 3dbcf61aa..fd9fa27e0 100644 --- a/src/__tests__/pull-publishes-queue.test.ts +++ b/src/__tests__/pull-publishes-queue.test.ts @@ -53,6 +53,7 @@ vi.mock('../team-push.js', () => ({ reportUsageToTeam: vi.fn().mockResolvedValue vi.mock('../usage-tracker.js', () => ({ readUsageEvents: vi.fn().mockResolvedValue([]), truncateUsageAfterReport: vi.fn().mockResolvedValue(undefined), + capUsageEvents: vi.fn().mockResolvedValue(undefined), })); vi.mock('../roles.js', () => ({ loadRolesManifest: vi.fn().mockResolvedValue({ diff --git a/src/__tests__/pull-scope-isolation.test.ts b/src/__tests__/pull-scope-isolation.test.ts index 0acbfbf67..3e1345de8 100644 --- a/src/__tests__/pull-scope-isolation.test.ts +++ b/src/__tests__/pull-scope-isolation.test.ts @@ -64,6 +64,7 @@ vi.mock('../team-push.js', () => ({ vi.mock('../usage-tracker.js', () => ({ readUsageEvents: vi.fn().mockResolvedValue([]), truncateUsageAfterReport: vi.fn().mockResolvedValue(undefined), + capUsageEvents: vi.fn().mockResolvedValue(undefined), })); vi.mock('../roles.js', () => ({ @@ -96,7 +97,7 @@ import { log } from '../utils/logger.js'; import { reconcileTeamHooksForConfig } from '../hooks.js'; import { reconcileMcpForConfig } from '../mcp-reconcile.js'; import { reportUsageToTeam } from '../team-push.js'; -import { readUsageEvents, truncateUsageAfterReport } from '../usage-tracker.js'; +import { capUsageEvents, readUsageEvents, truncateUsageAfterReport } from '../usage-tracker.js'; import { releaseLock } from '../update.js'; import { SYNC_LOCK_FILENAME, type TeamaiConfig, type LocalConfig } from '../types.js'; @@ -208,6 +209,27 @@ describe('pull scope isolation (issue #73)', () => { else expect(truncateUsageAfterReport).not.toHaveBeenCalled(); }); + it.each([true, false])('caps the usage file only after the report has truncated it (#788): %s', async (success) => { + vi.mocked(detectProjectConfig).mockResolvedValue(projectConfig); + vi.mocked(readUsageEvents).mockResolvedValue([{ skill: 'review', timestamp: new Date().toISOString(), tool: 'claude' }]); + vi.mocked(reportUsageToTeam).mockResolvedValueOnce(success); + await pull({ silent: true }); + expect(capUsageEvents).toHaveBeenCalledTimes(1); + expect(capUsageEvents).toHaveBeenCalledWith(projectConfig); + const capOrder = vi.mocked(capUsageEvents).mock.invocationCallOrder[0]; + expect(vi.mocked(reportUsageToTeam).mock.invocationCallOrder[0]).toBeLessThan(capOrder); + if (success) expect(vi.mocked(truncateUsageAfterReport).mock.invocationCallOrder[0]).toBeLessThan(capOrder); + }); + + it('caps the usage file of a scope with usageReport: false (#788)', async () => { + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.mocked(loadLocalConfigForScope).mockResolvedValue(userConfig); + vi.mocked(loadTeamConfig).mockResolvedValue({ ...teamConfig, usageReport: false }); + await pull({ silent: true }); + expect(reportUsageToTeam).not.toHaveBeenCalled(); + expect(capUsageEvents).toHaveBeenCalledWith(userConfig); + }); + it.each([true, false])('keeps late completion alive without starting a second report: %s', async (success) => { vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); vi.mocked(detectProjectConfig).mockResolvedValue(projectConfig); @@ -397,5 +419,8 @@ describe('pull scope isolation (issue #73)', () => { // HTTP-kind repo: kind !== 'http' guard filters out both report targets, // so targets is empty and the business repo's team-repo dir is never reset. expect(reportUsageToTeam).not.toHaveBeenCalled(); + // Its local file is the only source `teamai stats` has, so it is capped + // rather than consumed (#788). + expect(capUsageEvents).toHaveBeenCalledWith(httpProjectConfig); }); }); diff --git a/src/__tests__/pull-sync-truth.test.ts b/src/__tests__/pull-sync-truth.test.ts index bf1d93cab..ef87dfa36 100644 --- a/src/__tests__/pull-sync-truth.test.ts +++ b/src/__tests__/pull-sync-truth.test.ts @@ -56,6 +56,7 @@ vi.mock('../team-push.js', () => ({ reportUsageToTeam: vi.fn().mockResolvedValue vi.mock('../usage-tracker.js', () => ({ readUsageEvents: vi.fn().mockResolvedValue([]), truncateUsageAfterReport: vi.fn().mockResolvedValue(undefined), + capUsageEvents: vi.fn().mockResolvedValue(undefined), })); vi.mock('../update.js', () => ({ acquireLock: vi.fn().mockResolvedValue(true), diff --git a/src/__tests__/usage-tracking.test.ts b/src/__tests__/usage-tracking.test.ts index a00e5d544..39a8a4132 100644 --- a/src/__tests__/usage-tracking.test.ts +++ b/src/__tests__/usage-tracking.test.ts @@ -3,6 +3,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import fse from 'fs-extra'; +import { spawnSync } from 'node:child_process'; vi.mock('../utils/logger.js', () => ({ log: { @@ -20,6 +21,8 @@ import { appendUsageEvent, readUsageEvents, truncateUsageAfterReport, + capUsageEvents, + USAGE_EVENT_CAP, track, trackFromStdin, trackSlashCommand, @@ -266,6 +269,212 @@ describe('truncateUsageAfterReport', () => { }); }); +describe('capUsageEvents (#788)', () => { + const usagePath = () => path.join(tmpDir, '.teamai', 'user-usage.jsonl'); + const line = (i: number) => JSON.stringify({ skill: `s${i}`, timestamp: '2026-01-01T00:00:00Z', tool: 'claude' }); + const seed = async (count: number) => { + await fse.outputFile(usagePath(), Array.from({ length: count }, (_, i) => line(i)).join('\n') + '\n'); + }; + + it('keeps only the newest events of a file over the cap', async () => { + await seed(USAGE_EVENT_CAP + 7); + + await capUsageEvents(userScope()); + + const events = await readUsageEvents(userScope()); + expect(events).toHaveLength(USAGE_EVENT_CAP); + expect(events[0].skill).toBe('s7'); + expect(events[events.length - 1].skill).toBe(`s${USAGE_EVENT_CAP + 6}`); + }); + + it('does not rewrite a file at the cap', async () => { + await seed(USAGE_EVENT_CAP); + const past = new Date('2026-01-01T00:00:00Z'); + await fs.promises.utimes(usagePath(), past, past); + + await capUsageEvents(userScope()); + + expect((await fs.promises.stat(usagePath())).mtimeMs).toBe(past.getTime()); + expect(await readUsageEvents(userScope())).toHaveLength(USAGE_EVENT_CAP); + }); + + it('creates no file where the scope has recorded nothing', async () => { + await capUsageEvents(userScope()); + + expect(fs.existsSync(usagePath())).toBe(false); + }); + + const late: UsageEvent = { skill: 'late', timestamp: '2026-01-02T00:00:00Z', tool: 'claude' }; + const skills = async () => (await readUsageEvents(userScope())).map((e) => e.skill); + const lockPath = () => `${usagePath()}.lock`; + + it('holds an append issued mid-cap until the capped file is in place', async () => { + await seed(USAGE_EVENT_CAP + 2); + const before = await fs.promises.readFile(usagePath(), 'utf-8'); + const realWrite = fs.promises.writeFile; + let append: Promise | undefined; + let landedMidCap = false; + const spy = vi.spyOn(fs.promises, 'writeFile').mockImplementation(async (file, data, options) => { + append = appendUsageEvent(late, userScope()); + await new Promise((r) => setTimeout(r, 100)); + landedMidCap = (await fs.promises.readFile(usagePath(), 'utf-8')) !== before; + return realWrite(file, data, options); + }); + try { + await capUsageEvents(userScope()); + } finally { + spy.mockRestore(); + } + await append; + + expect(landedMidCap).toBe(false); + const kept = await skills(); + expect(kept).toHaveLength(USAGE_EVENT_CAP + 1); + expect(kept[0]).toBe('s2'); + expect(kept[kept.length - 1]).toBe('late'); + }); + + it('loses no append when two caps run at once', async () => { + await seed(USAGE_EVENT_CAP + 100); + const appends = Array.from({ length: 5 }, (_, i) => + new Promise((r) => setTimeout(r, i * 5)).then(() => + appendUsageEvent({ skill: `late${i}`, timestamp: '2026-01-02T00:00:00Z', tool: 'claude' }, userScope()), + ), + ); + + await Promise.all([capUsageEvents(userScope()), capUsageEvents(userScope()), ...appends]); + + const kept = await skills(); + expect(kept.filter((s) => s.startsWith('late')).sort()).toEqual(['late0', 'late1', 'late2', 'late3', 'late4']); + expect(kept).toContain(`s${USAGE_EVENT_CAP + 99}`); + expect(fs.existsSync(lockPath())).toBe(false); + }); + + it('keeps the mode of the file it replaces', async () => { + await seed(USAGE_EVENT_CAP + 2); + await fs.promises.chmod(usagePath(), 0o600); + + await capUsageEvents(userScope()); + + expect((await fs.promises.stat(usagePath())).mode & 0o777).toBe(0o600); + expect(await readUsageEvents(userScope())).toHaveLength(USAGE_EVENT_CAP); + }); + + it('removes temp files a killed rewrite left behind', async () => { + await seed(USAGE_EVENT_CAP + 2); + const orphan = `${usagePath()}.4242.0123456789ab.tmp`; + await fs.promises.writeFile(orphan, 'partial'); + + await capUsageEvents(userScope()); + + expect(fs.existsSync(orphan)).toBe(false); + }); + + it('leaves the file intact and no temp file behind when the write fails', async () => { + await seed(USAGE_EVENT_CAP + 2); + const before = await fs.promises.readFile(usagePath(), 'utf-8'); + const realWrite = fs.promises.writeFile; + const spy = vi.spyOn(fs.promises, 'writeFile').mockImplementation(async (file, data, options) => { + await realWrite(file, String(data).slice(0, 100), options); + throw Object.assign(new Error('ENOSPC: no space left on device'), { code: 'ENOSPC' }); + }); + try { + await capUsageEvents(userScope()); + } finally { + spy.mockRestore(); + } + + expect(await fs.promises.readFile(usagePath(), 'utf-8')).toBe(before); + expect((await fs.promises.readdir(path.dirname(usagePath()))).filter((n) => n.endsWith('.tmp'))).toEqual([]); + }); + + it('caps the target of a symlinked usage file and keeps the link', async () => { + const target = path.join(tmpDir, 'synced', 'usage.jsonl'); + await fse.outputFile(target, Array.from({ length: USAGE_EVENT_CAP + 2 }, (_, i) => line(i)).join('\n') + '\n'); + await fs.promises.symlink(target, usagePath()); + + await capUsageEvents(userScope()); + + expect((await fs.promises.lstat(usagePath())).isSymbolicLink()).toBe(true); + expect(await readUsageEvents(userScope())).toHaveLength(USAGE_EVENT_CAP); + }); + + it('caps a project scope in its own data home and leaves the user scope alone', async () => { + const workspace = path.join(tmpDir, 'workspace'); + const project: LocalConfig = { ...userScope(), scope: 'project', projectRoot: workspace, dataHome: path.join(workspace, '.teamai') }; + await fse.outputFile(path.join(workspace, '.teamai', 'usage.jsonl'), Array.from({ length: USAGE_EVENT_CAP + 3 }, (_, i) => line(i)).join('\n') + '\n'); + await seed(USAGE_EVENT_CAP + 3); + + await capUsageEvents(project); + + const events = await readUsageEvents(project); + expect(events).toHaveLength(USAGE_EVENT_CAP); + expect(events[0].skill).toBe('s3'); + expect(await readUsageEvents(userScope())).toHaveLength(USAGE_EVENT_CAP + 3); + }); +}); + +describe('usage file lock (#788)', () => { + const usagePath = () => path.join(tmpDir, '.teamai', 'user-usage.jsonl'); + const event = (skill: string): UsageEvent => ({ skill, timestamp: '2026-01-01T00:00:00Z', tool: 'claude' }); + const skills = async () => (await readUsageEvents(userScope())).map((e) => e.skill); + const writeLock = (pid: number) => + fse.outputFile(`${usagePath()}.lock`, JSON.stringify({ pid, startedAt: '2026-01-01T00:00:00Z', owner: 'other' })); + + it('reclaims a lock whose owner is gone', async () => { + await writeLock(spawnSync(process.execPath, ['-e', '']).pid ?? 0); + + await appendUsageEvent(event('a'), userScope()); + + expect(await skills()).toEqual(['a']); + expect(fs.existsSync(`${usagePath()}.lock`)).toBe(false); + }); + + it('appends within the hook budget while the lock stays held', async () => { + await writeLock(process.pid); + + const started = Date.now(); + await appendUsageEvent(event('a'), userScope()); + + expect(Date.now() - started).toBeLessThan(1000); + expect(await skills()).toEqual(['a']); + }); + + it('holds an append issued mid-truncate until the truncated file is in place', async () => { + for (const s of ['a', 'b', 'c']) await appendUsageEvent(event(s), userScope()); + const realWrite = fs.promises.writeFile; + let append: Promise | undefined; + const spy = vi.spyOn(fs.promises, 'writeFile').mockImplementation(async (file, data, options) => { + append = appendUsageEvent(event('late'), userScope()); + await new Promise((r) => setTimeout(r, 100)); + return realWrite(file, data, options); + }); + try { + await truncateUsageAfterReport(2, userScope()); + } finally { + spy.mockRestore(); + } + await append; + + expect(await skills()).toEqual(['c', 'late']); + }); + + it('leaves the file intact when the truncated copy cannot be written', async () => { + for (const s of ['a', 'b', 'c']) await appendUsageEvent(event(s), userScope()); + const before = await fs.promises.readFile(usagePath(), 'utf-8'); + const spy = vi.spyOn(fs.promises, 'writeFile').mockRejectedValue( + Object.assign(new Error('ENOSPC: no space left on device'), { code: 'ENOSPC' }), + ); + try { + await truncateUsageAfterReport(2, userScope()); + } finally { + spy.mockRestore(); + } + + expect(await fs.promises.readFile(usagePath(), 'utf-8')).toBe(before); + }); +}); + describe('usage recorded while every scope shared ~/.teamai/usage.jsonl (#748)', () => { const sharedPath = () => path.join(tmpDir, '.teamai', 'usage.jsonl'); const legacy = '{"skill":"from-another-project","timestamp":"2026-01-01T00:00:00Z","tool":"claude"}\n'; diff --git a/src/pull.ts b/src/pull.ts index f13b1e0e3..65788ce20 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -2008,7 +2008,7 @@ export async function pull( pendingUsageReport = (async () => { try { const { reportUsageToTeam } = await import('./team-push.js'); - const { truncateUsageAfterReport, readUsageEvents } = await import('./usage-tracker.js'); + const { truncateUsageAfterReport, readUsageEvents, capUsageEvents } = await import('./usage-tracker.js'); const targets: Array<{ repoPath: string; username: string; opts: { skipTruncate: true; selfConfig: LocalConfig } }> = []; // Per-target opt-out (teamai.yaml `usageReport: false`): a repo that // disables stat commits is dropped from the targets — e.g. teams @@ -2052,6 +2052,16 @@ export async function pull( log.error(`Auto-report to ${t.repoPath} skipped: ${(e as Error).message}`); } } + + // Cap every active scope, reporting or not (#788): http and + // `usageReport: false` scopes, or a remote rejecting every push, would + // otherwise grow forever. Only after the truncates above — a cap between + // a report's read and its truncate would shift the lines it deletes onto + // events never sent (#750). The usage file's own lock serializes the cap + // with hook appends and with another pull's cap, http scopes included. + for (const scope of [reconcileProject, reconcileUser]) { + if (scope) await capUsageEvents(scope); + } } catch (e) { log.debug(`Auto-report skipped: ${(e as Error).message}`); } diff --git a/src/usage-tracker.ts b/src/usage-tracker.ts index 901054044..34bfb461a 100644 --- a/src/usage-tracker.ts +++ b/src/usage-tracker.ts @@ -1,3 +1,4 @@ +import { randomBytes } from 'node:crypto'; import fs from 'node:fs'; import path from 'node:path'; import { log } from './utils/logger.js'; @@ -17,6 +18,7 @@ import { ensureDir, readJson, writeJson, pathExists } from './utils/fs.js'; import { getUserHome } from './utils/home.js'; import { resolveHookCwd } from './utils/hook-cwd.js'; import { resolveConfigForDir, resolveMemberToolRoots } from './config.js'; +import { acquireLock, releaseLock } from './update.js'; /** * The usage JSONL of one scope: `/usage.jsonl`, so each scope reports @@ -235,7 +237,7 @@ export async function appendUsageEvent(event: UsageEvent, config: LocalConfig): const usagePath = getUsagePath(config); await ensureDir(path.dirname(usagePath)); const line = JSON.stringify(event) + '\n'; - await fs.promises.appendFile(usagePath, line, 'utf-8'); + await withUsageLock(usagePath, APPEND_LOCK_WAIT, () => fs.promises.appendFile(usagePath, line, 'utf-8')); log.debug(`Tracked skill: ${event.skill}`); } catch (e) { log.error(`Failed to write usage event: ${(e as Error).message}`); @@ -274,23 +276,111 @@ export async function readUsageEvents(config: LocalConfig): Promise { try { - const usagePath = getUsagePath(config); - const content = await fs.promises.readFile(usagePath, 'utf-8'); - const lines = content.split('\n').filter((l) => l.trim()); - if (reportedCount >= lines.length) { - // All lines were reported — clear file - await fs.promises.writeFile(usagePath, '', 'utf-8'); - } else { - // Keep unreported lines - const remaining = lines.slice(reportedCount).join('\n') + '\n'; - await fs.promises.writeFile(usagePath, remaining, 'utf-8'); - } + // All lines reported → an empty file; otherwise keep the unreported lines. + await rewriteUsageFile(config, (lines) => lines.slice(reportedCount)); log.debug(`Truncated usage.jsonl: removed ${reportedCount} reported events`); } catch (e) { log.error(`Failed to truncate usage.jsonl: ${(e as Error).message}`); } } +/** Most events a scope's usage file keeps; `pull` drops the oldest beyond it (#788). */ +export const USAGE_EVENT_CAP = 5_000; + +/** + * Keep only the newest {@link USAGE_EVENT_CAP} events of a scope's usage file, + * so a scope that never reports (http, `usageReport: false`, a rejecting + * remote) stops growing without emptying `teamai stats`. The report truncates + * the first N lines it read, so this must run after that truncate, never + * between the report's read and its truncate. Counts non-empty lines, as the + * truncate does. A file at or below the cap is not rewritten. + */ +export async function capUsageEvents(config: LocalConfig): Promise { + let dropped = 0; + try { + await rewriteUsageFile(config, (lines) => { + if (lines.length <= USAGE_EVENT_CAP) return null; + dropped = lines.length - USAGE_EVENT_CAP; + return lines.slice(-USAGE_EVENT_CAP); + }); + if (dropped) log.debug(`Capped usage.jsonl: dropped ${dropped} oldest events`); + } catch (e) { + if (typeof e === 'object' && e !== null && 'code' in e && e.code === 'ENOENT') return; + log.error(`Could not cap ${getUsagePath(config)} to ${USAGE_EVENT_CAP} events: ${e instanceof Error ? e.message : String(e)}`); + } +} + +/** A hook append waits at most ~250 ms for the usage lock, inside its foreground budget. */ +const APPEND_LOCK_WAIT = { attempts: 10, delayMs: 25 }; +/** A rewrite waits up to ~5 s for a peer's rewrite to finish. */ +const REWRITE_LOCK_WAIT = { attempts: 100, delayMs: 50 }; + +/** + * Run `fn` holding the lock every writer of this usage file takes (#788): hook + * appends, the report's truncate and the cap. A rewrite then cannot drop an + * append made while it runs, and two pulls cannot interleave their rewrites. + * A lock whose owner is gone is reclaimed; one still held after the wait is + * written through anyway rather than dropping the event or the rewrite. + */ +async function withUsageLock( + usagePath: string, + wait: { attempts: number; delayMs: number }, + fn: (locked: boolean) => Promise, +): Promise { + const lockPath = `${usagePath}.lock`; + let locked = false; + for (let i = 0; i < wait.attempts && !locked; i++) { + locked = await acquireLock(lockPath); + if (!locked) await new Promise((r) => setTimeout(r, wait.delayMs)); + } + if (!locked) log.debug(`${lockPath} still held after ${wait.attempts * wait.delayMs} ms; writing without it`); + try { + return await fn(locked); + } finally { + if (locked) await releaseLock(lockPath); + } +} + +/** + * Replace a scope's usage file with the non-empty lines `keep` returns, or + * leave it untouched when `keep` returns null. The copy is written to a temp + * file beside the file, with its mode, and renamed over it, so a kill or a full + * disk leaves the old file whole. A symlinked file is replaced at its target. + */ +async function rewriteUsageFile(config: LocalConfig, keep: (lines: string[]) => string[] | null): Promise { + const usagePath = getUsagePath(config); + await withUsageLock(usagePath, REWRITE_LOCK_WAIT, async (locked) => { + // Replace the file itself, not a symlink to it. + const target = await fs.promises.realpath(usagePath); + if (locked) await removeOrphanTemps(target); + const lines = (await fs.promises.readFile(target, 'utf-8')).split('\n').filter((l) => l.trim()); + const kept = keep(lines); + if (!kept) return; + const mode = (await fs.promises.stat(target)).mode & 0o7777; + const tmpPath = `${target}.${process.pid}.${randomBytes(6).toString('hex')}.tmp`; + try { + await fs.promises.writeFile(tmpPath, kept.length ? kept.join('\n') + '\n' : '', { encoding: 'utf-8', mode }); + // The create mode passes through the umask. + await fs.promises.chmod(tmpPath, mode); + await fs.promises.rename(tmpPath, target); + } catch (e) { + await fs.promises.rm(tmpPath, { force: true }).catch(() => undefined); + throw e; + } + }); +} + +/** Remove the temp copies a killed rewrite left beside `target`; only the lock holder writes one. */ +async function removeOrphanTemps(target: string): Promise { + const dir = path.dirname(target); + const prefix = `${path.basename(target)}.`; + const names = await fs.promises.readdir(dir).catch(() => []); + for (const name of names) { + if (!name.startsWith(prefix) || !/^\d+\.[0-9a-f]{12}\.tmp$/.test(name.slice(prefix.length))) continue; + await fs.promises.rm(path.join(dir, name), { force: true }).catch(() => undefined); + } +} + /** * Add a skill to the known-skills set (persisted across truncations). * Silently fails on I/O errors to avoid disrupting the AI coding session. From 42fa89549205e95f7d0a2a3064c8bf7251b9bb2e Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 24 Sep 2026 13:47:56 +0200 Subject: [PATCH 2/8] fix(usage): address CI review (#788) The usage lock fell back to writing without it after its wait: a hook that gave up after ~250 ms appended to a file a slow rewrite was about to replace, and two rewrites past ~5 s could overwrite each other. No writer goes past the lock now. An append that cannot take it records its line in a side file of its own (.pending-.jsonl, O_EXCL), and every lock holder first appends those to the usage file and removes them; one without its newline is still being written and is left for the next holder. A rewrite that cannot take the lock leaves the file as it is and says which lock to remove. --- CHANGELOG.md | 2 +- docs/designs/team-intelligence-platform.md | 3 +- docs/usage-guide.md | 5 +- docs/usage-guide.zh-CN.md | 3 +- src/__tests__/usage-tracking.test.ts | 64 +++++++++++++++++- src/usage-tracker.ts | 79 ++++++++++++++++------ 6 files changed, 131 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cff670c7e..a619b7d9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes -- A scope's usage file no longer grows without bound where it is never reported: an http source, a team with `usageReport: false`, or a remote that rejects every push. `teamai pull` keeps its newest 5,000 events, so `teamai stats` still shows recent usage there, the local file being its only source. In a reporting scope whose report does not complete while it holds more than 5,000 events, the dropped events were never reported. The cap runs after the report has removed the events it sent, so it cannot shift that removal onto events not yet reported, and a file at or below the cap is not rewritten. Hook appends, the report's truncate and the cap now share one lock beside the usage file, and a rewrite goes through a temp file that keeps the file's mode, so a rewrite no longer loses an event recorded while it runs, nor the file on a kill or a full disk (for [#788](https://github.com/Tencent/teamai-cli/issues/788)). +- A scope's usage file no longer grows without bound where it is never reported: an http source, a team with `usageReport: false`, or a remote that rejects every push. `teamai pull` keeps its newest 5,000 events, so `teamai stats` still shows recent usage there, the local file being its only source. In a reporting scope whose report does not complete while it holds more than 5,000 events, the dropped events were never reported. The cap runs after the report has removed the events it sent, so it cannot shift that removal onto events not yet reported, and a file at or below the cap is not rewritten. Hook appends, the report's truncate and the cap now share one lock beside the usage file, and a rewrite goes through a temp file that keeps the file's mode, so a rewrite no longer loses an event recorded while it runs, nor the file on a kill or a full disk. A hook that cannot take the lock within ~250 ms records its event in a `*.pending-.jsonl` file beside it, which the next lock holder appends; a rewrite that cannot take it within ~5 s leaves the file as it is (for [#788](https://github.com/Tencent/teamai-cli/issues/788)). - `teamai stats` no longer counts a session twice, and no longer counts another project's sessions. Its dashboard section added the whole machine's local `events.jsonl` metrics to the scope's already-reported totals from the team repo, so every session that a `pull` had reported — and that stays in the event log until compaction — was counted once by the team total and once again locally, and sessions whose `cwd` belonged to a different project were added to this scope's as well. It now filters the event log the way `teamai pull` reports it (a project scope keeps only the sessions under its own root; the user scope resolves no root, so nothing is excluded and the log passes through unfiltered) and adds only what that scope has not reported yet, derived from the same per-session `reported-*` snapshots the report path advances — so the local figure agrees with the team's instead of exceeding it. When the reported totals could not be read at all (no stats file, an unreadable one, a reports worktree that is not there), or when they exist but hold nothing yet, nothing is subtracted — the local `reported-*` snapshots are machine-global while the team file is per-scope, so a non-empty team total is what licenses trusting them, and a session the member can see happening is never hidden. The per-repo and by-hour breakdowns read that same filtered log, so they stay inside the scope; they answer a different question from the headline — what this machine's retained event log holds, per repo — and both the headings and the `--by-repo` / `--by-time` flag descriptions now name that source instead of presenting them as a split of the headline (for [#768](https://github.com/Tencent/teamai-cli/issues/768)). - The data-partition migration no longer retires a legacy `/.teamai/` while the partition's `config.yaml` cannot be read. `teamai init`, `pull` and `push` took a partition config that merely existed as a finished migration and renamed the legacy directory to `.teamai.bak`, although it held the only config that still loaded. A partition `config.yaml` that is empty or cannot be opened, does not parse, does not validate, or is not `scope: project` now leaves the legacy directory in place and names the file to fix; once it is fixed, the next of those commands retires it as before. A partition directory whose `config.yaml` was moved aside is no longer replaced by a fresh copy of the legacy directory, which deleted what the partition held; the migration waits and says to restore the file or move the directory aside (for [#797](https://github.com/Tencent/teamai-cli/issues/797)). - The legacy `teamai dashboard-report` command no longer records dashboard events in a directory that never set up teamai. A current install writes only `teamai hook-dispatch`, whose dashboard-report handler declares `requiresConfig` and is dropped when no config resolves for the hook's `cwd`; the old subcommand stayed ungated, so a hook left behind by an earlier install kept recording events for every project it fired in, and those sessions were then reported by whichever scope pulled next. It now applies the same gate `teamai contribute-check` was given, asked about the session's `cwd` — or, for a host that sends none, the directory the hook runs in (for [#768](https://github.com/Tencent/teamai-cli/issues/768)). diff --git a/docs/designs/team-intelligence-platform.md b/docs/designs/team-intelligence-platform.md index 644d7f2c6..071c1f65f 100644 --- a/docs/designs/team-intelligence-platform.md +++ b/docs/designs/team-intelligence-platform.md @@ -112,7 +112,8 @@ reports worktree → git add → git commit → git push (teamai-reports) HTTP 与 `usageReport: false` 这些从不上报的 scope，#788）。截断必须在清理之后： 清理按上报读到的条数删除文件开头的行，中间插入截断会把删除挪到未上报的事件上。 Hook 追加、清理与截断共用 usage 文件旁的一把锁（`.lock`），改写经临时文件 -再 rename，保留原文件权限。 +再 rename，保留原文件权限。Hook 约 250 ms 拿不到锁时写入 `*.pending-.jsonl`（O_EXCL）， +由下一个持锁者并入；改写约 5 秒拿不到锁时不改文件。 此修复不提供持久化批次或远端去重：进程在推送与确认之间终止、多仓库部分 成功后的重试仍可能产生重复统计。等待超时不等于取消 Git 或强制退出进程。 diff --git a/docs/usage-guide.md b/docs/usage-guide.md index b42b9fa3a..af4f1f45b 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -1800,7 +1800,10 @@ without going empty; a reporting scope whose report does not complete while it holds more than 5,000 drops its oldest unreported events the same way. The cap runs only after a report has removed the events it sent. Hook appends, the report's truncate and the cap take one lock beside the usage file, so a rewrite -does not lose an event recorded while it runs. +does not lose an event recorded while it runs. A hook that cannot take the lock +within ~250 ms records its event in a `*.pending-.jsonl` file next to it, +which the next lock holder appends to the usage file; a rewrite that cannot take +it within ~5 s leaves the file as it is. ### Git submodules diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 93ef1877b..aa9240953 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -1726,7 +1726,8 @@ Pull 在上报步骤之后把每个 scope 的使用文件限制为最新 5,000 事件。对 HTTP 源或 `usageReport: false` 的团队，该文件是 `teamai stats` 唯一的 数据来源，因此文件保持有界而不会被清空；上报未完成且事件超过 5,000 条的上报 scope 也以同样方式丢弃最早的未上报事件。该上限只在上报清理完已发送事件之后执行。Hook 追加、上报后的清理与该上限共用使用文件旁的一把锁， -因此改写文件时不会丢失期间记录的事件。 +因此改写文件时不会丢失期间记录的事件。Hook 在约 250 ms 内拿不到锁时，把事件写入旁边的 +`*.pending-.jsonl` 文件，由下一个持锁者追加进使用文件；改写在约 5 秒内拿不到锁时保持文件不变。 ### Git 子模块 diff --git a/src/__tests__/usage-tracking.test.ts b/src/__tests__/usage-tracking.test.ts index 39a8a4132..fb418a045 100644 --- a/src/__tests__/usage-tracking.test.ts +++ b/src/__tests__/usage-tracking.test.ts @@ -430,16 +430,76 @@ describe('usage file lock (#788)', () => { expect(fs.existsSync(`${usagePath()}.lock`)).toBe(false); }); - it('appends within the hook budget while the lock stays held', async () => { + const pendingFiles = async () => + (await fs.promises.readdir(path.dirname(usagePath()))).filter((n) => n.startsWith('user-usage.pending-')); + + it('records an event beside the file within the hook budget while the lock stays held, and folds it in later', async () => { + await appendUsageEvent(event('a'), userScope()); await writeLock(process.pid); const started = Date.now(); - await appendUsageEvent(event('a'), userScope()); + await appendUsageEvent(event('b'), userScope()); expect(Date.now() - started).toBeLessThan(1000); expect(await skills()).toEqual(['a']); + expect(await pendingFiles()).toHaveLength(1); + + await fs.promises.rm(`${usagePath()}.lock`); + await appendUsageEvent(event('c'), userScope()); + + expect(await skills()).toEqual(['a', 'b', 'c']); + expect(await pendingFiles()).toEqual([]); + }); + + it('keeps an append that gives up on the lock while a slow cap rewrites the file', async () => { + await fse.outputFile( + usagePath(), + Array.from({ length: USAGE_EVENT_CAP + 2 }, (_, i) => JSON.stringify(event(`s${i}`))).join('\n') + '\n', + ); + const realWrite = fs.promises.writeFile; + let append: Promise | undefined; + const spy = vi.spyOn(fs.promises, 'writeFile').mockImplementation(async (file, data, options) => { + if (String(file).includes('.pending-')) return realWrite(file, data, options); + append = appendUsageEvent(event('late'), userScope()); + await append; + return realWrite(file, data, options); + }); + try { + await capUsageEvents(userScope()); + } finally { + spy.mockRestore(); + } + await appendUsageEvent(event('next'), userScope()); + + const kept = await skills(); + expect(kept).toHaveLength(USAGE_EVENT_CAP + 2); + expect(kept.slice(-2).sort()).toEqual(['late', 'next']); + }); + + it('leaves a side file alone while it is still being written', async () => { + const partial = path.join(tmpDir, '.teamai', 'user-usage.pending-00000000-0000-4000-8000-000000000000.jsonl'); + await fse.outputFile(partial, ''); + + await appendUsageEvent(event('a'), userScope()); + + expect(await skills()).toEqual(['a']); + expect(fs.existsSync(partial)).toBe(true); }); + it('does not rewrite the file while another holder keeps the lock', async () => { + await fse.outputFile( + usagePath(), + Array.from({ length: USAGE_EVENT_CAP + 2 }, (_, i) => JSON.stringify(event(`s${i}`))).join('\n') + '\n', + ); + const before = await fs.promises.readFile(usagePath(), 'utf-8'); + await writeLock(process.pid); + + await capUsageEvents(userScope()); + await truncateUsageAfterReport(2, userScope()); + + expect(await fs.promises.readFile(usagePath(), 'utf-8')).toBe(before); + }, 20_000); + it('holds an append issued mid-truncate until the truncated file is in place', async () => { for (const s of ['a', 'b', 'c']) await appendUsageEvent(event(s), userScope()); const realWrite = fs.promises.writeFile; diff --git a/src/usage-tracker.ts b/src/usage-tracker.ts index 34bfb461a..ff47c5e77 100644 --- a/src/usage-tracker.ts +++ b/src/usage-tracker.ts @@ -1,4 +1,4 @@ -import { randomBytes } from 'node:crypto'; +import { randomBytes, randomUUID } from 'node:crypto'; import fs from 'node:fs'; import path from 'node:path'; import { log } from './utils/logger.js'; @@ -237,8 +237,15 @@ export async function appendUsageEvent(event: UsageEvent, config: LocalConfig): const usagePath = getUsagePath(config); await ensureDir(path.dirname(usagePath)); const line = JSON.stringify(event) + '\n'; - await withUsageLock(usagePath, APPEND_LOCK_WAIT, () => fs.promises.appendFile(usagePath, line, 'utf-8')); - log.debug(`Tracked skill: ${event.skill}`); + if (await withUsageLock(usagePath, APPEND_LOCK_WAIT, () => fs.promises.appendFile(usagePath, line, 'utf-8'))) { + log.debug(`Tracked skill: ${event.skill}`); + return; + } + // The lock is still held: record the event in a side file of its own for + // the next lock holder to fold in, rather than race a rewrite. + const pendingPath = path.join(path.dirname(usagePath), `${pendingPrefix(usagePath)}${randomUUID()}.jsonl`); + await fs.promises.writeFile(pendingPath, line, { encoding: 'utf-8', flag: 'wx' }); + log.debug(`Tracked skill: ${event.skill} (in ${pendingPath}; ${usagePath}.lock is held)`); } catch (e) { log.error(`Failed to write usage event: ${(e as Error).message}`); } @@ -319,25 +326,56 @@ const REWRITE_LOCK_WAIT = { attempts: 100, delayMs: 50 }; * Run `fn` holding the lock every writer of this usage file takes (#788): hook * appends, the report's truncate and the cap. A rewrite then cannot drop an * append made while it runs, and two pulls cannot interleave their rewrites. - * A lock whose owner is gone is reclaimed; one still held after the wait is - * written through anyway rather than dropping the event or the rewrite. + * The holder first folds in the side files of appends that gave up waiting. A + * lock whose owner is gone is reclaimed. Returns false, without running `fn`, + * when the lock is still held after the wait. */ -async function withUsageLock( +async function withUsageLock( usagePath: string, wait: { attempts: number; delayMs: number }, - fn: (locked: boolean) => Promise, -): Promise { + fn: () => Promise, +): Promise { const lockPath = `${usagePath}.lock`; - let locked = false; - for (let i = 0; i < wait.attempts && !locked; i++) { - locked = await acquireLock(lockPath); - if (!locked) await new Promise((r) => setTimeout(r, wait.delayMs)); + for (let i = 0; i < wait.attempts; i++) { + if (await acquireLock(lockPath)) { + try { + await foldPendingEvents(usagePath); + await fn(); + } finally { + await releaseLock(lockPath); + } + return true; + } + await new Promise((r) => setTimeout(r, wait.delayMs)); } - if (!locked) log.debug(`${lockPath} still held after ${wait.attempts * wait.delayMs} ms; writing without it`); - try { - return await fn(locked); - } finally { - if (locked) await releaseLock(lockPath); + return false; +} + +/** Name prefix of the side files an append writes while the usage lock is held. */ +function pendingPrefix(usagePath: string): string { + return `${path.basename(usagePath, '.jsonl')}.pending-`; +} + +/** + * Append the side files of appends that gave up on the lock to the usage file, + * then remove them. Each holds one whole line; one without its newline is + * still being written and waits for the next holder. + */ +async function foldPendingEvents(usagePath: string): Promise { + const dir = path.dirname(usagePath); + const prefix = pendingPrefix(usagePath); + const names = await fs.promises.readdir(dir).catch(() => []); + for (const name of names) { + if (!name.startsWith(prefix) || !name.endsWith('.jsonl')) continue; + const pendingPath = path.join(dir, name); + try { + const content = await fs.promises.readFile(pendingPath, 'utf-8'); + if (!content.endsWith('\n')) continue; + await fs.promises.appendFile(usagePath, content, 'utf-8'); + await fs.promises.rm(pendingPath, { force: true }); + } catch (e) { + log.debug(`Could not fold ${pendingPath} into ${usagePath}: ${e instanceof Error ? e.message : String(e)}`); + } } } @@ -349,10 +387,10 @@ async function withUsageLock( */ async function rewriteUsageFile(config: LocalConfig, keep: (lines: string[]) => string[] | null): Promise { const usagePath = getUsagePath(config); - await withUsageLock(usagePath, REWRITE_LOCK_WAIT, async (locked) => { + const rewritten = await withUsageLock(usagePath, REWRITE_LOCK_WAIT, async () => { // Replace the file itself, not a symlink to it. const target = await fs.promises.realpath(usagePath); - if (locked) await removeOrphanTemps(target); + await removeOrphanTemps(target); const lines = (await fs.promises.readFile(target, 'utf-8')).split('\n').filter((l) => l.trim()); const kept = keep(lines); if (!kept) return; @@ -368,6 +406,9 @@ async function rewriteUsageFile(config: LocalConfig, keep: (lines: string[]) => throw e; } }); + if (!rewritten) { + throw new Error(`${usagePath}.lock is still held after 5 s, so the file was left as it is; remove the lock if no teamai process is running`); + } } /** Remove the temp copies a killed rewrite left beside `target`; only the lock holder writes one. */ From 583f908ae71f81b155131acaa4288182aad72ed9 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 24 Sep 2026 14:08:59 +0200 Subject: [PATCH 3/8] fix(usage): ignore the usage lock, temps and pending files; keep pending files private (#788) --- CHANGELOG.md | 2 +- docs/designs/team-intelligence-platform.md | 4 ++- docs/usage-guide.md | 5 ++- docs/usage-guide.zh-CN.md | 2 ++ src/__tests__/single-repo-mode.test.ts | 40 ++++++++++++++++++++++ src/__tests__/usage-tracking.test.ts | 21 ++++++++++++ src/init.ts | 9 +++++ src/usage-tracker.ts | 5 ++- 8 files changed, 84 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39cea277a..1dbe70acf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes -- A scope's usage file no longer grows without bound where it is never reported: an http source, a team with `usageReport: false`, or a remote that rejects every push. `teamai pull` keeps its newest 5,000 events, so `teamai stats` still shows recent usage there, the local file being its only source. In a reporting scope whose report does not complete while it holds more than 5,000 events, the dropped events were never reported. The cap runs after the report has removed the events it sent, so it cannot shift that removal onto events not yet reported, and a file at or below the cap is not rewritten. Hook appends, the report's truncate and the cap now share one lock beside the usage file, and a rewrite goes through a temp file that keeps the file's mode, so a rewrite no longer loses an event recorded while it runs, nor the file on a kill or a full disk. A hook that cannot take the lock within ~250 ms records its event in a `*.pending-.jsonl` file beside it, which the next lock holder appends; a rewrite that cannot take it within ~5 s leaves the file as it is (for [#788](https://github.com/Tencent/teamai-cli/issues/788)). +- A scope's usage file no longer grows without bound where it is never reported: an http source, a team with `usageReport: false`, or a remote that rejects every push. `teamai pull` keeps its newest 5,000 events, so `teamai stats` still shows recent usage there, the local file being its only source. In a reporting scope whose report does not complete while it holds more than 5,000 events, the dropped events were never reported. The cap runs after the report has removed the events it sent, so it cannot shift that removal onto events not yet reported, and a file at or below the cap is not rewritten. Hook appends, the report's truncate and the cap now share one lock beside the usage file, and a rewrite goes through a temp file that keeps the file's mode, so a rewrite no longer loses an event recorded while it runs, nor the file on a kill or a full disk. A hook that cannot take the lock within ~250 ms records its event in a `*.pending-.jsonl` file beside it, which the next lock holder appends; a rewrite that cannot take it within ~5 s leaves the file as it is. A pending file gets no wider mode than the usage file (owner-only while there is none), and an in-workspace `.teamai/.gitignore` ignores the lock, a rewrite's temp copy and the pending files; `pull` and `push` add those entries to an existing single-repo one (for [#788](https://github.com/Tencent/teamai-cli/issues/788)). - `teamai stats` no longer counts a session twice, and no longer counts another project's sessions. Its dashboard section added the whole machine's local `events.jsonl` metrics to the scope's already-reported totals from the team repo, so every session that a `pull` had reported — and that stays in the event log until compaction — was counted once by the team total and once again locally, and sessions whose `cwd` belonged to a different project were added to this scope's as well. It now filters the event log the way `teamai pull` reports it (only the sessions recorded in the current scope, see #785) and adds only what that scope has not reported yet, derived from the same per-scope `reported-*` snapshots the report path advances — so the local figure agrees with the team's instead of exceeding it. When the reported totals could not be read at all (no stats file, an unreadable one, a reports worktree that is not there), or when they exist but hold nothing yet, nothing is subtracted — a snapshot can name a session the team file never received, so a non-empty team total is what licenses trusting them, and a session the member can see happening is never hidden. The per-repo and by-hour breakdowns read that same filtered log, so they stay inside the scope; they answer a different question from the headline — what this machine's retained event log holds, per repo — and both the headings and the `--by-repo` / `--by-time` flag descriptions now name that source instead of presenting them as a split of the headline (for [#768](https://github.com/Tencent/teamai-cli/issues/768)). - The data-partition migration no longer retires a legacy `/.teamai/` while the partition's `config.yaml` cannot be read. `teamai init`, `pull` and `push` took a partition config that merely existed as a finished migration and renamed the legacy directory to `.teamai.bak`, although it held the only config that still loaded. A partition `config.yaml` that is empty or cannot be opened, does not parse, does not validate, or is not `scope: project` now leaves the legacy directory in place and names the file to fix; once it is fixed, the next of those commands retires it as before. A partition directory whose `config.yaml` was moved aside is no longer replaced by a fresh copy of the legacy directory, which deleted what the partition held; the migration waits and says to restore the file or move the directory aside (for [#797](https://github.com/Tencent/teamai-cli/issues/797)). - The legacy `teamai dashboard-report` command no longer records dashboard events in a directory that never set up teamai. A current install writes only `teamai hook-dispatch`, whose dashboard-report handler declares `requiresConfig` and is dropped when no config resolves for the hook's `cwd`; the old subcommand stayed ungated, so a hook left behind by an earlier install kept recording events for every project it fired in, and those sessions were then reported by whichever scope pulled next. It now applies the same gate `teamai contribute-check` was given, asked about the session's `cwd` — or, for a host that sends none, the directory the hook runs in (for [#768](https://github.com/Tencent/teamai-cli/issues/768)). diff --git a/docs/designs/team-intelligence-platform.md b/docs/designs/team-intelligence-platform.md index 071c1f65f..8bda05d4a 100644 --- a/docs/designs/team-intelligence-platform.md +++ b/docs/designs/team-intelligence-platform.md @@ -113,7 +113,9 @@ HTTP 与 `usageReport: false` 这些从不上报的 scope，#788）。截断必 清理按上报读到的条数删除文件开头的行，中间插入截断会把删除挪到未上报的事件上。 Hook 追加、清理与截断共用 usage 文件旁的一把锁（`.lock`），改写经临时文件 再 rename，保留原文件权限。Hook 约 250 ms 拿不到锁时写入 `*.pending-.jsonl`（O_EXCL）， -由下一个持锁者并入；改写约 5 秒拿不到锁时不改文件。 +由下一个持锁者并入；改写约 5 秒拿不到锁时不改文件。pending 文件权限不宽于 usage 文件 +（尚无 usage 文件时为 0600）。工作区内 `.teamai/.gitignore` 忽略 `usage.jsonl.*` 与 +`usage.pending-*.jsonl`，单仓库模式的已有文件由 `migrateSelfModeGitignoreContent` 补齐。 此修复不提供持久化批次或远端去重：进程在推送与确认之间终止、多仓库部分 成功后的重试仍可能产生重复统计。等待超时不等于取消 Git 或强制退出进程。 diff --git a/docs/usage-guide.md b/docs/usage-guide.md index af4f1f45b..764aaa427 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -1803,7 +1803,10 @@ report's truncate and the cap take one lock beside the usage file, so a rewrite does not lose an event recorded while it runs. A hook that cannot take the lock within ~250 ms records its event in a `*.pending-.jsonl` file next to it, which the next lock holder appends to the usage file; a rewrite that cannot take -it within ~5 s leaves the file as it is. +it within ~5 s leaves the file as it is. A pending file gets no wider mode than +the usage file (owner-only while there is none). An in-workspace +`.teamai/.gitignore` ignores the lock, a rewrite's temp copy and the pending +files; `pull` and `push` add those entries to an existing single-repo one. ### Git submodules diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index aa9240953..fedf4dc90 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -1728,6 +1728,8 @@ Pull 在上报步骤之后把每个 scope 的使用文件限制为最新 5,000 scope 也以同样方式丢弃最早的未上报事件。该上限只在上报清理完已发送事件之后执行。Hook 追加、上报后的清理与该上限共用使用文件旁的一把锁， 因此改写文件时不会丢失期间记录的事件。Hook 在约 250 ms 内拿不到锁时，把事件写入旁边的 `*.pending-.jsonl` 文件，由下一个持锁者追加进使用文件；改写在约 5 秒内拿不到锁时保持文件不变。 +pending 文件的权限不宽于使用文件（尚无使用文件时仅所有者可读写）。工作区内的 `.teamai/.gitignore` +忽略该锁、改写的临时副本与 pending 文件；`pull` 与 `push` 会为已有的单仓库 `.gitignore` 补上这些条目。 ### Git 子模块 diff --git a/src/__tests__/single-repo-mode.test.ts b/src/__tests__/single-repo-mode.test.ts index b9e14fd37..407a9f346 100644 --- a/src/__tests__/single-repo-mode.test.ts +++ b/src/__tests__/single-repo-mode.test.ts @@ -1,4 +1,7 @@ import { describe, it, expect } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import { getReportsDir, @@ -12,6 +15,29 @@ import { } from '../types.js'; import { buildSelfModeGitignore, migrateSelfModeGitignoreContent } from '../init.js'; +/** The names of `names` that `gitignore`, as `.teamai/.gitignore`, makes git ignore. */ +function ignoredBy(gitignore: string, names: string[]): string[] { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-gitignore-')); + try { + execFileSync('git', ['init', '-q'], { cwd: dir }); + fs.mkdirSync(path.join(dir, '.teamai')); + fs.writeFileSync(path.join(dir, '.teamai', '.gitignore'), gitignore); + return names.filter((name) => { + try { + execFileSync('git', ['check-ignore', '-q', '--no-index', `.teamai/${name}`], { cwd: dir }); + return true; + } catch { + return false; + } + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +/** The files a usage write leaves beside `usage.jsonl` while it runs or when it gives up on the lock (#788). */ +const USAGE_SIDE_FILES = ['usage.jsonl.lock', 'usage.jsonl.123.0123456789ab.tmp', 'usage.pending-0f8e2c1a-1b2c-4d5e-8f90-123456789abc.jsonl']; + function makeConfig(kind: 'git' | 'http' | 'self', localPath = '/repo/.teamai'): LocalConfig { return { repo: { @@ -141,6 +167,10 @@ describe('buildSelfModeGitignore', () => { expect(gi.split('\n').map((l) => l.trim())).toContain('env.sh'); }); + it('ignores the usage lock, its rewrite temp and pending files, so a usage write leaves git status clean', () => { + expect(ignoredBy(gi, USAGE_SIDE_FILES)).toEqual(USAGE_SIDE_FILES); + }); + it('ignores the learnings worktree, its lock and the queue, so contributing leaves git status clean', () => { const lines = gi.split('\n').map((l) => l.trim()); expect(lines).toContain('learnings-wt/'); @@ -166,6 +196,7 @@ describe('migrateSelfModeGitignoreContent (self-heal old gitignore)', () => { const old = [ 'env.sh', 'env.local', 'teamai.lock', 'learnings-wt/', '.learnings-lock', 'pending-learnings/', 'env/', + 'usage.jsonl.*', 'usage.pending-*.jsonl', ].join('\n'); const { changed, content } = migrateSelfModeGitignoreContent(old); expect(changed).toBe(false); // nothing to remove, env.local already present @@ -196,6 +227,7 @@ describe('migrateSelfModeGitignoreContent (self-heal old gitignore)', () => { const old = [ '# env is machine-local', 'config.yaml', 'env.local', 'teamai.lock', 'learnings-wt/', '.learnings-lock', 'pending-learnings/', + 'usage.jsonl.*', 'usage.pending-*.jsonl', ].join('\n'); const { changed, content } = migrateSelfModeGitignoreContent(old); // No bare `env` line, env.local already present → unchanged. @@ -210,6 +242,14 @@ describe('migrateSelfModeGitignoreContent (self-heal old gitignore)', () => { expect(content).toContain('token\nteamai.lock'); }); + it('adds the usage lock, rewrite temp and pending patterns to a file that ignores only usage.jsonl', () => { + const old = ['config.yaml', 'token', 'teamai.lock', 'env.local', 'usage.jsonl', 'known-skills.json'].join('\n'); + const { changed, content } = migrateSelfModeGitignoreContent(old); + expect(changed).toBe(true); + expect(ignoredBy(content, USAGE_SIDE_FILES)).toEqual(USAGE_SIDE_FILES); + expect(content).toContain('usage.jsonl\nusage.jsonl.*\nusage.pending-*.jsonl\nknown-skills.json'); + }); + it('adds the learnings worktree, its lock and the queue', () => { const old = ['config.yaml', 'token', 'teamai.lock', 'env.local', 'reports-wt/', 'knowledge-wt/'].join('\n'); const { changed, content } = migrateSelfModeGitignoreContent(old); diff --git a/src/__tests__/usage-tracking.test.ts b/src/__tests__/usage-tracking.test.ts index fb418a045..0e7f958e9 100644 --- a/src/__tests__/usage-tracking.test.ts +++ b/src/__tests__/usage-tracking.test.ts @@ -451,6 +451,27 @@ describe('usage file lock (#788)', () => { expect(await pendingFiles()).toEqual([]); }); + const pendingMode = async () => + (await fs.promises.stat(path.join(path.dirname(usagePath()), (await pendingFiles())[0]))).mode & 0o777; + + it('gives a side file the mode of the usage file', async () => { + await appendUsageEvent(event('a'), userScope()); + await fs.promises.chmod(usagePath(), 0o600); + await writeLock(process.pid); + + await appendUsageEvent(event('b'), userScope()); + + expect(await pendingMode()).toBe(0o600); + }); + + it('keeps a side file private to its owner while there is no usage file yet', async () => { + await writeLock(process.pid); + + await appendUsageEvent(event('a'), userScope()); + + expect(await pendingMode()).toBe(0o600); + }); + it('keeps an append that gives up on the lock while a slow cap rewrites the file', async () => { await fse.outputFile( usagePath(), diff --git a/src/init.ts b/src/init.ts index 32ec58a62..8def53cd8 100644 --- a/src/init.ts +++ b/src/init.ts @@ -630,6 +630,10 @@ export function buildSelfModeGitignore(): string { // resolution (self mode uses this name to avoid colliding with the env/ dir). 'env.local', 'usage.jsonl', + // The usage lock, a rewrite's temp copy and the events a hook records while + // the lock is held (#788). + 'usage.jsonl.*', + 'usage.pending-*.jsonl', 'known-skills.json', 'search-index.json', 'managed-mcp.json', @@ -697,6 +701,9 @@ export function migrateSelfModeGitignoreContent(content: string): { changed: boo ensure('learnings-wt/', 'reports-wt/'); ensure('.learnings-lock', '.reports-lock'); ensure('pending-learnings/', 'knowledge-wt/'); + // The usage lock, rewrite temps and pending events arrived with the usage cap (#788). + ensure('usage.jsonl.*', 'usage.jsonl'); + ensure('usage.pending-*.jsonl', 'usage.jsonl.*'); return { changed, content: filtered.join('\n') }; } @@ -1715,6 +1722,8 @@ export async function init(options: GlobalOptions & { 'sessions/', 'dashboard/', 'usage.jsonl', + 'usage.jsonl.*', + 'usage.pending-*.jsonl', 'known-skills.json', 'learnings/', 'search-index.json', diff --git a/src/usage-tracker.ts b/src/usage-tracker.ts index ff47c5e77..746c04bf7 100644 --- a/src/usage-tracker.ts +++ b/src/usage-tracker.ts @@ -243,8 +243,11 @@ export async function appendUsageEvent(event: UsageEvent, config: LocalConfig): } // The lock is still held: record the event in a side file of its own for // the next lock holder to fold in, rather than race a rewrite. + // It holds what the usage file holds, so it gets no wider mode than that file + // (the umask can only narrow it); owner-only while there is no file yet. const pendingPath = path.join(path.dirname(usagePath), `${pendingPrefix(usagePath)}${randomUUID()}.jsonl`); - await fs.promises.writeFile(pendingPath, line, { encoding: 'utf-8', flag: 'wx' }); + const mode = await fs.promises.stat(usagePath).then((s) => s.mode & 0o777, () => 0o600); + await fs.promises.writeFile(pendingPath, line, { encoding: 'utf-8', flag: 'wx', mode }); log.debug(`Tracked skill: ${event.skill} (in ${pendingPath}; ${usagePath}.lock is held)`); } catch (e) { log.error(`Failed to write usage event: ${(e as Error).message}`); From fc5f5369807e0dcf6f0e73fa59ef8a7941bb64c5 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 24 Sep 2026 14:30:30 +0200 Subject: [PATCH 4/8] fix(usage): add the usage side-file ignores to an existing project .gitignore (#788) teamai init writes a project scope's .teamai/.gitignore only when it is missing, so a legacy in-workspace install keeps one that ignores usage.jsonl alone, and a hook that times out on the usage lock leaves a trackable usage.pending-.jsonl in the business repo. The usage tracker now adds usage.jsonl.* and usage.pending-*.jsonl to that file before it writes a pending file or a rewrite's temp copy. Best-effort: a failure never costs the event, and single-repo mode keeps its own pull/push heal. --- CHANGELOG.md | 2 +- docs/designs/team-intelligence-platform.md | 3 +- docs/usage-guide.md | 4 ++- docs/usage-guide.zh-CN.md | 3 +- src/__tests__/usage-tracking.test.ts | 39 ++++++++++++++++++++++ src/usage-tracker.ts | 30 +++++++++++++++++ 6 files changed, 77 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4694b3296..d585ec1c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes -- A scope's usage file no longer grows without bound where it is never reported: an http source, a team with `usageReport: false`, or a remote that rejects every push. `teamai pull` keeps its newest 5,000 events, so `teamai stats` still shows recent usage there, the local file being its only source. In a reporting scope whose report does not complete while it holds more than 5,000 events, the dropped events were never reported. The cap runs after the report has removed the events it sent, so it cannot shift that removal onto events not yet reported, and a file at or below the cap is not rewritten. Hook appends, the report's truncate and the cap now share one lock beside the usage file, and a rewrite goes through a temp file that keeps the file's mode, so a rewrite no longer loses an event recorded while it runs, nor the file on a kill or a full disk. A hook that cannot take the lock within ~250 ms records its event in a `*.pending-.jsonl` file beside it, which the next lock holder appends; a rewrite that cannot take it within ~5 s leaves the file as it is. A pending file gets no wider mode than the usage file (owner-only while there is none), and an in-workspace `.teamai/.gitignore` ignores the lock, a rewrite's temp copy and the pending files; `pull` and `push` add those entries to an existing single-repo one (for [#788](https://github.com/Tencent/teamai-cli/issues/788)). +- A scope's usage file no longer grows without bound where it is never reported: an http source, a team with `usageReport: false`, or a remote that rejects every push. `teamai pull` keeps its newest 5,000 events, so `teamai stats` still shows recent usage there, the local file being its only source. In a reporting scope whose report does not complete while it holds more than 5,000 events, the dropped events were never reported. The cap runs after the report has removed the events it sent, so it cannot shift that removal onto events not yet reported, and a file at or below the cap is not rewritten. Hook appends, the report's truncate and the cap now share one lock beside the usage file, and a rewrite goes through a temp file that keeps the file's mode, so a rewrite no longer loses an event recorded while it runs, nor the file on a kill or a full disk. A hook that cannot take the lock within ~250 ms records its event in a `*.pending-.jsonl` file beside it, which the next lock holder appends; a rewrite that cannot take it within ~5 s leaves the file as it is. A pending file gets no wider mode than the usage file (owner-only while there is none), and an in-workspace `.teamai/.gitignore` ignores the lock, a rewrite's temp copy and the pending files; `pull` and `push` add those entries to an existing single-repo one, and the first pending file or rewrite adds them to an existing project-scope one (for [#788](https://github.com/Tencent/teamai-cli/issues/788)). - `teamai stats` no longer counts a session twice, and no longer counts another project's sessions. Its dashboard section added the whole machine's local `events.jsonl` metrics to the scope's already-reported totals from the team repo, so every session that a `pull` had reported — and that stays in the event log until compaction — was counted once by the team total and once again locally, and sessions whose `cwd` belonged to a different project were added to this scope's as well. It now filters the event log the way `teamai pull` reports it (only the sessions recorded in the current scope, see #785) and adds only what that scope has not reported yet, derived from the same per-scope `reported-*` snapshots the report path advances — so the local figure agrees with the team's instead of exceeding it. When the reported totals could not be read at all (no stats file, an unreadable one, a reports worktree that is not there), or when they exist but hold nothing yet, nothing is subtracted — a snapshot can name a session the team file never received, so a non-empty team total is what licenses trusting them, and a session the member can see happening is never hidden. The per-repo and by-hour breakdowns read that same filtered log, so they stay inside the scope; they answer a different question from the headline — what this machine's retained event log holds, per repo — and both the headings and the `--by-repo` / `--by-time` flag descriptions now name that source instead of presenting them as a split of the headline (for [#768](https://github.com/Tencent/teamai-cli/issues/768)). - The data-partition migration no longer retires a legacy `/.teamai/` while the partition's `config.yaml` cannot be read. `teamai init`, `pull` and `push` took a partition config that merely existed as a finished migration and renamed the legacy directory to `.teamai.bak`, although it held the only config that still loaded. A partition `config.yaml` that is empty or cannot be opened, does not parse, does not validate, or is not `scope: project` now leaves the legacy directory in place and names the file to fix; once it is fixed, the next of those commands retires it as before. A partition directory whose `config.yaml` was moved aside is no longer replaced by a fresh copy of the legacy directory, which deleted what the partition held; the migration waits and says to restore the file or move the directory aside (for [#797](https://github.com/Tencent/teamai-cli/issues/797)). - `teamai pull` syncs nothing in a project whose config exists but cannot be read, and says why. Detection skipped the broken file and pulled whatever loaded next: the user scope, or a legacy `.teamai/` behind a broken partition that may belong to another team, whose skills, rules and docs were deployed and to which the project's usage was reported. It now prints ``Nothing was synced: : . Fix the file, or move it aside and run `teamai init` to write a new one.`` and exits 1. A session start there runs no pull, seeds no agent directory and stashes no package hint; `teamai pull --silent`, which hooks from before `hook-dispatch` still run, prints nothing, writes the reason to `~/.teamai/debug.log` and exits 1. This is the rule team hooks and usage follow since [#748](https://github.com/Tencent/teamai-cli/issues/748) (for [#784](https://github.com/Tencent/teamai-cli/issues/784)). diff --git a/docs/designs/team-intelligence-platform.md b/docs/designs/team-intelligence-platform.md index 8bda05d4a..738f0b26a 100644 --- a/docs/designs/team-intelligence-platform.md +++ b/docs/designs/team-intelligence-platform.md @@ -115,7 +115,8 @@ Hook 追加、清理与截断共用 usage 文件旁的一把锁（`.jsonl`（O_EXCL）， 由下一个持锁者并入；改写约 5 秒拿不到锁时不改文件。pending 文件权限不宽于 usage 文件 （尚无 usage 文件时为 0600）。工作区内 `.teamai/.gitignore` 忽略 `usage.jsonl.*` 与 -`usage.pending-*.jsonl`，单仓库模式的已有文件由 `migrateSelfModeGitignoreContent` 补齐。 +`usage.pending-*.jsonl`，单仓库模式的已有文件由 `migrateSelfModeGitignoreContent` 补齐， +项目级的已有文件在写 pending 文件或改写前由 `ignoreUsageSideFiles` 补齐。 此修复不提供持久化批次或远端去重：进程在推送与确认之间终止、多仓库部分 成功后的重试仍可能产生重复统计。等待超时不等于取消 Git 或强制退出进程。 diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 828a6bbf6..00de1805d 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -1806,7 +1806,9 @@ which the next lock holder appends to the usage file; a rewrite that cannot take it within ~5 s leaves the file as it is. A pending file gets no wider mode than the usage file (owner-only while there is none). An in-workspace `.teamai/.gitignore` ignores the lock, a rewrite's temp copy and the pending -files; `pull` and `push` add those entries to an existing single-repo one. +files; `pull` and `push` add those entries to an existing single-repo one, and +the usage file's first pending file or rewrite adds them to an existing +project-scope one. ### Git submodules diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 6a6c46aaa..44175bc6a 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -1729,7 +1729,8 @@ scope 也以同样方式丢弃最早的未上报事件。该上限只在上报 因此改写文件时不会丢失期间记录的事件。Hook 在约 250 ms 内拿不到锁时，把事件写入旁边的 `*.pending-.jsonl` 文件，由下一个持锁者追加进使用文件；改写在约 5 秒内拿不到锁时保持文件不变。 pending 文件的权限不宽于使用文件（尚无使用文件时仅所有者可读写）。工作区内的 `.teamai/.gitignore` -忽略该锁、改写的临时副本与 pending 文件；`pull` 与 `push` 会为已有的单仓库 `.gitignore` 补上这些条目。 +忽略该锁、改写的临时副本与 pending 文件；`pull` 与 `push` 会为已有的单仓库 `.gitignore` 补上这些条目， +已有的项目级 `.gitignore` 则在使用文件第一次写 pending 文件或改写时补上。 ### Git 子模块 diff --git a/src/__tests__/usage-tracking.test.ts b/src/__tests__/usage-tracking.test.ts index 0e7f958e9..2678634f3 100644 --- a/src/__tests__/usage-tracking.test.ts +++ b/src/__tests__/usage-tracking.test.ts @@ -556,6 +556,45 @@ describe('usage file lock (#788)', () => { }); }); +describe('usage files in a legacy in-workspace .teamai/ (#788)', () => { + // What `teamai init --scope project` wrote before the usage lock existed. + const legacyGitignore = [ + '# teamai local config (do not commit)', 'config.yaml', 'state.json', 'token', 'teamai.lock', + '.update-lock', 'env', 'env.sh', 'sessions/', 'dashboard/', 'usage.jsonl', 'known-skills.json', + 'learnings/', 'search-index.json', 'votes/', '', + ].join('\n'); + const workspace = () => path.join(tmpDir, 'workspace'); + const project = (): LocalConfig => ({ ...userScope(), scope: 'project', projectRoot: workspace(), dataHome: path.join(workspace(), '.teamai') }); + const usagePath = () => path.join(workspace(), '.teamai', 'usage.jsonl'); + const git = (...args: string[]) => spawnSync('git', args, { cwd: workspace(), encoding: 'utf-8' }); + const event = (skill: string): UsageEvent => ({ skill, timestamp: '2026-01-01T00:00:00Z', tool: 'claude' }); + + beforeEach(async () => { + await fse.outputFile(path.join(workspace(), '.teamai', '.gitignore'), legacyGitignore); + git('init', '-q'); + }); + + it('keeps an event recorded while the lock is held out of git status', async () => { + await appendUsageEvent(event('a'), project()); + await fse.outputFile(`${usagePath()}.lock`, JSON.stringify({ pid: process.pid, startedAt: '2026-01-01T00:00:00Z', owner: 'other' })); + + await appendUsageEvent(event('b'), project()); + + expect((await fs.promises.readdir(path.dirname(usagePath()))).some((n) => n.startsWith('usage.pending-'))).toBe(true); + expect(git('status', '--porcelain', '--untracked-files=all').stdout.split('\n').filter(Boolean)) + .toEqual(['?? .teamai/.gitignore']); + }); + + it('ignores the lock and a rewrite\'s temp copy once the file has been capped', async () => { + await fse.outputFile(usagePath(), Array.from({ length: USAGE_EVENT_CAP + 1 }, (_, i) => JSON.stringify(event(`s${i}`))).join('\n') + '\n'); + + await capUsageEvents(project()); + + const ignored = git('check-ignore', '--no-index', '.teamai/usage.jsonl.lock', '.teamai/usage.jsonl.123.0123456789ab.tmp'); + expect(ignored.stdout.split('\n').filter(Boolean)).toHaveLength(2); + }); +}); + describe('usage recorded while every scope shared ~/.teamai/usage.jsonl (#748)', () => { const sharedPath = () => path.join(tmpDir, '.teamai', 'usage.jsonl'); const legacy = '{"skill":"from-another-project","timestamp":"2026-01-01T00:00:00Z","tool":"claude"}\n'; diff --git a/src/usage-tracker.ts b/src/usage-tracker.ts index 746c04bf7..68ba2406d 100644 --- a/src/usage-tracker.ts +++ b/src/usage-tracker.ts @@ -245,6 +245,7 @@ export async function appendUsageEvent(event: UsageEvent, config: LocalConfig): // the next lock holder to fold in, rather than race a rewrite. // It holds what the usage file holds, so it gets no wider mode than that file // (the umask can only narrow it); owner-only while there is no file yet. + await ignoreUsageSideFiles(config, usagePath); const pendingPath = path.join(path.dirname(usagePath), `${pendingPrefix(usagePath)}${randomUUID()}.jsonl`); const mode = await fs.promises.stat(usagePath).then((s) => s.mode & 0o777, () => 0o600); await fs.promises.writeFile(pendingPath, line, { encoding: 'utf-8', flag: 'wx', mode }); @@ -394,6 +395,7 @@ async function rewriteUsageFile(config: LocalConfig, keep: (lines: string[]) => // Replace the file itself, not a symlink to it. const target = await fs.promises.realpath(usagePath); await removeOrphanTemps(target); + await ignoreUsageSideFiles(config, usagePath); const lines = (await fs.promises.readFile(target, 'utf-8')).split('\n').filter((l) => l.trim()); const kept = keep(lines); if (!kept) return; @@ -414,6 +416,34 @@ async function rewriteUsageFile(config: LocalConfig, keep: (lines: string[]) => } } +/** The lock, a rewrite's temp copy and the events a hook records while the lock is held. */ +const USAGE_SIDE_FILE_PATTERNS = ['usage.jsonl.*', 'usage.pending-*.jsonl']; + +/** + * Add {@link USAGE_SIDE_FILE_PATTERNS} to the `.gitignore` beside a project + * scope's usage file when it predates them. `teamai init` writes that file only + * once, so a legacy in-workspace `.teamai/` still ignores `usage.jsonl` alone + * and the side files would show in the business repo's git status. Single-repo + * mode heals its own file (migrateSelfModeGitignore). Best-effort: a missing + * `.gitignore` stays missing, and a failure never costs the event. + */ +async function ignoreUsageSideFiles(config: LocalConfig, usagePath: string): Promise { + if (config.scope !== 'project' || config.repo.kind === 'self') return; + const gitignorePath = path.join(path.dirname(usagePath), '.gitignore'); + try { + const lines = (await fs.promises.readFile(gitignorePath, 'utf-8')).split('\n'); + const missing = USAGE_SIDE_FILE_PATTERNS.filter((p) => !lines.some((l) => l.trim() === p)); + if (!missing.length) return; + const anchor = lines.findIndex((l) => l.trim() === 'usage.jsonl'); + const at = anchor >= 0 ? anchor + 1 : lines.length - (lines[lines.length - 1] === '' ? 1 : 0); + lines.splice(at, 0, ...missing); + await fs.promises.writeFile(gitignorePath, lines.join('\n'), 'utf-8'); + } catch (e) { + if (typeof e === 'object' && e !== null && 'code' in e && e.code === 'ENOENT') return; + log.debug(`Could not add the usage side files to ${gitignorePath}: ${e instanceof Error ? e.message : String(e)}`); + } +} + /** Remove the temp copies a killed rewrite left beside `target`; only the lock holder writes one. */ async function removeOrphanTemps(target: string): Promise { const dir = path.dirname(target); From 064370e8d994376e9dc341807094cdcb46496f53 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 24 Sep 2026 15:30:27 +0200 Subject: [PATCH 5/8] fix(usage): heal the project .gitignore atomically (#788) The side-file heal rewrote the legacy project .teamai/.gitignore in place. A full disk or a kill after the truncate left it empty or partial, and that file also ignores `token` and `env`, so a later `git add .` could stage them. It now goes through writeFileAtomic (same-directory temp + rename, mode kept), so a failed write leaves the original byte-identical. --- src/__tests__/usage-tracking.test.ts | 29 ++++++++++++++++++++++++++++ src/usage-tracker.ts | 5 +++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/__tests__/usage-tracking.test.ts b/src/__tests__/usage-tracking.test.ts index 2678634f3..8d84f9177 100644 --- a/src/__tests__/usage-tracking.test.ts +++ b/src/__tests__/usage-tracking.test.ts @@ -593,6 +593,35 @@ describe('usage files in a legacy in-workspace .teamai/ (#788)', () => { const ignored = git('check-ignore', '--no-index', '.teamai/usage.jsonl.lock', '.teamai/usage.jsonl.123.0123456789ab.tmp'); expect(ignored.stdout.split('\n').filter(Boolean)).toHaveLength(2); }); + + it('leaves the .gitignore whole when the disk fills while it is healed', async () => { + // The disk fills after the first bytes of the healed .gitignore, wherever it is written. + const fillDisk = (file: unknown, data: unknown) => { + if (!path.basename(String(file)).startsWith('.gitignore')) return false; + fs.writeFileSync(String(file), String(data).slice(0, 40)); + return true; + }; + const enospc = () => Object.assign(new Error('ENOSPC: no space left on device, write'), { code: 'ENOSPC' }); + const writeFile = fs.promises.writeFile; + vi.spyOn(fs.promises, 'writeFile').mockImplementation(async (file, data, options) => { + if (fillDisk(file, data)) throw enospc(); + return writeFile(file, data, options); + }); + const outputWrite = fse.writeFile; + vi.spyOn(fse, 'writeFile').mockImplementation(async (file: fs.PathOrFileDescriptor, data: string | NodeJS.ArrayBufferView, options?: fs.WriteFileOptions | fs.NoParamCallback) => { + if (fillDisk(file, data)) throw enospc(); + if (typeof options === 'function') return outputWrite(file, data, options); + return outputWrite(file, data, options); + }); + await fse.outputFile(usagePath(), Array.from({ length: USAGE_EVENT_CAP + 1 }, (_, i) => JSON.stringify(event(`s${i}`))).join('\n') + '\n'); + + await capUsageEvents(project()); + vi.restoreAllMocks(); + + expect(fs.readFileSync(path.join(workspace(), '.teamai', '.gitignore'), 'utf-8')).toBe(legacyGitignore); + expect((await fs.promises.readdir(path.join(workspace(), '.teamai'))).filter((n) => n.startsWith('.gitignore'))).toEqual(['.gitignore']); + expect(await readUsageEvents(project())).toHaveLength(USAGE_EVENT_CAP); + }); }); describe('usage recorded while every scope shared ~/.teamai/usage.jsonl (#748)', () => { diff --git a/src/usage-tracker.ts b/src/usage-tracker.ts index 68ba2406d..f8cd2dede 100644 --- a/src/usage-tracker.ts +++ b/src/usage-tracker.ts @@ -14,7 +14,7 @@ import { CLAUDE_TOOL_ID, DEFAULT_CLAUDE_ROOT, } from './types.js'; -import { ensureDir, readJson, writeJson, pathExists } from './utils/fs.js'; +import { ensureDir, readJson, writeJson, writeFileAtomic, pathExists } from './utils/fs.js'; import { getUserHome } from './utils/home.js'; import { resolveHookCwd } from './utils/hook-cwd.js'; import { resolveConfigForDir, resolveMemberToolRoots } from './config.js'; @@ -437,7 +437,8 @@ async function ignoreUsageSideFiles(config: LocalConfig, usagePath: string): Pro const anchor = lines.findIndex((l) => l.trim() === 'usage.jsonl'); const at = anchor >= 0 ? anchor + 1 : lines.length - (lines[lines.length - 1] === '' ? 1 : 0); lines.splice(at, 0, ...missing); - await fs.promises.writeFile(gitignorePath, lines.join('\n'), 'utf-8'); + // A full disk or a kill mid-write must not leave it empty: it also ignores `token` and `env`. + await writeFileAtomic(gitignorePath, lines.join('\n')); } catch (e) { if (typeof e === 'object' && e !== null && 'code' in e && e.code === 'ENOENT') return; log.debug(`Could not add the usage side files to ${gitignorePath}: ${e instanceof Error ? e.message : String(e)}`); From 97a394b1e557836d2d38b06790ac7983b426e576 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 24 Sep 2026 16:01:08 +0200 Subject: [PATCH 6/8] fix(usage): address CI review (#788) migrateSelfModeGitignore rewrote an existing single-repo .teamai/.gitignore in place; a full disk or a kill after the truncate left it partial, and it also ignores `token` and `env.local`. It now goes through writeFileAtomic, like the project-scope heal. Folding a pending file was not idempotent: a holder that died, or whose rm failed, after the append left the side file for the next holder to append again. The fold now skips a line the usage file already holds (a line carries its millisecond timestamp) and still removes the side file. --- src/__tests__/single-repo-mode.test.ts | 29 ++++++++++++++++++++++++-- src/__tests__/usage-tracking.test.ts | 23 ++++++++++++++++++++ src/init.ts | 5 +++-- src/usage-tracker.ts | 11 ++++++++-- 4 files changed, 62 insertions(+), 6 deletions(-) diff --git a/src/__tests__/single-repo-mode.test.ts b/src/__tests__/single-repo-mode.test.ts index 407a9f346..186f9f5b2 100644 --- a/src/__tests__/single-repo-mode.test.ts +++ b/src/__tests__/single-repo-mode.test.ts @@ -1,8 +1,9 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import fse from 'fs-extra'; import { getReportsDir, getKnowledgeDir, @@ -13,7 +14,7 @@ import { TeamaiConfigSchema, type LocalConfig, } from '../types.js'; -import { buildSelfModeGitignore, migrateSelfModeGitignoreContent } from '../init.js'; +import { buildSelfModeGitignore, migrateSelfModeGitignore, migrateSelfModeGitignoreContent } from '../init.js'; /** The names of `names` that `gitignore`, as `.teamai/.gitignore`, makes git ignore. */ function ignoredBy(gitignore: string, names: string[]): string[] { @@ -179,6 +180,30 @@ describe('buildSelfModeGitignore', () => { }); }); +describe('migrateSelfModeGitignore', () => { + it('leaves the .gitignore whole when the disk fills while it is healed', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-self-heal-')); + const gitignorePath = path.join(root, '.teamai', '.gitignore'); + const old = ['config.yaml', 'token', 'teamai.lock', 'env.local', 'usage.jsonl'].join('\n'); + fs.mkdirSync(path.dirname(gitignorePath)); + fs.writeFileSync(gitignorePath, old); + // The disk fills after the first bytes, wherever the healed file is written. + const spy = vi.spyOn(fse, 'writeFile').mockImplementation(async (file: fs.PathOrFileDescriptor, data: string | NodeJS.ArrayBufferView) => { + fs.writeFileSync(String(file), String(data).slice(0, 10)); + throw Object.assign(new Error('ENOSPC: no space left on device, write'), { code: 'ENOSPC' }); + }); + try { + await migrateSelfModeGitignore({ ...makeConfig('self', path.join(root, '.teamai')), projectRoot: root }); + } finally { + spy.mockRestore(); + } + + expect(fs.readFileSync(gitignorePath, 'utf-8')).toBe(old); + expect(fs.readdirSync(path.dirname(gitignorePath))).toEqual(['.gitignore']); + fs.rmSync(root, { recursive: true, force: true }); + }); +}); + describe('migrateSelfModeGitignoreContent (self-heal old gitignore)', () => { it('removes a standalone `env` line and adds machine-local entries', () => { const old = ['config.yaml', 'env', 'env.sh', 'members/'].join('\n'); diff --git a/src/__tests__/usage-tracking.test.ts b/src/__tests__/usage-tracking.test.ts index 8d84f9177..d8e5eb13f 100644 --- a/src/__tests__/usage-tracking.test.ts +++ b/src/__tests__/usage-tracking.test.ts @@ -451,6 +451,29 @@ describe('usage file lock (#788)', () => { expect(await pendingFiles()).toEqual([]); }); + it('folds a side file once when it outlives its append', async () => { + await appendUsageEvent(event('a'), userScope()); + await writeLock(process.pid); + await appendUsageEvent(event('b'), userScope()); + await fs.promises.rm(`${usagePath()}.lock`); + // The side file cannot be removed once its event is in the file (or the holder dies there). + const realRm = fs.promises.rm; + const spy = vi.spyOn(fs.promises, 'rm').mockImplementation(async (file, options) => { + if (String(file).includes('.pending-')) throw Object.assign(new Error('EPERM: operation not permitted'), { code: 'EPERM' }); + return realRm(file, options); + }); + try { + await appendUsageEvent(event('c'), userScope()); + } finally { + spy.mockRestore(); + } + + await appendUsageEvent(event('d'), userScope()); + + expect(await skills()).toEqual(['a', 'b', 'c', 'd']); + expect(await pendingFiles()).toEqual([]); + }); + const pendingMode = async () => (await fs.promises.stat(path.join(path.dirname(usagePath()), (await pendingFiles())[0]))).mode & 0o777; diff --git a/src/init.ts b/src/init.ts index 8def53cd8..6f6cf910e 100644 --- a/src/init.ts +++ b/src/init.ts @@ -7,7 +7,7 @@ import { configureGitUser, initRepo, isGitRepo, getRemoteUrl, remotesMatch, reda import { pushRepoDirectly } from './utils/git.js'; import { getProvider, detectProviderForInit, RepoNotFoundError, OrganizationNotFoundError, RepoCreatePermissionError } from './providers/index.js'; import { parseGenericGitExistingRemote } from './providers/git/repo-url.js'; -import { ensureDir, writeFile, pathExists, expandHome, readFileSafe, remove } from './utils/fs.js'; +import { ensureDir, writeFile, writeFileAtomic, pathExists, expandHome, readFileSafe, remove } from './utils/fs.js'; import { log, spinner } from './utils/logger.js'; import { CLAUDE_TOOL_ID, @@ -723,7 +723,8 @@ export async function migrateSelfModeGitignore(localConfig: LocalConfig): Promis if (current === null) return; // no gitignore to migrate const { changed, content } = migrateSelfModeGitignoreContent(current); if (!changed) return; - await writeFile(gitignorePath, content); + // A full disk or a kill mid-write must not leave it partial: it also ignores `token` and `env.local`. + await writeFileAtomic(gitignorePath, content); log.info( 'Updated .teamai/.gitignore for current machine-local files — ' + 'please `git add .teamai/.gitignore` and commit it.', diff --git a/src/usage-tracker.ts b/src/usage-tracker.ts index f8cd2dede..3c8f5ace1 100644 --- a/src/usage-tracker.ts +++ b/src/usage-tracker.ts @@ -363,19 +363,26 @@ function pendingPrefix(usagePath: string): string { /** * Append the side files of appends that gave up on the lock to the usage file, * then remove them. Each holds one whole line; one without its newline is - * still being written and waits for the next holder. + * still being written and waits for the next holder. A line the file already + * holds was folded by a holder that died or could not remove the side file, + * so it is not appended again (a line carries its millisecond timestamp). */ async function foldPendingEvents(usagePath: string): Promise { const dir = path.dirname(usagePath); const prefix = pendingPrefix(usagePath); const names = await fs.promises.readdir(dir).catch(() => []); + let folded: Set | undefined; for (const name of names) { if (!name.startsWith(prefix) || !name.endsWith('.jsonl')) continue; const pendingPath = path.join(dir, name); try { const content = await fs.promises.readFile(pendingPath, 'utf-8'); if (!content.endsWith('\n')) continue; - await fs.promises.appendFile(usagePath, content, 'utf-8'); + folded ??= new Set((await fs.promises.readFile(usagePath, 'utf-8').catch(() => '')).split('\n')); + if (!folded.has(content.slice(0, -1))) { + await fs.promises.appendFile(usagePath, content, 'utf-8'); + folded.add(content.slice(0, -1)); + } await fs.promises.rm(pendingPath, { force: true }); } catch (e) { log.debug(`Could not fold ${pendingPath} into ${usagePath}: ${e instanceof Error ? e.message : String(e)}`); From e80b19e2a85c5aeba0707539d8bf4ba44639f9ee Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 24 Sep 2026 16:16:43 +0200 Subject: [PATCH 7/8] fix(usage): tell a pending event apart from an identical one (#788) The fold skips a side file's line when the usage file already holds it, so a genuine event with the same skill, tool and millisecond as one already in the file was dropped as a duplicate. A side file's line now carries the id its file name already has; readers keep only skill, tool and timestamp, so nothing downstream sees it. --- src/__tests__/usage-tracking.test.ts | 45 +++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/__tests__/usage-tracking.test.ts b/src/__tests__/usage-tracking.test.ts index d8e5eb13f..279e9bee8 100644 --- a/src/__tests__/usage-tracking.test.ts +++ b/src/__tests__/usage-tracking.test.ts @@ -31,7 +31,7 @@ import { extractSkillName, skillExistsOnDisk, } from '../usage-tracker.js'; -import { aggregateUsage } from '../stats.js'; +import { aggregateUsage, showStats } from '../stats.js'; import { mergeStats } from '../team-push.js'; import { calculateSkillHealth, scoreToStars, calculateTeamHealth } from '../skill-health.js'; import { getRecommendations } from '../skill-recommend.js'; @@ -474,6 +474,49 @@ describe('usage file lock (#788)', () => { expect(await pendingFiles()).toEqual([]); }); + it('keeps a side file event identical to one already in the file', async () => { + await appendUsageEvent(event('a'), userScope()); + await writeLock(process.pid); + await appendUsageEvent(event('a'), userScope()); + await fs.promises.rm(`${usagePath()}.lock`); + + await appendUsageEvent(event('b'), userScope()); + + expect(await skills()).toEqual(['a', 'a', 'b']); + expect(await pendingFiles()).toEqual([]); + }); + + it('keeps two identical events recorded in side files at once', async () => { + await writeLock(process.pid); + await Promise.all([appendUsageEvent(event('a'), userScope()), appendUsageEvent(event('a'), userScope())]); + expect(await pendingFiles()).toHaveLength(2); + await fs.promises.rm(`${usagePath()}.lock`); + + await appendUsageEvent(event('b'), userScope()); + + expect(await skills()).toEqual(['a', 'a', 'b']); + expect(await pendingFiles()).toEqual([]); + }); + + it('keeps the side file id out of what readers and `teamai stats` see', async () => { + await writeLock(process.pid); + await appendUsageEvent(event('a'), userScope()); + await fs.promises.rm(`${usagePath()}.lock`); + await appendUsageEvent(event('b'), userScope()); + expect(await fs.promises.readFile(usagePath(), 'utf-8')).toContain('"pendingId"'); + + expect(await readUsageEvents(userScope())).toStrictEqual([event('a'), event('b')]); + const out = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + await showStats(); + const printed = out.mock.calls.flat().join('\n'); + expect(printed).toContain('a'); + expect(printed).not.toContain('pendingId'); + } finally { + out.mockRestore(); + } + }); + const pendingMode = async () => (await fs.promises.stat(path.join(path.dirname(usagePath()), (await pendingFiles())[0]))).mode & 0o777; From 0f920ef99301b058af91eae8e7eb7b48c4b62eff Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 24 Sep 2026 16:20:15 +0200 Subject: [PATCH 8/8] fix(usage): fold a side file by its id, not its line (#788) Two hooks recording the same skill and tool in the same millisecond while the lock is held write identical side files, and the fold kept only one. A side file's line now carries the id already in its file name, and the fold skips it only when the usage file holds that id. readUsageEvents keeps only skill, timestamp and tool, so the id never reaches stats or the report. --- src/__tests__/team-push-interventions.test.ts | 16 +++++++++ src/usage-tracker.ts | 36 ++++++++++++++----- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/__tests__/team-push-interventions.test.ts b/src/__tests__/team-push-interventions.test.ts index f1d20f3fd..1b421ef39 100644 --- a/src/__tests__/team-push-interventions.test.ts +++ b/src/__tests__/team-push-interventions.test.ts @@ -290,3 +290,19 @@ describe('reportUsageToTeam — preserve fields across partial reports (Issue #4 expect(pushRepoDirectly).not.toHaveBeenCalled(); }); }); + +describe('reportUsageToTeam — usage recorded while the usage lock was held (#788)', () => { + it('reports the event without the side file id it was folded in with', async () => { + const timestamp = new Date().toISOString(); + const usagePath = path.join(tmpDir, '.teamai', 'user-usage.jsonl'); + fs.mkdirSync(path.dirname(usagePath), { recursive: true }); + fs.writeFileSync(usagePath, JSON.stringify({ skill: 'review', timestamp, tool: 'claude', pendingId: '0f8e2c1a-1b2c-4d5e-8f90-a1b2c3d4e5f6' }) + '\n'); + + expect(await reportUsageToTeam(repoDir, 'me', { selfConfig: gitConfig() })).toBe(true); + + const yaml = fs.readFileSync(reportsStatsPath(), 'utf-8'); + expect(YAML.parse(yaml).skills.review.count).toBe(1); + expect(yaml).not.toContain('pendingId'); + expect(yaml).not.toContain('0f8e2c1a'); + }); +}); diff --git a/src/usage-tracker.ts b/src/usage-tracker.ts index 3c8f5ace1..68e37be17 100644 --- a/src/usage-tracker.ts +++ b/src/usage-tracker.ts @@ -246,9 +246,12 @@ export async function appendUsageEvent(event: UsageEvent, config: LocalConfig): // It holds what the usage file holds, so it gets no wider mode than that file // (the umask can only narrow it); owner-only while there is no file yet. await ignoreUsageSideFiles(config, usagePath); - const pendingPath = path.join(path.dirname(usagePath), `${pendingPrefix(usagePath)}${randomUUID()}.jsonl`); + // The id lets a fold tell whether this very line is already in the file; + // readers drop it (readUsageEvents), so it never leaves this machine. + const pendingId = randomUUID(); + const pendingPath = path.join(path.dirname(usagePath), `${pendingPrefix(usagePath)}${pendingId}.jsonl`); const mode = await fs.promises.stat(usagePath).then((s) => s.mode & 0o777, () => 0o600); - await fs.promises.writeFile(pendingPath, line, { encoding: 'utf-8', flag: 'wx', mode }); + await fs.promises.writeFile(pendingPath, JSON.stringify({ ...event, pendingId }) + '\n', { encoding: 'utf-8', flag: 'wx', mode }); log.debug(`Tracked skill: ${event.skill} (in ${pendingPath}; ${usagePath}.lock is held)`); } catch (e) { log.error(`Failed to write usage event: ${(e as Error).message}`); @@ -269,7 +272,8 @@ export async function readUsageEvents(config: LocalConfig): Promise { const dir = path.dirname(usagePath); @@ -378,10 +382,13 @@ async function foldPendingEvents(usagePath: string): Promise { try { const content = await fs.promises.readFile(pendingPath, 'utf-8'); if (!content.endsWith('\n')) continue; - folded ??= new Set((await fs.promises.readFile(usagePath, 'utf-8').catch(() => '')).split('\n')); - if (!folded.has(content.slice(0, -1))) { + const id = pendingIdOf(content); + folded ??= new Set( + (await fs.promises.readFile(usagePath, 'utf-8').catch(() => '')).split('\n').map(pendingIdOf).filter((i) => i !== undefined), + ); + if (id === undefined || !folded.has(id)) { await fs.promises.appendFile(usagePath, content, 'utf-8'); - folded.add(content.slice(0, -1)); + if (id !== undefined) folded.add(id); } await fs.promises.rm(pendingPath, { force: true }); } catch (e) { @@ -390,6 +397,17 @@ async function foldPendingEvents(usagePath: string): Promise { } } +/** The id a side file gave its line, if the line has one. */ +function pendingIdOf(line: string): string | undefined { + if (!line.includes('"pendingId"')) return undefined; + try { + const { pendingId } = JSON.parse(line) as { pendingId?: unknown }; + return typeof pendingId === 'string' ? pendingId : undefined; + } catch { + return undefined; + } +} + /** * Replace a scope's usage file with the non-empty lines `keep` returns, or * leave it untouched when `keep` returns null. The copy is written to a temp