From 039579414017d2cf78eecb80911c31f122ee7422 Mon Sep 17 00:00:00 2001 From: Ramiro Rivera Date: Mon, 27 Jul 2026 18:16:24 +0200 Subject: [PATCH 1/4] fix(hooks): adopt a buddy_react reaction written under another session id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buddy_react fires, buddy_stats reports Source: tool with the model-written text, and the statusline bubble still shows a canned pool line. Observed live: reaction.4758cc43.json source: tool "*ears twitch* thirty-one..." reaction.d26d5a00.json source: fallback "..." The MCP server is long-lived and resolves BUDDY_SID once at launch; the hook and the statusline resolve it per invocation. When those diverge, buddy_react writes a real reaction into a file nothing renders, this hook finds an empty file for *its* session, and overwrites the bubble with the pool — indistinguishable from the reactions being fake, which is the exact complaint the provenance work set out to answer. The guard now looks for a fresh tool reaction across every session file rather than only its own, and adopts it into this session's file so the statusline actually renders it. Ours still wins when both are fresh. Found only because of the field: in one file, in the other. - server/hooks/buddy-comment.ts - server/hooks/buddy-comment.test.ts Fixes #167 --- server/hooks/buddy-comment.test.ts | 26 +++++++++++ server/hooks/buddy-comment.ts | 69 ++++++++++++++++++++++++++---- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/server/hooks/buddy-comment.test.ts b/server/hooks/buddy-comment.test.ts index 2f5532e..2a56bdf 100644 --- a/server/hooks/buddy-comment.test.ts +++ b/server/hooks/buddy-comment.test.ts @@ -252,3 +252,29 @@ describe("buddy comment Stop hook", () => { expect(events.turns).toBe(1); }); }); + +describe("cross-session buddy_react adoption", () => { + test("adopts a fresh tool reaction written under a different session id", () => { + const stateDir = mkdtempSync(join(tmpdir(), "buddy-xsession-")); + writeFileSync(join(stateDir, "status.json"), JSON.stringify({ name: "Cobalt", species: "pikachu" })); + // buddy_react wrote here (MCP server's session id) + writeFileSync( + join(stateDir, "reaction.OTHERSID.json"), + JSON.stringify({ reaction: "*ears twitch*", timestamp: Date.now(), reason: "turn", source: "tool" }), + ); + + const result = handleBuddyComment( + JSON.stringify({ last_assistant_message: "hello", session_id: "MYSID" }), + { stateDir, sessionId: "MYSID", now: () => Date.now(), spawnDetached: () => {} } as never, + ); + + // The pool must NOT have clobbered it... + expect(result.source).toBe("tool"); + // ...and it must be readable from THIS session's file, which the + // statusline is the only thing that reads. + const own = JSON.parse(readFileSync(join(stateDir, "reaction.MYSID.json"), "utf8")); + expect(own.reaction).toBe("*ears twitch*"); + expect(own.source).toBe("tool"); + rmSync(stateDir, { recursive: true, force: true }); + }); +}); diff --git a/server/hooks/buddy-comment.ts b/server/hooks/buddy-comment.ts index 0f0bfe6..43faf3b 100755 --- a/server/hooks/buddy-comment.ts +++ b/server/hooks/buddy-comment.ts @@ -1,6 +1,6 @@ #!/usr/bin/env bun -import { mkdirSync, readFileSync, renameSync, writeFileSync } from "fs"; +import { mkdirSync, readdirSync, readFileSync, renameSync, writeFileSync } from "fs"; import { join } from "path"; import { reactionPool } from "./reaction-data.ts"; import { @@ -44,7 +44,9 @@ const BUDDY_COMMENT_PATTERN = //g; * Provenance of the reaction the hook wrote. Mirrors server/state.ts * `ReactionSource`; loaded as `none` on legacy files without the field. */ -export type ReactionSource = "comment" | "fallback" | "none"; +// "tool" is returned when a buddy_react reaction was adopted rather than +// written by this hook; it mirrors server/state.ts's ReactionSource. +export type ReactionSource = "tool" | "comment" | "fallback" | "none"; export interface BuddyCommentResult { comment?: string; @@ -131,14 +133,13 @@ function readTimestampSeconds(path: string): number { * is clamped to `now` so a skewed clock on a prior run does not poison * the comparison. */ -function toolFiredThisTurn( - reactionPath: string, +function isFreshToolReaction( + candidate: ReactionFile | null, stopMarkerPath: string, nowMs: number, ): boolean { - const existing = readJsonFile(reactionPath); - if (!existing || existing.source !== "tool") return false; - const ts = typeof existing.timestamp === "number" ? existing.timestamp : 0; + if (!candidate || candidate.source !== "tool") return false; + const ts = typeof candidate.timestamp === "number" ? candidate.timestamp : 0; if (ts <= 0) return false; if (ts > nowMs + FUTURE_TIMESTAMP_TOLERANCE_MS) return false; const lastStopSec = readTimestampSeconds(stopMarkerPath); @@ -146,6 +147,47 @@ function toolFiredThisTurn( return ts > effectiveLastStopMs; } +/** + * The MCP server and this hook do not always agree on the session id: the + * server is long-lived and resolves BUDDY_SID once at launch, while the hook + * and the statusline resolve it per invocation. When they diverge, + * `buddy_react` writes a real model-authored reaction into a file nothing + * renders, this hook sees an empty file for *its* session, and overwrites the + * bubble with a canned pool line — which looks exactly like the reactions + * being fake. + * + * So look for a fresh tool reaction across every session file, not just ours, + * and adopt it. Ours still wins when both are fresh. + */ +function findFreshToolReaction( + stateDir: string, + ownReactionPath: string, + stopMarkerPath: string, + nowMs: number, +): ReactionFile | null { + const own = readJsonFile(ownReactionPath); + if (isFreshToolReaction(own, stopMarkerPath, nowMs)) return own; + + let best: ReactionFile | null = null; + let entries: string[] = []; + try { + entries = readdirSync(stateDir); + } catch { + return null; + } + for (const entry of entries) { + if (!entry.startsWith("reaction.") || !entry.endsWith(".json")) continue; + const path = join(stateDir, entry); + if (path === ownReactionPath) continue; + const candidate = readJsonFile(path); + if (!isFreshToolReaction(candidate, stopMarkerPath, nowMs)) continue; + const bestTs = typeof best?.timestamp === "number" ? best.timestamp : 0; + const candidateTs = typeof candidate?.timestamp === "number" ? candidate.timestamp : 0; + if (!best || candidateTs > bestTs) best = candidate; + } + return best; +} + export function handleBuddyComment( rawInput: string, runtime: HookRuntime = {}, @@ -170,9 +212,18 @@ export function handleBuddyComment( const cooldown = nonNegativeInteger(config.commentCooldown, 30); // ─── Don't clobber a buddy_react tool reaction from this turn ─────────── - if (toolFiredThisTurn(reactionPath, stopMarkerFile, now)) { + // Checked across all session files: see findFreshToolReaction for why. + const freshTool = findFreshToolReaction(stateDir, reactionPath, stopMarkerFile, now); + if (freshTool) { + // Adopt it into this session's file so the statusline — which reads only + // its own session — actually renders the model-authored line. + const own = readJsonFile(reactionPath); + if (own?.reaction !== freshTool.reaction || own?.source !== "tool") { + mkdirSync(stateDir, { recursive: true }); + atomicWriteJson(reactionPath, freshTool); + } atomicWriteTimestamp(stopMarkerFile, now); - return { source: "none", updated: false }; + return { source: "tool", updated: false }; } // ─── Cooldown: rate-limit the reaction write AND bookkeeping ────────── From 9b1aa528814506bbfbc4941b0772ed4318739e0e Mon Sep 17 00:00:00 2001 From: Ramiro Rivera Date: Mon, 27 Jul 2026 18:31:11 +0200 Subject: [PATCH 2/4] fix(install): stop hook registrations accumulating on every install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dedupe filter matched commands containing 'coding-buddy' or 'claude-buddy', but the installed path is /app/hooks/buddy-comment.sh — neither string appears. So the filter never matched and every install appended another copy. A dogfooding machine was found with EIGHT Stop entries. Duplicates are not cosmetic. The first invocation adopts a buddy_react reaction and stamps the stop marker; the rest then judge that same reaction stale and overwrite the bubble with a canned pool line. The last write wins, so the user always saw canned text no matter how many times the underlying bug was fixed. Two changes: - match buddy hook commands by script name and state path, not package name, and apply it to Stop, PostToolUse and UserPromptSubmit alike - give the hook a same-turn grace window so repeated invocations within one turn cannot demote a reaction the first invocation just adopted - cli/install.ts - server/hooks/buddy-comment.ts - server/hooks/buddy-comment.test.ts --- cli/install.ts | 24 +++++++++++++++++++++--- server/hooks/buddy-comment.test.ts | 25 +++++++++++++++++++++++++ server/hooks/buddy-comment.ts | 14 +++++++++++++- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/cli/install.ts b/cli/install.ts index 2033480..d32514a 100644 --- a/cli/install.ts +++ b/cli/install.ts @@ -24,6 +24,17 @@ import { loadCompanion, saveCompanion, resolveUserId, writeStatusState } from ". import { generateFallbackName } from "../core/reactions.ts" import { copyRuntimeApp, stableRuntimePaths } from "./runtime-app.ts"; +/** Recognise any buddy-owned hook command, however it is pathed. */ +function isBuddyHookCommand(command: unknown): boolean { + if (typeof command !== "string") return false; + return ( + command.includes("coding-buddy") || + command.includes("claude-buddy") || + command.includes("buddy-state") || + /\/(buddy-comment|suggest|react|mood-react|name-react|file-type-react)\.(sh|ts)\b/.test(command) + ); +} + const CYAN = "\x1b[36m"; const GREEN = "\x1b[32m"; const YELLOW = "\x1b[33m"; @@ -203,7 +214,7 @@ function installHooks(settings: Record, appDir: string) { // plus file-type specific reactions on Write/Edit. if (!settings.hooks.PostToolUse) settings.hooks.PostToolUse = []; settings.hooks.PostToolUse = settings.hooks.PostToolUse.filter( - (h: any) => !h.hooks?.some((hh: any) => hh.command?.includes("coding-buddy") || hh.command?.includes("claude-buddy")), + (h: any) => !h.hooks?.some((hh: any) => isBuddyHookCommand(hh.command)), ); settings.hooks.PostToolUse.push({ matcher: "Bash", @@ -216,8 +227,15 @@ function installHooks(settings: Record, appDir: string) { // Stop: extract comment from Claude's response if (!settings.hooks.Stop) settings.hooks.Stop = []; + // Match on the hook script filenames, not the package name: the installed + // path is /app/hooks/buddy-comment.sh, which contains neither + // "coding-buddy" nor "claude-buddy". The old filter therefore never matched + // and every install appended another pair — observed at 8 Stop entries on a + // dogfooding machine. Duplicates are not harmless: the first invocation + // adopts a buddy_react reaction and stamps the stop marker, then the rest + // see it as stale and overwrite the bubble with a canned pool line. settings.hooks.Stop = settings.hooks.Stop.filter( - (h: any) => !h.hooks?.some((hh: any) => hh.command?.includes("coding-buddy") || hh.command?.includes("claude-buddy")), + (h: any) => !h.hooks?.some((hh: any) => isBuddyHookCommand(hh.command)), ); settings.hooks.Stop.push({ hooks: [commandHook(commentHook)], @@ -230,7 +248,7 @@ function installHooks(settings: Record, appDir: string) { // reaction, plus mood-react based on prompt content. if (!settings.hooks.UserPromptSubmit) settings.hooks.UserPromptSubmit = []; settings.hooks.UserPromptSubmit = settings.hooks.UserPromptSubmit.filter( - (h: any) => !h.hooks?.some((hh: any) => hh.command?.includes("coding-buddy") || hh.command?.includes("claude-buddy")), + (h: any) => !h.hooks?.some((hh: any) => isBuddyHookCommand(hh.command)), ); settings.hooks.UserPromptSubmit.push({ hooks: [commandHook(nameHook)], diff --git a/server/hooks/buddy-comment.test.ts b/server/hooks/buddy-comment.test.ts index 2a56bdf..b28295a 100644 --- a/server/hooks/buddy-comment.test.ts +++ b/server/hooks/buddy-comment.test.ts @@ -278,3 +278,28 @@ describe("cross-session buddy_react adoption", () => { rmSync(stateDir, { recursive: true, force: true }); }); }); + +describe("duplicate Stop hook invocations", () => { + test("a second invocation in the same turn does not clobber the adopted reaction", () => { + const stateDir = mkdtempSync(join(tmpdir(), "buddy-dup-")); + writeFileSync(join(stateDir, "status.json"), JSON.stringify({ name: "Cobalt", species: "pikachu" })); + writeFileSync( + join(stateDir, "reaction.MYSID.json"), + JSON.stringify({ reaction: "*ears flick*", timestamp: Date.now(), reason: "turn", source: "tool" }), + ); + + const input = JSON.stringify({ last_assistant_message: "hi", session_id: "MYSID" }); + const runtime = { stateDir, sessionId: "MYSID", now: () => Date.now(), spawnDetached: () => {} } as never; + + // Duplicate registrations mean the hook runs several times per turn. + handleBuddyComment(input, runtime); + handleBuddyComment(input, runtime); + const third = handleBuddyComment(input, runtime); + + expect(third.source).toBe("tool"); + const own = JSON.parse(readFileSync(join(stateDir, "reaction.MYSID.json"), "utf8")); + expect(own.reaction).toBe("*ears flick*"); + expect(own.source).toBe("tool"); + rmSync(stateDir, { recursive: true, force: true }); + }); +}); diff --git a/server/hooks/buddy-comment.ts b/server/hooks/buddy-comment.ts index 43faf3b..40b0262 100755 --- a/server/hooks/buddy-comment.ts +++ b/server/hooks/buddy-comment.ts @@ -133,6 +133,18 @@ function readTimestampSeconds(path: string): number { * is clamped to `now` so a skewed clock on a prior run does not poison * the comparison. */ +/** + * How long after a stop marker a tool reaction still counts as "this turn". + * + * The Stop hook can legitimately fire more than once per turn — duplicate + * registrations accumulate in settings.json, and one dogfooding machine was + * observed with EIGHT Stop entries. Without this grace window the first + * invocation adopts the buddy_react reaction and stamps the marker, and every + * subsequent invocation then judges that same reaction stale and overwrites + * the bubble with a canned pool line. The user only ever sees the last write. + */ +const SAME_TURN_GRACE_MS = 10_000; + function isFreshToolReaction( candidate: ReactionFile | null, stopMarkerPath: string, @@ -144,7 +156,7 @@ function isFreshToolReaction( if (ts > nowMs + FUTURE_TIMESTAMP_TOLERANCE_MS) return false; const lastStopSec = readTimestampSeconds(stopMarkerPath); const effectiveLastStopMs = Math.min(lastStopSec * 1000, nowMs); - return ts > effectiveLastStopMs; + return ts > effectiveLastStopMs - SAME_TURN_GRACE_MS; } /** From b02ae1e23bd9f974df88fbf25b9baadef07a97be Mon Sep 17 00:00:00 2001 From: Ramiro Rivera Date: Mon, 27 Jul 2026 18:36:51 +0200 Subject: [PATCH 3/4] fix(hooks): bound adoption age, report adoption as an update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three reviewers independently flagged the same gap: a session with no stop marker yet has lastStopSec 0, so any positive timestamp read as fresh — letting a leftover tool reaction from an unrelated session surface on a new session's first turn. Bounded to the statusline's default reactionTTL: never adopt something the statusline would already treat as expired. Regression test covers a brand-new session with only a stale cross-session reaction present. Also: adoption writes to disk, so it now reports updated:true rather than false (a consumer using the flag to trigger a re-render would have been misled), and the new tests register their temp dirs with the shared cleanup so a failure cannot leak them into /tmp. - server/hooks/buddy-comment.ts - server/hooks/buddy-comment.test.ts --- server/hooks/buddy-comment.test.ts | 33 ++++++++++++++++++++++++++++-- server/hooks/buddy-comment.ts | 18 +++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/server/hooks/buddy-comment.test.ts b/server/hooks/buddy-comment.test.ts index b28295a..5196a23 100644 --- a/server/hooks/buddy-comment.test.ts +++ b/server/hooks/buddy-comment.test.ts @@ -256,6 +256,7 @@ describe("buddy comment Stop hook", () => { describe("cross-session buddy_react adoption", () => { test("adopts a fresh tool reaction written under a different session id", () => { const stateDir = mkdtempSync(join(tmpdir(), "buddy-xsession-")); + dirs.push(stateDir); writeFileSync(join(stateDir, "status.json"), JSON.stringify({ name: "Cobalt", species: "pikachu" })); // buddy_react wrote here (MCP server's session id) writeFileSync( @@ -275,13 +276,13 @@ describe("cross-session buddy_react adoption", () => { const own = JSON.parse(readFileSync(join(stateDir, "reaction.MYSID.json"), "utf8")); expect(own.reaction).toBe("*ears twitch*"); expect(own.source).toBe("tool"); - rmSync(stateDir, { recursive: true, force: true }); }); }); describe("duplicate Stop hook invocations", () => { test("a second invocation in the same turn does not clobber the adopted reaction", () => { const stateDir = mkdtempSync(join(tmpdir(), "buddy-dup-")); + dirs.push(stateDir); writeFileSync(join(stateDir, "status.json"), JSON.stringify({ name: "Cobalt", species: "pikachu" })); writeFileSync( join(stateDir, "reaction.MYSID.json"), @@ -300,6 +301,34 @@ describe("duplicate Stop hook invocations", () => { const own = JSON.parse(readFileSync(join(stateDir, "reaction.MYSID.json"), "utf8")); expect(own.reaction).toBe("*ears flick*"); expect(own.source).toBe("tool"); - rmSync(stateDir, { recursive: true, force: true }); + }); +}); + +describe("adoption age ceiling", () => { + test("does not adopt a stale cross-session tool reaction on a fresh session", () => { + const stateDir = mkdtempSync(join(tmpdir(), "buddy-stale-")); + dirs.push(stateDir); + writeFileSync(join(stateDir, "status.json"), JSON.stringify({ name: "Cobalt", species: "pikachu" })); + // Older than the statusline would ever render, and from another session. + writeFileSync( + join(stateDir, "reaction.OTHERSID.json"), + JSON.stringify({ + reaction: "*from an ancient turn*", + timestamp: Date.now() - 3_600_000, + reason: "turn", + source: "tool", + }), + ); + + // Brand-new session: no .last_stop_hook marker exists yet. + const result = handleBuddyComment( + JSON.stringify({ last_assistant_message: "hi", session_id: "NEWSID" }), + { stateDir, sessionId: "NEWSID", now: () => Date.now(), spawnDetached: () => {} } as never, + ); + + expect(result.source).not.toBe("tool"); + expect(existsSync(join(stateDir, "reaction.NEWSID.json")) && + JSON.parse(readFileSync(join(stateDir, "reaction.NEWSID.json"), "utf8")).reaction) + .not.toBe("*from an ancient turn*"); }); }); diff --git a/server/hooks/buddy-comment.ts b/server/hooks/buddy-comment.ts index 40b0262..af05c90 100755 --- a/server/hooks/buddy-comment.ts +++ b/server/hooks/buddy-comment.ts @@ -145,6 +145,17 @@ function readTimestampSeconds(path: string): number { */ const SAME_TURN_GRACE_MS = 10_000; +/** + * Hard ceiling on how old an adopted tool reaction may be. + * + * A brand-new session has no stop marker, so `readTimestampSeconds` returns 0 + * and every positive timestamp would otherwise look "fresh" — letting a + * leftover reaction from an unrelated session surface on the first turn. + * Bounded to the statusline's default reactionTTL: never adopt something the + * statusline would already treat as expired. + */ +const MAX_ADOPTION_AGE_MS = 900_000; + function isFreshToolReaction( candidate: ReactionFile | null, stopMarkerPath: string, @@ -154,6 +165,7 @@ function isFreshToolReaction( const ts = typeof candidate.timestamp === "number" ? candidate.timestamp : 0; if (ts <= 0) return false; if (ts > nowMs + FUTURE_TIMESTAMP_TOLERANCE_MS) return false; + if (nowMs - ts > MAX_ADOPTION_AGE_MS) return false; const lastStopSec = readTimestampSeconds(stopMarkerPath); const effectiveLastStopMs = Math.min(lastStopSec * 1000, nowMs); return ts > effectiveLastStopMs - SAME_TURN_GRACE_MS; @@ -230,12 +242,16 @@ export function handleBuddyComment( // Adopt it into this session's file so the statusline — which reads only // its own session — actually renders the model-authored line. const own = readJsonFile(reactionPath); + let wrote = false; if (own?.reaction !== freshTool.reaction || own?.source !== "tool") { mkdirSync(stateDir, { recursive: true }); atomicWriteJson(reactionPath, freshTool); + wrote = true; } atomicWriteTimestamp(stopMarkerFile, now); - return { source: "tool", updated: false }; + // `updated` reports whether this invocation wrote the bubble, so adoption + // counts — a consumer using it to trigger a re-render must see the write. + return { source: "tool", updated: wrote }; } // ─── Cooldown: rate-limit the reaction write AND bookkeeping ────────── From 77f8d582bbc37bd6c49cb8b5854615b3724a4c54 Mon Sep 17 00:00:00 2001 From: Ramiro Rivera Date: Mon, 27 Jul 2026 18:45:24 +0200 Subject: [PATCH 4/4] test(statusline): pin terminal rows, not just columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four statusline test env blocks pinned BUDDY_STATUSLINE_COLS but left rows to the ambient terminal. Once density keyed on rows, those tests silently became environment-dependent: a developer on a 30-row terminal gets the compact tier, the bubble is dropped, and the rarity-colour assertion fails on main. CI has no tty, so rows fall back to the full-tier default and the failure is structurally invisible there. Same class as the width/tty problem the determinism work fixed — rows just never got the same treatment. - statusline/buddy-status.test.ts --- statusline/buddy-status.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/statusline/buddy-status.test.ts b/statusline/buddy-status.test.ts index 553af2b..5ed3576 100644 --- a/statusline/buddy-status.test.ts +++ b/statusline/buddy-status.test.ts @@ -53,6 +53,7 @@ function runStatusline( TMUX_PANE: "", BUDDY_STATUSLINE_ROWS: "50", BUDDY_STATUSLINE_COLS: columns, + BUDDY_STATUSLINE_ROWS: "50", TERM: "xterm-256color", NO_COLOR: "", BUDDY_SHELL: "", @@ -110,6 +111,7 @@ describe("buddy statusline colors", () => { TMUX_PANE: "", BUDDY_FAKE_NOW: "0", BUDDY_STATUSLINE_COLS: "80", + BUDDY_STATUSLINE_ROWS: "50", TERM: "xterm-256color", NO_COLOR: "", BUDDY_SHELL: "", @@ -233,6 +235,7 @@ describe("buddy statusline colors", () => { TMUX_PANE: "", BUDDY_FAKE_NOW: "0", BUDDY_STATUSLINE_COLS: process.stdin.isTTY ? "" : "60", + BUDDY_STATUSLINE_ROWS: "50", COLUMNS: "60", TERM: "xterm-256color", LC_ALL: "C", @@ -283,6 +286,7 @@ describe("buddy statusline colors", () => { for (const override of ["0", "abc"]) { const result = runStatusline(configDir, "{}\n", "60", { BUDDY_STATUSLINE_COLS: override, + BUDDY_STATUSLINE_ROWS: "50", COLUMNS: "60", }); const lines = result.stdout.toString().split("\n").filter(Boolean);