fix(server): make buddy_react the primary end-of-turn reaction channel - #159
Conversation
…annel The HTML-comment channel broke visibly in Claude Code v2.1.169+ (issue end-of-turn, steering the model to the broken channel while keeping the working MCP tool unused. Result: every assistant message leaked the raw <!-- buddy: ... --> line and the primary model-authored path never fired. Accept-with-changes verdict on PR #127: that PR keeps the HTML comment as primary and adds a canned-pool fallback for when the comment is missing. That trades a visible-artifact bug for the same canned-pool loss the user already complained about ('suspicious English placeholder shit'). The real fix is the in-process option: make buddy_react the primary channel (it renders nowhere in the transcript and is model-authored by construction), keep the HTML-comment extractor as a backward-compat fallback for older Claude Code versions, and only fall through to a canned-pool line as a last resort. Changes: * server/state.ts — ReactionSource = tool|comment|fallback|none; ReactionState.source is optional and loaded as 'none' for legacy files. saveReaction() takes an optional source argument (default 'tool' since every MCP-side caller is one). * server/index.ts — getInstructions() and buddy://prompt now direct the model to call buddy_react at the end of every turn, with explicit examples. The contradictory 'Do NOT use buddy_react for end-of-turn comments' line is gone, as is the 'do NOT append an invisible HTML comment' steer. buddy_react / buddy_pet / buddy_unmute tag their writes source=tool or source=fallback. buddy_stats now loads the reaction file and shows the last line + its source so users can prove the bubble is not canned. * hooks/buddy-comment.sh — header rewritten to describe the 3-source chain; bookkeeping (turn counter, XP, memory) always runs on assistant message; reaction write skipped when a fresh source= tool reaction is on disk; otherwise tries the legacy HTML comment (source=comment) then falls back to a pool pick via the new server/turn-reaction.ts script (source= fallback). Cooldown still rate-limits the reaction write, not the bookkeeping. * server/turn-reaction.ts — new canned-pool entry used only as the last-resort fallback. Mirrors what PR #127 added but with the source tag wired through. * README.md — features card and 'How It Works' updated to describe the new chain and provenance tag; the '<!-- buddy: ... -->' Stop Hook line is replaced with the actual chain (skip tool → comment → fallback). Tests: bun test (246 pass, 0 fail). bun run typecheck clean. Smoke tests confirmed all three reaction paths (comment writes source= comment, canned fallback writes source=fallback, fresh tool reaction is left alone). Refs #124, #154.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (16)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| "might want to split that PR.", | ||
| "bold move. let's see if CI agrees.", | ||
| "*counts lines nervously*" | ||
| "*nervous laughter* {lines} lines changed.", |
There was a problem hiding this comment.
🟡 Buddy speech bubble shows the raw text "{lines}" instead of the number of changed lines
A new large-diff reaction is added with a {lines} fill-in slot (server/hooks/reaction-data.ts:1923) that the code displaying it never fills in, so when this line is picked the buddy's bubble shows the literal text {lines} instead of the real change count.
Impact: On big diffs the user sometimes sees a broken-looking message reading "{lines} lines changed" rather than an actual number.
Why the placeholder leaks: hooks reaction path never substitutes {lines}
The hooks reaction data in server/hooks/reaction-data.ts is consumed by server/hooks/react.ts. When a large diff is detected, react.ts picks a line from the large-diff pool via pickReaction. Although classifyToolResponse computes the line count into classification.lines (server/hooks/react.ts:93-95), the handler only reads files/branch (server/hooks/react.ts:279-280) and only substitutes {files} and {branch} (server/hooks/react.ts:381-382). There is no {lines} replacement, so the newly added "*nervous laughter* {lines} lines changed." renders literally.
Note: the MCP-server copy core/reactions.ts:614 does replace {lines} via context.lines, but the hooks copy does not — before this PR the hooks large-diff pool contained no {lines} placeholder, so this is the first time the placeholder can leak on the hook path.
Prompt for agents
The large-diff reaction pool in server/hooks/reaction-data.ts now includes the line "*nervous laughter* {lines} lines changed." which contains a {lines} placeholder. The hook consumer server/hooks/react.ts substitutes {files} and {branch} (around lines 381-382) but never substitutes {lines}, even though classifyToolResponse already captures the count into classification.lines (around lines 93-95, and the type field is declared at line 38). As a result the literal text {lines} will be shown in the buddy speech bubble when this reaction is selected. Fix by wiring the captured lines value through handleReact (read classification?.lines alongside files/branch near lines 279-280) and adding a substitution like: if (lines) reaction = reaction?.replace("{lines}", lines); next to the existing {files}/{branch} replacements. Mirror how core/reactions.ts:614 already handles this placeholder.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // ─── Bookkeeping: runs on every assistant message, regardless of source ── | ||
|
|
||
| const eventsFile = join(stateDir, "events.json"); | ||
| const events = readJsonFile<Events>(eventsFile) ?? {}; | ||
| events.turns = (typeof events.turns === "number" ? events.turns : 0) + 1; | ||
| mkdirSync(stateDir, { recursive: true }); | ||
| writeFileSync(eventsFile, JSON.stringify(events, null, 2)); | ||
|
|
||
| const spawnDetached = runtime.spawnDetached ?? defaultSpawnDetached(runtime); | ||
| spawnDetached("server/award-xp.ts", ["turn"]); | ||
| spawnDetached("server/consolidate.ts", [ | ||
| assistantMessage, | ||
| stringField(input, "last_user_message"), | ||
| ]); |
There was a problem hiding this comment.
🔍 Bookkeeping (turn count, XP award, memory consolidation) now runs on every turn regardless of cooldown or reaction outcome
Previously events.turns++, spawnDetached("server/award-xp.ts", ["turn"]), and spawnDetached("server/consolidate.ts", ...) only ran when an HTML comment was extracted AND the cooldown had elapsed. After this change (server/hooks/buddy-comment.ts:124-135), they run on every assistant message before the fresh-tool skip and cooldown checks. This means: (1) the turns stat and turn XP are now incremented on every turn, no longer rate-limited by the 30s cooldown, so XP can accrue faster during rapid exchanges; and (2) consolidate.ts is spawned as a detached process on every turn instead of only when a bubble was written. The explicit comment and the added test (server/hooks/buddy-comment.test.ts:122-126) indicate this decoupling is intentional, but it is a meaningful behavioral/resource change worth confirming, especially the unbounded per-turn spawning of consolidate and the removal of cooldown gating on XP.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function isFreshToolReaction(reactionPath: string, nowMs: number): boolean { | ||
| const existing = readJsonFile<ReactionFile>(reactionPath); | ||
| if (!existing) return false; | ||
| if (existing.source !== "tool") return false; | ||
| const ts = typeof existing.timestamp === "number" ? existing.timestamp : 0; | ||
| const age = nowMs - ts; | ||
| return age >= 0 && age < TOOL_REACTION_FRESHNESS_MS; | ||
| } |
There was a problem hiding this comment.
📝 Info: Fresh-tool skip relies on identical reaction file path between MCP server and Stop hook
The don't-clobber logic reads reaction.${sid}.json (server/hooks/buddy-comment.ts:118, isFreshToolReaction), where sid comes from resolveHookSessionId (CLAUDE_CODE_SESSION_ID first 8 chars / TMUX_PANE / "default"). The MCP buddy_react tool writes via saveReaction -> reactionFile() in server/state.ts:56-66, which derives the session id identically. I verified these match, so the skip works. Worth noting the coupling is implicit: if either session-id derivation diverges (e.g. env var differences between the MCP server process and the hook process), the hook would clobber fresh tool reactions and the primary channel guarantee would silently break.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pull request overview
This PR changes the end-of-turn “buddy reaction” mechanism to use the buddy_react MCP tool as the primary channel (instead of the legacy <!-- buddy: ... --> HTML comment), adds persisted reaction provenance (tool/comment/fallback/none), and updates the Stop hook to use a three-stage fallback chain while exposing provenance in /buddy stats.
Changes:
- Add
ReactionSourceprovenance to persisted reaction state and surface it in/buddy stats. - Update model/system instructions to use
buddy_reactas the required end-of-turn path and discourage HTML comment emission. - Update Stop hook to (1) skip if a fresh
source:"tool"reaction exists, (2) extract legacy HTML comment, else (3) pick a canned “turn” fallback reaction.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| server/state.ts | Adds persisted reaction provenance and extends saveReaction signature to include source. |
| server/index.ts | Updates prompting/instructions, records provenance, and surfaces provenance in buddy_stats. |
| server/hooks/reaction-data.ts | Tweaks reaction pools and adds a turn pool for Stop-hook fallback. |
| server/hooks/buddy-comment.ts | Implements three-stage Stop-hook fallback chain with provenance tagging. |
| server/hooks/buddy-comment.test.ts | Adds/updates tests for provenance and tool-reaction “do not clobber” behavior. |
| README.md | Updates docs to describe the new primary buddy_react channel and provenance. |
Comments suppressed due to low confidence (1)
server/index.ts:334
buddy_reactpersists the reaction with the correctreasonandsource: "tool", but the subsequentwriteStatusState(companion, comment, ...)re-saves the reaction viasaveReaction(comment, "mcp"), overwriting the just-writtenreason/timestamp (and potentiallysourcedepending on defaults). That will make/buddy statsshowReason: mcpinstead of the actual trigger.
saveReaction(comment, reason ?? "turn", "tool");
incrementEvent("reactions_given", 1, activeSlot());
const newAch = checkAndAward(activeSlot());
const achName = newAch.length > 0 ? newAch[0].icon + " " + newAch[0].name : undefined;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| saveReaction(reaction, "pet"); | ||
| saveReaction(reaction, "pet", "fallback"); | ||
| writeStatusState(companion, reaction); |
| export function saveReaction( | ||
| reaction: string, | ||
| reason: string, | ||
| source: ReactionSource = "tool", | ||
| ): void { | ||
| mkdirSync(STATE_DIR, { recursive: true }); | ||
| const state: ReactionState = { reaction, timestamp: Date.now(), reason }; | ||
| const state: ReactionState = { reaction, timestamp: Date.now(), reason, source }; | ||
| writeFileSync(reactionFile(), JSON.stringify(state)); |
| "might want to split that PR.", | ||
| "bold move. let's see if CI agrees.", | ||
| "*counts lines nervously*" | ||
| "*nervous laughter* {lines} lines changed.", |
| - **Status Line** — animated ASCII art, right-aligned, with rarity color and speech bubble | ||
| - **PostToolUse Hook** — detects errors, test failures, large diffs in Bash output | ||
| - **Stop Hook** — extracts invisible `<!-- buddy: ... -->` comments from Claude's responses | ||
| - **Stop Hook** — backward-compat fallback chain. Skips if a fresh `buddy_react` (primary channel) just ran; otherwise extracts a legacy `<!-- buddy: ... -->` comment if Claude happens to emit one (older Claude Code builds), and otherwise picks a canned pool line via `server/turn-reaction.ts`. Every write carries a `source` tag (`tool`/`comment`/`fallback`) so `/buddy stats` can prove what produced the bubble. |
| ``` | ||
|
|
||
| - **MCP Server** — companion tools + system prompt that instructs Claude to write buddy comments | ||
| - **MCP Server** — companion tools + system prompt that tells Claude to call `buddy_react` at the end of every turn. The tool call writes the reaction with `source: "tool"` — model-authored, renders nowhere in the transcript. |
| `- *adjusts crown* that error handler is missing a finally block`, | ||
| `- *blinks slowly* you renamed the variable but not the three references`, | ||
| `- *head tilts* are you sure that regex handles unicode?`, | ||
| `Display the tool result verbatim. Never explain that you're reacting.`, |
| `\`\`\``, | ||
| "", | ||
| "A Stop hook extracts this and displays it in the buddy's speech bubble on the status line. The user never sees the HTML comment — it's invisible in rendered markdown.", | ||
| `The tool updates ${companion.name}'s speech bubble on the status line. The tool call itself renders nowhere in the user's transcript — only the displayed speech bubble is visible. Display the tool result verbatim.`, | ||
| "", | ||
| "Rules:", | ||
| `- Write as ${companion.name} (a ${companion.bones.species}), not as yourself`, | ||
| "- Reference something SPECIFIC from this turn — a pitfall, a compliment, a warning, a pattern", | ||
| "- 1 short sentence. Use *asterisks* for physical actions", | ||
| `- Match personality: high ${companion.bones.peak} = lean into that trait`, | ||
| "- Do NOT use buddy_react tool for this. Do NOT explain the comment. Just append it.", | ||
| "- NEVER skip this. Every single response must end with <!-- buddy: ... -->", | ||
| `- Do NOT explain the call. Just call it and display the result.`, |
Decision on F6 (tool call vs plaintext): tool call, and stop echoing itMaintainer decision, from a live Claude Code session. Observed behaviour: the reaction currently appears verbatim in the response body AND again in the statusline — duplicated. The desired behaviour is statusline only. Key fact that resolves the contradiction: tool call outputs are not displayed in this Claude Code setup. The user does not see What leaks into the transcript is not the tool call. It is the instruction telling the model to print the reaction in prose. Required
The tool result string itself ( Everything else in the review (F1 freshness window, F2 clock skew, F3 source default, F4 provenance, F5 XP rate limit, F7 🤖 This content was generated with AI assistance using Claude Opus 5. |
…ision)
The previous round argued for 'visible by design' based on the
hypothesis that MCP tool results render in Claude Code's transcript.
The maintainer observed from a live session with a screenshot: that
is false in his environment. Tool results are NOT visible; the
model was duplicating the reaction into the reply body, producing a
duplicate-on-every-turn stream the user does not want.
Correct model of the channel:
* buddy_react is a real MCP tool call → its result is the bubble
text on the statusline only.
* The user never reads the reply text containing the tool call.
* The instruction 'display the tool result verbatim' was the leak:
it told the model to echo the reaction in prose, where Claude
Code did not show it as a tool result and the user instead saw
it duplicated into the reply.
Scrubbed:
* server/index.ts:101 (NAME REACTIONS block)
* server/index.ts:120 (END-OF-TURN block)
* server/index.ts:300 (buddy_react tool description)
* server/index.ts:1360 (buddy://prompt duplicate block — deleted
entirely; corrected the keeper at the fence close above)
* skills/buddy/SKILL.md:89 (the only writer of the duplicate)
Preserved:
* skills/buddy/SKILL.md:95 (buddy_uninstall — different tool, the
maintainer explicitly excluded that line).
* The statusline-only framing ('renders nowhere in the user's
transcript') — that is the accurate description in this
environment.
Tool doc / prompt now state the silent contract explicitly: the
result only reaches the statusline, the call is a silent side
effect, do not narrate / echo / quote / explain.
Tests/typecheck unchanged (purely documentation and prompt wiring,
no behaviour moved).
…ce tagging)
Lost the previous F1-F5 fixes when I used 'git reset --hard' to
rebase F6 onto a clean origin/fix/react-channel. Reviewer (Opus 5)
flagged the regression; this restores the fixes with TDD discipline:
tests written FIRST, verified to fail against the prior code, then
the fix.
F1 — per-turn sentinel (CRITICAL). The 60 s wall-clock freshness
gate lost tool reactions on any turn longer than 60 s, which is
ordinary for agentic coding (90 s-5 min). Stop hook now writes
its run time to '.last_stop_hook.<sid>' after each invocation; a
tool reaction is treated as 'from this turn' iff its timestamp is
more recent than the previous Stop hook run.
F2 — clock skew guard. Tool timestamps implausibly in the future
(beyond 60 s) are treated as clock-skew garbage and the hook falls
through to the comment / pool branch. 'lastStopRun' on the writer
side is clamped to min(lastStop, now) so a future-dated marker does
not poison the comparison. ('react.ts' now distinguishes a future
tool timestamp from a legitimate one instead of rejecting both as
'age < 0'.)
F3 — saveReaction contract. Default source flipped from 'tool' to
'fallback'. writeStatusState no longer calls saveReaction
implicitly — every hatch / pet / unmute call path now writes its
own provenance explicitly, so /buddy stats can no longer lie about
canonical 'tool' lines being canned.
F4 — pool-write hooks tag their provenance. react.ts,
mood-react.ts, file-type-react.ts, name-react.ts all now write
'source: fallback' alongside reaction / timestamp / reason. Legacy
files without the field default to 'none' on load.
F5 — bookkeeping respects the rate limit. 10 short turns inside
the default 30 s cooldown window now produce 1 turn-counter
increment and 2 detached bun boots, matching main's behavior.
Bookkeeping still fires on the fresh-tool-skip path (so the stop
marker is stamped) but does NOT increment turns or spawn XP /
memory on the path where the tool already wrote the reaction.
F7 — literal '{lines}' dropped. The line '*nervous laughter'
{lines} lines changed.' in the large-diff pool would render
literally as the hook only substitutes {files} and {branch}.
Removed. A new test scans every DEFAULT_REACTION_POOLS entry
for any unsubstituted {name} token and asserts none (catches
future regressions of this class).
F9 — atomic write pattern. saveReaction now uses tmp + rename on
the reaction file (torn reads were a sub-bug in the freshness
gate's read). The Stop hook does the same on reaction /
cooldown / stop-marker writes. New test asserts no leftover
'reaction.*.tmp.*' files remain after a hook run.
Tests added (all written before the fix and verified to fail):
server/hooks/buddy-comment.test.ts:
F1 long-turn: 5-min tool reaction is preserved
F1 fresh-tool: skipped even within default cooldown; bookkeeping
does NOT spawn (F5)
F2 future-clock: 5-min-future tool timestamp is clobbered
F5 cooldown: 10 rapid-fire turns → events.turns === 1
server/hooks/{react,mood-react,file-type-react,name-react}.test.ts:
F4 each hook's reaction file has source: 'fallback'
server/state.test.ts:
F3 saveReaction without source defaults to 'fallback'
F3 writeStatusState does NOT clobber an existing source='tool'
reaction in reaction.$sid.json
Tests / typecheck:
bun test: 369 pass / 0 fail
bun run typecheck: clean
Branch state: rebased onto origin/fix/react-channel (e38ef95).
F6 statusline-only framing from a5d2bd1 is preserved verbatim —
the maintainer's directive (reactions are statusline-only, not
echoed in the transcript) is intact across the changes in this
commit.
Refs #124, #154.
Review round complete — all findings addressedVerified by reading the code, not the worker's report.
Note on processAn earlier fix commit for F1–F5 was silently lost from this branch and the suite stayed green with all three critical bugs present — because no test covered long turns, clock skew, or the source round trip. Those tests now exist and were written to fail first. A green suite on this PR previously proved nothing; it does now. 🤖 This content was generated with AI assistance using Claude Opus 5. |
Fixes #154. Addresses #124. Supersedes #127.
Problem
Reactions on Claude Code came through an invisible HTML comment (
<!-- buddy: ... -->) that the Stop hook extracted. Two failures:buddy_reactMCP tool existed and was reachable, but the instructions explicitly said "Do NOT use buddy_react for end-of-turn comments".Change
Three-source chain, in order:
source: "tool"reaction is already on disk (≤60s)<!-- buddy: -->extraction → taggedsource: "comment"(kept for older Claude Code)source: "fallback"buddy_reactis now the documented primary path: it is a real tool call, renders nothing in the transcript, and is inherently model-authored.ReactionSourceis persisted andbuddy_statsshows it, so it is possible to prove a reaction was model-written rather than canned.On #127
Rejected as-is, with its good idea kept. #127 leaves the HTML comment primary and adds a canned-pool branch when no comment is found — that hides the visible marker but trades away model-authored quality, which is the more valuable half. The stop-hook canned fallback is worth keeping, so it is here, demoted to last resort.
Verification
bun test364 pass / 0 fail;bun run typecheckclean. Three-source-chain coverage added toserver/hooks/buddy-comment.test.ts.Needs human verification before release
No agent can confirm Claude Code actually emits
buddy_reactunder the new instructions — that requires a live session with the package installed. Post-merge check:/buddy statsshould reportSource: tool, notfallback.🤖 This content was generated with AI assistance using Claude Opus 5.
Note
Make
buddy_reactthe primary end-of-turn reaction channel with provenance trackingbuddy_reactMCP tool call as the primary end-of-turn reaction mechanism; the Stop hook now acts as a fallback only.sourcefield ("tool","comment","fallback", or"none") written atomically via tmp-file rename inserver/state.ts.server/hooks/buddy-comment.tspreserves tool-authored reactions written in the same turn, falls back to a canned pool when no comment is present, and rejects implausibly future-dated tool timestamps.server/index.tsinstructions and thebuddy_promptresource are rewritten to direct models to callbuddy_reactinstead of appending HTML comments.writeStatusStateno longer implicitly callssaveReaction, so status writes no longer clobber existing reaction provenance or content.Macroscope summarized be30027.