Keep resumed agent notification hooks armed during restore - #9064
Merged
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughResume-aware Claude and Codex wrappers now consume provider/session-bound restore markers, preserve authorized resume behavior across socket conditions, prevent marker leakage, and set resume diagnostics. Session persistence, tests, debug logging, CI coverage, and a workspace availability overload are updated. ChangesResume hook reliability
Conversation availability API
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant RestoreCommand
participant AgentWrapper
participant CmuxCLI
participant AgentProcess
RestoreCommand->>AgentWrapper: Pass provider/session restore marker
AgentWrapper->>AgentWrapper: Parse resume id and evaluate socket ownership
AgentWrapper->>CmuxCLI: Request resume hook arguments
CmuxCLI-->>AgentWrapper: Return hook arguments
AgentWrapper->>AgentProcess: Launch resumed agent without restore-marker leakage
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 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 |
![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 `@tests/test_codex_wrapper_resume_hooks.py`:
- Around line 19-27: Extract the shared make_executable, read_lines, and expect
helpers from test_codex_wrapper_resume_hooks.py and test_claude_wrapper_hooks.py
into a small test-utils module, then import and reuse them in both test files.
Preserve the existing helper behavior, including the canonical read_lines
newline handling, and remove the duplicated local definitions.
🪄 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 Plus
Run ID: b13d377b-c557-4449-851d-802d649d99f3
📒 Files selected for processing (8)
.github/workflows/ci.ymlCLI/cmux.swiftResources/bin/cmux-claude-wrapperResources/bin/cmux-codex-wrapperSources/DockSplitStore+SessionRestore.swiftSources/Workspace.swifttests/test_claude_wrapper_hooks.pytests/test_codex_wrapper_resume_hooks.py
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@Sources/SurfaceResumeCommandCanonicalizer`+PortableAgentExecutable.swift:
- Around line 96-128: Update applying(toStoredCommand:) so cmux-based commands
are recognized when the executable basename is cmux and the following recorded
argument is a supported agent launch kind such as teams, omc, or omx. Route
these commands through the same authorization flow instead of returning the
original cmux command, while preserving existing bare-agent and wrapper-token
behavior; if these launch kinds have a separate authorization path, explicitly
exclude them here and delegate accordingly.
🪄 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 Plus
Run ID: 205a6bc3-be04-44a8-bef9-9061abc2785e
📒 Files selected for processing (12)
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestoreLaunch.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentRestoreLaunchTests.swiftResources/bin/cmux-claude-wrapperResources/bin/cmux-codex-wrapperSources/RestorableAgentSession.swiftSources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swiftcmuxTests/ForkParentFallbackResidualTests.swiftcmuxTests/SessionPersistenceResumeBindingTests.swiftcmuxTests/SessionPersistenceTests.swifttests/test_claude_wrapper_hooks.pytests/test_claude_wrapper_mutual_shim_loop.pytests/test_codex_wrapper_resume_hooks.py
This was referenced Jul 29, 2026
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
1 participant
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
agent-hookbindings.Root cause and evidence
#8839 changed auto-resume to type the resume command into the normal interactive login shell. Both wrappers made one bounded 750 ms socket ping during that startup window and used the result to decide whether cmux hooks existed for the full agent process lifetime. A missing socket file, connection backlog, or slow ping caused permanent passthrough.
For Codex, resume does not emit SessionStart. A startup probe miss therefore removed both the wrapper-generated SessionStart and the injected Stop hook. No later event could bind the resumed session or create the unread notification. Claude used the same lifetime gate and had the same notification-loss class.
The regression stress harness launches 12 missing-socket and 12 stale-socket Codex restores. The test-only commit demonstrates that all restore launches lost SessionStart and Stop injection before the fix.
Ownership fix
AgentRestoreLaunchinPackages/macOS/CMUXAgentLaunchis the single value owner for restore authorization. It can be constructed only for Codex or Claude with a UUID-shaped session ID. It supplies the provider wrapper route, custom executable environment key, portable shell wrapper, and exactCMUX_AGENT_RESTORE_LAUNCH=<provider>:<session-id>authorization.The app composes that value with stored-command parsing in both restore paths:
SessionRestorableAgentSnapshot.resumeStartupInput()SurfaceResumeBindingSnapshotvalues whose source isagent-hookEvery authorized restore is routed through the managed per-surface wrapper. If persistence captured a live absolute provider binary, the wrapper receives that exact selection through
CMUX_CUSTOM_CODEX_PATHorCMUX_CUSTOM_CLAUDE_PATHinstead of silently selecting another binary fromPATH.Startup input transports the one-shot assignment through
/usr/bin/envbefore the routed command. This spelling works when typed into bash, zsh, fish, csh, or tcsh. Persistent resume commands and fork commands remain unchanged. Unsupported providers, invalid session IDs, CLI bindings, and other non-hook commands remain unmarked.Each wrapper consumes and unsets the token immediately. It bypasses socket health only when the token exactly matches the provider and explicit resume ID parsed from argv. Wrong-provider, wrong-session, legacy boolean, and missing-ID markers still require the live-socket ownership check.
Claude can legitimately resolve through a finite user-owned shim chain. The wrapper re-issues the same authorization only at a recognized wrapper-shim boundary, consumes it on the next wrapper pass, and unsets it before the real Claude process. This preserves hook settings and cmux surface context during a startup socket outage without leaking authorization into nested unrelated launches.
Once hooks remain installed, the existing target resolver validates current workspace and surface accessibility, prefers live TTY or PID binding over stale ambient surface state, and republishes the resolved resume binding at SessionStart and Stop.
Diagnostics
DEBUG builds record bounded, privacy-safe decisions through the existing 500-entry debug ring and tagged log:
Session and target identifiers are truncated. Message content is represented only by byte or character counts.
Tests
/usr/bin/envauthorization transport.agent-hookstartup input, live custom executables, tcsh dispatch, unsupported providers, invalid IDs, CLI bindings, persistent resume commands, and fork commands.CI also exposed an existing actor-isolation warning in
Workspace+ForkAgentConversationAvailability.swift. A separate repair removes the actor-isolated.shareddefault argument and resolves it inside a no-argument overload. No warning-budget file was changed.Scope
Fixes #9059.
#9040 is the deterministic Dock-pane ring rendering bug and is not changed here. #6184 and #1027 remain separate historical reports.