diff --git a/README.md b/README.md index 5c8ccbbd..5f090fc5 100644 --- a/README.md +++ b/README.md @@ -319,6 +319,10 @@ This plugin captures session activity and stores it in your Deeplake workspace: | `HIVEMIND_CAPTURE` | `true` | Set to `false` to disable capture | | `HIVEMIND_CAPTURE_ONLY_CLI` | _(none)_ | Set to `true` to capture only interactive CLI sessions. Sessions spawned by the Claude Agent SDK (Python/TypeScript) are skipped; their `CLAUDE_CODE_ENTRYPOINT` is `sdk-py` / `sdk-ts`, so they fail the substring check for `cli`. | | `HIVEMIND_SKILLIFY_EVERY_N_TURNS` | `20` | Assistant turns between auto skill-mining attempts. Lower = more frequent mining (cheaper sessions, noisier output); higher = fewer attempts on longer histories. | +| `HIVEMIND_SUMMARY_EVERY_N_MSGS` | `50` | Captured events between periodic session summaries. The first summary of a session runs at 10 events regardless. Raise it to cut background summary runs. | +| `HIVEMIND_SUMMARY_EVERY_HOURS` | `2` | Time-based summary cadence, used when at least one new event has arrived since the last summary. | +| `HIVEMIND_WIKI_WORKER` | _(none)_ | Set to `1` to disable the background session-summary worker entirely (no `claude -p` summary runs). Also set automatically inside the worker as a recursion guard. Capture and recall keep working. | +| `HIVEMIND_GRAPH_ON_STOP` | _(none)_ | Set to `0` to disable the code-graph rebuild that runs on `Stop` / `SessionEnd`. | | `HIVEMIND_EMBEDDINGS` | `true` | Set to `false` to force lexical-only mode | | `HIVEMIND_PROACTIVE_RECALL_DISABLED` | _(none)_ | Set to `1` to disable **proactive recall** (auto-searching team memory on each recall-worthy prompt and injecting a relevant snippet into the agent's context). On by default. Does **not** affect capture or the agent's own grep/skill recall. Alt form: `HIVEMIND_PROACTIVE_RECALL=0`. | | `HIVEMIND_RECALL_MIN_OVERLAP` | `2` | Proactive recall (lexical mode): min distinct prompt keywords a summary must share to be injected. Higher = stricter. | diff --git a/harnesses/pi/extension-source/hivemind.ts b/harnesses/pi/extension-source/hivemind.ts index e1e6dd31..ce3563a1 100644 --- a/harnesses/pi/extension-source/hivemind.ts +++ b/harnesses/pi/extension-source/hivemind.ts @@ -652,6 +652,13 @@ interface SummaryState { lastSummaryAt: number; lastSummaryCount: number; totalCount: number; + // Mirror of src/hooks/summary-state.ts. A run that fails never calls + // finalizeSummary, so without these the first-summary clause below stays + // true forever and refires on every captured event (issue #331). They must + // also survive a read/write round-trip here, because this file rewrites the + // whole state object that the shared worker finalizes against. + lastAttemptAt?: number; + attemptsSinceSuccess?: number; } interface SummaryConfig { everyNMessages: number; @@ -688,15 +695,28 @@ function readSummaryState(sessionId: string): SummaryState | null { lastSummaryAt: Number(raw.lastSummaryAt) || 0, lastSummaryCount: Number(raw.lastSummaryCount) || 0, totalCount: Number(raw.totalCount) || 0, + lastAttemptAt: Number(raw.lastAttemptAt) || 0, + attemptsSinceSuccess: Number(raw.attemptsSinceSuccess) || 0, }; } catch { return null; } } -function writeSummaryState(sessionId: string, state: SummaryState): void { +/** + * Returns false when the state could not be persisted. Most callers treat a + * failed write as non-fatal, but the #331 attempt stamp must fail closed: an + * unpersisted stamp means the next captured event refires immediately, which + * is the bug the back-off exists to prevent. + */ +function writeSummaryState(sessionId: string, state: SummaryState): boolean { try { mkdirSync(SUMMARY_STATE_DIR, { recursive: true }); writeFileSync(summaryStatePath(sessionId), JSON.stringify(state)); - } catch { /* non-fatal */ } + return true; + } catch { return false; } +} + +function releaseSummaryLock(sessionId: string): void { + try { unlinkSync(summaryLockPath(sessionId)); } catch { /* already gone */ } } function bumpCounter(sessionId: string): SummaryState { @@ -706,8 +726,18 @@ function bumpCounter(sessionId: string): SummaryState { return cur; } -function shouldTriggerNow(state: SummaryState, cfg: SummaryConfig): boolean { +/** Mirror of retryBackoffMs in src/hooks/summary-state.ts: 1..30 minutes. */ +function retryBackoffMs(attemptsSinceSuccess: number): number { + if (attemptsSinceSuccess <= 0) return 0; + return Math.min(2 ** (attemptsSinceSuccess - 1), 30) * 60 * 1000; +} + +function shouldTriggerNow(state: SummaryState, cfg: SummaryConfig, now = Date.now()): boolean { const msgsSince = state.totalCount - state.lastSummaryCount; + // A run started but never finalized — it failed. Hold off instead of + // respawning the worker on the next captured event (issue #331). + const attempts = state.attemptsSinceSuccess ?? 0; + if (attempts > 0 && now - (state.lastAttemptAt ?? 0) < retryBackoffMs(attempts)) return false; // First-chat trigger: index a fresh session quickly (10 events) instead of // waiting until N=50. Mirrors summary-state.ts in CC/codex. if (state.lastSummaryCount === 0 && state.totalCount >= FIRST_SUMMARY_AT) return true; @@ -796,6 +826,15 @@ function spawnWikiWorker( cwd: string, reason: "periodic" | "final", ): void { + // Documented off switch for background summaries (README Configuration): + // it must cover the FINAL session-shutdown summary too, not just the + // periodic one, or "disables the background session-summary worker + // entirely" is false. Doubles as the recursion guard the worker sets on + // its own children. + if (process.env.HIVEMIND_WIKI_WORKER === "1") { + logHm(`spawnWikiWorker(${reason}): HIVEMIND_WIKI_WORKER=1 — background summaries disabled`); + return; + } if (!existsSync(PI_WIKI_WORKER_PATH)) { logHm(`spawnWikiWorker(${reason}): no worker at ${PI_WIKI_WORKER_PATH} — install via 'hivemind pi install' or rebuild`); return; @@ -807,6 +846,25 @@ function spawnWikiWorker( logHm(`spawnWikiWorker(${reason}): lock held — skipping (a worker is already running)`); return; } + // Stamp the attempt AFTER winning the lock, against freshly-read state + // (issue #331). Stamping in the caller — before the lock, using the state + // object read at trigger time — could overwrite a concurrent worker's + // successful finalization with stale counters, and would record a + // lock-suppressed non-spawn as a failed attempt. Periodic only: a final + // summary at session shutdown gets no back-off, matching summary-state.ts. + if (reason === "periodic") { + const fresh = readSummaryState(sessionId); + const stamped = fresh !== null && writeSummaryState(sessionId, { + ...fresh, + lastAttemptAt: Date.now(), + attemptsSinceSuccess: (fresh.attemptsSinceSuccess ?? 0) + 1, + }); + if (!stamped) { + logHm(`spawnWikiWorker(${reason}): could not persist the attempt stamp — releasing the lock and skipping (a spawn we cannot record would refire on every event)`); + releaseSummaryLock(sessionId); + return; + } + } // tmp dir owned by the worker; it removes it on completion. const tmpDir = join(tmpdir(), `deeplake-wiki-${sessionId}-${Date.now()}`); try { mkdirSync(tmpDir, { recursive: true }); } catch { /* ignore */ } @@ -832,16 +890,32 @@ function spawnWikiWorker( promptTemplate: WIKI_PROMPT_TEMPLATE, }; try { writeFileSync(configPath, JSON.stringify(config)); } - catch (e: any) { logHm(`spawnWikiWorker(${reason}): writeFileSync failed: ${e?.message ?? e}`); return; } + catch (e: any) { + // Release: pi's tryAcquireSummaryLock has no stale reclaim, so a lock + // left behind here is held for the rest of the session and every later + // periodic AND final spawn is skipped. + logHm(`spawnWikiWorker(${reason}): writeFileSync failed: ${e?.message ?? e}`); + releaseSummaryLock(sessionId); + return; + } logHm(`spawnWikiWorker(${reason}): spawning ${PI_WIKI_WORKER_PATH} session=${sessionId} provider=${config.piProvider} model=${config.piModel}`); try { - spawn(process.execPath, [PI_WIKI_WORKER_PATH, configPath], { + const child = spawn(process.execPath, [PI_WIKI_WORKER_PATH, configPath], { detached: true, stdio: "ignore", env: { ...process.env, HIVEMIND_WIKI_WORKER: "1", HIVEMIND_CAPTURE: "false" }, - }).unref(); + }); + // ENOENT / EPERM arrive as an ASYNC 'error' event, never a sync throw — + // without this listener the worker never runs AND the lock is never + // released, so the session stops summarizing silently. + child.on("error", (err: any) => { + logHm(`spawnWikiWorker(${reason}): spawn errored: ${err?.message ?? err}`); + releaseSummaryLock(sessionId); + }); + child.unref(); } catch (e: any) { logHm(`spawnWikiWorker(${reason}): spawn failed: ${e?.message ?? e}`); + releaseSummaryLock(sessionId); } } diff --git a/src/hooks/capture.ts b/src/hooks/capture.ts index c0406e6e..06359f7c 100644 --- a/src/hooks/capture.ts +++ b/src/hooks/capture.ts @@ -21,6 +21,7 @@ import { loadTriggerConfig, shouldTrigger, tryAcquireLock, + markSummaryAttempt, releaseLock, ensureSessionOwner, } from "./summary-state.js"; @@ -273,6 +274,12 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con wikiLog(`Periodic: threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); try { + // Stamp the attempt BEFORE spawning: a run that fails never reaches + // finalizeSummary, and without this the trigger would refire on the + // very next captured event (issue #331). Inside the try so a failed + // state write releases the lock instead of leaking it until the + // 10-minute stale reclaim. + markSummaryAttempt(sessionId); spawnWikiWorker({ config, sessionId, diff --git a/src/hooks/codex/capture.ts b/src/hooks/codex/capture.ts index cae7bbc0..577b6aa8 100644 --- a/src/hooks/codex/capture.ts +++ b/src/hooks/codex/capture.ts @@ -33,6 +33,7 @@ import { loadTriggerConfig, shouldTrigger, tryAcquireLock, + markSummaryAttempt, releaseLock, } from "../summary-state.js"; import { bundleDirFromImportMeta, spawnCodexWikiWorker, wikiLog } from "./spawn-wiki-worker.js"; @@ -200,6 +201,12 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con wikiLog(`Periodic: threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); try { + // Stamp the attempt BEFORE spawning: a run that fails never reaches + // finalizeSummary, and without this the trigger would refire on the + // very next captured event (issue #331). Inside the try so a failed + // state write releases the lock instead of leaking it until the + // 10-minute stale reclaim. + markSummaryAttempt(sessionId); spawnCodexWikiWorker({ config, sessionId, diff --git a/src/hooks/cursor/capture.ts b/src/hooks/cursor/capture.ts index e3214aa3..893bdd2b 100644 --- a/src/hooks/cursor/capture.ts +++ b/src/hooks/cursor/capture.ts @@ -32,6 +32,7 @@ import { loadTriggerConfig, shouldTrigger, tryAcquireLock, + markSummaryAttempt, releaseLock, } from "../summary-state.js"; import { bundleDirFromImportMeta, spawnCursorWikiWorker, wikiLog } from "./spawn-wiki-worker.js"; @@ -231,6 +232,12 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con } wikiLog(`Periodic: threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); try { + // Stamp the attempt BEFORE spawning: a run that fails never reaches + // finalizeSummary, and without this the trigger would refire on the + // very next captured event (issue #331). Inside the try so a failed + // state write releases the lock instead of leaking it until the + // 10-minute stale reclaim. + markSummaryAttempt(sessionId); spawnCursorWikiWorker({ config, sessionId, diff --git a/src/hooks/hermes/capture.ts b/src/hooks/hermes/capture.ts index 4e77d7ed..2700b6c7 100644 --- a/src/hooks/hermes/capture.ts +++ b/src/hooks/hermes/capture.ts @@ -33,6 +33,7 @@ import { loadTriggerConfig, shouldTrigger, tryAcquireLock, + markSummaryAttempt, releaseLock, } from "../summary-state.js"; import { bundleDirFromImportMeta, spawnHermesWikiWorker, wikiLog } from "./spawn-wiki-worker.js"; @@ -226,6 +227,12 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con } wikiLog(`Periodic: threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); try { + // Stamp the attempt BEFORE spawning: a run that fails never reaches + // finalizeSummary, and without this the trigger would refire on the + // very next captured event (issue #331). Inside the try so a failed + // state write releases the lock instead of leaking it until the + // 10-minute stale reclaim. + markSummaryAttempt(sessionId); spawnHermesWikiWorker({ config, sessionId, diff --git a/src/hooks/summary-state.ts b/src/hooks/summary-state.ts index 95c2974f..3576fa25 100644 --- a/src/hooks/summary-state.ts +++ b/src/hooks/summary-state.ts @@ -23,6 +23,16 @@ export interface SummaryState { lastSummaryAt: number; lastSummaryCount: number; totalCount: number; + /** + * Epoch ms of the last summary run we *started*, successful or not. + * Distinct from lastSummaryAt, which only advances on success. + */ + lastAttemptAt?: number; + /** + * Summary runs started since the last successful one. Drives the retry + * back-off in shouldTrigger; reset to 0 by finalizeSummary. + */ + attemptsSinceSuccess?: number; } const STATE_DIR = join(homedir(), ".claude", "hooks", "summary-state"); @@ -291,10 +301,48 @@ export function finalizeSummary(sessionId: string, jsonlLines: number): void { lastSummaryAt: Date.now(), lastSummaryCount: jsonlLines, totalCount: Math.max(prev?.totalCount ?? 0, jsonlLines), + lastAttemptAt: prev?.lastAttemptAt, + attemptsSinceSuccess: 0, }); }); } +/** + * Record that a summary run is being STARTED. Called by the periodic trigger + * immediately after it wins the spawn lock, before the worker is spawned. + * + * Without this, a summary run that fails (claude bin unresolvable, non-zero + * exit, no summary file produced) never advances any counter: the worker + * returns early without calling finalizeSummary and releases the lock in its + * `finally`, so `lastSummaryCount` stays 0 and shouldTrigger's + * first-summary clause keeps firing on EVERY captured event. The observed + * result is one full `claude -p` process per tool call for the rest of the + * session (issue #331). + */ +export function markSummaryAttempt(sessionId: string): void { + withRmwLock(sessionId, () => { + const prev = readState(sessionId); + writeState(sessionId, { + lastSummaryAt: prev?.lastSummaryAt ?? Date.now(), + lastSummaryCount: prev?.lastSummaryCount ?? 0, + totalCount: prev?.totalCount ?? 0, + lastAttemptAt: Date.now(), + attemptsSinceSuccess: (prev?.attemptsSinceSuccess ?? 0) + 1, + }); + }); +} + +/** + * Exponential back-off after failed summary runs: 1, 2, 4, 8, 16 minutes, + * capped at 30. A run that succeeds resets the counter via finalizeSummary, + * so a healthy session never waits. + */ +export function retryBackoffMs(attemptsSinceSuccess: number): number { + if (attemptsSinceSuccess <= 0) return 0; + const minutes = Math.min(2 ** (attemptsSinceSuccess - 1), 30); + return minutes * 60 * 1000; +} + export interface TriggerConfig { everyNMessages: number; everyHours: number; @@ -313,6 +361,11 @@ const FIRST_SUMMARY_AT = 10; export function shouldTrigger(state: SummaryState, cfg: TriggerConfig, now = Date.now()): boolean { const msgsSince = state.totalCount - state.lastSummaryCount; + // A previous run started but never finalized — it failed. Hold off for the + // back-off window instead of respawning a full `claude -p` on the next + // captured event (issue #331). markSummaryAttempt stamps lastAttemptAt. + const attempts = state.attemptsSinceSuccess ?? 0; + if (attempts > 0 && now - (state.lastAttemptAt ?? 0) < retryBackoffMs(attempts)) return false; if (state.lastSummaryCount === 0 && state.totalCount >= FIRST_SUMMARY_AT) return true; if (msgsSince >= cfg.everyNMessages) return true; if (msgsSince > 0 && now - state.lastSummaryAt >= cfg.everyHours * 3600 * 1000) return true; diff --git a/tests/claude-code/capture-hook.test.ts b/tests/claude-code/capture-hook.test.ts index 3b699511..edce62a2 100644 --- a/tests/claude-code/capture-hook.test.ts +++ b/tests/claude-code/capture-hook.test.ts @@ -23,6 +23,7 @@ const spawnMock = vi.fn(); const wikiLogMock = vi.fn(); const tryAcquireLockMock = vi.fn(); const releaseLockMock = vi.fn(); +const markSummaryAttemptMock = vi.fn(); const bumpTotalCountMock = vi.fn(); const loadTriggerConfigMock = vi.fn(); const shouldTriggerMock = vi.fn(); @@ -41,6 +42,7 @@ vi.mock("../../src/hooks/spawn-wiki-worker.js", () => ({ bundleDirFromImportMeta: () => "/fake/bundle", })); vi.mock("../../src/hooks/summary-state.js", () => ({ + markSummaryAttempt: (...a: any[]) => markSummaryAttemptMock(...a), tryAcquireLock: (...a: any[]) => tryAcquireLockMock(...a), releaseLock: (...a: any[]) => releaseLockMock(...a), bumpTotalCount: (...a: any[]) => bumpTotalCountMock(...a), @@ -99,6 +101,7 @@ beforeEach(() => { wikiLogMock.mockReset(); tryAcquireLockMock.mockReset().mockReturnValue(true); releaseLockMock.mockReset(); + markSummaryAttemptMock.mockReset(); bumpTotalCountMock.mockReset().mockReturnValue({ lastSummaryAt: Date.now(), lastSummaryCount: 0, totalCount: 1, }); @@ -355,6 +358,10 @@ describe("capture hook — periodic trigger helper", () => { ); expect(spawnMock).toHaveBeenCalledTimes(1); expect(spawnMock.mock.calls[0][0]).toMatchObject({ sessionId: "sid-1", reason: "Periodic" }); + // The attempt must be stamped BEFORE the spawn, otherwise a failing + // worker leaves lastSummaryCount at 0 and the trigger refires on every + // subsequent captured event (issue #331). + expect(markSummaryAttemptMock).toHaveBeenCalledWith("sid-1"); }); it("logs 'periodic trigger suppressed' when the lock is already held", async () => { @@ -367,6 +374,17 @@ describe("capture hook — periodic trigger helper", () => { ); }); + it("releases the lock if markSummaryAttempt throws", async () => { + // The stamp writes to the state file. If that write fails it must not + // leak the spawn lock — otherwise the session stops summarizing until + // the 10-minute stale reclaim. + shouldTriggerMock.mockReturnValue(true); + markSummaryAttemptMock.mockImplementation(() => { throw new Error("state write failed"); }); + await runHook(); + expect(spawnMock).not.toHaveBeenCalled(); + expect(releaseLockMock).toHaveBeenCalledWith("sid-1"); + }); + it("releases the lock if spawnWikiWorker throws", async () => { shouldTriggerMock.mockReturnValue(true); spawnMock.mockImplementation(() => { throw new Error("spawn failed"); }); diff --git a/tests/claude-code/summary-state.test.ts b/tests/claude-code/summary-state.test.ts index 9f316f18..3f00ce78 100644 --- a/tests/claude-code/summary-state.test.ts +++ b/tests/claude-code/summary-state.test.ts @@ -643,3 +643,87 @@ describe("cross-process concurrency", () => { mod.releaseLock(sid); }, 30_000); }); + +/** + * Regression cover for issue #331: a summary run that FAILS never reaches + * finalizeSummary, so lastSummaryCount stays 0. Before the back-off, that made + * shouldTrigger's first-summary clause true on every subsequent captured + * event — one full `claude -p` process per tool call for the rest of the + * session (the "flashing console window every 5-6s" on Windows). + */ +describe("failed-run back-off (issue #331)", () => { + const cfg = { everyNMessages: 50, everyHours: 2 }; + + it("retryBackoffMs doubles per failure and caps at 30 minutes", () => { + expect(mod.retryBackoffMs(0)).toBe(0); + expect(mod.retryBackoffMs(1)).toBe(60_000); + expect(mod.retryBackoffMs(2)).toBe(2 * 60_000); + expect(mod.retryBackoffMs(3)).toBe(4 * 60_000); + expect(mod.retryBackoffMs(5)).toBe(16 * 60_000); + expect(mod.retryBackoffMs(6)).toBe(30 * 60_000); + expect(mod.retryBackoffMs(99)).toBe(30 * 60_000); + }); + + it("suppresses the retry while the back-off window is open", () => { + const now = Date.now(); + expect(mod.shouldTrigger( + { lastSummaryAt: now, lastSummaryCount: 0, totalCount: 11, lastAttemptAt: now - 30_000, attemptsSinceSuccess: 1 }, + cfg, now, + )).toBe(false); + }); + + it("allows the retry once the back-off window has elapsed", () => { + const now = Date.now(); + expect(mod.shouldTrigger( + { lastSummaryAt: now, lastSummaryCount: 0, totalCount: 11, lastAttemptAt: now - 61_000, attemptsSinceSuccess: 1 }, + cfg, now, + )).toBe(true); + }); + + it("does not change behavior for a session with no failed attempts", () => { + const now = Date.now(); + expect(mod.shouldTrigger( + { lastSummaryAt: now, lastSummaryCount: 0, totalCount: 10, lastAttemptAt: now - 1000, attemptsSinceSuccess: 0 }, + cfg, now, + )).toBe(true); + }); + + it("collapses a 30-event failing session from 21 spawns down to 1", () => { + // Simulate the real loop: every event bumps the counter, and every run we + // trigger fails (never calls finalizeSummary, only markSummaryAttempt). + const sid = newSessionId(); + let spawns = 0; + for (let i = 0; i < 30; i++) { + const state = mod.bumpTotalCount(sid); + if (!mod.shouldTrigger(state, cfg)) continue; + spawns++; + mod.markSummaryAttempt(sid); + } + expect(spawns).toBe(1); + }); + + it("markSummaryAttempt stamps the attempt without advancing lastSummaryCount", () => { + const sid = newSessionId(); + for (let i = 0; i < 12; i++) mod.bumpTotalCount(sid); + const before = Date.now(); + mod.markSummaryAttempt(sid); + const s = JSON.parse(readFileSync(mod.statePath(sid), "utf-8")); + expect(s.attemptsSinceSuccess).toBe(1); + expect(s.lastAttemptAt).toBeGreaterThanOrEqual(before); + expect(s.lastSummaryCount).toBe(0); + expect(s.totalCount).toBe(12); + mod.markSummaryAttempt(sid); + expect(JSON.parse(readFileSync(mod.statePath(sid), "utf-8")).attemptsSinceSuccess).toBe(2); + }); + + it("finalizeSummary clears the failure counter so the next run is not delayed", () => { + const sid = newSessionId(); + for (let i = 0; i < 12; i++) mod.bumpTotalCount(sid); + mod.markSummaryAttempt(sid); + mod.markSummaryAttempt(sid); + mod.finalizeSummary(sid, 12); + const s = JSON.parse(readFileSync(mod.statePath(sid), "utf-8")); + expect(s.attemptsSinceSuccess).toBe(0); + expect(mod.shouldTrigger(s, cfg, Date.now() + 3 * 3600 * 1000)).toBe(false); + }); +}); diff --git a/tests/codex/codex-capture-hook.test.ts b/tests/codex/codex-capture-hook.test.ts index 36dd993e..4380224e 100644 --- a/tests/codex/codex-capture-hook.test.ts +++ b/tests/codex/codex-capture-hook.test.ts @@ -14,6 +14,7 @@ const spawnMock = vi.fn(); const wikiLogMock = vi.fn(); const tryAcquireLockMock = vi.fn(); const releaseLockMock = vi.fn(); +const markSummaryAttemptMock = vi.fn(); const bumpTotalCountMock = vi.fn(); const loadTriggerConfigMock = vi.fn(); const shouldTriggerMock = vi.fn(); @@ -29,6 +30,7 @@ vi.mock("../../src/hooks/codex/spawn-wiki-worker.js", () => ({ bundleDirFromImportMeta: () => "/fake/codex/bundle", })); vi.mock("../../src/hooks/summary-state.js", () => ({ + markSummaryAttempt: (...a: any[]) => markSummaryAttemptMock(...a), tryAcquireLock: (...a: any[]) => tryAcquireLockMock(...a), releaseLock: (...a: any[]) => releaseLockMock(...a), bumpTotalCount: (...a: any[]) => bumpTotalCountMock(...a), @@ -83,6 +85,7 @@ beforeEach(() => { wikiLogMock.mockReset(); tryAcquireLockMock.mockReset().mockReturnValue(true); releaseLockMock.mockReset(); + markSummaryAttemptMock.mockReset(); bumpTotalCountMock.mockReset().mockReturnValue({ lastSummaryAt: 0, lastSummaryCount: 0, totalCount: 1, }); @@ -209,6 +212,10 @@ describe("codex capture hook — periodic trigger", () => { await runHook(); expect(spawnMock).toHaveBeenCalledTimes(1); expect(spawnMock.mock.calls[0][0]).toMatchObject({ sessionId: "sid-1", reason: "Periodic" }); + // The attempt must be stamped BEFORE the spawn, otherwise a failing + // worker leaves lastSummaryCount at 0 and the trigger refires on every + // subsequent captured event (issue #331). + expect(markSummaryAttemptMock).toHaveBeenCalledWith("sid-1"); }); it("suppresses when lock held", async () => { diff --git a/tests/pi/pi-extension-source.test.ts b/tests/pi/pi-extension-source.test.ts index e1fe3749..80d33f9e 100644 --- a/tests/pi/pi-extension-source.test.ts +++ b/tests/pi/pi-extension-source.test.ts @@ -328,3 +328,102 @@ describe("pi extension — per-directory .hivemind wiring", () => { expect(PI_SRC).toContain("capture is disabled for this directory"); }); }); + +describe("pi extension — failed-summary back-off (issue #331)", () => { + it("preserves the attempt fields across a state read/write round-trip", () => { + // pi rewrites the WHOLE state object into the same dir the shared + // wiki-worker finalizes against. Dropping these on read would strip them + // from the file and restore the infinite-refire behavior. + expect(PI_SRC).toContain("lastAttemptAt: Number(raw.lastAttemptAt) || 0"); + expect(PI_SRC).toContain("attemptsSinceSuccess: Number(raw.attemptsSinceSuccess) || 0"); + }); + + it("suppresses the trigger while a failed run's back-off window is open", () => { + expect(PI_SRC).toMatch( + /if \(attempts > 0 && now - \(state\.lastAttemptAt \?\? 0\) < retryBackoffMs\(attempts\)\) return false;/, + ); + }); + + it("stamps the attempt after winning the lock, not before", () => { + // Stamping in the caller (before tryAcquireSummaryLock) would overwrite a + // concurrent worker's finalization with stale counters and would record a + // lock-suppressed non-spawn as a failure. + const trigger = PI_SRC.slice( + PI_SRC.indexOf("function maybeTriggerPeriodicSummary"), + ); + expect(trigger).not.toContain("attemptsSinceSuccess"); + + const spawn = PI_SRC.slice(PI_SRC.indexOf("function spawnWikiWorker")); + const lockAt = spawn.indexOf("tryAcquireSummaryLock(sessionId)"); + const stampAt = spawn.indexOf("attemptsSinceSuccess: (fresh.attemptsSinceSuccess ?? 0) + 1"); + expect(lockAt).toBeGreaterThan(-1); + expect(stampAt).toBeGreaterThan(lockAt); + }); + + it("stamps against freshly-read state, not the trigger-time snapshot", () => { + const spawn = PI_SRC.slice(PI_SRC.indexOf("function spawnWikiWorker")); + expect(spawn).toContain("const fresh = readSummaryState(sessionId)"); + expect(spawn).toContain("...fresh,"); + }); + + it("does not back off the final session-shutdown summary", () => { + const spawn = PI_SRC.slice(PI_SRC.indexOf("function spawnWikiWorker")); + expect(spawn).toContain('if (reason === "periodic")'); + }); + + it("caps the back-off at 30 minutes, matching summary-state.ts", () => { + expect(PI_SRC).toContain("Math.min(2 ** (attemptsSinceSuccess - 1), 30) * 60 * 1000"); + }); +}); + +describe("pi extension — #331 review follow-ups", () => { + it("fails closed when the attempt stamp cannot be persisted", () => { + // A spawn we cannot record is exactly the bug: the next captured event + // refires immediately. Release the lock and skip instead. + const spawn = PI_SRC.slice(PI_SRC.indexOf("function spawnWikiWorker")); + expect(spawn).toContain("if (!stamped) {"); + const stampedAt = spawn.indexOf("if (!stamped) {"); + const releaseAt = spawn.indexOf("releaseSummaryLock(sessionId)", stampedAt); + const returnAt = spawn.indexOf("return;", releaseAt); + expect(releaseAt).toBeGreaterThan(stampedAt); + expect(returnAt).toBeGreaterThan(releaseAt); + }); + + it("writeSummaryState reports failure rather than swallowing it", () => { + expect(PI_SRC).toMatch(/function writeSummaryState\([^)]*\): boolean \{/); + expect(PI_SRC).toContain("} catch { return false; }"); + }); + + it("honors HIVEMIND_WIKI_WORKER=1 for the final summary too, not just periodic", () => { + // README's Configuration table promises this disables the background + // summary worker ENTIRELY. Guarding only maybeTriggerPeriodicSummary + // would leave session_shutdown's "final" spawn running, making the + // documentation false. + const spawn = PI_SRC.slice(PI_SRC.indexOf("function spawnWikiWorker")); + const guardAt = spawn.indexOf('process.env.HIVEMIND_WIKI_WORKER === "1"'); + const lockAt = spawn.indexOf("tryAcquireSummaryLock(sessionId)"); + expect(guardAt).toBeGreaterThan(-1); + // and it gates BEFORE the lock, so a disabled worker never takes one + expect(lockAt).toBeGreaterThan(guardAt); + }); + + it("releases the lock on config-write failure", () => { + // pi's tryAcquireSummaryLock has no stale reclaim, so any leaked lock is + // held for the rest of the session and skips every later spawn. + const spawn = PI_SRC.slice(PI_SRC.indexOf("function spawnWikiWorker")); + const writeFail = spawn.indexOf("writeFileSync failed"); + const release = spawn.indexOf("releaseSummaryLock(sessionId)", writeFail); + const ret = spawn.indexOf("return;", release); + expect(writeFail).toBeGreaterThan(-1); + expect(release).toBeGreaterThan(writeFail); + expect(ret).toBeGreaterThan(release); + }); + + it("releases the lock on both sync and async spawn failure", () => { + const spawn = PI_SRC.slice(PI_SRC.indexOf("function spawnWikiWorker")); + // async 'error' (ENOENT/EPERM arrive as an event, not a throw) + expect(spawn).toMatch(/child\.on\("error"[\s\S]{0,220}releaseSummaryLock\(sessionId\)/); + // sync throw + expect(spawn).toMatch(/spawn failed[\s\S]{0,120}releaseSummaryLock\(sessionId\)/); + }); +}); diff --git a/tests/shared/summary-attempt-stamp-source.test.ts b/tests/shared/summary-attempt-stamp-source.test.ts new file mode 100644 index 00000000..005a7071 --- /dev/null +++ b/tests/shared/summary-attempt-stamp-source.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +/** + * Cross-agent source guard for the #331 back-off. + * + * Only the claude_code and codex capture hooks have functional trigger tests + * (their suites mock summary-state); cursor and hermes do not, so their copies + * of maybeTriggerPeriodicSummary are only reachable at the source level. All + * four are near-identical by construction, and the two properties that matter + * are ordering ones a refactor can silently break: + * + * 1. the attempt is stamped BEFORE the worker is spawned — otherwise a + * failing run leaves lastSummaryCount at 0 and the trigger refires on + * every captured event, which is the bug itself; + * 2. the stamp sits INSIDE the try that releases the spawn lock — otherwise + * a throwing state write leaks the lock until the 10-minute stale reclaim. + */ + +const CAPTURES = [ + "src/hooks/capture.ts", + "src/hooks/codex/capture.ts", + "src/hooks/cursor/capture.ts", + "src/hooks/hermes/capture.ts", +] as const; + +describe("periodic summary trigger — attempt stamping (issue #331)", () => { + for (const rel of CAPTURES) { + const src = readFileSync(join(process.cwd(), rel), "utf-8"); + const trigger = src.slice(src.indexOf("function maybeTriggerPeriodicSummary")); + + it(`${rel} stamps the attempt before spawning the worker`, () => { + const stampAt = trigger.indexOf("markSummaryAttempt(sessionId)"); + const spawnAt = trigger.search(/spawn\w*WikiWorker\(\{/); + expect(stampAt, "markSummaryAttempt missing").toBeGreaterThan(-1); + expect(spawnAt, "wiki worker spawn missing").toBeGreaterThan(-1); + expect(spawnAt).toBeGreaterThan(stampAt); + }); + + it(`${rel} stamps inside the try that releases the lock`, () => { + const tryAt = trigger.indexOf("try {", trigger.indexOf("tryAcquireLock")); + const stampAt = trigger.indexOf("markSummaryAttempt(sessionId)"); + const releaseAt = trigger.indexOf("releaseLock(sessionId)"); + expect(tryAt).toBeGreaterThan(-1); + expect(stampAt).toBeGreaterThan(tryAt); + expect(releaseAt).toBeGreaterThan(stampAt); + }); + } +});