#28275 fix(quit): state files have one owner each and no quit blocks the main thread on them

Summary

This PR fixes the arch-ratchet tip rises of #27749 and #25535. These state files flush at quit and had no reviewed exception:

• The statics move to instances. AppServices now owns one AgentPaneStateFiles registry and gives it to AgentUnsentDrafts and AgentPaneScrollAnchors. The registry rule is the same as before: one writer per path. The paired-phones file (MobileHostPairedDevicesFile) is now a class. IPhoneHostService owns one instance per state directory, and its queue and the record of the list in force are members of that instance.

• No quit blocks the main thread on these files. This PR removes all three allowBlocking(.terminate) flushes. The quit awaits the files with AppServices.stateFilesDrained(). The wait holds no thread and has no limit of its own; the quit awaits it only after its deadline is armed:

  • Forced quit (a second quit signal): before, ForcedQuitExit prepare ran its flush before the 160 ms deadline was armed, so it could wait 2 s. Now the exit arms the deadline first. Then, in save, it waits for the state files and for the Home cache write at the same time.

  • Normal quit: before, applicationWillTerminate and IPhoneHostService.stop() could wait 2 s each, so 4 s in all. Now QuitCoordinator's engine-stop step does this, in order:

    1. It arms QuitDeadline.engineStopLimit (15 s).
    2. It drains the files.
    3. It closes the engines (closeEngines, a seam for tests).
    4. It drains the files again.

    The second drain catches a draft that a closing page saves (the composer's page-hide flush). The prepareWindows drain and the bare 2 s limit are gone (R9 review, R-L1).

• A lost draft is named and told (R9 review, R-L5). If a quit deadline ends the process while a state file is still saving, the deadline's end (AgentPaneStateLoss.deadlineEnd) does two things:

  • It logs a .fault with the file names only, never content.
  • It records the names in <state>/agent-state-lost.json.

  The next launch reads and deletes that record. If a new chat's unsent text was lost, it shows a toast: "Text typed in a new chat just before cmux quit was not saved." The string is localized in 21 locales. Both deadline reasons now name the state files. Before this PR, such a loss was silent. The paired-phones list is in the loss record too, and it fails closed, but the deadline's end never calls the Keychain. A hung securityd, the case QuitDeadline exists for, must not hold the process's end.

  • MobileHostPairedDevicesFile counts its saves and withdrawals that are queued or running.
  • The iPhone host owns a MobileHostPairedQuitWatch that gives the deadline's end its current file from any thread.
  • If a save is still in flight when the deadline fires, the end writes a marker (paired-devices.cut-off, beside the file), names the file in the fault and the record, and ends.
  • If the marker cannot be written (a full disk), the end logs a fault. The record then names the file with the note paired-devices.cut-off (not written).
  • At the next launch, the new file object fails closed when the marker exists OR the last quit's record names the file. It checks this first on its own queue, before any read. The launch reads the record once (AgentPaneStateLoss.lastQuitNames()), so this check and the notice share the same names in either order. Nothing stored counts in that run. It clears the Keychain seal, so the stored list no longer matches its seal and every phone must pair again (the digest rule of load). A removed phone does not get its access back.
  • The fact must stay on disk until the seal is cleared:
    • The record keeps naming the paired list after the launch reads it. It names only that list, so a lost draft is told once.
    • A quit deadline in that run writes the paired name beside its own names.
    • The paired file tells the record (pairedCutOffSettled()) once the fact is safe without it: its marker is on disk, or its seal is cleared.
  • When only the record named the save, the launch writes the marker BEFORE the clear. A failed write is a fault of its own, and the record keeps the fact.
  • The clear counts as a save in flight, so a quit deadline during a hung clear marks and names the file again.
  • If the Keychain refuses the clear, the list stays refused for that run and a fault is logged. The fault names what keeps the fact for the next launch: the marker, or the record when no marker could be written.
  • The second toast, "A change to your paired iPhones was cut off when cmux quit. Pair your iPhones again." (21 locales), shows only when that launch actually cleared the seal.
  • The unsent-draft toast shows at once. The pair-again toast comes when its own Keychain check answers, so a slow Keychain never holds the draft toast back (showNotices).
  • AppServices.quitDeadlineEnd() builds the one deadline end that both quits use.

• One path for the drain. QueueDrain is deleted. Each file awaits its own serial queue (AgentPaneStateFile.saved()), so the drain needs no timer and no race.

• Testable quit steps: QuitCoordinator.steps() builds the QuitSteps that complete runs. deadline and closeEngines are replaceable, so a test runs the coordinator's own steps without ending its process or starting Chromium.

• Line budget: SignalRelay moves from CmuxNextApp/Quit to CmuxNextControl/Diagnostics, next to ForcedQuitExit and QuitDeadline. The loss record and its notice live in CmuxNextAgentPane. With this, CmuxNextApp's line count does not rise (+0).

Sites fixed (paths under Packages/macOS/CmuxNext/Sources/)

Row	Site	Added by
global_mutable_statics	CmuxNextAgentPane/AgentPaneStateFile.swift:31 static let files = Mutex<...>	d02ea1752d3 (#27749)
global_mutable_statics	CmuxNextMobile/MobileHost/MobileHostPairedDevices.swift:188 static let queue	06d4a464f17 (#25535)
global_mutable_statics	CmuxNextMobile/MobileHost/MobileHostPairedDevices.swift:197 static let inForce	06d4a464f17 (#25535)
blocking_allow_escapes	CmuxNextApp/AppDelegate.swift:42 flush in ForcedQuitExit prepare	d02ea1752d3 (#27749)
blocking_allow_escapes	CmuxNextApp/AppDelegate.swift:514 flush in applicationWillTerminate	d02ea1752d3 (#27749)
blocking_allow_escapes	CmuxNextMobileHost/IPhoneHostService.swift:387 flush in stop()	06d4a464f17 (#25535)

AppDelegate.swift:514 was first listed as an exception candidate. An independent exception review then asked for it to be fixed with an async wait in the .terminateLater phase, so this PR fixes it too.

Ratchet delta (arch_counts.py --delta <merge-base 5a3b7e1cef4> HEAD)

global_mutable_statics             -3 {'CmuxNextAgentPane': -1, 'CmuxNextMobile': -2}
blocking_allow_escapes             -3 {'CmuxNextApp': -2, 'CmuxNextMobileHost': -1}
app_lines                          +0

All other rows: +0. arch-ratchet.py --base origin/feat-cmux-next --head HEAD says "no architecture count rises". This PR does not change arch-ratchet.json (--update was not run).

Testing

No clock decides any quit test. A test helper, DrainGate, holds a state file's queue with a save, and only the start of a drain lets that save go:

• AgentPaneStateFiles(drainBegan:) is an injected init hook, not a static. The drain calls it as it begins, and the gate releases the held save from it (or from the test's own defer, after its checks).
• So a quit step that never drains never sees the draft written, on any host. A step that drains always does.
• The gate also records the order of the quit steps.

The quit tests build their services through one helper, QuitStateFilesTests.services. It makes NSApplication.shared first, answers no key window, and puts in a QuitDeadline whose end does nothing. A test process has no app object; the root fix of that trap is its own PR, fix/arch-rise-appservices-no-nsapp-trap.

• QuitStateFilesTests.theForcedQuitArmsItsDeadlineBeforeItWaitsForTheStateFiles: the step order is ["deadline armed", "drain began"], and the draft is on disk when the forced exit ends.
• QuitStateFilesTests.theQuitAwaitsAQueuedDraftWithoutBlockingTheMainThread: the main actor releases the held save while the quit waits. A wait that blocked the main thread would never end, and .timeLimit(.minutes(1)) fails the test.
• QuitStepsStateFilesTests runs QuitCoordinator's own stopBrowserEngines:
  • theEnginesCloseAfterTheQueuedDraftIsOnDisk: two drains happen, and the draft is on disk when the engines close.
  • aDraftSavedWhileTheEnginesCloseIsOnDiskWhenTheQuitEnds: a draft queued during the engines' close is on disk when the step returns.
• AgentPaneStateLossTests:
  • The deadline's end records the files still saving, and the next launch takes the record once.
  • A cut-off paired list is recorded, and its notice shows only when the seal was cleared.
  • Nothing still saving records nothing.
  • A lost scroll place gets no notice.
• MobileHostPairingTests.aRemovalCutOffByAQuitDeadlineIsNotTrustedAfterARelaunch: a removal's save is held at the seal, before its own clear, when the deadline's end runs. The test checks four things:
  • The end names the paired file.
  • The end makes zero seal calls. The test seal counts every call of digest, setDigest and clearDigest.
  • The next launch clears the seal before its first read.
  • That read trusts no phone.
• MobileHostPairingTests.aCutOffRemovalWithoutItsMarkerIsNotTrustedAfterARelaunch: an injected disk refuses the marker. The record names the file with the note, no marker is on disk, and the next launch, told only by the record, trusts no phone and clears the seal.
• MobileHostPairingTests.aRecordOnlyLaunchWhoseClearHangsStillLeavesTheListUntrusted: a launch told only by the record has its seal clear held. A quit deadline during that clear finds .marked, and the launch after it, with no record, trusts no phone.
• MobileHostPairingTests.aRecordOnlyLaunchWithNoMarkerAndARefusedClearKeepsTheRecord: a launch with no marker (the disk refuses it) and a refused clear never tells the record the fact is safe, and the launch after it still trusts no phone.
• AgentPaneStateLossTests.theRecordKeepsNamingACutOffPairedListUntilItIsSettled: after the launch reads the record, it names only the paired list. A deadline in that run keeps that name beside its own, and only settling removes it.
• MobileHostPairingTests.aRemovalTakesTheOldDigestAwayAfterTheSavesBeforeIt and aRemovalNeverWaitsOnTheCallersThread now release their held save on a signal, after the removal is queued, not after 0.3 s or 0.6 s. The second no longer measures time: a removal that waited on the caller's thread would never return, and its time limit fails the test.
• AgentPaneStateLossTests.thePairedCheckAndTheNoticeShareOneReadOfTheRecord: the notice reads the record first, and the paired check still sees it.
• QuitStateFilesTests.theLossRecordNamesThePairedListByItsFileName: the record's paired-phones name matches the file's own name.
• AgentPaneStateFileTests: a file counts as saving until its saves ran; drained() waits for queued saves and holds no thread.
• MobileHostPairingTests: one file object per run. GatedSeal became HeldSeal, which holds the same file's queue.
• The tests in ChatScrollRestore*, UnsentDraftTests and RestoredNewTabPageTitleTests build their own registry.

Observed / Not run

• Observed: prepush-gate.py reports no new violations. arch-ratchet.py reports no rises. lint-ios-package-conventions.sh --namespace-fix reports no violations. localization_catalog.py check reports nothing for the new key.
• Earlier failed fleet runs (not evidence):
  • Compile a1714da97296c3cf467069ff failed on a test isolation error, fixed in 57dfaa1b9ce.
  • Green 04224ccd89106f08f50cc908 at 07f1b957df7 crashed on NSApp.keyWindow.
  • Green eb20257db2553e2e4a4dd378 at 57dfaa1b9ce crashed on NSApp.isActive.
  • Both crashes are fixed by NSApplication.shared in the test helper.
• Compile 6c6043c82f095749e151f624 at c85dcca0031 failed: a public method named an internally imported type (CmuxToastCenter). Fixed in 96a1de8f7c3.
• Compile f064b3161ab4caa8918617ec at 96a1de8f7c3 passed. The R9 re-review then found the Keychain call on the deadline thread, fixed in 67afa2c83a3. The steps queued at 96a1de8f7c3 were cancelled.
• Compile 8400b1b5d3dcc4763a67535a at 67afa2c83a3 passed. The R9 review then found two silent-loss cases (a failed marker write, and the draft toast waiting on the Keychain), fixed in fc54befe74d. The proof steps at 67afa2c83a3 were cancelled.
• Compile c822c59f4aaedeabf6bb007c at fc54befe74d failed: a public init's default argument called an internally imported function (blockingWrite). Fixed in e22c234afc2. I checked every public declaration this PR adds or changes for default arguments or types from internal imports, and found no other case.
• Compile 8d21112a25c2cd9435417c71 at e22c234afc2 passed. The R9 review then found that the record-only path could still trust a cut-off removal: a hung clear, then a second end, left no record and no marker. Fixed in d6f29ce775d. The proof steps at e22c234afc2 were cancelled.
• Compile dcbec3f24cd1d802ceb3bd20 at d6f29ce775d passed. Proof results at d6f29ce775d. They are evidence for that head only, not for the current head:
  • Green ce744edd3717933ee9dfd3c6 passed. The four suite runs had 3, 2, 5 and 29 tests, and the build completed.
  • Red 97a826e11fa23297d8fc74da (on fix/arch-rise-quit-flush-redproof-6) failed on exactly the 9 expected tests, each on its assertions, with no crash:
    • theForcedQuitArmsItsDeadlineBeforeItWaitsForTheStateFiles
    • both QuitStepsStateFilesTests tests
    • theDeadlineRecordsTheFilesStillSavingAndTheNextLaunchTakesThem
    • aCutOffPairedListIsRecordedAndTold
    • thePairedCheckAndTheNoticeShareOneReadOfTheRecord
    • aCutOffRemovalWithoutItsMarkerIsNotTrustedAfterARelaunch
    • aRemovalCutOffByAQuitDeadlineIsNotTrustedAfterARelaunch
    • aRecordOnlyLaunchWhoseClearHangsStillLeavesTheListUntrusted The R9 review then found that the record was deleted at the launch's first read, so a failed marker write at launch plus a refused or hung clear could still lose the fact. Fixed in 626d97e7958, with the two timed tests made signal-driven.
• Not run yet: the compile of this head, and the fleet red (on fix/arch-rise-quit-flush-redproof-7) and green of QuitStateFilesTests, QuitStepsStateFilesTests, AgentPaneStateLossTests and MobileHostPairingTests.

Changelog

• Changed: quitting cmux never blocks on saving a new chat's unsent text or the paired iPhones. If a quit ends before they are saved, the next launch says so, and a cut-off iPhone change makes every iPhone pair again.

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

---

Summary by cubic

Fixes the arch-ratchet tip rises for the agent-pane state files (cmux#27749) and the paired-phones file (cmux#25535): each file now has one owner, and no quit blocks the main thread waiting on them.

• Replaces the process-global statics with instances: AppServices owns one AgentPaneStateFiles registry, and IPhoneHostService owns one MobileHostPairedDevicesFile per state directory, keeping the one-writer-per-path rule.
• Removes all three allowBlocking(.terminate) flushes. The forced quit arms its 160 ms deadline before awaiting the files; QuitCoordinator's engine stop arms its 15 s deadline, drains the files, closes the engines, and drains again so a closing page's saved draft is on disk. The first drain is bounded by the deadline's time left less the reply margin and the engine close's wait, so it can't starve CefShutdown; an overrun runs beside the close and is awaited before the quit ends. The drains hold no thread.
• A quit deadline that ends the process while a file still saves logs a fault (names only) and records them in <state>/agent-state-lost.json; the next launch toasts a lost unsent draft.
• A cut-off paired-phones save fails closed: the deadline's end writes only a marker and never calls the Keychain, so a hung securityd cannot hold the process's end; the next launch clears the seal before its first read so no stored list counts after relaunch, and earns its own toast. Both toasts localize in 21 locales.
• When only the record names the cut-off save, the launch writes the marker before clearing, and the record keeps naming the list until the marker is on disk or the seal is cleared, so a refused or hung clear can't lose the fact. The launch's clear counts as a save in flight, so a deadline during a hung clear marks it again.
• QuitCoordinator builds its steps via quitSteps() with a replaceable closeEngines seam, so the quit tests run the coordinator's own engine stop with a queued draft.
• Moves SignalRelay to CmuxNextControl/Diagnostics to keep CmuxNextApp's line count from rising.
• The quit tests hold a file's queue with a DrainGate that the registry's drainBegan hook releases when a drain starts, so no clock decides them. The test AppServices initializer takes the state files so the tests can hold one, and their services answer no key window because a test process has no NSApp. Draft, scroll-restore, and title tests build their own AgentPaneStateFiles, sharing one per test so a fresh instance reads after its queued saves, as the static registry made it do.

Takes back #27749: its quit flush blocked the main thread twice (allowBlocking(.terminate) { AgentPaneStateFile.flushAll() } in ForcedQuitExit prepare before the 160 ms forced deadline was armed, and again in applicationWillTerminate) with no reviewed arch-ratchet exception (reward-hacking audit, arch-ratchet tip rises). The drafts are still saved. The quit now awaits the same files after its deadline is armed, with the main thread free. A draft that the deadline cuts off is recorded and told at the next launch.

No capture: the one new UI is an existing toast (CmuxToast) with new text. It shows only at the launch after a quit deadline ended the process while a draft was still saving, and a capture tour cannot reach that state without killing a quit mid-save.