Repository navigation
Fix OMO subagent pane respawn - #5465
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place. 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 end-to-end terminal surface respawn: explicit surface ID threading through panel/registry, workspace-level respawn orchestration preserving pane/tab identity, RPC handler routing, CLI tmux-compat respawn-pane parsing and command fallbacks, and Python regression tests with a fake socket server. ChangesTerminal Surface Respawn
Sequence Diagram(s)sequenceDiagram
participant CLI
participant TerminalController
participant Workspace
participant TerminalSurfaceRegistry
participant TerminalSurface
CLI->>TerminalController: parse respawn-pane / surface.respawn
TerminalController->>Workspace: respawnTerminalSurface(panelId, command, workingDirectory, tmuxStartCommand, focus)
Workspace->>TerminalSurfaceRegistry: unregister old surface
TerminalSurfaceRegistry->>TerminalSurface: confirm instance identity / replace registration
Workspace->>TerminalSurface: create replacement panel/surface and update mappings
Workspace->>TerminalController: return respawn result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 15❌ Failed checks (1 warning, 14 inconclusive)
✅ Passed checks (4 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?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: 5
🤖 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 20191-20214: The code always calls
client.sendV2("surface.respawn", ...) even when the tmux force flag "-k" isn't
provided; change the flow in the block that builds params (after
parseTmuxArguments and after tmuxResolvePaneTarget/tmuxSelectedSurfaceId) to
enforce tmux semantics: if the parsed flags do not include "-k" (use the parsed
flag accessor, e.g. parsed.containsFlag("-k") or equivalent) then either (A)
perform a live-pane check using the target/surfaceId and refuse/throw an error
when a live process exists, or if live-pane detection is not implemented yet,
explicitly reject the non-`-k` call by returning an error message instructing
the user to pass -k; only call client.sendV2("surface.respawn", ...) when -k is
present or the live-pane check permits replacement.
In `@cmuxTests/WorkspaceSplitStartupCommandTests.swift`:
- Around line 235-271: The new test function
testRespawnTerminalSurfacePreservesPaneTabAndSurfaceIdentity was added into an
XCTestCase-based file but must be migrated in-place to the repository's Swift
Testing style; convert only this touched test to the Swift Testing API (update
the test declaration and imports to the project's Swift Testing format, e.g.,
replace XCTestCase-style usage with the project's test function signature and
any required attribute/runner calls), keep all assertions and referenced symbols
(Workspace, workspace.newTerminalSplit, workspace.respawnTerminalSurface,
TerminalSurfaceRegistry.shared, etc.) unchanged, and ensure the file compiles by
adding the necessary import and adapting any test lifecycle calls used by this
single test without bulk-rewriting other tests in the file.
In `@Sources/TerminalController.swift`:
- Around line 8700-8723: The handler currently resolves the workspace first via
v2ResolveWorkspace(params:) and then checks params["surface_id"], which biases
resolution to the preselected workspace; instead, when params contains
"surface_id" parse surfaceId (v2UUID) and then try v2ResolveTabManager(params:)
only if that returned tabManager actually owns the surfaceId (i.e. its
workspace/tab manager contains the panel); if it does not own the panel, call
AppDelegate.shared?.locateSurface(surfaceId:) to get the correct workspace/tab
manager for that surfaceId and use those for the subsequent
ws.terminalPanel(for:) check and respawn logic; retain the original fall-back
behavior (use focusedPanelId) when params lacks "surface_id". Ensure you update
the guards around surfaceId and workspace resolution so ws always refers to the
workspace that owns the surface before calling ws.terminalPanel(for:).
- Around line 8679-8750: The v2SurfaceRespawn function currently returns
user-facing error messages as bare English literals; replace each .err(...,
message: "...") call in v2SurfaceRespawn with localized strings using
String(localized: "rpc.v2.surface.respawn.<key>", defaultValue: "<English
text>") (e.g. keys like tab_manager_unavailable, invalid_focus, respawn_failed,
workspace_not_found, surface_not_found_for_id, no_focused_surface,
surface_not_terminal) and add matching entries for each key into
Resources/Localizable.xcstrings for all supported locales (English and Japanese)
with the default English text and appropriate Japanese translations. Ensure you
update every occurrence in v2SurfaceRespawn (and any duplicate messages such as
the second "Failed to respawn surface") so all user-facing error "message"
fields come from the localization API and the keys match the new xcstrings
entries.
In `@Sources/Workspace.swift`:
- Around line 14693-14703: respawnTerminalSurface currently recreates the
replacement TerminalPanel with a hardcoded GHOSTTY_SURFACE_CONTEXT_SPLIT which
can change behavior for non-split panes; instead pass the original panel's
context into the new TerminalPanel (i.e. use the existing panel's context
property such as oldPanel.context or existingPanel.context) so the created panel
preserves the launch/config context; update the TerminalPanel initializer call
in respawnTerminalSurface to use that original context value rather than
GHOSTTY_SURFACE_CONTEXT_SPLIT.
🪄 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: 88550344-7c85-4367-844f-8666d52d84db
📒 Files selected for processing (7)
CLI/cmux.swiftSources/GhosttyTerminalView.swiftSources/Panels/TerminalPanel.swiftSources/TerminalController.swiftSources/Workspace.swiftcmuxTests/WorkspaceSplitStartupCommandTests.swifttests/test_cli_omo_tmux_respawn_pane.py
Contributor
Greptile SummaryFixes the OMO subagent pane respawn by introducing a new
Confidence Score: 4/5Safe to merge after addressing the missing translations — the core respawn logic is sound and well-tested, but 18 of 20 supported locales will see English fallbacks for all new error messages. The respawn lifecycle (teardown ordering, cleanupControllerSurfaceState: false to keep string refs, stale-close guard, id reuse) is carefully constructed and covered by both Swift unit tests and the Python fake-socket regression. The one concrete gap is the string catalog: 8 new user-facing strings ship with only en and ja translations while the catalog already carries 18 additional locales, so every user outside English/Japanese will see untranslated error copy for the respawn path. Resources/Localizable.xcstrings needs translations for the 18 unsupported locales before the new respawn error strings are fully shipped. Important Files Changed
Sequence DiagramsequenceDiagram
actor Caller as CLI / tmux-compat
participant CLI as cmux CLI
participant TC as TerminalController
participant WS as Workspace
participant Reg as TerminalSurfaceRegistry
Caller->>CLI: respawn-pane -k -t pane_token [cmd]
CLI->>CLI: tmuxResolveSurfaceTarget → surfaceId (UUID)
CLI->>CLI: tmuxStoredStartCommand (surface.list) if no explicit cmd
CLI->>TC: surface.respawn(surface_id, command, tmux_start_command)
TC->>TC: v2MainSync — locate surface + workspace
TC->>WS: respawnTerminalSurface(panelId, command)
WS->>WS: capture focus/title/env/pinned from oldPanel
WS->>WS: "discardClosedPanelLifecycleState(cleanupController=false)"
WS->>Reg: unregister(oldPanel.surface)
WS->>WS: teardownSurface()
WS->>WS: "TerminalPanel(id=panelId)"
WS->>WS: "panels[panelId] = replacementPanel"
Reg-->>WS: "register(newSurface, id=panelId)"
WS->>WS: bonsplitController.updateTab / focus/select
TC-->>CLI: surface_id, workspace_id, surface_ref, type
CLI-->>Caller: OK
Reviews (6): Last reviewed commit: "fix: address omo respawn review feedback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 20216-20222: The current expression uses try? with
tmuxStoredStartCommand when building commandText, which masks any thrown lookup
errors and falls back to a shell; instead call tmuxStoredStartCommand explicitly
and handle errors separately: use tmuxStartCommand(parsed.positional) first,
then attempt tmuxStoredStartCommand(workspaceId:target.workspaceId,
surfaceId:target.surfaceId, client:client) inside a do/catch; if it returns nil
use the shell fallback, but if it throws surface or rethrow the error (or return
a .failure) so lookup failures aren’t silently converted into the login-shell
fallback—update the code around commandText and ensure the caller of
respawn-pane (or the respawn-pane handler) can receive or surface that thrown
error.
In `@Sources/GhosttyTerminalViewSupport.swift`:
- Around line 42-44: The method debugWaitAfterCommand() in
GhosttyTerminalViewSupport.swift accesses the file-private property
configTemplate (CmuxSurfaceConfigTemplate?) declared in GhosttyTerminalView,
causing a compile error; fix by moving the debugWaitAfterCommand implementation
into the same file/type that declares private configTemplate
(GhosttyTerminalView) OR add an internal accessor on GhosttyTerminalView (e.g.,
an internal computed property or method in GhosttyTerminalView that returns
configTemplate?.waitAfterCommand ?? false) and update debugWaitAfterCommand() to
call that internal bridge instead of referencing configTemplate directly.
In `@Sources/Workspace.swift`:
- Around line 14678-14680: The code is forcing template.waitAfterCommand = true
which overrides the inherited terminal's exit behavior; update the logic in the
respawn path (where inheritedConfig and CmuxSurfaceConfigTemplate are used — and
where inheritedTerminalConfig(...) is applied) to preserve the existing
inheritedConfig.waitAfterCommand if present and only set waitAfterCommand = true
when there is an explicit "remain on exit" request (e.g., a flag parameter or
explicit config field), otherwise leave template.waitAfterCommand unchanged so
close-on-exit flows and SSH/custom-program workspaces can teardown normally.
🪄 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: 84d6e867-39ca-45da-8c34-0b75ec0f62a1
📒 Files selected for processing (5)
CLI/cmux.swiftSources/GhosttyTerminalViewSupport.swiftSources/Workspace.swiftcmuxTests/WorkspaceSplitStartupCommandTests.swifttests/test_cli_omo_tmux_respawn_pane.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/test_cli_omo_tmux_respawn_pane.py (2)
288-311: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAssert
sent_textis empty before clearing it.The empty respawn test validates that
surface.respawnis called with the correct stored command, but it does not explicitly verify that nosurface.send_textcall was made. The next test (lines 338-339) includes this check. Add an assertion before line 311 to match the coverage pattern and strengthen the regression test against the "typed into placeholder" failure mode described in the PR objectives.🛡️ Suggested assertion to add before clearing state
if empty_respawn_params.get("tmux_start_command") != PLACEHOLDER_COMMAND: raise AssertionError(f"empty respawn did not preserve tmux start metadata: {empty_respawn_params!r}") + if state.sent_text: + raise AssertionError(f"empty respawn must replace the pane, not send text: {state.sent_text!r}") state.respawn_params.clear() state.sent_text.clear()🤖 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 `@tests/test_cli_omo_tmux_respawn_pane.py` around lines 288 - 311, The test does not assert that no send_text was invoked after the empty respawn; add an assertion that state.sent_text is empty (e.g., assert not state.sent_text or assert state.sent_text == []) immediately after verifying empty_respawn_params and before state.sent_text.clear() so the empty respawn case ensures no surface.send_text calls occurred (locate the block handling empty_respawn / empty_respawn_params in tests/test_cli_omo_tmux_respawn_pane.py).
394-396: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winTest relies on execution order and accumulated state.
The assertion
if len(state.respawn_params) != 2assumes exactly one priorsurface.respawncall remains in state from the OMO test. If tests are reordered or additional tests are inserted, this will break. Consider either clearingstate.respawn_paramsat the start of this test and checking for exactly 1 call, or adding a comment documenting the dependency on the OMO test's uncleaned state.♻️ Option to isolate test state
def assert_public_respawn_uses_same_surface_lifecycle( cli_path: str, socket_path: Path, fake_home: Path, state: FakeCmuxState, ) -> None: + # Clear state to isolate this test's RPC calls + state.respawn_params.clear() + state.sent_text.clear() + public = run_cli( cli_path, socket_path, fake_home, [ "respawn-pane", "--workspace", "workspace:1", "--surface", "surface:2", "--command", "echo TEST_PUBLIC", ], ) assert_success(public, "public respawn-pane") - if len(state.respawn_params) != 2: + if len(state.respawn_params) != 1: raise AssertionError(f"expected public respawn to call surface.respawn: {state.respawn_params!r}") - respawn_params = state.respawn_params[-1] + respawn_params = state.respawn_params[0]🤖 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 `@tests/test_cli_omo_tmux_respawn_pane.py` around lines 394 - 396, The test depends on prior calls accumulated in state.respawn_params; to isolate it, reset or clear state.respawn_params at the start of this test (e.g., set state.respawn_params = [] before exercising the code) and then assert that len(state.respawn_params) == 1 and use respawn_params = state.respawn_params[-1]; alternatively, if you prefer not to mutate state, change the assertion to look for at least one call (len(state.respawn_params) >= 1) and document the dependency — reference the symbol state.respawn_params and the lines that read/assign respawn_params to locate where to apply the change.
🤖 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 `@cmuxTests/WorkspaceSplitStartupCommandTests.swift`:
- Around line 269-270: The assertions use the stale originalPane captured before
respawn; after calling respawnTerminalSurface, re-fetch the current pane ID from
workspace via paneId(forPanelId: originalPanelId) (unwrap it) and then run the
two bonsplitController checks against that currentPane instead of originalPane
so tabs(inPane:) queries the up-to-date tree and validates originalTabCount and
presence of originalTabId.
---
Outside diff comments:
In `@tests/test_cli_omo_tmux_respawn_pane.py`:
- Around line 288-311: The test does not assert that no send_text was invoked
after the empty respawn; add an assertion that state.sent_text is empty (e.g.,
assert not state.sent_text or assert state.sent_text == []) immediately after
verifying empty_respawn_params and before state.sent_text.clear() so the empty
respawn case ensures no surface.send_text calls occurred (locate the block
handling empty_respawn / empty_respawn_params in
tests/test_cli_omo_tmux_respawn_pane.py).
- Around line 394-396: The test depends on prior calls accumulated in
state.respawn_params; to isolate it, reset or clear state.respawn_params at the
start of this test (e.g., set state.respawn_params = [] before exercising the
code) and then assert that len(state.respawn_params) == 1 and use respawn_params
= state.respawn_params[-1]; alternatively, if you prefer not to mutate state,
change the assertion to look for at least one call (len(state.respawn_params) >=
1) and document the dependency — reference the symbol state.respawn_params and
the lines that read/assign respawn_params to locate where to apply the change.
🪄 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: 5c11b3c8-17dd-4540-8a55-6574775ad067
📒 Files selected for processing (5)
CLI/cmux.swiftSources/GhosttyTerminalView.swiftSources/Workspace.swiftcmuxTests/WorkspaceSplitStartupCommandTests.swifttests/test_cli_omo_tmux_respawn_pane.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_cli_omo_tmux_respawn_pane.py (1)
430-435:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
tmux_start_commandin public respawn assertions.Public
respawn-paneis expected to send bothcommandandtmux_start_command; this test only verifiescommand. Add the metadata assertion to prevent partial-regression false passes.Proposed assertion addition
if respawn_params.get("command") != "echo TEST_PUBLIC": raise AssertionError(f"public respawn carried wrong command: {respawn_params!r}") + if respawn_params.get("tmux_start_command") != "echo TEST_PUBLIC": + raise AssertionError(f"public respawn did not persist tmux start metadata: {respawn_params!r}") if state.sent_text: raise AssertionError(f"public respawn must not send text: {state.sent_text!r}")🤖 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 `@tests/test_cli_omo_tmux_respawn_pane.py` around lines 430 - 435, The test currently only checks respawn_params["command"] and misses verifying the tmux_start_command metadata; update the assertions to also validate respawn_params.get("tmux_start_command") matches the expected tmux start command used for public respawns by adding an assertion like: check respawn_params.get("tmux_start_command") == <expected tmux start command> (use the actual expected string or test constant from the code under test) alongside the existing checks for respawn_params, SUBAGENT_SURFACE_ID, and "command".
♻️ Duplicate comments (1)
Sources/TerminalController.swift (1)
8680-8688:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't require an ambient
TabManagerwhensurface_idis explicit.This guard rejects the request before the explicit-
surface_idbranch can resolve ownership viaAppDelegate.shared?.locateSurface(surfaceId:). A raw RPC caller that sends onlysurface_idcan therefore get"TabManager not available"even when the target surface exists. Movev2ResolveTabManager(params:)into the no-surface_idpath so explicit targets are resolved solely from their owning surface.Suggested minimal fix
- guard let fallbackTabManager = v2ResolveTabManager(params: params) else { - return .err( - code: "unavailable", - message: String( - localized: "rpc.v2.surface.respawn.tabManagerUnavailable", - defaultValue: "TabManager not available" - ), - data: nil - ) - } - let command = v2OptionalTrimmedRawString(params, "command") @@ if v2HasNonNullParam(params, "surface_id") { guard let requestedSurfaceId = v2UUID(params, "surface_id") else { @@ ws = locatedWorkspace tabManager = located.tabManager surfaceId = requestedSurfaceId } else { + guard let fallbackTabManager = v2ResolveTabManager(params: params) else { + result = .err( + code: "unavailable", + message: String( + localized: "rpc.v2.surface.respawn.tabManagerUnavailable", + defaultValue: "TabManager not available" + ), + data: nil + ) + return + } guard let resolvedWorkspace = v2ResolveWorkspace(params: params, tabManager: fallbackTabManager) else {Based on learnings: panel/surface-targeted v2 routes should only use
v2ResolveTabManager(params:)when that manager actually owns the target; otherwise fall back toAppDelegate.shared?.locateSurface(surfaceId:)to avoid active-window bias.Also applies to: 8725-8751
🤖 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 `@Sources/TerminalController.swift` around lines 8680 - 8688, The guard that calls v2ResolveTabManager(params:) is rejecting requests too early; move the v2ResolveTabManager(params:) call into the branch that handles missing surface_id so explicit surface targets instead resolve ownership via AppDelegate.shared?.locateSurface(surfaceId:), and only use v2ResolveTabManager(params:) when the manager actually owns the tab (adjust the logic around the fallbackTabManager guard and the explicit- surface_id branch accordingly); apply the same change pattern to the similar block around the v2 routes at lines referenced (8725-8751) so explicit surface_id requests do not fail with "TabManager not available."
🤖 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_cli_omo_tmux_respawn_pane.py`:
- Around line 300-316: The test currently verifies the non-forced respawn fails
and that state.respawn_params is empty but then clears state.sent_text without
asserting it stayed empty; update the test around run_cli/assert_failure to also
assert that state.sent_text is empty (e.g., assert not state.sent_text or
state.sent_text == []) before calling state.sent_text.clear() so missing -k
cannot silently cause legacy text-injection side effects; locate the block using
run_cli, assert_failure, state.respawn_params and state.sent_text to add this
guard.
---
Outside diff comments:
In `@tests/test_cli_omo_tmux_respawn_pane.py`:
- Around line 430-435: The test currently only checks respawn_params["command"]
and misses verifying the tmux_start_command metadata; update the assertions to
also validate respawn_params.get("tmux_start_command") matches the expected tmux
start command used for public respawns by adding an assertion like: check
respawn_params.get("tmux_start_command") == <expected tmux start command> (use
the actual expected string or test constant from the code under test) alongside
the existing checks for respawn_params, SUBAGENT_SURFACE_ID, and "command".
---
Duplicate comments:
In `@Sources/TerminalController.swift`:
- Around line 8680-8688: The guard that calls v2ResolveTabManager(params:) is
rejecting requests too early; move the v2ResolveTabManager(params:) call into
the branch that handles missing surface_id so explicit surface targets instead
resolve ownership via AppDelegate.shared?.locateSurface(surfaceId:), and only
use v2ResolveTabManager(params:) when the manager actually owns the tab (adjust
the logic around the fallbackTabManager guard and the explicit- surface_id
branch accordingly); apply the same change pattern to the similar block around
the v2 routes at lines referenced (8725-8751) so explicit surface_id requests do
not fail with "TabManager not available."
🪄 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: f339791a-b8fd-4026-b18e-11dd02479bc4
📒 Files selected for processing (7)
CLI/cmux.swiftResources/Localizable.xcstringsSources/GhosttyTerminalView.swiftSources/TerminalController.swiftSources/Workspace.swiftcmuxTests/WorkspaceSplitStartupCommandTests.swifttests/test_cli_omo_tmux_respawn_pane.py
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 4ad8d8f. Configure here.
austinywang
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
June 5, 2026 20:58
Stale CodeRabbit review superseded by later commits; inline threads resolved and latest CodeRabbit check is green.
austinywang
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
June 5, 2026 20:58
Stale CodeRabbit review superseded by later commits; inline threads resolved and latest CodeRabbit check is green.
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
Sources/TerminalController.swift (1)
8680-8688:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefer
v2ResolveTabManageruntil the no-surface_idfallback path.Lines 8680-8688 reject the call before
surface_idis inspected. That means a valid explicit target can still returnunavailablewhenever the caller’s preselected/currentTabManagerdoes not resolve, even though Lines 8737-8751 correctly locate the owning workspace fromsurface_id. Only requirefallbackTabManagerin the branch that falls back to the focused surface.Proposed fix
- guard let fallbackTabManager = v2ResolveTabManager(params: params) else { - return .err( - code: "unavailable", - message: String( - localized: "rpc.v2.surface.respawn.tabManagerUnavailable", - defaultValue: "TabManager not available" - ), - data: nil - ) - } + let fallbackTabManager = v2ResolveTabManager(params: params) @@ } else { + guard let fallbackTabManager else { + result = .err( + code: "unavailable", + message: String( + localized: "rpc.v2.surface.respawn.tabManagerUnavailable", + defaultValue: "TabManager not available" + ), + data: nil + ) + return + } guard let resolvedWorkspace = v2ResolveWorkspace(params: params, tabManager: fallbackTabManager) else {Based on learnings: panel/surface-targeted routes should resolve against the owning surface/window rather than fail on the active/preselected manager first, to avoid active-window bias.
🤖 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 `@Sources/TerminalController.swift` around lines 8680 - 8688, The early guard calling v2ResolveTabManager and returning .err when it fails is executed before inspecting params.surface_id, causing explicit surface targets to be rejected; change control flow so v2ResolveTabManager (and the fallbackTabManager requirement) is deferred and only invoked in the branch that falls back to the focused/preselected surface (i.e., when params.surface_id is nil or when you intend to use the active manager), while the existing logic that resolves the owning workspace from surface_id should proceed without requiring fallbackTabManager; update the conditional around v2ResolveTabManager/fallbackTabManager accordingly so explicit surface_id resolution (the code around resolving the owning workspace later) is not blocked by an unavailable preselected TabManager.
🤖 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 `@Resources/Localizable.xcstrings`:
- Around line 13764-13770: Find the user-facing string entry whose English value
is "TabManager not available" (and its Japanese translation "TabManager
を利用できません") and replace both with a neutral, user-safe message such as "Unable to
access the target workspace" and an appropriate Japanese equivalent; ensure you
update all locale entries referencing this same string key (not just the en/ja
shown) so no API error body exposes internal implementation names and keep
internal identifiers only in logs.
In `@Sources/Workspace.swift`:
- Around line 14694-14697: The code is clearing surface notifications during an
identity-preserving respawn; adjust the cleanup flags so notifications survive
when the surface ID is preserved. In the call that currently passes
publishSurfaceClosedEvent: false, clearSurfaceNotifications: true,
requestTransferredRemoteCleanup: true, cleanupControllerSurfaceState: false,
compute clearSurfaceNotifications as !preservesSurfaceForDetach (or the
equivalent boolean that indicates whether the detach preserves the surface
identity) and route this cleanup through discardClosedPanelLifecycleState(...,
clearSurfaceNotifications: !preservesSurfaceForDetach) so unread/badge state is
retained for identity-preserving respawns.
In `@tests/test_cli_omo_tmux_respawn_pane.py`:
- Line 315: After clearing state.sent_text you must assert it is empty to guard
against accidental calls to surface.send_text; add an assertion like assert
state.sent_text == [] (or len(state.sent_text) == 0) immediately after the
state.sent_text.clear() line in the non-forced respawn test so the test fails if
legacy send_text behavior is triggered. Reference: the state.sent_text
collection and the non-forced respawn test where state.sent_text.clear() is
called; ensure the assertion mirrors the pattern used at lines 339–340 and
369–370.
---
Duplicate comments:
In `@Sources/TerminalController.swift`:
- Around line 8680-8688: The early guard calling v2ResolveTabManager and
returning .err when it fails is executed before inspecting params.surface_id,
causing explicit surface targets to be rejected; change control flow so
v2ResolveTabManager (and the fallbackTabManager requirement) is deferred and
only invoked in the branch that falls back to the focused/preselected surface
(i.e., when params.surface_id is nil or when you intend to use the active
manager), while the existing logic that resolves the owning workspace from
surface_id should proceed without requiring fallbackTabManager; update the
conditional around v2ResolveTabManager/fallbackTabManager accordingly so
explicit surface_id resolution (the code around resolving the owning workspace
later) is not blocked by an unavailable preselected TabManager.
🪄 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: 1393a7e3-77b2-427c-aeb0-df03cba86136
📒 Files selected for processing (7)
CLI/cmux.swiftResources/Localizable.xcstringsSources/GhosttyTerminalView.swiftSources/TerminalController.swiftSources/Workspace.swiftcmuxTests/WorkspaceSplitStartupCommandTests.swifttests/test_cli_omo_tmux_respawn_pane.py
austinywang
dismissed
coderabbitai[bot]’s stale review
June 5, 2026 22:00
Dismissed stale CodeRabbit review after addressed threads were resolved and CodeRabbit approved the current commit.
Merged
This was referenced Jun 8, 2026
This was referenced Jun 17, 2026
austinywang
added a commit
that referenced
this pull request
Jul 4, 2026
Add the respawn-pane/respawnp case to the Go remote daemon's tmux-compat dispatcher, mirroring the local Swift CLI (CLI/cmux.swift, added in #5465, refined in #6499): require -k, resolve the -t target to its surface, and forward to the existing surface.respawn RPC. The command after -- is shell-invoked via /bin/sh -c for the pane process (#6447) and kept raw in tmux_start_command; without a command, the surface's stored start command and then a login shell are used; -c becomes working_directory; the claude-teams sandbox opt-in recorded in CMUX_CLAUDE_TEAMS_SANDBOXED is re-exported inside the wrapping shell. Claude Code >= 2.1.183 launches agent-team teammate panes with `split-window ... cat` followed by `respawn-pane -k ...`, so over `cmux ssh` the shimmed tmux previously failed with "unsupported tmux command: respawn-pane" and teammate panes never started. Forwarding-only: no new daemon backend, and independent of the Swift-side surface.respawn env-propagation rework in #6309. Fixes #7014 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
austinywang
added a commit
that referenced
this pull request
Sep 21, 2026
…stack fixture rollout evidence `cmux respawn-pane` has sent `surface.respawn` instead of `surface.send_text` since #5465 (8cafcc3); the window-flag fixture's mock still rejected that method, so the CLI exited 1. Answer `surface.respawn`, asserting the window and surface ids, `tmux_start_command`, and that the shell-invoked command carries the user command but never the `--window` flag, which is the test's intent. The Codex interrupted-stack fixture's transcript had no `session_meta` line, so `CodexSessionResumeVerifier` (#10100) found no rollout evidence and the prompt published `surface.resume.clear`. Prepend the session line as the other Codex fixtures do. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
teamleaderleo
added a commit
that referenced
this pull request
Sep 23, 2026
* test(terminal): restore the presented-surface fixture contract so package tests compile `swift test --package-path Packages/macOS/CmuxTerminal` has not compiled on main since merge 38b32bb: TerminalSurfaceRendererCallbackTests calls `PresentedSurfaceFixture(installRendererCallbacks: false)`, but the fixture initializer only takes `windowVisibleAtCreation`. The flag came from the issue-2824 branch (77c46d0, 6fe70f6) and was dropped when the fixture was reworked for the native callback lifecycle (8008b7c, 97c6088); the merge re-applied the test call without the fixture side. Package tests only register render callbacks through the fixture (RendererCallbackTestSupport), so a fixture that skipped registration would leave `cmux_test_ghostty_renderer_present` with nothing to route. Restore the calls to `PresentedSurfaceFixture()`, the combination that was green at 0251a44. Verified locally: 294 tests in 37 suites pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(popover): stop rewriting the presentation binding during view update `ArrowlessPopoverAnchor.updateNSView` calls `coordinator.dismiss()` on every update where `isPresented` is already false. PR #13311 (01f0a50) made `dismiss()` write `isPresented = false` on the no-popover path, which SwiftUI reports as "Modifying state during view update" because updateNSView runs inside the view update. The sidebar footer mounts two anchors whose parents re-evaluate on every `selectedTabId` change, so every workspace switch emitted faults and `SidebarWorkspaceSwitchLayoutFaultTests` failed with 15 of them. Pass `resetPresentation: false` from updateNSView: the binding is already false on that branch, so the write was redundant. `popoverDidClose` still resets the binding when AppKit closes a live popover. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(cloud): satisfy the plus menu's sign-in gate and route Cmd+Y through a registered window `NewCloudWorkspaceShortcutTests` never passed: the plus menu deliberately hides its Cloud rows unless the account is signed in (#12305, mirroring the command palette and File menu), and a test `AppDelegate()` has no account flow. The XCTest version crashed the app host on `rows[1]`, xcodebuild restarted it, and the lenient gate accepted the partial run; #13178 now rejects that and #13193 migrated the suite to Swift Testing, so the failures became visible. Inject the signed-in state through the existing `isAuthenticated:` seam, add signed-out coverage of the gate, route the Cmd+Y event through a registered main window (as `testReboundKeyRoutesAndOldKeyDoesNot` does, since shortcut routing bypasses events bound to windows the delegate cannot resolve), and register a window context for the unavailable-Cloud check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(chrome): assert the composited Bonsplit chrome contract instead of a stale alpha hex `WorkspaceChromeColorTests` expected `bonsplitChromeHex` to return `#1122337F` (theme with opacity as alpha). Since b6d3470 (2026-05-19) `compositedTerminalColor` composites the theme over the window base and returns an opaque color, and 4cbb354 made that deliberate: Bonsplit derives its tab glyph contrast from the rendered backdrop, and an `#RRGGBBAA` hex reproduces the white-on-white bug it fixed. The tests kept failing unnoticed because the app-host gate only counted "unexpected" XCTest failures until the strict check (acedf3f) reached main through #12053. Exercise the `chromeBackgroundColor` seam that every production call site uses with literal expectations, verify the ambient default path against the resolver plus independent blend arithmetic so the result is deterministic under any host appearance, and keep coverage for opaque, shared-backdrop, pane-clear, pane-border, and explicit translucent chrome colors. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(tmux): wait for the main window to adopt the mirror pane before sending keys `RemoteTmuxMirrorPaneInputMappingTests` required `panel.surface.uiWindow != nil` right after selecting the workspace. A manual-I/O mirror pane spawns eagerly in its hidden bootstrap window, which `uiWindow` deliberately excludes, and the main window's portal adopts the pane host only once AppKit and SwiftUI get run-loop time; `waitForLiveSurface` returns immediately for an already-live surface, so the check ran before adoption and the four key-delivery tests failed at line 176. The failure was hidden until the strict app-host gate. Order the harness window front and pump the run loop until the surface and its native view are in that window, as the other hosted-view input suites do, and assert against the harness window instead of any window. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(browser): report the refused connection through the navigation delegate `browserPanelRetriesDiscardedRestoreAfterConnectionRefused` waited up to 30s for WebKit to fail a provisional load to a bound-but-unlistened loopback port. On the hosted app-host runners that failure never arrives: the load neither fails nor commits, so the test timed out at line 147 (no "provisional navigation failed" log line appears for it in any shard). Stop the in-flight load and report `NSURLErrorCannotConnectToHost` for the attempted URL through the panel's real navigation delegate, the pattern `BrowserFailedNavigationReloadTests` already uses. The restore bookkeeping, error page, `restore_pending` clearing, and retry policy under test run unchanged and every assertion is kept. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(cloud): scope reserved-workspace cleanup to its pending card and return the bind window PR #13202 (5d616a7) imported `CloudMachineWorkspaceAdoptionTests` and `CloudMachineWorkspaceResolutionTests` from the still-open 13141-cloud-vm-workspace branch without the production changes they assert, and its own run skipped the app-host shards, so they landed red: - `NewMachineSheetPresenter.closeReservedWorkspace` closed the whole workspace, which is a no-op for the last tab and discards user panes added next to a creating card. Port the branch behavior: remove only unadopted loading cards owned by the cancelled machine, clear that binding, keep user content, and give a last-tab loading workspace a local anchor first. `MachineCreateCoordinator` passes the created or reconciling machine id so cancelling machine X cannot clear a binding to machine Y (the tests now assert that scoping). - `v2WorkspaceCloudVMBind` now returns `window_id` alongside the workspace refs, as the bind acknowledgement test expects. - The resolution test selected "first" while the fixture's terminal key is "term-first"; use the key so the placement resolves as intended. - `CloudPaneCreationRetryTests` asserted `discarded` synchronously after the projection returned, but the coordinator applies its generation fence behind `CloudOperationContext.withPhase`'s recorder await, which suspends on a cold per-suite process. Settle with bounded yielding before asserting. Runtime behavior change (cancelled Cloud create cleanup); needs dogfood. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(cli): restore deferred socket connection, explicit SSH control options, and Codex stop idle observations Branch commit c8bfb58 ("fix: repair failures exposed by strict app-host CI", 2026-09-10) made `cmux vm dev|layout|env` finish local validation and dry runs before opening the socket, passed the caller's explicit ssh options into `userConfiguredControlOptions(fromSSHConfigOutput:explicitOptions:)` so a normalized `ControlPersist=0` from `ssh -G` is not mistaken for host customization, and published `idleObserved` for transcript-terminal prior turns before a legacy Codex Stop so the journal drops an obsolete running turn. The branch's later single-parent commit 38b32bb reverted `CLI/cmux.swift` to main's version while keeping the stricter fixtures, and PR #12053 landed that inconsistent state; the fixtures (CLIVMDevTests, CLIVMLayoutEnvTests, the SSH sharing tests, and the Codex missed-prompt Stop test) have failed since. Restore the three CLI changes. `SocketClient.configureAuthentication` and `SocketPasswordResolver` already exist on main, and the CmuxFoundation overload landed with the branch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(cli): align CLI integration fixtures with the shipped hook, SSH, and session-list contracts The main copy of `CLINotifyProcessIntegrationRegressionTests` predates several shipped contracts that the lenient app-host gate never enforced and that 38b32bb reverted on the issue-2824 branch: Claude hook acks print `{}` (#7963), Codex resume bindings require rollout evidence so fixtures carry a `session_meta` transcript (#10100), SessionStart publishes a binding so `/clear` counts start after the clear, fresh-terminal SSH startup commands are script paths that the support decoder must read, cmux control-path options follow a resolved `ssh -G` (#8308), and `ssh session list` reports the localized "remote state unavailable" summary with `--json` detail (#9971). The missed-prompt Codex Stop test now asserts the restored `agent.idle.observed` for the prior turn. Flagged for review: the three `ssh pty-attach` cases now expect only `workspace.remote.pty_bridge` once the endpoint is established, matching #12726's `preserveLifecycleForRecovery`; if pre-READY bridge failures were meant to keep reconciling, the flag should be set only after READY instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(workspace): restore the app-host repairs for fork, focus recovery, and shortcut routing Merge 8d33410 on the issue-2824 branch took main's copy of `cmuxTests/WorkspaceUnitTests.swift` wholesale and discarded the branch's September repairs (c8bfb58, c402b9c, 2127d97); PR #12053 then landed without them while the strict gate started counting the failures. Restore them against current production, plus the shortcut-suite fixes: - Fork in a remote workspace: `sshBootstrapArguments` has used `/usr/bin/ssh` since #9114, and agent socket propagation requires a socket that exists on disk (3bf87e3), so bind a real unix socket and expect the absolute path. - Fork Conversation context actions dispatch asynchronously (#7259, #8173); await the fork panel before asserting. - Git branch and pull request updates publish through `sidebarObservationPublisher` since #6226, not `objectWillChange`. - Config sanitization: `addWorkspaceIfActive` rebuilds templates from the font-size lineage since #8543; override the lineage hook instead. - Focus recovery: AppKit focus is authorized only for a registered, selected workspace whose window carries the main-window identity; use the `TerminalPortalTestWorkspace` fixture and the same registration/pump path as `WorkspaceTerminalFocusRecoverySwiftTests`. - Shortcut routing: clear both Cloud defaults after 9c2ba78 swapped them; neutralize Ghostty's imported goto_split fallback (⌘] by ANSI keyCode) in the unshifted-symbol test and assert the digit shortcut does not match; wait for the async runtime start before judging keyDown forwarding (skip loudly without a live surface); wait for the portal to mount before the second-Escape check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(restore): keep a restore identity when the persisted surface id collides Since #13098 (e0e77eb), `newTerminalSurfaceOutcome` treats a nil `restoredSurfaceId` as an interactive create and routes it to the selected pane's Cloud source. Session restore passed nil whenever the persisted panel id was still live (duplicate-workspace or restore-into-live), so a legacy managed-Cloud SSH workspace restored into a live manager was turned into a remote tab create: the scaffold panel stayed startup-suppressed with no initial command and `TabManagerSessionSnapshotTests` failed to unwrap it. Mint a fresh UUID on collision instead of nil; it is free by construction, so the restore keeps its identity and the old-to-new remap works as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(terminal): align snapshot, projection, terminal, and browser fixtures with shipped contracts Stale expectations and fixed-spin timing in app-host suites that the lenient gate never counted: - Cloud-projected panes restore as manual-mirror reservations with a staged remote identity (#12675), not a local placeholder projection. - Restore reuses the persisted runtime id when free (aff0e32), so assert liveness rather than a new id; the catalog ignores writes for a Cloud machine without a registered provider (#11877), so register the fixture provider before publishing. - Wheel sync requires an authoritative scrollbar response (bbc3edf); the fixture now answers like `AuthoritativeScrollbarSurfaceView`. - Search overlay mount, first-responder focus, runtime creation, and the visibility-restore redraw run through deferred main-actor tasks; wait for them with the class's `waitUntil` helpers instead of fixed run-loop spins. - Owning the socket path lock is definitive (959f38a): a refused inode left by a dead listener is replaced, so the restarted listener accepts. - The split-divider hit band extends `dividerHitExpansion` past the divider (667cc43); derive the pass-through boundary from the constant. - Browser page background blends against Ghostty's effective terminal color scheme, which is host dependent; read the same preference the product uses. - Cloud Machines defaults on in dev builds (#12318); pin the toggle off for the default-mode palette contract. - Prepared navigation requests keep the caller's cache policy (#13003). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(browser): align lifecycle, identity, host-view, loopback bridge, and portal rebind tests with shipped contracts Browser app-host suites that the lenient gate never counted: - `BrowserPanelWebViewLifecycleTests`: out-of-range discard delays are rejected to the default (the cmux.json loader relies on the nil), and the panel's own `isLoading` stays true for the indicator floor, so wait for both flags and assert no discard blockers before discarding. - `browserNavigationUsesEmbeddedWebKitIdentity`: WebKit reports the native identity as nil or "" (#9482); accept either. - `WindowBrowserHostViewTests`: production routes Dock-divider hits by yielding to AppKit so the live sidebar tracker receives them (#10902, e2e3818); the tests asserting the portal owns and forwards the hit were merged red against a design that never shipped. Realign the stale-frame test to the pass-through contract and remove the four own-and-forward tests with their fixtures. **Flagged for review:** if own-and-forward is still wanted, that is a hit-testing product change for its own PR. - `testRemoteWorkspaceRuntimeBridgeAliasesMultipleLoopbackPortsFromSamePage`: the navigation delegate restarts main-frame loads to apply the user-agent policy (11c6efe); a direct `loadHTMLString` with an HTTP base skipped that step, so the data load was cancelled and replayed as a deferred request. Apply the identity first and assert the document is current. - `portalRebindPreservesDocumentAndRoutesRefreshToTheSameWebView`: the portal host is a theme-frame sibling of `contentView` for non-glass windows (#12929); assert same-window instead of descendant-of-contentView. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(terminal): wait for asynchronous runtime creation in the remaining direct-interaction tests The same "Expected runtime surface before ..." precondition that 9f258dd made wait for the deferred runtime start still sampled after a fixed run-loop spin in the detach-race, close-lifecycle, repeat-key, and repeat-IME tests, and CI on the branch head showed them failing that way. Use the class's `waitUntil` for those four sites too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(cli): model surface.respawn for respawn-pane and give the Codex stack fixture rollout evidence `cmux respawn-pane` has sent `surface.respawn` instead of `surface.send_text` since #5465 (8cafcc3); the window-flag fixture's mock still rejected that method, so the CLI exited 1. Answer `surface.respawn`, asserting the window and surface ids, `tmux_start_command`, and that the shell-invoked command carries the user command but never the `--window` flag, which is the test's intent. The Codex interrupted-stack fixture's transcript had no `session_meta` line, so `CodexSessionResumeVerifier` (#10100) found no rollout evidence and the prompt published `surface.resume.clear`. Prepend the session line as the other Codex fixtures do. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(socket): keep mobile.panel.artifact.fetch off the local socket and advertise the served artifact reads 02c1ba4 removed `mobile.panel.artifact.fetch` from the socket worker methods because it needs the authenticated mobile execution context, but that half of the change was lost in a merge: the policy still routed fetch to the worker lane, which has no handler for it, so the local socket answered `internal_error` instead of the `method_not_found` boundary that `TerminalControllerSocketSecurityTests` pins. Restore the removal. `system.capabilities` never advertised `mobile.panel.artifact.stat` and `.thumbnail` although both are served on the worker lane (04ff18e added them only to the test's expectation); advertise those two and keep fetch out. The remote relay allowlist is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(remote): align tmux seed transport, SSH, socket command, and port scanner fixtures with shipped behavior - `RemoteTmuxPaneSeedTransportTests`: manual-I/O mirror panes spawn eagerly (#9272) and stay runtime-backed after portal churn (#9769), so the pane renders its assigned grid before a seed arrives and nothing was retained. Publish a larger tmux pane grid than any test surface applies so the retention under test sees the lag it exists for. - `SSHRemoteCWDRegressionTests`: the persistent-PTY exec helper runs only with `protectsFromHangup: true` (cd9f34f); pass it, and widen the first-spawn guard. - `SSHDeepSleepReattachTests`: a Cloud-owned workspace rejects launch overrides on splits (#13098); create the custom-identity pane before configuring the remote connection. - `SSHConfiguredRemoteCommandHostTests`: ssh-pty-attach validates the bridge `daemon_version` before dialing (#12726); the mock now reports one. - `CloudManualMirrorTransportTests`: the pane failure card uses the short title since 9bf6cb8. - `SurfaceSocketCommandTests`: `vm.workspace_new` admits its optimistic workspace through the active main window (#13152, #13155), so bind a bare window to the fixture context; a receipt without a starter terminal costs one snapshot (6d43ea6). - `PortScannerPublicationTests`: the forced-result acknowledgement hops off the main actor, so an unchanged port set may be deduped under a later refresh; drain publications until the retirement lands. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test: align mobile artifact fetch lane assertion * repair: close remaining full-suite contracts * test(restore,sidebar): align two stale contracts with shipped behavior testRemoteWorkspaceAutoResumeKeepsRemoteStartupCommand asserted the restored remote panel's local spawn cwd equals the remote-host path. It never can: OneShotTerminalLauncherStore.enterableWorkingDirectory rejects a path that is not locally enterable, which is what keeps the owning shell valid (#7031). The remote cwd does survive restore — as the panel's trusted remote directory report, and as the `cd` prefix the resume input carries (both already asserted). Assert it where it actually lives and pin the spawn cwd to nil. testSidebarPullRequestsTrackFocusedPanelOnly expected a background panel's PR to be hidden from sidebarPullRequestsInDisplayOrder(). That list is documented as the workspace's deduplicated rows in pane/tab order, both consumers (taskStatusSignals, the control-sidebar snapshot) want every panel, and the sibling branch test asserts the same all-panel model. Focus scoping lives in the `pullRequest` binding, which the test already covers. Renamed to match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: give Cloud catalog tests live destination workspaces #13196 made SurfaceCatalog.validateOwnership refuse a destination that is not a live workspace. That rule stays. Tests that projected, restored or opened browsers into a made-up workspace ID now failed with destinationNotFound, timed out waiting on a provider that was never called, or passed a later check for the wrong reason. LiveWorkspaceFixture registers real Workspace objects and hands the catalog a CloudWorkspaceRenameService that resolves them, the way the app's composition root does. Its workspaces() list stays empty so the native projection coordinator does not start mirroring into them. For tests on SurfaceCatalog.shared, withAppRegistration registers the TabManager as a windowless main-window context for the test body. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: normalize pbxproj after live workspace fixture Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: admit agent renames of agent-owned accepted Cloud names e6926fb (#13403) made submitCloudPanelRename run admitsTerminalRename for automatic names. Its last clause only admitted replacing an accepted name when the local panel still carried `.auto` provenance, but since 1e1d319 an accepted daemon name reconciles locally as `.remote` and the owner lives in the tab's nameAuthority. Every agent title after the first accepted one was refused, so "Failed agent rename keeps the accepted title", "Mirroring an agent-named placement does not block its next agent title" and "An older automatic result and old snapshots cannot replace an accepted name" failed on their second agentName call. Admit an automatic rename when no write is pending and the accepted tab name is owned by the daemon's `auto` authority. User-owned names, pending writes and local user titles on any projection still refuse it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: expect adopted machine rows to keep the pending create identity #12919 (021f792) made New Machine creation optimistic: once a running create's machine appears in the fleet list or catalog, its row keeps the `pending-machine:<operation>` node ID so selection and expansion survive adoption (see committedProjectionKeepsThePendingNodeIdentityUntilFleetAdoptsIt). pendingRowStepsAsideOnceItsMachineHasARow still expected `machine:<id>`. Assert the new identity and that the row is the adopted machine, not a stand-in: the stand-in is gone, one row remains, and it shows the created machine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: restore per-display noVNC targets and refresh stale Cloud tests Product (#13196 regression): the 6db7593 main merge into 13192-cloud-display-ownership took main's CmuxTuiSurfaceProviders.swift and CloudPortRoutePlan.swift, dropping eb3edd7's per-display ports. 23c807b restored the display coordinator but not these hunks, so withPrivateBrowserURL rewrote every display to 6901 and a daemon pointer without a discovered target fell back to display 1. Restore both hunks and drop the duplicate port-less privateDesktopURL overload. Tests: - CloudDisplayCatalogTests: 178d35e (#13196) made every guest command run `list` as a readiness probe, so fakes dispatching on " list" answered creation with the list catalog. Dispatch on the create action line, and pin the command shape. - CloudPortOpenRegressionTests: 178d35e (#13196) filters RFB/noVNC ports only when the display catalog owns them (displayPortsOwned). Assert both the desktop and non-desktop results. - CloudTreeOneMachineManyWorkspacesTests: #12740 added a final Resources section under each machine. Expected trees include it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: publish every guest display from daemon graph updates The 6db7593 main merge into 13192-cloud-display-ownership (#13196) put back main's [desktopDisplayResource()] pools in CmuxTuiSurfaceProvider's refresh, publish and delta paths. 23c807b restored the display coordinator lifecycle but not these pools, so a display created beyond display:1 vanished on the next daemon publish, and a delta that touched it removed it. Restore eb3edd7's displayResources pools, the display-kind delta check, and the injectable displayCoordinator. Drop the now-unused desktopDisplayResource(). Adds a test that creates display:2 through the provider and asserts that both displays and their ports survive a full publish and a display delta. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: fence the fake Cloud projection reply with its mutation cursor aDetachedTerminalDropsItsStaleTabBeforeMoving installs a daemon graph since c402b9c (#13403). When the placement lane drains it reconciles against that graph, which predates the projected tab, and clears tab_projected. The real reply always carries a mutation cursor (CmuxTuiSnapshotParser.placedTab requires one) that fences exactly this; the fake returned none. Give the fake a projectCursor and set it ahead of the installed graph. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: register the restored window before relinking Cloud projections #13196 made SurfaceCatalog.validateOwnership refuse a destination the app cannot resolve, so SurfaceCatalog.shared no longer relinked a restored projection into a TabManager that no main window owns. Register it for the test body with LiveWorkspaceFixture.withAppRegistration, as #13651 does for CloudClosedPanelRestoreTests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: wait for the relay ports kick, not the first relay line Since #8442, a relay prompt also reports shell state. Both RPCs run in separate background children, and the zsh prompt-refresh test waited only until the log was non-empty, so it could read report_shell_state alone. Wait (deadline-bounded) for the ports_kick line itself, in the zsh test and its bash sibling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cli): relay output of an SSH session that ends right after auth The expect wrapper watches the first two seconds after sending the password for a rejection with log_user 0. A session that authenticates and exits inside that window hit the eof branch and its output was dropped. Flush the buffered output before exiting. #13207 replaced the test's 9 s sleep with a FIFO release, which is what exposed this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: restore the no-connection path for rejected vm dev input 4120e35 taught runVMDev that invalid input never connects: keep the listener open, then check its backlog after the CLI exits. #13207 dropped that again, so each rejected run waited 60 s for a mock-server expectation that only the listener closing (after the wait) fulfills. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: drain the terminal while the SCP host-key failure runs The pty case read the master only after the CLI exited. A pty's output queue holds about 1 KiB and the host-key failure report is longer, so the CLI blocked writing stderr until the 30 s timeout. Read the master on a thread while the CLI runs. The test starts its own sshd; no runner host dependency is involved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: fail fast when a gated Cloud call ends before its fake is entered Every app-host restart in the 09-21/09-22 shard logs I sampled was a Swift Testing time-limit hit in one of two suites, and each hit relaunches the test host: - SurfaceCatalogTests: when catalog.project threw before reaching the provider (destinationNotFound, fixed by 3dc91b2), each gated test parked in MaterializeGate.waitUntilEntered() until the 300 s limit. Five tests restarted the host one after another, about 25 minutes per shard. - CloudDisplayCatalogTests: when the fake no longer recognized the create command (fixed by 70eaf44), create() failed before the exec started and `await started.result` parked until the 60 s limit. The waits now also end when the caller's task finishes, so the next such setup failure is an ordinary failed #require. The cancellation test also waits on its own cancellation signal instead of a 60 s Task.sleep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: reveal a retired terminal through the portal rebind, as the app does Since #12607 hiding a terminal removes its hosted view from the window, and only a bind reinstalls it. The test flipped portal visibility on the detached view, so no size commit could ever land and the final shrink check failed every run. Rebind like TerminalPortalReconciliation and drop the wait loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: pin key-window status in terminal focus suites The app-host test process runs headless and is usually not the active app, so makeKeyAndOrderFront never makes a programmatic window key. Terminal focus paths gate on isKeyWindow (automatic first-responder apply, focus redraws, deferred focus reapply, ensureFocus window activation), so these suites passed only when an earlier test in the shard had activated the app. Share the existing KeyStatusTestWindow and use it in WorkspaceTerminalFocusRecoveryTests, WorkspaceTerminalFocusRecoverySwiftTests and TerminalNotificationDirectInteractionTests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: report which layer holds off-plan tmux mirror geometry offPlanGeometryWithUnchangedSizingInputsReconverges fails every run with all three output-parity re-arms spent and the hosted view still at the perturbed 0.8 divider. A standalone bonsplit replay of the same sequence converges, and a sibling test without a bound (portal-visible) workspace heals the same displacement. Add the live split view's arranged widths and the split model's imposed extent to the failure message so the next run shows whether bonsplit refused the apply, the imposition was cleared, or the portal did not follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: restore Cloud projections before the workspace is published TabManager.restoreSessionSnapshot restores each workspace's surface projections before it assigns tabs, so SurfaceCatalog.restore's ownership check could not find the destination and silently dropped every restored remote projection. Check ownership against the workspace being restored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: repair stale and broken app-host expectations - CLI ssh attach: count identity UUIDs exactly; #11497 added an auth token uuidgen - device mirror directories: give the fake device trusted presence (e3d424c gate) - font zoom mirrors: deltas from the 8pt inherited base (e6926fb arithmetic) - Computer Use refresh: fake daemon reply was invalid JSON in a raw string (#13599) - quit alert: compare button alignment rects, not padded frames - remote split cwd rescue: cwd travels as CMUX_REMOTE_INITIAL_CWD since #12054 - default freestyle split: Cloud-owned splits route to Cloud since fa5dc4c Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: activate the app host before waiting for terminal focus `focusTerminalForTesting` waits for `window.isKeyWindow` before it hands first responder to the terminal, but `makeKeyAndOrderFront` only makes a programmatic window key while the test host is the active app. The app-host process starts inactive under `xcodebuild test`, so the wait succeeded only when an earlier test in the shard happened to activate the app. Its callers use a real `createMainWindow()` window, which cannot be swapped for `KeyStatusTestWindow` the way the pinned focus suites were. Activate explicitly, and give the wait a CI-appropriate timeout: the activation and the key-window transition land on later main run-loop turns, and the pump's one-second default expires before the window goes key on a contended runner. Observed on run 35743588302: `plainTerminalTextDoesNotResolveAppShortcutContext` (shard 1/6), `keyboardCopyModeKeyClearsTerminalUnread` and `workspaceFontSizeShortcutPreservesBackgroundTerminalUnread` (shard 2/6) all fail at this helper's return value, and shard 6/6 shows the same process state directly as `NSApp.keyWindow -> nil`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: give standalone terminal fixtures a live portal authority `setVisibleInUI` and `setActive` fold their request through `Workspace.portalRenderingEnabled(for:)`, which denies any workspace id the app delegate cannot resolve to a selected tab. Three direct interaction tests build their surface with `tabId: UUID()`, so once any earlier test installs an `AppDelegate.shared` the authority denies the portal, the hosted view is never actually made visible or active, and the fixture stops exercising the behavior it asserts: the surface never takes Ghostty focus and never schedules a visibility-restore redraw. Register a real selected workspace and build the surface with its id, so the fixture gets the same authority the app grants the selected tab. This is the fixture, not the product: the authority check is deliberate, and denying an unresolvable workspace is what keeps queued portal callbacks from reviving an inactive workspace. Fixes on run 35743588302 shard 4/6: `testKeyDownRecoveryDoesNotReplayFocusAfterResponderMovesAway`, `testVisibilityRestoreRefreshesSurfaceWhileTerminalIsInactive`, and `testDirectFirstResponderFocusRefreshesCursorStateAfterForeignResponder`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: normalize project.pbxproj after merging main Run 35763999808 failed `Fast static checks` before any macOS job could start, so the app-host test fixes on this branch were never exercised: error: cmux.xcodeproj/project.pbxproj is not normalized. Run scripts/normalize-pbxproj.py to fix. The branch head alone checks clean. CI builds the merge of this branch into main, and that merge is what leaves the file unnormalized, so the failure does not reproduce without merging main first. The change is one build-phase entry moving back into alphabetical order. `LiveWorkspaceFixture.swift in Sources` keeps the same UUID and the same occurrence count, and `scripts/lint-pbxproj-test-wiring.sh` still reports ok across 1049 test files, so no target lost a source file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Drop a cancelled fork-probe request, and name the inputs behind the last two app-host failures (#13744) * fix: drop a cancelled fork-probe request while it waits behind an active probe A second fork-availability request for the same panel with a different fallback snapshot parks in `applyPendingForkValidations`' contention branch: it restores its request to the pending queue and awaits the active probe. That wait had no cancellation handler, unlike every other wait in this type, so a cancelled caller kept its request in the queue until the active probe finished. The probe's completion restarts the single-flight refresh for whatever is still pending, and the cancelled request rode along: its fallback was probed and its result replaced the surviving request's validation for that panel. Give the wait the same cancellation handler its siblings have, and drop the waiter's own pending requests when it is cancelled, so the restart that follows the active probe sees only live requests. Covers cancelledSharedForkProbeRefreshPreservesSurvivingFallbackSnapshot, which failed when the restart won the race against the cancelled task's own cleanup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: name the input behind the off-plan re-arm and the sidebar reveal double pass Both failures currently report only their outcome, and each has two incompatible explanations that the message cannot tell apart. offPlanGeometryWithUnchangedSizingInputsReconverges reports `rearms=3` with `imposed=nil`. That is either three recovery passes that ran and failed to impose, or a re-arm budget already spent before the perturbation, in which case no recovery pass ran at all. Bracket the recovery window with the existing DEBUG sizing counters and report the budget at perturbation time plus the planned outers. visibilityToggleKeepsAppKitTableContainerMounted reports exactly two projections per row. Report how many the first run-loop turn produced and which async signals landed inside the reveal window (the workspace directory channel, workspace order, the shared agent index), since the hidden phase queues main-queue work that can land during the reveal. No assertion changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * test: repair the remaining app-host failures and the shard-2 host relaunches (#13759) Most of these share one cause: the app-host test process runs headless and is not the active app, so AppKit and WebKit withhold state the tests assumed. NSApp.keyWindow is nil, makeKeyAndOrderFront never makes a window key, occlusionState never carries .visible, and behind XCTest's shielding window WebKit suspends requestAnimationFrame entirely. The four shard-2 host relaunches were one hung await: renderMarkdown awaited two animation frames inside callAsyncJavaScript (cd9f34f), which behind the shielding window never fire, so four MarkdownPanelTests cases each hung until XCTest's five-minute allowance killed the host. Every WebKit call in that file now fails fast with a stated reason instead of hanging, renderMarkdown waits on the viewer's own render contract, and the scroll restore the rAF await was papering over no longer clobbers a scroll that lands after a content update. Two real product regressions: windowless shortcut events stopped pruning the orphaned main-window context (6d7b23f), and ticket minting read the routes and the v2 installation identity as two separate cache reads. No assertion is weakened; several are strengthened. Rebased onto fix/app-host-green after 87a7016 landed a different remedy for the same key-window cause. The three focus tests this branch fixed through a test-target MainWindowKeyStatusPin (a swizzle of NSWindow.isKeyWindow, needed because CmuxMainWindow is final) are exactly the three that commit fixes by activating the app host and widening the pump's timeout. The pin is dropped: activating the host makes isKeyWindow true for real rather than forcing the answer, and a process-wide swizzle installed for one helper is a worse neighbour to the rest of the shard. cmuxTests/AppDelegateMainWindowTestingSupport.swift is no longer touched by this branch. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * test: measure Cloud title ink beyond the icon and trace measured invalidations Real rendering at 75% and 1x reproduces disconnected icon ink misidentified as the title. Retain the alignment tolerance and verify that displaced text still fails. Enable DEBUG tracing only during the existing sidebar and minimal-mode measured intervals. Count assertions and timing remain unchanged. * test: preserve carrier preparation before fleet discovery * fix: retain independent cloud carrier prewarming --------- Co-authored-by: austinpower1258 <austinwang115@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
teamleaderleo
added a commit
that referenced
this pull request
Sep 25, 2026
…meouts (#14210) * test(terminal): restore the presented-surface fixture contract so package tests compile `swift test --package-path Packages/macOS/CmuxTerminal` has not compiled on main since merge 38b32bb191: TerminalSurfaceRendererCallbackTests calls `PresentedSurfaceFixture(installRendererCallbacks: false)`, but the fixture initializer only takes `windowVisibleAtCreation`. The flag came from the issue-2824 branch (77c46d0c02, 6fe70f6f68) and was dropped when the fixture was reworked for the native callback lifecycle (8008b7c062, 97c60889b4); the merge re-applied the test call without the fixture side. Package tests only register render callbacks through the fixture (RendererCallbackTestSupport), so a fixture that skipped registration would leave `cmux_test_ghostty_renderer_present` with nothing to route. Restore the calls to `PresentedSurfaceFixture()`, the combination that was green at 0251a448c9. Verified locally: 294 tests in 37 suites pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(popover): stop rewriting the presentation binding during view update `ArrowlessPopoverAnchor.updateNSView` calls `coordinator.dismiss()` on every update where `isPresented` is already false. PR #13311 (01f0a503b9) made `dismiss()` write `isPresented = false` on the no-popover path, which SwiftUI reports as "Modifying state during view update" because updateNSView runs inside the view update. The sidebar footer mounts two anchors whose parents re-evaluate on every `selectedTabId` change, so every workspace switch emitted faults and `SidebarWorkspaceSwitchLayoutFaultTests` failed with 15 of them. Pass `resetPresentation: false` from updateNSView: the binding is already false on that branch, so the write was redundant. `popoverDidClose` still resets the binding when AppKit closes a live popover. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(cloud): satisfy the plus menu's sign-in gate and route Cmd+Y through a registered window `NewCloudWorkspaceShortcutTests` never passed: the plus menu deliberately hides its Cloud rows unless the account is signed in (#12305, mirroring the command palette and File menu), and a test `AppDelegate()` has no account flow. The XCTest version crashed the app host on `rows[1]`, xcodebuild restarted it, and the lenient gate accepted the partial run; #13178 now rejects that and #13193 migrated the suite to Swift Testing, so the failures became visible. Inject the signed-in state through the existing `isAuthenticated:` seam, add signed-out coverage of the gate, route the Cmd+Y event through a registered main window (as `testReboundKeyRoutesAndOldKeyDoesNot` does, since shortcut routing bypasses events bound to windows the delegate cannot resolve), and register a window context for the unavailable-Cloud check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(chrome): assert the composited Bonsplit chrome contract instead of a stale alpha hex `WorkspaceChromeColorTests` expected `bonsplitChromeHex` to return `#1122337F` (theme with opacity as alpha). Since b6d3470668 (2026-05-19) `compositedTerminalColor` composites the theme over the window base and returns an opaque color, and 4cbb354cfc made that deliberate: Bonsplit derives its tab glyph contrast from the rendered backdrop, and an `#RRGGBBAA` hex reproduces the white-on-white bug it fixed. The tests kept failing unnoticed because the app-host gate only counted "unexpected" XCTest failures until the strict check (acedf3f244) reached main through #12053. Exercise the `chromeBackgroundColor` seam that every production call site uses with literal expectations, verify the ambient default path against the resolver plus independent blend arithmetic so the result is deterministic under any host appearance, and keep coverage for opaque, shared-backdrop, pane-clear, pane-border, and explicit translucent chrome colors. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(tmux): wait for the main window to adopt the mirror pane before sending keys `RemoteTmuxMirrorPaneInputMappingTests` required `panel.surface.uiWindow != nil` right after selecting the workspace. A manual-I/O mirror pane spawns eagerly in its hidden bootstrap window, which `uiWindow` deliberately excludes, and the main window's portal adopts the pane host only once AppKit and SwiftUI get run-loop time; `waitForLiveSurface` returns immediately for an already-live surface, so the check ran before adoption and the four key-delivery tests failed at line 176. The failure was hidden until the strict app-host gate. Order the harness window front and pump the run loop until the surface and its native view are in that window, as the other hosted-view input suites do, and assert against the harness window instead of any window. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(browser): report the refused connection through the navigation delegate `browserPanelRetriesDiscardedRestoreAfterConnectionRefused` waited up to 30s for WebKit to fail a provisional load to a bound-but-unlistened loopback port. On the hosted app-host runners that failure never arrives: the load neither fails nor commits, so the test timed out at line 147 (no "provisional navigation failed" log line appears for it in any shard). Stop the in-flight load and report `NSURLErrorCannotConnectToHost` for the attempted URL through the panel's real navigation delegate, the pattern `BrowserFailedNavigationReloadTests` already uses. The restore bookkeeping, error page, `restore_pending` clearing, and retry policy under test run unchanged and every assertion is kept. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(cloud): scope reserved-workspace cleanup to its pending card and return the bind window PR #13202 (5d616a7f44) imported `CloudMachineWorkspaceAdoptionTests` and `CloudMachineWorkspaceResolutionTests` from the still-open 13141-cloud-vm-workspace branch without the production changes they assert, and its own run skipped the app-host shards, so they landed red: - `NewMachineSheetPresenter.closeReservedWorkspace` closed the whole workspace, which is a no-op for the last tab and discards user panes added next to a creating card. Port the branch behavior: remove only unadopted loading cards owned by the cancelled machine, clear that binding, keep user content, and give a last-tab loading workspace a local anchor first. `MachineCreateCoordinator` passes the created or reconciling machine id so cancelling machine X cannot clear a binding to machine Y (the tests now assert that scoping). - `v2WorkspaceCloudVMBind` now returns `window_id` alongside the workspace refs, as the bind acknowledgement test expects. - The resolution test selected "first" while the fixture's terminal key is "term-first"; use the key so the placement resolves as intended. - `CloudPaneCreationRetryTests` asserted `discarded` synchronously after the projection returned, but the coordinator applies its generation fence behind `CloudOperationContext.withPhase`'s recorder await, which suspends on a cold per-suite process. Settle with bounded yielding before asserting. Runtime behavior change (cancelled Cloud create cleanup); needs dogfood. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(cli): restore deferred socket connection, explicit SSH control options, and Codex stop idle observations Branch commit c8bfb580ba ("fix: repair failures exposed by strict app-host CI", 2026-09-10) made `cmux vm dev|layout|env` finish local validation and dry runs before opening the socket, passed the caller's explicit ssh options into `userConfiguredControlOptions(fromSSHConfigOutput:explicitOptions:)` so a normalized `ControlPersist=0` from `ssh -G` is not mistaken for host customization, and published `idleObserved` for transcript-terminal prior turns before a legacy Codex Stop so the journal drops an obsolete running turn. The branch's later single-parent commit 38b32bb191 reverted `CLI/cmux.swift` to main's version while keeping the stricter fixtures, and PR #12053 landed that inconsistent state; the fixtures (CLIVMDevTests, CLIVMLayoutEnvTests, the SSH sharing tests, and the Codex missed-prompt Stop test) have failed since. Restore the three CLI changes. `SocketClient.configureAuthentication` and `SocketPasswordResolver` already exist on main, and the CmuxFoundation overload landed with the branch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(cli): align CLI integration fixtures with the shipped hook, SSH, and session-list contracts The main copy of `CLINotifyProcessIntegrationRegressionTests` predates several shipped contracts that the lenient app-host gate never enforced and that 38b32bb191 reverted on the issue-2824 branch: Claude hook acks print `{}` (#7963), Codex resume bindings require rollout evidence so fixtures carry a `session_meta` transcript (#10100), SessionStart publishes a binding so `/clear` counts start after the clear, fresh-terminal SSH startup commands are script paths that the support decoder must read, cmux control-path options follow a resolved `ssh -G` (#8308), and `ssh session list` reports the localized "remote state unavailable" summary with `--json` detail (#9971). The missed-prompt Codex Stop test now asserts the restored `agent.idle.observed` for the prior turn. Flagged for review: the three `ssh pty-attach` cases now expect only `workspace.remote.pty_bridge` once the endpoint is established, matching #12726's `preserveLifecycleForRecovery`; if pre-READY bridge failures were meant to keep reconciling, the flag should be set only after READY instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(workspace): restore the app-host repairs for fork, focus recovery, and shortcut routing Merge 8d33410585 on the issue-2824 branch took main's copy of `cmuxTests/WorkspaceUnitTests.swift` wholesale and discarded the branch's September repairs (c8bfb580ba, c402b9c515, 2127d97afd); PR #12053 then landed without them while the strict gate started counting the failures. Restore them against current production, plus the shortcut-suite fixes: - Fork in a remote workspace: `sshBootstrapArguments` has used `/usr/bin/ssh` since #9114, and agent socket propagation requires a socket that exists on disk (3bf87e3b4d), so bind a real unix socket and expect the absolute path. - Fork Conversation context actions dispatch asynchronously (#7259, #8173); await the fork panel before asserting. - Git branch and pull request updates publish through `sidebarObservationPublisher` since #6226, not `objectWillChange`. - Config sanitization: `addWorkspaceIfActive` rebuilds templates from the font-size lineage since #8543; override the lineage hook instead. - Focus recovery: AppKit focus is authorized only for a registered, selected workspace whose window carries the main-window identity; use the `TerminalPortalTestWorkspace` fixture and the same registration/pump path as `WorkspaceTerminalFocusRecoverySwiftTests`. - Shortcut routing: clear both Cloud defaults after 9c2ba78be4 swapped them; neutralize Ghostty's imported goto_split fallback (⌘] by ANSI keyCode) in the unshifted-symbol test and assert the digit shortcut does not match; wait for the async runtime start before judging keyDown forwarding (skip loudly without a live surface); wait for the portal to mount before the second-Escape check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(restore): keep a restore identity when the persisted surface id collides Since #13098 (e0e77eb74f), `newTerminalSurfaceOutcome` treats a nil `restoredSurfaceId` as an interactive create and routes it to the selected pane's Cloud source. Session restore passed nil whenever the persisted panel id was still live (duplicate-workspace or restore-into-live), so a legacy managed-Cloud SSH workspace restored into a live manager was turned into a remote tab create: the scaffold panel stayed startup-suppressed with no initial command and `TabManagerSessionSnapshotTests` failed to unwrap it. Mint a fresh UUID on collision instead of nil; it is free by construction, so the restore keeps its identity and the old-to-new remap works as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(terminal): align snapshot, projection, terminal, and browser fixtures with shipped contracts Stale expectations and fixed-spin timing in app-host suites that the lenient gate never counted: - Cloud-projected panes restore as manual-mirror reservations with a staged remote identity (#12675), not a local placeholder projection. - Restore reuses the persisted runtime id when free (aff0e32e93), so assert liveness rather than a new id; the catalog ignores writes for a Cloud machine without a registered provider (#11877), so register the fixture provider before publishing. - Wheel sync requires an authoritative scrollbar response (bbc3edfae4); the fixture now answers like `AuthoritativeScrollbarSurfaceView`. - Search overlay mount, first-responder focus, runtime creation, and the visibility-restore redraw run through deferred main-actor tasks; wait for them with the class's `waitUntil` helpers instead of fixed run-loop spins. - Owning the socket path lock is definitive (959f38a4c3): a refused inode left by a dead listener is replaced, so the restarted listener accepts. - The split-divider hit band extends `dividerHitExpansion` past the divider (667cc431d9); derive the pass-through boundary from the constant. - Browser page background blends against Ghostty's effective terminal color scheme, which is host dependent; read the same preference the product uses. - Cloud Machines defaults on in dev builds (#12318); pin the toggle off for the default-mode palette contract. - Prepared navigation requests keep the caller's cache policy (#13003). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(browser): align lifecycle, identity, host-view, loopback bridge, and portal rebind tests with shipped contracts Browser app-host suites that the lenient gate never counted: - `BrowserPanelWebViewLifecycleTests`: out-of-range discard delays are rejected to the default (the cmux.json loader relies on the nil), and the panel's own `isLoading` stays true for the indicator floor, so wait for both flags and assert no discard blockers before discarding. - `browserNavigationUsesEmbeddedWebKitIdentity`: WebKit reports the native identity as nil or "" (#9482); accept either. - `WindowBrowserHostViewTests`: production routes Dock-divider hits by yielding to AppKit so the live sidebar tracker receives them (#10902, e2e381825f); the tests asserting the portal owns and forwards the hit were merged red against a design that never shipped. Realign the stale-frame test to the pass-through contract and remove the four own-and-forward tests with their fixtures. **Flagged for review:** if own-and-forward is still wanted, that is a hit-testing product change for its own PR. - `testRemoteWorkspaceRuntimeBridgeAliasesMultipleLoopbackPortsFromSamePage`: the navigation delegate restarts main-frame loads to apply the user-agent policy (11c6efee86); a direct `loadHTMLString` with an HTTP base skipped that step, so the data load was cancelled and replayed as a deferred request. Apply the identity first and assert the document is current. - `portalRebindPreservesDocumentAndRoutesRefreshToTheSameWebView`: the portal host is a theme-frame sibling of `contentView` for non-glass windows (#12929); assert same-window instead of descendant-of-contentView. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(terminal): wait for asynchronous runtime creation in the remaining direct-interaction tests The same "Expected runtime surface before ..." precondition that 9f258dd7c7 made wait for the deferred runtime start still sampled after a fixed run-loop spin in the detach-race, close-lifecycle, repeat-key, and repeat-IME tests, and CI on the branch head showed them failing that way. Use the class's `waitUntil` for those four sites too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(cli): model surface.respawn for respawn-pane and give the Codex stack fixture rollout evidence `cmux respawn-pane` has sent `surface.respawn` instead of `surface.send_text` since #5465 (8cafcc3bac); the window-flag fixture's mock still rejected that method, so the CLI exited 1. Answer `surface.respawn`, asserting the window and surface ids, `tmux_start_command`, and that the shell-invoked command carries the user command but never the `--window` flag, which is the test's intent. The Codex interrupted-stack fixture's transcript had no `session_meta` line, so `CodexSessionResumeVerifier` (#10100) found no rollout evidence and the prompt published `surface.resume.clear`. Prepend the session line as the other Codex fixtures do. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(socket): keep mobile.panel.artifact.fetch off the local socket and advertise the served artifact reads 02c1ba4dd9 removed `mobile.panel.artifact.fetch` from the socket worker methods because it needs the authenticated mobile execution context, but that half of the change was lost in a merge: the policy still routed fetch to the worker lane, which has no handler for it, so the local socket answered `internal_error` instead of the `method_not_found` boundary that `TerminalControllerSocketSecurityTests` pins. Restore the removal. `system.capabilities` never advertised `mobile.panel.artifact.stat` and `.thumbnail` although both are served on the worker lane (04ff18eea6 added them only to the test's expectation); advertise those two and keep fetch out. The remote relay allowlist is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(remote): align tmux seed transport, SSH, socket command, and port scanner fixtures with shipped behavior - `RemoteTmuxPaneSeedTransportTests`: manual-I/O mirror panes spawn eagerly (#9272) and stay runtime-backed after portal churn (#9769), so the pane renders its assigned grid before a seed arrives and nothing was retained. Publish a larger tmux pane grid than any test surface applies so the retention under test sees the lag it exists for. - `SSHRemoteCWDRegressionTests`: the persistent-PTY exec helper runs only with `protectsFromHangup: true` (cd9f34ff1b); pass it, and widen the first-spawn guard. - `SSHDeepSleepReattachTests`: a Cloud-owned workspace rejects launch overrides on splits (#13098); create the custom-identity pane before configuring the remote connection. - `SSHConfiguredRemoteCommandHostTests`: ssh-pty-attach validates the bridge `daemon_version` before dialing (#12726); the mock now reports one. - `CloudManualMirrorTransportTests`: the pane failure card uses the short title since 9bf6cb8c94. - `SurfaceSocketCommandTests`: `vm.workspace_new` admits its optimistic workspace through the active main window (#13152, #13155), so bind a bare window to the fixture context; a receipt without a starter terminal costs one snapshot (6d43ea699a). - `PortScannerPublicationTests`: the forced-result acknowledgement hops off the main actor, so an unchanged port set may be deduped under a later refresh; drain publications until the retirement lands. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test: align mobile artifact fetch lane assertion * repair: close remaining full-suite contracts * test(restore,sidebar): align two stale contracts with shipped behavior testRemoteWorkspaceAutoResumeKeepsRemoteStartupCommand asserted the restored remote panel's local spawn cwd equals the remote-host path. It never can: OneShotTerminalLauncherStore.enterableWorkingDirectory rejects a path that is not locally enterable, which is what keeps the owning shell valid (#7031). The remote cwd does survive restore — as the panel's trusted remote directory report, and as the `cd` prefix the resume input carries (both already asserted). Assert it where it actually lives and pin the spawn cwd to nil. testSidebarPullRequestsTrackFocusedPanelOnly expected a background panel's PR to be hidden from sidebarPullRequestsInDisplayOrder(). That list is documented as the workspace's deduplicated rows in pane/tab order, both consumers (taskStatusSignals, the control-sidebar snapshot) want every panel, and the sibling branch test asserts the same all-panel model. Focus scoping lives in the `pullRequest` binding, which the test already covers. Renamed to match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: give Cloud catalog tests live destination workspaces #13196 made SurfaceCatalog.validateOwnership refuse a destination that is not a live workspace. That rule stays. Tests that projected, restored or opened browsers into a made-up workspace ID now failed with destinationNotFound, timed out waiting on a provider that was never called, or passed a later check for the wrong reason. LiveWorkspaceFixture registers real Workspace objects and hands the catalog a CloudWorkspaceRenameService that resolves them, the way the app's composition root does. Its workspaces() list stays empty so the native projection coordinator does not start mirroring into them. For tests on SurfaceCatalog.shared, withAppRegistration registers the TabManager as a windowless main-window context for the test body. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: normalize pbxproj after live workspace fixture Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: admit agent renames of agent-owned accepted Cloud names e6926fbbda (#13403) made submitCloudPanelRename run admitsTerminalRename for automatic names. Its last clause only admitted replacing an accepted name when the local panel still carried `.auto` provenance, but since 1e1d319dd1 an accepted daemon name reconciles locally as `.remote` and the owner lives in the tab's nameAuthority. Every agent title after the first accepted one was refused, so "Failed agent rename keeps the accepted title", "Mirroring an agent-named placement does not block its next agent title" and "An older automatic result and old snapshots cannot replace an accepted name" failed on their second agentName call. Admit an automatic rename when no write is pending and the accepted tab name is owned by the daemon's `auto` authority. User-owned names, pending writes and local user titles on any projection still refuse it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: expect adopted machine rows to keep the pending create identity #12919 (021f792b95) made New Machine creation optimistic: once a running create's machine appears in the fleet list or catalog, its row keeps the `pending-machine:<operation>` node ID so selection and expansion survive adoption (see committedProjectionKeepsThePendingNodeIdentityUntilFleetAdoptsIt). pendingRowStepsAsideOnceItsMachineHasARow still expected `machine:<id>`. Assert the new identity and that the row is the adopted machine, not a stand-in: the stand-in is gone, one row remains, and it shows the created machine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: restore per-display noVNC targets and refresh stale Cloud tests Product (#13196 regression): the 6db759357f main merge into 13192-cloud-display-ownership took main's CmuxTuiSurfaceProviders.swift and CloudPortRoutePlan.swift, dropping eb3edd7f98's per-display ports. 23c807b50a restored the display coordinator but not these hunks, so withPrivateBrowserURL rewrote every display to 6901 and a daemon pointer without a discovered target fell back to display 1. Restore both hunks and drop the duplicate port-less privateDesktopURL overload. Tests: - CloudDisplayCatalogTests: 178d35e5da (#13196) made every guest command run `list` as a readiness probe, so fakes dispatching on " list" answered creation with the list catalog. Dispatch on the create action line, and pin the command shape. - CloudPortOpenRegressionTests: 178d35e5da (#13196) filters RFB/noVNC ports only when the display catalog owns them (displayPortsOwned). Assert both the desktop and non-desktop results. - CloudTreeOneMachineManyWorkspacesTests: #12740 added a final Resources section under each machine. Expected trees include it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: publish every guest display from daemon graph updates The 6db759357f main merge into 13192-cloud-display-ownership (#13196) put back main's [desktopDisplayResource()] pools in CmuxTuiSurfaceProvider's refresh, publish and delta paths. 23c807b50a restored the display coordinator lifecycle but not these pools, so a display created beyond display:1 vanished on the next daemon publish, and a delta that touched it removed it. Restore eb3edd7f98's displayResources pools, the display-kind delta check, and the injectable displayCoordinator. Drop the now-unused desktopDisplayResource(). Adds a test that creates display:2 through the provider and asserts that both displays and their ports survive a full publish and a display delta. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: fence the fake Cloud projection reply with its mutation cursor aDetachedTerminalDropsItsStaleTabBeforeMoving installs a daemon graph since c402b9c515 (#13403). When the placement lane drains it reconciles against that graph, which predates the projected tab, and clears tab_projected. The real reply always carries a mutation cursor (CmuxTuiSnapshotParser.placedTab requires one) that fences exactly this; the fake returned none. Give the fake a projectCursor and set it ahead of the installed graph. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: register the restored window before relinking Cloud projections #13196 made SurfaceCatalog.validateOwnership refuse a destination the app cannot resolve, so SurfaceCatalog.shared no longer relinked a restored projection into a TabManager that no main window owns. Register it for the test body with LiveWorkspaceFixture.withAppRegistration, as #13651 does for CloudClosedPanelRestoreTests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: wait for the relay ports kick, not the first relay line Since #8442, a relay prompt also reports shell state. Both RPCs run in separate background children, and the zsh prompt-refresh test waited only until the log was non-empty, so it could read report_shell_state alone. Wait (deadline-bounded) for the ports_kick line itself, in the zsh test and its bash sibling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cli): relay output of an SSH session that ends right after auth The expect wrapper watches the first two seconds after sending the password for a rejection with log_user 0. A session that authenticates and exits inside that window hit the eof branch and its output was dropped. Flush the buffered output before exiting. #13207 replaced the test's 9 s sleep with a FIFO release, which is what exposed this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: restore the no-connection path for rejected vm dev input 4120e35654 taught runVMDev that invalid input never connects: keep the listener open, then check its backlog after the CLI exits. #13207 dropped that again, so each rejected run waited 60 s for a mock-server expectation that only the listener closing (after the wait) fulfills. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: drain the terminal while the SCP host-key failure runs The pty case read the master only after the CLI exited. A pty's output queue holds about 1 KiB and the host-key failure report is longer, so the CLI blocked writing stderr until the 30 s timeout. Read the master on a thread while the CLI runs. The test starts its own sshd; no runner host dependency is involved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: fail fast when a gated Cloud call ends before its fake is entered Every app-host restart in the 09-21/09-22 shard logs I sampled was a Swift Testing time-limit hit in one of two suites, and each hit relaunches the test host: - SurfaceCatalogTests: when catalog.project threw before reaching the provider (destinationNotFound, fixed by 3dc91b2321), each gated test parked in MaterializeGate.waitUntilEntered() until the 300 s limit. Five tests restarted the host one after another, about 25 minutes per shard. - CloudDisplayCatalogTests: when the fake no longer recognized the create command (fixed by 70eaf44819), create() failed before the exec started and `await started.result` parked until the 60 s limit. The waits now also end when the caller's task finishes, so the next such setup failure is an ordinary failed #require. The cancellation test also waits on its own cancellation signal instead of a 60 s Task.sleep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: reveal a retired terminal through the portal rebind, as the app does Since #12607 hiding a terminal removes its hosted view from the window, and only a bind reinstalls it. The test flipped portal visibility on the detached view, so no size commit could ever land and the final shrink check failed every run. Rebind like TerminalPortalReconciliation and drop the wait loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: pin key-window status in terminal focus suites The app-host test process runs headless and is usually not the active app, so makeKeyAndOrderFront never makes a programmatic window key. Terminal focus paths gate on isKeyWindow (automatic first-responder apply, focus redraws, deferred focus reapply, ensureFocus window activation), so these suites passed only when an earlier test in the shard had activated the app. Share the existing KeyStatusTestWindow and use it in WorkspaceTerminalFocusRecoveryTests, WorkspaceTerminalFocusRecoverySwiftTests and TerminalNotificationDirectInteractionTests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: report which layer holds off-plan tmux mirror geometry offPlanGeometryWithUnchangedSizingInputsReconverges fails every run with all three output-parity re-arms spent and the hosted view still at the perturbed 0.8 divider. A standalone bonsplit replay of the same sequence converges, and a sibling test without a bound (portal-visible) workspace heals the same displacement. Add the live split view's arranged widths and the split model's imposed extent to the failure message so the next run shows whether bonsplit refused the apply, the imposition was cleared, or the portal did not follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: restore Cloud projections before the workspace is published TabManager.restoreSessionSnapshot restores each workspace's surface projections before it assigns tabs, so SurfaceCatalog.restore's ownership check could not find the destination and silently dropped every restored remote projection. Check ownership against the workspace being restored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: repair stale and broken app-host expectations - CLI ssh attach: count identity UUIDs exactly; #11497 added an auth token uuidgen - device mirror directories: give the fake device trusted presence (e3d424c722 gate) - font zoom mirrors: deltas from the 8pt inherited base (e6926fbbda arithmetic) - Computer Use refresh: fake daemon reply was invalid JSON in a raw string (#13599) - quit alert: compare button alignment rects, not padded frames - remote split cwd rescue: cwd travels as CMUX_REMOTE_INITIAL_CWD since #12054 - default freestyle split: Cloud-owned splits route to Cloud since fa5dc4cc10 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: activate the app host before waiting for terminal focus `focusTerminalForTesting` waits for `window.isKeyWindow` before it hands first responder to the terminal, but `makeKeyAndOrderFront` only makes a programmatic window key while the test host is the active app. The app-host process starts inactive under `xcodebuild test`, so the wait succeeded only when an earlier test in the shard happened to activate the app. Its callers use a real `createMainWindow()` window, which cannot be swapped for `KeyStatusTestWindow` the way the pinned focus suites were. Activate explicitly, and give the wait a CI-appropriate timeout: the activation and the key-window transition land on later main run-loop turns, and the pump's one-second default expires before the window goes key on a contended runner. Observed on run 35743588302: `plainTerminalTextDoesNotResolveAppShortcutContext` (shard 1/6), `keyboardCopyModeKeyClearsTerminalUnread` and `workspaceFontSizeShortcutPreservesBackgroundTerminalUnread` (shard 2/6) all fail at this helper's return value, and shard 6/6 shows the same process state directly as `NSApp.keyWindow -> nil`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: give standalone terminal fixtures a live portal authority `setVisibleInUI` and `setActive` fold their request through `Workspace.portalRenderingEnabled(for:)`, which denies any workspace id the app delegate cannot resolve to a selected tab. Three direct interaction tests build their surface with `tabId: UUID()`, so once any earlier test installs an `AppDelegate.shared` the authority denies the portal, the hosted view is never actually made visible or active, and the fixture stops exercising the behavior it asserts: the surface never takes Ghostty focus and never schedules a visibility-restore redraw. Register a real selected workspace and build the surface with its id, so the fixture gets the same authority the app grants the selected tab. This is the fixture, not the product: the authority check is deliberate, and denying an unresolvable workspace is what keeps queued portal callbacks from reviving an inactive workspace. Fixes on run 35743588302 shard 4/6: `testKeyDownRecoveryDoesNotReplayFocusAfterResponderMovesAway`, `testVisibilityRestoreRefreshesSurfaceWhileTerminalIsInactive`, and `testDirectFirstResponderFocusRefreshesCursorStateAfterForeignResponder`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: normalize project.pbxproj after merging main Run 35763999808 failed `Fast static checks` before any macOS job could start, so the app-host test fixes on this branch were never exercised: error: cmux.xcodeproj/project.pbxproj is not normalized. Run scripts/normalize-pbxproj.py to fix. The branch head alone checks clean. CI builds the merge of this branch into main, and that merge is what leaves the file unnormalized, so the failure does not reproduce without merging main first. The change is one build-phase entry moving back into alphabetical order. `LiveWorkspaceFixture.swift in Sources` keeps the same UUID and the same occurrence count, and `scripts/lint-pbxproj-test-wiring.sh` still reports ok across 1049 test files, so no target lost a source file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * perf(tests): stop app-host unit tests waiting out real timeouts The app-host suite spends most of its wall clock in a handful of tests that wait out production timeouts, churn oversized fixtures, or run benchmarks. Each one now drives the same behaviour through an injected timeout, clock, or completion signal, with a separate cheap assertion pinning the default value to the production/server default. Production changes are configuration seams and one real fix: - KeyboardShortcutSettings.resetAll only removes keys that are stored, so a reset no longer fans out one UserDefaults.didChangeNotification per action. - SocketStartupWaiter / AgentRestorePreflightTimeout / BrowserDownloadWaitTimeout own the CLI wait windows (default plus a narrowing environment override), so client and handler cannot drift apart. - PortScanner and BrowserScreenshotWebViewSnapshotter take their schedules as parameters instead of hardcoding them. Benchmarks move out of the app-host unit suite behind the existing gating pattern, keeping their correctness assertions in the unit suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: keep the scroll settle default readable from a nonisolated context The default lives on a @MainActor enum, so every default-argument use of it is evaluated in a nonisolated context and trips the Swift 6 isolation warning. The value is an immutable Sendable constant, so mark it nonisolated rather than isolating four call sites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): stop kicking the port scanner from the poll loop Review found two defects in the slow-test pass. 1. `PortScannerPortRetirementTests` polled every 25ms and called `scanner.kick()` on every poll, against the 10ms compressed coalesce delay this PR introduced. `kick()` calls `startCoalesce()` whenever no burst is running, and `startCoalesce()` cancels and re-arms the timer. On a loaded runner the timer's jitter is the same order as the coalesce window, so the kick burst can cancel the timer indefinitely: no scan ever runs and the test burns its 20s deadline. The previous 500ms poll against a 200ms delay left a 2.5x margin that the compressed schedule removed. Fixed by removing the kick from the poll loop rather than by widening the interval, so the speed win stays and the flake vector is gone instead of made less likely. A kick already guarantees `minimumScansPerKick` scans - exactly the complete misses the reconciler needs to retire a port - so one kick after `stopListening()` is sufficient. That is the shape `lateBurstKickRetiresStoppedListener` already used. `onKick` is gone from `waitForPublication` entirely, so the poll interval is no longer coupled to the coalesce delay and the footgun cannot come back. 2. `scripts/test-command-palette-nucleo-ffi.sh` set `CMUX_COMMAND_PALETTE_SEARCH_BENCHMARKS=1` and then passed `-only-testing:cmuxTests/CommandPaletteNucleoFFITests`, a different class from the four `CommandPaletteSearchEngineTests` benchmarks this PR gated behind that variable, so those four ran nowhere. The script now names both classes and asserts one BENCH line per gated benchmark, since a skipped benchmark otherwise passes silently. Nothing referenced the script either, so even the FFI tests it names were not run by CI. `.github/workflows/command-palette-search-benchmarks.yml` is its scheduled caller, modelled on the `tmux-corpus.yml` nightly (same checkout/GhosttyKit/zig/rust/SPM-cache setup, same `cmux-unit` scheme). The script takes an optional `CMUX_NUCLEO_FFI_SOURCE_PACKAGES` so the workflow can reuse the cached package clone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: keep the benchmark caller dispatch-only while automatic CI is paused tmux-corpus.yml and perf-activation.yml -- the two closest macOS benchmark nightlies -- both carry "Temporarily manual-only beginning 2026-07-13 to pause automatic CI" and have their crons removed. Adding a live cron here would quietly reverse that reduction, which is the opposite of what this branch is for. The gate still has a caller, and the script's BENCH assertions still fail the job if a gated benchmark silently skips; running it is now a deliberate act rather than a daily cost. Drop "nightly" from the name, since it is not one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): model the title-word ranking term in the search reference testBenchmarkCorporaMatchReferencePipelineOnSmallFixture asserts the optimized engine equals a reference pipeline rebuilt in the test. On the query "workspace 31" the engine scored workspace.large.31 at 17598 and the reference at 16000, so the suite failed. The engine ranks on three terms; the reference modelled two. The missing one is commandPaletteTitleWordScore: a query that is, or prefixes, a title's search words scores the tokens' upper bounds plus a per-token title bonus. For "workspace 31" that is 6799 + 6799 + 2000*2 = 17598, exactly the observed gap. The reference now models that term too, reimplemented rather than calling the engine's copy, because a reference that shared the implementation would assert nothing about it. FixtureEntry prepares the two title texts the term needs once at construction, so the benchmark timing loops — which build fixtures outside the timed region — do not start charging the legacy side for preparation the engine does not repeat either. Verified by compiling the engine sources against this reference on Linux and comparing all 34 corpus/query pairs the parity tests use: the large benchmark corpus goes from 1 mismatch to 0, and the command, switcher and switcher-benchmark corpora stay at 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): drop the ineffective SSH auth retry budget rewrite persistentAttachExitsAtForegroundAuthenticationFailureLimit() rewrote the generated startup script to shrink the 20-failure foreground-authentication budget to 3, then asserted 3 attempts. CI recorded 20: the rewrite never changed the behaviour it was meant to bound. The budget the test edited lives in the shell wrapper, but the preserved __ssh-* CLI helper is what owns authentication retries and counts them itself, so rewriting the wrapper's literal leaves the helper executing its own 20. The shrink was decoration over a real 20-attempt run. The test now keeps the production budget and asserts 20. The fake sleep already removes the backoff between attempts, which is where the wall-clock cost actually was, so the same failing run reached the assertion in 4.25 s against the 6.1 s this test cost before the PR. That is less than the <1 s the PR's table claims for this row; the table is corrected in the PR body. replacingWithinScript, scriptDecodeLevels and applyingReplacements existed only for this rewrite and have no other callers, so they go with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): close the persistent CLI client before awaiting the mock testDefaultFreestyleSSHAttachHidesPersistentRetryLimitInCountdown failed with "Asynchronous wait failed: Exceeded timeout of 5 seconds, with unfulfilled expectations: cli mock socket handled", after 5.227 s. startMockServer fulfills once the first connection is done. This test drives a persistent SSH attach, so the client holds its connection open across the retry countdown and the mock has no "done" to observe. The wait could only ever end by timing out; it had been passing on the longer window this PR narrowed. The test now terminates the child once it has observed the progress it actually asserts — the "Retrying in 0.1s (attempt 2)." banner — and waits for the mock afterwards. The countdown assertions that follow are unchanged and still read the captured output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Retrigger CI after retargeting this PR onto main The synchronize event that pushed ed3ab7a61e fired while the base was fix/app-host-green, a branch the #13643 squash merge had already deleted, so GitHub could not build the merge ref and the pull_request CI workflow never started. Only the two pull_request_target workflows ran, which left the required ci-status check absent rather than red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make the two new timeout holders instantiable values CI's iOS conventions guard reported two violations this branch introduced: AgentRestorePreflightTimeout and BrowserDownloadWaitTimeout were both caseless enums with an all-static public surface, which the lint refuses because such a type can never be substituted. The refusal is right, and for this branch in particular: the point of the change is that a test should be able to hold a timeout agreement without spending it. A static holder cannot be narrowed, so the tests could only assert the shipped numbers. The restore preflight budget moves onto AgentRestorePreflightInvocation, which is the type it bounds and already carries the environment it is read from. Names lose the now-redundant prefix: defaultSeconds becomes defaultTimeoutSeconds, environmentKey becomes timeoutEnvironmentKey, and seconds(environment:) becomes timeoutSeconds(environment:). BrowserDownloadWaitTimeout becomes a struct whose three windows are stored properties with the shipped values as init defaults, plus a `standard` value the handler and the CLI client both read. Both call sites take `.standard`; the behaviour is unchanged. The regression test now also constructs a narrow window and asserts the client still outwaits the handler, which makes the invariant a property of the pair rather than an assertion about 10 seconds. Verified with swiftc 6.1.3 on Linux: both files type-check under -swift-version 6, and the extracted behaviour matches the previous constants exactly (10s default, 10s ceiling on the override, whitespace trimmed, non-finite and non-positive overrides ignored; 10000ms default window, 120000ms cap, 15.0s client response timeout). The conventions diff lint now reports no new violations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove the KeyStatusTestWindow the main merge declared twice `macOS compile admission` failed with "ambiguous use of init(contentRect:styleMask:backing:defer:)" at WorkspaceTerminalFocusRecoveryTests.swift:551 and WorkspaceUnitTests.swift:4890. Neither file is touched by this branch. The cause is cmuxTests/AppDelegateMainWindowTestingSupport.swift, where the merge of main kept both sides' addition of the same class: `final class KeyStatusTestWindow` appeared twice, byte-identical including its doc comment, so every construction of it was ambiguous. Git had no conflict to report — the two additions did not overlap — which is the third time on this branch that a clean automerge produced code that could not compile. Checked the rest of the merge for the same shape: no other duplicated top-level type in cmuxTests, and none in CLI/ or Sources/. The one other repeated name, `optimizedResults` in CommandPaletteNucleoFixtures.swift, is a legitimate pair of overloads taking `corpus:` and `entries:`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: expect the reconnect budget #13959 honors, not the old clamp The new budget probe asserted that "21" and "021" clamp to 20. Main's #13959 made a well-formed budget above 20 honored up to SSHReconnectBudget's ceiling, so the merge ran green through git and red on CI: stdout "21". The cases now carry their expected value, from SSHReconnectBudget where it is a policy constant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: gate the benchmark runner behind the paid-overflow switch Main's #13994 guard rejects a vars.MACOS_RUNNER_15 read without CI_PAID_MACOS_OVERFLOW, since it can hold a metered WarpBuild label. This branch's new benchmark workflow predates the guard, so the merge of main passed git and failed workflow-guard-tests. Same form as perf-activation.yml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: close CodeRabbit gaps in the fast app-host test rewrites - PortScanner late-burst: the runner stops listening and issues the late kick from inside the fifth lsof call, instead of the test task racing a one-second timer gap it could miss in either direction. - Sidebar quiescence: the quiet window is measured in time and spans twice the sidebar's 50ms coalesce stage, so a pending coalesced or debounced row invalidation cannot land after the drain declared quiet. - SSH exit prompt: waitUntilBlocked samples the launched process and all of its descendants, so it holds whether the shell execs the startup script in place or forks it; terminate() retires that tree so a forked, parked helper cannot hold the output pipes or leak into the test host. - Sidebar git refresh: wait until the panel is a poll candidate again before each refresh; a completed read can precede the probe clearing. - Diff fixtures pin SHA-1 objects and loose-file refs, which the handwritten refs and in-process blob ids assume. - Rename the registry test to the post-list enrollment order and assert it. - TerminalController uses BrowserDownloadWaitTimeout's shared clamp. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep the Cloud carrier prewarm on activation A merge from the fix/app-host-green lineage deleted the prewarm that syncPollingToActivationPolicy() starts (999693e015, perf: prewarm cloud carrier before first machine), so enrollment waited for the first listPage() round trip. The polling test was then rewritten to assert that slower order. Neither change is part of this PR, and #13403 carries the same removal for its owner to argue. Both files now match main, whose test asserts the prewarm starts before fleet discovery finishes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: keep the git fixture's formats when the host sets Git defaults GIT_DEFAULT_REF_FORMAT and GIT_DEFAULT_HASH override the init options the handwritten-ref fixtures depend on. runGitProcess no longer passes them on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: report browser.download.wait's requested timeout as a number again The shared timeout window left requested_timeout_ms an Optional, which the timeout error payload coerced to Any and which put a new warning over the Swift warning budget. The handler resolves the default and the lower bound before clamping, as it did before, so the field is the same Int as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: settle the Global Search palette before each routing test KeyboardShortcutSettings.resetAll() no longer posts one UserDefaults.didChangeNotification per action, so the next test's init no longer spends enough main-thread time for the previous test's animated NSPopover close to finish. Two routing tests then saw isShown == true from the palette an earlier test had opened (run 35967763918, shard 7). Dismiss the palette and pump the run loop until it closes, bounded at 2 s, in both suites' init, matching the helper the sibling suites already use. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: route the palette benchmark workflow to a hosted runner on forks main's fork runner routing guard (test_ci_fork_runner_routing.py) now rejects a runs-on expression that can reach a Blacksmith label outside manaflow-ai. Lead with the owner fork branch, as perf-activation.yml does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: drop the extra paren in the palette benchmark runs-on expression 572dc2a3aa7 kept the old closing paren and added another, so actionlint could not parse the expression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: set up Bun on the shard that runs agent notification semantics The notification semantics step resolves `bun` with `command -v bun`, but Bun was only set up on the CLI regression shard (4) while the step runs on the focused regression shard (6). Main never reaches the step because its unit batch fails first, so the missing Bun only shows once shard 6's unit tests pass, as they do on this PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: scope the CmuxWebView keyDown hook to each reentry test CmuxWebViewKeyDownReentryTests swizzled CmuxWebView.keyDown(with:) for the rest of the process. When CmuxWebViewWebContentUndoTests ran after it in the same app host, browserCmdZPerformsWebContentUndoWhenPageDeclinesTheChord recursed in the test hook until the stack overflowed (seen on macOS 15 and 26 once this PR's shard layout put the two suites back to back). Install the hook per test window, call the captured original implementation directly instead of re-dispatching through a swapped selector, and restore the original implementation when the window closes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: keep the OpenCode feed harness socket inside sun_path testOpenCodeFeedPluginEmitsCompletionForBothIdleEventShapes put its Unix socket under FileManager.temporaryDirectory plus a UUID-named root. On a Blacksmith runner that is /private/var/folders/.../T/, and the path runs past the 104-byte sun_path limit, so the Bun harness fails in listen() and prints nothing. Use a short /tmp path for the socket instead. This surfaced once the agent notification semantics step ran on this PR; main never reaches the step because its shard 6 unit batch fails first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: wait for the relayed pwd line, not the first relay line zshRelayPromptReportsRemotePWD broke out of its wait as soon as the relay log was non-empty. _cmux_precmd sends report_shell_state and report_pwd as separate background relay calls, so on a busy runner the log held only the shell-state line when the test read it. Wait for the report_pwd line itself, up to 5 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: tie the E2E stale-snapshot case to OWNED_MAX_AGE_MINUTES #14341 raised OWNED_MAX_AGE_MINUTES from 20 to 45, so a 30 minute old snapshot is now fresh enough and the "snapshot too old" case in test_run_e2e takes the owned Mac, failing app-host-execution guards. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 3dfc85f17f2b54936fb044ee718387b2f4258903) * test: pin the full paste worker to lossy rich text, per #14121 #14121 sends mixed rich and plain text through the plain-text helper, so richPasteDoesNotUsePlainTextHelper (HTML plus a clean plain string) now gets a successful fast-path paste instead of the full worker's error, and it has failed on every main run since. The helper still declines rich text whose plain export lost characters (U+FFFD or repeated '?'), because the full worker can recover them. Assert that case instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: austinpower1258 <austinwang115@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.
Closes #5248
Summary
__tmux-compatto handlerespawn-pane/respawnpthroughsurface.respawninstead of typed input.cmux respawn-panetext injection with the samesurface.respawnpath.Captured before output
Failure A as reported did not reproduce on the current checkout: after shim
split-window,list-panesalready included both source and subagent panes:Failure B exact
invalid_params: Surface is not a terminaldid not reproduce through publiccmux respawn-pane; it returnedOK, but only typed into the sleeping placeholder shell instead of replacing it:The actual OMO shim step 2 did reproduce as broken because
__tmux-compathad norespawn-paneimplementation:Regression test red output before the fix:
Captured after output
Tagged live shim after-output captured from the fixed build:
The requested cloud reload script is absent in this checkout:
Fallback build command used:
The fallback build compiled successfully (
** BUILD SUCCEEDED **) and produced the tagged app, but--launchstill timed out waiting for/tmp/cmux-debug-issue-5248-omo-subagent-attach.sock. A direct executable launch of that built app also did not create the tagged socket, so the final current-commit verification was the fake-socket CLI regression plus CI.Checks run
git diff --checkpython3 -m py_compile tests/test_cli_omo_tmux_respawn_pane.pypython3 -m json.tool Resources/Localizable.xcstrings >/dev/nullCMUX_CLI_BIN='<tagged cli>' python3 tests/test_cli_omo_tmux_respawn_pane.py->PASS: OMO tmux shim lists and respawns subagent panesCMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag issue-5248-omo-subagent-attach --launch-> build succeeded; tagged socket launch timed out10c8a3f8de50ce07def0880acdeefcc308844f6f: 19 passed, 0 failed, 0 pending, 3 skippedLocalization audit
Changed user-facing respawn/tmux error messages are localized in
Resources/Localizable.xcstringsfor English and Japanese. Parsed the xcstrings file withpython3 -m json.toolafter the update.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches terminal lifecycle, RPC routing, and tmux compatibility; incorrect teardown or targeting could break panes or focus, but changes are scoped with tests and explicit error handling.
Overview
Terminal respawn now replaces the shell behind an existing pane instead of typing a command into it. A new
surface.respawnRPC andWorkspace.respawnTerminalSurfacetear down the old Ghostty process and start a new one while keeping the same surface, Bonsplit tab, and pane ids, plus titles, cwd, pinned state, and startup environment.CLI/tmux:
__tmux-compatgainsrespawn-pane/respawnp(requires-k, optional-c, command or storedtmux_start_commandfallback). Publiccmux respawn-paneresolves workspace/surface handles and callssurface.respawninstead ofsurface.send_text.Supporting fixes:
TerminalSurface/TerminalPanelcan take an explicit id for in-place replacement; registry unregister avoids clearing focus placement when another surface shares the id; native close callbacks ignore stale surfaces after respawn. New localized RPC/tmux errors and Swift/Python regression tests for the OMO subagent flow.Reviewed by Cursor Bugbot for commit 10c8a3f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes OMO subagent pane respawn by replacing the terminal process in place so panes keep their id, tab, and focus. Aligns tmux
respawn-panewith stored-command fallback,-ccwd, enforced-k, and preserves startup environment and tmux start metadata; closes #5248.surface.respawnRPC andWorkspace.respawnTerminalSurfaceto swap the process in place while preserving ids, titles, cwd, focus, pinned state, wait-after, and startup env.respawn-pane/respawnpin__tmux-compat: requires-k, supports-c, targets viaCMUX_SURFACE_ID, and falls back to storedtmux_start_command/pane_start_command/initial_commandor a login shell.cmux respawn-paneto resolve workspace/surface handles to ids and callsurface.respawnwithtmux_start_command(no moresurface.send_text).TerminalSurface/TerminalPanel; ignore stale native close callbacks and keep focus placement if a same-id surface remains so respawned panes stay attached.-kenforcement, stored-command fallback, and ensure all paths usesurface.respawn.Written for commit 10c8a3f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Localization
Tests