Skip to content

cli: answer queued agent hooks inside the agent's hook timeout - #14834

Merged
teamleaderleo merged 7 commits into
mainfrom
fix/hook-enqueue-wall-clock-bound
Sep 28, 2026
Merged

teamleaderleo merged 7 commits into
mainfrom
fix/hook-enqueue-wall-clock-bound

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

A Claude UserPromptSubmit hook installed by cmux ran past Claude Code's 5 s hook timeout, so Claude discarded its output and reported the hook as timed out. The hook passes CMUXTERM_CLI_RESPONSE_TIMEOUT_SEC=0.5, but that bounds only socket waits. Several steps in cmux hooks enqueue had no deadline:

  • Resolving the socket password can fall through to SecItemCopyMatching, and the PID route-resolution client resolved it a second time.
  • Stdin was read until EOF.
  • On an unexpected connect failure the CLI started Sentry synchronously.

cmux hooks enqueue now answers inside a fixed wall-clock budget:

  • Watchdog. hooks enqueue arms a 2 s timer (AgentHookDeliveryPolicy.admissionWallClockSeconds) at dispatch. When it expires, the process writes the same neutral {} the shell fallback prints and exits 0. The command's own answer and the watchdog go through one lock (AgentHookEnqueueWallClock.respond), so exactly one {} is written. The 2 s budget leaves the other 3 s of the declared timeout for process launch, which happens before the timer can be armed. CMUX_AGENT_HOOK_ENQUEUE_BUDGET_SEC can lower the budget for tests but never raise it.
  • One password resolution. The route client reuses the password the admission client already resolved, including a resolved "no password" (SocketClient.hasConfiguredAuthentication), instead of running SocketPasswordResolver again.
  • Bounded stdin. Input is read with poll and a 1 s deadline. If the writer keeps stdin open, whatever arrived by then is admitted. The 1 MiB cap is unchanged.
  • No Sentry. Telemetry is disabled for hooks enqueue. Its failures fail open by design and are expected while the app is busy or quitting.

The event is dropped only if the app hasn't admitted it within 2 s. That's the same outcome as before, when the agent killed the hook and discarded its output, but the agent now gets a well-formed answer instead of a timeout.

Evidence

  • f2c142e4b2c adds tests/test_cli_hook_enqueue_wall_clock.py (macos-cli-no-socket lane). It runs the real CLI against a fake app socket in three cases: a responsive app, stdin left open, and an app that accepts the enqueue request and never answers (budget lowered to 0.3 s). Each case must exit 0 with {}, within 2.5 s where a bound applies. 6d8be52b37d is the fix.
  • The pre-fix failure is not observed yet, only derived from the code: the old CLI blocks in FileHandle.read until stdin closes, and exits 1 with a timeout error when the app stalls. I didn't build the CLI locally: the shared Mac was at load ~94, and the local rule is to leave native builds to CI. This PR's CI is the first native build and the first run of the new test.
  • AgentHookDeliveryPolicyTests gains a check that the wall-clock budget sits above the admission response timeout and within half the declared hook timeout.
  • python3 scripts/verify-local.py --affected mf/main --swift-changed mf/main: 13/13 static checks passed, including Swift syntax on the changed files and test-registry validation.

Not covered: no app-host tests changed. Nothing was dogfooded against a running app.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes cmux hooks enqueue running past Claude Code's 5 s hook timeout. The command passed 0.5 s as a response timeout that bounded only socket waits; password resolution (including a duplicate keychain lookup), unbounded stdin reads, and Sentry startup had no deadline, so the agent killed the hook and discarded its output. The hook now answers with the neutral {} and exits 0 within the declared timeout, and the fallback that drains the Claude spool cannot drop a record the watchdog would cut mid-claim.

Bug Fixes

  • Arms a 2 s wall-clock watchdog at dispatch; on expiry it writes {} and exits 0, with one shared lock guaranteeing exactly one response.
  • Spooled-record claims run inside a critical section, so a budget that expires mid-claim answers and exits only after that record reaches the app.
  • Reuses the admission client's resolved password, including a resolved "no password", for the route client instead of resolving it again.
  • Reads stdin with a 1 s poll deadline; what arrived is admitted when it is complete JSON, otherwise the neutral fallback is used.
  • Disables Sentry telemetry for hooks enqueue; its failures are expected and fail open.
  • Adds regression tests for a responsive app, stdin left open, a stalled app, and a spool drain stalled under the watchdog, plus a policy check that the budget fits inside the declared timeout.

Written for commit 53f2e0c. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 2 commits September 26, 2026 10:05
Regression for a Claude UserPromptSubmit hook that Claude Code killed after
its 5 s timeout although the hook passes CMUXTERM_CLI_RESPONSE_TIMEOUT_SEC=0.5.
The socket waits are bounded; nothing bounds the whole command. Against a
fake app socket this covers an agent that never closes stdin and an app that
accepts the enqueue request and never answers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A Claude UserPromptSubmit hook ran past Claude Code's 5 s hook timeout although
the hook passes CMUXTERM_CLI_RESPONSE_TIMEOUT_SEC=0.5. That setting bounds only
socket waits. Password resolution can reach the keychain (twice: the route
client resolved it again), stdin was read until EOF, and nothing bounded the
command as a whole.

- Arm a wall-clock watchdog for `hooks enqueue`
  (AgentHookDeliveryPolicy.admissionWallClockSeconds, 2 s). Past it the
  process writes the neutral {} the shell fallback prints and exits 0. The
  command's own answer and the watchdog share one lock, so exactly one
  response is written.
- The route-resolution client reuses the password the admission client
  already resolved instead of running SocketPasswordResolver again.
- Read stdin with a 1 s poll deadline; a writer that keeps stdin open gets
  what arrived admitted.
- Never start Sentry for hooks enqueue: its failures are expected and fail
  open.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 46 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0dbc35dc-8833-42c5-9a10-0df8fcc44b23

📥 Commits

Reviewing files that changed from the base of the PR and between 1755ea8 and 53f2e0c.

📒 Files selected for processing (8)
  • CLI/CLISocketSentryTelemetry.swift
  • CLI/CMUXCLI+AgentHookAdmission.swift
  • CLI/CMUXCLI+AgentHookSpoolForwarder.swift
  • CLI/cmux.swift
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentHookDeliveryPolicy.swift
  • Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentHookDeliveryPolicyTests.swift
  • tests/test-execution.toml
  • tests/test_cli_hook_enqueue_wall_clock.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

teamleaderleo and others added 2 commits September 26, 2026 10:19
Review follow-up: a writer that keeps stdin open and is cut mid-document by
the 1 s read deadline now gets the neutral payload instead of truncated JSON.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Automatic catch-up couldn't merge main (368ec9f6d0b5): CLI/CMUXCLI+AgentHookAdmission.swift (both sides changed the same lines). Nothing was pushed; merge it by hand. A new push or /catch-up tries again.

Label no-auto-catch-up to opt out · Catch-up run

…ck-bound

# Conflicts:
#	CLI/CMUXCLI+AgentHookAdmission.swift
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Dogfood build of 53f2e0c200723140d7a63cac26c4e3aa7d8d5ba5

cmux DEV pr-14834-53f2e0c2.app

The link opens this exact commit in the cmux dev menu bar app. The build starts on each push and the page waits until it is ready; a newer push replaces it. It signs in against production, so Cloud or backend changes still need a tagged build with a development backend.

teamleaderleo and others added 2 commits September 28, 2026 07:03
…before the watchdog exits

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Main's #14931 made the hooks enqueue fallback drain the Claude spool first.
The drain unlinks a record before admitting it, so the 2 s watchdog could exit
after the claim and drop the event. A claim now runs inside a critical section:
an expiry during it answers and exits when the claim ends.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Merged main (#14931 moved queued admission into admitQueuedAgentHook, which the Claude spool forwarder also calls). The neutral {} now prints only from enqueueAgentHook, through AgentHookEnqueueWallClock.respond; the forwarder never prints.

The merge exposed one interaction: hooks enqueue now drains the session's spool before its own event, and a drain claims a record by unlinking it. The 2 s watchdog could exit between that unlink and the admission request, dropping an event #14931 promises to deliver. Each claim now runs inside a critical section on the wall clock (beginClaim/endClaim). An expiry during a claim answers {} and exits as soon as that claim ends (worst case one bounded admission, about 0.7 s), and no further record is claimed, so the rest stay for the forwarder or the next drainer.

  • 90cb17eae89a adds a tests/test_cli_hook_enqueue_wall_clock.py case: one published spool record, a stalled route resolution, a 0.05 s budget. The record must reach the app or stay on disk. The harness also stops inheriting CMUX_CLAUDE_HOOK_SPOOL_DIR so a local run can't drain a developer's real spool.
  • 53f2e0c20072 is the fix. Not built locally; CI is the first compile.

@teamleaderleo
teamleaderleo merged commit 16f1270 into main Sep 28, 2026
68 checks passed
@teamleaderleo
teamleaderleo deleted the fix/hook-enqueue-wall-clock-bound branch September 28, 2026 12:01
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 53f2e0c200: every check was green at merge (22 verified; 17 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 28, 2026
ee20686 fix: keep SSH exit prompt off PTY output drain (manaflow-ai#15337)
96e7a27 reload.sh: expand the empty resolver args safely under bash 3.2 (manaflow-ai#15352)
558d6b9 ci: move owned gui jobs to Blacksmith only when its queue is shorter (manaflow-ai#15336)
fff0b82 UI fuzzer: seeded action sequences, oracles, minimized repros and deduplicated issues (manaflow-ai#15297)
94a6387 Add an agent activity mode to workspace auto-reordering (manaflow-ai#15216)
e5231be CI: post screenshots and a GIF of each app PR's build in its dogfood comment (manaflow-ai#15280)
16f1270 cli: answer queued agent hooks inside the agent's hook timeout (manaflow-ai#14834)
3fd61eb Sidebar: show the most urgent pane's status when panes share an agent key (manaflow-ai#15260)
0975d0b Release discarded CodeRouter response bodies after retry (manaflow-ai#15253)
42f93d4 Re-verify the session against a body-supplied VM billing team (manaflow-ai#15339)
bdb6920 Keep the mail broker from orphaning a reply to an unknown parent (manaflow-ai#15330)

# Conflicts:
#	.github/workflows/ci-guards.yml
#	.github/workflows/ci-macos.yml
#	.github/workflows/test-e2e.yml
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