From ca879ada4e6cacf7c7cbd1e9137d2a684f9ad718 Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:21:14 +0000 Subject: [PATCH 01/10] fix(summary): back off after a failed summary run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A summary run that fails never reaches finalizeSummary: wiki-worker.ts returns early when `claude -p` exits non-zero or writes no summary file, then releases the spawn lock in its `finally`. lastSummaryCount therefore stays 0, and shouldTrigger's first-summary clause (lastSummaryCount === 0 && totalCount >= 10) keeps evaluating true on every subsequent captured event — one full `claude -p` process per tool call for the rest of the session. Track attempts separately from successes: markSummaryAttempt stamps lastAttemptAt + attemptsSinceSuccess before a run is spawned, finalizeSummary resets the counter, and shouldTrigger holds off for an exponential window (1 -> 30 minutes) while a failure is outstanding. Refs #331 --- src/hooks/summary-state.ts | 53 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) 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; From 88c5c4c690b0d85d0c4a6ea42a3521129b5cf37e Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:22:28 +0000 Subject: [PATCH 02/10] fix(capture): stamp the summary attempt before spawning the worker All four periodic triggers (claude_code, codex, cursor, hermes) win the spawn lock and then spawn without recording that a run started. Call markSummaryAttempt between the two so a run that fails is visible to shouldTrigger's back-off instead of being invisible bookkeeping. Refs #331 --- src/hooks/capture.ts | 6 ++++++ src/hooks/codex/capture.ts | 6 ++++++ src/hooks/cursor/capture.ts | 6 ++++++ src/hooks/hermes/capture.ts | 6 ++++++ 4 files changed, 24 insertions(+) diff --git a/src/hooks/capture.ts b/src/hooks/capture.ts index c0406e6e..c4f3053c 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"; @@ -271,6 +272,11 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con return; } + // 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). + markSummaryAttempt(sessionId); + wikiLog(`Periodic: threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); try { spawnWikiWorker({ diff --git a/src/hooks/codex/capture.ts b/src/hooks/codex/capture.ts index cae7bbc0..f154d009 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"; @@ -198,6 +199,11 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con return; } + // 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). + markSummaryAttempt(sessionId); + wikiLog(`Periodic: threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); try { spawnCodexWikiWorker({ diff --git a/src/hooks/cursor/capture.ts b/src/hooks/cursor/capture.ts index e3214aa3..c9ce472b 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"; @@ -229,6 +230,11 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con log(`periodic trigger suppressed (lock held) session=${sessionId}`); return; } + + // 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). + markSummaryAttempt(sessionId); wikiLog(`Periodic: threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); try { spawnCursorWikiWorker({ diff --git a/src/hooks/hermes/capture.ts b/src/hooks/hermes/capture.ts index 4e77d7ed..35d9f2d2 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"; @@ -224,6 +225,11 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con log(`periodic trigger suppressed (lock held) session=${sessionId}`); return; } + + // 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). + markSummaryAttempt(sessionId); wikiLog(`Periodic: threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); try { spawnHermesWikiWorker({ From cd476eeb4ab5570e8464d2980ad28c9669006f28 Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:22:53 +0000 Subject: [PATCH 03/10] test(summary): cover the failed-run spawn loop from #331 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The regression test replays the real loop — bump the counter per event, spawn when shouldTrigger fires, never finalize — over 30 events. With the back-off removed it produces 21 spawns; with it, 1. Also pins the back-off schedule (1/2/4/8/16 min, capped at 30), that a clean session is unaffected, that markSummaryAttempt does not advance lastSummaryCount, and that finalizeSummary clears the failure counter. The capture-hook suites mock summary-state with explicit factories, so markSummaryAttempt has to be declared there or the trigger throws and silently skips the spawn; they now also assert it fires before the spawn. --- tests/claude-code/capture-hook.test.ts | 7 +++ tests/claude-code/summary-state.test.ts | 84 +++++++++++++++++++++++++ tests/codex/codex-capture-hook.test.ts | 7 +++ 3 files changed, 98 insertions(+) diff --git a/tests/claude-code/capture-hook.test.ts b/tests/claude-code/capture-hook.test.ts index 3b699511..0f8e55df 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 () => { 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 () => { From eed64c63ee860a7e92360e41d265a9be65c3c0c4 Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:23:02 +0000 Subject: [PATCH 04/10] docs: document the summary cadence and worker off-switches HIVEMIND_SUMMARY_EVERY_N_MSGS, HIVEMIND_SUMMARY_EVERY_HOURS, HIVEMIND_WIKI_WORKER and HIVEMIND_GRAPH_ON_STOP all exist in the bundle but were undocumented, so the only remedy a user could find for background worker load was disabling the whole plugin. Refs #331 --- README.md | 4 ++++ 1 file changed, 4 insertions(+) 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. | From f834051c3009aaf0251885da655ff3fe49fdc89a Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:28:15 +0000 Subject: [PATCH 05/10] fix(capture): stamp inside the try so a failed write releases the lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit markSummaryAttempt ran between tryAcquireLock and the spawn try/catch, so a throwing state write left the spawn lock held until the 10-minute stale reclaim — the session would stop summarizing for that whole window. Found by codex review. --- src/hooks/capture.ts | 11 ++++++----- src/hooks/codex/capture.ts | 11 ++++++----- src/hooks/cursor/capture.ts | 11 ++++++----- src/hooks/hermes/capture.ts | 11 ++++++----- tests/claude-code/capture-hook.test.ts | 11 +++++++++++ 5 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/hooks/capture.ts b/src/hooks/capture.ts index c4f3053c..06359f7c 100644 --- a/src/hooks/capture.ts +++ b/src/hooks/capture.ts @@ -272,13 +272,14 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con return; } - // 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). - markSummaryAttempt(sessionId); - 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 f154d009..577b6aa8 100644 --- a/src/hooks/codex/capture.ts +++ b/src/hooks/codex/capture.ts @@ -199,13 +199,14 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con return; } - // 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). - markSummaryAttempt(sessionId); - 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 c9ce472b..893bdd2b 100644 --- a/src/hooks/cursor/capture.ts +++ b/src/hooks/cursor/capture.ts @@ -230,13 +230,14 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con log(`periodic trigger suppressed (lock held) session=${sessionId}`); return; } - - // 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). - markSummaryAttempt(sessionId); 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 35d9f2d2..2700b6c7 100644 --- a/src/hooks/hermes/capture.ts +++ b/src/hooks/hermes/capture.ts @@ -225,13 +225,14 @@ function maybeTriggerPeriodicSummary(sessionId: string, cwd: string, config: Con log(`periodic trigger suppressed (lock held) session=${sessionId}`); return; } - - // 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). - markSummaryAttempt(sessionId); 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/tests/claude-code/capture-hook.test.ts b/tests/claude-code/capture-hook.test.ts index 0f8e55df..edce62a2 100644 --- a/tests/claude-code/capture-hook.test.ts +++ b/tests/claude-code/capture-hook.test.ts @@ -374,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"); }); From 8ca38cf403b489b9162d726c619bf736e4eaa4c6 Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:28:20 +0000 Subject: [PATCH 06/10] fix(pi): apply the same failed-summary back-off to the pi extension The pi extension ships as standalone TypeScript with its own copy of the summary-state logic, so it kept the infinite-refire behavior after a failed run. It is also the only writer that rewrites the whole state object read from the shared dir, so without the new fields in its read path it would strip them from state the shared worker finalizes against. Found by codex review. --- harnesses/pi/extension-source/hivemind.ts | 28 ++++++++++++++++++++++- tests/pi/pi-extension-source.test.ts | 28 +++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/harnesses/pi/extension-source/hivemind.ts b/harnesses/pi/extension-source/hivemind.ts index e1e6dd31..2fd51481 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,6 +695,8 @@ 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; } } @@ -706,8 +715,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; @@ -941,6 +960,13 @@ function maybeTriggerPeriodicSummary(creds: Creds, sessionId: string, cwd: strin const cfg = loadSummaryConfig(); if (!shouldTriggerNow(state, cfg)) return; logHm(`periodic threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); + // Stamp the attempt before spawning so a failing worker backs off rather + // than refiring on every captured event (issue #331). + writeSummaryState(sessionId, { + ...state, + lastAttemptAt: Date.now(), + attemptsSinceSuccess: (state.attemptsSinceSuccess ?? 0) + 1, + }); spawnWikiWorker(creds, sessionId, cwd, "periodic"); } diff --git a/tests/pi/pi-extension-source.test.ts b/tests/pi/pi-extension-source.test.ts index e1fe3749..180a7d52 100644 --- a/tests/pi/pi-extension-source.test.ts +++ b/tests/pi/pi-extension-source.test.ts @@ -328,3 +328,31 @@ 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 before spawning the wiki worker", () => { + const trigger = PI_SRC.slice(PI_SRC.indexOf("function maybeTriggerPeriodicSummary")); + const stampAt = trigger.indexOf("attemptsSinceSuccess: (state.attemptsSinceSuccess ?? 0) + 1"); + const spawnAt = trigger.indexOf('spawnWikiWorker(creds, sessionId, cwd, "periodic")'); + expect(stampAt).toBeGreaterThan(-1); + expect(spawnAt).toBeGreaterThan(stampAt); + }); + + it("caps the back-off at 30 minutes, matching summary-state.ts", () => { + expect(PI_SRC).toContain("Math.min(2 ** (attemptsSinceSuccess - 1), 30) * 60 * 1000"); + }); +}); From 4789ce6ed7e0a5ee87970510f3140fa994483300 Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:33:55 +0000 Subject: [PATCH 07/10] test(summary): guard attempt-stamp placement across all four capture hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only claude_code and codex have functional trigger tests; cursor and hermes never mock summary-state, so their copies of maybeTriggerPeriodicSummary are unreachable from a functional test and showed up as the branch-coverage dip on this PR. Pins the two ordering properties a refactor can silently break: the stamp lands before the spawn, and inside the try that releases the lock. Verified against both regressions — removing the stamp fails the first assertion, moving it outside the try fails the second. --- .../summary-attempt-stamp-source.test.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 tests/shared/summary-attempt-stamp-source.test.ts 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); + }); + } +}); From aacaca6196f9f8ef7255672c7c1d024740ba328f Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:37:30 +0000 Subject: [PATCH 08/10] fix(pi): stamp the attempt after winning the lock, against fresh state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit stamped in maybeTriggerPeriodicSummary, before spawnWikiWorker acquires the summary lock, and wrote back the whole state object read at trigger time. Two consequences: a concurrent worker that finalized in between would have its success overwritten by stale counters, and a spawn suppressed by the lock was still recorded as a failed attempt. Move the stamp inside spawnWikiWorker, after the lock is won, and build it from a fresh read. Periodic only — the final session-shutdown summary gets no back-off, matching summary-state.ts. Found by codex review. --- harnesses/pi/extension-source/hivemind.ts | 23 ++++++++++++----- tests/pi/pi-extension-source.test.ts | 31 ++++++++++++++++++----- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/harnesses/pi/extension-source/hivemind.ts b/harnesses/pi/extension-source/hivemind.ts index 2fd51481..7457825b 100644 --- a/harnesses/pi/extension-source/hivemind.ts +++ b/harnesses/pi/extension-source/hivemind.ts @@ -826,6 +826,22 @@ 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); + if (fresh) { + writeSummaryState(sessionId, { + ...fresh, + lastAttemptAt: Date.now(), + attemptsSinceSuccess: (fresh.attemptsSinceSuccess ?? 0) + 1, + }); + } + } // 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 */ } @@ -960,13 +976,6 @@ function maybeTriggerPeriodicSummary(creds: Creds, sessionId: string, cwd: strin const cfg = loadSummaryConfig(); if (!shouldTriggerNow(state, cfg)) return; logHm(`periodic threshold hit (total=${state.totalCount}, since=${state.totalCount - state.lastSummaryCount}, N=${cfg.everyNMessages}, hours=${cfg.everyHours})`); - // Stamp the attempt before spawning so a failing worker backs off rather - // than refiring on every captured event (issue #331). - writeSummaryState(sessionId, { - ...state, - lastAttemptAt: Date.now(), - attemptsSinceSuccess: (state.attemptsSinceSuccess ?? 0) + 1, - }); spawnWikiWorker(creds, sessionId, cwd, "periodic"); } diff --git a/tests/pi/pi-extension-source.test.ts b/tests/pi/pi-extension-source.test.ts index 180a7d52..fa835e47 100644 --- a/tests/pi/pi-extension-source.test.ts +++ b/tests/pi/pi-extension-source.test.ts @@ -344,12 +344,31 @@ describe("pi extension — failed-summary back-off (issue #331)", () => { ); }); - it("stamps the attempt before spawning the wiki worker", () => { - const trigger = PI_SRC.slice(PI_SRC.indexOf("function maybeTriggerPeriodicSummary")); - const stampAt = trigger.indexOf("attemptsSinceSuccess: (state.attemptsSinceSuccess ?? 0) + 1"); - const spawnAt = trigger.indexOf('spawnWikiWorker(creds, sessionId, cwd, "periodic")'); - expect(stampAt).toBeGreaterThan(-1); - expect(spawnAt).toBeGreaterThan(stampAt); + 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", () => { From c2ef660ad5428ae7d5ca318dbb6fe6cc96e60edd Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:46:12 +0000 Subject: [PATCH 09/10] fix(pi): fail closed on an unpersisted attempt stamp, honor the off switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings, both on the pi extension: writeSummaryState swallowed every write error, so a stamp that never reached disk still let the worker spawn — and the next captured event refired immediately, which is the exact behavior the back-off exists to prevent. It now reports failure, and the periodic path releases the lock and skips the spawn when the state cannot be read or the stamp cannot be persisted. The README documents HIVEMIND_WIKI_WORKER=1 as disabling background summaries, but pi only checked HIVEMIND_CAPTURE and kept spawning periodic workers. Honor it in the trigger, which doubles as the same recursion guard the bundled hooks use. Found by CodeRabbit; the first also matches codex's "pi's stamp is best-effort" finding. --- harnesses/pi/extension-source/hivemind.ts | 33 +++++++++++++++++------ tests/pi/pi-extension-source.test.ts | 26 ++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/harnesses/pi/extension-source/hivemind.ts b/harnesses/pi/extension-source/hivemind.ts index 7457825b..8e9050f6 100644 --- a/harnesses/pi/extension-source/hivemind.ts +++ b/harnesses/pi/extension-source/hivemind.ts @@ -701,11 +701,22 @@ function readSummaryState(sessionId: string): SummaryState | null { } 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 { @@ -834,12 +845,15 @@ function spawnWikiWorker( // summary at session shutdown gets no back-off, matching summary-state.ts. if (reason === "periodic") { const fresh = readSummaryState(sessionId); - if (fresh) { - writeSummaryState(sessionId, { - ...fresh, - lastAttemptAt: Date.now(), - attemptsSinceSuccess: (fresh.attemptsSinceSuccess ?? 0) + 1, - }); + 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. @@ -972,6 +986,9 @@ function spawnPiSkillifyWorker(creds: Creds, sessionId: string, cwd: string): vo function maybeTriggerPeriodicSummary(creds: Creds, sessionId: string, cwd: string): void { if (process.env.HIVEMIND_CAPTURE === "false") return; + // Documented off switch for background summaries (README Configuration). + // Also the recursion guard the worker itself sets. + if (process.env.HIVEMIND_WIKI_WORKER === "1") return; const state = bumpCounter(sessionId); const cfg = loadSummaryConfig(); if (!shouldTriggerNow(state, cfg)) return; diff --git a/tests/pi/pi-extension-source.test.ts b/tests/pi/pi-extension-source.test.ts index fa835e47..470693ff 100644 --- a/tests/pi/pi-extension-source.test.ts +++ b/tests/pi/pi-extension-source.test.ts @@ -375,3 +375,29 @@ describe("pi extension — failed-summary back-off (issue #331)", () => { 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 the documented HIVEMIND_WIKI_WORKER=1 off switch", () => { + // README's Configuration table promises this disables background + // summaries; pi previously only checked HIVEMIND_CAPTURE. + const trigger = PI_SRC.slice(PI_SRC.indexOf("function maybeTriggerPeriodicSummary")); + expect(trigger).toContain('process.env.HIVEMIND_WIKI_WORKER === "1"'); + }); +}); From 0537577b63d63aef6af862eb2b3c3125f2efbb0a Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Wed, 29 Jul 2026 18:54:54 +0000 Subject: [PATCH 10/10] fix(pi): make the off switch cover the final summary, stop leaking the lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two blocking review findings. HIVEMIND_WIKI_WORKER=1 only guarded the periodic trigger, so session_shutdown still spawned the final worker — the README line I added in this PR promises it disables background summaries entirely, which was false. Move the guard into spawnWikiWorker, ahead of the lock, so it covers both reasons and a disabled worker never takes a lock. The lock was released on stamp failure but not on config-write failure, sync spawn failure, or the async 'error' event that carries ENOENT/EPERM. pi's tryAcquireSummaryLock has no stale reclaim, unlike summary-state.ts, so any one of those held the lock for the rest of the session and silently skipped every later periodic and final spawn. Found by codex review. --- harnesses/pi/extension-source/hivemind.ts | 34 +++++++++++++++++---- tests/pi/pi-extension-source.test.ts | 36 +++++++++++++++++++---- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/harnesses/pi/extension-source/hivemind.ts b/harnesses/pi/extension-source/hivemind.ts index 8e9050f6..ce3563a1 100644 --- a/harnesses/pi/extension-source/hivemind.ts +++ b/harnesses/pi/extension-source/hivemind.ts @@ -826,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; @@ -881,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); } } @@ -986,9 +1011,6 @@ function spawnPiSkillifyWorker(creds: Creds, sessionId: string, cwd: string): vo function maybeTriggerPeriodicSummary(creds: Creds, sessionId: string, cwd: string): void { if (process.env.HIVEMIND_CAPTURE === "false") return; - // Documented off switch for background summaries (README Configuration). - // Also the recursion guard the worker itself sets. - if (process.env.HIVEMIND_WIKI_WORKER === "1") return; const state = bumpCounter(sessionId); const cfg = loadSummaryConfig(); if (!shouldTriggerNow(state, cfg)) return; diff --git a/tests/pi/pi-extension-source.test.ts b/tests/pi/pi-extension-source.test.ts index 470693ff..80d33f9e 100644 --- a/tests/pi/pi-extension-source.test.ts +++ b/tests/pi/pi-extension-source.test.ts @@ -394,10 +394,36 @@ describe("pi extension — #331 review follow-ups", () => { expect(PI_SRC).toContain("} catch { return false; }"); }); - it("honors the documented HIVEMIND_WIKI_WORKER=1 off switch", () => { - // README's Configuration table promises this disables background - // summaries; pi previously only checked HIVEMIND_CAPTURE. - const trigger = PI_SRC.slice(PI_SRC.indexOf("function maybeTriggerPeriodicSummary")); - expect(trigger).toContain('process.env.HIVEMIND_WIKI_WORKER === "1"'); + 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\)/); }); });