From 785648e6da1b913497ac86b27f2929728682328b Mon Sep 17 00:00:00 2001 From: ydflow <314143294+ydflow@users.noreply.github.com> Date: Sat, 26 Sep 2026 13:22:33 +0800 Subject: [PATCH 1/5] fix(dashboard): serialize events.jsonl append and compaction (#804) events.jsonl was appended lock-free by every dashboard hook on the machine and periodically rewritten by compactEvents (read -> filter -> temp file -> rename). The atomic rename only prevented a torn file: an append that landed between the rewrite's read and its rename was overwritten and lost, so the dashboard's sessions, interventions and prompt counts under-counted until the next rebuild. Two concurrent compactions also shared one fixed temp name, so their rewrites could clobber each other's copy. Both writers now take .lock (acquireLock from update.ts, the pattern the usage file took for the same lost update in #790/#803): a hook append waits up to ~250 ms, inside its foreground budget, and one that gives up records its line in an events.pending-.jsonl side file that the next lock holder folds into the file before it writes. The line carries a pendingId, so a fold never appends a side file twice, identical events in separate side files are both kept, and no reader (readEventsRaw) and no compacted file keeps the id. A compaction waits up to ~5 s for a peer's rewrite and skips, leaving the file as it is for the next compaction, when a live holder outlasts the wait; a lock whose owner is gone is reclaimed, and a rewrite's temp copy a killed compaction left (events.jsonl...tmp) is removed by the next one, which now also preserves the file's mode. The side files and the lock live in ~/.teamai/dashboard/ beside the log, which no repository tracks. --- CHANGELOG.md | 1 + docs/designs/data-directory-layout.md | 21 +++ src/__tests__/dashboard-collector.test.ts | 163 ++++++++++++++++++ src/dashboard-collector.ts | 196 ++++++++++++++++++---- 4 files changed, 351 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0f7326aa..72243da25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ All notable changes to this project will be documented in this file. See [standa - A misspelled top-level key in `mcp/mcp.yaml` or `hooks/hooks.yaml` (`server:` for `servers:`, `hook:` for `hooks:`) no longer removes every installed team MCP server or hook: such a file read as empty. It now fails like a file that does not parse, so pull keeps what is installed, and pull and `teamai doctor` name the file, the keys found and the key expected. An extra top-level key beside `servers:` or `hooks:` is still ignored (for [#822](https://github.com/Tencent/teamai-cli/issues/822)). - The closing line of `teamai recall` output is in English (for [#822](https://github.com/Tencent/teamai-cli/issues/822)). +- The dashboard event log no longer drops events to a compaction race. `~/.teamai/dashboard/events.jsonl` is appended by every dashboard hook on the machine, and compaction shrank it by a lock-free read → filter → temp-file → rename, so an event appended between the rewrite's read and its rename was overwritten and lost: the dashboard's sessions, interventions and prompt counts under-counted until the next rebuild. Every writer of the file now takes `events.jsonl.lock` (the `acquireLock` retry the usage file took for the same lost update, [#803](https://github.com/Tencent/teamai-cli/issues/803)): a hook append waits up to ~250 ms, inside its foreground budget, and one that gives up records its line in an `events.pending-.jsonl` side file that the next lock holder folds into the file before it writes, so an event is late, never gone; the line's `pendingId` keeps a fold from appending a side file twice, identical events in separate side files are both kept, and no reader and no compacted file keeps the id. A compaction waits up to ~5 s for a peer's rewrite and skips — leaving the file as it is for the next compaction — when a live holder outlasts the wait, and a lock whose owner is gone is reclaimed (for [#804](https://github.com/Tencent/teamai-cli/issues/804)). - A broken team file no longer wipes or downgrades what a member has installed. An `mcp/mcp.yaml` or `hooks/hooks.yaml` that did not parse reconciled to an empty set and removed every team MCP server or hook from every tool, and two active namespaces defining one skill or agent aborted the pull for the whole scope, skipping rules, env, docs and cleanup. Now a file in the active set that does not parse or cannot be read, a name repeated inside one file, or one name in two active namespaces stops only that resource type for the run: env keeps `env.sh`, hooks and MCP keep their entries (the built-in hooks, with the session-start pull, are still installed where missing: with the root hooks file's `builtin:` overrides when it parses, and otherwise with their defaults only in a tool that has none yet; `teamai init` says the team hooks were not installed), model profiles leave switched agents alone, skills and agents keep what is installed, and every other type still syncs. The warning names the file or both files and the fix, and for env, hooks, MCP and models is also written to `~/.teamai/debug.log` for session-start pulls (for [#707](https://github.com/Tencent/teamai-cli/issues/707)). - In single-repo (`kind: self`) mode, every checkout of the business repo now publishes learnings and reports. The `teamai-learnings` and `teamai-reports` checkouts and the queue of unpublished learnings lived in each checkout's own `.teamai/`, and git checks a branch out in one worktree only, so the first checkout to create them locked every other one out (`'teamai-learnings' is already used by worktree`), and a learning queued in a linked worktree was deleted with it by a plain `git worktree remove`. They now live in the project partition that every checkout shares, and the search index is kept per checkout, so `recall` no longer serves another checkout's index with paths into it. On upgrade, `init`, `pull`, `push`, `contribute` and `import --from-mr` migrate a checkout's old install, moving its queue into the partition without overwriting anything (or into `pending-learnings.self` when another checkout has since switched the project to another kind; an old git-mode install beside the knowledge of a switch to single-repo mode goes to `.teamai.bak`, its queue to `pending-learnings.git`); `contribute` and `import --from-mr` stop, saving nothing, while that data cannot move out of the checkout; and the first command that needs a side-branch checkout removes the old one from `.teamai/`; one with uncommitted changes is kept, a warning names it and says what to do, and `recall maintenance` and `recall promote` stop until then, while another command holds the learnings or reports lock, and when a checkout cannot be created, naming the cause. A git-mode install of the same project uses the same partition paths, so a checkout that belongs to the other repository is refused, never reused or removed, with the command that clears it, and so is one whose repository was moved or deleted; its votes are not counted, `init` deletes the search indexes built for the old repository, learnings the old install still had queued are set aside in `pending-learnings.` rather than published by the new one (and in `pending-learnings.-` when `init` points an install at another team repository of the same kind, which on `main` published them to the new one; the same repository written another way keeps them, [#823](https://github.com/Tencent/teamai-cli/issues/823) item 13), and `uninstall` lists every unpublished queue before it asks. (for [#808](https://github.com/Tencent/teamai-cli/issues/808)). - 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)). diff --git a/docs/designs/data-directory-layout.md b/docs/designs/data-directory-layout.md index c61c9657b..9e7944a03 100644 --- a/docs/designs/data-directory-layout.md +++ b/docs/designs/data-directory-layout.md @@ -800,6 +800,27 @@ what the machine had reported by then, never from another scope's later report. The seed holds a session's whole total, so a session still running at the upgrade goes on from the reported total, as before. +Every writer of `events.jsonl` — each hook's append and the periodic +compaction — takes `events.jsonl.lock` beside it (#804), the pattern the usage +file took for the same lost update (#803): compaction's +read → filter → temp-file → rename cannot drop an append that lands while it +runs, and two compactions cannot interleave their rewrites. A hook append waits +up to ~250 ms, inside its foreground budget, and one that gives up records its +line in an `events.pending-.jsonl` side file, with the file's mode, that +the next lock holder folds into the file before it writes — so an event is +late, never gone. The line carries a `pendingId`, so a fold never appends a +side file twice (a holder that died after appending it but before removing it +leaves it for the next one), two side files of identical events are both kept, +and no reader and no compacted file keeps the id: `readEventsRaw` drops it, and +a compaction rewrites the line without it. A side file without its trailing +newline is still being written and waits for the next holder. A compaction +waits up to ~5 s for a peer's rewrite, and skips — leaving the file as it is +for the next compaction — when a live holder outlasts the wait; a lock whose +owner is gone is reclaimed, and a rewrite's temp copy left by a killed +compaction (`events.jsonl...tmp`) is removed by the next one. The +side files and the lock live in `~/.teamai/dashboard/` beside the log, which +no repository tracks, so nothing needs adding to a workspace `.gitignore`. + **`anchor` on save.** Previously only migration wrote a partition's `anchor` reverse-lookup file, so freshly-init'd partitions had none. `saveLocalConfigForScope` now writes it whenever the config lands in a partition (via the shared diff --git a/src/__tests__/dashboard-collector.test.ts b/src/__tests__/dashboard-collector.test.ts index 09a1107ab..d59dc4ce9 100644 --- a/src/__tests__/dashboard-collector.test.ts +++ b/src/__tests__/dashboard-collector.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { spawnSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; @@ -2146,6 +2147,168 @@ describe('compactEvents', () => { }); }); +// ─── events file lock (#804) ──────────────────────────── + +describe('events file lock (#804)', () => { + const eventsPath = () => path.join(tmpDir, '.teamai', 'dashboard', 'events.jsonl'); + const event = (sessionId: string, extra: Partial = {}): DashboardEvent => + ({ type: 'session_start', timestamp: '2026-01-01T00:00:00Z', sessionId, tool: 'claude', ...extra }); + const ensureEventsDir = async () => fs.promises.mkdir(path.dirname(eventsPath()), { recursive: true }); + const ids = async () => (await readEvents(eventsPath())).map((e) => e.sessionId); + const writeLock = async (pid: number) => { + await ensureEventsDir(); + await fs.promises.writeFile( + `${eventsPath()}.lock`, + JSON.stringify({ pid, startedAt: '2026-01-01T00:00:00Z', owner: 'other' }), + 'utf-8', + ); + }; + const pendingFiles = async () => + (await fs.promises.readdir(path.dirname(eventsPath()))).filter((n) => n.startsWith('events.pending-')); + // The seed every compaction test needs: past the threshold, and every + // session alive at its own monitor (process.pid), so compaction keeps it. + const seedActive = async () => { + await ensureEventsDir(); + const line = (sessionId: string) => + JSON.stringify({ type: 'session_start', timestamp: '2026-01-01T00:00:00Z', sessionId, tool: 'copilot', monitorPid: process.pid }); + await fs.promises.writeFile(eventsPath(), Array.from({ length: 5_000 }, (_, i) => line(`filler-${i}`)).join('\n') + '\n', 'utf-8'); + }; + + it('reclaims a lock whose owner is gone', async () => { + await writeLock(spawnSync(process.execPath, ['-e', '']).pid ?? 0); + + await appendEvent(event('a')); + + expect(await ids()).toEqual(['a']); + expect(fs.existsSync(`${eventsPath()}.lock`)).toBe(false); + }); + + it('records an event beside the file within the hook budget while the lock stays held, and folds it in later', async () => { + await appendEvent(event('a')); + await writeLock(process.pid); + + const started = Date.now(); + await appendEvent(event('b')); + + expect(Date.now() - started).toBeLessThan(1000); + expect(await ids()).toEqual(['a']); + expect(await pendingFiles()).toHaveLength(1); + + await fs.promises.rm(`${eventsPath()}.lock`); + await appendEvent(event('c')); + + expect(await ids()).toEqual(['a', 'b', 'c']); + expect(await pendingFiles()).toEqual([]); + }); + + it('keeps an append that lands between compaction\'s read and its rename', async () => { + await seedActive(); + + // While compaction holds its snapshot, an append lands. The lock makes the + // append record a side file the next holder folds in, so the rewrite + // cannot drop it, as it did before the lock (#804). The append lands after + // compaction's second read — the snapshot the rewrite is built from — so + // the old read-all-then-overwrite lost it at the rename. + const realReadFile = fs.promises.readFile; + let reads = 0; + const spy = vi.spyOn(fs.promises, 'readFile').mockImplementation(async (file, options) => { + const content = await realReadFile(file, options); + if (String(file) === eventsPath() && ++reads === 2) { + await appendEvent(event('live-b', { monitorPid: process.pid })); + } + return content; + }); + try { + await compactEvents(eventsPath()); + } finally { + spy.mockRestore(); + } + + // The next holder folds the side file in; the event was never lost. + await appendEvent(event('live-c', { monitorPid: process.pid })); + + const sessionIds = await ids(); + expect(sessionIds).toContain('live-b'); + expect(sessionIds).toContain('live-c'); + expect(await pendingFiles()).toEqual([]); + }); + + it('folds a side file into the compaction and keeps its id out of the file', async () => { + await seedActive(); + await writeLock(process.pid); + await appendEvent(event('live-b', { monitorPid: process.pid })); + expect(await pendingFiles()).toHaveLength(1); + await fs.promises.rm(`${eventsPath()}.lock`); + + await compactEvents(eventsPath()); + + const content = fs.readFileSync(eventsPath(), 'utf-8'); + expect(content).toContain('live-b'); + expect(content).not.toContain('pendingId'); + expect(await pendingFiles()).toEqual([]); + }); + + it('folds a side file even when the file is below the compaction threshold', async () => { + await appendEvent(event('a')); + await writeLock(process.pid); + await appendEvent(event('b')); + await fs.promises.rm(`${eventsPath()}.lock`); + + await compactEvents(eventsPath()); + + expect(await ids()).toEqual(['a', 'b']); + expect(await pendingFiles()).toEqual([]); + expect(fs.existsSync(`${eventsPath()}.lock`)).toBe(false); + }); + + it('folds a side file once when it outlives its append', async () => { + await appendEvent(event('a')); + await writeLock(process.pid); + await appendEvent(event('b')); + await fs.promises.rm(`${eventsPath()}.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 appendEvent(event('c')); + } finally { + spy.mockRestore(); + } + + await appendEvent(event('d')); + + expect(await ids()).toEqual(['a', 'b', 'c', 'd']); + expect(await pendingFiles()).toEqual([]); + }); + + it('keeps two identical events recorded in side files at once', async () => { + await writeLock(process.pid); + await Promise.all([appendEvent(event('x')), appendEvent(event('x'))]); + expect(await pendingFiles()).toHaveLength(2); + await fs.promises.rm(`${eventsPath()}.lock`); + + await appendEvent(event('y')); + + expect(await ids()).toEqual(['x', 'x', 'y']); + expect(await pendingFiles()).toEqual([]); + }); + + it('keeps the side file id out of what readers see', async () => { + await writeLock(process.pid); + await appendEvent(event('a')); + await fs.promises.rm(`${eventsPath()}.lock`); + await appendEvent(event('b')); + expect(await fs.promises.readFile(eventsPath(), 'utf-8')).toContain('"pendingId"'); + + const events = await readEvents(eventsPath()); + expect(events.map((e) => e.sessionId)).toEqual(['a', 'b']); + expect(events.some((e) => 'pendingId' in e)).toBe(false); + }); +}); + // ─── countInterventions (CodeBuddy index.json) ───────── describe('countInterventions (CodeBuddy index.json)', () => { diff --git a/src/dashboard-collector.ts b/src/dashboard-collector.ts index d636a0d1d..8187b9bed 100644 --- a/src/dashboard-collector.ts +++ b/src/dashboard-collector.ts @@ -1,7 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; import readline from 'node:readline'; -import { createHash, randomUUID } from 'node:crypto'; +import { createHash, randomBytes, randomUUID } from 'node:crypto'; import { log } from './utils/logger.js'; import { deriveDispatchSessionId } from './utils/session-id.js'; import { resolveHookCwd } from './utils/hook-cwd.js'; @@ -10,6 +10,7 @@ import { repoKeys, repoLabel } from './utils/repo-attribution.js'; import { isProcessAlive, resolveMonitorPid } from './pid-monitor.js'; import { normalizeToolName } from './utils/tool-names.js'; import { redactWithEnv } from './utils/redact.js'; +import { acquireLock, releaseLock } from './update.js'; import { DASHBOARD_COMPACTION_THRESHOLD, DASHBOARD_IDLE_TIMEOUT_MS, @@ -1509,6 +1510,108 @@ export async function dataHomeKey(dataHome: string): Promise { return createHash('sha256').update(norm.replace(/\/+$/, '')).digest('hex').slice(0, 16); } +// ─── Events file lock (#804) ──────────────────────────── +// +// appendEvent and compactEvents serialize on `.lock`, the +// pattern the usage file took for the same lost-update race (#788, #803): +// compaction's read-modify-write must not drop an append that lands while it +// runs, and two compactions must not 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 (acquireLock). + +/** A hook append waits at most ~250 ms for the events lock, inside its foreground budget. */ +const EVENTS_APPEND_LOCK_WAIT = { attempts: 10, delayMs: 25 }; +/** A compaction waits up to ~5 s for a peer's compaction to finish. */ +const EVENTS_REWRITE_LOCK_WAIT = { attempts: 100, delayMs: 50 }; + +/** + * Run `fn` holding the lock every writer of the events file takes (#804): a + * hook append and the compaction rewrite. A rewrite then cannot drop an + * append made while it runs, and two rewrites cannot interleave. Returns + * false, without running `fn`, when the lock is still held after the wait. + */ +async function withEventsLock( + eventsPath: string, + wait: { attempts: number; delayMs: number }, + fn: () => Promise, +): Promise { + const lockPath = `${eventsPath}.lock`; + for (let i = 0; i < wait.attempts; i++) { + if (await acquireLock(lockPath)) { + try { + await foldPendingEvents(eventsPath); + 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 events lock is held. */ +function eventsPendingPrefix(eventsPath: string): string { + return `${path.basename(eventsPath, '.jsonl')}.pending-`; +} + +/** + * Append the side files of appends that gave up on the lock to the events + * 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(eventsPath: string): Promise { + const dir = path.dirname(eventsPath); + const prefix = eventsPendingPrefix(eventsPath); + 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(eventsPath, 'utf-8').catch(() => '')).split('\n').map(pendingIdOf).filter((i) => i !== undefined), + ); + if (id === undefined || !folded.has(id)) { + await fs.promises.appendFile(eventsPath, content, 'utf-8'); + if (id !== undefined) folded.add(id); + } + await fs.promises.rm(pendingPath, { force: true }); + } catch (e) { + log.debug(`dashboard: could not fold ${pendingPath} into ${eventsPath}: ${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; + } +} + +/** Remove the temp copies a killed compaction 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); + } +} + /** * Append a DashboardEvent to the events JSONL file. * Silently fails on I/O errors to avoid disrupting the AI session. @@ -1518,13 +1621,27 @@ export async function appendEvent(event: DashboardEvent): Promise { const eventsPath = getEventsPath(); await ensureDir(path.dirname(eventsPath)); const line = JSON.stringify(event) + '\n'; - await fs.promises.appendFile(eventsPath, line, 'utf-8'); const detail = event.toolName ? ` [tool=${event.toolName}]` : event.promptSummary ? ` [prompt=${event.promptSummary}]` : ''; - log.debug(`dashboard: recorded ${event.type} for session ${event.sessionId.slice(0, 16)}${detail}`); + const summary = `dashboard: recorded ${event.type} for session ${event.sessionId.slice(0, 16)}${detail}`; + if (await withEventsLock(eventsPath, EVENTS_APPEND_LOCK_WAIT, () => fs.promises.appendFile(eventsPath, line, 'utf-8'))) { + log.debug(summary); + 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 (#804). + // It holds what the events 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. The id lets a fold tell whether this very line is already in the + // file; readers drop it (readEventsRaw), so it never leaves this machine. + const pendingId = randomUUID(); + const pendingPath = path.join(path.dirname(eventsPath), `${eventsPendingPrefix(eventsPath)}${pendingId}.jsonl`); + const mode = await fs.promises.stat(eventsPath).then((s) => s.mode & 0o777, () => 0o600); + await fs.promises.writeFile(pendingPath, JSON.stringify({ ...event, pendingId }) + '\n', { encoding: 'utf-8', flag: 'wx', mode }); + log.debug(`${summary} (in ${path.basename(pendingPath)}; ${path.basename(eventsPath)}.lock is held)`); } catch (e) { log.error(`dashboard: failed to write event: ${(e as Error).message}`); } @@ -1543,8 +1660,11 @@ async function readEventsRaw(filePath: string): Promise { const trimmed = line.trim(); if (!trimmed) continue; try { - const parsed = JSON.parse(trimmed) as DashboardEvent; + const parsed = JSON.parse(trimmed) as DashboardEvent & { pendingId?: unknown }; if (parsed.type && parsed.sessionId && parsed.timestamp) { + // A folded side file's id stays in the file (foldPendingEvents) until + // a compaction rewrites the line; no reader ever sees it. + delete parsed.pendingId; events.push(parsed); } } catch { @@ -2040,35 +2160,51 @@ export function aggregateSessionInterventions( export async function compactEvents(eventsPath?: string): Promise { const filePath = eventsPath ?? getEventsPath(); try { - const content = await fs.promises.readFile(filePath, 'utf-8'); - const lines = content.split('\n').filter(l => l.trim()); - - if (lines.length < DASHBOARD_COMPACTION_THRESHOLD) return; - - // Compaction rewrites the file, so it must preserve raw (append-order, - // un-deduped) events — dedup is a read-time view, not a disk mutation. - // Dedup never changes the active-session set, so activeIds is identical. - const events = await readEventsRaw(filePath); - const activeSessions = rebuildSessions(events); - const activeIds = new Set(activeSessions.map(s => s.sessionId)); - const monitored = new Map(); - for (const e of events) { - if (e.type === 'session_start' && typeof e.monitorPid === 'number') monitored.set(e.sessionId, e.monitorPid); - } - for (const [sessionId, pid] of monitored) { - if (isProcessAlive(pid)) activeIds.add(sessionId); - } + const locked = await withEventsLock(filePath, EVENTS_REWRITE_LOCK_WAIT, async () => { + // Replace the file itself, not a symlink to it. + const target = await fs.promises.realpath(filePath).catch(() => filePath); + await removeOrphanTemps(target); + const content = await fs.promises.readFile(target, 'utf-8'); + const lines = content.split('\n').filter(l => l.trim()); + + if (lines.length < DASHBOARD_COMPACTION_THRESHOLD) return; + + // Compaction rewrites the file, so it must preserve raw (append-order, + // un-deduped) events — dedup is a read-time view, not a disk mutation. + // Dedup never changes the active-session set, so activeIds is identical. + const events = await readEventsRaw(target); + const activeSessions = rebuildSessions(events); + const activeIds = new Set(activeSessions.map(s => s.sessionId)); + const monitored = new Map(); + for (const e of events) { + if (e.type === 'session_start' && typeof e.monitorPid === 'number') monitored.set(e.sessionId, e.monitorPid); + } + for (const [sessionId, pid] of monitored) { + if (isProcessAlive(pid)) activeIds.add(sessionId); + } - // Keep only events for active sessions - const kept = events.filter(e => activeIds.has(e.sessionId)); - const compacted = kept.map(e => JSON.stringify(e)).join('\n') + '\n'; + // Keep only events for active sessions + const kept = events.filter(e => activeIds.has(e.sessionId)); + const compacted = kept.map(e => JSON.stringify(e)).join('\n') + '\n'; - // Atomic write: write to temp, then rename - const tmpPath = filePath + '.tmp'; - await fs.promises.writeFile(tmpPath, compacted, 'utf-8'); - await fs.promises.rename(tmpPath, filePath); + // Atomic write: write to temp with the file's mode, then rename. The + // lock makes the temp name ours; a killed compaction's copy is an + // orphan a later one removes (removeOrphanTemps). + const mode = await fs.promises.stat(target).then((s) => s.mode & 0o7777, () => 0o600); + const tmpPath = `${target}.${process.pid}.${randomBytes(6).toString('hex')}.tmp`; + try { + await fs.promises.writeFile(tmpPath, compacted, { encoding: 'utf-8', mode }); + await fs.promises.rename(tmpPath, target); + } catch (e) { + await fs.promises.rm(tmpPath, { force: true }).catch(() => undefined); + throw e; + } - log.debug(`dashboard: compacted ${lines.length} → ${kept.length} events`); + log.debug(`dashboard: compacted ${lines.length} → ${kept.length} events`); + }); + // An opportunistic cleanup: leave the file as it is and let the next + // compaction try again, rather than reporting a lost race as a failure. + if (!locked) log.debug(`dashboard: ${path.basename(filePath)}.lock is still held after 5 s, so compaction is skipped`); } catch (e) { log.error(`dashboard: compaction failed: ${(e as Error).message}`); } From 356f4b58ab7b076b9bedab1fad44724cc5950a8c Mon Sep 17 00:00:00 2001 From: ydflow <314143294+ydflow@users.noreply.github.com> Date: Sat, 26 Sep 2026 14:11:22 +0800 Subject: [PATCH 2/5] test(dashboard-report-scope): provide the events lock in the update.js mock dashboard-collector now imports acquireLock and releaseLock from update.js (#804), and this file's factory mock replaced that module without them, so appendEvent lost every event to the missing export and 49 report tests failed on CI. Follow the pull-* tests' mock shape: acquireLock resolves true and releaseLock resolves undefined, keeping these tests lock-free, as they ran before the lock. --- src/__tests__/dashboard-report-scope.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/__tests__/dashboard-report-scope.test.ts b/src/__tests__/dashboard-report-scope.test.ts index b4ae939d6..73ff79cb5 100644 --- a/src/__tests__/dashboard-report-scope.test.ts +++ b/src/__tests__/dashboard-report-scope.test.ts @@ -18,7 +18,13 @@ vi.mock('node:child_process', async (importOriginal) => ({ spawn: vi.fn(() => ({ on: vi.fn(), stdin: { on: vi.fn(), end: vi.fn((_: string, done: () => void) => done()) }, unref: vi.fn() })), })); vi.mock('../pull.js', () => ({ pull: vi.fn(async () => undefined) })); -vi.mock('../update.js', () => ({ doUpdate: vi.fn(async () => undefined) })); +vi.mock('../update.js', () => ({ + doUpdate: vi.fn(async () => undefined), + // appendEvent and compactEvents take the events file's lock (#804); the + // no-op acquisition keeps these tests lock-free, as they were before it. + acquireLock: vi.fn().mockResolvedValue(true), + releaseLock: vi.fn().mockResolvedValue(undefined), +})); vi.mock('../local-agent.js', () => ({ reportAndSyncFromHook: vi.fn(async () => null) })); // Each scope's reports branch is a plain directory next to its team repo. vi.mock('../utils/reports-branch.js', () => ({ From c74940b6187f5132529f8099dfd450c513a2f714 Mon Sep 17 00:00:00 2001 From: ydflow <314143294+ydflow@users.noreply.github.com> Date: Sat, 26 Sep 2026 14:21:29 +0800 Subject: [PATCH 3/5] test(dashboard-collector): match the compaction read by file name macOS resolves a file under os.tmpdir() through /private, so compaction's realpath'd target never equals eventsPath() and the counterexample's injection never fired: live-b was never appended and the test failed on every macos job. Match the read by its file name instead, which the realpath keeps. --- src/__tests__/dashboard-collector.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/__tests__/dashboard-collector.test.ts b/src/__tests__/dashboard-collector.test.ts index d59dc4ce9..3609eeed6 100644 --- a/src/__tests__/dashboard-collector.test.ts +++ b/src/__tests__/dashboard-collector.test.ts @@ -2208,12 +2208,14 @@ describe('events file lock (#804)', () => { // append record a side file the next holder folds in, so the rewrite // cannot drop it, as it did before the lock (#804). The append lands after // compaction's second read — the snapshot the rewrite is built from — so - // the old read-all-then-overwrite lost it at the rename. + // the old read-all-then-overwrite lost it at the rename. Matched by file + // name: compaction reads the realpath'd target, which on macOS prefixes + // the temp dir with /private and would never equal eventsPath(). const realReadFile = fs.promises.readFile; let reads = 0; const spy = vi.spyOn(fs.promises, 'readFile').mockImplementation(async (file, options) => { const content = await realReadFile(file, options); - if (String(file) === eventsPath() && ++reads === 2) { + if (path.basename(String(file)) === 'events.jsonl' && ++reads === 2) { await appendEvent(event('live-b', { monitorPid: process.pid })); } return content; From eee74fd9af41adf04fdf6fa6ce859fe51bcfb8ce Mon Sep 17 00:00:00 2001 From: ydflow <314143294+ydflow@users.noreply.github.com> Date: Sat, 26 Sep 2026 14:48:18 +0800 Subject: [PATCH 4/5] fix(dashboard): skip the events lock when compaction has nothing to do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The detached compaction after every append took the events lock even when the log was below the threshold and no side files were pending — nearly always. On a state directory a test teardown deletes concurrently, that turned the no-op into lock-file churn racing the deletion, which CI caught as an unhandled ENOENT on macOS. Read first, lock-free: below the threshold with no side files the compaction returns without creating the lock, the I/O profile of the pre-#804 compaction; a rewrite, or side files to fold, still take it. --- CHANGELOG.md | 2 +- docs/designs/data-directory-layout.md | 11 ++++++++--- src/__tests__/dashboard-collector.test.ts | 12 +++++++----- src/dashboard-collector.ts | 14 ++++++++++++++ 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72243da25..d45697a40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ All notable changes to this project will be documented in this file. See [standa - A misspelled top-level key in `mcp/mcp.yaml` or `hooks/hooks.yaml` (`server:` for `servers:`, `hook:` for `hooks:`) no longer removes every installed team MCP server or hook: such a file read as empty. It now fails like a file that does not parse, so pull keeps what is installed, and pull and `teamai doctor` name the file, the keys found and the key expected. An extra top-level key beside `servers:` or `hooks:` is still ignored (for [#822](https://github.com/Tencent/teamai-cli/issues/822)). - The closing line of `teamai recall` output is in English (for [#822](https://github.com/Tencent/teamai-cli/issues/822)). -- The dashboard event log no longer drops events to a compaction race. `~/.teamai/dashboard/events.jsonl` is appended by every dashboard hook on the machine, and compaction shrank it by a lock-free read → filter → temp-file → rename, so an event appended between the rewrite's read and its rename was overwritten and lost: the dashboard's sessions, interventions and prompt counts under-counted until the next rebuild. Every writer of the file now takes `events.jsonl.lock` (the `acquireLock` retry the usage file took for the same lost update, [#803](https://github.com/Tencent/teamai-cli/issues/803)): a hook append waits up to ~250 ms, inside its foreground budget, and one that gives up records its line in an `events.pending-.jsonl` side file that the next lock holder folds into the file before it writes, so an event is late, never gone; the line's `pendingId` keeps a fold from appending a side file twice, identical events in separate side files are both kept, and no reader and no compacted file keeps the id. A compaction waits up to ~5 s for a peer's rewrite and skips — leaving the file as it is for the next compaction — when a live holder outlasts the wait, and a lock whose owner is gone is reclaimed (for [#804](https://github.com/Tencent/teamai-cli/issues/804)). +- The dashboard event log no longer drops events to a compaction race. `~/.teamai/dashboard/events.jsonl` is appended by every dashboard hook on the machine, and compaction shrank it by a lock-free read → filter → temp-file → rename, so an event appended between the rewrite's read and its rename was overwritten and lost: the dashboard's sessions, interventions and prompt counts under-counted until the next rebuild. Every writer that may modify the file now takes `events.jsonl.lock` (the `acquireLock` retry the usage file took for the same lost update, [#803](https://github.com/Tencent/teamai-cli/issues/803)): a hook append waits up to ~250 ms, inside its foreground budget, and one that gives up records its line in an `events.pending-.jsonl` side file that the next lock holder folds into the file before it writes, so an event is late, never gone; the line's `pendingId` keeps a fold from appending a side file twice, identical events in separate side files are both kept, and no reader and no compacted file keeps the id. A compaction that finds the log below its threshold and no side files to fold still runs lock-free, so the common case costs one read and no lock. A rewrite waits up to ~5 s for a peer's rewrite and skips — leaving the file as it is for the next compaction — when a live holder outlasts the wait, and a lock whose owner is gone is reclaimed (for [#804](https://github.com/Tencent/teamai-cli/issues/804)). - A broken team file no longer wipes or downgrades what a member has installed. An `mcp/mcp.yaml` or `hooks/hooks.yaml` that did not parse reconciled to an empty set and removed every team MCP server or hook from every tool, and two active namespaces defining one skill or agent aborted the pull for the whole scope, skipping rules, env, docs and cleanup. Now a file in the active set that does not parse or cannot be read, a name repeated inside one file, or one name in two active namespaces stops only that resource type for the run: env keeps `env.sh`, hooks and MCP keep their entries (the built-in hooks, with the session-start pull, are still installed where missing: with the root hooks file's `builtin:` overrides when it parses, and otherwise with their defaults only in a tool that has none yet; `teamai init` says the team hooks were not installed), model profiles leave switched agents alone, skills and agents keep what is installed, and every other type still syncs. The warning names the file or both files and the fix, and for env, hooks, MCP and models is also written to `~/.teamai/debug.log` for session-start pulls (for [#707](https://github.com/Tencent/teamai-cli/issues/707)). - In single-repo (`kind: self`) mode, every checkout of the business repo now publishes learnings and reports. The `teamai-learnings` and `teamai-reports` checkouts and the queue of unpublished learnings lived in each checkout's own `.teamai/`, and git checks a branch out in one worktree only, so the first checkout to create them locked every other one out (`'teamai-learnings' is already used by worktree`), and a learning queued in a linked worktree was deleted with it by a plain `git worktree remove`. They now live in the project partition that every checkout shares, and the search index is kept per checkout, so `recall` no longer serves another checkout's index with paths into it. On upgrade, `init`, `pull`, `push`, `contribute` and `import --from-mr` migrate a checkout's old install, moving its queue into the partition without overwriting anything (or into `pending-learnings.self` when another checkout has since switched the project to another kind; an old git-mode install beside the knowledge of a switch to single-repo mode goes to `.teamai.bak`, its queue to `pending-learnings.git`); `contribute` and `import --from-mr` stop, saving nothing, while that data cannot move out of the checkout; and the first command that needs a side-branch checkout removes the old one from `.teamai/`; one with uncommitted changes is kept, a warning names it and says what to do, and `recall maintenance` and `recall promote` stop until then, while another command holds the learnings or reports lock, and when a checkout cannot be created, naming the cause. A git-mode install of the same project uses the same partition paths, so a checkout that belongs to the other repository is refused, never reused or removed, with the command that clears it, and so is one whose repository was moved or deleted; its votes are not counted, `init` deletes the search indexes built for the old repository, learnings the old install still had queued are set aside in `pending-learnings.` rather than published by the new one (and in `pending-learnings.-` when `init` points an install at another team repository of the same kind, which on `main` published them to the new one; the same repository written another way keeps them, [#823](https://github.com/Tencent/teamai-cli/issues/823) item 13), and `uninstall` lists every unpublished queue before it asks. (for [#808](https://github.com/Tencent/teamai-cli/issues/808)). - 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)). diff --git a/docs/designs/data-directory-layout.md b/docs/designs/data-directory-layout.md index 9e7944a03..dffd545ea 100644 --- a/docs/designs/data-directory-layout.md +++ b/docs/designs/data-directory-layout.md @@ -800,11 +800,16 @@ what the machine had reported by then, never from another scope's later report. The seed holds a session's whole total, so a session still running at the upgrade goes on from the reported total, as before. -Every writer of `events.jsonl` — each hook's append and the periodic -compaction — takes `events.jsonl.lock` beside it (#804), the pattern the usage +Every writer that may modify `events.jsonl` — each hook's append, and a +compaction that finds the log past its threshold or side files to fold — +takes `events.jsonl.lock` beside it (#804), the pattern the usage file took for the same lost update (#803): compaction's read → filter → temp-file → rename cannot drop an append that lands while it -runs, and two compactions cannot interleave their rewrites. A hook append waits +runs, and two compactions cannot interleave their rewrites. A compaction +whose log is below the threshold and holds no side files does the common +case lock-free: one read, and it returns without touching the lock, so a +state directory that is being torn down concurrently never meets a lock +creation. A hook append waits up to ~250 ms, inside its foreground budget, and one that gives up records its line in an `events.pending-.jsonl` side file, with the file's mode, that the next lock holder folds into the file before it writes — so an event is diff --git a/src/__tests__/dashboard-collector.test.ts b/src/__tests__/dashboard-collector.test.ts index 3609eeed6..f87f29a09 100644 --- a/src/__tests__/dashboard-collector.test.ts +++ b/src/__tests__/dashboard-collector.test.ts @@ -2207,15 +2207,17 @@ describe('events file lock (#804)', () => { // While compaction holds its snapshot, an append lands. The lock makes the // append record a side file the next holder folds in, so the rewrite // cannot drop it, as it did before the lock (#804). The append lands after - // compaction's second read — the snapshot the rewrite is built from — so - // the old read-all-then-overwrite lost it at the rename. Matched by file - // name: compaction reads the realpath'd target, which on macOS prefixes - // the temp dir with /private and would never equal eventsPath(). + // compaction's second read under the lock — the snapshot the rewrite is + // built from — so the old read-all-then-overwrite lost it at the rename. + // Reads are counted only while the lock file exists: compaction's + // lock-free pre-check read runs before it, and the realpath'd target it + // reads under the lock differs from eventsPath() on macOS (/private + // prefix), so both are matched by file name. const realReadFile = fs.promises.readFile; let reads = 0; const spy = vi.spyOn(fs.promises, 'readFile').mockImplementation(async (file, options) => { const content = await realReadFile(file, options); - if (path.basename(String(file)) === 'events.jsonl' && ++reads === 2) { + if (path.basename(String(file)) === 'events.jsonl' && fs.existsSync(`${eventsPath()}.lock`) && ++reads === 2) { await appendEvent(event('live-b', { monitorPid: process.pid })); } return content; diff --git a/src/dashboard-collector.ts b/src/dashboard-collector.ts index 8187b9bed..a8baef781 100644 --- a/src/dashboard-collector.ts +++ b/src/dashboard-collector.ts @@ -2160,6 +2160,20 @@ export function aggregateSessionInterventions( export async function compactEvents(eventsPath?: string): Promise { const filePath = eventsPath ?? getEventsPath(); try { + // Cheap lock-free pre-check: one of these runs detached after every + // append, and nearly always finds nothing to do. Below the threshold with + // no side files to fold it must not create the lock at all — the state it + // would lock may be a sandbox another test (or a rolling state dir) is + // deleting concurrently. + const pre = await fs.promises.readFile(filePath, 'utf-8').catch(() => null); + if (pre === null) return; + const preLines = pre.split('\n').filter(l => l.trim()); + const hasSideFiles = await fs.promises.readdir(path.dirname(filePath)).then( + (names) => names.some((n) => n.startsWith(eventsPendingPrefix(filePath)) && n.endsWith('.jsonl')), + () => false, + ); + if (preLines.length < DASHBOARD_COMPACTION_THRESHOLD && !hasSideFiles) return; + const locked = await withEventsLock(filePath, EVENTS_REWRITE_LOCK_WAIT, async () => { // Replace the file itself, not a symlink to it. const target = await fs.promises.realpath(filePath).catch(() => filePath); From 627d974a260ce6d093327d0bd9ee1511ce3c63d7 Mon Sep 17 00:00:00 2001 From: ydflow <314143294+ydflow@users.noreply.github.com> Date: Sat, 26 Sep 2026 19:40:49 +0800 Subject: [PATCH 5/5] fix(dashboard): keep a surviving side file recognizable, fold and classify in time order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on #841: A fold that appended its side file but could not remove it left the file unrecognizable after the next compaction: the rewrite dropped the line's pendingId, so the next holder could not tell the side file was already in the log and appended the event a second time, double-counting it in the dashboard's metrics. The id now stays in the raw file — a rewrite keeps it, as the usage file's rewrite does (#788) — until the side file itself is gone; readEvents drops it, so no reader ever sees it. Side files also folded in readdir's arbitrary order, and the compaction classified sessions from that raw order: a side file that outlived newer appends could place an older event after them, re-mark a live session stopped once its stopped-display window had passed, and the rewrite would drop all its events. Side files now fold in their events' own time order, and the compaction classifies in time order too — the order every reader rebuilds from (dedupeEvents sorts); the rewritten file keeps the raw order. --- CHANGELOG.md | 2 +- docs/designs/data-directory-layout.md | 15 +++-- src/__tests__/dashboard-collector.test.ts | 57 ++++++++++++++++++- src/dashboard-collector.ts | 67 ++++++++++++++++++----- 4 files changed, 119 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d45697a40..6a226b277 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ All notable changes to this project will be documented in this file. See [standa - A misspelled top-level key in `mcp/mcp.yaml` or `hooks/hooks.yaml` (`server:` for `servers:`, `hook:` for `hooks:`) no longer removes every installed team MCP server or hook: such a file read as empty. It now fails like a file that does not parse, so pull keeps what is installed, and pull and `teamai doctor` name the file, the keys found and the key expected. An extra top-level key beside `servers:` or `hooks:` is still ignored (for [#822](https://github.com/Tencent/teamai-cli/issues/822)). - The closing line of `teamai recall` output is in English (for [#822](https://github.com/Tencent/teamai-cli/issues/822)). -- The dashboard event log no longer drops events to a compaction race. `~/.teamai/dashboard/events.jsonl` is appended by every dashboard hook on the machine, and compaction shrank it by a lock-free read → filter → temp-file → rename, so an event appended between the rewrite's read and its rename was overwritten and lost: the dashboard's sessions, interventions and prompt counts under-counted until the next rebuild. Every writer that may modify the file now takes `events.jsonl.lock` (the `acquireLock` retry the usage file took for the same lost update, [#803](https://github.com/Tencent/teamai-cli/issues/803)): a hook append waits up to ~250 ms, inside its foreground budget, and one that gives up records its line in an `events.pending-.jsonl` side file that the next lock holder folds into the file before it writes, so an event is late, never gone; the line's `pendingId` keeps a fold from appending a side file twice, identical events in separate side files are both kept, and no reader and no compacted file keeps the id. A compaction that finds the log below its threshold and no side files to fold still runs lock-free, so the common case costs one read and no lock. A rewrite waits up to ~5 s for a peer's rewrite and skips — leaving the file as it is for the next compaction — when a live holder outlasts the wait, and a lock whose owner is gone is reclaimed (for [#804](https://github.com/Tencent/teamai-cli/issues/804)). +- The dashboard event log no longer drops events to a compaction race. `~/.teamai/dashboard/events.jsonl` is appended by every dashboard hook on the machine, and compaction shrank it by a lock-free read → filter → temp-file → rename, so an event appended between the rewrite's read and its rename was overwritten and lost: the dashboard's sessions, interventions and prompt counts under-counted until the next rebuild. Every writer that may modify the file now takes `events.jsonl.lock` (the `acquireLock` retry the usage file took for the same lost update, [#803](https://github.com/Tencent/teamai-cli/issues/803)): a hook append waits up to ~250 ms, inside its foreground budget, and one that gives up records its line in an `events.pending-.jsonl` side file that the next lock holder folds into the file before it writes, so an event is late, never gone; the line's `pendingId` keeps a fold from appending a side file twice, identical events in separate side files are both kept, and the id stays in the raw file — a compaction keeps it too, as the usage file's rewrite does — until the side file itself is gone, while no reader ever sees it. Side files fold in their events' own time order, and a compaction classifies sessions in time order as every reader does, so a side file that outlived newer appends cannot place an older event after them and re-mark a live session stopped. A compaction that finds the log below its threshold and no side files to fold still runs lock-free, so the common case costs one read and no lock. A rewrite waits up to ~5 s for a peer's rewrite and skips — leaving the file as it is for the next compaction — when a live holder outlasts the wait, and a lock whose owner is gone is reclaimed (for [#804](https://github.com/Tencent/teamai-cli/issues/804)). - A broken team file no longer wipes or downgrades what a member has installed. An `mcp/mcp.yaml` or `hooks/hooks.yaml` that did not parse reconciled to an empty set and removed every team MCP server or hook from every tool, and two active namespaces defining one skill or agent aborted the pull for the whole scope, skipping rules, env, docs and cleanup. Now a file in the active set that does not parse or cannot be read, a name repeated inside one file, or one name in two active namespaces stops only that resource type for the run: env keeps `env.sh`, hooks and MCP keep their entries (the built-in hooks, with the session-start pull, are still installed where missing: with the root hooks file's `builtin:` overrides when it parses, and otherwise with their defaults only in a tool that has none yet; `teamai init` says the team hooks were not installed), model profiles leave switched agents alone, skills and agents keep what is installed, and every other type still syncs. The warning names the file or both files and the fix, and for env, hooks, MCP and models is also written to `~/.teamai/debug.log` for session-start pulls (for [#707](https://github.com/Tencent/teamai-cli/issues/707)). - In single-repo (`kind: self`) mode, every checkout of the business repo now publishes learnings and reports. The `teamai-learnings` and `teamai-reports` checkouts and the queue of unpublished learnings lived in each checkout's own `.teamai/`, and git checks a branch out in one worktree only, so the first checkout to create them locked every other one out (`'teamai-learnings' is already used by worktree`), and a learning queued in a linked worktree was deleted with it by a plain `git worktree remove`. They now live in the project partition that every checkout shares, and the search index is kept per checkout, so `recall` no longer serves another checkout's index with paths into it. On upgrade, `init`, `pull`, `push`, `contribute` and `import --from-mr` migrate a checkout's old install, moving its queue into the partition without overwriting anything (or into `pending-learnings.self` when another checkout has since switched the project to another kind; an old git-mode install beside the knowledge of a switch to single-repo mode goes to `.teamai.bak`, its queue to `pending-learnings.git`); `contribute` and `import --from-mr` stop, saving nothing, while that data cannot move out of the checkout; and the first command that needs a side-branch checkout removes the old one from `.teamai/`; one with uncommitted changes is kept, a warning names it and says what to do, and `recall maintenance` and `recall promote` stop until then, while another command holds the learnings or reports lock, and when a checkout cannot be created, naming the cause. A git-mode install of the same project uses the same partition paths, so a checkout that belongs to the other repository is refused, never reused or removed, with the command that clears it, and so is one whose repository was moved or deleted; its votes are not counted, `init` deletes the search indexes built for the old repository, learnings the old install still had queued are set aside in `pending-learnings.` rather than published by the new one (and in `pending-learnings.-` when `init` points an install at another team repository of the same kind, which on `main` published them to the new one; the same repository written another way keeps them, [#823](https://github.com/Tencent/teamai-cli/issues/823) item 13), and `uninstall` lists every unpublished queue before it asks. (for [#808](https://github.com/Tencent/teamai-cli/issues/808)). - 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)). diff --git a/docs/designs/data-directory-layout.md b/docs/designs/data-directory-layout.md index dffd545ea..b21050c9d 100644 --- a/docs/designs/data-directory-layout.md +++ b/docs/designs/data-directory-layout.md @@ -813,11 +813,16 @@ creation. A hook append waits up to ~250 ms, inside its foreground budget, and one that gives up records its line in an `events.pending-.jsonl` side file, with the file's mode, that the next lock holder folds into the file before it writes — so an event is -late, never gone. The line carries a `pendingId`, so a fold never appends a -side file twice (a holder that died after appending it but before removing it -leaves it for the next one), two side files of identical events are both kept, -and no reader and no compacted file keeps the id: `readEventsRaw` drops it, and -a compaction rewrites the line without it. A side file without its trailing +late, never gone. Side files fold in their events' own time order, not +readdir's, and the compaction classifies sessions in time order too (the +order every reader rebuilds from), so a side file that outlived newer appends +cannot place an older event after them and re-mark a live session stopped. +The line carries a `pendingId`, so a fold never appends a side file twice (a +holder that died after appending it but before removing it leaves it for the +next one), two side files of identical events are both kept, and the id stays +in the raw file — a rewrite keeps it, as the usage file's does — until the +side file itself is gone, while no reader ever sees it: `readEvents` drops it. +A side file without its trailing newline is still being written and waits for the next holder. A compaction waits up to ~5 s for a peer's rewrite, and skips — leaving the file as it is for the next compaction — when a live holder outlasts the wait; a lock whose diff --git a/src/__tests__/dashboard-collector.test.ts b/src/__tests__/dashboard-collector.test.ts index f87f29a09..e9ec26507 100644 --- a/src/__tests__/dashboard-collector.test.ts +++ b/src/__tests__/dashboard-collector.test.ts @@ -2237,7 +2237,7 @@ describe('events file lock (#804)', () => { expect(await pendingFiles()).toEqual([]); }); - it('folds a side file into the compaction and keeps its id out of the file', async () => { + it('keeps a folded side file recognizable through a compaction, and its id out of readers', async () => { await seedActive(); await writeLock(process.pid); await appendEvent(event('live-b', { monitorPid: process.pid })); @@ -2246,12 +2246,65 @@ describe('events file lock (#804)', () => { await compactEvents(eventsPath()); + // The rewrite keeps the folded line's id, so a side file that survived its + // fold (its rm failed) is still recognized and never appended twice. const content = fs.readFileSync(eventsPath(), 'utf-8'); expect(content).toContain('live-b'); - expect(content).not.toContain('pendingId'); + expect(content).toContain('pendingId'); + const events = await readEvents(eventsPath()); + expect(events.some((e) => 'pendingId' in e)).toBe(false); + expect(await pendingFiles()).toEqual([]); + }); + + it('does not append a surviving side file twice across a compaction', async () => { + await seedActive(); + await appendEvent(event('a', { monitorPid: process.pid })); + await writeLock(process.pid); + await appendEvent(event('b', { monitorPid: process.pid })); + await fs.promises.rm(`${eventsPath()}.lock`); + // The fold appends b but cannot remove its side file; a compaction runs + // while it survives. + 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 appendEvent(event('c', { monitorPid: process.pid })); + await compactEvents(eventsPath()); + } finally { + spy.mockRestore(); + } + expect(await pendingFiles()).toHaveLength(1); + + await appendEvent(event('d', { monitorPid: process.pid })); + + expect(await ids()).toEqual([...Array.from({ length: 5_000 }, (_, i) => `filler-${i}`), 'a', 'b', 'c', 'd']); expect(await pendingFiles()).toEqual([]); }); + it('classifies sessions in time order, so a late older event cannot stop a live one', async () => { + await seedActive(); + // A session whose newer prompt sits before an older end in the raw file — + // the placement a late-folding side file can produce. Classified in raw + // file order the session reads as long stopped and the rewrite would drop + // its events; in time order it is still running. + const now = new Date().toISOString(); + const old = new Date(Date.now() - 3_600_000).toISOString(); + const line = (o: Record) => JSON.stringify(o); + await fs.promises.appendFile(eventsPath(), [ + line({ type: 'prompt_submit', timestamp: now, sessionId: 'x', tool: 'claude' }), + line({ type: 'session_end', timestamp: old, sessionId: 'x', tool: 'claude' }), + ].join('\n') + '\n', 'utf-8'); + + await compactEvents(eventsPath()); + + const kept = fs.readFileSync(eventsPath(), 'utf-8').split('\n').filter(Boolean) + .map((l) => JSON.parse(l) as { sessionId: string }) + .filter((e) => e.sessionId === 'x'); + expect(kept).toHaveLength(2); + }); + it('folds a side file even when the file is below the compaction threshold', async () => { await appendEvent(event('a')); await writeLock(process.pid); diff --git a/src/dashboard-collector.ts b/src/dashboard-collector.ts index a8baef781..d9bd45e55 100644 --- a/src/dashboard-collector.ts +++ b/src/dashboard-collector.ts @@ -1558,23 +1558,35 @@ function eventsPendingPrefix(eventsPath: string): string { /** * Append the side files of appends that gave up on the lock to the events - * 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. + * file, then remove them — in the events' own time order, not readdir's + * arbitrary order, so a side file that outlived several newer appends does + * not land after them in the file. Each side file 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(eventsPath: string): Promise { const dir = path.dirname(eventsPath); const prefix = eventsPendingPrefix(eventsPath); const names = await fs.promises.readdir(dir).catch(() => []); - let folded: Set | undefined; + const pending: Array<{ pendingPath: string; content: string; timestamp: number }> = []; 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 timestamp = Date.parse(pendingTimestampOf(content)); + pending.push({ pendingPath, content, timestamp: Number.isNaN(timestamp) ? 0 : timestamp }); + } catch (e) { + log.debug(`dashboard: could not fold ${pendingPath} into ${eventsPath}: ${e instanceof Error ? e.message : String(e)}`); + } + } + pending.sort((a, b) => a.timestamp - b.timestamp); + let folded: Set | undefined; + for (const { pendingPath, content } of pending) { + try { const id = pendingIdOf(content); folded ??= new Set( (await fs.promises.readFile(eventsPath, 'utf-8').catch(() => '')).split('\n').map(pendingIdOf).filter((i) => i !== undefined), @@ -1590,6 +1602,16 @@ async function foldPendingEvents(eventsPath: string): Promise { } } +/** The timestamp of the event a side file holds, for folding side files in time order. */ +function pendingTimestampOf(content: string): string { + try { + const { timestamp } = JSON.parse(content) as { timestamp?: unknown }; + return typeof timestamp === 'string' ? timestamp : ''; + } catch { + return ''; + } +} + /** 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; @@ -1650,7 +1672,14 @@ export async function appendEvent(event: DashboardEvent): Promise { /** * Read raw events from the JSONL file, in file (append) order. Skips corrupted * lines. Callers that must preserve the on-disk stream verbatim (e.g. compaction) - * use this; everything else goes through {@link readEvents}, which also dedupes. + * use this; everything else goes through {@link readEvents}, which also dedupes + * and drops a folded side file's `pendingId`. + * + * A folded side file's id stays in the file (and in every rewrite of it, + * compaction included) until the side file itself is gone — a fold that could + * not remove its side file must stay recognizable, or the next holder would + * append the event a second time. The usage file's rewrite keeps the id the + * same way (#788). */ async function readEventsRaw(filePath: string): Promise { try { @@ -1660,11 +1689,8 @@ async function readEventsRaw(filePath: string): Promise { const trimmed = line.trim(); if (!trimmed) continue; try { - const parsed = JSON.parse(trimmed) as DashboardEvent & { pendingId?: unknown }; + const parsed = JSON.parse(trimmed) as DashboardEvent; if (parsed.type && parsed.sessionId && parsed.timestamp) { - // A folded side file's id stays in the file (foldPendingEvents) until - // a compaction rewrites the line; no reader ever sees it. - delete parsed.pendingId; events.push(parsed); } } catch { @@ -1743,10 +1769,16 @@ export function dedupeEvents(events: DashboardEvent[]): DashboardEvent[] { /** * Read all events from the JSONL file, cross-tool-deduped. Skips corrupted lines. + * A folded side file's `pendingId` is machine-local bookkeeping (foldPendingEvents + * keeps it in the raw file so a surviving side file is never appended twice) and + * never reaches a reader. */ export async function readEvents(eventsPath?: string): Promise { const filePath = eventsPath ?? getEventsPath(); - return dedupeEvents(await readEventsRaw(filePath)); + return dedupeEvents(await readEventsRaw(filePath)).map((event) => { + delete (event as DashboardEvent & { pendingId?: unknown }).pendingId; + return event; + }); } // ─── Session state rebuild ────────────────────────────── @@ -2187,10 +2219,17 @@ export async function compactEvents(eventsPath?: string): Promise { // un-deduped) events — dedup is a read-time view, not a disk mutation. // Dedup never changes the active-session set, so activeIds is identical. const events = await readEventsRaw(target); - const activeSessions = rebuildSessions(events); + // Classify sessions in time order, not raw file order: a side file that + // outlived newer appends can sit after them in the file even though it + // folds in time order, and every reader rebuilds sessions from + // time-sorted events (dedupeEvents sorts). Classifying from raw order + // could re-mark a live session stopped from a late older event past its + // stopped-display window, and the rewrite would drop its events. + const chronological = [...events].sort((a, b) => Date.parse(a.timestamp) - Date.parse(b.timestamp)); + const activeSessions = rebuildSessions(chronological); const activeIds = new Set(activeSessions.map(s => s.sessionId)); const monitored = new Map(); - for (const e of events) { + for (const e of chronological) { if (e.type === 'session_start' && typeof e.monitorPid === 'number') monitored.set(e.sessionId, e.monitorPid); } for (const [sessionId, pid] of monitored) {