Fix validation of unresolved workspace reorder targets

[austinywang]

reorder-workspace --before/--after can report “Specify exactly one target” when its sole target is an unknown workspace reference. The server counted resolved IDs instead of supplied options. It could also accept --index alongside an unknown relative target and call the mutation planner.

The shared workspace.reorder validator now counts supplied non-null selectors, rejects conflicting options before calling the planner, and returns not_found for an unresolved sole relative target. Valid relative targets and dry-run behavior retain the existing planner and mutation path.

Investigation of #13506 captured the shipped 0.64.25 CLI request: it sends before_workspace_id, workspace_id, and dry_run, with no index. The same holds for --after and both long aliases. This PR addresses the reproduced server validation defect; it does not establish that the reporter's target reference was stale.

Validation:

• Local baseline at regression-only commit d8fb98623ba3a471288e838bf3717cfa84809b93: six failed expectations; valid-reference cases already pass. Its hosted macOS run was skipped by admission debounce, so CI red is not evidence that the regression test ran.
• Fix 84b9ec2dc70c454a6ce115a9a4b9663835b0029f: three parameterized tests, eight cases pass. Covers unknown before/after refs, conflicting index options, planner exclusion on failure, valid refs, and dry runs.
• Localization diff audit and strict catalog validation pass: zero changed keys, eight catalogs, nine macOS locales.
• Conflict-only gate passes against main 8bb58c74fd835be9c0dd7067055a7aa258db3e49.
• Exact-SHA fleet build 45c9949354691f23e8bbd1fc succeeded on 84b9ec2dc70c454a6ce115a9a4b9663835b0029f. Separate submission and terminal receipts include artifact digest and cleanup evidence. Published build: 13506-workspace-reorder-targets.
• Live verification launched that artifact in a separate 1000×700 window. Before every command, identify checked the explicit tagged socket, bundle identifier, and app path. CLI before/after moves and both aliases succeed; dry runs preserve order; unknown relative refs return not_found; mixed index/unknown-ref requests return invalid_params without reordering. Index moves also pass. Workspace-list responses and a real debug-socket screenshot were captured; the test app was closed.
• HQ publication manifest and artifact SHA-256 match d6583b19b9be09465979aff438260cc4a8c1e2a139c084bd8d52160435640831. The verified archive was restored into HQ's tag cache after automatic restoration failed; the final link GET returned HTTP 200 for this tag.
• All five required checks pass. No inline comments or change-request reviews remain. The general review's test-evidence correction is incorporated above. Its two explicitly nonblocking follow-up suggestions are outside this merged CLI validation fix: malformed direct-RPC selectors and identifying the offending relative target in the error payload. The latter retains the existing planner's payload contract; this PR does not claim those diagnostics improved.

Impact: CmuxControlSocket owns validation, shared by CLI and direct RPC. The existing app context and workspace planner retain mutation and pin/group ordering. No persistence, UI, relay allowlist, or shortcut changes. The remote relay continues to deny this method.

Attribution: unregistered; run/session issue-13506-workspace-reorder-targets.

[coderabbitai]

Warning

Review limit reached

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 810c554f-1085-496c-befd-ea32e64c61ed

📥 Commits

Reviewing files that changed from the base of the PR and between a93efa5 and 84b9ec2.

📒 Files selected for processing (3)

• Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+Workspace.swift
• Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlWorkspaceReorderTargetTests.swift
• Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/FakeWorkspaceControlCommandContext.swift

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[teamleaderleo]

Second independent review. The fix is correct and I would land it.

Verified the re-diagnosis at the reporter's exact build: at b685a275c2, CLI/cmux.swift:9866-9875 only sends index when --index is passed, so the issue's stated cause (the CLI sending an index alongside before_workspace_id) is wrong. The real defect is server-side — targetCount counted resolved IDs, so an unminted workspace:N ref collapsed to zero targets and produced the reported invalid_params. That also explains why a syntactically valid but bogus UUID answered not_found while the CLI ref form did not: a UUID parses and reaches the planner, an unknown kind:N ref does not.

The second half matters more than the message. TerminalController+ControlWorkspaceContext.swift:151-159 ignores before/after whenever toIndex is non-nil, so --index plus an unknown relative target used to silently perform an index move while discarding the target the caller asked for. The new checks reject that before anything mutates. The .notFound short-circuit matches what the planner already returns for a resolvable-but-absent target, so it is consistent rather than new behavior.

Ran nothing locally — the package is macOS-only and this host is Linux. Read CI instead: macos / swift-package-tests on 84b9ec2 logs Suite ControlWorkspaceReorderTargetTests passed within Test run with 468 tests in 61 suites passed. By inspection the new tests fail without the fix at six expectations, matching the description.

Two things worth a follow-up, neither blocking:

• ControlCommandCoordinator+Workspace.swift:304-306 — when the unresolved thing is the target, the not_found payload still carries the subject workspace_id. Someone debugging the reporter's exact command is told the workspace they named is missing when it resolved fine. Echoing the offending before_workspace_id/after_workspace_id would finish the job this PR starts.
• :289-291 — hasNonNull is true for "", whitespace, and non-string JSON, while uuid() returns nil for all of them, so a malformed target now reports not_found instead of invalid_params. Unreachable from the CLI, reachable by direct RPC. Same unknown ref also yields invalid_params for workspace_id but not_found for before_workspace_id.

On #13506: this changes the reporter's outcome from invalid_params to not_found — the command still fails, and their error proves workspace:2 did not resolve. The issue's Expected condition is not met by this diff, so I am leaving it open. The PR body makes no closing claim, so merging does not close it.

One evidence correction: the two-commit structure did not demonstrate red-before-green here. On d8fb986 the macos workflow was skipped by admission debounce and the tests gate failed with macOS workflow did not pass: skipped, so that red is the gate, not the regression test.

A first independent agent review at this same commit also found no defects; it was never posted here. Merging on two agreeing reviews per the standing rule.

— AlderQuarry g1 🎒
Run: run_cmux_land_ready_prs_20260923_A