CmuxFleet: core engine — seams, app actuator/bridge, hook supervision, persistence - #7418
austinywang wants to merge 11 commits into
Conversation
…7359) * Add failing regression tests for ssh hosts configured with RemoteCommand/RequestTTY cmux ssh against a host alias whose ssh_config sets `RequestTTY yes` and `RemoteCommand sudo su -` exits 255 with OpenSSH's "Cannot execute command-line and remote command." and loops the reconnect banner (issue #7246): every cmux-controlled invocation that supplies its own remote command (foreground auth `true`, bootstrap installer hop, daemon stdio transport, coordinator batch plumbing, ssh-tmux control commands) inherits the host RemoteCommand instead of overriding it. Covers, all red without the fix: - cmuxTests/SSHConfiguredRemoteCommandHostTests: end-to-end `cmux ssh` startup scripts (persistent-PTY foreground-auth flow and bootstrap install flow) against a fake ssh that mirrors OpenSSH's rule, plus the app-side SSHPTYAttachStartupCommandBuilder foreground auth argv. - cmuxTests/RemoteTmuxHostRemoteCommandOverrideTests: shared ssh-tmux control args, interactive auth, and tmux -CC control-mode argv. - CmuxCoreTests: daemonTransportArguments (cmuxd stdio transport). - CmuxRemoteSessionTests: coordinator batch exec argv (port scan) and override/RequestTTY ordering ahead of caller-configured options. Part 1 of 2 (test-only, expected red); the fix lands separately so CI proves these tests catch the bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Override host-configured RemoteCommand in cmux-controlled ssh invocations Fixes `cmux ssh` (and every other cmux-built ssh exec) against host aliases whose ssh_config sets `RemoteCommand` (typically with `RequestTTY yes`): OpenSSH refuses a command-line remote command while a configured RemoteCommand is in effect ("Cannot execute command-line and remote command.", exit 255), so the foreground auth hop died before the session ever started and the pane looped reconnect attempts (issue #7246). New shared CmuxFoundation constant `SSHHostConfiguredRemoteCommand` (`-o RemoteCommand=none`, OpenSSH >= 7.6 — macOS has shipped newer clients since 10.13.2) applied at every builder that appends its own remote command: - CLI `cmux ssh`: foreground-auth hop, bootstrap installer hop, and the `cmux ssh <dest> -- <command>` passthrough branch (inserted right after `ssh`, so it also wins over caller-supplied options under OpenSSH's first-value-per-option rule). The interactive session hop keeps carrying cmux's own `-o RemoteCommand=<bootstrap>`, which already overrides the host config; bare interactive invocations (VM attach) are untouched. - App restore/reattach: SSHPTYAttachStartupCommandBuilder foreground auth. - Coordinator batch plumbing (bootstrap probes/install, BootstrapTTY, port scans, upload cleanup, relay metadata, stale-listener cleanup): sshCommonArguments(batchMode:) now also pins `-o RequestTTY=no` so a host `RequestTTY force` cannot CRLF-corrupt parsed pipes. - CmuxCore daemonTransportArguments (cmuxd stdio transport). - ssh-tmux stack via RemoteTmuxHost.sshControlArguments (interactive auth, `tmux -CC` control mode — which keeps its forced `-tt` — and one-shot discovery/mutation commands). - File explorer listing, remote git status, and drag-drop upload cleanup argv builders. Invocations with no remote command (`-N` forwards, `-O` control ops, `-G` config dumps, plain interactive shells) are unchanged, and hosts without a configured RemoteCommand see identical behavior — the override is inert there. The CLIRemoteShellStartupPerformanceTests fake ssh now mirrors OpenSSH's real RemoteCommand semantics (first value wins, `none` clears) so the installer hop's new override falls through to the positional command exactly like real ssh. Fixes #7246 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Make SSHHostConfiguredRemoteCommand an instantiable struct per package conventions The package-conventions lint forbids all-static public namespace types in packages; follow the SSHAgentSocketResolver pattern (public struct with a public initializer) and access the override via an instance at every call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Part 2 of the Fleet chain (#7361). Adds the fleet.* command domain to CmuxControlSocket following the coordinator seam pattern: typed wire values, ControlFleetContext seam protocol, handleFleet dispatch for fleet.list/create/start/stop/status and fleet.task.add/list/retry/cancel/open, plus coordinator tests. The app target conforms with pre-engine stubs (empty reads, unavailable mutations) until PR 3 wires the FleetEngine bridge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eat-fleet-core-engine
…ence Part 3 of the Fleet chain (#7361). FleetEngine (CmuxFleet package) drives the pure supervisor/scheduler through seam protocols: provisioning git worktrees + unfocused grouped workspaces per task, typing the agent command into the real terminal, supervision via workstream hook events, kqueue pid-exit watchers, stall/backoff timers and a reconcile tick (workspace-gone, PR badge changes, prompt-idle fallback), with JSON persistence across relaunches. App-side seams live in Sources/Fleet/; the workstream hook tap rides a new CmuxWorkstreamEventPublishing extraction that shrinks the at-cap CmuxEventPublishing.swift. The PR 2 fleet.* socket stubs are replaced by the engine bridge, so fleet create/start/task.add/list/retry/cancel/open work end-to-end over cmux rpc. Fleet notification strings are localized (EN/JA). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Greptile SummaryAdds the Fleet core engine with app seams (
Confidence Score: 4/5The engine logic, timer/process-watcher patterns, actor isolation, and SSH fix are all solid; the main outstanding concern noted in a prior review thread — blocking disk I/O running on the main actor inside FleetAppPersistence — has not been addressed in this PR and affects every task state transition. The fleet engine architecture is well-structured and the new app seams are correct. The unresolved blocking I/O on the main actor (raised in the previous review thread) remains in FleetAppPersistence.save() and FleetEngine.restore(), and the FleetAppHost.shared singleton introduced here holds live runtime state in global scope. Sources/Fleet/FleetAppPersistence.swift (blocking I/O on main actor, unaddressed from prior thread) and Sources/Fleet/FleetAppHost.swift (new singleton for runtime engine state). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
CS[Control Socket fleet commands] -->|MainActor| CC[ControlCommandCoordinator]
CC -->|ControlFleetContext| TC[TerminalController]
TC --> FAH[FleetAppHost.shared]
FAH --> FE[FleetEngine]
WS[Workstream Hook publishWorkstreamEvent] -->|Task MainActor| FAH
FE -->|FleetActuating| FAA[FleetAppActuator]
FE -->|FleetWorldReading| FAWR[FleetAppWorldReader]
FE -->|FleetTimerScheduling| FAT[FleetAppTimers]
FE -->|FleetProcessWatching| FAPW[FleetAppProcessWatcher]
FE -->|FleetPersisting| FAP[FleetAppPersistence]
FAA -->|Task.detached| GIT[git worktree add]
FAA --> TM[TabManager addWorkspace]
FAP -->|atomic write| DISK[(fleet-state.json)]
FE -->|restore on init| DISK
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
CS[Control Socket fleet commands] -->|MainActor| CC[ControlCommandCoordinator]
CC -->|ControlFleetContext| TC[TerminalController]
TC --> FAH[FleetAppHost.shared]
FAH --> FE[FleetEngine]
WS[Workstream Hook publishWorkstreamEvent] -->|Task MainActor| FAH
FE -->|FleetActuating| FAA[FleetAppActuator]
FE -->|FleetWorldReading| FAWR[FleetAppWorldReader]
FE -->|FleetTimerScheduling| FAT[FleetAppTimers]
FE -->|FleetProcessWatching| FAPW[FleetAppProcessWatcher]
FE -->|FleetPersisting| FAP[FleetAppPersistence]
FAA -->|Task.detached| GIT[git worktree add]
FAA --> TM[TabManager addWorkspace]
FAP -->|atomic write| DISK[(fleet-state.json)]
FE -->|restore on init| DISK
Reviews (3): Last reviewed commit: "Reuse live workspaces on fleet retry; dr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "{{PROMPT}}": shellQuoted(prompt), | ||
| "{{TITLE}}": shellQuoted(task.title), | ||
| "{{BODY}}": shellQuoted(task.body), | ||
| "{{TASK_ID}}": task.id.rawValue, | ||
| "{{DIR}}": directory, | ||
| "{{BRANCH}}": branch ?? "", | ||
| ] | ||
| var rendered = template |
There was a problem hiding this comment.
{{DIR}} breaks commands on paths with spaces
{{PROMPT}}, {{TITLE}}, and {{BODY}} are correctly wrapped by shellQuoted(), but {{DIR}} and {{BRANCH}} are inserted verbatim into the shell command. If the user's repo lives in a path with spaces — /Users/Jane Smith/code → worktree at /Users/Jane Smith/code-fleet/task — any template that expands {{DIR}} (e.g. cd {{DIR}} && claude {{PROMPT}}) will split on the space and fail silently. {{BRANCH}} is sanitized by FleetPathSanitizer so the risk is lower there, but {{DIR}} includes the user-supplied repoRoot and is directly unsafe.
| "{{PROMPT}}": shellQuoted(prompt), | |
| "{{TITLE}}": shellQuoted(task.title), | |
| "{{BODY}}": shellQuoted(task.body), | |
| "{{TASK_ID}}": task.id.rawValue, | |
| "{{DIR}}": directory, | |
| "{{BRANCH}}": branch ?? "", | |
| ] | |
| var rendered = template | |
| let replacements = [ | |
| "{{PROMPT}}": shellQuoted(prompt), | |
| "{{TITLE}}": shellQuoted(task.title), | |
| "{{BODY}}": shellQuoted(task.body), | |
| "{{TASK_ID}}": task.id.rawValue, | |
| "{{DIR}}": shellQuoted(directory), | |
| "{{BRANCH}}": branch.map(shellQuoted) ?? "", | |
| ] |
| enum FleetWorkstreamTap { | ||
| /// Forwards one hook event when Fleet already owns the event workspace. |
There was a problem hiding this comment.
FleetWorkstreamTap, FleetControlSocketMapping (Sources/Fleet/FleetControlSocketMapping.swift), and FleetPromptTemplate (Packages/macOS/CmuxFleet/Sources/CmuxFleet/Engine/FleetPromptTemplate.swift) are all caseless enums whose entire public surface is static functions. The cmux-no-ambient-global-state rule flags this pattern. The canonical fix is to represent the mapping as free functions (for nonisolated utilities) or as methods on the owning type — e.g. FleetWorkstreamTap.handle could be a method on FleetAppHost or FleetEngine, and the pure mapping utilities in FleetControlSocketMapping could be extensions on FleetTaskState/ControlFleetTaskStateName. The pattern appears in all three files.
Rule Used: Flag new ambient global state in production Swift:... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
fleetID.map(FleetID.init) is ambiguous across the three String initializers; name init(rawValue:) explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
The package-conventions-lint namespace-type rule rejects all-static public types. Convert the caseless enum to a Sendable struct with a public init and instance render(...), matching FleetPathSanitizer and FleetBackoff in this package. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| func load() -> FleetPersistedState? { | ||
| guard let fileURL, | ||
| let data = try? Data(contentsOf: fileURL) | ||
| else { return nil } | ||
| do { | ||
| let decoder = JSONDecoder() | ||
| decoder.dateDecodingStrategy = .iso8601 | ||
| let state = try decoder.decode(FleetPersistedState.self, from: data) | ||
| lastData = data | ||
| return state | ||
| } catch { | ||
| #if DEBUG | ||
| cmuxDebugLog("fleet.persistence.load.failed \(error.localizedDescription)") | ||
| #endif | ||
| return nil | ||
| } |
There was a problem hiding this comment.
@MainActor in both save() and load()
load() calls Data(contentsOf: fileURL) + JSONDecoder.decode synchronously on the main actor. The first call to any fleet control-socket command (e.g. fleet.create, fleet.list) reaches FleetAppHost.shared.engine, which triggers FleetEngine.init → restore() → persistence.load(). That entire chain runs synchronously on the main actor, so a cold-start fleet command blocks the main thread for a full file read + JSON decode while the socket worker is waiting.
save() has the same problem in the hot direction: FileManager.createDirectory + data.write(to:options:.atomic) run synchronously on @MainActor on every task state transition. With several concurrent tasks, this can be called multiple times per second, each time blocking the main thread for an atomic rename.
The FleetPersisting protocol is marked @MainActor, which forces all conformers to block the main thread for I/O. Making load() async (or @concurrent) and moving the write inside a Task.detached(priority: .utility) would keep disk work off the main actor; the main-actor portion only needs to apply the decoded state.
Rule Used: Flag production Swift that reads, decodes, or scan... (source)
Regression tests first (red) per repo policy:
- reconcile must deliver prChanged to .failed tasks so an open PR
rescues them to .awaitingReview and a merged PR to .done with
workspace cleanup; today reconcile skips terminal tasks entirely.
- FleetPromptTemplate must shell-quote {{DIR}} and {{BRANCH}} so
paths with spaces cannot split the rendered agent command.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reconcile pass skipped every terminal task, so the supervisor's
prChanged rescue transitions (failed -> awaitingReview on an open PR,
failed -> done on a merged PR) were unreachable: a task that failed
before its PR badge populated stayed failed forever. Keep probing
pull-request status for .failed tasks that still have a workspace while
still skipping .done/.cancelled, and keep the workspace-gone
cancellation for non-terminal tasks only.
FleetPromptTemplate now shell-quotes {{DIR}} and {{BRANCH}} like the
other string placeholders so a repository path with spaces cannot split
the rendered agent command.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FleetControlSocketMapping and FleetWorkstreamTap were caseless enums whose whole surface was static functions (static-as-namespace policy). Express the socket wire mapping as extensions on the mapped types (FleetTaskState/ControlFleetTaskStateName properties and ControlFleetSnapshot/ControlFleetTaskSnapshot inits) and move the workstream tap onto FleetAppHost as an instance method, dropping the now-redundant liveHost/hasLiveEngine statics. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Red per repo regression policy: - retrying a failed task whose workspace still exists must not provision a duplicate workspace; the relaunch reuses the original workspaceID and only resends the agent command. - when the old workspace is gone and a replacement is provisioned, the old taskIDByWorkspaceID mapping must be dropped so stale hooks for the replaced workspace no longer bind to the task. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Green for the preceding red tests: re-provisioning a task whose workspace still exists reuses it (no duplicate workspace, command resent in place), and when a replacement workspace is created the old taskIDByWorkspaceID entry is removed so stale hooks cannot bind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Scope
Part 3 of #7361 — the Fleet core engine. Stacked on #7374 (PR 1, base branch of this PR) and #7403 (PR 2, merged into this branch); the last commit is this PR's own diff.
FleetEngine(Packages/macOS/CmuxFleet): imperative engine driving the pureFleetSupervisor/FleetSchedulerthrough seam protocols (FleetActuating,FleetWorldReading,FleetTimerScheduling,FleetProcessWatching,FleetPersisting). Handles dispatch ticks, provisioning bookkeeping with per-task generations (cancel-during-provision closes orphan workspaces), attempt-scoped backoff + stall timers, hook-signal mapping (claudeStop= per-turn activity;SessionEnd/pid-exit are end-of-run), reconcile pass (workspace-gone → cancelled, PR badge → prChanged → awaiting_review/done, prompt-idle fallback with grace window), and JSON persistence with restore.Sources/Fleet/): actuator provisions a git worktree (git worktree add -b fleet/<task>) + an unfocused workspace grouped under the fleet's name (addWorkspace(select: false)+ group APIs), types the agent command via the queuedsendInputResultpath, kills via pid/ETX, posts localized notifications (EN/JA); world reader consumes the sidebar PR badge and shell-activity state; timers/pid watchers follow theFeedCoordinator.armPidWatcherDispatchSourceProcesspattern with identity-guarded, cycle-free fire paths; persistence mirrors the session-snapshot conventions (atomic, sortedKeys, identical-content skip).publishWorkstreamEventmoved to a newSources/CmuxWorkstreamEventPublishing.swift(shrinking at-capCmuxEventPublishing.swift502 → 434) and now forwards phase-"received" hooks to the engine when it is live.fleet.*socket stubs are replaced by the engine bridge;fleet.task.openfocuses the task workspace through the same bodyworkspace.focususes — the single focus-changing fleet path.CmuxControlSocketpackage untouched; both budget TSVs untouched; no new package dependencies.Tests
cd Packages/macOS/CmuxFleet && swift test— 56 tests (engine happy path, needsInput round-trip, turn-vs-end semantics, retry/backoff to failed, pid-exit + stale-attempt drops, PR handoff to awaiting_review/done, scheduler caps, cancel/retry, reconcile workspace-gone + prompt-idle grace (positive and negative), stall, persistence round-trip/restore, open targets, unknown-workspace + terminal-state hook ignores, cancel-during-provision stale-outcome drop).cd Packages/macOS/CmuxControlSocket && swift test— 187 tests unchanged.scripts/normalize-pbxproj.pyidempotent,check-pbxproj.shgreen); all new files < 500 lines.claude -p; two tasks provisioned into worktree workspaces, supervised to PR handoff) — transcript in the PR discussion after dogfood.Plan/code/judge loop: plan + 3 coder rounds + 2 judge passes; final judge issue (timer retain cycle) fixed verbatim.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds the Fleet core engine with app seams, control-socket bridge, hook supervision, and persistence so fleets can be created, started, and run tasks end-to-end, with PR rescue for failed tasks and smarter retries. Also fixes SSH failures on hosts with a configured RemoteCommand by overriding it with
-o RemoteCommand=noneacross cmux-controlled SSH invocations.New Features
CmuxFleetengine: provisions git worktrees, launches agents, supervises via workstream hooks, runs backoff/stall timers and pid watchers, reconciles state (now rescues failed tasks on PR open/merge and only cancels on workspace-gone for non-terminals), reuses live workspaces on retry and drops stale workspace mappings, and persists/restores JSON snapshots.Sources/Fleet/, workstream hook tap (CmuxWorkstreamEventPublishing.swift) forwarding to the engine, and localized Fleet notifications (EN/JA).fleet.*in@CmuxControlSocketand bridged to the engine;fleet.task.openfocuses via the existing workspace focus path.FleetPromptTemplateis now an instantiable struct to comply with package conventions.Bug Fixes
-o RemoteCommand=noneahead of destinations for all cmux-supplied commands across CLI,@CmuxCoredaemon transport, tmux control, file explorer, git status, and@CmuxRemoteSession; for batch SSH execs, also force-o RequestTTY=no.FleetPromptTemplateplaceholders for{{DIR}}and{{BRANCH}}so paths with spaces cannot split rendered commands.FleetIDin the fleet task-list control-bridge to ensure correct ID parsing.Written for commit fdb31ea. Summary will update on new commits.