Repository navigation
Clear agent Running status on interrupt signals - #4390
Closed
austinywang wants to merge 27 commits into
Closed
austinywang wants to merge 27 commits into
austinywang wants to merge 27 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesStop-Failure Hook and Monitor Session State
Sequence Diagram: sequenceDiagram
participant CLAUDE_WRAPPER
participant CMUX_CLI
participant RUNTIME_STORE
participant SOCKET
CLAUDE_WRAPPER->>CMUX_CLI: emit hooks claude stop-failure
CMUX_CLI->>RUNTIME_STORE: upsert runtimeStatus = Stop/StopFailure (include session/workspace/surface/cwd/env)
CMUX_CLI->>SOCKET: send scoped set_status / turn-end notification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (14 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR clears stale agent running status after interrupted or terminal turns. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (17): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 19319-19333: The guard in
hasOtherRunningCodexMonitorSession(sessionId:workspaceId:env:) only excludes by
workspaceId which prevents panel-scoped idle updates; update the function
signature to accept an optional surfaceId (e.g., surfaceId: String?) and change
the check to dedupe by surfaceId when provided (call
codexHookSessionStore(...).hasRunningSession with both workspaceId and
surfaceId), and only perform the workspace-wide exclusion when surfaceId == nil;
also update callers (e.g., publishCodexMonitorIdle) to pass the surfaceId so
panel-scoped set_status events aren’t suppressed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e157f6c4-9327-4220-9abb-17cd2de96816
📒 Files selected for processing (5)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/cmux.swiftResources/bin/claudecmuxTests/CLINotifyProcessIntegrationRegressionTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 663-679: The current
hasRunningSession(workspaceId:surfaceId:excludingSessionId:) only treats
sessions with record.runtimeStatus == .running as active, which allows a
concurrent session in .needsInput to be ignored and let another publish Idle;
update the predicate inside withLockedState so it treats .needsInput as an
active state as well (i.e., consider record.runtimeStatus == .running ||
record.runtimeStatus == .needsInput) when deciding whether a same-scope session
exists, keeping the existing workspace/surface/excludingSessionId checks
(function: hasRunningSession, property: record.runtimeStatus).
In `@cmuxTests/WorkspaceRemoteConnectionTests.swift`:
- Around line 3071-3115: The test races because startMockServerAccepting runs
async and you assert on state.commands immediately; update the test (the block
using startMockServerAccepting, then runProcess and the XCTAssertTrue on
state.commands) to synchronize before asserting by either switching to
startMockServer and waiting on the serverHandled XCTestExpectation with
wait(for: [serverHandled], timeout: 5), or keep startMockServerAccepting and
call the waitForSocketCommand helper to wait until the expected "set_status
codex Idle" command appears in state.commands; reference
startMockServerAccepting, startMockServer, serverHandled, waitForSocketCommand
and the XCTAssertTrue that checks state.commands to locate where to add the
wait.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 18d4feed-e677-4278-b972-8e0ff9b64cc4
📒 Files selected for processing (3)
CLI/cmux.swiftResources/Localizable.xcstringscmuxTests/WorkspaceRemoteConnectionTests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
austinywang
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
CLI/cmux.swift (1)
678-678:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTreat
.needsInputas active for workspace-scoped checks.Line 678 only counts
.needsInputwhensurfaceIdis present. For workspace-level callers, another session already in Needs Input will not blockIdle, so a healthy completion can overwrite a live Needs Input state for the same workspace.Suggested fix
- && (record.runtimeStatus == .running || (normalizedSurface != nil && record.runtimeStatus == .needsInput)) + && (record.runtimeStatus == .running || record.runtimeStatus == .needsInput)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLI/cmux.swift` at line 678, The conditional that decides whether an existing session blocks an Idle completion currently only treats record.runtimeStatus == .needsInput as active when normalizedSurface (surfaceId) is present; update the check so .needsInput counts as active regardless of normalizedSurface for workspace-scoped callers. Locate the conditional referencing record.runtimeStatus, normalizedSurface and surfaceId in CLI/cmux.swift and remove the normalizedSurface-dependent gating for .needsInput (keep .running behavior the same) so a live Needs Input session blocks workspace-level Idle replacements too.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@CLI/cmux.swift`:
- Line 678: The conditional that decides whether an existing session blocks an
Idle completion currently only treats record.runtimeStatus == .needsInput as
active when normalizedSurface (surfaceId) is present; update the check so
.needsInput counts as active regardless of normalizedSurface for
workspace-scoped callers. Locate the conditional referencing
record.runtimeStatus, normalizedSurface and surfaceId in CLI/cmux.swift and
remove the normalizedSurface-dependent gating for .needsInput (keep .running
behavior the same) so a live Needs Input session blocks workspace-level Idle
replacements too.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9b0a9f7d-a01f-49f7-8102-a6fa7b3ec678
📒 Files selected for processing (1)
CLI/cmux.swift
…atus-after-interrupt
…atus-after-interrupt
…atus-after-interrupt # Conflicts: # .github/swift-file-length-budget.tsv # CLI/FeedEventClassifier.swift # CLI/cmux.swift # Resources/Localizable.xcstrings # Resources/bin/cmux-claude-wrapper # cmuxTests/FeedEventClassificationTests.swift # tests/test_bash_integration_no_done_notifications.py # tests/test_claude_wrapper_hooks.py
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort 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 3eeee97. Configure here.
| now: now | ||
| ) | ||
| state.sessions[normalized] = record | ||
| return true |
There was a problem hiding this comment.
Nested turn clears idle early
Medium Severity
recordCodexMonitorIdleIfCurrent pops the completed turn from the prompt stack but always writes agentLifecycle and runtimeStatus as idle. When activePromptDepth stays above zero after that pop, the Codex monitor still succeeds and publishCodexMonitorIdle can clear panel lifecycle and status while an outer prompt turn remains active.
Reviewed by Cursor Bugbot for commit 3eeee97. Configure here.
AsifAd
pushed a commit
to AsifAd/cmux
that referenced
this pull request
Jul 17, 2026
…lish Cherry-pick upstream interrupt/Codex-monitor work (stop-failure hook, recordCodexMonitorIdleIfCurrent, surface-scoped running checks). Fix stop-failure to clear Running to Idle instead of Completed.
6 tasks
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related
Fixes #4389. This also gives Claude StopFailure the missing status transition noted in #2488; Codex missing-Stop cases like #4276 are covered when the transcript reaches a terminal healthy completion.
Testing
Note
Medium Risk
Changes agent hook lifecycle, shared workspace status, and Codex transcript monitor concurrency guards—user-visible status can be wrong if turn/session matching regresses, but behavior is heavily regression-tested.
Overview
Fixes stale Running status when turns end without a normal Stop hook—especially after interrupts.
Claude: Adds a
stop-failurehook path wired like Stop for status/session cleanup, but skips completion summarization so failed/interrupted turns don’t look like a finished response. Thecmux-claude-wrapperinstalls StopFailure hooks; feed/telemetry treat StopFailure like Stop (non-actionable).Codex monitor: When transcript parsing sees a healthy terminal completion without Stop, the monitor now records idle runtime state, clears panel-scoped lifecycle, and publishes Idle only when guards pass (current turn, no retired lease, no other live session on the same surface/workspace as appropriate). User-input and failure paths also persist hook session state with
--cwd. Transcript scanning resets on newtask_startedand avoids treating old completed turns as the current one when monitoring unscoped turns.Session “running” checks:
hasRunningSessiontreats needs input as active on a surface; idle suppression considers other sessions per surface, not only workspace-wide.Tests / i18n: Regression coverage for wrapper StopFailure, feed classification, and Codex monitor idle/race behavior; localized Claude hook help includes
stop-failure.Reviewed by Cursor Bugbot for commit 3eeee97. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Clears stale Running after interrupts by treating Claude
stop-failureas a turn end and by having the Codex monitor set Idle on clean terminal completion, scoped by surface and guarded against stale/racing sessions. Preserves shared workspace status while clearing panel lifecycle and keeps Idle updates current by validating the active turn. Fixes #4389.stop-failuresubcommand that clears Running via the Stop path but skips completion; wrapper installs it; feed/telemetry map to Stop; CLI help includesstop-failureand is localized.--cwd; record runtime status and publish Needs Input/Error; Codex monitor sets panel Idle only for the current turn, only if no other live same-surface session, and if the lease isn’t retired; clears panel lifecycle and publishes shared Idle only if no other live workspace session; reset on newtask_started; ignore old terminal turns and keep watching the current unscoped turn.stop-failurewiring/telemetry and wrapper hook; Codex Idle without Stop with current-turn guard, stale-session suppression, live-process guard, ignoring old terminal turns, unscoped-current pending, and preserving shared status while another panel is running.Written for commit 3eeee97. Summary will update on new commits.
Summary by CodeRabbit
New Features
stop-failureClaude hook as a turn-end/stop signal and suppresses completion summarization for itDocumentation
StopFailurehookTests
stop-failurehook wiring and monitor idle/status behavior