Repository navigation
debug.tunables activate: the fleet's bench film builds can turn the tunable store on - #33553
Conversation
DebugTunables.handle and DebugTunables.state take the store they read and write (default TunableStore.shared, which the socket method and every other caller still use). A test can then drive a store of its own: other suites in the test process activate TunableStore.shared, so the shared store cannot show a store that launch left off (FRAME OWNER REFACTOR S0, the next commits).
…red)
FRAME OWNER REFACTOR S0. The fleet's film builds are Release builds with
bench hooks (CMUX_BENCH_HOOKS). Their launch never turns the tunable store
on (DebugSettingsService.start() needs DevTools: a Debug compile or a nightly
bundle), so `debug.tunables set` changes nothing there and `state` says
active false (film round px-col-drag-2 on FEAT DOGFOOD BUILD 1d8e759e). One
build then cannot film a Debug switch off and on, and the speed sweep cannot
scale the springs.
New file DebugTunablesActivateTests (a store of its own per test: other
suites turn TunableStore.shared on, and a store never turns off):
- without `activate`, a `set` changes nothing and `state` says off (passes
on the base too: benches that never call activate stay the same);
- `activate` turns the store on with no override file, and each later `set`
reaches the next read of a switch and of the `move` spring;
- a reader that tracks a tunable after `activate` is told of the next `set`;
- `activate` keeps a store that is already on (DEV, NIGHTLY) as it is.
Expected on this commit: the last three fail ("unknown action"; the value
and `active` do not change).
…e-owner-s0-bench-tunables
…h builds
FRAME OWNER REFACTOR S0. The fleet films and benches Release builds with
bench hooks (CMUX_BENCH_HOOKS). Their launch never turns the tunable store
on (DebugSettingsService.start() needs DevTools: a Debug compile or a
nightly bundle), so `debug.tunables set` changed nothing there and
`state` said active false. One build could not film a Debug switch off and
on, and the speed sweep could not scale the springs.
`debug.tunables {"action": "activate"}`: on a store that launch left off,
it registers TunableCatalog.all and calls activate(file: nil), so overrides
apply for this launch, in memory only; on a store that is already on (DEV,
NIGHTLY) it does nothing. It answers `state`. It exists only where the
method does (#if DEBUG || CMUX_BENCH_HOOKS), and nothing changes unless a
client calls it, so other benches stay the same.
Memory only: nothing is written and the next launch starts off again (the
next launch would not read a file, and a stale one could change another
owner's bench). plans/cmux-next/debug-settings.md (Gate) now says what a
`set` after `activate` reaches: per-read tunables (a switch read per
frame, MotionSpring.base) at their next read, a reader that tracks the key
after `activate`, not one that tracked it before; launch-time readers
(dragResize's synced choice, layerHosting at the next page show,
icons.family, auth.signInWindow) do not follow.
Red: DebugTunablesActivateTests at 9316d72 (feat 1a7e2c1
merged into the test commit 4ee6acb, which does not build its tests on
its own base: the RowBadgeSpinnerDensityTests base red #33377 fixed).
Green: the same suite at this commit. Command for both: cmux-ci run --class
isolated --script scripts/ci/package-test-lane.sh --arg=suite
--arg=Packages/macOS/CmuxNext --arg=DebugTunablesActivateTests --ref <sha>.
The fleet step ids and results are in the pull request body.
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
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)
Shipped in:
|
… reach (review) Review of 3be0406 (independent subagent, FIX on the doc only): - browserResize.dragResize's synced choice: Chromium is the default engine and ChromiumWarmup starts it about 3 s after launch at the first idle moment, so activate and set must come before that, not "before the first Chromium page". - A reader that tracked a key before activate never hears of a change: SidebarTunablesWatch keeps its launch snapshot (8 row values) until a relaunch (only sidebar.rowCloseSpot follows), and an ObservationStream loop that started before stays where it is. - icons.family: agent pages made after the set use it. - Readers gated on DevTools.isEnabled or wired only by DebugSettingsService.start() keep their Release behavior in a bench build: auth.signInWindow, home.chiefViewGallery, sidebar.sections.localPrototype, home.flightRecorder and its captures. Test: the `file` null check proved nothing (it is DebugSettingsService's file, which only start() sets, null on the red ref too) and is gone. It passed on the red ref, so which tests fail there does not change.
|
Found 2 test failures on Blacksmith runners: Failures
|
|
Base finding, not this PR: |
|
Review (claude-subagent): FIX at 3be0406 Independent review subagent (correctness first, then repository rules). The code was found correct: register before activate,
|
|
Base findings, not this PR:
|
…cond review) Second independent review (of 90b21bb, FIX on the doc; no code defect): - Every cmux page shows in Chromium by default (browser.defaultEngine, agent.chromium), and the launch's New Tab spare page (NewTabSparePool, 250 ms after the first frame) starts Chromium through PageWebView.startChromium, so CEFRuntime reads dragResizeAtLaunch then, normally before a socket client has sent `activate`. The "about 3 s" ChromiumWarmup window was wrong: a bench build cannot count on reaching the `synced` choice with `activate`; `debug.cef` (`state`, `trigger`) shows when Chromium started. - The first New Tab after a `set` takes the spare page made at launch, which keeps the old icon family. - An observation made before `activate` stays silent only when it reads tunables alone; one that also reads other state fires for that state and follows from then on. This commit changes only plans/cmux-next/debug-settings.md. Fleet proof of the branch (FRAME OWNER S0), command for each: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=DebugTunablesActivateTests --ref <sha> --key <lane>-<sha12> (through proof-step.py; the controller takes this script only as class light): - Red at 9316d72 (feat 1a7e2c1 merged into the test commit 4ee6acb, no fix): step 36c6e3dff7bf61a7f25ca21e (cmux10) FAILED as expected, 3 of 4 tests: `activate` answers "unknown action"; `active`, browserResize.pageBackdrop and the `move` spring do not change after a `set`; the reader that tracks after `activate` is told 0 times. withoutActivateASetChangesNothing passed, as it must on both sides. - Green at 3be0406 (the fix): step a122187239af62861d765bd1 (aws-m4pro-5), 4 of 4 passed. Green at 90b21bb (this commit's parent: the same Swift as this commit): step f1ac080a739723faf9edf6e6 (aws-m4pro-3), 4 of 4 passed. - Compile-checks (cmux-ci compile-check --app --package CmuxNext --recipe-version 2): 9316d72 passed (817e15f36602047c4f49687e), 3be0406 passed (8faf68223e4eefd3abda2a69), 90b21bb passed (997c8ae58d61ba6be177c60c).
|
Review (claude-subagent): FIX at 90b21bb Second independent review subagent (the repair of the first review's doc findings, then a fresh pass over the whole diff). Code: no defects found (register writes under the state mutex before the releasing store of
|
Third independent review (SHIP at 9145aba; these were nits): - Not every cmux page shows in Chromium: the update island and the icon picker require WebKit (transparent overlay). The doc names New Tab and the agent chats, the pages that start Chromium early. - The launch spare is taken by a New Tab, by a new workspace's first page (takeSpare) and by an existing chat of this Mac (takeConnectedSpare), not only by New Tab; each keeps the old icon family. - debug.cef's `state` and `trigger` say whether and why Chromium started; `ready_after_launch_s` says when. Only plans/cmux-next/debug-settings.md changes; the Swift is 90b21bb's.
|
Review (claude-subagent): SHIP at 8893b5a Independent review subagents, correctness first, then repository rules. Four passes:
Tests: each of the 3 tests that need |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
The fleet films and benches Release builds with bench hooks (
CMUX_BENCH_HOOKS,cmux-ci build cmux --release). Their launch never turns the tunable store on:DebugSettingsService.start()returns unlessDevTools.isEnabled(a Debug compile or a nightly bundle), soTunableStore.sharedstays off with no descriptors.debug.tunables setthen changes nothing andstatesays"active": false(film round px-col-drag-2 on FEAT DOGFOOD BUILD 1d8e759e). One build can't film a Debug switch off and on, and the speed sweep can't scale the springs.debug.tunablesgets anactivateaction. On a store that launch left off, it registersTunableCatalog.alland callsactivate(file: nil), so overrides apply for this launch, in memory only. On a store that's already on (DEV, NIGHTLY), it does nothing. It answersstate(active: true). Nothing changes unless a client calls it, and it exists only where the method does (#if DEBUG || CMUX_BENCH_HOOKS): Release, RC and NIGHTLY don't have it.A film kit calls
debug.tunables {"action": "activate"}once after launch, checksactive, thensets the stage switch ormotion.spring.<token>.availablestill reports the Debug Settings window (DevTools), so it stays false in a bench build.Why memory only: nothing is written and the next launch starts off again. A file wouldn't help (the next launch doesn't read it) and a stale one could change another owner's bench.
What a
setafteractivatereaches is now in plans/cmux-next/debug-settings.md:MotionSpring.basewhen a spring starts): its next read;activate: told of each change. One that tracked it before isn't (an off store returns before it registers the read):SidebarTunablesWatchkeeps its launch snapshot of 8 row values until a relaunch, and anObservationStreamloop that started before stays put;browserResize.dragResize'ssyncedchoice is read once, when Chromium starts. cmux pages (New Tab and agent chats included) show in Chromium by default, and the launch's New Tab spare page starts it about 250 ms after the first frame, normally before a socket client has sentactivate, so a bench build can't count on reachingsyncedthis way (debug.cefstate,triggerandready_after_launch_ssay whether, why and when Chromium started);layerHostingapplies at a page's next show; the first page that takes the launch spare (a New Tab, a new workspace's first page, an existing chat) keeps the oldicons.family;DevTools.isEnabledor wired only byDebugSettingsService.start()keep their Release behavior:auth.signInWindow,home.chiefViewGallery,sidebar.sections.localPrototype,home.flightRecorder.With no file,
loadingstays false,settledOverridegives the override and nothing is written.DebugTunables.handleand.statetake the store as a parameter (default.shared) so the test drives a store of its own; the socket method and every other caller still pass the shared one. FRAME OWNER REFACTOR stage S0 (refactor owner pixel-perfect).Testing
New suite
DebugTunablesActivateTests(its own file, a freshTunableStoreper test because other suites turn the shared store on):activate, asetchanges nothing andstatesays off (passes on the base too: benches that never callactivatestay the same);activateturns the store on, and each latersetreaches the next read ofbrowserResize.pageBackdropand of themovespring;activateis told of the nextset;activateleaves a store that's already on as it is.Command (fleet, both runs; the controller runs
package-test-lane.shonly as classlight, it refusesisolated):cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=DebugTunablesActivateTests --ref <sha> --key <lane>-<sha12>(submitted through the lead'sproof-step.py, which adds the key)36c6e3dff7bf61a7f25ca21e(cmux10) failed as expected, 3 of 4 tests:activateanswers"unknown action",activeand thepageBackdrop/movespring values don't change after aset, and the tracking reader is told 0 times.withoutActivateASetChangesNothingpassed, as it must on both sides.a122187239af62861d765bd1(aws-m4pro-5); at 90b21bb stepf1ac080a739723faf9edf6e6(aws-m4pro-3); at 9145aba step5a0eae3b8770b2e2768274e5(Blacksmith overflow, hq run 41635215511). The head 8893b5a changes only plans/cmux-next/debug-settings.md over 9145aba, so it runs the same Swift. Since the red, the Swift changed only by dropping one assertion that passed on the red ref too (fileis null there as well), so which tests fail without the fix doesn't change.cmux-ci compile-check --app --package CmuxNext --recipe-version 2: CmuxNext with its tests, the app, cmux-tui): red ref 9316d72 passed (817e15f36602047c4f49687e), 3be0406 passed (8faf68223e4eefd3abda2a69), 90b21bb passed (997c8ae58d61ba6be177c60c), 9145aba passed (d74700306ab801c4e670d90b). The test commit alone (4ee6acb, on feat 6479948) did not build its tests:38feb5580a6424b18c50a734failed only on the base redRowBadgeSpinnerDensityTests.swift:32that base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (from #33120) #33377 fixed, hence the merge of feat.Observed / Not run
Observed: the red step failing on exactly the 3 tests that need
activate, with the messages above; the green steps passing 4 of 4 at 3be0406, 90b21bb and 9145aba; the compile-checks above. In PR CI,cmux-next swift testran DebugTunablesActivateTests first and it passed; that job and the release-notary guard are red on base findings (linked in the PR comments).Not run: a live bench build round (the refactor owner's film kit calls
activatein its next round); Release/RC compiles beyond CI's (the change is inside#if DEBUG || CMUX_BENCH_HOOKS).Changelog
none
Proof
No capture: debug and bench socket method only, no UI.
Checklist
debug.tunablesis registered only on the local control router