Add shareable orchestration templates (cmux orchestration): package format, store, trust-gated runs - #8032
Add shareable orchestration templates (cmux orchestration): package format, store, trust-gated runs#8032austinywang wants to merge 13 commits into
Conversation
…n planner Pure SwiftPM package for shareable orchestration templates: versioned orchestration.json manifest (parameters, substrate, agents, linear steps), template directory validation with error/warning findings, install store under ~/.cmuxterm/orchestrations (install/list/info/remove/update over seam protocols with in-memory fakes), placeholder rendering, run planning (worktree, clone-pool, script substrates), and an init scaffold. Related: #7361 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New ControlOrchestrationContext seam + coordinator handler in CmuxControlSocket (orchestration.list/info/plan/run) with fakes and coordinator tests. App-side: TerminalController conformance plans runs via CmuxOrchestration and enforces the trust gate (needs_confirmation until the user confirms scripts/agent commands/substrate); OrchestrationRunController provisions workspaces off-main (worktree/clone/script) and creates grouped, unfocused workspaces with the agent command as typed terminal input. Related: #7361 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Store verbs (init/validate/install/list/info/remove/update/configure) run locally against ~/.cmuxterm/orchestrations with an interactive parameter interview, so they work without a running app; plan/run go through the v2 orchestration.* socket methods. run shows the plan (workspaces, agent commands, scripts, substrate) and enforces the first-run trust confirmation; --yes skips the prompt. Contract doc gains the namespace row and a no-socket help probe. Related: #7361 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The example is the format's living test fixture: CmuxOrchestration package tests validate it and plan a worktree run from it on every CI run. Related: #7361 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CLI/cmux.swift is over the 900-line hard cap and may not grow; moving parseOption/parseRepeatedOption/optionValue/hasFlag verbatim offsets the orchestration namespace's dispatch/help additions (net -60 lines). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR adds the ChangesOrchestration platform
CLI and supporting integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ControlCommandCoordinator
participant TerminalController
participant OrchestrationRunPlanner
participant OrchestrationRunController
participant TabManager
CLI->>ControlCommandCoordinator: Send orchestration.plan or orchestration.run
ControlCommandCoordinator->>TerminalController: Dispatch orchestration request
TerminalController->>OrchestrationRunPlanner: Build and validate run plan
TerminalController->>OrchestrationRunController: Start confirmed plan
OrchestrationRunController->>TabManager: Provision and attach workspaces
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 21❌ Failed checks (1 warning, 20 inconclusive)
✅ Passed checks (4 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 |
Greptile SummaryThis PR adds shareable orchestration templates and trust-gated fleet execution. The main changes are:
Confidence Score: 5/5No additional blocking issue qualifies for this review round.
Important Files Changed
Reviews (7): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| params["confirm_trust"] = true | ||
| let runPayload = try client.sendV2(method: "orchestration.run", params: params) |
There was a problem hiding this comment.
The user confirms the earlier orchestration.plan response, but orchestration.run replans by name and receives only confirm_trust = true. If the template is updated between those calls, the newly installed scripts and commands can be marked trusted and executed even though they were not shown. Bind confirmation to the displayed template commit or plan ID and reject a revision mismatch.
There was a problem hiding this comment.
Fixed in dd75e89: orchestration.plan now returns a trust_fingerprint (SHA-256 over substrate kind, script paths, agent command templates, and template version). The CLI echoes it back as confirm_fingerprint, and the app rejects a first-run confirmation whose fingerprint doesn't match the freshly replanned trust material, so a template that changed between review and confirmation can no longer be silently approved.
| } | ||
| if !fileSystem.fileExists(atPath: join(templateDirectory, reference.path)) { | ||
| findings.append(.init( | ||
| severity: .error, | ||
| code: "missing-file", | ||
| message: "\(reference.role) file '\(reference.path)' does not exist in the template", | ||
| path: reference.path | ||
| )) |
There was a problem hiding this comment.
The path check rejects lexical traversal, but fileExists follows symlinks without proving that the resolved file stays below templateDirectory. A template can reference a relative prompt or provision script whose symlink resolves outside the package; planning then reads that local file into an agent prompt, or provisioning executes a different file than the reviewed package path. Canonicalize each referenced path and reject any result outside the canonical template root.
There was a problem hiding this comment.
Fixed in dd75e89: validation now walks the template and errors on any symbolic link (symlink finding), so a symlinked prompt/script can't resolve outside the template root. Install/update validate before anything reaches the store, so symlinked templates are rejected outright. Covered by rejectsSymlinksAnywhereInTemplate.
| } | ||
|
|
||
| let promptFile = Self.promptFileRelativePath | ||
| values["prompt"] = OrchestrationPlaceholders.shellQuoted(renderedPrompt) |
There was a problem hiding this comment.
The rendered prompt is shell-quoted, but prompt_file is inserted as a raw path. When the installation or workspace root contains spaces or shell metacharacters, an agent command such as agent --prompt-file {{prompt_file}} receives split or altered arguments when typed into the terminal. Provide a shell-safe value for this command placeholder.
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!
There was a problem hiding this comment.
Fixed in dd75e89: {{prompt_file}} is now shell-quoted the same way as {{prompt}}. (Today the value is a constant relative path with no metacharacters, but quoting keeps agent commands safe if the location ever changes.)
| C617000000000000000000A3 /* CmuxGit in Frameworks */, | ||
| 799F1BEF876006EEB8CD1035 /* CMUXMobileCore in Frameworks */, | ||
| E3B7A30000000000000000D3 /* CmuxNotifications in Frameworks */, | ||
| C0FFEE0000000000000000A3 /* CmuxOrchestration in Frameworks */, |
There was a problem hiding this comment.
The app target compiles new sources that import CmuxOrchestration and links its product in the Frameworks phase, but its packageProductDependencies list does not include that product while the CLI target does. On a clean app-target resolution this can leave the module undeclared and fail compilation with No such module CmuxOrchestration. Add the product dependency to the main cmux target.
Rule Used: Flag SwiftPM package, Xcode project, .gitignore, w... (source)
There was a problem hiding this comment.
Fixed in dd75e89: the app target's packageProductDependencies now lists CmuxOrchestration explicitly (the Frameworks-phase-only shape mirrored CmuxControlSocket's existing wiring, but the explicit entry is more robust on clean resolution).
package-conventions-lint forbids all-static public namespace types: manifest parsing moves onto OrchestrationManifest (parse/ParseOutput), parameter override coercion becomes manifest.coerceParameterOverrides, OrchestrationWellKnownParameter gains real String-raw cases, and OrchestrationPlaceholders becomes an instantiable value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ile, app product dependency
- orchestration.plan returns a trust_fingerprint (SHA-256 of substrate,
scripts, agent commands, template version); a first-run confirmation must
echo it back, so a template that changes between review and confirmation
is rejected (TOCTOU).
- Validation rejects symbolic links anywhere in a template; a symlinked
prompt or script could resolve outside the template root and leak its
target into rendered prompts.
- {{prompt_file}} is now shell-quoted like {{prompt}}.
- The app target's packageProductDependencies now lists CmuxOrchestration
explicitly instead of relying on the Frameworks phase alone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
| private func symlinkPaths(under root: String, prefix: String = "", depth: Int = 0) -> [String] { | ||
| guard depth < 6, let entries = try? fileSystem.contentsOfDirectory(atPath: root) else { return [] } | ||
| var results: [String] = [] | ||
| for entry in entries.sorted() { | ||
| if entry == ".git" { continue } | ||
| let absolute = join(root, entry) | ||
| let relative = prefix.isEmpty ? entry : "\(prefix)/\(entry)" | ||
| if fileSystem.isSymbolicLink(atPath: absolute) { | ||
| results.append(relative) | ||
| } else if fileSystem.directoryExists(atPath: absolute) { | ||
| results.append(contentsOf: symlinkPaths(under: absolute, prefix: relative, depth: depth + 1)) | ||
| } | ||
| } | ||
| return results | ||
| } |
There was a problem hiding this comment.
The symlink scan stops after six directory levels. A manifest can reference a valid relative prompt or provision script below that depth, such as a/b/c/d/e/f/g/run.sh, but a symlink there is never checked. Planning and provisioning then follow the symlink, so an external file can still be read or executed. Remove the fixed depth limit so every referenced template path remains inside the template boundary.
| private func symlinkPaths(under root: String, prefix: String = "", depth: Int = 0) -> [String] { | |
| guard depth < 6, let entries = try? fileSystem.contentsOfDirectory(atPath: root) else { return [] } | |
| var results: [String] = [] | |
| for entry in entries.sorted() { | |
| if entry == ".git" { continue } | |
| let absolute = join(root, entry) | |
| let relative = prefix.isEmpty ? entry : "\(prefix)/\(entry)" | |
| if fileSystem.isSymbolicLink(atPath: absolute) { | |
| results.append(relative) | |
| } else if fileSystem.directoryExists(atPath: absolute) { | |
| results.append(contentsOf: symlinkPaths(under: absolute, prefix: relative, depth: depth + 1)) | |
| } | |
| } | |
| return results | |
| } | |
| private func symlinkPaths(under root: String, prefix: String = "") -> [String] { | |
| guard let entries = try? fileSystem.contentsOfDirectory(atPath: root) else { return [] } | |
| var results: [String] = [] | |
| for entry in entries.sorted() { | |
| if entry == ".git" { continue } | |
| let absolute = join(root, entry) | |
| let relative = prefix.isEmpty ? entry : "\(prefix)/\(entry)" | |
| if fileSystem.isSymbolicLink(atPath: absolute) { | |
| results.append(relative) | |
| } else if fileSystem.directoryExists(atPath: absolute) { | |
| results.append(contentsOf: symlinkPaths(under: absolute, prefix: relative)) | |
| } | |
| } | |
| return results | |
| } |
There was a problem hiding this comment.
Fixed in 49873fa: the symlink walk (and the secret sweep) no longer cap at six levels. A symlinked directory is flagged before descent, so recursion stays bounded by the template's real directory depth with no cycle risk.
There was a problem hiding this comment.
Fixed in 49873fa/52cfccdb18: the symlink and secret scans no longer cap at six directory levels. A symlinked directory is flagged before descent (never recursed into), so recursion is bounded by the template's real directory depth and arbitrarily deep symlinks are caught.
…shold The symlink check pushed OrchestrationValidator.swift to 506 lines; the whole-tree scans (symlink rejection, secret sweep) move verbatim to OrchestrationValidatorScans.swift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| self.workspaceRoot = workspaceRoot | ||
| self.templateVersion = templateVersion | ||
| } | ||
|
|
||
| /// Stable digest of the trust-relevant material (substrate, scripts, | ||
| /// agent commands, template version). A client that showed the user a |
There was a problem hiding this comment.
The fingerprint covers script paths and raw agent command templates, but not prompt or layout contents. If an update changes a prompt without changing the manifest version or command template, the fingerprint from the displayed plan still matches the replanned run. The rendered terminal command can then contain instructions the user did not review. Include content digests for every template file that affects provisioning or terminal input, including prompts and layouts.
There was a problem hiding this comment.
Fixed in 49873fa: the fingerprint (now v2) includes a contentDigest over the prompt template, layout JSON, and substrate script bytes, so a content-only edit between plan and confirmation invalidates the pending confirmation even when paths, commands, and version are unchanged. Covered by trustFingerprintTracksPromptContent.
There was a problem hiding this comment.
Fixed in 49873fa/52cfccdb18: the trust fingerprint now includes a content digest over the selected prompt template, the layout JSON, and every substrate script's bytes. A content-only edit (same paths, commands, and version) between plan and confirmation now changes the fingerprint and the run is rejected with a re-plan message. Covered by the fingerprint test's prompt-edit assertion.
…rprint - The template walks (symlink rejection, secret sweep) no longer cap at six directory levels; symlinked directories are flagged before descent, so recursion is bounded by the template's real depth. - The trust fingerprint now includes a digest of the executable template contents (prompt, layout, substrate script bytes), so a content-only edit between plan and confirmation invalidates the pending confirmation even when paths, commands, and version are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
…print - The symlink and secret scans no longer stop at six directory levels; a symlinked directory is flagged before descent, so recursion is bounded by the template's real depth and deep symlinks can't slip past. - The trust fingerprint now includes a digest of the executable template contents (selected prompt, layout JSON, substrate script bytes), so a content-only edit between plan and confirmation is rejected even when paths, agent commands, and version are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 39
🤖 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 15806-15807: Update the generic usage() help text’s top-level
Commands list to include orchestration, matching the existing
Self.orchestrationHelpText() dispatch and its placement among related commands.
Ensure plain cmux --help and cmux help expose the new command without changing
the orchestration subcommand help.
- Around line 3255-3261: Add "orchestration" to Self.topLevelCommandNames in
CommandSuggestions so shouldOpenAsPathArgument() recognizes it as a top-level
command even when a matching filesystem path exists. Preserve the existing local
orchestration routing in runOrchestrationLocalNamespace and command-first
dispatch behavior.
In `@CLI/CMUXCLI`+OptionParsing.swift:
- Around line 74-76: Update hasFlag to inspect only arguments before the
standalone "--" terminator, so flags appearing afterward are treated as
forwarded arguments rather than cmux options. Preserve existing exact flag
matching for pre-terminator arguments and return false when the requested flag
occurs only after "--".
In `@CLI/CMUXCLI`+Orchestration.swift:
- Around line 348-358: Update the parameter persistence and reload flow in
configure around store.setResolvedParameters and store.installed(named:) to
propagate failures as CLIError instead of discarding them or returning silently.
Preserve the existing success path and ensure “parameters saved” is only
reported after both required store operations succeed.
In `@CLI/CMUXCLI`+OrchestrationRun.swift:
- Around line 89-104: Update the orchestration run flow around
effectiveJSONOutput so non-dry JSON runs emit only the machine-readable JSON
payload on stdout. Suppress printOrchestrationPlan and interactive
trust-confirmation output in JSON mode, and require --yes when trust is not
already confirmed; users must review the plan separately via plan --json.
In `@cmux.xcodeproj/project.pbxproj`:
- Line 5714: Commit or update the Xcode SwiftPM lockfile at the workspace’s
shared SwiftPM location to include the resolved CmuxOrchestration package
dependency introduced by the XCLocalSwiftPackageReference entry. Ensure
Package.resolved records the complete, reproducible dependency resolution for
standalone Xcode builds.
In `@docs/orchestrations.md`:
- Line 34: Update the fenced code blocks in docs/orchestrations.md at lines
34-34 and 179-179 with the text language identifier, and at line 148-148 with
the bash language identifier; no other content changes are needed.
In
`@Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Orchestration/ControlCommandCoordinator`+Orchestration.swift:
- Around line 135-138: Update the string-task handling in the orchestration
command coordinator to return an invalid_params error when the trimmed title is
empty, rather than continuing and silently omitting that task. Preserve trimming
and task creation for nonblank strings, and ensure mixed inputs fail the entire
request consistently with empty object titles.
In
`@Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Orchestration/ControlOrchestrationContext.swift`:
- Line 2: Mark the transport DTOs ControlOrchestrationSummary,
ControlOrchestrationTaskInput, and ControlOrchestrationRunInputs as nonisolated
while preserving their immutable Sendable value-model definitions.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationManifest.swift`:
- Around line 179-221: Replace hard-coded user-facing diagnostics with
structured, localizable errors: in
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationManifest.swift
lines 179-221, update the manifest parsing and describe logic to return
structured parse errors without raw debugDescription; in
OrchestrationParameter.swift lines 132-152, use stable reason codes and
arguments; in OrchestrationStep.swift lines 52-107 and
OrchestrationSubstrate.swift lines 52-57, structure invalid-policy and
invalid-kind diagnostics; in OrchestrationValidator.swift lines 60-405 and
OrchestrationValidatorScans.swift lines 68-72, localize findings before
presentation; add complete matching keys and translations for every locale in
Resources/Localizable.xcstrings.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationStep.swift`:
- Around line 100-116: Update OrchestrationStep.init(from:) in the .retry
decoding branch to validate the decoded attempts value as non-negative before
constructing retry(attempts:). Preserve the default value of 1 when attempts is
absent, and throw a suitable DecodingError for negative values.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationVersion.swift`:
- Around line 10-18: Update OrchestrationVersion’s major, minor, and patch
storage and initializer validation so every component is non-negative and cannot
be mutated into an invalid value after initialization. Preserve the existing
default values and ensure invalid inputs are rejected using the project’s
established invariant-enforcement approach.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationParameterResolution.swift`:
- Around line 11-14: Localize all user-facing orchestration errors using the
project’s localized APIs and add matching complete catalog entries: update the
unknown-parameter reason in
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationParameterResolution.swift
lines 11-14, the unresolved-placeholder error in
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Template/OrchestrationPlaceholders.swift
lines 65-67, and the invalid-name and existing-file errors in
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Scaffold/OrchestrationScaffold.swift
lines 29-37. Preserve each error’s existing meaning while exposing localized
text or structured reasons for boundary localization.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationProvisioner.swift`:
- Around line 76-84: Sanitize provisioning failures at the
OrchestrationProvisioner boundary by replacing raw launch errors, exit codes,
and stderr with safe product-level messages while retaining redacted details
only for internal diagnostics. Update
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationProvisioner.swift
lines 76-84 accordingly; update
Packages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationProvisionerTests.swift
lines 105-121 to assert the sanitized public error instead of raw stderr.
- Around line 41-43: The orchestration-facing messages are hard-coded and must
use localization. In OrchestrationProvisioner.swift lines 41-43, localize every
provisioning error with stable catalog keys; in OrchestrationRunPlanner.swift
lines 82-94, localize every planning error and note, including interpolated
values, and add matching translated catalog entries for all new keys.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationRunPlan.swift`:
- Around line 184-195: Use one canonical, unambiguous encoding for all
trust-hash inputs. In OrchestrationRunPlan.fingerprint, include workspaceRoot
and replace delimiter-only concatenation with explicit-length encoding or
canonical Codable serialization for every field and array element. In
OrchestrationRunPlanner’s hashing flow, apply the same encoding to prompt,
layout, and script path/content records; keep both sites consistent so distinct
inputs cannot produce the same material.
- Around line 173-177: Update sha256Hex to avoid per-byte String(format:)
conversion: use a cached hexadecimal lookup table and a preallocated UTF-8
buffer, writing two hex characters for each digest byte before constructing the
final String.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationRunPlanner.swift`:
- Around line 96-101: Update the agent resolution flow in
OrchestrationRunPlanner to consider every authoritative source: explicit
request.agentID, the resolved agent parameter, the first step’s agent, then
manifest.prompt’s default agent. Apply one consistent precedence path when
selecting the agent used for execution, preserving the existing first-step
behavior, and add coverage using distinct agents to verify the precedence.
- Around line 129-142: Update parameterValues construction in the orchestration
run planner to overwrite the well-known repoRoot and workspaceRoot entries with
the resolved absolute repoRoot and workspaceRoot values after copying the raw
parameters. Preserve all other parameter values unchanged so prompts and
commands receive the same paths used for provisioning.
- Around line 122-127: Update the concurrency handling in the orchestration run
planner so zero or negative values are rejected rather than bypassing the task
cap. Validate the .int concurrency parameter before planning tasks, using the
existing error-reporting mechanism, while preserving the current prefix limiting
behavior for positive values.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Scaffold/OrchestrationScaffold.swift`:
- Around line 33-46: Update the scaffold generation flow around
Self.files(name:) to first build the complete list of destination paths and
verify that none already exists, including orchestration.json, README.md,
WORKFLOW.md, and nested prompt files. Throw OrchestrationManifestError before
creating directories or writing any files when a destination conflicts, then
reuse the validated plan for directory creation and writes.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Seams/OrchestrationGitClient.swift`:
- Around line 8-12: Update DefaultOrchestrationGitClient.runGit so stdout and
stderr are consumed concurrently rather than sequentially, preventing either
pipe from blocking when its buffer fills. Preserve the existing command
execution, exit-status handling, and captured output behavior.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Seams/OrchestrationProcessRunner.swift`:
- Around line 21-27: Update OrchestrationProcessRunner.run and its
implementations so stdout and stderr are drained concurrently rather than read
sequentially, preventing blocked child processes when either pipe fills. Avoid
unbounded in-memory buffering by applying an explicit output cap or streaming
output while preserving the returned OrchestrationProcessResult contract.
- Around line 21-67: Update DefaultOrchestrationProcessRunner.run and the
process execution wrapper in
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Seams/OrchestrationProcessRunner.swift:21-67
and
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Seams/OrchestrationGitClient.swift:8-58
to use one shared helper that drains stdout and stderr concurrently, preventing
either pipe from blocking the child; cap the amount retained for each stream
instead of buffering all output in memory, while preserving the existing
OrchestrationProcessResult values and process execution behavior.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Store/OrchestrationInstallRecord.swift`:
- Around line 66-80: Update OrchestrationInstallSource.detect to resolve an
existing filesystem path before applying any Git classification, so local paths
such as /tmp/template.git remain local sources. Remove the .git suffix
heuristic; for ambiguous non-existing inputs, fail closed or require an explicit
source kind instead of inferring transport from strings.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Store/OrchestrationStore.swift`:
- Around line 128-146: Update the installation flow around installPath,
templateDirectory(for:), and writeRecord so replacements are assembled
completely in a sibling staging directory, including the template copy and
installation record, before changing the authoritative installation. Atomically
rename or swap the completed staging directory into installPath only after all
preparation succeeds; preserve the existing installation on any copy or
record-write failure, and apply the same behavior to the update path around
lines 202–217.
- Around line 3-24: Remove CLI-ready user copy from
OrchestrationStoreError.description and keep its cases as structured errors with
sanitized internal diagnostic details only; do not interpolate raw source URLs
or upstream errors. Update the CLI boundary handling for
OrchestrationStoreError, including the affected paths around install/source
handling, to map each case to localized product-facing messages using safe
values such as orchestration names, while preserving detailed diagnostics
separately for internal logging.
- Around line 251-258: Validate the decoded record’s schemaVersion in the
install-record loading flow before returning it, comparing it with
currentSchemaVersion. If the versions differ or are unsupported, throw
OrchestrationStoreError.corruptInstall with the install name and relevant
detail, preserving the existing corrupt-record handling and failing closed until
migration support exists.
- Around line 40-46: The OrchestrationStore mutation methods install, update,
remove, setResolvedParameters, and confirmTrust need coordinated per-install
serialization and atomic install.json replacement. Add a per-install locking
mechanism shared by these methods, hold it across each read/modify/rewrite or
directory-removal operation, and write updated records through a temporary file
followed by an atomic replacement; preserve existing behavior outside the
protected mutation scope.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Validation/OrchestrationValidator.swift`:
- Around line 68-82: In the orchestration validation flow, run
validateNoSymlinks immediately after confirming the template directory and
before constructing or reading the manifest in OrchestrationValidator. If
symlink findings are present, return the validation report immediately so no
manifest, prompt, or script files are read; preserve the existing
missing-directory and missing-manifest handling for non-symlink cases.
- Around line 300-307: Update the validation logic around the referenced-file
checks in OrchestrationValidator to require each referenced path to be a regular
file, rejecting directories instead of relying solely on fileExists. For
referenced scripts, validate executable permissions and report non-executable
files with error severity rather than warning, including the affected path and
existing diagnostic context; apply the same requirements to the additional check
around the separately noted range.
In
`@Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Validation/OrchestrationValidatorScans.swift`:
- Around line 62-66: Update the file iteration in OrchestrationValidatorScans so
oversized template files are not fully loaded before validation and cannot
bypass secret scanning. Preflight each file’s size before reading, then
stream-scan files within the allowed limit (or explicitly reject files over the
template limit), preserving UTF-8 text scanning for accepted files.
In
`@Packages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationTestSupport.swift`:
- Around line 119-157: Update removeItem and copyItem to keep symlinkPaths
consistent: remove the target and all descendants from symlinkPaths when
deleting, and propagate matching symlinkPaths entries to destination paths when
copying both individual files and directories, alongside executable metadata.
In `@Resources/Localizable.xcstrings`:
- Around line 234182-234198: Update the orchestration.run.readyBody localization
entry to use count-aware singular and plural variants for the workspace count,
replacing the literal “workspace(s)” wording. Add equivalent grammatical
variants for each existing locale while preserving the current message meaning
and interpolation placeholder.
In `@Sources/OrchestrationRunController.swift`:
- Around line 138-145: Update the failure notification in the orchestration run
handling flow around store.addNotification so its body uses the localized string
API with the sanitized failures list as interpolation data. Ensure the
user-facing message does not expose implementation details, while preserving the
existing failure content and notification behavior.
- Around line 129-136: Update the localized body in the orchestration run-ready
notification to use ICU-style pluralization with explicit one and other variants
keyed from plan.workspaces.count, replacing the String(format:) "%1$d"
interpolation. Preserve the existing workspace count and message meaning while
allowing locale-correct plural translations.
- Around line 43-46: Update the catch block in OrchestrationRunController to log
the raw error through internal telemetry, then append only workspacePlan.title
to failures. Remove String(describing: error) from the user-facing failure text
so callers can present a generic error without exposing provisioner details.
In `@Sources/TerminalController`+ControlOrchestrationContext.swift:
- Around line 23-24: Replace the raw String(describing: error) values returned
by the catch blocks in TerminalController control orchestration with stable,
product-facing failure messages. Apply the same mapping at the additional catch
sites noted in the review, while recording detailed errors only through
privately redacted logging; do not expose upstream messages or internal
implementation details via the socket API.
- Around line 5-10: Update controlOrchestrationList, controlOrchestrationInfo,
orchestrationPlan, and controlOrchestrationRun so install listing, record
parsing, and template scanning execute off `@MainActor`, while retaining
main-actor execution only for UI/socket coordination and run actuation. Replace
String(describing:) in each failed path with concise sanitized user-facing
messages that do not expose raw errors or filesystem details.
🪄 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: 27596f41-f083-4720-a2dc-4507820e0f95
⛔ Files ignored due to path filters (1)
cmux.xcworkspace/contents.xcworkspacedatais excluded by!**/*.xcworkspace/contents.xcworkspacedata
📒 Files selected for processing (51)
.github/workflows/ci.ymlCLI/CMUXCLI+OptionParsing.swiftCLI/CMUXCLI+Orchestration.swiftCLI/CMUXCLI+OrchestrationRun.swiftCLI/cmux.swiftExamples/Orchestrations/issue-fleet/README.mdExamples/Orchestrations/issue-fleet/WORKFLOW.mdExamples/Orchestrations/issue-fleet/instructions/AGENTS.mdExamples/Orchestrations/issue-fleet/orchestration.jsonExamples/Orchestrations/issue-fleet/prompts/review.mdExamples/Orchestrations/issue-fleet/prompts/task.mdPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/ControlCommandContext.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/ControlCommandCoordinator.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Orchestration/ControlCommandCoordinator+Orchestration.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Orchestration/ControlOrchestrationContext.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorOrchestrationTests.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlOrchestrationContextTestStubs.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/FakeOrchestrationControlCommandContext.swiftPackages/macOS/CmuxOrchestration/Package.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationManifest.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationParameter.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationStep.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationSubstrate.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Model/OrchestrationVersion.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationParameterResolution.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationProvisioner.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationRunPlan.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationRunPlanner.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Scaffold/OrchestrationScaffold.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Seams/OrchestrationFileSystem.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Seams/OrchestrationGitClient.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Seams/OrchestrationProcessRunner.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Store/OrchestrationInstallRecord.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Store/OrchestrationStore.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Template/OrchestrationPlaceholders.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Validation/OrchestrationValidator.swiftPackages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Validation/OrchestrationValidatorScans.swiftPackages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/ExampleTemplateTests.swiftPackages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationManifestTests.swiftPackages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationPlaceholdersTests.swiftPackages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationProvisionerTests.swiftPackages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationRunPlannerTests.swiftPackages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationStoreTests.swiftPackages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationTestSupport.swiftPackages/macOS/CmuxOrchestration/Tests/CmuxOrchestrationTests/OrchestrationValidatorTests.swiftResources/Localizable.xcstringsSources/OrchestrationRunController.swiftSources/TerminalController+ControlOrchestrationContext.swiftcmux.xcodeproj/project.pbxprojdocs/cli-contract.mddocs/orchestrations.md
| // Orchestration store verbs manage ~/.cmuxterm/orchestrations locally | ||
| // and must work with no cmux running; only run/plan need the socket. | ||
| if command == "orchestration", !Self.orchestrationCommandNeedsSocket(commandArgs) { | ||
| try runOrchestrationLocalNamespace(commandArgs: commandArgs, jsonOutput: jsonOutput) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'func orchestrationCommandNeedsSocket' -A 15 CLI/Repository: manaflow-ai/cmux
Length of output: 1333
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' CLI/CMUXCLI+Orchestration.swift
printf '\n---\n'
rg -n 'topLevelCommandNames|shouldOpenAsPathArgument|case "orchestration"|orchestration' CLI/cmux.swift CLI/CMUXCLI+*.swiftRepository: manaflow-ai/cmux
Length of output: 14166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '45,85p' CLI/CMUXCLI+CommandSuggestions.swift
printf '\n---\n'
sed -n '5590,5610p' CLI/cmux.swift
printf '\n---\n'
sed -n '35076,35295p' CLI/cmux.swift | rg -n 'orchestration|Commands:|usage'Repository: manaflow-ai/cmux
Length of output: 1999
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '"orchestration"' CLI/CMUXCLI+CommandSuggestions.swift CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 515
Add orchestration to the top-level command set
shouldOpenAsPathArgument() only skips names in Self.topLevelCommandNames, so cmux orchestration ... can be treated as a path when a local file or directory named orchestration exists. Add the command name to CLI/CMUXCLI+CommandSuggestions.swift:53 so routing in CLI/cmux.swift:5602 stays command-first.
🤖 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 `@CLI/cmux.swift` around lines 3255 - 3261, Add "orchestration" to
Self.topLevelCommandNames in CommandSuggestions so shouldOpenAsPathArgument()
recognizes it as a top-level command even when a matching filesystem path
exists. Preserve the existing local orchestration routing in
runOrchestrationLocalNamespace and command-first dispatch behavior.
| case "orchestration": | ||
| return Self.orchestrationHelpText() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
orchestration is missing from the top-level cmux --help command list.
The generic usage() string printed by cmux help/cmux --help (around L35099-35272) enumerates all top-level commands but does not mention orchestration, even though this PR adds a substantial new command surface (list, info, plan, run, local template management). Users running plain cmux --help won't discover it; only cmux orchestration --help (which this new subcommandUsage case correctly serves) will.
📝 Suggested addition to the Commands: list in usage()
hooks setup|uninstall [--agent <name>]
hooks <agent> <install|uninstall|event> [options; opencode supports --project]
hooks feed --source <agent> [--event <event>]
+ orchestration <list|info|plan|run|...> [args...]
ping🤖 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 `@CLI/cmux.swift` around lines 15806 - 15807, Update the generic usage() help
text’s top-level Commands list to include orchestration, matching the existing
Self.orchestrationHelpText() dispatch and its placement among related commands.
Ensure plain cmux --help and cmux help expose the new command without changing
the orchestration subcommand help.
| func hasFlag(_ args: [String], name: String) -> Bool { | ||
| args.contains(name) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stop flag detection at the -- terminator.
hasFlag contradicts the documented parser contract and treats forwarded arguments as cmux flags. For example, a post-terminator --yes still bypasses the orchestration trust prompt.
Proposed fix
func hasFlag(_ args: [String], name: String) -> Bool {
- args.contains(name)
+ args.prefix { $0 != "--" }.contains(name)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func hasFlag(_ args: [String], name: String) -> Bool { | |
| args.contains(name) | |
| } | |
| func hasFlag(_ args: [String], name: String) -> Bool { | |
| args.prefix { $0 != "--" }.contains(name) | |
| } |
🤖 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 `@CLI/CMUXCLI`+OptionParsing.swift around lines 74 - 76, Update hasFlag to
inspect only arguments before the standalone "--" terminator, so flags appearing
afterward are treated as forwarded arguments rather than cmux options. Preserve
existing exact flag matching for pre-terminator arguments and return false when
the requested flag occurs only after "--".
| if !values.isEmpty { | ||
| _ = try? store.setResolvedParameters(name: name, values: values) | ||
| } | ||
|
|
||
| guard interviewUnanswered else { return } | ||
| let installation: InstalledOrchestration | ||
| do { | ||
| installation = try store.installed(named: name) | ||
| } catch { | ||
| return | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not report success after parameter persistence fails.
Both writes discard errors, and the reload failure silently skips the interview. configure can therefore print “parameters saved” while the store remains unchanged. Propagate these failures as CLIError instead.
Proposed fix
if !values.isEmpty {
- _ = try? store.setResolvedParameters(name: name, values: values)
+ do {
+ _ = try store.setResolvedParameters(name: name, values: values)
+ } catch {
+ throw CLIError(message: String(describing: error))
+ }
}
let installation: InstalledOrchestration
do {
installation = try store.installed(named: name)
} catch {
- return
+ throw CLIError(message: String(describing: error))
}
if !answers.isEmpty {
- _ = try? store.setResolvedParameters(name: name, values: answers)
+ do {
+ _ = try store.setResolvedParameters(name: name, values: answers)
+ } catch {
+ throw CLIError(message: String(describing: error))
+ }
}Also applies to: 372-374
🤖 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 `@CLI/CMUXCLI`+Orchestration.swift around lines 348 - 358, Update the parameter
persistence and reload flow in configure around store.setResolvedParameters and
store.installed(named:) to propagate failures as CLIError instead of discarding
them or returning silently. Preserve the existing success path and ensure
“parameters saved” is only reported after both required store operations
succeed.
| if effectiveJSONOutput, dryRun { | ||
| print(jsonString(planPayload)) | ||
| return | ||
| } | ||
| printOrchestrationPlan(plan, trustConfirmed: trustConfirmed) | ||
| if dryRun { | ||
| return | ||
| } | ||
|
|
||
| if !trustConfirmed && !assumeYes { | ||
| guard isatty(fileno(stdin)) != 0 else { | ||
| throw CLIError(message: "First run of '\(name)' needs trust confirmation. Re-run with --yes after reviewing the plan above.") | ||
| } | ||
| print("First run of this template. It will type the agent commands above into real terminals" + | ||
| (orchestrationPlanHasScripts(plan) ? " and execute the listed template scripts" : "") + ".") | ||
| print("Proceed? [y/N]: ", terminator: "") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep run --json stdout machine-readable.
Non-dry runs print the human plan and potentially the confirmation prompt before the JSON payload, so stdout cannot be decoded as JSON. Suppress human output in JSON mode and require --yes after separately reviewing plan --json.
Proposed fix
if effectiveJSONOutput, dryRun {
print(jsonString(planPayload))
return
}
-printOrchestrationPlan(plan, trustConfirmed: trustConfirmed)
+if !effectiveJSONOutput {
+ printOrchestrationPlan(plan, trustConfirmed: trustConfirmed)
+}
if dryRun {
return
}
if !trustConfirmed && !assumeYes {
+ if effectiveJSONOutput {
+ throw CLIError(
+ message: "Trust confirmation required. Review `cmux orchestration plan \(name) --json`, then re-run with --yes."
+ )
+ }
guard isatty(fileno(stdin)) != 0 else {Also applies to: 119-121
🤖 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 `@CLI/CMUXCLI`+OrchestrationRun.swift around lines 89 - 104, Update the
orchestration run flow around effectiveJSONOutput so non-dry JSON runs emit only
the machine-readable JSON payload on stdout. Suppress printOrchestrationPlan and
interactive trust-confirmation output in JSON mode, and require --yes when trust
is not already confirmed; users must review the plan separately via plan --json.
| } catch { | ||
| failures.append("\(workspacePlan.title): \(String(describing: error))") | ||
| continue | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not expose raw upstream errors in user-facing text.
Appending String(describing: error) captures raw stderr from the external provisioner commands (e.g., git clone or scripts). Displaying this in the UI violates the guidelines against exposing implementation details and raw upstream errors.
Log the raw error to internal telemetry instead, and append only the safe workspacePlan.title to the failures list so a generic error can be presented. As per path instructions: "User-facing errors, alerts, command output, API error bodies, and recovery copy must not expose implementation details. Do not expose raw upstream errors... Keep provider, billing, database, and authentication implementation details in sanitized logs or internal telemetry, not user-visible text."
🛡️ Proposed fix
} catch {
- failures.append("\(workspacePlan.title): \(String(describing: error))")
+#if DEBUG
+ cmuxDebugLog("orchestration.run.workspaceFailed workspace=\(workspacePlan.title) error=\(String(describing: error))")
+#endif
+ failures.append(workspacePlan.title)
continue
}🤖 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/OrchestrationRunController.swift` around lines 43 - 46, Update the
catch block in OrchestrationRunController to log the raw error through internal
telemetry, then append only workspacePlan.title to failures. Remove
String(describing: error) from the user-facing failure text so callers can
present a generic error without exposing provisioner details.
Source: Path instructions
| body: String( | ||
| format: String( | ||
| localized: "orchestration.run.readyBody", | ||
| defaultValue: "%1$d workspace(s) provisioned and running." | ||
| ), | ||
| plan.workspaces.count | ||
| ) | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use ICU-style plural localization keys.
Using %1$d inside String(format: String(localized:)) is an anti-pattern for plurals. Based on learnings, prefer explicit .one and .other localization keys for pluralized text to ensure correct translation across all locales.
♻️ Proposed fix
- body: String(
- format: String(
- localized: "orchestration.run.readyBody",
- defaultValue: "%1$d workspace(s) provisioned and running."
- ),
- plan.workspaces.count
- )
+ body: plan.workspaces.count == 1
+ ? String(localized: "orchestration.run.readyBody.one", defaultValue: "1 workspace provisioned and running.")
+ : String(localized: "orchestration.run.readyBody.other", defaultValue: "\(plan.workspaces.count) workspaces provisioned and running.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| body: String( | |
| format: String( | |
| localized: "orchestration.run.readyBody", | |
| defaultValue: "%1$d workspace(s) provisioned and running." | |
| ), | |
| plan.workspaces.count | |
| ) | |
| ) | |
| body: plan.workspaces.count == 1 | |
| ? String(localized: "orchestration.run.readyBody.one", defaultValue: "1 workspace provisioned and running.") | |
| : String(localized: "orchestration.run.readyBody.other", defaultValue: "\(plan.workspaces.count) workspaces provisioned and running.") | |
| ) |
🤖 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/OrchestrationRunController.swift` around lines 129 - 136, Update the
localized body in the orchestration run-ready notification to use ICU-style
pluralization with explicit one and other variants keyed from
plan.workspaces.count, replacing the String(format:) "%1$d" interpolation.
Preserve the existing workspace count and message meaning while allowing
locale-correct plural translations.
Source: Learnings
| store.addNotification( | ||
| tabId: anchorWorkspaceID, | ||
| surfaceId: nil, | ||
| title: String(localized: "orchestration.run.failedTitle", defaultValue: "Orchestration run had failures"), | ||
| subtitle: plan.groupName, | ||
| body: failures.joined(separator: "\n") | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use a localized string for the error notification body.
The failure notification body uses non-localized string concatenation, which bypasses the localized API requirement. Wrap the sanitized failures list inside a proper localized string.
As per path instructions: "User-facing errors... and recovery copy must not expose implementation details."
🛡️ Proposed fix
store.addNotification(
tabId: anchorWorkspaceID,
surfaceId: nil,
title: String(localized: "orchestration.run.failedTitle", defaultValue: "Orchestration run had failures"),
subtitle: plan.groupName,
- body: failures.joined(separator: "\n")
+ body: String(
+ localized: "orchestration.run.failedBody",
+ defaultValue: "Failed to provision: \(failures.joined(separator: ", "))"
+ )
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| store.addNotification( | |
| tabId: anchorWorkspaceID, | |
| surfaceId: nil, | |
| title: String(localized: "orchestration.run.failedTitle", defaultValue: "Orchestration run had failures"), | |
| subtitle: plan.groupName, | |
| body: failures.joined(separator: "\n") | |
| ) | |
| } | |
| store.addNotification( | |
| tabId: anchorWorkspaceID, | |
| surfaceId: nil, | |
| title: String(localized: "orchestration.run.failedTitle", defaultValue: "Orchestration run had failures"), | |
| subtitle: plan.groupName, | |
| body: String( | |
| localized: "orchestration.run.failedBody", | |
| defaultValue: "Failed to provision: \(failures.joined(separator: ", "))" | |
| ) | |
| ) |
🤖 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/OrchestrationRunController.swift` around lines 138 - 145, Update the
failure notification in the orchestration run handling flow around
store.addNotification so its body uses the localized string API with the
sanitized failures list as interpolation data. Ensure the user-facing message
does not expose implementation details, while preserving the existing failure
content and notification behavior.
Source: Path instructions
| /// The orchestration-domain witnesses for ``ControlCommandCoordinator``: | ||
| /// reads the installed-template store, plans runs through | ||
| /// `CmuxOrchestration`, enforces the trust gate, and hands accepted plans to | ||
| /// ``OrchestrationRunController`` for actuation. Store and template files | ||
| /// are small JSON documents (the `SavedLayoutStore` precedent), so reads | ||
| /// happen inline; provisioning always leaves the main actor. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the concrete synchronous I/O reached from these MainActor witnesses.
ast-grep outline Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Store/OrchestrationStore.swift --items all
ast-grep outline Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationRunPlanner.swift --items all
rg -n -C3 '\b(list|installed|plan|readData|directoryContents|contentsOfDirectory)\s*\(' \
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestrationRepository: manaflow-ai/cmux
Length of output: 21507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' Sources/TerminalController+ControlOrchestrationContext.swift
printf '\n====\n'
sed -n '1,220p' Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Store/OrchestrationStore.swift
printf '\n====\n'
sed -n '1,220p' Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationRunPlanner.swift
printf '\n====\n'
sed -n '1,240p' Sources/TerminalController.swiftRepository: manaflow-ai/cmux
Length of output: 40510
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C4 '`@MainActor`|internal_error|String\(describing:|ControlCommandCoordinator|Orchestration' \
Sources/TerminalController+ControlOrchestrationContext.swift \
Sources/TerminalController.swift \
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Store/OrchestrationStore.swift \
Packages/macOS/CmuxOrchestration/Sources/CmuxOrchestration/Run/OrchestrationRunPlanner.swiftRepository: manaflow-ai/cmux
Length of output: 50373
Move orchestration storage and planning off @MainActor. controlOrchestrationList, controlOrchestrationInfo, orchestrationPlan, and controlOrchestrationRun still synchronously list installs, read/parse install records, and scan template files on the main actor. That can stall UI and other socket handling on larger installs or slow disks. Also replace the String(describing:) passthroughs in these failed paths with sanitized user-facing messages.
🤖 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`+ControlOrchestrationContext.swift around lines 5
- 10, Update controlOrchestrationList, controlOrchestrationInfo,
orchestrationPlan, and controlOrchestrationRun so install listing, record
parsing, and template scanning execute off `@MainActor`, while retaining
main-actor execution only for UI/socket coordination and run actuation. Replace
String(describing:) in each failed path with concise sanitized user-facing
messages that do not expose raw errors or filesystem details.
Sources: Coding guidelines, Path instructions
| } catch { | ||
| return .failed(String(describing: error)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not return raw internal errors over the socket API.
String(describing: error) can expose local paths, malformed file contents, and storage implementation details through internal_error. Map these catches to stable product-facing messages and keep detailed diagnostics in privately redacted logs.
As per coding guidelines, API errors must not expose raw upstream messages or internal implementation details.
Also applies to: 36-40, 95-98, 134-140, 161-166
🤖 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`+ControlOrchestrationContext.swift around lines 23
- 24, Replace the raw String(describing: error) values returned by the catch
blocks in TerminalController control orchestration with stable, product-facing
failure messages. Apply the same mapping at the additional catch sites noted in
the review, while recording detailed errors only through privately redacted
logging; do not expose upstream messages or internal implementation details via
the socket API.
Source: Coding guidelines
…orchestration-templates
…mplates # Conflicts: # Resources/Localizable.xcstrings
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLI/cmux.swift (1)
35186-35188: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
orchestrationis missing from the top-levelcmux --helpcommand list.The generic
usage()string enumerates all top-level commands but does not mentionorchestration, even though this PR adds a substantial new command surface (list,info,plan,run, local template management). Users running plaincmux --helpwon't discover it; onlycmux orchestration --helpwill.📝 Proposed fix to add `orchestration`
hooks setup|uninstall [--agent <name>] hooks <agent> <install|uninstall|event> [options; opencode supports --project] hooks feed --source <agent> [--event <event>] + orchestration <init|validate|install|list|info|remove|update|configure|plan|run> [args...] ping🤖 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 `@CLI/cmux.swift` around lines 35186 - 35188, Update the generic usage/help command list in CLI/cmux.swift to include the top-level orchestration command alongside the other commands, so plain cmux --help exposes its subcommands and users can discover it.
♻️ Duplicate comments (1)
CLI/cmux.swift (1)
3275-3281: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
orchestrationto the top-level command set.
shouldOpenAsPathArgument()only skips names inSelf.topLevelCommandNames, socmux orchestration ...can be treated as a path when a local file or directory namedorchestrationexists. Ensure the command name is added toCLI/CMUXCLI+CommandSuggestions.swiftso routing inCLI/cmux.swiftstays command-first.Please run the following script to verify if the missing command name has been added to the command suggestions:
#!/bin/bash # Description: Verify if orchestration is added to topLevelCommandNames rg -n 'topLevelCommandNames' -A 15 CLI/CMUXCLI+CommandSuggestions.swift || true🤖 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 `@CLI/cmux.swift` around lines 3275 - 3281, Update the topLevelCommandNames definition in CMUXCLI+CommandSuggestions.swift to include "orchestration", ensuring shouldOpenAsPathArgument() routes cmux orchestration commands as commands even when a matching local path exists.
🤖 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.
Outside diff comments:
In `@CLI/cmux.swift`:
- Around line 35186-35188: Update the generic usage/help command list in
CLI/cmux.swift to include the top-level orchestration command alongside the
other commands, so plain cmux --help exposes its subcommands and users can
discover it.
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 3275-3281: Update the topLevelCommandNames definition in
CMUXCLI+CommandSuggestions.swift to include "orchestration", ensuring
shouldOpenAsPathArgument() routes cmux orchestration commands as commands even
when a matching local path exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9403240d-0e6d-4049-accb-7f98777906bb
📒 Files selected for processing (2)
.github/workflows/ci.ymlCLI/cmux.swift
lawrencecchen
added
the
stale-revisit
3 participants
Summary
Adds the shareable orchestration template layer: a packaged, versioned format that captures a whole way of running fleets of coding agents (prompts, workspace shapes, agent commands, provisioning style), so one user can share their setup as a git repo and another can install and run it. This is the template/packaging layer described in the Fleet plan (related: #7361); it is deliberately independent of the fleet engine skeleton in #7374 (no dependency on
Packages/macOS/CmuxFleet, no file overlap beyond the shared CI package list / workspace registration lines) and exposes clean seams (OrchestrationRunPlan, per-task agent command + prompt + cwd + caps) the engine can consume later.What's included
Packages/macOS/CmuxOrchestration(new pure SwiftPM package, Swift 6, dependency-free, 75 tests):orchestration.jsonmanifest model with versionedschemaVersion, actionable parse errors, and unknown-key detection.string/int/bool/path/choice/agent, defaults, typed coercion). Resolved values live per-install in~/.cmuxterm/orchestrations/<name>/install.json, never inside the template.worktree(git worktree per task),clone-pool(v1: fresh clone per task),script(template-supplied provision script — the SSH/cloud escape hatch).OrchestrationFileSystem,OrchestrationGitClient,OrchestrationProcessRunner) with in-memory fakes — the CmuxControlSocket testing shape. Update keeps parameter answers, drops stale keys, and resets trust confirmation.{{placeholder}}rendering (strict: unknown placeholders fail at validate/plan time), shell quoting, run planner (branch/directory naming, prompt file,CMUX_ORCHESTRATION*env,minCmuxVersiongate), provisioner,initscaffold.orchestration.*v2 socket domain (CmuxControlSocket + app):orchestration.list/orchestration.info/orchestration.plan/orchestration.run, following the coordinator seam pattern (ControlOrchestrationContext+ fakes + 8 coordinator tests).TerminalController+ControlOrchestrationContextplans via the package and enforces the trust gate;OrchestrationRunControllerprovisions sequentially off the main actor, then creates workspaces unfocused, grouped in the sidebar, with the agent command delivered as typed terminal input (initialTerminalInput, or the layout's setup command when the template ships a layout). Completion/failure lands as a cmux notification (strings localized en/ja).cmux orchestrationCLI namespace (CLI/CMUXCLI+Orchestration*.swift):init,validate,install,list,info,remove,update,configurerun locally (no socket needed) with an interactive parameter interview;plan/rungo through the socket.runprints the full plan (workspaces, agent commands, scripts, substrate) before executing.CLI/cmux.swiftmay not grow (900-line hard cap), so the option-parsing helpers were extracted verbatim intoCMUXCLI+OptionParsing.swift; net −60 lines. No budget TSV changes.Trust model (enforced in code):
--yesskips the prompt; the socket method errors withneeds_confirmationuntil confirmed.Example + docs:
Examples/Orchestrations/issue-fleetis a real worktree-substrate template that package tests validate and plan on every CI run (the format's living fixture);docs/orchestrations.mddocuments the format, placeholders, trust model, CLI, socket API, and storage;docs/cli-contract.mdgains the namespace row and a no-socket help probe.Scope cuts (explicit)
stepschain (parsed, validated, tested), but v1runexecutes the first step only and says so in the plan notes. Full chain execution + success-condition supervision (pr-exists,hook-event,exit-code) belongs to the fleet engine.instructions/fragments: validated and documented, not yet auto-applied to workspaces.configurecommand instead.Testing
swift test— CmuxOrchestration: 75 tests, 11 suites, all passing (manifest, validator, store, planner, provisioner, placeholders, scaffold, example fixture).swift test— CmuxControlSocket: 234 tests passing including the new orchestration coordinator suite.swift_file_length_budget.py(merge-base mode, green),check-workspace-package-groups.py,check-package-resolved-policy.py,check-pbxproj.sh,lint-pbxproj-test-wiring.sh.Resources/Localizable.xcstringsfor en and ja. CLI stdout is exempt per existing convention; no web/docs message catalogs are affected. App-target build is validated by CI (no local xcodebuild per task constraints).Related: #7361
🤖 Generated with Claude Code
Summary by cubic
Adds shareable orchestration templates: a versioned package + CLI to install, plan, and trust‑gated run fleets of coding agents that provision task workspaces and type agent commands for you. Trust uses a fingerprint over command/substrate metadata and executable template content digests (prompts/layout/scripts); symlink and secret scans walk the whole tree safely.
New Features
CmuxOrchestrationpackage:orchestration.jsonschema (parameters, steps, agents, substrates), strict validator (rejects symlinks; whole‑tree secret scan inOrchestrationValidatorScans.swift), placeholder rendering, local store at~/.cmuxterm/orchestrations(install/list/info/remove/update), run planner + provisioner forworktree,clone-pool, andscript;{{prompt_file}}is shell‑quoted.orchestration.list/info/plan/runinCmuxControlSocket;planreturns a trust fingerprint (SHA‑256 over substrate, agent commands, template version, and digests of executable template files). The app enforces a first‑run/update trust gate that must echo this fingerprint; workspaces are created unfocused and grouped; localized run notifications.cmux orchestration:init/validate/install/list/info/remove/update/configurerun locally;plan/runvia socket and print the full plan;--yesconfirms trust non‑interactively and includes the fingerprint. Option parsing helpers moved toCMUXCLI+OptionParsing.swift.Examples/Orchestrations/issue-fleetanddocs/orchestrations.md; CI buildsCmuxOrchestration; app target declares a product dependency onCmuxOrchestration.OrchestrationManifest.parse(...)withParseOutput; parameter override coercion ismanifest.coerceParameterOverrides(...);OrchestrationWellKnownParameteruses string‑raw cases;OrchestrationPlaceholdersis an instantiable value type.Migration
cmux orchestration install Examples/Orchestrations/issue-fleetthencmux orchestration run issue-fleet --task "Do X" --task "Do Y".--yes); the confirmation includes the plan’s fingerprint, which covers commands, substrate, and executable template content.updatepreserves answers and requires reconfirmation.--param k=voverrides install parameters; templates with symlinks are rejected byvalidate/install. Related: Fleet: built-in agent orchestration loop (workspace per task, board, supervision) #7361Written for commit 253d246. Summary will update on new commits.
Summary by CodeRabbit
cmux orchestrationcommand suite for template lifecycle: init/validate/install/update/configure/list/info/remove plus plan/run (local and socket-backed).--paramoverrides, agent selection, dry-run, JSON output, interactive parameter interview, and trust-confirmation (with fingerprint binding).issue-fleetorchestration example.CmuxOrchestrationSwiftPM tests.