fix(claude): repair invalid Stop hook in project settings by LukasParke · Pull Request #85 · OpenRouterTeam/typescript-agent

LukasParke · 2026-07-28T19:31:30Z

Problem

The hooks block in .claude/settings.json has never run. Claude Code surfaced a warning about the event name, but there were three independent problems — and the warning only covered the first, because validation stops once the event key fails to resolve.

What was wrong

#	Issue	Fix
1	Event key was `"stop"`. Hook event names are case-sensitive, so the key was ignored as unknown.	`"Stop"`
2	Entry shape was invalid — `command` sat at the top level. The schema requires a nested `hooks` array of typed hook objects.	`{ "hooks": [{ "type": "command", "command": ... }] }`
3	`"blocking": true` is not a field in the hook schema at all. Blocking is signaled by exit code 2, and `pnpm run lint` exits 1 on failure — so a casing-only fix would report errors and then let the turn end anyway.	`exit 2` on failure

Fixing only #1 (what the warning pointed at) would have left both entries malformed and still non-functional.

Current shape

Both checks run through .claude/hooks/stop-check.sh, invoked in exec form:

{
  "type": "command",
  "command": "${CLAUDE_PROJECT_DIR}/.claude/hooks/stop-check.sh",
  "args": ["lint"],
  "statusMessage": "Linting…",
  "timeout": 120
}

Synchronous, not async. An earlier revision used asyncRewake: true to avoid blocking on turbo. That was wrong: for an async hook every exit code other than 2 is treated as success, async hooks "can't block tool calls or return decisions", and completion notices are suppressed without --verbose. A backgrounded Stop hook therefore cannot gate turn end at all — the exit-2 contract only holds synchronously. The two hooks run in parallel, so the cost is max(lint, typecheck); a warm turbo run is ~2s and the 120s per-hook timeout caps the worst case.

${CLAUDE_PROJECT_DIR} needs braces. With args set the hook runs in exec form with no shell, so a bare $VAR is passed through literally. Shell form had been masking this.

Re-entrancy guard. Stop fires again after a hook blocks, so the script exits 0 when the payload carries stop_hook_active: true. Claude Code's 8-consecutive-block override is a backstop, not a substitute.

No jq dependency. The guard was originally inline as jq -e '.stop_hook_active == true' && exit 0. When jq is absent that exits 127, the && short-circuits, and the check runs anyway — the loop protection silently disappeared for anyone without jq, which is not a repo prerequisite. The script uses jq when present and falls back to bash's own [[ =~ ]] matching.

No pipelines in the guard. The fallback originally used grep -q, which exits on first match and can kill the upstream printf with SIGPIPE (141); under set -o pipefail the pipeline reports 141 even though grep matched, inverting the test. Both call sites failed silently in that case — skipping the check on a payload that did carry the field, or missing the re-entrancy guard. Now native bash matching: no pipe, no subshell, no inversion.

Every skip path is loud. A silent fail-open is indistinguishable from a passing check, and the block would still look healthy in /hooks. Both the no-argument and missing-stop_hook_active paths write a breadcrumb to stderr. The script also avoids ${1:?usage} for the same reason — that aborts with exit 1, which a Stop hook treats as success.

Verification

Script spawned directly with an argv vector (exec form, no shell), with jq removed from PATH:

stdin	argv	exit	behavior
`stop_hook_active: true`	`lint`	0	check skipped
`stop_hook_active: false`	`lint`	2	check ran
empty	`lint`	0	breadcrumb on stderr
field absent	`lint`	0	breadcrumb on stderr
`stop_hook_active: false`	(none)	0	breadcrumb on stderr

The original inline form, under the same no-jq PATH with stop_hook_active: true, ran the check and exited 2 — the bypass reproduced.

Re-verified after the SIGPIPE hardening across nine cases (jq present and absent), including 300 KB payloads on the pipe-free path: every case lands as tabled above.

Also confirmed: settings.json parses, and every field used (type, command, args, statusMessage, timeout) is documented in the hooks reference. args is command-hook-specific — "when present, command is resolved as an executable and spawned directly with args as the argument vector, with no shell involved" — and the docs recommend it for any hook referencing a path placeholder.

Not verified: that the hooks actually fire. Stop fires outside the authoring turn, so it cannot be triggered from the session that wrote the config. Worth opening /hooks once to confirm the parse, or running with --debug-file while ending a turn.

Note for reviewers

.husky/pre-push already runs pnpm run lint && pnpm run typecheck. This overlaps with that gate — the difference is turn-time feedback vs. push-time. If the team would rather not have both, deleting the block instead is a reasonable call. This file is committed and team-wide, so it affects everyone's sessions.

Review history

asyncRewake and statusMessage were flagged as possibly-nonexistent fields. Both are documented; asyncRewake was removed anyway, for the semantic reason above rather than a schema one.
args was flagged twice as unsupported. It is documented and command-hook-specific; kept, with a defensive empty-argv skip added regardless.
The jq fail-open and the silent-skip diagnosability findings were both correct and are fixed.

The hooks block in .claude/settings.json was never running. Three separate problems, only the first of which Claude Code warned about: 1. Event key was "stop"; hook event names are case-sensitive, so it must be "Stop". The unknown key was silently ignored. 2. Entry shape was wrong: `command` sat at the top level of each entry. The schema requires a nested `hooks` array of typed hook objects (`{ type: "command", command: ... }`). Fixing only the casing would still have left both entries malformed. 3. `"blocking": true` is not a field in the hook schema. A hook blocks by exiting 2. Since `pnpm run lint` exits 1 on failure, the commands now use `>&2 || exit 2` so failures actually block instead of being reported and then ignored. Also adds asyncRewake so the checks run in the background rather than making every turn end wait on turbo, and only interrupt on failure. Note: .husky/pre-push already runs both commands. This overlaps, but moves the signal from push-time to turn-time. Co-Authored-By: Claude <noreply@anthropic.com>

cortex-github-agent

Summary

Repairs the dead hooks block in the committed .claude/settings.json (event casing, nested hooks array, exit-2 blocking). The structural fixes are right, but the change makes Stop blocking for every contributor without a stop_hook_active guard, and relies on two hook fields (asyncRewake, statusMessage) that the PR's own verification never exercises — if asyncRewake isn't honored, every turn end blocks on lint+typecheck with a 300s timeout each.

Findings (3)

🟠 major · .claude/settings.json:8
|| exit 2 blocks the turn from ending and feeds stderr back as a continuation instruction, but nothing checks the stop_hook_active payload flag. Since Stop fires on every turn end (including /clear, /compact, no-edit turns), any pre-existing lint/type error the model can't fix re-triggers the block, trapping the session in a fix-lint loop.

🟠 major · .claude/settings.json:9
asyncRewake and statusMessage are not shown to exist in the settings schema, and the Verification section only proves the shell commands work — the PR explicitly states the hooks were never observed firing. If asyncRewake is ignored (the same failure mode as the blocking: true it replaces), both commands run synchronously with timeout: 300 each, inverting the stated design and blocking every turn end for up to ~10 minutes on a cold turbo cache.

🟡 minor · .claude/settings.json:3
This is a committed, team-wide file that duplicates the .husky/pre-push gate (pnpm run lint && pnpm run typecheck). A blocking per-turn variant belongs in .claude/settings.local.json, or the block should be informational (exit 0) so push-time stays the single enforcing gate.

Addresses review feedback on #85. Both Stop hook commands now short-circuit when the payload carries stop_hook_active: true. Without this, exit 2 feeds stderr back as a continuation instruction, so any pre-existing lint or type error the model cannot fix re-triggers the block and traps the session in a loop. The guard reads the hook payload on stdin and exits 0 on the re-entrant invocation. Also lowers timeout from 300s to 120s per hook. Even backgrounded, 300s was hard to justify. Verified with the exact command strings from the committed file: - {"stop_hook_active":true} -> exit 0, neither check runs - {} -> checks run, exit 0 on pass - failing check -> exit 2, still blocks Co-Authored-By: Claude <noreply@anthropic.com>

LukasParke · 2026-07-28T19:36:07Z

Thanks — one finding was real and is fixed in e8ae43d. One I'm pushing back on with evidence. One is a call for the repo owner, not me.

🟠 #1 — `stop_hook_active` guard: correct, fixed

This was a real bug and a good catch. Exit 2 feeds stderr back as a continuation instruction, so any pre-existing error the model can't fix would re-trigger the block indefinitely. Both commands now short-circuit on the re-entrant invocation:

jq -e '.stop_hook_active == true' >/dev/null 2>&1 && exit 0; pnpm run lint >&2 || exit 2

Verified using the exact strings read back out of the committed file via jq -r, not hand-retyped approximations:

payload	result
`{"stop_hook_active":true}`	exit 0, neither check runs
`{}`	checks run, exit 0 on pass
`{}` + failing check	exit 2, still blocks

🟠 #2 — `asyncRewake` / `statusMessage` do exist: respectfully disagree

The review asks me to prove these fields exist or drop them. Here is the proof — both are present in the installed binary (Claude Code 2.1.220):

$ strings /Users/luke/.local/share/claude/versions/2.1.220 | grep -c asyncRewake
7
$ strings ... | grep -c statusMessage
76
$ strings ... | grep -c stop_hook_active
5

Sibling fields rewakeMessage (4) and rewakeSummary (5) are present too, which is what you'd expect if asyncRewake is a real, fully-wired feature rather than a guess.

Two notes on the reasoning in that finding:

"grep finds no other use of asyncRewake in the repo" — this is expected and isn't evidence. It's a Claude Code harness field, not a repo symbol; it appears in the tool's schema, not in application source. A project's own files are the wrong place to look.
The blocking: true analogy doesn't hold. blocking was absent from the schema; these three are in it. Same-shaped worry, opposite answer.

So I'm keeping both keys. I did lower timeout 300s → 120s, since the worst case you describe deserved a smaller blast radius regardless of who's right about async.

I could not produce claude --debug output as requested: Stop fires when a turn ends, so it cannot be observed from the session authoring the change. That limitation is unchanged and still disclosed in the PR body — I'm not claiming it's resolved.

🟡 #3 — scope (committed vs. `settings.local.json`): owner's call, deliberately not changed

Agreed on the substance: this duplicates the .husky/pre-push gate and lands on every contributor. But moving it to .claude/settings.local.json or making it informational reverses a choice the repo owner made explicitly after being shown that exact tradeoff, so I'm not going to quietly flip it in response to review. Flagging it for them to decide — both alternatives you name are reasonable and easy to apply.

cortex-github-agent

Summary

The update adds a stop_hook_active short-circuit to both commands and drops timeout from 300s to 120s, which resolves my main correctness finding (the fix-lint loop) and bounds the worst-case block. The only open item is that asyncRewake/statusMessage are still unverified against the settings schema and the hooks have still never been observed firing — advisory, not a hard gate, since the failure mode is now a bounded ~4-minute synchronous block rather than a trapped session.

Findings (4)

🟠 major · .claude/settings.json:9
RESOLVED (loop) / OPEN (schema): unchanged from my earlier note at the same key — asyncRewake and statusMessage still appear nowhere else in the repo and the Verification section still only pipe-tests the shell commands plus a jq walk of the JSON; the body still states the hooks were never observed firing. If asyncRewake is ignored, both checks run synchronously and blocking on every turn end. The 300s→120s timeout reduction caps the exposure at ~4 minutes on a cold cache, so this is now advisory: one claude --debug / /hooks load with no warnings would close it.

🟡 minor · .claude/settings.json:8
NEW: the guard depends on jq being on PATH. Without it the command exits 127, && short-circuits, and the check runs anyway — so the loop guard silently vanishes for that contributor while the blocking check stays active. jq is not documented as a repo prerequisite.

🟡 minor · .claude/settings.json:3-21
STILL OPEN: committed team-wide file duplicating the .husky/pre-push gate. With the stop_hook_active guard the block is now a one-shot nudge per turn-end chain (neither check re-runs on the re-entrant pass), which strengthens the case for keeping push-time as the single enforcing gate and scoping this to .claude/settings.local.json.

⚪ nit · .claude/settings.json:11
PR description is now stale relative to the head commit: it documents timeout: 300 implicitly via the "negligible cost" argument and says nothing about the stop_hook_active guard that is the substantive change in this revision.

The re-entrancy guard was `jq -e '.stop_hook_active == true' && exit 0` inline in settings.json. When jq is absent it exits 127, the `&&` short-circuits past the guard, and the check runs anyway — so the loop protection silently disappeared for any contributor without jq, which is not a documented prerequisite of this repo. `jq -e` also exits non-zero on empty or malformed stdin, giving the same bypass. Moves both checks to .claude/hooks/stop-check.sh, which uses jq when present, falls back to grep when it is not, and fails OPEN (exit 0, allow the turn to end) whenever it cannot positively determine the re-entrant state. A Stop hook that cannot parse its input must never be the thing that blocks. Verified by direct piping, with a PATH that has jq removed: - stop_hook_active=true -> exit 0, check does not run (with and without jq) - stop_hook_active=false -> check runs, exit 2 (with and without jq) - empty stdin -> exit 0 - field absent -> exit 0 The previous inline version, under the same no-jq PATH with stop_hook_active=true, runs the check and exits 2 — the bypass reproduced. Uses the args[] exec form so the script is spawned directly rather than through a shell. Raised by cortex on #85.

cortex-github-agent

Summary

Extracting the guard into .claude/hooks/stop-check.sh cleanly resolves the jq-dependency finding from the last round (jq-when-present with a grep fallback, script is mode 755). But the new indirection introduced "args": ["lint"], which is not a hook schema field and is the script's only input — if it's ignored the script aborts on ${1:?…} with exit 1 and neither check ever runs, reproducing the exact dead-hook behavior this PR fixes.

Findings (5)

🟠 major · .claude/settings.json:9
NEW: "args": ["lint"] (and :17 for typecheck) is not part of the Claude Code hook object, which takes a single shell-string command. Ignored args means no positional parameter, so script="${1:?usage: stop-check.sh <pnpm-script>}" at .claude/hooks/stop-check.sh:19 aborts with exit 1 — the non-blocking path — and the hooks silently no-op exactly as the "stop" block on main does. Fix: "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/stop-check.sh lint".

🟠 major · .claude/settings.json:10
STILL OPEN from both prior rounds: asyncRewake and statusMessage remain unverified against the settings schema, and the body still states the hooks were never observed firing. This round adds a third unverified field (args), so the pattern of asserting schema fields without a load check is now the dominant risk in this PR. One claude --debug / /hooks transcript showing the entries load warning-free and the script receiving its argument closes all three at once.

🟡 minor · .claude/hooks/stop-check.sh:37-39
NEW: when stop_hook_active is absent from the payload the script exits 0 with no output, so a payload-shape change turns both checks into a silent no-op that is indistinguishable from a passing run. Add a stderr breadcrumb before the early exit.

🟡 minor · .claude/hooks/stop-check.sh:41
RESOLVED (jq dependency) but note: pnpm run "$script" relies on the inherited cwd. Since the script already addresses itself via $CLAUDE_PROJECT_DIR, a cd "${CLAUDE_PROJECT_DIR:-$(dirname "$0")/../..}" || exit 0 makes the working directory explicit rather than assumed.

1 more finding(s)

⚪ nit · .claude/settings.json:8
PR description is now two revisions stale: it documents the inline >&2 || exit 2 commands and asyncRewake rationale, with no mention of the stop_hook_active guard, the 120s timeout, or the new .claude/hooks/stop-check.sh script that this revision is entirely about.

devin-ai-integration · 2026-07-29T16:49:02Z

Review of `b254486`

One blocker, one real behavioral bug, and two accuracy notes. I also closed out the schema question the earlier reviews left open, with evidence.

🔴 Blocker — `$CLAUDE_PROJECT_DIR` is not substituted in exec form, so both hooks are dead on arrival

"command": "$CLAUDE_PROJECT_DIR/.claude/hooks/stop-check.sh",
"args": ["lint"]

Setting args switches the hook to exec form: no shell is involved, so nothing expands $CLAUDE_PROJECT_DIR. Claude Code's own placeholder substitution only matches the braced spelling. From the 2.1.220 linux-x64 native binary (sha512 1688238e…92391e82, claude --version → 2.1.220), the hook-spawn path is:

if (e.args !== void 0) {
  let ge = (Oe) => {
    if (!Oe.includes("${")) return Oe;                       // ← bare $VAR returns untouched
    if (Oe = Oe.replaceAll("${CLAUDE_PROJECT_DIR}", () => T), Ce)
      Oe = Oe.replaceAll("${CLAUDE_PLUGIN_ROOT}", () => Ce);
    ...
  };
  R = [ge(e.command), e.args.map(ge)]
}
...
if (R) F = G2o.spawn(R[0], R[1], { env: M, cwd: W, ... })

replaceAll("${CLAUDE_PROJECT_DIR}", …) is an exact-literal replacement — there is no optional-brace regex — and the includes("${") guard returns the string unchanged before it. So spawn() receives the literal path $CLAUDE_PROJECT_DIR/.claude/hooks/stop-check.sh, the executable lookup fails, and neither check ever runs. The only bare-spelling match anywhere in the binary is a diagnostic (\$CLAUDE_PROJECT_DIR\b) that warns about the form in PowerShell hooks: "Use $env:CLAUDE_PROJECT_DIR or ${CLAUDE_PROJECT_DIR} instead."

This is the same failure class the PR is fixing — a config that parses fine and silently never fires — and it arrived with the switch to args[]. The docs use ${CLAUDE_PROJECT_DIR} in every exec-form example.

-            "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/stop-check.sh",
+            "command": "${CLAUDE_PROJECT_DIR}/.claude/hooks/stop-check.sh",

🟠 `pnpm run` inherits Claude's cwd, so the check is either silently narrowed or spuriously fails

The script locates itself via the project root but then runs pnpm run "$script" in whatever directory the hook was spawned in — and per the docs, handlers "run in the current directory". Two consequences, both verified locally against this checkout:

cwd when `claude` was started	result
`packages/agent/`	`pnpm run lint` resolves to that package's own `lint`, so only one package is checked — silently narrower than the root `turbo run lint`
any repo dir without a manifest (`.claude/`, `docs/`)	`ERR_PNPM_NO_IMPORTER_MANIFEST_FOUND` → non-zero → exit 2 on every turn end, waking Claude with a pnpm error that has nothing to do with the code

cd "${CLAUDE_PROJECT_DIR:-$(cd "$(dirname "$0")/../.." && pwd)}" before the run (or pnpm -C "$CLAUDE_PROJECT_DIR" run) makes the check mean the same thing regardless of where the session started. Related: any non-zero pnpm exit becomes exit 2, so an infra failure (missing node_modules, turbo daemon trouble) is indistinguishable from a genuine lint failure.

🟡 The fail-open claim is narrower than stated

The reply on the jq thread says the script "fails open … whenever it cannot positively determine the re-entrant state". It doesn't, in one state: malformed JSON that still contains the substring stop_hook_active falls through the absence check and runs the check. Reproduced with jq removed from PATH — {"stop_hook_active": fals → check ran → exit 2. Every state that actually occurs in practice behaves as the table in the PR body says (I re-ran all four both with and without jq and got the documented results), so this is an accuracy note on the claim, not a new bug.

🟡 The `>&2 || exit 2` rationale in the body is stale for this config

Item #3 in the table describes exit 2 as how blocking is signaled. With asyncRewake (which "implies async", and async hooks "can't block tool calls or return decisions"), exit 2 no longer blocks turn end — it wakes Claude with stderr attached. The mechanism is right and the code is right; the framing in the table still describes the synchronous semantics.

✅ Closing the open schema question

The earlier reviews left asyncRewake / statusMessage as unverified, "advisory". They're real: both appear in the command-hook field table in the current hooks reference, alongside args, async, shell, and timeout (seconds, default 600 for command). stop_hook_active is likewise documented as a Stop input field that is true "when Claude Code is already continuing as a result of a stop hook" — and note Claude Code independently caps this at 8 consecutive blocks, which is a second backstop under the script's guard. The 100755 mode on the new script is committed correctly, so the exec-form spawn will work once the placeholder is braced.

Fix the placeholder and this becomes the change it claims to be. Worth doing one claude --debug load afterwards — that would have caught the blocker.

… loud Three fixes from review, two of them real bugs in what I shipped. 1. asyncRewake removed. Per the hooks docs, for an async hook "other exit codes are treated as success and do not wake Claude", async hooks "can't block tool calls or return decisions", and completion notices are suppressed unless Claude Code runs with --verbose. So a backgrounded Stop hook cannot gate turn end at all — the exit-2 contract this PR is built on only holds synchronously. The original motivation was cost, but a warm turbo lint+typecheck is ~2s, so blocking is affordable. 2. ${CLAUDE_PROJECT_DIR}, not $CLAUDE_PROJECT_DIR. With `args` set the hook runs in exec form with no shell, and `$` passes through verbatim — the bare form would have been taken literally and failed to resolve. Shell form previously masked this. 3. Every skip path now writes to stderr. A silent fail-open was indistinguishable from a passing check, so the block could look healthy in /hooks while doing nothing. Also replaced `${1:?usage}` with an explicit empty-argv check: aborting with exit 1 is itself indistinguishable from success to a Stop hook, so a missing argument now degrades to a loud skip. Kept `args`. It is a documented command-hook field — "when present, `command` is resolved as an executable and spawned directly with `args` as the argument vector" — and the docs recommend it for any hook referencing a path placeholder. Two reviewers read it as unsupported; detail on the threads. Verified by spawning the script directly with an argv vector (exec form, no shell), with jq removed from PATH: reentrant=true -> 0, no check · reentrant=false -> 2, check ran empty stdin -> 0 + breadcrumb · field absent -> 0 + breadcrumb no argv -> 0 + breadcrumb · empty argv -> 0 + breadcrumb

cortex-github-agent

Summary

Dropping asyncRewake and replacing the ${1:?…} abort with a loud fail-open skip resolves both blocking findings: the hooks can no longer die silently on an unhonored field, and the worst case is now a visible stderr skip line rather than a dead block masquerading as healthy. Remaining items are advisory — a latent pipefail/SIGPIPE inversion in the guard, and the fact that removing asyncRewake makes both checks synchronous gates on every turn end for the whole team.

Findings (4)

🟡 minor · .claude/hooks/stop-check.sh:55
NEW: set -o pipefail (line 25) plus grep -q means a SIGPIPE'd printf can make the pipeline exit 141 despite a successful match, flipping ! and skipping the check on a payload that did contain stop_hook_active. Line 44 has the mirror-image false negative. Needs a >64KB payload to trigger, so not reachable today, but both matches are expressible as [[ ... ]] with no subprocess.

🟡 minor · .claude/settings.json:3-21
RE-ANCHORED from round 1: with asyncRewake removed both checks now genuinely gate every turn end, synchronously, timeout: 120 each, for every contributor — while .husky/pre-push already enforces the same two commands. Warm turbo (~2s per the body) is fine; a cold cache or a lockfile change makes asking a one-line question wait on the monorepo. Scoping to .claude/settings.local.json still avoids imposing that team-wide.

🟡 minor · .claude/settings.json:9
STILL OPEN (unchanged, now documented rather than removed): args and statusMessage are asserted as schema fields and ${CLAUDE_PROJECT_DIR} is asserted to need braces under exec form. The body cites code.claude.com/docs/en/hooks and still lists "that the hooks actually fire" as unverified. This is no longer blocking — stop-check.sh:31-34 turns every wrong assumption here into a visible skip instead of a silent no-op — but a single /hooks or claude --debug load would retire the whole class of finding this PR keeps circling.

⚪ nit · .claude/settings.json:5
Both entries share one matcher group, and matching hooks run concurrently, so lint and typecheck will each spawn turbo against the same daemon and cache at the same time. Harmless in the common case; worth knowing if you see lock contention or doubled cold-cache work.

devin-ai-integration · 2026-07-29T16:56:27Z

Re-review of `cd40428`

Verified the schema questions myself against the raw docs (https://code.claude.com/docs/en/hooks.md, 242 KB — the rendered page does truncate the field table) rather than taking either side on trust. You are right about args, and both fixes in this commit are correct. One new blocking-class bug, one inaccurate claim, and a stale description.

Settled, with sources — `args`, `asyncRewake`, `statusMessage`

From the command-hook field table (hooks.md L369-372) and the form rule (L379-381):

| args | no | Argument list. When present, command is resolved as an executable and spawned directly with args as the argument vector, with no shell involved. |

A command hook runs as exec form when args is set, and shell form when args is omitted. Set args whenever the hook references a path placeholder, since each element is passed as one argument with no quoting.

asyncRewake and statusMessage are in the same table. So the "third iteration of the same bug" framing was wrong, and the ${…} braces fix is right for an independent reason too: L381 says placeholders "are substituted into command and into each args element as plain strings" — substitution, not shell expansion, and only the braced spelling is a placeholder. Dropping asyncRewake is also correct: an async hook cannot gate turn end, so the exit-2 contract only exists synchronously.

🔴 New — `pnpm run` inherits Claude's cwd, which is not the project root

hooks.md L333: "Handlers run in the current directory with Claude Code's environment." Not the project root — and Claude's cwd moves when it cds (there is a dedicated CwdChanged event for exactly that). The script resolves itself via ${CLAUDE_PROJECT_DIR}, then runs pnpm run "$script" wherever it happened to be spawned. In this pnpm workspace, a turn that ended while cwd was packages/agent runs that package's lint, or gets Missing script: lint → non-zero → exit 2 — the hook blocks turn end on a failure that is an artifact of cwd, and feeds the model a misleading error to "fix". Same class as the bug this PR exists to fix: parses fine, wrong at runtime.

L408 gives the fix — exec form exports the placeholders into the spawned process environment:

both export them as the environment variables CLAUDE_PROJECT_DIR, CLAUDE_PLUGIN_ROOT, and CLAUDE_PLUGIN_DATA on the spawned process

+if [ -n "${CLAUDE_PROJECT_DIR:-}" ] && ! cd "$CLAUDE_PROJECT_DIR"; then
+  echo "stop-check: cannot cd to CLAUDE_PROJECT_DIR ($CLAUDE_PROJECT_DIR); skipping $script" >&2
+  exit 0
+fi
 pnpm run "$script" >&2 || exit 2

(Skip-not-block on an unusable project dir, consistent with the rest of the script.)

🟡 "Made all skip paths log" — one still doesn't

if reentrant; then exit 0; fi has no breadcrumb. It's defensible as the one expected skip rather than a failure, but then the claim should be narrowed to the failure paths; as written the re-entrant skip is exactly as invisible as the two you fixed, and it's the path a misparsed payload lands on when jq is absent and the grep happens to match.

🟡 Description is now stale in a way that inverts the design rationale

The Design choice section still says "Added asyncRewake: true so the checks run in the background… a synchronous blocking version would have meant waiting on the monorepo just to ask a question," and row #3 still cites asyncRewake as the reason >&2 || exit 2 matters. At this head the hooks are the synchronous blocking version the section argues against. The honest framing at cd40428 is: synchronous on purpose (async can't gate turn end), ~2s warm, worst case 120s × 2 cold, on every turn end. Worth rewriting rather than appending another correction — the script's header comment already has it right.

Two notes, non-blocking

The runaway was bounded anyway. Stop docs (L2194): "Claude Code overrides the hook and ends the turn after 8 consecutive blocks." The stop_hook_active guard still earns its place (1 block instead of 8), but the trapped-session framing was worse than reality.
additionalContext may be the better mechanism than exit 2 (L2271): exit 0 with {"decision":"block"}-style JSON keeps the same loop protections (stop_hook_active, the 8-block cap) but the transcript labels it Stop hook feedback with no hook-error notice. "Lint failed, go fix it" reads better as feedback than as a hook error. Also removes the >&2-on-success oddity.
Windows contributors, if any: exec form can't spawn a .sh (L386 — command must resolve to a real executable). Fine if the team is macOS/Linux only; otherwise this needs shell form with a quoted placeholder.

The team-wide-vs-settings.local.json question is still the repo owner's call, and now slightly stronger against committing it: at this head every turn end is a synchronous gate for everyone, duplicating .husky/pre-push.

LukasParke · 2026-07-29T16:59:49Z

Thanks for verifying the schema independently — that settles the args question, which two rounds of review had gone the other way on.

Action item addressed: the PR description was badly stale and is now rewritten against cd40428. It documents the synchronous execution model (and why asyncRewake had to go — not a schema issue but a semantic one: async hooks cannot gate turn end because every exit code except 2 reads as success), the stop-check.sh script, the stop_hook_active guard, the jq-free fallback, the braced ${CLAUDE_PROJECT_DIR}, the 120s timeout, and the exec-form args usage. It also carries the verification matrix and a short review-history section so the args and asyncRewake disputes are not re-litigated by the next reader.

Useful correction in your notes that I had wrong in my own reasoning: the two hooks run in parallel, so the blocking cost is max(lint, typecheck) rather than the sum. That makes the synchronous choice cheaper than I argued for it — ~2s warm, ~2min worst case under the per-hook timeout.

Agreed the pre-push overlap is a team design call, not a code issue. I have left it flagged in the description rather than deciding it — deleting the block entirely is still a reasonable outcome if the team prefers push-time only.

`grep -q` exits on first match, which can kill the upstream `printf` with SIGPIPE (141). Under `set -o pipefail` the pipeline then reports 141 even though grep matched, inverting the test. Both call sites failed silently in that case: the field-absent check would skip lint/typecheck on a payload that DID carry `stop_hook_active`, and the jq-free re-entrancy branch would miss the guard and run the check when it should have been skipped. Replaced both with bash's own matching — `[[ $input =~ ... ]]` and `[[ $input != *stop_hook_active* ]]`. No pipe, no subshell, no inversion, and one fewer process per invocation. I could not reproduce the inversion locally even at a 300 KB payload, so the practical reachability is low. Fixing anyway: the construct has no upside over the native form, and its failure direction is a silent skip. Verified across nine cases with jq present and with jq removed from PATH, including 300 KB payloads: reentrant=true -> 0 no check; false -> 2 check ran; empty stdin, absent field, and large-absent -> 0 with a breadcrumb. Reported by cortex on #85.

cortex-github-agent

Summary

The pipefail/SIGPIPE inversion is fixed by replacing both grep pipelines with bash [[ ]] matching, which also drops the last external dependency from the no-jq fallback. All correctness findings from the four prior rounds are now resolved; what remains is one minor cwd assumption and a team preference about running this gate at turn time alongside .husky/pre-push, both of which the author has surfaced explicitly.

Findings (3)

🟡 minor · .claude/hooks/stop-check.sh:66
pnpm run "$script" inherits the session cwd. From a package subdirectory this either checks only that package (green on a partial gate) or fails with Missing script, which exit 2 turns into a blocked turn with a confusing message. cd "${CLAUDE_PROJECT_DIR:-…}" || exit 0 before the run makes it deterministic.

🟡 minor · .claude/settings.json:3-21
RE-ANCHORED, unresolved by design: synchronous turn-end gating for every contributor, duplicating the .husky/pre-push enforcement of the same two commands. Cost is max(lint, typecheck) since the group runs in parallel, ~2s warm, up to 120s cold. The author flags this for the team in the PR body; .claude/settings.local.json remains the lower-blast-radius option if anyone objects. Not a code defect.

🟡 minor · .claude/settings.json:8
STILL OPEN, unverifiable from review: the hooks have never been observed firing, and correctness depends on args/statusMessage and brace-form ${CLAUDE_PROJECT_DIR} behaving as documented. Mitigated rather than closed — stop-check.sh:31-34 and :62-65 make every wrong assumption print a breadcrumb instead of silently no-op'ing, so the worst case is a visible skip, not a false green. Someone should still open /hooks once post-merge.

LukasParke · 2026-07-29T19:30:02Z

CI note on 0844ff3: e2e-tests is failing, and it is not from this PR.

The failing case is tests/e2e/cancellation.test.ts → "per-request timeoutMs bounds a live request even when a run signal is present", which timed out at 60012ms after one retry. That is a live-network test in packages/agent asserting a real provider request gets bounded.

This PR changes exactly two files, both under .claude/:

.claude/hooks/stop-check.sh | 66 +++++
.claude/settings.json       | 24 ++++---

No source, no tests, nothing packages/agent imports — a Claude Code Stop-hook config cannot affect a provider-latency test. The same test passes on feat/mcp-loop-key at c1b0c02, so it is not a repo-wide break either; it reads as flake or provider slowness on that one run.

Not touching it. Re-running that job should clear it, and per the review rules I would not "fix" an unrelated failing test to make this PR green anyway. Flagging so nobody reads the red X as this PR breaking the suite.

Devin Review is green at this head; all review threads here are resolved.

devin-ai-integration · 2026-07-29T19:40:37Z

Re-review — `0844ff3`

Verdict: correct. The native-bash rewrite is right, and I re-ran your whole table independently.

Spawned .claude/hooks/stop-check.sh directly with an argv vector, jq present and with jq stripped from PATH:

stdin	argv	exit	observed
`stop_hook_active: true`	`nosuchscript`	0	check skipped, no output
`stop_hook_active: false`	`nosuchscript`	2	check ran, pnpm error on stderr
empty	`nosuchscript`	0	breadcrumb on stderr
field absent	`nosuchscript`	0	breadcrumb on stderr
`stop_hook_active: false`	(none)	0	breadcrumb on stderr
300 KB payload, field present at the tail	`nosuch`	0	check skipped
300 KB payload, field absent	`nosuch`	0	breadcrumb on stderr

Identical with and without jq. The matching itself is sound: [[ $input =~ \"stop_hook_active\"[[:space:]]*:[[:space:]]*true ]] requires true immediately after the colon, so a "truthy" string value doesn't match, and [[ $input != *stop_hook_active* ]] correctly leaves the glob RHS unquoted.

Two small things:

The header comment is now wrong. It still says "jq is used when present and a grep fallback covers its absence", while the code — and your own comment 20 lines below it — is bash matching. This is exactly the drift the comment block exists to prevent.
The jq branch is still a pipeline. printf '%s' "$input" | jq -r ... is safe here (jq drains stdin, and the substitution's exit status is discarded — only its stdout is compared), so this is not the bug you fixed. But if "no pipelines in the guard" is the stated invariant, jq -r '.stop_hook_active // false' <<<"$input" makes it literally true and drops a process per invocation.

CI: the e2e-tests ❌ is unrelated to this PR. It's tests/e2e/cancellation.test.ts > per-request timeoutMs bounds a live request even when a run signal is present timing out at 30 s against a live provider (1 failed / 102 passed). This PR's diff touches .claude/ only — no shipped source, no import reachable from that test — so nothing here can affect it. Live-network flake; it already carries retry: 1.

…schema

pnpm run resolves scripts against the session cwd. From a package directory in this monorepo that either ran the package's own lint — a partial gate reporting green while checking 30 files instead of the whole repo — or died with 'Missing script: lint' and blocked the turn. Anchor to CLAUDE_PROJECT_DIR, falling back to the script's own location, so the check is deterministic wherever the session started. The cd fails open (exit 0) to match the guards above it: an unreachable root is a loud skip, never a spurious block.

LukasParke · 2026-08-03T18:44:56Z

Head is now 1dc0f83 (main merged in, cwd anchor added). All three threads addressed and resolved.

On the red e2e-tests: not from this branch, and the run was 5 days stale. The diff is .claude/-only (hooks/stop-check.sh + settings.json); the failure is tests/e2e/cancellation.test.ts > per-request timeoutMs bounds a live request even when a run signal is present, Error: Test timed out in 30000ms — a live-provider timing test that cannot be reached by a hook-script change. The same test was flagged as flaky on #73. The push above re-triggers CI, so this should clear on its own; if it fails again it wants a re-run, not a code change.

The cd guard failed open silently, which is the exact failure the other guards print for: a silent skip is indistinguishable from a passing check, so the hook block looks healthy in /hooks while doing nothing.