Skip to content

fix(hooks): fail open when worker is unreachable instead of blocking commands#3269

Draft
posthog[bot] wants to merge 1 commit into
mainfrom
posthog-code/fail-open-worker-unreachable-hook
Draft

fix(hooks): fail open when worker is unreachable instead of blocking commands#3269
posthog[bot] wants to merge 1 commit into
mainfrom
posthog-code/fail-open-worker-unreachable-hook

Conversation

@posthog

@posthog posthog Bot commented Jul 15, 2026

Copy link
Copy Markdown

Summary

The UserPromptSubmit session-init hook could block the user's Claude Code command (e.g. /model) with UserPromptSubmit operation blocked by hook whenever the claude-mem worker was unavailable.

The per-call worker paths already fail open, but a fail-loud counter sat on top: recordWorkerUnreachable() tracks consecutive worker-unreachable failures and, once the threshold (default 3) was reached, unconditionally called emitBlockingError()process.exit(2). On a UserPromptSubmit hook, exit 2 is Claude Code's "block this operation" signal, so normal commands got killed — worst when the worker was slow to start (large DB → readiness wait times out → counter climbs).

This change makes the hook genuinely fail open:

  • By default a tripped threshold now surfaces a non-blocking stderr warning and the hook continues (exit 0), so commands always pass through.
  • The old blocking behavior is preserved behind a new opt-in setting, CLAUDE_MEM_HOOK_FAIL_LOUD_BLOCK (default false).
  • The hook_failed distress telemetry is unchanged — only the process.exit(2) default is gone.

Why

Over 90 days the session-init hook alone emitted thousands of blocking events across hundreds of users. Blocking a core interaction — the user types a command and claude-mem refuses to let it through — is exactly the papercut that drives uninstalls. Failing open with a warning is the correct default; the hard stop is now opt-in.

Test plan

  • New tests/worker-fail-open.test.ts runs recordWorkerUnreachable() in an isolated subprocess and asserts the actual exit code: exit 0 + non-blocking warning by default, exit 2 only when CLAUDE_MEM_HOOK_FAIL_LOUD_BLOCK=true.
  • Updated the source-contract test in tests/cli/hook-stream-discipline.test.ts for the fail-open default.
  • tsc --noEmit clean; hook-lifecycle, hook-io, and settings-defaults suites pass.

Closes #3267
Refs #3161
Refs #2996


Created with PostHog Code from an inbox report.

…commands

The UserPromptSubmit `session-init` hook could kill the user's Claude Code
command (e.g. `/model`) whenever the claude-mem worker was unavailable.
`recordWorkerUnreachable()` tracks consecutive worker-unreachable failures and,
once the threshold was reached, unconditionally called `emitBlockingError()`,
which does `process.exit(2)`. On a UserPromptSubmit hook, exit code 2 is Claude
Code's "block this operation" signal, so normal commands got blocked — worst
when the worker was slow to start.

Make the hook genuinely fail open: by default a tripped threshold now surfaces
a non-blocking stderr warning and the hook continues (exit 0). The blocking
behavior is preserved behind a new opt-in setting,
`CLAUDE_MEM_HOOK_FAIL_LOUD_BLOCK` (default `false`). The `hook_failed` distress
telemetry is unchanged.

Generated-By: PostHog Code
Task-Id: bdc456fa-14f5-4c2c-8b56-746f8d4c038a
@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes the worker-unreachable hook path to fail open by default. The main changes are:

  • Adds CLAUDE_MEM_HOOK_FAIL_LOUD_BLOCK as an opt-in setting for exit-2 blocking.
  • Emits a non-blocking diagnostic when the fail-loud threshold is reached by default.
  • Keeps threshold telemetry intact while avoiding blocked Claude Code commands.
  • Adds subprocess tests for the default exit-0 path and opt-in exit-2 path.

Confidence Score: 5/5

Safe to merge with minimal risk.

The change is narrowly scoped and preserves the previous blocking behavior behind an explicit setting. The settings path uses the existing defaults and environment override mechanism. Subprocess tests verify the actual exit codes for both fail-open and opt-in blocking modes. No correctness or security issues were identified.

No files require special attention.

T-Rex T-Rex Logs

What T-Rex did

  • Ran the worker fail-open hook stream test and captured the exact command, working directory, Bun output, and EXIT_CODE: 0 in the test log.
  • Examined the evidence log showing test names and outcomes, including does not block (exit 0) and blocks (exit 2) only when CLAUDE_MEM_HOOK_FAIL_LOUD_BLOCK is opted in, along with hook stream source-contract coverage.
  • Confirmed the test run completed successfully with exit code 0, as shown in the test log.
  • Linked and reviewed two artifacts containing the test log and the evidence log to support review.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
src/shared/SettingsDefaultsManager.ts Adds the new CLAUDE_MEM_HOOK_FAIL_LOUD_BLOCK default with environment override support through the existing settings loader.
src/shared/worker-utils.ts Changes the worker-unreachable threshold path to emit a non-blocking diagnostic by default while preserving opt-in blocking behavior.
tests/cli/hook-stream-discipline.test.ts Updates the source-contract assertion to require the fail-open gate and diagnostic path.
tests/worker-fail-open.test.ts Adds subprocess coverage for both default exit-0 fail-open behavior and opt-in exit-2 blocking behavior.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Hook as UserPromptSubmit hook
participant Counter as recordWorkerUnreachable()
participant Settings as SettingsDefaultsManager/loadFromFileOnce
participant IO as hook-io
participant Claude as Claude Code

Hook->>Counter: worker request fails
Counter->>Counter: increment consecutive failure state
Counter->>Settings: read threshold and CLAUDE_MEM_HOOK_FAIL_LOUD_BLOCK
alt failures below threshold
    Counter-->>Hook: return failure count
    Hook-->>Claude: exit 0
else threshold reached and block opt-in is false
    Counter->>IO: emitDiagnostic(non-blocking warning)
    Counter-->>Hook: return failure count
    Hook-->>Claude: exit 0 (command continues)
else threshold reached and block opt-in is true
    Counter->>IO: emitBlockingError(message)
    IO-->>Claude: stderr + exit 2 (command blocked)
end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Hook as UserPromptSubmit hook
participant Counter as recordWorkerUnreachable()
participant Settings as SettingsDefaultsManager/loadFromFileOnce
participant IO as hook-io
participant Claude as Claude Code

Hook->>Counter: worker request fails
Counter->>Counter: increment consecutive failure state
Counter->>Settings: read threshold and CLAUDE_MEM_HOOK_FAIL_LOUD_BLOCK
alt failures below threshold
    Counter-->>Hook: return failure count
    Hook-->>Claude: exit 0
else threshold reached and block opt-in is false
    Counter->>IO: emitDiagnostic(non-blocking warning)
    Counter-->>Hook: return failure count
    Hook-->>Claude: exit 0 (command continues)
else threshold reached and block opt-in is true
    Counter->>IO: emitBlockingError(message)
    IO-->>Claude: stderr + exit 2 (command blocked)
end
Loading

Reviews (1): Last reviewed commit: "fix(hooks): fail open when worker is unr..." | Re-trigger Greptile

@posthog

posthog Bot commented Jul 15, 2026

Copy link
Copy Markdown
Author

CI investigation: the failing test job is pre-existing, cross-file test pollution — not this PR

I dug into the red typecheck · build · test · bundle-size check. The 29 test failures are unrelated to this diff and are a pre-existing test-isolation bug in the suite. Evidence:

  • Identical failures on an unrelated PR. fix(chroma-sync): reconcile duplicate IDs in place to stop unbounded index growth #3268 (a chroma change, no overlap with this PR) fails with the same groups: createRecallMcpServer, findClaudeExecutable (all sub-groups), and server boot: mode loading. Earlier PRs today (Retry transient litellm parse errors from OpenRouter #3263, fix(worker): guard firstString + debug logger against stack overflow #3264) are green — there's a time boundary today, i.e. it flips based on when CI runs, not on the diff.
  • The stack trace crosses test files — a leaked module mock:
    error: Claude executable not found
       at findClaudeExecutableImpl (tests/worker/claude-setup-gate.test.ts:149:52)
       at <anonymous>              (tests/shared/find-claude-executable.test.ts:105:12)
    
    find-claude-executable.test.ts is running the findClaudeExecutableImpl stub that claude-setup-gate.test.ts installed via mock.module(...). Bun does not auto-restore mock.module() between files, so when the runner's file order puts the mocking file before the victim, the victim imports the leaked (throwing) mock. createRecallMcpServer (client.listTools is not a function) and server boot fail the same way — leaked module mocks from earlier files.
  • Not reproducible with this diff. Locally, this branch and main produce identical results (same 3 unrelated runOneTimeV12_4_3Cleanup env failures, none of the 29). My new test file runs isolated subprocesses and does not pollute the affected files (they pass with and without it).

Why it's intermittent across PRs

CI's build job runs npm install with no committed root lockfile and bun-version: latest, so dependency resolution and test-file execution order aren't pinned. That nondeterminism is what decides whether the leaking file runs before its victims on a given run.

This PR itself is green

tsc --noEmit is clean and the change's own tests pass (tests/worker-fail-open.test.ts, tests/cli/hook-stream-discipline.test.ts). I deliberately did not push a fix for the mock leakage: it lives in unrelated test files, is outside this PR's scope (fail-open hook behavior), affects the whole repo (see #3268), and I can't reproduce it locally to verify a fix would take. It's best fixed centrally.

Suggested central fix (separate change)

  • Restore module mocks between files — e.g. afterAll(() => mock.restore()) in the files that call mock.module() (tests/worker/claude-setup-gate.test.ts, the recall-mcp and server-boot mockers), and/or
  • Pin CI: commit a root lockfile and use --frozen-lockfile so install + ordering are deterministic.

Happy to open a dedicated PR for the test-isolation fix if you'd like.

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.

UserPromptSubmit Hook Blocks Commands Intermittently When Worker Is Probably Not Running

0 participants