Remember a disconnected display instead of forgetting it on sight

[ncchen99]

What & why

Fixes #93.

A disconnect is a choice about one particular display, stored by UUID — but reconcile dropped the record the moment that display showed up online again, so the choice lasted only until the next time macOS enumerated it. For a monitor that is cabled but switched off at the wall that is every boot: it enumerates as an entirely ordinary display, takes a slot in the arrangement, and the same disconnect has to be redone by hand.

reconcile now re-applies the disconnect for a record whose display is back, instead of deleting the record. Launch, relaunch, reboot and replug all arrive through the path DisplayManager already calls on every refresh, so there is no second mechanism and no new setting.

Per your note on the issue, the changed default rather than an opt-in.

The old "never disconnect on launch" promise is kept a different way:

• only displays the user disconnected themselves are ever touched;
• wouldLeaveNoActiveDisplay still refuses to take the last screen, restoreIfNoActiveDisplay is still the backstop;
• a re-apply that is refused, or that leaves the display verifiably still on screen, forgets the record exactly as before — reconcile keeps its invariant that no record exists for a display that is on screen;
• the result is read from enumeration, not from the transaction, for the reason verifyBackOnline gives: a lying success would otherwise have this retry at every refresh;
• Intel keeps the old behaviour outright, having no working disconnect to re-apply.

The Reconnect race you caught. reconnect() clears the record only once setEnabled(true) returns, and a reconfiguration callback inside that window runs refreshDisplays, and so reconcile, which would find the display online with its record still in place and switch it straight back off. reconnectInFlight — the same shape as softReconnectInFlight — names the displays a reconnect is running on, so reconcile reads intent from there rather than from a record that has not been cleared yet. restoreIfNoActiveDisplay goes through reconnect() too, so the backstop is covered by the same guard.

Re-entrancy. reapplyInFlight is per-UUID rather than a single flag: a burst of callbacks refreshes the display list several times over, and two records can legitimately be mid-re-apply at once. It is restoreInFlight's job in the same place, just not shared between unrelated displays.

reapplyOnWake is gone. It re-disconnected whatever macOS had re-enabled during sleep, which is now just one case of a display coming back online; the wake chain's own refreshDisplays runs reconcile like any other. One path instead of two.

That is a behaviour change on the wake path and not only a deletion, so stating it plainly: the chain no longer awaits the re-disconnect, and it goes from two refreshes to one. The re-apply is now started by reconcile and runs on its own, so wake returns before the display is back off, and a display macOS re-enabled later in the wake settle is caught by the reconfiguration callback's refresh rather than by that second explicit pass. It self-corrects either way, but the timing is not what it was.

How tested

Live on a MacBook Pro (M5, macOS 26.6.1) with a BenQ GW2780 over DisplayPort through a j5create JCD552 dock, using the built-in panel as the remembered display. Crisp quit for the first half so the re-apply is the only thing that could have acted; the display was re-enabled from outside Crisp with SLSConfigureDisplayEnabled, standing in for macOS bringing it back at boot:

Crisp quit → display re-enabled externally → still online at t+1s, t+2s, t+3s
Crisp launched                              → disconnected again within 1s
record kept, not dropped

Then the same with Crisp already running (the display-appears case rather than the launch case): re-enabled externally, disconnected again inside a second, record intact.

Then the ordinary things, on a build installed and running for a day: full reboot, sleep and wake, and unplugging the dock and plugging it back in. The remembered display stayed out of the arrangement across all of them, the built-in came back on undock, and the external's stored HiDPI mode was reapplied as before.

make check is green (SwiftLint strict, tests, localization keys).

Not verified live: the Reconnect race itself. Hitting the window means landing a reconfiguration callback inside reconnect's await on purpose, which I could not stage from outside the app; the guard is reasoned from the same MainActor ordering softReconnectInFlight relies on.

Checklist

• Builds locally (./scripts/release.sh v0.0.0-ci equivalent via make build)
• No new user-facing strings
• No UI change

[ncchen99]

One consequence of "forget the record if the re-apply doesn't take" that I should put on the record, because it happened to me while I was running the branch rather than in theory.

I keep two records: the built-in panel, and an Acer that is cabled over HDMI but switched off. After a session of reboots, sleeps and dock replugs, the built-in record was intact (and had its isBuiltin refreshed by the re-apply), while the Acer record was gone. The Acer's hardware side is flaky — an enable on its ID answers CGCompleteDisplayConfiguration 1001 right now — so the most likely path is that it enumerated online during a replug, the re-apply failed, and the record was dropped by design. I could not confirm it from the unified log, since this file logs nothing yet (#100 fixes that, which would have answered it outright).

So the honest shape of the tradeoff: a transient failure during a wake or replug storm silently costs the user the choice, and the symptom is exactly the annoyance the issue is about — disconnect it again. The alternative is keeping the record and retrying on the next refresh, which risks the other failure: the Reconnect row naming a display that is on screen, and your invariant gone.

I left the patch as it is, since dropping is what preserves the invariant and it matches what reconcile did before. If you would rather it survived a bad moment, a bounded retry — keep the record, try again on the next refresh, forget it only after the second failure — is a small change and I am happy to push it. Your call on which failure is the better one to have.

[ncchen99]

Following up on my own comment above: the tradeoff was worse than I described, and I've pushed a fix.

I framed dropping the record as costing the user their choice. It can cost them the display. setEnabled(false) can report an error and take anyway, and the old code read the record's fate from that result: error → forget. The display is then switched off at the window server with no record, so no Reconnect row names it, and replugging does not undo it — the window server is holding that state, not the cable. Nothing in the UI can reach it.

That is what happened to the Acer I mentioned. Full sequence, now that I have it from the reporter's side rather than by inference:

1. record present, display cabled but the cable turned out to be unplugged
2. Reconnect from the menu → fails, record correctly stays
3. HDMI plugged back in → display enumerates online → re-apply runs → record dropped
4. row gone from the menu; dock replug, cable replug and power-cycling the monitor all change nothing
5. SLSGetDisplayList still lists it, CGGetOnlineDisplayList does not: switched off, unreferenced

id 2 vendor=0x09d1 online=true   ← BenQ, the one lit screen
id 1 vendor=0x0610 online=false  ← built-in, disconnected on purpose, record intact
id 3 vendor=0x0472 online=false  ← Acer, disconnected, record gone: stranded

Recovering it needed SLSConfigureDisplayEnabled from outside the app.

The fix reads the outcome from enumeration instead of from the transaction, which is what the file already does in the other direction: still online after the attempt, refusals included → forget the record, as before, so your invariant holds; off → keep it, whatever the result said, because in that state the record is the only handle on the display.

That also answers the retry question from my last comment: no retry needed. The failure I was worried about — a transient error during a wake or replug storm — is now indistinguishable from success as far as the record is concerned, because both leave the display off, and off is the state the record exists to describe.

Worth noting reenableUnintentionallyDisabled did not catch this. It is scoped to displays outside the intentional set, which the Acer had just been dropped into, but it only runs from softReconnect's exhausted retries and stranded recovery — nothing calls it on an ordinary refresh. I am not proposing to widen it here, only recording that the safety net below this one does not cover the hole.

[didriksg]

Reviewed in full, and applied on top of #99 and #100 to check the combination builds: it does, with one trivial conflict in reconnect() where both sides add lines before setEnabled(true), your in-flight guard and my log line, and both are wanted.

The race fix is right. reconnectInFlight covers the window, reconcile skips it, reapplyRemembered re-checks on entry, and the record is cleared before the defer fires with no suspension point in between. Deciding the record's fate by enumeration rather than the return value is the right call, and your comment on why a dropped record strands the display is exactly the reasoning that belongs in this file.

One thing I would change before merging, and it comes from numbers measured today rather than from reading. After the disable, reapplyRemembered polls for 1 second before deciding. On direct-attached hardware here a disable reported success after 628 ms and an enable commit took 888 ms, so a healthy transaction already uses most of that second. This file budgets 4 seconds for the same wait in softReconnect, because a display link handshake runs 2 to 4 s. And setEnabled gives up at 10 s while the commit keeps running, which #33's capture shows is real on a hub: a 29.5 s WindowServer stall. Put those together on slow hardware and the sequence is: the transaction reports failure at 10 s, the 1 s poll sees the display still lit, the record is dropped, and then the commit lands and switches the display off with no record left. That is the stranding your own comment warns about, reached through the timeout path. Matching softReconnect's 4 s window and re-checking enumeration once more before dropping would close it. The long stall in #33 is on enable, and nobody has yet seen a disable block that long, so treat the 10 s branch as reasoned rather than reproduced; the 1 s against 2 to 4 s mismatch stands either way.

One decision I would rather see made than fall out as a side effect. If a remembered display is the only screen attached at boot, wouldLeaveNoActiveDisplay refuses, the display stays lit, and the record is dropped. A single boot in that configuration silently forgets the choice, which is the complaint #93 opened with. Keeping the invariant that the list never claims a display is off while it is on screen is defensible, but it deserves a sentence in the doc comment saying that is the intended trade.

Minor, for the description only: the wake path no longer awaits the re-disconnect and goes from two refreshes to one. It self-corrects through the reconfiguration callback, but it is a behaviour change.

Merge order on this file is #99, #100, then this, so it wants a rebase once those two land, plus the conflict above.

[ncchen99]

Rebased onto #99, #100, #106, #109 and #110. The reconnect() conflict resolved the way you described, both lines kept, your log line before the in-flight insert so a capture records the request before anything can observe it. #110 is a visible improvement from this side too: the reconnects that used to sit for seconds now land immediately, which also means the window below is reached far less often than it was.

The 1 second window. Changed, and the reasoning holds up against the file rather than just against the numbers: softReconnect budgets 4 s for the same wait and says why in place, and one second is most of a single healthy transaction on its own. It is 4 s now, plus a 2 s second look on the branch that forgets the record, since that is the direction where being wrong costs a display rather than a preference.

What I would rather say outright than have you find: that narrows the window, it does not close it. Nothing bounds the commit, and the 29.5 s in #33 is longer than any wait that belongs on a refresh path. A disable slow enough still reports failure, still finds the display lit when the looks run out, and still lands afterwards on a record that is gone.

The three cases that reach the drop branch are not equally ambiguous, though:

what happened	display still lit	dropping the record
wouldLeaveNoActiveDisplay refused	yes	safe: nothing was ever issued
disable reported success	yes	safe: the transaction is finished and it did not take
disable reported failure	yes	ambiguous: genuinely failed, or the wrapper gave up and the commit is still in WindowServer's hands

Only the third strands anything, and no length of window resolves it, because what would resolve it is knowing whether the commit is still running. The change that closes it properly is narrow: in that case only, keep the record and let the next reconcile decide. The cost is that the list can name a lit display for the length of one refresh, which is a bounded violation of your invariant instead of the permanent one it exists to prevent.

I have not pushed that, for two reasons. It is your invariant to spend. And the timeout path is reasoned rather than reproduced, exactly as you framed it: the long stall in #33 is on enable, and nothing has yet shown a disable blocking that way. Say the word and it is small. setEnabled would need to tell its wrapper timeout apart from a real CGBeginDisplayConfiguration failure, which collapse to the same .configurationFailed(.failure) today; it already measures the elapsed time that separates them, it just does not return it.

The boot-with-one-screen trade is now stated in reconcile's doc comment rather than left to fall out of the code: with the remembered display as the only screen attached, the last-screen guard refuses and a single boot in that configuration forgets the choice, which is the complaint #93 opened with, in the one arrangement where honouring it would mean booting to a black machine. Written as the trade it is, so the next person reading it does not take it for an oversight and "fix" it.

Description updated for the wake path: not just a deletion, the chain no longer awaits the re-disconnect and goes from two refreshes to one, so wake returns before the display is back off and a later re-enable is caught by the reconfiguration callback instead of by that second explicit pass.

Pushed as a third commit rather than squashed into the other two, so the response to the review reads on its own.

make check green on the rebase.

[didriksg]

Read the rebase and the third commit, and built it here: compile, strict lint and the x86_64 typecheck are green on 13f69db. The 4 s window plus the 2 s second look reads right, and the boot trade is better stated in the doc comment than it would have been in a review thread.

One thing the rebase could not have known about, since #109 landed after your branch was written. disconnect() on main now snapshots the other displays' modes before the disable and puts them back afterwards, because WindowServer applies its stored arrangement for the smaller display set the moment a display is disabled (#108: my other external dropped from 2560x1440 at 165 to 1920x1080 at 60 and the built-in changed scale). reapplyRemembered calls setEnabled(false) directly, so a re-applied disconnect at boot, relaunch, wake or replug goes through that drop with nothing to put the modes back, and at boot it would happen every time. The fix is the two calls disconnect() uses: currentModes(excluding: liveID) before the disable, restoreModes after it.

On the ambiguous branch: yes, for the timeout only. Give setEnabled a distinct case for the wrapper giving up (it already has the elapsed time) and keep the record in that one case. A real failure still drops it, so a display whose disable genuinely keeps failing cannot pull a fresh transaction out of every refresh. The list naming a lit display for one refresh is a cost I will take over a stranded display.

I will run it on my desk once those are in: disconnect, relaunch, wake, with the other displays' modes checked each time. With one screen attached here right now the re-apply refuses by design, so that waits for the next time the lid is open.

[ncchen99]

Both in, as 48ad7c8.

The modes. reapplyRemembered now takes the same snapshot and starts the same restore as disconnect(). One deviation from the literal instruction, which I would rather flag than bury: the restore is started immediately after setEnabled returns, not after the verify. The verify can hold for up to six seconds on the slow path, and restoreModes polls rather than sleeping precisely so the flip is as short as it can be — waiting for the verify would hand the user those six seconds at the wrong resolution. It only ever acts on a display that actually moved, so starting it after an attempt that turns out not to have taken costs nothing.

The ambiguous branch, timeout only, as you specified. setEnabled's wrapper now returns a distinct ToggleError.timedOut instead of collapsing into configurationFailed(.failure), and that is the one case where the record survives for the next refresh to decide. Every other failure drops it, so a display that genuinely cannot be switched off still cannot pull a fresh transaction out of every refresh. New case means a new localized string; added to the catalog by hand rather than letting Xcode rewrite it.

Run here, since you are down to one screen: BenQ GW2780 over DisplayPort through the dock, built-in as the remembered display. Crisp quit, the built-in re-enabled from outside with SLSConfigureDisplayEnabled so the re-apply is the only thing that can act, then launched:

built-in enabled externally, Crisp not running
  id 3 BenQ     ON   1600x900 @60  (px 3200x1800, modeID 94)
  id 1 built-in ON   1512x982 @120

Crisp launched
  display: disable 1: reported success after 573 ms
  id 3 BenQ     ON   1600x900 @60  (px 3200x1800, modeID 94)
  id 1 built-in off

BenQ sampled every 0.8 s for 11 s across the whole re-apply: modeID 94 throughout.
Both records intact afterwards, isBuiltin refreshed on the built-in's.

So: the re-apply path works and the restore is wired into it, but I did not reproduce #108 on this pair — WindowServer never moved the BenQ when the built-in went off, so restoreModes found nothing to put back and correctly did nothing. The restoring branch itself is unexercised here. Yours is the setup that produces the move, so that half still wants your desk rather than mine.

The timeout branch is unreproduced too, for the same reason it was reasoned rather than measured in your review: nothing here makes a disable block. With #110 in, disables on this hardware report success in about 570 ms.

make check green.

[ncchen99]

A measured number for the timeout branch, which we both had down as reasoned rather than reproduced. It turned up while I was chasing the count question on #92, so it is from the wake path rather than from a hub.

Two runs of the same sequence — built-in held disconnected, two externals, sleep, pull the dock while asleep, wake — and the same operation, setEnabled(true) on the built-in from the blackout rescue:

run 1, issued in DarkWake (before the user's keypress reached full wake)
  19:59:50.992  rescue decides
  20:00:09.703  slow enable 1: commit took 6690 ms
  20:00:10.443  enable 1: reported success after 7430 ms

run 2, issued after full wake
  20:10:09.340  rescue decides
  20:10:09.803  enable 1: reported success after 463 ms

Same machine, same display, sixteen times the duration. CGCompleteDisplayConfiguration blocked for 6.7 s because the transaction was issued while the machine was in DarkWake with the panel still down; it did not complete until the spacebar took the system to full wake. That is 7.4 s against the wrapper's 10 s, on ordinary hardware with nothing wrong, and the gap that produced it is under the user's thumb — a couple more seconds before they touch a key and it crosses.

Two caveats. It is an enable, not a disable, so it is not direct evidence for the branch we changed. And the rescue is what issued it, not reapplyRemembered. But it is the same wrapper over the same call, and the wake path is exactly where reapplyRemembered runs: a display that comes back at wake gets its re-apply while WindowServer is still settling, which is the condition that produced this. I would treat the 10 s timeout as reachable in practice now rather than theoretical, which makes the timedOut case earning its keep more likely than I assumed when I wrote it.

Nothing to change in the PR from this — the handling is already what you specified. Recording it because you asked for the 10 s branch to be treated as reasoned, and it now has a number next to it.

Also, for whenever you get the lid open: the mode restore in reapplyRemembered is still unexercised. I got #108 to reproduce here on the manual path — display 2 moved to 756x1344 after the disconnect, restoring 900x1600 @120: ok, so #109 works on this hardware — but I could not provoke the same move through the re-apply path in three attempts with two externals plus the built-in. Plausibly because that first restore left WindowServer's stored arrangement for the smaller set already correct, so there is nothing left to shuffle. The code path is shared, but the restoring branch has still only been seen working from disconnect().