Master Axiom cmux Cloud PR - #13151
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe PR adds Cloud read coordination and bounded refresh behavior, atomic guest CLI installation with rollback handling, cleanup-pending VM recovery, expanded diagnostics support for placement and RC payloads, and related tests and CI workflows. ChangesCloud reliability and diagnostics
Priority: ⬆️ High Estimated code review effort: 5 (Critical) | ~90 minutes Change: Bug fix · Severity of issue fixed: High Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 2 warnings)
✅ Passed checks (20 passed)
Full details: Linked Issues checkExplanation
Resolution For Full details: Cmux Swift Package BoundariesExplanation The diff adds independently testable Cloud read domain logic to the app target. Resolution Create a small macOS SwiftPM target named Full details: Cmux Full InternationalizationExplanation The PR adds Resolution Do not expose Full details: Cmux Architecture RethinkExplanation The Swift diff introduces a new network NotificationCenter side channel and splits recovery ownership across multiple MainActor objects. Resolution Create one explicit Cloud read lifecycle owner at bootstrap. Keep network state, transition fencing, and the single recovery action in ✨ Finishing Touches 💡 1📝 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 |
|
All contributors have signed the CLA ✍️ ✅ |
(cherry picked from commit 091789d)
(cherry picked from commit af20ca6)
(cherry picked from commit f9b3904)
(cherry picked from commit b015f9c)
(cherry picked from commit 0bfce3f)
(cherry picked from commit c80df5f)
(cherry picked from commit d651670)
(cherry picked from commit c14551a)
(cherry picked from commit 7440557)
Cmd-Opt-= and Cmd-Opt-- must zoom the canvas when the focused panel is a Markdown file in text mode. That editor is a SavingTextView but not a text file preview, so the canvas layout clause should still allow canvas zoom. Records the canvas zoom factors and checks the editor font is unchanged. This test fails on the current branch: #12814 widened filePreviewTextEditorFocused to every SavingTextView, which blocks canvasLayoutOutsideFocusedContent (and Cmd-0, covered by cmdZeroInCanvasResetsCanvasZoomWhenMarkdownSourceEditorIsFocused). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#7157 limited filePreviewTextEditorFocused to the focused text file preview's editor, matching the command palette's panelIsFilePreviewTextEditor, so Markdown source and Dock editors keep Canvas zoom and do not take preview zoom. #12814 widened the flag to every SavingTextView so word wrap works in those editors, which also turned off canvasLayoutOutsideFocusedContent there: Cmd-0, Cmd-Opt-= and Cmd-Opt-- stopped reaching the canvas. Keep both scopes. filePreviewTextEditorFocused is narrow again and drives the shared shortcut context, preview zoom and Canvas routing. The new fileEditorFocused covers every file editor and applies only to actions in the .filePreviewTextEditor context (word wrap): whenClauseContext(for:) projects it onto the file-editor atom for their `when` clause, both at the keyDown gate and when arming chords, and isAvailable uses it for their menu and palette state. FileEditorWordWrapShortcutTests.appShortcutRouting still covers Opt-Z in a bare file editor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"Best-effort telemetry cannot monopolize resident delivery capacity" enqueued three tool events back to back. Tool telemetry has one ingress slot, so the second and third are admitted only if the drain task has already moved the previous one into a lane. On a loaded app host it had not: one tool event was dropped, tool-4 was then admitted in its place, and the lifecycle event queued behind tool-4 in the surface-4 lane never started, so waitUntilStarted(count: 3) timed out. Wait for each of the first two tool deliveries to start before the next enqueue. Tool-4 is then rejected by the three-event best-effort reservation whether or not tool-3 has left ingress. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit f385fa6)
24 test helpers waited for a child process by queueing a blocking waitUntilExit() on DispatchQueue.global() and waiting on a semaphore with a timeout. Every queued waiter holds a pool thread until its child exits. Waiters for children that never exit accumulate across a batch; once the pool is exhausted a new waiter never starts, and a child that exited normally is reported as a timeout. testAgentTurnDiffBaselineStoresUntrackedSnapshotsOutsideGit hit this: it passes in ~0.6 s in eight of the nine runs examined and, in one, a read-only `git for-each-ref` reported status 124 after 30 s with empty stderr. cd7d25c fixed the same failure in one Claude hook helper. waitForProcessExit(_:timeout:) polls isRunning on the calling thread and returns DispatchTimeoutResult, so each `sem.wait(timeout: .now() + t)` becomes `waitForProcessExit(process, timeout: t)` with its surrounding logic unchanged. No test in cmuxTests queues a waitUntilExit() anymore. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit dba68ec)
testSenderRelativeSidebarActionKeysItsOriginatingWindowBeforeMutation gave AppKit one fixed 50 ms run-loop spin to move key status after makeKeyAndOrderFront. On a loaded runner the change can take several turns, so the same code passed 3 times and failed 6 across full-suite runs. Wait for the transition with a 5 s deadline instead. Also normalize project.pbxproj ordering for ProcessExitWait.swift. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit be5fa50)
foregroundAuthenticatedAttachUsesConfiguredRetryBudget runs the full 20-attempt fallback budget through fake ssh/cmux/sleep, which takes about 4.5 s per case on an idle runner, against a fixed 5 s process deadline. Under shard load the harness SIGTERMed the script after 19 sleeps (status 143). The deadline now scales with the attempts the case expects; the assertions are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d2126d9)
#10564 moved reload surface fanout from RunLoop.main.perform onto TerminalConfigurationApplyScheduler, which yields through MainActorDeferredActionScheduler's Task { @mainactor }. The four reload cases are synchronous main-actor tests that wait with a nested RunLoop.main.run, which cannot run main-actor tasks while the test job holds the main queue. The fanout never finished, so completions, the reload notification, and the queued transaction never arrived. Make the cases async and wait with waitWhileSuspended. Each first settles any in-flight reload. An idle full reload now commits synchronously, so the staged-appearance case holds one transaction open to exercise the queued path it describes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 6733553)
…n yet With the reload fanout on main-actor tasks, the notification arrives only after later turns, so asserting it had not fired right after reloadConfiguration proved nothing: an unblocked reload also reads false at that instant. The case now asserts the coordinator is parked in .waitingForFontWork, that five main-actor yields leave it there, and that the commit callback has not run, before releasing the barrier. Adds a DEBUG-only GhosttyApp.debugConfigurationReloadPhase for that. Raised by CodeRabbit on #14006. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 38d9a9c)
testConfiguredWorkspaceTerminalFontSizeResetRestoresEverySplit failed in the #13151 full-ci shard with twelve issues: four surfaces, each still 3 points below the configured size after Cmd+0 was accepted. An earlier case in the same process left a configuration reload fanning out. That reload holds the app-wide font-size work barrier (GhosttyTerminalView reload transaction), and the arbiter retains font mutations issued behind the barrier until reconciliation finishes, by design. The reset was accepted and queued, so the synchronous checks ran before it applied. Settle any in-flight reload while suspended (the helper from #14057) before issuing the reset, and assert no reload is active. The reset assertions are unchanged: every split, the dock terminal, and the split created after the zoom must still reset immediately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # cmux.xcodeproj/project.pbxproj
The card-layout expectation in "Cloud failure controls stay above native surfaces" fails in the full app-host shard with only `overlay.frame.width > 100 -> false`. Record the card, source terminal, content reference and window frames so the failing run names which one was small. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 3499302)
Conflict in cmux.xcodeproj/project.pbxproj: keep both the PR's CloudReadResponseGate.swift and main's SSHTuiMigrationTests.swift entries in the cmuxTests group; normalized with scripts/normalize-pbxproj.py. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Conflict in .github/workflows/auth-refresh-tests.yml: keep the PR's dual-Xcode overflow runner and pinned-Xcode step, prefixed with main's GitHub-hosted fork branch (#14023), matching the pattern main already uses in ci-macos.yml. cloud-vm-guest-install.yml (PR-only pull_request workflow) gains the same 'ubuntu-24.04' fork branch so tests/test_ci_fork_runner_routing.py passes on the merged tree. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Merged current Two merge commits,
Also checked statically: main's Dev build of — Shardwright pending |
ci.yml: main replaced the sleep-based macos-debounce job with macos-admission-gate (job dependencies, no polling). The gate and the macos caller keep this branch's cli route in their conditions. ci-macos.yml: compile admission keeps this branch's condition and takes main's fork-aware runs-on. CLI tests moved to cmuxCLITests take main's waitForProcessExit waits (manaflow-ai#13151). ProcessExitWait.swift moves to cmuxCLITestSupport and is compiled into both test targets, since four moved suites now call it. RemoteShellCWDRelayTests keeps main's pbxproj IDs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#14228) #13327 added single-flight stats reads inside VMResourceStatsStore. It was written before #13151 landed CloudReadRequestCoordinator, which already shares every concurrent /stats GET per account, team, and auth generation and cancels the HTTP read once its last caller leaves. The store-level task duplicated that sharing but ran unstructured, so a removed machine or a hidden panel kept its stats request alive until the 30 s timeout. The coordinator's waiter accounting also stopped seeing callers, and three VMClientReadCoalescingTests cases timed out on every main run. Stats reads now fence per caller with beginRead/finishRead, which keeps the revision and sequence ordering from #13327's store, and share the network request through the coordinator. The store's read(machineID:) and its tests go away with it; VMClientReadCoalescingTests covers sharing, cancellation, and resize invalidation at the client. This reverts commit c72f659df746cfcabf1e5ab1b92b99ec8b09b38c. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2 participants
Cloud diagnostics rejected valid placement failures and RC-client batches, then permanently dropped them on 400. Cloud reads could overlap or outlive their callers, auth refresh could exceed its original deadline, and guest installation failures could lose their rollback outcome. This master PR consolidates the scoped Cloud fixes after reconciling the current
mainbranch.Fixes #13138. Fixes #12625. Fixes #12626. Follow-up hardening for the already-merged #12624. Coverage and unresolved evidence: #13140.
Changes
Retry-After, bound cooldown memory, invalidate only affected scopes and paths, enforce access gates, and cancel or replace panel work correctly.Scale and regression evidence
The deterministic read-coordination fixture covers 1, 10, 100, and 1,000 machines with four owners and asserts one transport request per machine, independent caller deadlines, cancellation/teardown, offline rejection, bounded cooldowns, and stale-response replacement. This is normalized request/concurrency evidence for the changed path. The historical Axiom before/after numbers cannot be recomputed in this session because Query Read is denied for the supplied token; no live recurrence or latency claim is made.
Main reconciliation
The branch is clean,
origin/mainat3337293da049b7be37bc67489710986c389f54e4is an ancestor of the current head6c95ef332e497f681f45d1e530d77c31d5f7e8de, and the latest pull was a conflict-free merge. No merge, deployment, production mutation, or billing change has been performed.Evidence
bun run typecheck, Swift file budgets, PBX/project and test wiring, package grouping, package-resolved policy, andgit diff --check. No local Swift/Xcode build was run.8222beb98abbb1bccb62bdf2verified the previous pre-main-sync head and is superseded by the current main merge; a new exact-head tagged build is required after the fresh checks stabilize.Limits
Axiom Query Read remains denied with HTTP 403 for the supplied token. The source and ticket mapping is complete, but no live 24-hour, 7-day, or retention-window APL audit, production recurrence count, or runtime dogfood claim is made. Historical provider and daemon reports remain needs-repro in #13140. The coverage ledger is
/Users/austinwang/.local/state/cmux-axiom-cloud-audit/coverage.md.Review trigger
Full closeout review requested for the consolidated Cloud diagnostics, auth, read-coordination, guest-install, and cleanup changes. The current head is reviewed against
origin/main; automatic review threads are replied to and resolved. The PR stays unmerged until hosted checks and the tagged fleet build are green.Checklist
origin/mainreconciled conflict-free at3337293da0; the Cloud read, reachability, guest-install, and current main changes were preserved.