fix(hooks): eliminate subprocess spawns in session-start

[SamedBilginAlternet]

This PR targets dev, not main.

Who is submitting this PR? (required)

Field	Value
Your model + version	Claude Sonnet 5 (claude-sonnet-5)
Harness + version	Claude Code 2.1.226
All plugins installed	supabase@claude-plugins-official v0.1.13 (user-scope; unrelated to this change)
Human partner who reviewed this diff	Samed Bilgin (@SamedBilginAlternet)

What problem are you trying to solve?

Issue #2081: hooks/session-start forks 6 subprocesses on every session
start for work that needs none. It's harmless on Linux/macOS, but on
Windows (where fork() is emulated) the reporter measured 2254ms as
shipped vs. 291ms with the spawns removed, roughly 2s of pure overhead on
every session start, every /clear, and every auto-compact. They
confirmed byte-identical output (same SHA-256, same length, same exit
code) across 7 runs each.

I'm not the one who hit this in a live session, and I want to be upfront
about that rather than imply otherwise: I found this by reading the issue
tracker, not from a session where the slowdown affected me directly. What
made me pick this one specifically is that the issue isn't a request for
someone else to investigate. The reporter already fully diagnosed it,
wrote a complete patch, and verified it byte-identical to the original
(SHA-256 69cde947..., which I independently reproduced myself before
touching any code, same hash). They explicitly said they didn't open a PR
themselves only because of one open question about a minor error-path
behavior change, and invited whoever picked it up to send one. This PR
implements their patch, with one adjustment (see below), plus my own
independent verification of every claim rather than taking the issue at
its word.

What does this PR change?

Implements the three-part patch from the issue: (1) uses
CLAUDE_PLUGIN_ROOT/CURSOR_PLUGIN_ROOT directly when the host sets it,
falling back to ${0%/*} parameter expansion instead of forking
dirname+cd+pwd; (2) reads SKILL.md via $(<file) instead of
forking cat; (3) drops the three printf ... | cat pipes, which don't
do anything printf alone doesn't already do, and inlines the JSON-escape
substitutions on the target variable instead of capturing them through a
function call, removing that fork too. strace -f -e trace=clone: 8
clone() calls per invocation down to 0.

Also adds two cases to tests/hooks/test-session-start.sh covering the
two disclosed behavior changes below (missing-SKILL.md fallback text,
and CLAUDE_PLUGIN_ROOT being honored over $0), so a future edit that
silently reverts either one fails CI instead of only being caught by
someone re-reading this PR body.

Is this change appropriate for the core library?

Yes. hooks/session-start is core plugin infrastructure that runs for
every user of every harness on every session start. It isn't
project-specific, domain-specific, or a third-party integration.

What alternatives did you consider?

I want to be honest about the actual sequence here rather than presenting
a clean narrative. I initially wrote my own, more conservative version of
this fix: the ${0%/*} path-resolution piece and dropping | cat, but I
left the SKILL.md read as $(cat ...) untouched, since removing that
fork changes the error-path semantics and I wasn't sure it was worth the
risk. I also tried converting the JSON-escape function into a nameref
out-parameter to remove its fork, but reverted that because ShellCheck
(SC2154) can't see the nameref assignment and flags the target variable
as unassigned. This repo's shell scripts have zero existing shellcheck disable comments, and I didn't want to be the first PR to add one for a
small gain.

Only after drafting that did I read the rest of the issue body (past the
initial repro/measurement) and find that the reporter had already solved
both problems I was hedging on: $(<file) reads the file without forking
cat and is still a command substitution, so it still strips trailing
newlines the same way $(cat ...) does; and inlining the substitutions
directly onto the target variable removes the escape function's fork
without needing a nameref at all. Their version is strictly better than
what I'd written (it gets to zero forks instead of three, and it doesn't
need a lint suppression), so I adopted it instead of my own draft.

The one place I kept their patch exactly as proposed rather than picking
the alternative they offered: they flagged that [ -r "$skill_file" ]
changes the failure path (the original's cat ... 2>&1 || echo ... folds
cat's own stderr text into the injected content on a partial failure;
the guard falls back to a clean fixed string instead) and asked
maintainers which behavior they wanted before opening a PR themselves. I
did not see a response to that question anywhere in the issue thread, so
I'm not resolving it unilaterally. I'm submitting their proposed behavior
and flagging it here for a maintainer to weigh in on, same as they did.

Does this PR contain multiple unrelated changes?

No. All three code changes (path resolution, the file read, removing
| cat) are the same fix for the same problem (unnecessary subprocess
spawns in this one script) and were designed together as one patch in the
source issue. The two added test cases cover behavior changes introduced
by that same fix, not a separate concern.

Existing PRs

• I have reviewed all open AND closed PRs for duplicates or prior art
• Related PRs: none found. Searched open+closed PRs for "session-start",
  "2081", "subprocess spawn", and "fork Windows": no PR references issue
  hooks/session-start: six subprocess spawns cost ~2s on Windows (7.7x slower; byte-identical output without them) #2081 or implements this patch. (fix(hooks): run-hook.cmd heredoc deadlocks all hooks on macOS (Homebrew bash >= 5.1) #2095 and fix(cursor): resolve session hook from plugin root #2053 touch
  session-start/cursor hook resolution but for unrelated bugs: macOS
  heredoc deadlock and Cursor plugin-root resolution, respectively.)

Environment tested

Harness	Harness version	Model	Model version/ID
Claude Code (Linux)	2.1.226	Claude Sonnet 5	claude-sonnet-5

I could not test the Windows wall-clock improvement firsthand: no Windows
environment available, and I'm relying on the issue's own measurement for
that number rather than reproducing it. What I verified myself, on Linux:

• strace -f -e trace=clone: 8 → 0 clone() calls per invocation
• SHA-256-identical output before/after across all 4 output-shape branches
  (Cursor, Claude Code, Copilot CLI, unknown-platform/SDK-standard); the
  Claude Code branch hash (69cde947...) matches the one the issue
  reporter posted independently
• The $0-has-no-slash edge case (no host env var set, script invoked as
  a bare name on PATH) still resolves correctly
• The missing-SKILL.md error path: confirmed the only difference from
  original behavior is the documented stderr-folding change above; built
  a throwaway copy of the hook with no skills/ directory and diffed
  original vs. patched output directly
• One behavior difference beyond what the issue documented, found while
  testing: the original never actually uses CLAUDE_PLUGIN_ROOT to locate
  its own files. It derives the path from $0 unconditionally and only
  reads CLAUDE_PLUGIN_ROOT later to pick the output JSON shape. So if a
  host ever set CLAUDE_PLUGIN_ROOT to something other than where the
  script actually lives, the original would silently ignore that and keep
  reading files relative to $0. This patch makes CLAUDE_PLUGIN_ROOT
  authoritative for file lookup when set, matching what the variable name
  implies it should do. I don't have evidence this divergence has caused
  a real bug, so I'm not claiming it as a second fix, just disclosing it
  as a behavior change I found, since the existing test suite always sets
  CLAUDE_PLUGIN_ROOT to match $0's real location and wouldn't catch
  this either way
• tests/hooks/test-session-start.sh: the existing 6 assertions pass
  unmodified, plus 2 new ones I added for the two disclosed behavior
  changes above (8/8 pass). I confirmed both new assertions actually fail
  against the pre-patch script before restoring the fix, so they're real
  regression coverage, not tests that would pass either way
• scripts/lint-shell.sh hooks/session-start tests/hooks/test-session-start.sh:
  clean (shellcheck + shfmt), no suppressions added
• bash -n hooks/session-start: syntax OK

Evaluation

• Initial prompt: none. This PR originates from reading issue hooks/session-start: six subprocess spawns cost ~2s on Windows (7.7x slower; byte-identical output without them) #2081, not
  from a session where I personally hit the bug.
• Eval sessions run after the change: not applicable. This is a mechanical
  subprocess-count fix, not a change to skill-behavior content, so there's
  no agent behavior to eval. Correctness was verified by
  byte-identical-output comparison (see Environment tested) rather than
  session evals.

Rigor

• If this is a skills change: N/A. This touches hooks/session-start,
  not skill content.
• This change was tested adversarially, not just on the happy path
  (missing-file error path, no-slash $0, all 4 output-shape
  branches, mismatched CLAUDE_PLUGIN_ROOT)
• I did not modify carefully-tuned content (Red Flags table,
  rationalizations, "human partner" language). This PR touches only
  shell mechanics in one hook script.

Human review

• A human has reviewed the COMPLETE proposed diff before submission

[obra]

Full review done (security + history archaeology). The perf goal is sound, the submission is careful, and the disclosed error-path change is fine — but review surfaced two functional regressions that need rework before this can land:

1. The | cat removal reverts a deliberate fix, and the PR's premise for removing them is wrong. The body says the pipes 'don't do anything printf alone doesn't already do' — but they were added on purpose in d72560e ('Pipe SessionStart hook printf through cat to absorb EPIPE on Windows'): Git Bash printf reports EPIPE as 'Permission denied' when the harness closes the hook's stdout early, and under set -euo pipefail that escapes as a confusing per-session diagnostic. Your patch removes the absorption without replacing it. The fix that keeps your perf win: handle EPIPE with zero spawns — printf ... || true (or an explicit exit-code check) on the output writes — and add a regression test that simulates the closed-stdout condition so d72560e's scenario stays covered.

2. Making CLAUDE_PLUGIN_ROOT authoritative for the SKILL.md read is a robustness regression. Today the env var only selects output shape; the content read resolves from $0 and survives env-var corruption. Issue SessionStart hook fails on WSL : CLAUDE_PLUGIN_ROOT contains Windows-style backslashes, run-hook.cmd "not found" #2091 shows harnesses delivering CLAUDE_PLUGIN_ROOT with backslash-mangled paths — under your change that degrades to the fallback string being injected every session: superpowers silently dead with no visible error. Keep $0-based resolution for the read, or at minimum fall back to it when the env-var path isn't readable, and pin that with a test using a deliberately garbage CLAUDE_PLUGIN_ROOT.

With those two corrections (both zero-subprocess, so the ~2s Windows win survives intact) and the two extra regression tests, this is in good shape. The care in the original submission — full disclosure, honest provenance, pinning your own behavior changes with tests — is exactly what we want; the gap was git history, not diligence.

— Claude Fable 5, Claude Code 2.1.228, reviewing on behalf of @obra