Repository navigation
CmuxIPCService: extract AppDelegate multi-window CLI routing (MultiWindowRouter behind MultiWindowRouting) - #5920
Merged
Merged
Conversation
Extract the window RPC domain (window.list/current/focus/create/close/displays/ display) out of TerminalController into a new @mainactor @observable ControlCommandCoordinator in CmuxControlSocket, behind the read-only ControlCommandContext seam (app target conforms; package never imports the app target). The coordinator owns the kind:N ControlHandleRegistry (RPC selection state per the decomposition plan); TerminalController delegates its ensureRef/ resolveRef/removeRef to it so refs stay consistent across moved and not-yet- moved domains. Faithful lift: the window bodies build ControlCallResult/JSONValue payloads whose Foundation object is identical to the legacy [String: Any] dictionaries, so the encoded wire bytes match. Dispatch runs on the main actor inside the existing withSocketCommandPolicy scope, so the per-read v2MainSync hops the legacy bodies used become plain in-isolation calls and disappear. window.current preserves both distinct legacy errors (unavailable vs not_found) via ControlCurrentWindowResolution. TerminalController.swift 22074 -> 21921 (budget ratcheted). 17 new package tests (128 total) drive every window method through a fake context, asserting byte-identical payloads, ref minting, routing-selector parsing, and the two window.current failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restructure the seam into a per-domain protocol umbrella (ControlCommandContext: ControlWindowContext, ...) so each domain can be built in its own files, and port the shared TerminalControllerV2ParamParsingSupport pure helpers + ref minting (workspaceRefs/tabRef/workspacePaneAndSurfaceRefs) into the coordinator as JSONValue twins. Foundation for moving the remaining RPC domains. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the app-focus (app.focus_override.set, app.simulate_active), main-actor feed (feed.jump, feed.list), and notification (create/create_for_surface/create_for_target/ list/dismiss/mark_read/open/jump_to_unread/clear) domains into the coordinator behind their per-domain seams (ControlAppFocusContext/ControlFeedContext/ ControlNotificationContext), composed into the ControlCommandContext umbrella. The core handle(_:) now chains per-domain handleX dispatchers. Worker-lane methods stay app-side: feed.push/permission.reply/question.reply/ exit_plan.reply, and notification.create_for_caller (its own resolver). Faithfulness: byte-identical payloads/errors (live socket sweep on ctl3c1 confirms every result + error shape). Notification localized strings are resolved in the app conformance (app bundle) and passed through ControlNotificationStrings, because String(localized:) inside the package would bind to the package bundle and silently drop the Japanese translations — a wire change for non-English locales. Test fakes get benign defaults for non-window seams via ControlCommandContextTestStubs so each fake implements only the domain it exercises (128 package tests still green). TerminalController.swift 21952 -> 21522 (budget ratcheted). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move workspace.group.* (17 methods), pane.* (9 methods), and mobile.host.status/ mobile.workspace.list/mobile.terminal.* (+terminal.* aliases) into the coordinator behind ControlWorkspaceGroupContext/ControlPaneContext/ControlMobileHostContext, composed into the umbrella; core handle(_:) chains the new handlers. Workspace Groups + Pane are full lifts (bodies deleted, payloads rebuilt as JSONValue, localized group strings routed app-side via ControlWorkspaceGroupStrings). Mobile Host is a faithful pass-through: its 8 bodies are SHARED with the mobile data-plane (mobileHostHandleRPC) so they stay in TerminalController (relaxed private->internal); the coordinator decouples via the seam and the conformance bridges V2CallResult. Pane folds the resize support helpers (kept app-side: Bonsplit-coupled); v2SurfaceMove relaxed private->internal for pane.join forwarding. Live socket sweep on ctl3c1 confirms faithful payloads + errors (group create/list, pane list/create split, mobile host status). TerminalController.swift 21522 -> 20296. 128 package tests green. Two new Pane files >500 lines get budget entries. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regression found by the no-regression code review of the moved domains: the ported int() did Int(value) on a JSON double, which TRAPS (crashes) on overflow/ NaN — reachable via pane.resize amount or workspace.group.move to_index with e.g. 1e30 — whereas legacy v2Int went through (params[key] as? NSNumber).intValue, which clamps. Also int()/double() didn't coerce a JSON boolean to a number the way the legacy as? NSNumber path did. Both now route doubles/bools through NSNumber.intValue/.doubleValue, matching v2Int/v2Double exactly (truncate-toward-zero, clamp out-of-range, bool->1/0). 5 regression tests cover truncation, overflow/NaN no-trap, and bool coercion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Workspace (21 methods incl. remote.*) and Surface (25 methods + debug.terminals) move into ControlCommandCoordinator behind ControlWorkspaceContext/ ControlSurfaceContext. ~2640 lines deleted from TerminalController.swift (20296 -> ~17650). Worker-lane workspace.remote.pty_* stay app-side. Two shared bodies the drafting agents wrongly flagged for deletion were RESTORED (internal/private): v2WorkspaceCreate(params:tabManager:) is still driven by the mobile data-plane v2MobileWorkspaceCreate; workspaceCloseProtectedMessage() by the v1 close path. surface.move + debug.terminals forward to the still-shared v2SurfaceMove/v2DebugTerminals (relaxed internal), like pane.join. Relaxed to internal for the conformances: tabManager, socketFastPathState, orderedPanels, readTerminalTextRawSnapshot. Live socket sweep on ctl3c1 confirms faithful payloads + errors across both domains (workspace list/current/create/rename/select/next/close, surface list/ current/health/send_text+read_text round-trip/resume.get, error shapes). 133 package tests green. KNOWN FOLLOW-UPS: workspace.create logic is duplicated (conformance reimplements + restored shared body) — dedupe by forwarding; the 2 Workspace files >500 lines (budget entries added) should be split; adversarial code-review verification of these 2 domains still pending (8 prior domains verified clean). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Coordinator/ had grown to 105 files. Move each domain's coordinator extension, seam protocol, and value/resolution/snapshot types into a per-domain subfolder (Window/AppFocus/Feed/Notification/Pane/Surface/Workspace/WorkspaceGroup/ MobileHost). The 4 shared core files stay at the Coordinator/ root: ControlCommandContext (umbrella), ControlCommandCoordinator (core dispatch + handle registry), ControlCommandCoordinator+Params (shared param/ref helpers), ControlRoutingSelectors. SwiftPM globs sources recursively, so this is purely organizational — no Package.swift/import changes. Budget paths updated for the moved Pane/Workspace coordinator files; TC.swift budget corrected to 17680 (the two restored shared bodies grew it after the last bump). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Workspace lift had reimplemented workspace.create logic in the conformance while the original v2WorkspaceCreate(params:tabManager:) was restored for the mobile data-plane caller -- two copies that could diverge. Replace the typed reimplementation with a passthrough that forwards to the single shared v2WorkspaceCreate (relaxed private->internal) and bridges its Foundation result, exactly like surface.move/debug.terminals/mobile. Deletes the now-unused ControlWorkspaceCreateInputs/ControlWorkspaceCreateResolution. One source of truth, byte-identical wire output. Comprehensive socket sweep on ctl3c1 (all 10 domains, 38 ok + 13 expected validation errors, zero crashes) confirms no regression: workspace.create happy path + its cwd/layout validation errors preserved; pane.resize amount=1e30 now clamps (invalid_state) instead of trapping (the int/double NSNumber fix). 133 package tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…view Surface (4): surface.clear_history with a present-but-invalid surface_id silently cleared the FOCUSED surface instead of returning not_found (wrong-target side effect; hasSurfaceIDParam now crosses the seam like send_text); surface.split with an unrecognized direction returned unavailable instead of invalid_params 'Missing or invalid direction (left|right|up|down)' (coordinator now validates the parseSplitDirection token set + a drift-safe .invalidDirection case); surface.split error precedence restored (direction -> agent-session -> divider; the agent-session token check moved before divider parsing); surface.resume.* explicit target restored to surface_id ?? tab_id ONLY (terminal_id is a general routing alias but was never a resume target) and the window branch now requires a RESOLVABLE window_id like origin. Workspace (4): select/close/rename get the routing precheck so unresolvable routing returns unavailable before param validation (legacy TabManager-first order, matching reorder); workspace.current with a stale selectedTabId returns .ok with workspace:null again instead of not_found. Dead code removed (JSONValue.isControlNull, surfaceIDForInput). All confirmed by live socket sweep on the rebuilt ctl3c1 (each previously-wrong response now byte-matches origin). 133 package tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…3c-1 # Conflicts: # .github/swift-file-length-budget.tsv
…ce conformance The tests-build-and-lag job failed solely on the Swift WARNING budget: the Workspace conformance's controlWorkspaceRemotePTYAttachEnd declared 'sessionID sessionID: String' (extraneous duplicate). Behavior identical; the job's build and lag phases were green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s (drafts integrated) Five domains drafted by the orchestrator's agents (handed off), repaired (browserNavContext accessor, allocateElementRef state call, v1 handlers unhooked from the v2 chain), wired into the umbrella + dispatch, with test stubs completed. 140 package tests green. App-side surgery follows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e coordinator TerminalController.swift 18,033 -> 10,748 (-7,285). The five remaining domains now dispatch through ControlCommandCoordinator: System (identify/tree/auth.login/ session.restore/settings.open/feedback.open/extension snapshot/workspace.action/ tab.action/drag_to_split/split_off), Project (project.* + markdown.open + file.open), Debug (39 debug.* methods), Sidebar v1 (44 verbs via a new handleSidebarV1 hook ahead of the v1 switch), Browser panel v1 (8 verbs), and all 89 main-actor browser.* methods. Browser per-surface state moved off the controller: ControlBrowserAutomationState (package) + dialog responders keyed by dialogID app-side (the Sendable V2BrowserPendingDialog redesign); cleanupSurfaceState purges the new state, faithfully mirroring the legacy eviction. Two conformances the drafts never included (ControlBrowserContext, ControlBrowserPanelContext) were authored byte-faithfully from the legacy bodies. Shared bodies kept + relaxed to internal (v2Identify, v2WorkspaceAction, v2SurfaceSplitOff, v2FileOpen, the 18 v1-debug impls, the JS pump, the worker-lane browser.download.wait cluster). Deliberate deltas (documented): controlFeedbackOpen drops the deprecated .activateIgnoringOtherApps activation option (documented no-op on macOS 14+, the project floor; keeping it fails the new-file warning budget); a sequence id bridges Int64->Int (lossless on arm64). Gates: package swift build + 140/140 tests; tagged app build BUILD SUCCEEDED; live socket sweep green across all domains (system.tree, auth.login parity, browser.open_split -> get.title returns the real page title end-to-end, project validation errors, v1 set-status via the new hook, debug.terminals, plus regression of the ten prior domains); zero new warnings; both budgets pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The System+Project adversarial review found file.open had been reimplemented in the coordinator/conformance while the original v2FileOpen stayed behind (it is driven directly by FilePreviewReviewFeedbackTests and MarkdownPanelTests) - two copies that could drift, and a stale dispatcher comment claiming forwarding. file.open now forwards to the single shared body and bridges its result, like workspace.create; the reimplementation and its now-unused ControlFileOpenResolution/ControlFileOpenSurface types are deleted. Review verdicts so far: System+Project all faithful (this was the only finding, not a behavior bug); Debug (39 verbs) + Sidebar v1 (44) + Browser-panel v1 (8) all faithful, zero divergences, #if DEBUG gating verified end-to-end. 140 package tests green; app build green; live probe of file.open through the shared body (happy path + both error shapes) byte-faithful. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h residue The Browser adversarial review (87/89 faithful) found its only two divergences share one root cause: focus_mode.set and zoom.set validated mode/direction BEFORE the TabManager/handle guards (legacy order: guards first). The shared browserFocusedAction helper gains a post-guard validate step; both methods' validation moves there. Live-verified: double-fault now returns unavailable/'TabManager not available', single-fault the mode/direction error. Residue: socketFastPathState drops its 'nonisolated' (after the cutover its only callers are the @mainactor sidebar/surface conformances; the worker-thread fast path retired with the legacy dispatcher). ServerEventTarget's @unchecked Sendable and the V2CallResult/V2SocketRequest twins stay deliberately: they serve the worker-lane and kept-shared bodies, which move in a later wave (the target itself dissolves with TerminalControlComposition in Wave 5). Verification totals for the five stacked domains: 143 methods/verbs reviewed per-method vs the pre-deletion originals; 141 faithful as-lifted, 2 fixed here. 140 package tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…uard) Inherited from the base branch; label equals name so the external signature is unchanged and no callers move. Keeps the local Swift warning budget green (documented toolchain skew: CI Xcode 16.4 accepts this silently). Net line delta: 0. Tests: n/a (no behavior change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract the multi-window CLI-over-socket capability (blueprint Wave-2 "PR 2") out of Sources/AppDelegate.swift into a new CmuxIPCService package: MultiWindowRouting seam, MultiWindowRouteResult DTO, and MultiWindowRouter (Sendable struct; cliURL/socketPath/environment constructor-injected). The private MultiWindowRouteCLIResult struct and runMultiWindowRouteCLI free function are deleted; the only caller (runMultiWindowWindowRouteCLIIfNeeded, UI-test scaffolding) constructs the router behind its existing guards and forwards. Faithful lift: the route body is byte-identical modulo the seam spellings (verified by normalized machine-diff against git show HEAD:Sources/AppDelegate.swift); the synchronous Process.run + waitUntilExit contract and the global-queue caller are preserved. The async-throws modernization lands in the next commit. The router carries the minimal equivalent of the app-side ProcessPipeReader read loop (chunked read, EINTR retry, partial data + logged warning) because that type stays app-target for its other callers; only the os_log category differs. MultiWindowRouter is a stateless Sendable struct, not the blueprint's sketched actor: it holds only immutable configuration, so an actor would serialize unrelated route calls while protecting nothing (LEARNINGS ruling; CmuxProcess.CommandRunner precedent). Net line delta: Sources/AppDelegate.swift 18044 -> 18003 (-41); +331 package lines (Package.swift + 3 sources + tests). Tests: 5 new behavior tests (swift test green; spawn a scripted CLI stand-in to pin --socket prepending, exact-env replacement, stderr/exit-status capture, the legacy "-1" launch-failure encoding, and non-UTF-8 output collapsing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughExtracts multi-window CLI routing into a new CmuxIPCService Swift package (protocol, router, types, and tests), replaces the AppDelegate’s legacy Process helper with async/await routing, and wires the package into the cmux app target. ChangesCmuxIPCService Package & Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (17 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR extracts the multi-window CLI-over-socket routing capability from
Confidence Score: 5/5Safe to merge; the extraction is behavior-preserving and the modernization strictly improves correctness by removing a blocking-thread-pool wait and fixing a pipe-buffer deadlock. The new package follows all established house patterns: Sendable struct router, nonisolated async route, terminationHandler continuation instead of waitUntilExit, concurrent detached pipe readers matching CommandRunner, correct MainActor inheritance in the AppDelegate caller, and nonisolated file-scoped logger. Seven behavior tests with a scripted CLI stand-in cover every behavioral claim in the PR description. Previously flagged issues (nonisolated logger, public Foundation import, blocking readers) have all been addressed in the head commits or correctly explained as house-pattern-consistent. No files require special attention. The only residual item the PR itself flags — manual smoke-test of the tagged build with a real second window — is a product-QA step, not a code defect. Important Files Changed
Sequence DiagramsequenceDiagram
participant AD as AppDelegate (MainActor)
participant T as Task (MainActor-inherited)
participant R as MultiWindowRouter
participant P as Process (cmux CLI)
participant DR as Detached Readers (2x)
AD->>T: Task(priority: .userInitiated)
T->>R: await routeCapturingLaunchFailure(create args)
R->>DR: Task.detached stdout/stderr readers
R->>P: process.run() + close write ends
P-->>R: terminationHandler fires
DR-->>R: "EOF -> Data"
R-->>T: MultiWindowRouteResult (create)
T->>R: await routeCapturingLaunchFailure(window2 args)
R->>DR: Task.detached readers
R->>P: process.run() + close write ends
P-->>R: terminationHandler fires
DR-->>R: "EOF -> Data"
R-->>T: MultiWindowRouteResult (window2List)
T->>R: await routeCapturingLaunchFailure(window1 args)
R->>DR: Task.detached readers
R->>P: process.run() + close write ends
P-->>R: terminationHandler fires
DR-->>R: "EOF -> Data"
R-->>T: MultiWindowRouteResult (window1List)
T->>AD: writeMultiWindowNotificationTestData (on MainActor)
Reviews (7): Last reviewed commit: "Merge origin/main into feat-ipc-service:..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Convert the lifted synchronous contract in a separated commit per the faithful-lift discipline: - MultiWindowRouting.route becomes async throws; launch failure now throws MultiWindowRouteLaunchError (new one-type file) whose CustomStringConvertible preserves the underlying error's String(describing:) text verbatim. - MultiWindowRouter replaces waitUntilExit (which blocked a global-queue thread) with a terminationHandler continuation, and drains stdout/stderr concurrently with the child via detached readers keyed by raw fd (the CmuxProcess.CommandRunner pattern). - MultiWindowRouteResult.status: String becomes terminationStatus: Int32; the caller formats String(terminationStatus), producing identical strings for every reachable value. - routeCapturingLaunchFailure protocol-extension convenience (the CommandRunning.runStandardOutput precedent) folds the throw back into the legacy capture encoding (-1 status, error description in stderr) so each of the three UI-test calls still runs independently and the test-data file bytes stay identical. - AppDelegate's DispatchQueue.global + DispatchQueue.main.async pair becomes one MainActor-inherited Task(priority: .userInitiated) with sequential awaits; ordering, the weak-self-gated write, and the main-thread write destination are preserved. Observer analysis: the only behavior change is that CLI output larger than the 64 KiB pipe buffer no longer deadlocks the call forever (legacy read the pipes only after exit); pinned by a new test. Net line delta: Sources/AppDelegate.swift 18003 -> 18003 (0); package +91 lines. Tests: 7 (was 5; +launch-error description preservation, +capture encoding, +pipe-buffer no-deadlock), green from a clean .build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
stage 3c (stacked): final five domains — System/Project/Debug/Sidebar/Browser
…est, unused imports) - @ObservationIgnored on the coordinator's handles registry: it is a struct mutated by ref() on nearly every response, so tracking it would invalidate any observer on every socket command (greptile). - windowCloseOkAndNotFound now also asserts the not_found branch (coderabbit). - Drop unused Foundation imports from ControlAppFocusContext and ControlMobileHostContext (coderabbit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
House-style shape for file-scoped os.Logger constants (matches the app-side ProcessPipeReader precedent); no behavior change. Net line delta: 0. Tests: 7 (unchanged, green). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ert the coordinator's v2 browser.* domain PR 5778 moved the JS-evaluating browser.* methods onto a nonisolated socket-worker lane while this branch had lifted the (pre-5778) browser domain into the @mainactor coordinator. The two designs are incompatible and main's is the behavioral reference, so this merge takes main's browser implementation wholesale and deletes the coordinator's browser-v2 domain (package files, app conformances, umbrella members, tests). The v1 browser-panel and sidebar handlers and the other 13 coordinator domains are untouched by main and stay. mobile.terminal.paste (new in PR 5876) dispatches from the legacy v2 switch. The browser domain gets re-lifted in a follow-up against the worker-lane architecture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Base moved to 2be538e (main sync: socket-worker browser JS lane, v2 browser.* revert, final-five 3c domains). Conflict resolution: .github/swift-file-length-budget.tsv only — union of both sides, conflicted entries recomputed to actual merged line counts (ContentView 19256, AppDelegate 17980). AppDelegate.swift and pbxproj auto-merged; pbxproj normalized and checked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Main brought 16 commits since the sync point, including the 3c coordinator merge (6743387), renderer realization GPU reclaim plus ghostty submodule bump to 5697db8 (aeb8847), browser proxy and download fixes, agent launch capture trust, iOS View as Text, and v0.64.15. Resolutions: - TerminalController+ControlWorkspaceContext.swift (add/add): took main's version verbatim; the branch side differed only by a trailing blank line inherited from feat-ctl-coordinator-3c-1. - swift-file-length-budget.tsv: regenerated from actual line counts (--write-budget), which also drops the 4 stale entries for the 3c browser-v2 coordinator files that exist on neither side. - ghostty submodule: main's 5697db8 (branch pointer 34cbf18 was inherited, no slice commits). Packages/CmuxControlSocket is identical to origin/main (zero-path diff); no inherited 3c-only socket content to strip. Gates: budget, lint-ios-package-conventions, swift build CmuxIPCService, xcodebuild Debug compile all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # .github/swift-file-length-budget.tsv
…5907 Resolve two shared-infra conflicts brought by CmuxTerminalCore (#5894) and CmuxSidebarGit (#5907): - cmux.xcodeproj/project.pbxproj: union the XCSwiftPackageProductDependency block (keep both CmuxIPCService and main's CmuxTerminalCore entries); normalize + check-pbxproj + xcodebuild -list all pass, braces balanced. - .github/swift-file-length-budget.tsv: regenerate from the merged tree via swift_file_length_budget.py --write-budget; budget respected (exit 0). ghostty submodule pointer matches origin/main. Local app build (xcodebuild -project cmux.xcodeproj -scheme cmux build) = BUILD SUCCEEDED, 0 errors, confirming no silent cross-file break from the merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit
added a commit
that referenced
this pull request
Jun 13, 2026
origin/main moved forward after the first resync (#6052 sidebar row cleanups, #5920 CmuxIPCService MultiWindowRouter extraction). Only conflict was the regenerable swift-file-length-budget.tsv; AppDelegate/ContentView/pbxproj auto-merged. Budget regenerated from the merged tree; pbxproj normalized + parses; runMultiWindowRouteCLI confirmed single-homed in CmuxIPCService. Local Debug app build: ** BUILD SUCCEEDED **. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit
added a commit
that referenced
this pull request
Jun 13, 2026
…/5920/5921) Resolve conflicts from the parallel main-side decomposition: - Workspace.swift: drop the WorkspaceRemote* inline types kept on main; 5896 lifted them to CmuxRemoteDaemon/CmuxRemoteSession/CmuxRemoteWorkspace. - AppDelegate.swift: drop the re-added runMultiWindowRouteCLI; it lifted to CmuxIPCService.MultiWindowRouter on main (#5920). - Import unions in ContentView / TerminalController / TerminalController+ ControlWorkspaceContext / GhosttyConfigTests (CmuxCore + CmuxPanes/ CmuxWorkspaces + CmuxBrowser + CmuxRemote* coexist). - pbxproj: union the conflict regions, then drop stale wiring for files that main deleted via decomposition (SplitEqualizer.swift -> CmuxPanes; RemoteLoopbackProxyAlias.swift -> CmuxCore). - swift-file-length-budget.tsv: regenerate from the merged tree. Port #6056 (OpenWrt BusyBox remote platform probe) into its new home: main added Sources/WorkspaceRemotePlatformProbeScript.swift as an extension on the now-lifted WorkspaceRemoteSessionController. Re-home the fix into RemoteSessionCoordinator+Bootstrap.swift (BusyBox-portable literal case normalization instead of `tr '[:upper:]' '[:lower:]'`, version path-segment sanitization, marker-stripped user-facing stdout, armv7 arch) and port the regression tests into CmuxRemoteSession's test target. Delete the orphan app-target probe + test files that referenced the deleted type. ghostty pointer unchanged (matches origin/main). CmuxControlSocket stays zero-diff vs main. RemoteSessionProcessRunner @suite(.serialized) preserved. Gates: ensure-ghosttykit OK, swift_file_length_budget exit 0, lint exit 0, xcodebuild app build SUCCEEDED, swift test CmuxRemoteSession 22/22 pass. Co-Authored-By: Claude Fable 5 <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.
Wave-2 slice of the AppDelegate decomposition (blueprint "PR 2", the canonical small service extraction): the multi-window CLI-over-socket routing capability moves out of
Sources/AppDelegate.swiftinto a newPackages/CmuxIPCService.Based on
feat-ctl-coordinator-3c-1on purpose; retarget tomainafter #5816 merges.What moved
MultiWindowRouteCLIResult(private struct) +runMultiWindowRouteCLI(private free function, synchronousProcess.run+waitUntilExit) deleted fromSources/AppDelegate.swift.CmuxIPCService:MultiWindowRouting(seam protocol),MultiWindowRouteResult(Sendable DTO),MultiWindowRouteLaunchError,MultiWindowRouter(production conformer;cliURL/socketPath/environmentconstructor-injected). Swift 6 manifest per conventions (6.0,.macOS(.v14), one.library, per-targetswiftLanguageMode(.v6)+ExistentialAny+InternalImportsByDefault), one major type per file, DocC on every public symbol.runMultiWindowWindowRouteCLIIfNeeded(UI-test scaffolding gated onCMUX_UI_TEST_WINDOW_ROUTE_CLI=1, exercised end to end bycmuxUITests/MultiWindowNotificationsUITests.swift), constructs the router behind its existing guards and forwards.Two-commit structure per the faithful-lift discipline:
git show HEAD:Sources/AppDelegate.swift.Process.run+waitUntilExitbecomesasync throws(deltas analyzed below).Line delta and tests
Sources/AppDelegate.swift: 18044 -> 18003 (-41; budget tsv updated, monotonic shrink held through both commits).import Testing), green viaswift testfrom a clean.build. Tests spawn a scripted CLI stand-in to pin:--socketprepending and argument forwarding, wholesale environment replacement (injected keys visible, parent env not inherited), stderr/exit-status capture, the throwing launch-failure contract withString(describing:)text preservation, the legacy-1capture encoding, non-UTF-8 output collapsing to empty, and >pipe-buffer output completing without deadlock.sessionIDparameter label inSources/TerminalController+ControlWorkspaceContext.swift(base-branch file; label equals name so no caller changes; keeps the local Xcode 26.5 warning budget green, CI Xcode 16.4 never emits it).Isolation decision (deviation from the blueprint's "actor" naming, justified)
The blueprint sketches
MultiWindowRouteras an actor. The router holds only immutable Sendable configuration (CLI URL, socket path, child environment); there is no mutable state to isolate, and an actor would serialize unrelated route calls while protecting nothing. Per the LEARNINGS ruling ("don't keep a clearly-stateless actor"; GitMetadataService actor -> struct on owner review) and the closest prior shape (CmuxProcess.CommandRunner, a stateless Sendable struct),MultiWindowRouteris aSendablestruct with a nonisolated async method. Seam and DTO names stay blueprint-faithful.Router construction happens at the call site rather than as a stored AppDelegate property: the dependencies (bundled CLI URL, socket path, merged env) are resolved behind the existing call-time guards that produce the CLI-observable failure markers (
missing_cli,socket_not_ready); a half-configured router stored at launch would add an optional property with no consumer. Constructor injection of a Sendable value where its configuration is known.Why this does not reuse CmuxProcess.CommandRunner
CommandRunning's contract differs from the route's exact semantics: it resolves executables against PATH/fallbacks (the route uses an exact bundled-CLI URL), it lets the child inherit the app environment (the route replaces the environment wholesale, which the tests pin), it nulls stdin, and it has its own timeout machinery (the route's timeout is the CLI's ownCMUXTERM_CLI_RESPONSE_TIMEOUT_SEC). Extending the sharedCommandRunningseam to carry env-replacement semantics would change a protocol other call sites depend on, exceeding this slice. Convergence can be evaluated later if the two contracts grow together.Deltas with observer analysis
Commit 1 (lift):
ProcessPipeReader(shared by other app sources, stays put), so the router carries the minimal equivalent read loop (chunkedread,EINTRretry, partial data + logged warning on error). Identical data path; the only delta is the os_log category and terser warning text. Observable only in unified-log diagnostics, never in CLI output or the UI-test data file.Commit 2 (modernization):
MultiWindowRouteLaunchErrorinstead of returning a-1result. The caller usesrouteCapturingLaunchFailure(package protocol extension, theCommandRunning.runStandardOutputconvenience precedent), which folds the throw back into the legacy encoding: termination status-1,String(describing:)of the error in stderr. The error'sCustomStringConvertiblepreserves the underlying Foundation error text verbatim, so the test-data file bytes are identical; each of the three calls still runs independently after an earlier launch failure, exactly like the legacy loop.waitUntilExit(blocking a global-queue thread) becomes aterminationHandlercontinuation; the caller'sDispatchQueue.global+DispatchQueue.main.asyncpair becomes one MainActor-inheritedTask(priority: .userInitiated)with sequential awaits. Ordering (create, then window-2 list, then window-1 list), the[weak self]-gated final write, and the write landing on the main thread are all preserved. No code between the calls does blocking work, so nothing observable moves on or off main.status: StringbecomesterminationStatus: Int32; the caller writesString(result.terminationStatus), producing identical strings for every reachable value.CommandRunnerpattern) instead of read after exit. Captured bytes are identical for every run that completed before; the only behavior change is that CLI output larger than the 64 KiB pipe buffer no longer deadlocks the call forever (previously the worker thread hung and the UI test timed out). Strict improvement, pinned by a test.End-to-end verification
MultiWindowNotificationsUITests(setsCMUX_UI_TEST_WINDOW_ROUTE_CLI=1and asserts on thewindowRoute*entries the extracted router writes into the shared test-data file) ran green on this branch via hosted CI: https://github.com/manaflow-ai/cmux/actions/runs/27384249666. That run covered the modernized router (commit 3c4273e); the one later commit (9cbcddd) only adds anonisolatedannotation to a file-scoped logger constant, with no codegen or behavior change.The one thing not verified
No human has launched the tagged build (
ipcsvc) to confirm general app behavior is unaffected. Nothing outside theCMUX_UI_TEST_WINDOW_ROUTE_CLI=1UI-test path executes this code, so the residual risk is link-time only (new package product wired into the app target). Manual check: open the tagged build, create a second window, and confirm workspaces open and route normally between windows.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores