Skip to content

Fix the Claude Code hooks so they actually work (#327) - #331

Merged
rahilp merged 3 commits into
327-claude-code-hooksfrom
fix/327-hooks-client
Sep 3, 2026
Merged

Fix the Claude Code hooks so they actually work (#327)#331
rahilp merged 3 commits into
327-claude-code-hooksfrom
fix/327-hooks-client

Conversation

@rahilp

@rahilp rahilp commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes the client half of #327. Base is 327-claude-code-hooks, the issue branch off feat/v3-team-edition.

What was broken

The hooks under integrations/claude-code-hooks/ have never worked end to end since they shipped. Three defects, only two of them reported:

  1. Recall never ran. session-start.js sent ?q=; GET /recall reads query and answers 400, and the script's own if (!res.ok) return swallowed it. Fixed on this branch already, but it never reached a release.
  2. Sessions were never captured. session-end.js parsed stdin as the transcript. Claude Code sends {session_id, transcript_path, cwd, hook_event_name, reason} and keeps the conversation in a JSONL file at transcript_path, so messages was always empty and the script returned before /capture.
  3. Capture could not have finished anyway. install.sh set no timeout, and Claude Code gives SessionEnd hooks a shared 1.5 s budget raised only to the largest per-hook timeout. POST /capture awaits an embedding, a Vectorize query and often a model call before replying.

Two more found while fixing those:

  1. Every failure was invisible. Both hooks exited 0, and Claude Code discards stderr from an exit-0 hook. A rotated token looked exactly like an empty brain — the same class of bug as Welcome to second-brain-cloudflare Discussions! #1.
  2. The installer could destroy settings. A settings.json with a comment or trailing comma was swallowed into {} and overwritten, taking permissions and env with it. Re-running after deleting the marker appended a second copy of both hooks rather than upgrading them.

What this does

  • session-end.js reads the hook payload, then walks the JSONL backwards in 64 KB blocks (1 MB ceiling) until it has three human turns, splitting on the newline byte before decoding so a block boundary can never cut a character. Keeps user/assistant text blocks only — no tool_result, tool_use, thinking, isMeta, isSidechain, isCompactSummary, or harness markup. Leads with a session — repo@branch — date (reason) header, reserves the human's request and the outcome, then fills newest-first inside a 2000-char cap.
  • session-start.js asks for the project tag first and falls back to free text, scoped to the personal layer, and frames the result as data rather than instructions.
  • Failures are visible: one stderr line and exit 1. Exit 0 is reserved for "nothing to do".
  • Credentials move to ~/.config/second-brain/config.json (mode 600) — the file the CLI and desktop app already share. The hook command is now plain node <path>, which also fixes Windows, where the old VAR=x node … form is a PowerShell syntax error, and keeps the token out of settings.json and out of ps.
  • install.sh reconciles by command substring instead of appending, refuses a malformed settings.json (with a backup and a temp-file write), sets timeout: 30 on SessionEnd and matcher: startup|clear|compact on SessionStart, never prompts without a TTY, and gains --check and --uninstall.
  • Capture is gated: it needs a Worker on 3.x (cached /health check) and a real conversation — one human turn of 40+ characters and 200+ characters of talk.

Testing

The old unit test re-implemented the hooks' helpers inside the test file and asserted on the copies, which is why ?q= survived for the life of the feature. The new tests import the real modules and run the real scripts.

  • test/unit/claude-code-hooks.test.ts — 34 cases against the shipped code: block boundaries at 7/64/100/333/1024 bytes, mid-character splits, truncated trailing lines (the file is written asynchronously), the byte ceiling, every observed noise prefix, the gate, and the framing.
  • test/integration/claude-code-hooks-contract.test.ts — spawns both scripts with the stdin Claude Code actually sends, records the requests, and replays each one through the real Worker. Covers the 401 path, the tag-to-free-text fallback, source skipping, the capability gate, and a slow server.
  • test/integration/claude-code-hooks-install.test.ts — runs the installer against a temp HOME: upgrade in place, idempotency, malformed-file refusal, no-TTY exit, --uninstall, and a check that the real home directory is untouched.
  • integrations/claude-code-hooks/fixtures/sample-transcript.jsonl — a scrubbed real transcript carrying every shape, because a synthetic fixture is how the previous test enshrined the bug.

Full suite green (3240 tests), tsc --noEmit clean, check:scope clean. Verified end to end against a stub Worker and against the real multi-megabyte transcripts on this machine: a 3.7 MB session parses in 9 ms with no tool output, secrets, or markup in the captured body.

Review notes

Two defects were found by testing the shipped code rather than the friendly cases, and are fixed in their own commits:

  • A memory whose text contained the frame's closing delimiter printed a second convincing one, so everything it said after that read as though the search results had ended. Dash runs are now folded; the delimiters are unforgeable.
  • On a real agentic transcript the captured body was eight assistant progress reports and no user turn at all — the budget filled newest-first and dropped the very turns the reader had walked back to find. The request and the outcome are now reserved first.

🤖 Generated with Claude Code

rahilp and others added 3 commits September 3, 2026 15:23
Neither hook has ever worked end to end, and nothing failed anywhere,
because the tests re-implemented the hooks' helpers inside the test file
and asserted on the copies.

Five defects, all fixed here:

1. session-start sent `?q=`; GET /recall reads `query` and answers 400
   without it. Already corrected on v3; kept and now covered by a test
   that replays the script's real URL against the real route.
2. session-end parsed stdin as `{messages}`. Claude Code sends hook
   metadata with a `transcript_path`; the conversation is JSONL on disk.
   The hook now reads that file backwards in blocks until it has three
   human turns (1 MB ceiling), splitting on the newline BYTE so a block
   boundary can cut a line but never a character, and keeps only
   human-readable turns: tool_use, tool_result, thinking, sidechain,
   isMeta, compaction summaries and harness noise prefixes are dropped.
3. No hook `timeout`. SessionEnd hooks share a 1.5 s budget raised only
   to the largest per-hook timeout, and POST /capture waits on an
   embedding and often a model call. The installer now writes
   `timeout: 30`.
4. Both hooks exited 0 on failure and Claude Code drops stderr from an
   exit-0 hook, so every failure looked like an empty brain. Any HTTP or
   network failure is now one stderr line and exit 1; exit 0 is reserved
   for "nothing to do".
5. install.sh swallowed a settings.json parse error into `{}` and then
   overwrote the file, and re-running after removing its marker appended
   duplicate hooks. It now reconciles — its own entries are replaced by
   command-path match regardless of checkout path or the old
   `VAR=x node …` form, everything else is preserved, a malformed file
   is refused, and a backup is written before the atomic rename.

Shape of the fix: shared logic lives in common.js so the tests can
require the real code rather than a mirror, which is what let #327 hide.
Credentials move to ~/.config/second-brain/config.json (the file the CLI
and desktop app already use) so the token is not in settings.json and
not in `ps`. Capture is on by default but gated on content — one human
turn of 40+ chars and 200+ chars of conversation — and on the Worker
reporting version 3.0+ via a 24 h-cached /health check, so v3 scripts
against an un-redeployed Worker say so once a day instead of failing on
every session close.

New: fixtures/sample-transcript.jsonl (every line shape the reader must
handle), `install.sh --check`, `install.sh --uninstall`, and a README.

One deviation from the spec: its contract test set HOME and the hook's
cwd to the same temp directory, so parseProjectName correctly reported
no project (it treats $HOME as not-a-project, as its own unit test
asserts) and the tag-arm fallback case saw one request instead of two.
Real Claude Code always runs with cwd set to a project directory below
HOME, so the harness got the project directory it was missing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The framed block only means anything if nothing inside it can counterfeit its
edges. A memory whose own text contained `----- second brain notes (end) -----`
printed a second, convincing closing line, and everything that memory said after
it read as though the search results had already ended — the exact injection the
frame exists to prevent, reintroduced by the content it wraps.

Dash runs of three or more are folded to an em dash, so a delimiter line cannot
be spelled from inside a snippet. Whitespace was already collapsed, which keeps
every memory on its own numbered line, and the two together make the structure
unforgeable: the only delimiter-shaped lines in the output are the two the hook
wrote. The words survive; only their power to look like structure is removed.

Found by testing the shipped frameOutput against a hostile memory rather than
the friendly ones the suite had.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verified against the real transcripts on this machine: a 3.7 MB agentic session
produced a memory of eight assistant progress reports and not one user turn. The
reader was right — it walks back until it has three human turns — but the
formatter then spent the whole 2000-char budget newest-first, and the tail of a
real session is almost entirely assistant narration, so the turns the reader had
worked to find were the first thing dropped. The gate passed because it counts
turns, not the body, so nothing complained.

What the person asked and how the session ended are now reserved before the
budget is spent on anything else, each with a fair share so one enormous prompt
cannot crowd out the outcome. The remaining budget still fills newest-first.

Re-measured over all nine transcripts here: every body that passes the gate now
contains a human turn, and the 3.7 MB session reads as the two requests that
drove it plus the result.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 91.12% 4734 / 5195
🔵 Statements 89.12% 5620 / 6306
🔵 Functions 91.2% 830 / 910
🔵 Branches 81.38% 3624 / 4453
File CoverageNo changed files found.
Generated in workflow #443 for commit c426a68 by the Vitest Coverage Report Action

@rahilp
rahilp merged commit 9671e2d into 327-claude-code-hooks Sep 3, 2026
3 checks passed
@rahilp
rahilp deleted the fix/327-hooks-client branch September 3, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant