diff --git a/apps/desktop/main/window-kernel-executor.test.ts b/apps/desktop/main/window-kernel-executor.test.ts index ed8523fa..77b453b9 100644 --- a/apps/desktop/main/window-kernel-executor.test.ts +++ b/apps/desktop/main/window-kernel-executor.test.ts @@ -213,10 +213,20 @@ describe('the stacking re-assert loop', () => { [wid(1), wid(2)], ); - // A show does not reorder (§0.1.10), so it re-asserts nothing. + // A show does not reorder `order` (§0.1.10), but it DOES re-trigger the + // full re-assert: a window becoming visible is itself a reason to replay + // every visible window's level, because the native always-on-top ctor + // flag is unreliable on its own and a window is registered `visible: + // false` — before that flag can be trusted to have stuck — so the show is + // the first point this kernel can reinforce it (`window-kernel.ts + // react`'s `justBecameVisible`). backend.calls.length = 0; exec.dispatch({ type: 'WindowShown', id: wid(3) }); - assert.deepStrictEqual(backend.of('setStacking'), []); + assert.deepStrictEqual( + backend.of('setStacking').map((c) => c.args[0]), + [wid(1), wid(2), wid(3)], + 'the show reinforces the pin, not just an order change', + ); // The next order change replays all three, bottom -> top. backend.calls.length = 0; diff --git a/apps/desktop/main/window-kernel.test.ts b/apps/desktop/main/window-kernel.test.ts index ad64e194..a32b60a6 100644 --- a/apps/desktop/main/window-kernel.test.ts +++ b/apps/desktop/main/window-kernel.test.ts @@ -902,6 +902,121 @@ describe('window-kernel: orderable gate on SetStacking', () => { }); }); +// ── a window becoming visible re-asserts the pin (§0.1.6) ─────────────────── +// +// The macOS ctor-time always-on-top flag is unreliable on its own, which is +// why the old machine re-asserted it after construction. The kernel folds +// always-on-top into `layer` and realizes it only through `SetStacking` +// (`stackingLevelForLayer` in the executor), so a window that becomes visible +// without reordering `order` — a NON-ACTIVATING show: session restore, the +// inactive show path in `windows.ts`, an extension window — must still +// re-trigger the full stacking re-assert, or a pinned window shown that way +// never gets its level reinforced. + +describe('window-kernel: a window becoming visible re-asserts stacking, not just an order change', () => { + it('a non-activating WindowShown alone re-asserts stacking for an always-on-top window', () => { + // `kind: 'browser'` starts orderable, so a plain show is the only event + // needed — no `OsFocusGained`, no reorder, nothing "activating" about it. + const state = fold( + [{ type: 'WindowRegistered', id: wid(1), kind: 'browser', layer: BASE_LAYER + 1, canBecomeKey: false, acceptsInput: false, class: 'hud' }], + initialState(), + ); + assert.strictEqual(state.order[0].visible, false, 'precondition: registered, not yet shown'); + assert.strictEqual(state.order[0].orderable, true, 'precondition: a browser-kind window starts orderable'); + + const ev: Event = { type: 'WindowShown', id: wid(1) }; + const shown = evolve(state, ev); + const effects = react(state, ev, shown); + + assert.deepStrictEqual( + effects.filter((e) => e.type === 'SetStacking'), + [{ type: 'SetStacking', id: wid(1), layer: BASE_LAYER + 1 }], + 'the show alone must reinforce the pin — no raise, no reorder, nothing else in this transition would', + ); + }); + + it('an AppActivated re-show (a visibility flip with no reorder) also re-asserts stacking for what it reveals', () => { + // The other production path this closes: `evolve`'s `AppActivated` arm + // patches `visible` in place on already-hidden records — same array + // positions, so `orderChanged` alone (the pre-existing gate) reads false. + const hud: Event = { + type: 'WindowRegistered', + id: wid(1), + kind: 'browser', + layer: BASE_LAYER + 1, + canBecomeKey: false, + acceptsInput: false, + class: 'hud', + }; + const swept = fold([hud, { type: 'WindowShown', id: wid(1) }, { type: 'AppResigned' }, { type: 'AppResignSettled' }], initialState()); + assert.strictEqual(swept.order[0].visible, false, 'precondition: the sweep hid it'); + + const activateEv: Event = { type: 'AppActivated' }; + const activated = evolve(swept, activateEv); + const effects = react(swept, activateEv, activated); + + assert.ok( + effects.some((e) => e.type === 'SetStacking' && e.id === wid(1)), + 'the reveal must reinforce the pin, exactly as a fresh show does', + ); + }); + + it('a plain WindowShown on a BASE_LAYER (non-pinned) window still re-asserts stacking — the trigger is visibility, not layer', () => { + // The gate is deliberately blind to WHY a window became visible or what + // its layer is — `react` never reads `role`/`class`, and restricting the + // trigger to pinned windows would be exactly that kind of decision. The + // full-stack replay this loop already performs on any order change is + // unaffected either way. + const state = fold( + [{ type: 'WindowRegistered', id: wid(1), kind: 'browser', layer: BASE_LAYER, canBecomeKey: true, acceptsInput: true, class: 'primary' }], + initialState(), + ); + const ev: Event = { type: 'WindowShown', id: wid(1) }; + const shown = evolve(state, ev); + const effects = react(state, ev, shown); + + assert.deepStrictEqual( + effects.filter((e) => e.type === 'SetStacking'), + [{ type: 'SetStacking', id: wid(1), layer: BASE_LAYER }], + ); + }); + + it('a window becoming visible before it is orderable does not trigger the re-assert — nothing could be positioned yet', () => { + const state = fold( + [{ type: 'WindowRegistered', id: wid(1), kind: 'page-host', layer: BASE_LAYER + 1, canBecomeKey: false, acceptsInput: false, class: 'hud' }], + initialState(), + ); + assert.strictEqual(state.order[0].orderable, false, 'precondition: a fresh page-host starts non-orderable'); + + const ev: Event = { type: 'WindowShown', id: wid(1) }; + const shown = evolve(state, ev); + const effects = react(state, ev, shown); + + assert.deepStrictEqual(effects.filter((e) => e.type === 'SetStacking'), []); + }); + + it('does not disturb the op-token biconditional — SetStacking carries no opSeq and is not part of it', () => { + // `window-kernel-properties.test.ts checkOpSeqInvariants`: opSeqCounter + // advances by exactly 1 iff the transition emits a ShowWindow/RaiseWindow. + // This show emits exactly one ShowWindow (the pre-existing visibility + // diff) plus the new SetStacking this describe pins — one token, not two. + const state = fold( + [{ type: 'WindowRegistered', id: wid(1), kind: 'browser', layer: BASE_LAYER + 1, canBecomeKey: false, acceptsInput: false, class: 'hud' }], + initialState(), + ); + const ev: Event = { type: 'WindowShown', id: wid(1) }; + const shown = evolve(state, ev); + const effects = react(state, ev, shown); + + const showWindowEffects = effects.filter((e) => e.type === 'ShowWindow'); + const setStackingEffects = effects.filter((e) => e.type === 'SetStacking'); + assert.strictEqual(showWindowEffects.length, 1, 'sanity: the show still emits exactly one ShowWindow'); + assert.strictEqual(setStackingEffects.length, 1, 'sanity: and exactly one SetStacking alongside it'); + assert.ok(!('opSeq' in setStackingEffects[0]), 'SetStacking carries no opSeq — it is not focus-family'); + assert.strictEqual(shown.opSeqCounter, state.opSeqCounter + 1, 'one advance, for the ShowWindow alone'); + }); +}); + // ── closing the key window ────────────────────────────────────────────────── describe('window-kernel: closing the key window raises nothing in its place', () => { @@ -2410,26 +2525,38 @@ describe('window-kernel: app activation, resignation, and the dance guard', () = { type: 'OsFocusGained', id: wid(1) }, ]; - it('resign then activate with no settle in between hides nothing and leaves nothing armed (the dance case)', () => { + it('resign then activate with no settle in between (the dance case) flickers the HUD but leaves it visible again', () => { const before = fold(registerPrimaryAndHud(), initialState()); const resigned = evolve(before, { type: 'AppResigned' }); assert.strictEqual(resigned.pendingAppResignSweep, true); + // The HUD hides on the raw edge — it is reversible visibility, not a + // destructive dismissal, so unlike a transient it does not wait for the + // settle to find out whether this resign is real or the dance cancels it. + assert.strictEqual(resigned.order.find((w) => w.id === wid(2))?.visible, false); + assert.strictEqual(resigned.order.find((w) => w.id === wid(2))?.hiddenByAppBlur, true); + // The key-holding window is never sweep-eligible — untouched going in. + assert.strictEqual(resigned.order.find((w) => w.id === wid(1))?.visible, true); + const activated = evolve(resigned, { type: 'AppActivated' }); assert.strictEqual(activated.pendingAppResignSweep, false); assert.strictEqual(activated.appActive, true); - // Nothing was hidden, so nothing carries the mark, and both windows are - // exactly as visible as they started. - for (const w of activated.order) { - assert.strictEqual(w.hiddenByAppBlur, undefined); - } + // The dance reverses the hide: both windows end up exactly as visible as + // they started. Only the HUD ever carried the mark, so only its mark is + // cleared — the primary window's stays absent, never having been set. + assert.strictEqual(activated.order.find((w) => w.id === wid(1))?.hiddenByAppBlur, undefined); + assert.strictEqual(activated.order.find((w) => w.id === wid(2))?.hiddenByAppBlur, false); assert.strictEqual(activated.order.find((w) => w.id === wid(1))?.visible, true); assert.strictEqual(activated.order.find((w) => w.id === wid(2))?.visible, true); }); - it('resign then settle hides the sweep-eligible (non-key-holding) window and marks it hiddenByAppBlur', () => { + it('the sweep-eligible (non-key-holding) window is still hidden and marked once resign and settle have both run', () => { const before = fold(registerPrimaryAndHud(), initialState()); const resigned = evolve(before, { type: 'AppResigned' }); + // The raw edge already did the hiding — settle is a backstop here, not + // the primary mechanism (see `evolve`'s `AppResigned`/`AppResignSettled` + // arms). + assert.strictEqual(resigned.order.find((w) => w.id === wid(2))?.visible, false); const settled = evolve(resigned, { type: 'AppResignSettled' }); assert.strictEqual(settled.pendingAppResignSweep, false); @@ -2607,6 +2734,28 @@ describe('window-kernel: which windows a real app resign hides', () => { assert.strictEqual(settled.order.find((w) => w.id === alreadyHiddenHud)?.hiddenByAppBlur, undefined); }); + it('the HUD hides on the RAW resign edge, before any settle — it does not linger over the app the user switched to', () => { + // The old reducer hid `class === 'hud'` immediately on APP_RESIGNED. + // Waiting for the 500ms settle (as the settled-only sweep used to) + // leaves a HUD floating over whatever app the user just switched to for + // the whole of that delay — the same cost an attached overlay would pay, + // which is why both are hidden here, on the same edge, for the same + // reason: `layer > BASE_LAYER` puts either kind of window above OTHER + // applications' windows too, not just above Peek's own stack. + const resigned = evolve(scene(), { type: 'AppResigned' }); + const swept = sweptBy(resigned); + + assert.deepStrictEqual(swept, [hud]); + assert.strictEqual(resigned.order.find((w) => w.id === hud)?.visible, false); + assert.strictEqual(resigned.order.find((w) => w.id === hud)?.hiddenByAppBlur, true); + + // Nothing else moved — same exclusions as the settled sweep, checked + // directly on the raw edge rather than after settle folds in behind it. + for (const id of [workspace, spaceBorder, testFixture, cmdPanel, quickView]) { + assert.strictEqual(resigned.order.find((w) => w.id === id)?.visible, true, `${id} must not be hidden by the raw-edge hide`); + } + }); + it('both halves of the HUD pair are load-bearing — neither alone selects the HUD', () => { const state = scene(); const rec = (id: WindowId) => state.order.find((w) => w.id === id)!; @@ -2641,16 +2790,27 @@ describe('window-kernel: which windows a real app resign hides', () => { assert.deepStrictEqual(sweptBy(activated), []); }); - it('the sweep emits a HideWindow effect for exactly the windows it hid', () => { - const resigned = evolve(scene(), { type: 'AppResigned' }); - const settleEv: Event = { type: 'AppResignSettled' }; - const settled = evolve(resigned, settleEv); - const effects = react(resigned, settleEv, settled); + it('the raw resign edge emits a HideWindow effect for exactly the windows it hides — the settle that follows emits nothing, since react has no diff left to project', () => { + const before = scene(); + const resignEv: Event = { type: 'AppResigned' }; + const resigned = evolve(before, resignEv); + const resignEffects = react(before, resignEv, resigned); assert.deepStrictEqual( - effects.filter((e) => e.type === 'HideWindow').map((e) => e.id), + resignEffects.filter((e) => e.type === 'HideWindow').map((e) => e.id), [hud], ); + + // The settle step that follows finds `hud` already `visible: false`, so + // its own re-application of `isAppResignSweepEligible` is a no-op and + // `react`'s structural diff has nothing to project a second time. + const settleEv: Event = { type: 'AppResignSettled' }; + const settled = evolve(resigned, settleEv); + const settleEffects = react(resigned, settleEv, settled); + assert.deepStrictEqual( + settleEffects.filter((e) => e.type === 'HideWindow'), + [], + ); }); }); diff --git a/apps/desktop/main/window-kernel.ts b/apps/desktop/main/window-kernel.ts index 277b7490..a8150a54 100644 --- a/apps/desktop/main/window-kernel.ts +++ b/apps/desktop/main/window-kernel.ts @@ -295,16 +295,18 @@ export interface WindowRecord { * any other reason (a plain `WindowHidden`, an overlay detach) must not be * re-shown by that re-activation. * - * TWO ARMS SET IT, on two different seams, and the difference is a rule: - * `AppResigned` hides attached overlays on the RAW edge, `AppResignSettled` - * hides the sweep-eligible floating surfaces after the settle delay. Both are - * reversible visibility, so both are undone by the same re-show. What waits - * for the settle is DISMISSAL (`lastAttention`), because dismissal is - * destructive — a dismissed palette does not come back, so the activation - * dance must not trigger it. A hide that `AppActivated` will undo carries no - * such risk, and the overlay cannot afford the delay: it floats above other - * applications' windows, so 500ms of it is 500ms of Peek's chrome hanging - * over the app the user just switched to. + * SET ON THE RAW `AppResigned` EDGE, not the settle, for both attached + * overlays and `isAppResignSweepEligible` floaters (the two share + * `layer > BASE_LAYER` — floating above the regular stack means floating + * above OTHER applications' windows too). `AppResignSettled` re-runs the + * same sweep only as a backstop for a window that became eligible in the + * gap between the two events. Both are reversible visibility, so both are + * undone by the same re-show. What waits for the settle is DISMISSAL + * (`lastAttention`), because dismissal is destructive — a dismissed palette + * does not come back, so the activation dance must not trigger it. A hide + * that `AppActivated` will undo carries no such risk, and a floating window + * cannot afford the delay: 500ms of it is 500ms of Peek's chrome or HUD + * hanging over the app the user just switched to. */ hiddenByAppBlur?: boolean; @@ -1737,37 +1739,46 @@ export function evolve(state: State, ev: Event): State { // `pendingAppResignSweep`'s doc: this may be the resign half of a dance an // `AppActivated` is about to cancel, and DISMISSAL cannot be taken back. // - // ATTACHED OVERLAYS HIDE HERE, ON THE RAW EDGE, and that difference is the - // rule rather than an exception to it (`WindowRecord.hiddenByAppBlur` - // carries the reasoning). An overlay is not a window the user manages: it - // is chrome drawn for its host, it sits above the regular stack, and above - // the regular stack means above OTHER applications' windows too. Waiting - // out the settle delay would leave Peek's chrome floating over the app the - // user just switched to for the whole of it. The hide is reversible and - // `AppActivated` reverses it, so the dance costs a flicker at worst where - // the sweep would cost a palette. + // ATTACHED OVERLAYS AND SWEEP-ELIGIBLE FLOATERS HIDE HERE, ON THE RAW + // EDGE, and that difference from the settled sweep is the rule rather + // than an exception to it (`WindowRecord.hiddenByAppBlur` carries the + // reasoning). Both share the one structural fact that puts them here: + // `layer > BASE_LAYER` (an attached overlay is chrome drawn above its + // host; `isAppResignSweepEligible` tests the same clause directly) means + // a window sits above the regular stack, and above the regular stack + // means above OTHER applications' windows too. Waiting out the settle + // delay would leave it floating over the app the user just switched to + // for the whole of it. The hide is reversible and `AppActivated` + // reverses it, so the dance costs a flicker at worst where a dismissal + // would cost a palette outright — the same trade `isAppResignSweepEligible` + // already accepted for the HUD once the settle delay used to gate it too. // - // ATTACHMENT, NOT ROLE, selects the victims — the same relation - // `selectTransientAutoclose` reads, and the reason that query skips - // overlays outright: an attached overlay's visibility belongs to its - // attachment, so it is driven here and never dismissed. An already-hidden - // overlay is left alone, which is what keeps a host in fullscreen (whose - // overlay the `OsFullscreenEntered` evolve arm already drove off screen, - // via `overlayFollowPatch`) from having its chrome revealed by the next + // ATTACHMENT AND `layer`, NEVER ROLE, select the victims. Attachment is + // the same relation `selectTransientAutoclose` reads, and the reason + // that query skips overlays outright: an attached overlay's visibility + // belongs to its attachment, so it is driven here and never dismissed. + // `isAppResignSweepEligible` layers on `!canBecomeKey` and excludes the + // transient roles for the same reason (each of those already owns its + // own app-resign policy) — the role clause is a defensive exclusion so + // this rule never doubles up with a different subsystem's, not the + // selector itself. An already-hidden window of either kind is left + // alone, which is what keeps a host in fullscreen (whose overlay the + // `OsFullscreenEntered` evolve arm already drove off screen, via + // `overlayFollowPatch`) from having its chrome revealed by the next // activation. // // No `opSeq` is stamped, exactly as the `AppResignSettled` sweep stamps // none: `lastOpSeq` tracks focus-family ops and `HideWindow` carries no // token. Nothing here touches `key` or `order` (RULE 2). - const hidesOverlay = state.order.some((w) => w.overlayFor !== undefined && w.visible); + const hidesImmediately = (w: WindowRecord): boolean => + w.visible && (w.overlayFor !== undefined || isAppResignSweepEligible(w)); + const hidesSomething = state.order.some(hidesImmediately); return { ...state, appActive: false, pendingAppResignSweep: true, - order: hidesOverlay - ? state.order.map((w) => - w.overlayFor !== undefined && w.visible ? { ...w, visible: false, hiddenByAppBlur: true } : w, - ) + order: hidesSomething + ? state.order.map((w) => (hidesImmediately(w) ? { ...w, visible: false, hiddenByAppBlur: true } : w)) : state.order, version: state.version + 1, }; @@ -1890,10 +1901,17 @@ export function evolve(state: State, ev: Event): State { case 'AppResignSettled': { // Inert unless still armed — an AppActivated in between (the dance) - // already disarmed this. When armed, the resign was real: hide every - // sweep-eligible window (`isAppResignSweepEligible` — the two orthogonal - // properties that made a window the old reducer's `hud` class) and mark - // them so AppActivated knows to undo exactly this sweep. + // already disarmed this. When armed, the resign was real. + // + // THE SWEEP-ELIGIBLE HIDE ITSELF ALREADY RAN, on the raw `AppResigned` + // edge above — see that arm's doc for why floaters moved there with the + // attached overlays. Re-running `isAppResignSweepEligible` here is a + // backstop, not the primary mechanism: it only catches a window that + // BECAME sweep-eligible after the raw edge and before this settle (e.g. + // one registered and shown while Peek was already backgrounded), which + // the raw edge had no chance to see. For everything visible at the raw + // edge this map is a no-op — already hidden, already excluded by its own + // `visible` clause. if (!state.pendingAppResignSweep) { return { ...state, version: state.version + 1 }; } @@ -1901,8 +1919,8 @@ export function evolve(state: State, ev: Event): State { // the app came back active without an `AppActivated` reaching the // machine. Belief-vs-reality here is `appActive`'s job, and a visible // surface must never be dismissed while the app is front — so this - // settle still disarms and still runs the reversible HUD hide above, - // but does NOT record attention as having left. `attentionLost` is a + // settle still disarms and still runs the backstop hide above, but does + // NOT record attention as having left. `attentionLost` is a // STATE test (`state.appActive`), not an event test, which is why the // guard belongs here, on the write, rather than in `react`. const attentionLost = !state.appActive; @@ -2681,7 +2699,10 @@ export function react(prev: State, ev: Event, next: State): Effect[] { // bottom -> top stacking over visible, ORDERABLE windows, one SetStacking // per window. A window that is not yet orderable (a fresh page-host before // its first composited frame) cannot be positioned by the OS, so it is - // omitted from the emission entirely. + // omitted from the emission entirely. Always-on-top is folded into `layer` + // and realized ONLY here (`stackingLevelForLayer` in the executor) — there + // is no separate pin effect — so this loop is also where a pin gets + // reinforced, not just where order gets replayed. const orderChanged = prev.order.length !== next.order.length || prev.order.some((w, i) => w.id !== next.order[i]?.id); @@ -2694,7 +2715,29 @@ export function react(prev: State, ev: Event, next: State): Effect[] { prev.order.find((w) => w.id === ev.id)?.orderable !== true && next.order.find((w) => w.id === ev.id)?.orderable === true; - if (orderChanged || justBecameOrderable) { + // A record becoming visible must trigger the same full re-emission, on the + // same reasoning: the native ctor-time always-on-top flag is unreliable on + // its own (the reason the old machine re-asserted it after construction), + // and a window is registered `visible: false` — before that flag can be + // trusted to have stuck — so the first opportunity this kernel has to + // reinforce it is the show. `orderChanged` alone misses this for a + // NON-ACTIVATING show (session restore, an inactive `windows.ts` show, an + // extension window): none of those reorder `order`, so without this clause + // a pinned window shown that way never gets its level reinforced. + // + // STRUCTURAL, like every other trigger in this function — a `visible` diff + // over `prev`/`next`, never a read of `ev.type` (`justBecameOrderable` + // above is the one exception already in this file; this clause does not + // add a second). `orderable` is carried over from the base gate for the + // same reason it gates the emission loop itself: a not-yet-orderable window + // cannot be positioned by the OS yet, so becoming visible before its first + // composited frame is not yet a reason to replay anything. + const justBecameVisible = next.order.some((w) => { + const prevRecord = prev.order.find((p) => p.id === w.id); + return prevRecord !== undefined && !prevRecord.visible && w.visible && w.orderable; + }); + + if (orderChanged || justBecameOrderable || justBecameVisible) { for (const w of next.order) { if (w.visible && w.orderable) { effects.push({ type: 'SetStacking', id: w.id, layer: w.layer }); @@ -3421,8 +3464,11 @@ export function selectTransientAutoclose( /** * Which windows a REAL app switch hides for the duration — `evolve`'s - * `AppResignSettled` arm applies exactly this, and `AppActivated` undoes - * exactly what it marked. Ported from `window-state.ts`'s `class === 'hud' && + * `AppResigned` arm applies exactly this on the raw resign edge (alongside the + * attached-overlay hide), and `AppResignSettled` re-applies it only as a + * backstop for a window that became eligible in the gap before the settle; + * `AppActivated` undoes exactly what either marked. Ported from + * `window-state.ts`'s `class === 'hud' && * visible` sweep, decomposed into the orthogonal facts that MADE a window that * class (`classifyWindowRegistration`: `alwaysOnTop && focusable === false`, * with the role classes taking precedence):