Skip to content

fix(brains): run the brains startup hook on Claude Code session fork [BRNS-MCPWEB-023] - #25

Merged
olegshmuelov merged 1 commit into
mainfrom
fix/mcpweb-023-fork-hook
Aug 9, 2026
Merged

fix(brains): run the brains startup hook on Claude Code session fork [BRNS-MCPWEB-023]#25
olegshmuelov merged 1 commit into
mainfrom
fix/mcpweb-023-fork-hook

Conversation

@olegshmuelov

Copy link
Copy Markdown
Contributor

Claude Code fires SessionStart with source=fork on --fork-session, and the Claude map's matcher excluded it — so a forked session ran no brains startup hook at all: no startup inbox pass, no device report, no update nudge. core.md itself usually survived, since a fork copies the parent's records and the CLI drops a redundant re-injection, which is what kept the gap quiet. This is the last missing source value: the matcher now covers the CLI's complete declared set.

What changed

  • plugins/brains/hooks/claude-hooks.json — SessionStart matcher gains fork.
  • tests/plugin-contract/run.ts — the pinned matcher constant moves in lockstep, plus one new configuration-pin assert recording why the Claude and Codex matchers now differ: Codex declares four SessionStart sources and no fork — a forked Codex thread arrives as startup or resume, both already matched. Codex forks have always run the hook; this change makes the two clients converge.
  • Both manifests 2.8.1 → 2.8.2.

Verification

  • All three suites green; artifact guard OK. Mutation-proved both guards: the JSON-only edit fails the contract test at the matcher pin, and adding fork to the Codex side fails the new assert on its own message.
  • Live A/B on real forked sessions (isolated, allow-listed env): unedited tree fires zero SessionStart hooks on fork; edited tree fires exactly one, whose stdout is the core block — delivered when the parent lacks it, deduped by the CLI when the parent already carries it. A --resume control on the unedited tree proves the harness detects firings.
  • Claude 2.1.220 (CI's pin) verified identical on the source enum, firing site, and matcher semantics.

Blast radius: no new command or payload — the existing startup hook runs on one more documented source value; side effects on fork are the same set that already runs on startup/resume/clear/compact.

Ticket: BRNS-MCPWEB-023

Claude Code fires SessionStart with source=fork on --fork-session, and the
Claude map's matcher excluded it, so a forked session ran no brains startup
hook at all — no inbox pass, no device report, no update nudge. core.md
itself usually survived, because a fork copies the parent's records and the
CLI drops a redundant re-injection, which is what kept this quiet.

The Codex map already covers this: Codex declares four SessionStart sources
and no fork, and a forked Codex thread arrives as startup or resume, both
already matched. So Codex forks have always run the hook and this change
makes the two clients converge. The contract test pins that asymmetry as a
deliberate configuration fact rather than leaving it to read as drift.

@nir-ssvlabs nir-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Four lines again, but the interesting part is that this PR diverges the two matchers where #24 converged them — so I checked whether that's principled or drift, and it's principled.

Delivery ladder is clean. #24 merged at 2.8.1, main is there now, and this goes to 2.8.2 on both manifests — forward and aligned. The three-PR sequence (2.7.12.8.02.8.12.8.2) has stayed strictly monotonic throughout.

The new assert is the right shape, and it earns its place. It is a negative pin — Codex's SessionStart matcher must NOT contain fork — and it exists to stop precisely the change #24 made: a future "the matchers differ, let's converge them" tidy-up. Without it, someone adds fork to the Codex side and pins a value that client never emits, and nothing fails. Framing it in the comment as a configuration pin rather than a runtime claim about Codex is the honest version, and it executes at top level in the contract run, so it isn't a guard that only its own test reaches.

A correction to my own review of #24, since it's on the record and wrong. There I wrote that brains-start.sh is "50 lines with zero network calls" and that the entire effect of the compact change was "re-emitting two files' text — no request, no state mutation". That is not right. Line 48 delegates to lib/brains-inbox.sh ([ -x "$LIB" ] && "$LIB" startup "$SESSION"), which carries four curl calls and hits devices, inbox/claude and ack. I grepped the file for curl and stopped at the file boundary instead of following the delegation. So compact — and now fork — genuinely runs the device report, the inbox pull and the ack, not just two cats. This PR's description is more accurate than my #24 review was, and it names those three effects correctly as what a forked session was missing.

With that corrected, the blast radius here is still right: a fork is a new session, so reporting the device and pulling the inbox is the behaviour you want, and it is the same set already running on startup|resume|clear|compact rather than anything new. The A/B (zero firings unedited, exactly one edited, with a --resume control proving the harness detects firings) is the right evidence for a hook-firing change — a control that proves the detector works is the part most A/Bs skip.

Coverage, stated plainly: I verified the version ladder, both matchers, the new assert's shape and that it runs, and the delegation chain out of brains-start.sh. I could not verify the external claim that Codex's CLI declares no fork source — codex-rs/hooks/src/events/session_start.rs is not in this repo, so that rests on your inspection, not mine. The assert is written to be true regardless (it pins our config, not their runtime), which is why that gap doesn't change the verdict.

Merge: ✅ ready — BLOCKED is only the missing approval (REVIEW_REQUIRED, 0 unresolved threads, all suites + artifact guard green); this clears it.

@olegshmuelov
olegshmuelov merged commit 9bc75b5 into main Aug 9, 2026
4 checks passed
@olegshmuelov
olegshmuelov deleted the fix/mcpweb-023-fork-hook branch August 9, 2026 14:57
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.

2 participants