Repository navigation
test(cmux-next): run SidebarDocStore-owning live-daemon suites one at a time - #27421
Conversation
… a time AppActions.bind reads services.sidebarLayout, whose first read sets the process-wide SidebarDocStore.current. The classic upgrade simulations, ClassicImportTests, ClassicMigrationTests and FirstLaunchTests are separate top-level .serialized suites, so a Classic filter run or the daemon-test job ran them at once against that one store. A suite trait (.ownsSidebarDocStore) holds a process-wide FIFO mutex for each gated suite's whole run; ungated suites still run in parallel around them. No assertion or timing changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-cc8bff1c3ddb4ec353c5
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base c14dc66; does not block |
|
Owned by an agent session · session |
|
Bead |
|
Found 6 test failures on Blacksmith runners: Failures
|
|
More evidence of the ClassicUpgradeSimulationTests order flake this PR targets, from the classification of #27052 (which touches no upgrade or archive code):
|
|
For awareness, from #26809's red triage: at feat-cmux-next 52b0452,
Fleet step b50ae2ae6894c55e50dfbb14 (width 1); at width 6, step d2a516728beecc2e8b5c9f80 had 18 order-differs lines against 9. So serializing the suites roughly halves it, and the rest happens with no overlap at all. That fits a Agent session: agent session 🤖 Generated with Claude Code |
|
Review (codex gpt-6-astra high, next-review): SHIP at 8081190 No blocking findings. |
|
Shipped in:
|
|
Shipped in:
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Confound
AppActions.bind(services)always runsSidebarSectionHandlers.bind, which readsservices.sidebarLayout. That lazy var's first read callsSidebarDocWiring.start, andstartsets the process-wide staticSidebarDocStore.currentto that services' store.DocMachinesand placement then read the static without checking whichAppServicesowns it.These top-level suites each bind
AppActionsand run a live daemon across awaits:ClassicUpgradeSimulationTestsandClassicUpgradeHugeSimulationTests, throughClassicUpgradeSimulator.init. They are the only callers ofClassicUpgradeSimulatoron base.ClassicImportTests,ClassicMigrationTestsandFirstLaunchTests.Each is
.serializedonly within itself. Swift Testing runs top-level suites in parallel, so a--filter Classicrun runs them together. So does CI's daemon-test job (...|FirstLaunchTests|ClassicImportTests|ClassicMigrationTests|ClassicUpgradeSimulationTests). Two daemons end up writing one static. The laterbindtakes the store, and the earlier simulation places workspaces from the other run's sidebar document. That can explain some of the "order differs" failures. Saving and restoring the static around a test does not help, because both runs are mid-flight when the swap happens.Fix (tests only)
A new suite trait,
.ownsSidebarDocStore(SidebarDocOwnerGate, which followsLiveDaemonGateandWindowLeakCheck). It is non-recursive, and its scope is provided once for the suite itself. That scope holds a process-wide FIFO async mutex for the suite's whole run, which covers every test in the suite, including the huge simulation's phases that share one launch. The five suites above carry the trait. Suites without it keep running in parallel around them, so the process still carries other load.I chose a trait over nesting the suites under a
.serializedparent so that test IDs (the CI filter and per-test base-red matching use them) and file indentation don't change. #26056 edits lines insideClassicUpgradeSimulationTests, and this PR changes only that file's@Suiteline. If #26056 lands, its newClassicUpgradeSimulationPerfTestsalso drivesClassicUpgradeSimulator, so it should carry.ownsSidebarDocStoretoo.Production code (
SidebarDocWiring,SidebarDocStore,DocMachines, placement), assertions and timings are unchanged. The production fix for the race is separate work.Still exposed in full-package runs (not gated here)
97 test files call
AppActions.bind, and every one of them writesSidebarDocStore.currentthe same way. In a full-package run any of them can still overlap a gated suite. This PR covers the suites the Classic filter and the daemon-test job schedule together. Gating every binder would serialize most of the package.Measurement
Harness:
cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNextwith--arg=Classic×10 andCMUX_CI_XCODE_APP=/Applications/Xcode_26.6.app, on one worker (--target-worker), base then head back to back.Base
a1951db= stepc3c0cbc10e9976bffc67c634, head8081190= step60afea902560f174180f206b, same worker, back to back. An earlier base step7167b321dbf489be0e78bdc9on a different worker is in the last column for reference.. Test run with)✘ Test run with)is not parkedThe last row is computed from the log's
◇ Test … startedand✔/✘/━ Test … afterlines, mapping each test function to its suite.◇ Suite … startedis printed before a suite's scope trait runs, so all four suites still print "started" together. In the head log the suite durations add up one after another instead (for example Import 12.5 s, then Huge ending at 346 s, Migration at 351 s, and the simulations at 412 s).In the head runs the 14 manyGroups "order differs" lines and the Huge :68/:70 issues remain. That fits the production race (
DocMachinesreading the static without checking its owner) and not the test overlap.The isolation fix is not expected to reach zero, because the production race remains. Its purpose is a measurement where the Classic suites no longer share one sidebar document.
Agent session: agent session
🤖 Generated with Claude Code