Repository navigation
Agent chat: inject Claude/Codex hook config at launch (live ingest goes live) - #5964
lawrencecchen wants to merge 10 commits into
Conversation
Launch side of live hook ingest (docs/agent-conversation-protocol.md): - AgentHookLaunchEnvironment computes the per-instance ingest socket path (tag/variant-scoped for dev, nightly, staging builds) and exports CMUX_AGENT_HOOK_EMIT_BIN + CMUX_AGENT_HOOK_SOCKET into terminal surfaces; the chat surface's spawned cmuxd-remote child gets the same socket pinned so listener and emitters agree. - cmux-claude-wrapper merges one agent-hook-emit entry per hook event into the single --settings payload it already owns (claude's --settings is last-wins, verified on 2.1.175, so a second flag would clobber). Tool hooks are async; a user-supplied --settings passes through untouched. - New cmux-codex-wrapper injects 'codex -c notify=[...]' per launch, never writing the user's config.toml and never overriding a user-configured notifier (CLI override or config.toml notify key). - Injection requires an emit binary that provably carries the verb (explicit dev override, or exact/newer cached release version): an old cached daemon invoked with the unknown verb falls into its CLI dispatch and would fail every Claude hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AgentHookLaunchEnvironment.swift had been wired with AgentChatPanel.swift's object ids, silently dropping the panel from the build; slug sanitization lives on SocketPathMarkerFiles, not SocketControlSettings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f21ccd1517
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let override = environment[socketEnvKey]?.trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !override.isEmpty { | ||
| return override |
There was a problem hiding this comment.
When a cmux app is launched from an existing cmux terminal, that shell now inherits CMUX_AGENT_HOOK_SOCKET from the parent surface, so this unconditional override wins before the bundle/tag classification below. In that nested-launch scenario a tagged debug, nightly, or staging build will reuse the parent instance's ingest socket instead of its own variant-scoped path, letting its daemon and agent hooks feed another cmux instance and defeating the isolation this change is trying to add. Please distinguish deliberate operator overrides from inherited cmux terminal env (or validate it against this bundle variant) before returning it here.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR wires the launch side of the agent conversation hook ingest: a new
Confidence Score: 4/5Safe to merge with one ordering question in the PermissionRequest hook configuration worth confirming. The provenance gate, socket-path isolation, and passthrough-when-missing semantics are carefully designed and well-tested. The one open question is whether Resources/bin/cmux-claude-wrapper — the PermissionRequest hook group ordering needs a second look depending on whether Important Files Changed
Reviews (4): Last reviewed commit: "Tests: guard the app module import for D..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if let emitBinaryURL = AgentHookLaunchEnvironment.injectableEmitBinaryURL( | ||
| outcome: AgentDaemonBinaryLocator().locate() | ||
| ), | ||
| let agentHookEnvironment = AgentHookLaunchEnvironment.launchEnvironment( | ||
| emitBinaryURL: emitBinaryURL, | ||
| socketPath: AgentHookLaunchEnvironment.ingestSocketPath() | ||
| ) { | ||
| for (key, value) in agentHookEnvironment { | ||
| setManagedEnvironmentValue(key, value) | ||
| } | ||
| } |
There was a problem hiding this comment.
AgentDaemonBinaryLocator().locate() is called synchronously inside createSurface, which is called from a @MainActor context (visible from the @MainActor-annotated claudeCommandShimStateForSurface and the surrounding call sites). When the exact-version binary isn't cached, locate() falls through to newestCachedBinary(), which calls fileManager.contentsOfDirectory(atPath:) — a blocking POSIX syscall — on every terminal surface creation. On a cloud-synced home directory, NFS mount, or when the daemon cache root doesn't exist yet, this stalls the main thread and blocks terminal rendering.
The comparable call in AgentChatWebViewController.swift correctly uses await Self.locateDaemonBinary(). The same treatment belongs here: the locator result should be resolved off-main and the emit-binary URL stored (e.g., computed once and cached in TerminalSurface, similar to how claudeCommandShim uses Task.detached(priority: .utility) for installation work) so that createSurface only reads a pre-resolved value.
| return "\(base)-\(variant)-\(slug)" | ||
| } | ||
|
|
||
| @usableFromInline static var isDebugBuild: Bool { |
There was a problem hiding this comment.
@usableFromInline is meaningful only for library modules where inlined callers in other modules need access to an internal symbol. AgentHookLaunchEnvironment lives in the cmux app target, which has no external inlining callers, so the annotation is a no-op here. It signals library semantics where none exist and will confuse future maintainers.
| @usableFromInline static var isDebugBuild: Bool { | |
| static var isDebugBuild: Bool { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sion Nightly/staging release builds share marketing versions with stable artifacts built from different SHAs, so the exact-version check proves nothing there; cached binaries are now injectable only on stable release builds, everything else uses the explicit dev override. Terminal surface creation also no longer rescans the daemon cache per surface: the relay is resolved once per app session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…orever A found relay stays cached for the session; a miss is retried at most every 30 seconds so the first cmux ssh enables injection for new terminals without an app restart, while a workspace-restore burst still costs a single cache scan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… feat-agchat-hooks-launch # Conflicts: # docs/agent-conversation-protocol.md
CMUX_AGENT_HOOK_SOCKET is exported into every cmux terminal for that pane's hooks, so a cmux app launched from inside another instance's terminal inherited that instance's socket and lost tagged/nightly/ staging isolation (the bind would fail; wrappers would emit to the wrong app). The override now counts only when the ambient CMUX_BUNDLE_ID is absent or matches this app's bundle id. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7b6bf08. Configure here.
The zsh wrapper installer unconditionally unaliased and redefined the command, unlike bash and fish which yield to a user-defined alias or function (binary selection, env injection, default args). Mirror the guard via whence -w; verified behaviorally in a clean zsh (user alias preserved, wrapper installed when absent). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…is absent Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50a899bf03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ambientBundleID == nil || ambientBundleID?.isEmpty == true | ||
| || ambientBundleID == bundleIdentifier { | ||
| return override |
There was a problem hiding this comment.
For base debug builds whose bundle id stays com.cmuxterm.app.debug, changing only CMUX_TAG is enough to create a separate tagged instance (for example the test harnesses launch CMUX_TAG=tests-v1 .../cmux DEV). If that launch happens from another base-debug cmux terminal, the inherited CMUX_AGENT_HOOK_SOCKET is accepted here because the bundle ids match, even though the tags differ, so the new instance emits to and pins its daemon child to the parent tag's ingest socket instead of its own /tmp/cmuxd-agentconv-<uid>-debug-<tag>/ingest.sock. Please also validate the inherited/current CMUX_TAG (or otherwise reject ambient cmux terminal env) before returning the override.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
2 participants
Launch side of live hook ingest for the agent conversation protocol (#5736). The daemon side (ingest socket,
agent-hook-emit, Claude payload translation; Codex notify translation on #5957) was already in place, but nothing configured agents to emit, so turns, permission banners, and low-latency tool rows never flowed in real usage.How it works
App (Swift). New
Sources/AgentChat/AgentHookLaunchEnvironment.swiftcomputes this instance's ingest socket path and exports two managed env vars into every terminal surface:CMUX_AGENT_HOOK_EMIT_BIN(stagedcmuxd-remotefrom the checksum-verified remote-daemons cache, viaAgentDaemonBinaryLocator) andCMUX_AGENT_HOOK_SOCKET. The chat surface's spawnedcmuxd-remote serve --stdiochild gets the same socket path pinned into its environment, so listener and emitters always agree.Socket path scheme. Stable release: the documented default
/tmp/cmuxd-agentconv-<uid>/ingest.sock. Every other variant is scoped using the control socket's bundle-id classification:/tmp/cmuxd-agentconv-<uid>-debug-<tag>/ingest.sockfor tagged dev builds,-debug,-nightly[-slug],-staging[-slug]likewise, so tagged builds never cross-talk with the user's stable daemon. An explicitCMUX_AGENT_HOOK_SOCKETin the app environment overrides.Claude.
Resources/bin/cmux-claude-wrapper(the existing per-launch--settingsowner) merges oneagent-hook-emitentry per event (UserPromptSubmit,PreToolUse,PostToolUse,Stop,Notification,PermissionRequest, timeout 5) into the settings payload it already injects. Commands use the wrapper's\"$ENV_VAR\"hook-shell-expansion convention, so DerivedData paths with spaces need no quoting games.PreToolUse/PostToolUseareasyncso tool calls gain zero latency; the daemon merge layer tolerates reordering.--settingscollision handling (verified, not assumed). Claude's--settingsis last-wins across repeated flags, not cumulative: on 2.1.175,claude --settings '{invalid' --settings '{}' -p hisucceeds while the reversed order fails on the invalid value. So a second--settingswould clobber cmux's existing hooks payload, and the merge must happen inside the single value the wrapper owns. A user-supplied--settingspasses through untouched and (being last) wins over cmux's; user settings are never clobbered, cmux's injection is lost for that launch, matching the wrapper's longstanding semantics. Injection is inline per exec, so resume/fork relaunches are idempotent with no settings files to manage.Codex. New
Resources/bin/cmux-codex-wrapper(registered in zsh/bash/fish shell integration like the Claude wrapper) injectscodex -c notify=["<emit>","agent-hook-emit","--socket","<sock>","--provider","codex"]per launch. It never writes~/.codex/config.tomland skips injection when the user already has a notifier (their own-c notifyor an uncommentednotifykey in config.toml; cmux's persistent Codex integration never sets one). On this branch codex frames are accepted-and-dropped by the emit verb (exit 0); they start flowing when #5957 lands (argv verified against that branch's binary).Safety gate (important). Every released
cmuxd-remotepredates the verb, and an old binary invoked asagent-hook-emitfalls through to its CLI dispatch (readsCMUX_SOCKET_PATH, connects to the app control socket, exits non-zero), which would stall or garble every Claude hook. Injection therefore requires provenance that provably carries the verb: the explicitCMUX_REMOTE_DAEMON_BINARYdev override on any build, or, on release builds, a cached daemon at the app's exact release version (same-SHA artifacts) or newer. Debug builds inject only with the override, which dev dogfood of this feature already requires. No binary, or an old one: neither env var is set and both wrappers skip injection entirely; agent launches never depend on the feature.Tests
cmuxTests/AgentHookLaunchEnvironmentTests.swift(Swift Testing, wired into project.pbxproj,lint-pbxproj-test-wiringclean): socket path scheme (stable default, tagged/untagged debug, CMUX_TAG, nightly/staging, env override), skip-when-no-binary, provenance/version gating, daemon-child socket pinning.tests/test_claude_wrapper_hooks.py(extended; in ci.yml): emit entries merged with exact command/timeout/async shape, settings stay clean without the env or with a non-executable emit binary, exactly one--settings, user--settingspasses through unmodified and last.tests/test_codex_wrapper_notify.py(new; added to ci.yml): notify argv shape and placement before the subcommand, passthrough outside cmux / disabled / missing env or binary, user CLI and config.toml notify respected, commented notify does not block,--helpuntouched.PreToolUsepayload throughcmuxd-remote agent-hook-emit --socket <tmp>to a listening socket from a path containing a space and produced the expected frame; the codex notify argv produced the expectedStopframe against a binary built fromfeat-agentconv-codex-hooks; emit with no listener exits 0 instantly.Gates:
xcodebuild ... -derivedDataPath /tmp/cmux-agchat-h buildgreen (build-only); wrapper python suites green;normalize-pbxproj/check-pbxproj/lint-pbxproj-test-wiringclean. No user-facing strings added (env and CLI plumbing only), so no localization changes.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Injects live agent hook config at launch so Claude and
codexemit real-time frames into the chat surface, enabling turn events, permission banners, and low‑latency tool rows. Adds safe gating, variant‑scoped sockets, and session‑cached relay resolution while preserving cross‑instance isolation and user shell aliases.New Features
CMUX_AGENT_HOOK_EMIT_BIN(stagedcmuxd-remote) andCMUX_AGENT_HOOK_SOCKETinto terminals and pins the same socket in the spawnedcmuxd-remote serve --stdiochild; relay lookup is cached per app session with a 30s re‑probe on misses./tmp/cmuxd-agentconv-<uid>/ingest.sock; dev/nightly/staging add suffixes. ACMUX_AGENT_HOOK_SOCKEToverride is ignored when inherited from another cmux instance’s terminal (checked viaCMUX_BUNDLE_ID).cmux-claude-wrappermerges oneagent-hook-emitentry per event into its single--settingsJSON (UserPromptSubmit,PreToolUse/PostToolUseasync,Stop,Notification,PermissionRequest). A user--settingspasses through and wins.cmux-codex-wrapperinjects-c notify=["<emit>","agent-hook-emit","--socket","<sock>","--provider","codex"]per launch; never writes~/.codex/config.toml, respects a user notifier (CLI or config), and skips outside cmux or whenCMUX_CODEX_HOOKS_DISABLED=1. Shell integration installs the wrapper for bash/zsh/fish.Bug Fixes
claude,grok, andcodexand installs wrappers only when absent.whenceguard no longer tripserrexitwhen the command is missing.Written for commit 55bf03b. Summary will update on new commits.
Note
Medium Risk
Touches every in-cmux Claude/Codex launch and gates on daemon version provenance; mis-gating or socket isolation bugs could drop live updates or, with an old relay, stall Claude hooks—mitigated by skip-when-unsafe and extensive tests.
Overview
Wires the launch side of agent conversation hook ingest so Claude and Codex can push live frames into the chat surface via
cmuxd-remote agent-hook-emit.App (Swift). New
AgentHookLaunchEnvironmentderives a variant-scoped ingest socket path and, when safe, exportsCMUX_AGENT_HOOK_EMIT_BINandCMUX_AGENT_HOOK_SOCKETinto terminal surfaces (session-cached relay lookup with 30s re-probe on miss).AgentDaemonBinaryLocatornow returns provenance so injection is gated: explicit dev override, or stable release with a cached daemon at the same/newer version—skipping older binaries that would break Claude hooks. The chat daemon child gets the same socket viaAgentDaemonClient+daemonChildEnvironment().Wrappers & shells.
cmux-claude-wrappermerges extraagent-hook-emithook entries into its single--settingsJSON when env is set (PreToolUse/PostToolUseasync). Newcmux-codex-wrapperadds per-launch-c notify=[...]without touchingconfig.toml, respecting user notify overrides. Bash/zsh/fish install the Codex wrapper; zsh now skips wrapping when the user already has an alias/function.Tests & docs. Swift and Python regression tests plus CI for Codex wrapper;
docs/agent-conversation-protocol.mdupdated for the Swift + wrapper split.Reviewed by Cursor Bugbot for commit 55bf03b. Bugbot is set up for automated code reviews on this repo. Configure here.