Repository navigation
Fix agent session resume: cwd-drift + post-kill cwd, DRY resume engine, lifecycle/fork tests - #5312
Merged
Merged
Conversation
Two regression tests through RestorableAgentSessionIndex.load that fail against the current resolver: - Claude: when the transcript exists but encodes to neither candidate and the config probe can't confirm, restore must prefer the launch cwd, not the drifted recorded cwd. - A directory-namespaced non-Claude agent (Gemini) with a drifted cwd must resolve to the launch cwd; today the Claude-only gate returns the drift. These reproduce the "No conversation found with session ID" failure on session restore for cwd-namespaced agents. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Agent session stores are namespaced by the directory the agent was launched in; cmux was resuming from the agent's drifted runtime cwd (a worktree it cd'd into mid-session), so `<agent> --resume <id>` looked in the wrong namespace and failed with "No conversation found with session ID". Two independent paths carried the drift, fixed via one shared engine: New shared package logic (Packages/CMUXAgentLaunch/AgentResumeWorkingDirectory): classifies each agent kind (cwd-in-file vs directory-namespaced) and resolves the resume working directory, preferring the launch cwd for directory- namespaced agents (Claude, Gemini, Cursor, Grok, Pi, Qoder) and keeping the runtime cwd for id-keyed agents (Codex, OpenCode, Amp, ...). Linked by both the app and the standalone cmux-cli, so there is one source of truth. 1. CLI surface resume binding (publishAgentSurfaceResumeBinding) — the path that re-runs the agent on a restored surface — now pins the binding to the launch cwd via the shared resolver instead of baking the drifted runtime cwd. This is the path that actually failed on a build that already had the Claude-only #5154 fix. 2. App-side resolver (restorableWorkingDirectory) generalized beyond Claude via RestorableAgentKind.cwdNamespacing, which now delegates to the shared classifier. Claude still verifies against the transcript first. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-agent resume-verb switch was duplicated across the app target (AgentResumeCommandBuilder.resumeArguments) and the standalone cmux-cli target (agentSurfaceResumeArguments + helpers), because cmux-cli cannot import the app's Sources. Extract the launcher pre-resolution and the built-in-kind verb switch into one pure builder, AgentResumeArgv, in the shared CMUXAgentLaunch package (linked by app + cli + tests). Behavior-preserving: the app and CLI switches were verified identical, and the shared builder is golden-tested per built-in kind and launcher wrapper (AgentResumeArgvTests). The app keeps its Vault/.custom path locally (CmuxVaultAgentRegistration lives in the app target); only the built-in and wrapper resolution is shared. Deletes ~95 lines of duplicated CLI helpers. Stacks on the resume cwd-drift fix (task-session-restore-resume / #5300). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR centralizes cwd-namespace policy and deterministic resume argv construction. It adds ChangesAgent Resume Working Directory Namespacing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 17 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (17 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: 2cfdd0cc8f
ℹ️ 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".
| /// (``launcherResolution(launcher:sessionId:executablePath:arguments:)``) is checked first, then the | ||
| /// per-kind verb (``builtInKind(kind:sessionId:executablePath:arguments:)``). Callers that also | ||
| /// support custom Vault agents slot that resolution between the two. | ||
| public enum AgentResumeArgv { |
There was a problem hiding this comment.
The /workspace/cmux AGENTS.md package-design rule explicitly forbids no-case namespace enums (enum Foo { static func ... }) for new package code because they bypass DI/test seams; this new AgentResumeArgv type is exactly a static-only namespace in Packages/. Please make this a value type with instance methods that callers construct/inject (or otherwise follow the documented package discipline) instead of adding another static namespace API.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Done — converted both AgentResumeArgv and AgentResumeWorkingDirectory from no-case namespace enums to stateless value structs with public init() + instance methods; all callers (app, cli, tests) now construct an instance. 61 package tests still pass (51a65dc).
— Claude Code
Contributor
Greptile SummaryFixes the "No conversation found" resume failure caused by cwd drift (agents
Confidence Score: 5/5Safe to merge. The cwd-drift fix is well-reasoned, the DRY refactor preserves all existing argument shapes, and the new shell script The refactored resume engine is a mechanical consolidation of two parallel switch tables into No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Hook as Agent Hook
participant CLI as cmux CLI
participant ARW as AgentResumeWorkingDirectory
participant ARA as AgentResumeArgv
participant Script as Shell Script Store
participant Shell as Outer Shell
Note over Hook,Shell: Session resume flow (post-PR)
Hook->>CLI: publishAgentSurfaceResumeBinding(kind, cwd, launchCommand)
CLI->>ARW: resolve(kind, runtimeCwd, launchWorkingDirectory)
alt cwdInFile (Codex/Amp/…)
ARW-->>CLI: runtimeCwd (id-keyed: cwd lives in session file)
else byDirectory (Claude/Grok/Gemini/…)
ARW-->>CLI: launchCwd (pinned to session namespace)
end
CLI->>ARA: launcherResolution(launcher, sessionId, executablePath, arguments)
alt cmux wrapper launcher (claudeTeams/omo/codexTeams)
ARA-->>CLI: .resolved([cmd argv])
else plain agent launcher
ARA-->>CLI: .passthrough
CLI->>ARA: builtInKind(kind, sessionId, …)
ARA-->>CLI: [cmd argv] or nil
end
CLI->>Script: writeLauncherScript(command, workingDirectory: resolvedCwd)
Script-->>Shell: zsh script with cd to resolvedCwd + agent command + outer cd back + exec -l
Note over Shell: Agent runs, user kills it
Shell->>Shell: cd resolvedCwd (outer shell lands in session dir, not surface default)
Reviews (7): Last reviewed commit: "Address CodeRabbit on the combined diff:..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+86
to
+91
| // One-shot wrappers have no resumable form. | ||
| #expect( | ||
| AgentResumeArgv.launcherResolution( | ||
| launcher: "omx", sessionId: "SID", executablePath: nil, arguments: ["cmux", "omx"] | ||
| ) == .resolved(nil) | ||
| ) |
Contributor
There was a problem hiding this comment.
launcherWrappers test verifies omx returns .resolved(nil) but never exercises the omc branch. Both live behind the same case "omx", "omc" arm, so a future refactor that accidentally splits or reorders those cases would go undetected.
Suggested change
| // One-shot wrappers have no resumable form. | |
| #expect( | |
| AgentResumeArgv.launcherResolution( | |
| launcher: "omx", sessionId: "SID", executablePath: nil, arguments: ["cmux", "omx"] | |
| ) == .resolved(nil) | |
| ) | |
| // One-shot wrappers have no resumable form. | |
| #expect( | |
| AgentResumeArgv.launcherResolution( | |
| launcher: "omx", sessionId: "SID", executablePath: nil, arguments: ["cmux", "omx"] | |
| ) == .resolved(nil) | |
| ) | |
| #expect( | |
| AgentResumeArgv.launcherResolution( | |
| launcher: "omc", sessionId: "SID", executablePath: nil, arguments: ["cmux", "omc"] | |
| ) == .resolved(nil) | |
| ) |
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!
Contributor
Author
There was a problem hiding this comment.
Added — there's now an explicit omc assertion next to omx so both one-shot wrapper cases in the shared arm are exercised (51a65dc).
— Claude Code
Bug: after a resumed agent exits, the surface landed in the default surface cwd, not the session's launch directory. The resume command's `cd` runs inside the `-lic` child shell (TerminalStartupReturnShellScript), so the outer `exec -l` login shell stayed in the script's launch cwd (nil -> baseConfig default). Fix: thread the resume working directory through the launcher and `cd` the outer shell back into it before `exec -l`, so killing a resumed agent leaves you in the session's directory. Tests (cmuxTests): - testResumeLauncherReturnsToLaunchCwdAfterAgentExits: the launcher returns the outer shell to the launch dir before exec -l (and not when no dir). - testReplacementRestoresNewestSessionForSurface: spawn/replace -> restore picks the newest session for the surface, not the stale one. - testRestoreIsIdempotentAcrossReloads: repeated reopen restores the same session + command. - testKilledSessionWithDeadProcessDoesNotRestore: a session whose recorded process is dead is dropped from the restore index (via processArgumentsProvider). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?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: b101eb22ba
ℹ️ 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".
| temporaryDirectory: temporaryDirectory, | ||
| returnToLoginShell: true | ||
| returnToLoginShell: true, | ||
| workingDirectory: workingDirectory ?? launchCommand?.workingDirectory |
There was a problem hiding this comment.
When auto-resuming a Vault/custom agent whose registration has cwd == .ignore, shellCommand deliberately omits the cwd prefix, and scanners store snapshot.workingDirectory as nil, but this new fallback still passes launchCommand?.workingDirectory into the launcher script. In that scenario, after the resumed agent exits, TerminalStartupReturnShellScript now cds the outer login shell into the ignored cwd, so a cwd-ignored agent changes the terminal directory anyway; use the same cwd policy here instead of falling back to the launch command cwd for ignored registrations.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed — the outer-shell cd is now gated on registration.cwd != .ignore, so the post-kill landing matches the resume command's own cwd policy (51a65dc).
— Claude Code
![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 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
… cwd, fork tests Review feedback (#5312): - Codex P1: AgentResumeArgv + AgentResumeWorkingDirectory were no-case namespace enums, which the package design discipline forbids for new package code. Convert both to stateless value structs with init() + instance methods; callers construct an instance. (61 package tests still green.) - Codex/Cursor: the post-kill return-to-launch-dir cd bypassed the `.ignore` cwd policy. Match the resume command's own cd: when registration.cwd == .ignore the agent resumes from the current dir (no cd), so the post-exit shell must not force the launch dir either. Fork coverage (requested): forked sessions must resume properly per agent. - testForkCommandUsesPerAgentVerbAndSessionCwd: codex (`fork <id>`) and opencode (`--session <id> --fork`) forks use the right verb, cd into the session dir, and are launchable (claude fork already covered). - testNonForkAgentsProduceNoForkCommand: gemini/grok/amp/cursor still resume but emit no (malformed) fork command, pinning the fork-capable set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Review (#5300 P2): a custom Vault agent defaults to cwd: .preserve and its resume template can expand {{cwd}}, but restorableWorkingDirectory classified the unknown custom id as .byDirectory and returned the launch dir — so a custom agent that cd'd into a worktree would resume in the wrong directory. Thread the registration into restorableWorkingDirectory and keep the runtime cwd whenever a registration is present (covers both .preserve and .ignore). (The sibling antigravity P2 is already handled: .custom("antigravity").rawValue is "antigravity", which the shared cwd classifier lists as .cwdInFile.) Test: testCustomVaultAgentPreservesRuntimeCwdOnRestore — a drifted custom agent restores into its runtime cwd, not the launch dir. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/RestorableAgentSession.swift">
<violation number="1" location="Sources/RestorableAgentSession.swift:1304">
P1: All registry-backed agents now force runtime cwd, which regresses Grok/Pi resume when cwd drifts from launch directory.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Confirms fork-after-restore works end-to-end: restoreSessionSnapshot captures the agent snapshot into restoredAgentSnapshotsByPanelId, and the UI fork action (Workspace.forkableAgentSnapshot(forPanelId:)) resolves it back with a valid per-agent fork command + launchable fork startup input. (Already wired; this pins it so a future change can't silently break forking a resumed session.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 450f30298d
ℹ️ 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".
Comment on lines
+1304
to
+1305
| if registration != nil { | ||
| return recordedCwd ?? launchCwd |
There was a problem hiding this comment.
When restoring registry-owned built-ins such as Grok or Pi, registration is non-nil because those built-ins are supplied by the Vault registry rather than RestorableAgentKind.allCases; this early return therefore bypasses the kind.cwdNamespacing == .byDirectory branch below. If the agent was launched in /repo and later cd'd into a worktree, the hook record's cwd is the drifted runtime cwd, so the resume command is run from the wrong directory and directory-namespaced agents can fail to find the conversation. Apply the cwd-namespacing decision before the generic custom-registration shortcut, or exclude registry-owned by-directory built-ins from this return.
Useful? React with 👍 / 👎.
lawrencecchen
added a commit
that referenced
this pull request
Jun 4, 2026
Root cause (adversarially-verified): the CLI agent launchers stamped the operator's FOCUSED pane into CMUX_WORKSPACE_ID/CMUX_SURFACE_ID instead of the launch surface, while leaving CMUX_PANEL_ID/CMUX_TAB_ID at the launch surface. So a codex launched in surface B while surface A is focused recorded surface A and restored into the wrong surface after reload (the "jumble"). Flows through the codex-family omx/codex-teams launchers; plain `codex` is unaffected because its env comes from the matched per-surface terminal startup env. - New shared resolver CMUXAgentLaunch/AgentSpawnIdentity: prefer the launcher's OWN surface/workspace identity, falling back to the focused pane only when the launcher has none. Pure value logic with golden tests. - configureTmuxCompatEnvironment (CLI/cmux.swift): resolve via AgentSpawnIdentity so omx/claude-teams stamp the launch surface, keeping CMUX_SURFACE_ID matched with the inherited CMUX_PANEL_ID. - runCodexTeams watcher args: derive the root workspace/surface from the launch env, not focusedContext, so the codex-teams watcher records the right surface. Independent of the cwd-resolution work (#5300/#5312): that is the directory axis, this is the surface-mapping axis. Full root cause + e2e survives-reload test plan in plans/feat-codex-surface-jumble/DESIGN.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lawrencecchen
changed the title
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 53697db. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53697db65e
ℹ️ 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".
| @@ -0,0 +1,109 @@ | |||
| import CMUXAgentLaunch | |||
There was a problem hiding this comment.
These new CMUXAgentLaunch package tests are not exercised by the current workflow: I checked .github/workflows/ci.yml and the SwiftPM package-test loop still only lists CmuxFoundation, CmuxSettings, CmuxSettingsUI, and CmuxSocketControl, while the workflow comment says the cmux-unit scheme does not run SPM package test targets. As a result, regressions in the newly extracted resume argv/cwd logic can pass CI even though the tests exist; add CMUXAgentLaunch to that package list (or otherwise wire this package’s tests into CI).
Useful? React with 👍 / 👎.
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 `@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentResumeArgv.swift`:
- Around line 159-160: Normalize arguments.first before choosing the fallback so
whitespace-only argv[0] doesn't bypass fallback: in AgentResumeArgv.swift update
the executable assignment to use normalized(executablePath) ??
normalized(arguments.first) ?? fallbackExecutable (and keep tail computed the
same using Array(arguments.dropFirst())). This ensures any candidate argv[0] is
run through normalized(...) before falling back to fallbackExecutable.
In `@Sources/RestorableAgentSession.swift`:
- Around line 1300-1306: The code for custom registrations unconditionally
returns recordedCwd ?? launchCwd which violates cwd: .ignore; update the branch
that handles non-nil registration (in the method that computes
resumeWorkingDirectory/produces SessionRestorableAgentSnapshot.workingDirectory)
to check the registration.cwd policy and return nil when registration.cwd ==
.ignore, otherwise return recordedCwd ?? launchCwd; reference the registration
variable and SessionEntry.resumeWorkingDirectory /
SessionRestorableAgentSnapshot.workingDirectory so the snapshot uses the single
source of truth.
🪄 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: be26ce6a-266e-4ec0-b390-01bf2d0436c7
📒 Files selected for processing (11)
CLI/cmux.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentCwdNamespacing.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentResumeArgv.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentResumeWorkingDirectory.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentResumeArgvTests.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentResumeWorkingDirectoryTests.swiftSources/RestorableAgentSession.swiftSources/RestorableAgentTypes.swiftSources/SessionPersistence.swiftcmuxTests/AgentSessionAutoResumeSettingsTests.swiftcmuxTests/RestorableAgentSessionIndexTests.swift
…nore cwd in snapshot - AgentResumeArgv.commandParts: normalize arguments.first so a whitespace-only argv[0] falls back to the default executable instead of emitting an invalid one. - restorableWorkingDirectory: a custom registration with cwd: .ignore now yields nil workingDirectory in the snapshot (it resumes from the current dir), so downstream restore consumers don't place the terminal in a saved cwd and break the cwd-policy contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/RestorableAgentSession.swift">
<violation number="1" location="Sources/RestorableAgentSession.swift:1306">
P2: `.ignore` cwd policy can be bypassed for custom resume templates because `{{cwd}}` still falls back to launch cwd.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // no saved cwd at all (downstream restore consumers read `workingDirectory` directly, not just | ||
| // the command builder). The by-directory namespace below is only for built-in agents. | ||
| if let registration { | ||
| return registration.cwd == .ignore ? nil : (recordedCwd ?? launchCwd) |
There was a problem hiding this comment.
P2: .ignore cwd policy can be bypassed for custom resume templates because {{cwd}} still falls back to launch cwd.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/RestorableAgentSession.swift, line 1306:
<comment>`.ignore` cwd policy can be bypassed for custom resume templates because `{{cwd}}` still falls back to launch cwd.</comment>
<file context>
@@ -1299,10 +1299,11 @@ struct RestorableAgentSessionIndex: Sendable {
+ // no saved cwd at all (downstream restore consumers read `workingDirectory` directly, not just
+ // the command builder). The by-directory namespace below is only for built-in agents.
+ if let registration {
+ return registration.cwd == .ignore ? nil : (recordedCwd ?? launchCwd)
}
</file context>
This was referenced Jun 4, 2026
Merged
lawrencecchen
added a commit
that referenced
this pull request
Jun 4, 2026
…5350) (#5351) * Fix codex sessions restoring into the wrong surface after reload (#4920) Root cause (adversarially-verified): the CLI agent launchers stamped the operator's FOCUSED pane into CMUX_WORKSPACE_ID/CMUX_SURFACE_ID instead of the launch surface, while leaving CMUX_PANEL_ID/CMUX_TAB_ID at the launch surface. So a codex launched in surface B while surface A is focused recorded surface A and restored into the wrong surface after reload (the "jumble"). Flows through the codex-family omx/codex-teams launchers; plain `codex` is unaffected because its env comes from the matched per-surface terminal startup env. - New shared resolver CMUXAgentLaunch/AgentSpawnIdentity: prefer the launcher's OWN surface/workspace identity, falling back to the focused pane only when the launcher has none. Pure value logic with golden tests. - configureTmuxCompatEnvironment (CLI/cmux.swift): resolve via AgentSpawnIdentity so omx/claude-teams stamp the launch surface, keeping CMUX_SURFACE_ID matched with the inherited CMUX_PANEL_ID. - runCodexTeams watcher args: derive the root workspace/surface from the launch env, not focusedContext, so the codex-teams watcher records the right surface. Independent of the cwd-resolution work (#5300/#5312): that is the directory axis, this is the surface-mapping axis. Full root cause + e2e survives-reload test plan in plans/feat-codex-surface-jumble/DESIGN.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add e2e test for the spawn-env surface fix via a hidden debug seam testTmuxCompatEnvStampsLaunchSurfaceNotFocusedPane drives the real `cmux` binary through the real configureTmuxCompatEnvironment (via a hidden __debug-tmux-compat-env subcommand) with a mock socket reporting focused=surface A and the launcher's own env=surface B. Asserts the stamped CMUX_SURFACE_ID == launch surface B (== CMUX_PANEL_ID), not the focused surface A. Exercises the actual launcher env path, not just the resolver. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: drop stray design doc, fix debug-seam socket source, surface-scope test tab id - Remove plans/feat-codex-surface-jumble/DESIGN.md (design docs live in the control repo, not cmux). - cubic P2: debugDumpTmuxCompatEnvironment now injects the passed socketPath into the env before resolving the focused context, so it uses the socket the command was pointed at. - CodeRabbit: the e2e fixture uses a surface-scoped CMUX_TAB_ID (distinct from the workspace id) and asserts it passes through, matching the repo's surface-scoped tab identity contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run CMUXAgentLaunch package tests as a real gate The CI swift-test step ran a hardcoded package list that omitted CMUXAgentLaunch, so its tests (AgentSpawnIdentity resolver, AgentLaunchSanitizer, Hermes/RovoDev resolvers, and the resume-engine builders once they land) only compiled, never executed. The package resolves standalone via SwiftPM (no GhosttyKit/app dep), so add it to the gate. Verified locally: 59 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Capture CODEX_HOME for plain codex resume/fork (#5350) Plain codex (no cmux launcher, no resolvable PID at hook time) produced an empty launchCommand, so the agent's launch env was never captured. When codex runs under the subrouter account manager, CODEX_HOME points at ~/.codex-accounts/<account>, not ~/.codex; dropping it made resume/fork emit a bare `codex resume/fork <id>` against the default home and fail with "No saved session found". agentLaunchCommandFromEnvironment now still emits an env-only record carrying the selected launch env (CODEX_HOME, CLAUDE_CONFIG_DIR, ...) when argv is unavailable but the env is non-empty. AgentResumeCommandBuilder already prefixes that env ahead of the kind's fallback verb, so resume and fork reproduce `CODEX_HOME=<home> codex resume/fork <id>`. Default home stays nil (unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Override leaked env surface id with PID/TTY truth in agent hooks (#5333) The codex surface jumble: a launcher or inherited subprocess can leak the operator's FOCUSED pane into CMUX_SURFACE_ID, and the generic hook trusted that env surface whenever both env ids were present (the corrector was suppressed), routing the session to the wrong pane. The no-pid-gate resume binding then persisted the wrong surface across reload. The hook now always resolves the agent process's own terminal binding (TTY first, then PID) and, when it points to a different accessible surface inside the SAME workspace, overrides the ambient-env surface with that ground truth. It stays a no-op when the surface came from an explicit --surface flag, when the TTY/PID binding is unavailable (remote/SSH), or when it already agrees, so the common path and remote sessions are unaffected. Adds two app-hosted CLI integration tests: CODEX_HOME survives an unavailable launch command (#5350), and a leaked env surface is overridden by the TTY-bound pane (#5333). Both are RED before their respective fixes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Review: clean up debug-seam shim dir; make new hook tests hermetic - debugDumpTmuxCompatEnvironment leaked a /tmp shim dir per invocation (incl. each CI run of the tmux-compat seam test); it is the one-shot dump path that never spawns a long-lived agent, so defer-remove the dir on exit. (greptile, cubic) - The two new codex-hook integration tests inherited the runner HOME; set HOME to the per-test temp root before spawning cmux, matching the existing runGenericHookPersistenceScenario pattern. (cubic) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Persist the env-only launch record so fork/resume keep CODEX_HOME The hook session store's update() only assigned launchCommand when arguments was non-empty, so the new env-only record (empty argv, carrying CODEX_HOME) was never written. A later hook event that falls back to mapped?.launchCommand, or the fork path reading the persisted store after reload, could then drop CODEX_HOME and fail with "No saved session found" again. Persist an argv-less env-only record when no argv-bearing record exists yet, without ever downgrading a richer earlier capture. Extends the G2 test to assert the persisted launchCommand carries CODEX_HOME. (cursor Bugbot) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Keep AgentSpawnIdentity a coherent workspace/surface pair resolve() fell back independently per axis, so a launcher inheriting only CMUX_WORKSPACE_ID while focus is in a different workspace produced an impossible (own workspace, focused surface) pair. Call sites stamped that into the spawn env, and hook routing then rejected the surface-not-in-workspace pair and dropped the hook (no resume binding). Borrow the focused surface only when the focused pane is in the resolved workspace; otherwise leave the surface nil so the hook's PID/TTY resolution picks the agent's real pane. Composes with the G3 hook override. Updates the per-axis test to the coherent semantics and adds the same-workspace case. (autoreview Codex / greptile / codex bot) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Let a stale ambient surface fall through to PID/TTY instead of dropping the hook The G3 correction only ran when the ambient CMUX_SURFACE_ID resolved to an accessible surface. A stale/invalid ambient surface (closed, or in another workspace) made resolvedDirectSurfaceArg nil, which set hasInvalidDirectSurfaceArg and hasUnusableDirectBinding, so resolveAgentHookTarget aborted BEFORE the TTY/PID binding could recover — the stale-env variant of the codex jumble still no-op'd the hook and lost the session across reload. Treat an invalid AMBIENT env id differently from an invalid EXPLICIT flag: only an explicit --workspace/--surface that fails to resolve is a hard error; a stale ambient id is treated as absent so routing falls through to the PID/TTY binding (ground truth). Adds a stale-env regression test. (autoreview Codex P1) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Make AgentSpawnIdentity coherent in both partial-env directions The prior coherence fix handled (own workspace, no own surface) but the mirror remained: an orphan own surface (inherited CMUX_SURFACE_ID with no CMUX_WORKSPACE_ID) was still paired with the focused workspace, producing a cross-workspace (focused workspace, own surface) identity. Treat the own surface as authoritative only paired with its own workspace; an orphan own surface is not trusted against the focused context, so the resolver uses the coherent focused pair or leaves it nil for PID/TTY recovery. Adds two orphan-surface tests (8 total, all green). (autoreview Codex P2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Keep non-restorable launches non-resumable: env-only fallback only for absent argv The env-only CODEX_HOME fallback also fired when sanitizedAgentLaunchArguments REJECTED a captured argv. That rejection is exactly how AgentLaunchSanitizer suppresses non-restorable invocations (codex exec/review/login, claude config, ...), so routing them through the env-only record persisted a bogus resumable record (and could publish a default `codex resume <id>`) for one-shot commands. Restrict the env-only fallback to the genuinely-unavailable-argv case (no CMUX_AGENT_LAUNCH_ARGV_B64 and an unresolved/exited PID). A captured-but-rejected argv returns nil again, so non-restorable launches stay non-resumable even when CODEX_HOME is present. The G2 test forces the no-argv path via a dead PID, so it stays green. Adds a non-restorable-exec regression test. (autoreview Codex P1) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: lawrencecchen <lawrence@cmux.com>
This was referenced Jun 5, 2026
Merged
This was referenced Jun 19, 2026
This was referenced Jun 27, 2026
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
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.
Combined resume-engine PR (supersedes #5300, which is the first two commits here).
Fixes
cds into the directory its session is actually filed under, not the drift-prone runtime cwd. Unifies the two parallel resume mechanisms (RestorableAgentSessionIndex + SurfaceResumeBinding).cdno longer leaks)..ignorecwd policy respected on the post-kill return..preserve).Refactor (DRY)
CMUXAgentLaunch:AgentResumeWorkingDirectory,AgentResumeArgv,AgentCwdNamespacing— one source of truth for the app + cli resume paths (removed ~95 duplicated lines). Converted from namespace-enums to value structs per package-design review.Tests
CMUXAgentLaunchpackage tests now run as a real CI gate.Independent of the codex surface-jumble work (#5331, surface-mapping axis).
🤖 Generated with Claude Code
Note
Medium Risk
Touches core session restore/resume paths for many agent kinds and both app and CLI; behavior changes are intentional but broad, mitigated by extensive new tests.
Overview
Fixes agent resume failing with “No conversation found” when the hook-reported cwd drifts (e.g. agent launched in repo root, later
cdinto a worktree). Directory-namespaced agents (Claude, Gemini, Grok, …) now resume from the launch working directory; id-keyed agents (Codex, OpenCode, Amp, …) still use the runtime cwd. The same rules apply to RestorableAgentSessionIndex and CLIsurface.resume.setbindings.Extracts shared logic into
CMUXAgentLaunch:AgentCwdNamespacing,AgentResumeWorkingDirectory, andAgentResumeArgv(replacing ~95 duplicated lines in the app andcmux-cli).RestorableAgentKind.cwdNamespacingdelegates to the shared classifier.After a resumed agent exits, the outer login shell
cds back to the session directory (resume launcher scripts and surface resume bindings);.ignorecustom agents skip that forced return.Adds regression tests for cwd drift, fork/resume lifecycle, custom Vault
.preservecwd, and package unit tests as a CI gate.Reviewed by Cursor Bugbot for commit 9b80482. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Documentation