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 2f5532e..5196a23 100644 --- a/server/hooks/buddy-comment.test.ts +++ b/server/hooks/buddy-comment.test.ts @@ -252,3 +252,83 @@ 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-")); + dirs.push(stateDir); + 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"); + }); +}); + +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"), + 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"); + }); +}); + +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 0f0bfe6..af05c90 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,19 +133,83 @@ 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, +/** + * 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; + +/** + * 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, 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; + if (nowMs - ts > MAX_ADOPTION_AGE_MS) return false; const lastStopSec = readTimestampSeconds(stopMarkerPath); const effectiveLastStopMs = Math.min(lastStopSec * 1000, nowMs); - return ts > effectiveLastStopMs; + return ts > effectiveLastStopMs - SAME_TURN_GRACE_MS; +} + +/** + * 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( @@ -170,9 +236,22 @@ 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); + 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: "none", 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 ────────── 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);