CmuxFleet: engine package skeleton (model, supervisor, scheduler, backoff) - #7374
Closed
austinywang wants to merge 7 commits into
Closed
austinywang wants to merge 7 commits into
austinywang wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds the CmuxFleet SwiftPM package, its core Fleet models and supervision logic, deterministic backoff/dispatch/path utilities, CI and workspace wiring, and unit tests covering the new behaviors. ChangesCmuxFleet package implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FleetSignal
participant FleetSupervisor
participant FleetBackoff
participant FleetCommand
FleetSignal->>FleetSupervisor: reduce(task, signal)
FleetSupervisor->>FleetSupervisor: validate state and choose transition
alt retry or relaunch
FleetSupervisor->>FleetBackoff: delayMS(attempt, maxRetryBackoffMS)
FleetBackoff-->>FleetSupervisor: delayMS
FleetSupervisor->>FleetCommand: scheduleBackoff / resendAgentCommand
else cancel or complete
FleetSupervisor->>FleetCommand: killAgent / cleanupWorkspace / cancelBackoff
else notify
FleetSupervisor->>FleetCommand: postNotification
end
FleetSupervisor-->>FleetSignal: updated task and emitted commands
Possibly related issues
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR adds
Confidence Score: 5/5This is a pure, dependency-free value-type package with no I/O, timers, UI, or app wiring — all logic is covered by deterministic unit tests that run in isolation. The reducer, scheduler, backoff, and sanitizer are all pure functions that are straightforwardly testable and tested. The matrix test in FleetSupervisorTests exercises every signal against every state, the stale-signal and PR-race suites cover key edge cases, and the FleetBackoff and FleetPathSanitizer tests cover boundary and overflow conditions. No blocking primitives, global state, or production-source test seams were introduced. The one open gap (a stalled task with an open PR receiving agentStopped silently no-ops) was surfaced in a prior review thread and remains open for a follow-up. No files require special attention beyond what is already tracked in the open prior review thread about FleetSupervisor and FleetTaskState. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Q[queued] -->|dispatched| P[provisioning]
Q -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA[cancelled]
P -->|provisioned| L[launching]
P -->|provisionFailed| F[failed]
P -->|userCancel / workspaceClosed| CA
L -->|agentSessionStarted| R[running]
L -->|pidExited / stallTimeout / agentStopped| RB[retryBackoff]
L -->|agentStopped + pr:open| AW[awaitingReview]
L -->|agentStopped + pr:terminal| D[done]
L -->|maxAttempts reached| F
L -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA
R -->|blockingItemReceived| NI[needsInput]
R -->|agentStopped + pr:open| AW
R -->|agentStopped + pr:terminal| D
R -->|pidExited / promptIdleObserved / stallTimeout / agentStopped| RB
R -->|maxAttempts reached| F
R -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA
NI -->|blockingItemResolved| R
NI -->|pidExited / stallTimeout| RB
NI -->|maxAttempts reached| F
NI -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA
RB -->|backoffElapsed| L
RB -->|prChanged:open| AW
RB -->|prChanged:terminal| D
RB -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA
AW -->|prChanged:terminal / sourceReachedTerminalState| D
AW -->|userRetry| Q
AW -->|userCancel| CA
F -->|userRetry| Q
F -->|prChanged:open| AW
F -->|prChanged:terminal| D
CA -->|userRetry| Q
D:::terminal
F:::terminal
CA:::terminal
classDef terminal fill:#f0f0f0,stroke:#999
%%{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
Q[queued] -->|dispatched| P[provisioning]
Q -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA[cancelled]
P -->|provisioned| L[launching]
P -->|provisionFailed| F[failed]
P -->|userCancel / workspaceClosed| CA
L -->|agentSessionStarted| R[running]
L -->|pidExited / stallTimeout / agentStopped| RB[retryBackoff]
L -->|agentStopped + pr:open| AW[awaitingReview]
L -->|agentStopped + pr:terminal| D[done]
L -->|maxAttempts reached| F
L -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA
R -->|blockingItemReceived| NI[needsInput]
R -->|agentStopped + pr:open| AW
R -->|agentStopped + pr:terminal| D
R -->|pidExited / promptIdleObserved / stallTimeout / agentStopped| RB
R -->|maxAttempts reached| F
R -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA
NI -->|blockingItemResolved| R
NI -->|pidExited / stallTimeout| RB
NI -->|maxAttempts reached| F
NI -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA
RB -->|backoffElapsed| L
RB -->|prChanged:open| AW
RB -->|prChanged:terminal| D
RB -->|userCancel / workspaceClosed / sourceReachedTerminalState| CA
AW -->|prChanged:terminal / sourceReachedTerminalState| D
AW -->|userRetry| Q
AW -->|userCancel| CA
F -->|userRetry| Q
F -->|prChanged:open| AW
F -->|prChanged:terminal| D
CA -->|userRetry| Q
D:::terminal
F:::terminal
CA:::terminal
classDef terminal fill:#f0f0f0,stroke:#999
Reviews (5): Last reviewed commit: "Fix CmuxFleet PR handoff races and path ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+68
to
+69
| case .queued: | ||
| to == .cancelled |
Contributor
There was a problem hiding this comment.
queued → provisioning edge in the state machine. FleetScheduler.tasksToDispatch returns queued tasks specifically so the engine can start provisioning them, but canTransition(from: .queued, to: .provisioning) returns false. Because FleetSupervisor.transition calls canTransition as a hard guard, any engine code that routes the queued → provisioning step through the pure supervisor will silently drop the transition and leave the task stuck in .queued forever. The engine must either bypass the guard for this leg or the table must include the edge.
Suggested change
| case .queued: | |
| to == .cancelled | |
| case .queued: | |
| to == .provisioning || to == .cancelled |
Contributor
Author
There was a problem hiding this comment.
Fixed in 7ea2797: added a dispatched(taskID:at:) signal that the reducer handles (queued -> provisioning, emitting provisionWorkspace) and canTransition now permits queued -> provisioning.
— Claude Code
| import Foundation | ||
|
|
||
| /// Reduces one Fleet task snapshot with one deterministic supervision signal. | ||
| public struct FleetSupervisor: Sendable { |
Contributor
There was a problem hiding this comment.
FleetSupervisor, FleetBackoff, FleetScheduler, and FleetPathSanitizer are all public structs whose entire API is static funcs — the cmux no-ambient-global-state rule explicitly flags "a type whose API is mostly static funcs." The canonical fix for a pure reducer is to make configuration a stored property so the type is constructable and injectable: callers build a FleetSupervisor(config:) once and call supervisor.reduce(task:signal:), avoiding both the static-only namespace shape and the repeated config parameter at every call site. The same pattern applies to FleetBackoff, FleetScheduler (taking maxConcurrentAgents/provisioningCap), and FleetPathSanitizer (taking maxLength).
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!
Contributor
Author
There was a problem hiding this comment.
Fixed in 7ea2797: FleetSupervisor, FleetScheduler, FleetBackoff, and FleetPathSanitizer are now instantiable values with constructor-injected configuration (zero static funcs remain in the package).
— Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
Packages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetTaskStateTests.swift (1)
23-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTest enshrines the same
.stalled-unreachable gap as the implementation.
transitionTableMatchesExpectedEdgeshardcodes anexpectedtable where no state transitions into.stalled, mirroring the gap flagged inFleetTaskState.canTransition. Once that's fixed, this table needs the corresponding entries added or the test will start failing.🤖 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 `@Packages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetTaskStateTests.swift` around lines 23 - 43, Update the transition expectations in FleetTaskStateTests.transitionTableMatchesExpectedEdges so they include the newly supported transitions into .stalled, since the hardcoded expected table currently mirrors the same gap as FleetTaskState.canTransition. Locate the test by the transitionTableMatchesExpectedEdges method and adjust the expected dictionary entries to reflect the corrected state machine, especially any from-state values that should now allow .stalled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/macOS/CmuxFleet/Sources/CmuxFleet/Engine/FleetBackoff.swift`:
- Line 2: FleetBackoff is being used as a static-only namespace, which triggers
the namespace-type lint. Move delayMS onto FleetSupervisionConfig via an
extension, or change FleetBackoff so it can be instantiated and expose delayMS
as an instance method instead of keeping it as an all-static public type.
In `@Packages/macOS/CmuxFleet/Sources/CmuxFleet/Engine/FleetScheduler.swift`:
- Around line 2-13: `FleetScheduler` is being used as a namespace-only static
type, which violates the package lint rule. Make `tasksToDispatch` an
instance-based API on a constructable type (or move the behavior to an extension
on `FleetTask`/`[FleetTask]`) so `FleetScheduler` is no longer an all-static
struct. Update the call sites in `FleetSchedulerTests` and any other users from
`FleetScheduler.tasksToDispatch(...)` to the new instance/extension entry point,
keeping the same task-sorting and cap behavior.
In `@Packages/macOS/CmuxFleet/Sources/CmuxFleet/Engine/FleetSupervisor.swift`:
- Line 4: `FleetSupervisor` is being used as a namespace-only static type and
triggers the lint failure; move the reducer behavior onto the natural receiver
`FleetTask` instead. Replace `FleetSupervisor.reduce(task:signal:config:)` with
an instance-style `FleetTask.reduced(with:config:)` (or equivalent extension
method), and relocate the helper logic from the `private static` helpers
`acceptsAgentStop`, `startAttempt`, `retryOrFail`, and `transition` into the
same `FleetTask` extension as private helpers. Update call sites like
`FleetSupervisorTests` to invoke the method on the task instance rather than on
`FleetSupervisor`.
In
`@Packages/macOS/CmuxFleet/Sources/CmuxFleet/Provision/FleetPathSanitizer.swift`:
- Line 2: The public FleetPathSanitizer type is namespace-only and triggers
package-conventions-lint because it has only static behavior with no
instantiable surface. Refactor FleetPathSanitizer into an extension on String
and move the public API from FleetPathSanitizer.directoryName(for:) to an
instance-style String method such as fleetSanitizedDirectoryName(), updating all
call sites accordingly. Keep the existing helper logic in the same file as
private static or fileprivate helpers, including isAllowed, trimmed, and
shouldTrim, so the behavior stays unchanged while satisfying the lint.
---
Duplicate comments:
In `@Packages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetTaskStateTests.swift`:
- Around line 23-43: Update the transition expectations in
FleetTaskStateTests.transitionTableMatchesExpectedEdges so they include the
newly supported transitions into .stalled, since the hardcoded expected table
currently mirrors the same gap as FleetTaskState.canTransition. Locate the test
by the transitionTableMatchesExpectedEdges method and adjust the expected
dictionary entries to reflect the corrected state machine, especially any
from-state values that should now allow .stalled.
🪄 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: 34951315-a47f-4ec0-82dd-d09424d8da33
📒 Files selected for processing (25)
.github/workflows/ci.ymlPackages/macOS/CmuxFleet/Package.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Engine/FleetBackoff.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Engine/FleetScheduler.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Engine/FleetSupervisionConfig.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Engine/FleetSupervisor.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetCommand.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetIdentifiers.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetNotificationKind.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetPullRequestState.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetPullRequestStatus.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetRun.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetRunEndReason.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetSignal.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetTask.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetTaskSourceKind.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Model/FleetTaskState.swiftPackages/macOS/CmuxFleet/Sources/CmuxFleet/Provision/FleetPathSanitizer.swiftPackages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetBackoffTests.swiftPackages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetPathSanitizerTests.swiftPackages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetSchedulerTests.swiftPackages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetSupervisorTests.swiftPackages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetTaskStateTests.swiftPackages/macOS/CmuxFleet/Tests/CmuxFleetTests/FleetTestSupport.swiftcmux.xcworkspace/contents.xcworkspacedata
Comment on lines
+88
to
+95
| case let .agentStopped(taskID, at): | ||
| guard taskID == task.id, acceptsAgentStop(from: task.state) else { | ||
| return (task, []) | ||
| } | ||
| if task.pr != nil { | ||
| return transition(task: task, to: .awaitingReview, at: at) | ||
| } | ||
| return retryOrFail(task: task, at: at, killAgent: false) |
Contributor
There was a problem hiding this comment.
.stalled → .awaitingReview transition missing, causing agentStopped to silently no-op
acceptsAgentStop explicitly accepts .stalled (line 178), so the agentStopped handler lets stalled tasks through its guard. When task.pr != nil, the handler calls transition(task:, to: .awaitingReview, at:). However, canTransition(from: .stalled, to: .awaitingReview) returns false — only .retryBackoff, .failed, and .cancelled are permitted from .stalled. transition therefore returns (task, []), dropping the signal entirely. A stalled task with an attached PR will stay stuck in .stalled indefinitely with no recovery path.
The fix is either to add || to == .awaitingReview to the .stalled arm of canTransition, or to fall through to retryOrFail when task.pr != nil but the target transition would be .awaitingReview and the source state doesn't permit it.
lawrencecchen
added
the
stale-revisit
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
3 participants
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.
Scope
Part 1 of #7361.
Adds a new pure SwiftPM package at
Packages/macOS/CmuxFleetwith deterministic, dependency-free Fleet model and reducer logic only. This PR includes typed identifiers, task/run snapshots, signals, commands, supervisor reducer, scheduler, retry backoff, and path sanitizer.No app wiring, control-socket domain, IO, timers, UI, or localization changes are included.
Tests
cd Packages/macOS/CmuxFleet && swift testpython3 scripts/swift_file_length_budget.pypython3 scripts/check-workspace-package-groups.py --checkpython3 scripts/check-package-resolved-policy.pyNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds
CmuxFleet, a deterministic Fleet engine core (model, supervisor, scheduler, backoff) with tests. Scopes run signals by attempt and preps the engine skeleton for #7361; no app wiring or I/O.New Features
Packages/macOS/CmuxFleetwith typed IDs and task/run/PR types.FleetSupervisorreducer and deterministicFleetSchedulerwith concurrency and provisioning caps.FleetBackoffandFleetPathSanitizerutilities; directory names use a stable hash suffix to avoid collisions; CI builds/testsCmuxFleet.Bug Fixes
Written for commit 6d10dd7. Summary will update on new commits.
Summary by CodeRabbit