diff --git a/CHANGELOG.md b/CHANGELOG.md index cb11e05f7..75d215f39 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. 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 recall ` searches nothing in a project whose config exists but cannot be read, and says why. Detection skipped the broken file and searched whatever loaded next: the user scope, or a legacy `.teamai/` behind a broken partition that may belong to another team, whose knowledge was returned and whose documents got recalled counts. It now prints ``Nothing was searched: : . Fix the file, or move it aside and run `teamai init` to write a new one.`` and exits 1, and `recall --check` does the same instead of printing `NOT_RELEVANT`, which told the recall subagent the team had no knowledge. The `teamai-recall` subagent a pull from this release deploys relays that line to the member, telling the main conversation to move the file or re-run `teamai init` only with their consent. (for [#796](https://github.com/Tencent/teamai-cli/issues/796)). diff --git a/docs/designs/team-intelligence-platform.md b/docs/designs/team-intelligence-platform.md index 53771acf7..738f0b26a 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,16 @@ reports worktree → git add → git commit → git push (teamai-reports) 推送和本地确认,并在完成后释放相关同步锁。失败重试即使重建出与上次提交 相同的文件,也会重新推送已有提交,无需创建空提交。 +所有目标处理完后,Pull 把每个活跃 scope 的 usage 文件截为最新 5,000 条(含 +HTTP 与 `usageReport: false` 这些从不上报的 scope,#788)。截断必须在清理之后: +清理按上报读到的条数删除文件开头的行,中间插入截断会把删除挪到未上报的事件上。 +Hook 追加、清理与截断共用 usage 文件旁的一把锁(`.lock`),改写经临时文件 +再 rename,保留原文件权限。Hook 约 250 ms 拿不到锁时写入 `*.pending-.jsonl`(O_EXCL), +由下一个持锁者并入;改写约 5 秒拿不到锁时不改文件。pending 文件权限不宽于 usage 文件 +(尚无 usage 文件时为 0600)。工作区内 `.teamai/.gitignore` 忽略 `usage.jsonl.*` 与 +`usage.pending-*.jsonl`,单仓库模式的已有文件由 `migrateSelfModeGitignoreContent` 补齐, +项目级的已有文件在写 pending 文件或改写前由 `ignoreUsageSideFiles` 补齐。 + 此修复不提供持久化批次或远端去重:进程在推送与确认之间终止、多仓库部分 成功后的重试仍可能产生重复统计。等待超时不等于取消 Git 或强制退出进程。 diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 25c66c982..054218e64 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -1778,7 +1778,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 @@ -1794,6 +1794,23 @@ 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. 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. 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, and +the usage file's first pending file or rewrite adds them to an existing +project-scope one. + ### 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 327186b07..6fbbf7dce 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -1709,7 +1709,7 @@ Copilot 会话以及从软链接路径启动的会话。旧版本记录的事件 位于其根目录下的那些,user scope 一条也不上报。每个 scope 还各自保存已上报快照, 因此会话中途切换到另一个项目时,两个团队各自收到在其中记录的那部分;升级后的首次上报 从原先各 scope 共用的快照开始,不会重复上报。目标确认成功后才清理自己的使用事件, -推送失败会保留事件。上报完成前继续持有相关同步锁, +推送失败会保留事件(最多保留最新 5,000 条,见下文)。上报完成前继续持有相关同步锁, 避免另一次 Pull 与尚未完成的上报竞争。 这仍是尽力上报,不提供崩溃恢复保证:远端推送成功与本地确认之间如果进程 @@ -1723,6 +1723,16 @@ Copilot 会话以及从软链接路径启动的会话。旧版本记录的事件 usageReport: false ``` +Pull 在上报步骤之后把每个 scope 的使用文件限制为最新 5,000 条事件,丢弃更早的 +事件。对 HTTP 源或 `usageReport: false` 的团队,该文件是 `teamai stats` 唯一的 +数据来源,因此文件保持有界而不会被清空;上报未完成且事件超过 5,000 条的上报 +scope 也以同样方式丢弃最早的未上报事件。该上限只在上报清理完已发送事件之后执行。Hook 追加、上报后的清理与该上限共用使用文件旁的一把锁, +因此改写文件时不会丢失期间记录的事件。Hook 在约 250 ms 内拿不到锁时,把事件写入旁边的 +`*.pending-.jsonl` 文件,由下一个持锁者追加进使用文件;改写在约 5 秒内拿不到锁时保持文件不变。 +pending 文件的权限不宽于使用文件(尚无使用文件时仅所有者可读写)。工作区内的 `.teamai/.gitignore` +忽略该锁、改写的临时副本与 pending 文件;`pull` 与 `push` 会为已有的单仓库 `.gitignore` 补上这些条目, +已有的项目级 `.gitignore` 则在使用文件第一次写 pending 文件或改写时补上。 + ### 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__/single-repo-mode.test.ts b/src/__tests__/single-repo-mode.test.ts index b9e14fd37..186f9f5b2 100644 --- a/src/__tests__/single-repo-mode.test.ts +++ b/src/__tests__/single-repo-mode.test.ts @@ -1,5 +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, @@ -10,7 +14,30 @@ 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[] { + 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 { @@ -141,6 +168,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/'); @@ -149,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'); @@ -166,6 +221,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 +252,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 +267,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__/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/__tests__/usage-tracking.test.ts b/src/__tests__/usage-tracking.test.ts index a00e5d544..279e9bee8 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, @@ -28,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'; @@ -266,6 +269,427 @@ 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); + }); + + 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('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('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([]); + }); + + 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; + + 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(), + 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; + 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 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); + }); + + 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)', () => { 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/init.ts b/src/init.ts index 32ec58a62..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, @@ -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') }; } @@ -716,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.', @@ -1715,6 +1723,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/pull.ts b/src/pull.ts index 6196b50af..b7c715826 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -2023,7 +2023,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 @@ -2067,6 +2067,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..68e37be17 100644 --- a/src/usage-tracker.ts +++ b/src/usage-tracker.ts @@ -1,3 +1,4 @@ +import { randomBytes, randomUUID } from 'node:crypto'; import fs from 'node:fs'; import path from 'node:path'; import { log } from './utils/logger.js'; @@ -13,10 +14,11 @@ 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'; +import { acquireLock, releaseLock } from './update.js'; /** * The usage JSONL of one scope: `/usage.jsonl`, so each scope reports @@ -235,8 +237,22 @@ 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'); - 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. + // 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); + // 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, 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}`); } @@ -256,7 +272,8 @@ 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. + * 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( + usagePath: string, + wait: { attempts: number; delayMs: number }, + fn: () => Promise, +): Promise { + const lockPath = `${usagePath}.lock`; + 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)); + } + 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. A side file whose id the + * file already holds was folded by a holder that died or could not remove it, + * so it is not appended again; identical events keep their own ids and lines. + */ +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; + 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'); + if (id !== undefined) folded.add(id); + } + await fs.promises.rm(pendingPath, { force: true }); + } catch (e) { + log.debug(`Could not fold ${pendingPath} into ${usagePath}: ${e instanceof Error ? e.message : String(e)}`); + } + } +} + +/** 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 + * 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); + 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); + 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; + 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; + } + }); + 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`); + } +} + +/** 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); + // 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)}`); + } +} + +/** 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.