Repository navigation
mux: per-surface agent state machine with hook authority and title detection - #7412
lawrencecchen wants to merge 2 commits into
Conversation
…tection Adds AgentState/AgentSource/AgentStore in agent.rs: hook reports (session lifecycle events from coding-agent hooks) take authority over title-based detection, detection never clears an authoritative hook entry, a hook clear with no detection fallback emits a terminal Done transition, and a session-only report (no state) records the session ref without entering the hook state map. A background title-activity detector debounces PTY output and classifies agents by title after a quiet window. New control-socket commands: report-agent, list-agents, notify. New broadcast events: agent-state-changed, notification. Includes scripts/hooks/claude-code-hook.sh as the reference hook wiring Claude Code's Notification/Stop/SessionStart events to report-agent.
|
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: Advanced 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 02de262. Configure here.
| return None; | ||
| } | ||
| self.effective.remove(&surface); | ||
| None |
There was a problem hiding this comment.
Detection clear emits no event
Medium Severity
When title detection stops matching an agent and no hook holds authority, clear_detection drops the effective entry but always returns None, so clear_agent_detection never emits AgentStateChanged. Event subscribers can keep showing an agent that list-agents no longer includes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 02de262. Configure here.
| /// reaps the surface out of the tree itself, so frontends only need to | ||
| /// drop their render state. | ||
| pub fn surface_exited(&self, id: SurfaceId) { | ||
| self.finish_agent(id, AgentSource::Detection); |
There was a problem hiding this comment.
Exit Done mislabels hook source
Medium Severity
On PTY exit, surface_exited always calls finish_agent with AgentSource::Detection, and finish_and_clear puts that parameter on the terminal Done event. Surfaces whose effective state came from hooks are reported as detection when the process exits.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 02de262. Configure here.
![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: 02de2625f4
ℹ️ 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 message: | ||
| request["custom_status"] = message | ||
| elif event == "Stop": | ||
| request["state"] = "done" |
There was a problem hiding this comment.
When Claude emits a Stop in a PTY that stays open, this sends state: done, and report_agent stores that as the effective hook entry. Hook entries continue to override detector updates (AgentStore::apply_detection returns without updating while a hook entry exists), while the later SessionStart report has no state and does not clear hook authority, so starting Claude again in the same surface can leave the agent stuck as done until another hook state such as a notification arrives. Treat Stop as a finish/clear of the hook state rather than a persistent hook report.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds a per-surface agent state machine (
Confidence Score: 3/5The hook-authority state machine is sound in its core design, but the Stop event leaves the hook map permanently occupied — new sessions on the same surface won't show their Working state until the next hook write — and a narrow race during surface exit can inject a ghost agent entry and broadcast a spurious state-change event. The Stop→Done path in the hook script contradicts the design's own stated hook-clear semantics, producing stale Done state visible to all subscribers between consecutive sessions on a reused surface. The surface_exited ordering leaves a window where the detector thread can re-populate the agent store just after it was cleared, creating both a zombie list entry and a false AgentStateChanged broadcast. mux/scripts/hooks/claude-code-hook.sh (Stop semantics), mux/crates/mux-core/src/mux.rs (surface_exited ordering), mux/crates/mux-core/src/agent.rs (AgentStateChange missing session_ref) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
PTY[PTY output byte] --> OBS[observe_agent_activity\nevery read]
OBS -->|DetectorEvent| DET[AgentDetector thread\ndebounce 150ms / quiet 2s]
DET -->|DetectorAction::Active\nor Quiet| TITLE[apply_detection_from_title]
TITLE -->|surface_is_pty check| STORE[AgentStore]
HOOK[Hook script\nclaude-code-hook.sh] -->|report-agent cmd| SOCK[control socket]
SOCK -->|ReportAgent| REPORT[Mux::report_agent]
REPORT -->|source=hook, state=done| AH[apply_hook Done\nhook entry STAYS]
REPORT -->|source=hook, no agent/state| CH[clear_agent_hook\nfinish_and_clear or detection fallback]
REPORT -->|source=detection| AD[apply_detection\nsuppressed if hook present]
AH --> STORE
CH --> STORE
AD --> STORE
STORE -->|hook takes priority| EFF[effective map]
EFF -->|AgentStateChanged event| SUB[subscribers]
EXIT[surface_exited] --> FAC[finish_agent / clear store]
FAC --> CS[close_surface]
DET -.->|race window| GHOST[ghost re-insert between\nfinish_agent and close_surface]
GHOST -.-> STORE
style AH fill:#ffcccc
style GHOST fill:#ffcccc
%%{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"}}}%%
flowchart TD
PTY[PTY output byte] --> OBS[observe_agent_activity\nevery read]
OBS -->|DetectorEvent| DET[AgentDetector thread\ndebounce 150ms / quiet 2s]
DET -->|DetectorAction::Active\nor Quiet| TITLE[apply_detection_from_title]
TITLE -->|surface_is_pty check| STORE[AgentStore]
HOOK[Hook script\nclaude-code-hook.sh] -->|report-agent cmd| SOCK[control socket]
SOCK -->|ReportAgent| REPORT[Mux::report_agent]
REPORT -->|source=hook, state=done| AH[apply_hook Done\nhook entry STAYS]
REPORT -->|source=hook, no agent/state| CH[clear_agent_hook\nfinish_and_clear or detection fallback]
REPORT -->|source=detection| AD[apply_detection\nsuppressed if hook present]
AH --> STORE
CH --> STORE
AD --> STORE
STORE -->|hook takes priority| EFF[effective map]
EFF -->|AgentStateChanged event| SUB[subscribers]
EXIT[surface_exited] --> FAC[finish_agent / clear store]
FAC --> CS[close_surface]
DET -.->|race window| GHOST[ghost re-insert between\nfinish_agent and close_surface]
GHOST -.-> STORE
style AH fill:#ffcccc
style GHOST fill:#ffcccc
Reviews (1): Last reviewed commit: "mux: per-surface agent state machine wit..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| elif event == "Stop": | ||
| request["state"] = "done" |
There was a problem hiding this comment.
Sending {state: "done"} calls apply_hook(Done), which inserts a Done entry into the hook authority map (AgentStore::hook) but never clears it. From that point on, apply_detection always returns None because hook.contains_key(surface) is true (agent.rs line 173). When the user starts a new Claude Code session on the same surface, all title/spinner detection (Working/Idle) is silently suppressed until the next hook event fires — SessionStart only records the session ref, so the effective state stays Done until the first Notification or subsequent hook write.
Per the PR's own design intent ("a hook clear with no detection fallback emits a terminal Done transition"), Stop should use the hook-clear semantic: send {source:"hook"} with no agent and no state, which routes through clear_agent_hook. clear_hook then either falls back to the current detection state (if a new session is already visible in the title) or emits Done via finish_and_clear if nothing is detected — correct in both cases.
| pub fn surface_exited(&self, id: SurfaceId) { | ||
| self.finish_agent(id, AgentSource::Detection); | ||
| self.close_surface(id); | ||
| self.emit(MuxEvent::SurfaceExited(id)); |
There was a problem hiding this comment.
surface_exited produces ghost agent entries
surface_exited clears the agent store via finish_agent (line 912), then removes the surface from state via close_surface (line 913). The detector thread holds a Weak<Mux> and can upgrade it at any time. In the window between those two lines, the detector thread can upgrade the weak ref, enter apply_detection_from_title, call surface_is_pty which returns true (surface still in state), re-insert a detection + effective entry into agent_store, and emit a spurious AgentStateChanged(Working) broadcast. After close_surface runs, the surface is gone from state but the newly re-inserted agent_store entry stays, permanently polluting list_agents results with an exited surface.
A robust fix is to filter list_agents output through surface_is_pty, and to re-check in apply_detection_from_title after acquiring the agent lock that the surface is still present.
| @@ -275,6 +284,9 @@ impl Surface { | |||
| if !responses.is_empty() { | |||
| let _ = surface.write_bytes(&responses); | |||
| } | |||
| if let Some(mux) = mux.upgrade() { | |||
| mux.observe_agent_activity(surface.id, current_title); | |||
| } | |||
There was a problem hiding this comment.
When the title hasn't changed (the else branch at line 277), pty.title.lock() is acquired and the title String is cloned, then immediately sent as a new DetectorEvent to the detector channel. This happens for every PTY read, including high-throughput terminal output (large file streams, cat of big files, etc.), where reads can be thousands per second. Each iteration allocates a new String and sends it across the channel.
Since AgentDetector::ingest overwrites the pending entry for the surface anyway, only the latest title matters. Consider gating the observe_agent_activity call on title changes: skip the send when the title hasn't changed and cache the active_reported state so activity is still tracked without the per-byte clone and send.
| pub custom_status: Option<String>, | ||
| } | ||
|
|
||
| #[derive(Debug, Default)] | ||
| pub(crate) struct AgentStore { | ||
| detection: HashMap<SurfaceId, AgentInfo>, | ||
| hook: HashMap<SurfaceId, AgentInfo>, | ||
| effective: HashMap<SurfaceId, AgentInfo>, |
There was a problem hiding this comment.
AgentStateChange omits session_ref, so Done broadcasts lose session identity
AgentStateChange has no session_ref field. change_from silently drops it, even though finish_and_clear preserves current.session_ref in the done AgentInfo it constructs. A subscriber receiving agent-state-changed for a Done transition cannot identify which Claude Code session just completed without issuing a follow-up list-agents call — but by that point the entry has already been cleared, so the session ref is lost entirely.
Adding session_ref: Option<String> to AgentStateChange and threading it through change_from would let subscribers correlate Done events to specific sessions without a TOCTOU window.
| agents.iter().find(|agent| words.contains(&agent.as_str())).cloned() | ||
| } | ||
|
|
||
| pub fn title_has_action_required(title: &str) -> bool { | ||
| title.to_lowercase().contains("action required") | ||
| } | ||
|
|
||
| pub fn title_has_braille_spinner(title: &str) -> bool { | ||
| title.trim_start().chars().next().is_some_and(|ch| ('\u{2800}'..='\u{28ff}').contains(&ch)) | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] | ||
| #[serde(rename_all = "kebab-case")] | ||
| pub enum AgentState { | ||
| Working, |
There was a problem hiding this comment.
identify_agent, title_has_braille_spinner, and title_has_action_required derive agent type and lifecycle state from raw terminal title strings. When no hook is configured for a surface these heuristics are the sole source for AgentStateChanged events. A window title that coincidentally contains "codex" or "claude", a braille character from an unrelated tool, or a third-party tool echoing "action required" would all produce incorrect transitions.
When a hook is not configured, failing closed (no detection, no state emitted) would be more correct than guessing. Consider requiring an explicit opt-in flag to enable title-based fallback detection, or at minimum gating the detection path so it emits Unknown rather than Working/Blocked when no hook has ever reported for that surface.
|
Fleet instruction update for head |
teamleaderleo
added
ready-to-land
This branch was successfully deployed
2 participants
Summary
AgentState/AgentSource/AgentStoreinagent.rs): hook reports (session lifecycle events from coding-agent hooks) take authority over title-based detection.state) records the session ref without entering the hook state map.report-agent,list-agents,notify. New broadcast events:agent-state-changed,notification.scripts/hooks/claude-code-hook.sh, the reference hook wiring Claude Code'sNotification/Stop/SessionStartevents toreport-agent.Base is
feat-rust-tui-backend(PR #7180) since this branch builds on its mux.rs/server.rs changes; will rebase ontomainonce #7180 merges.Test plan
cargo test --workspace,cargo clippy --workspace --all-targets -- -D warnings,cargo fmt --checkall pass.cargo build -p mux-tuisucceeds.scripts/smoke-tui.pyandscripts/smoke-attach.pypass.agent.rscover: hook-override-then-clear-then-fallback, detection-does-not-clear-hook-authority, done-on-exit, unknown-is-real-classification, last_change-preserved-on-identical-re-report, detector-quiet-window.tests/pty.rs:control_socket_reports_and_broadcasts_agent_state,control_socket_session_report_does_not_take_hook_authority.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
New background detector thread and event surface on the mux hot path (PTY reader → agent activity), but logic is isolated with unit/integration tests and hook authority rules are explicit.
Overview
Adds per-PTY-surface agent tracking so UIs and attach clients can see which coding agent is on a tab and whether it is working, blocked, idle, or done.
A new
agentmodule definesAgentStore(separate detection vs hook layers, hooks win), title heuristics (agent name in title, braille spinner, “action required”), and a debouncedAgentDetectorthread fed from PTY output.Muxwires reports, emitsAgentStateChanged/Notification, and finishes agent state when a surface exits.The control socket gains
report-agent,list-agents, andnotify, with matching subscribe events. PTY children getCMUX_MUX_SURFACE/CMUX_MUX_SURFACE_ID; configurable agent names flow from TUI config intoSurfaceOptions.scripts/hooks/claude-code-hook.shmaps Claude Code hook events toreport-agentover the unix socket.Reviewed by Cursor Bugbot for commit 02de262. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a per-surface agent state machine so surfaces can report which coding agent is active and whether it's working, blocked, idle, or done. Hook reports take authority over title-based detection, and a background detector debounces PTY output to classify agents after a quiet window.
New Features
claude,codex,opencode,pi). Braille spinner ⇒ Working; "Action Required" ⇒ Blocked; otherwise Idle when quiet.report-agent,list-agents, andnotifycommands plusagent-state-changedandnotificationbroadcast events.tabs.agentsconfig feedsSurfaceOptions.agents; children receiveCMUX_MUX_SURFACE/CMUX_MUX_SURFACE_ID.scripts/hooks/claude-code-hook.shwiring Claude Code's Notification/Stop/SessionStart events toreport-agent.Migration
tabs.agents; defaults apply when unset.CMUX_MUX_SOCKETin env.agent-state-changedandnotification.Written for commit 6e22a52. Summary will update on new commits.