experiments in a post-browser web
10
fork

Configure Feed

Select the types of activity you want to include in your feed.

fix(window-targeting): harden tile:window:{maximize,fullscreen,center} against bad ids; correct .d.ts; add regression tests

User reported that running the maximize command while a search window
(e.g. cmd: #todo) is open in front of a content window maximizes the
content window behind it instead of the search window.

I could not reproduce the reported scenario in a Playwright test —
across three variants (manual focus call, natural open-then-open
without explicit focus, real peek://search/home.html with the manifest
key) the focus tracker correctly returned the front-most workspace
window and getFocusedVisibleWindowId resolved to it. So the production
bug must depend on additional state (cmd-panel autoclose timing,
NSPanel focus events, or something my test environment doesn't model).

Defensive fixes that close adjacent failure modes:

- Harden tile:window:maximize / fullscreen / center IPC handlers to
reject non-number args.id loudly. Previously these silently fell back
to BrowserWindow.fromWebContents(event.sender) when the caller passed
a non-number, which would let a "forgot to unwrap the envelope" caller
bug propagate as wrong-window targeting. Now returns a clear
"Invalid window id" error.

- Correct backend/electron/tile-api.d.ts: getFocusedVisibleWindowId is
declared as Promise<{success, windowId, error}>, but the preload
unwraps and returns Promise<number | null>. The wrong type silently
approves bugs where a caller treats the result as a {success, ...}
envelope.

- Three regression tests in tests/desktop/window-targeting.spec.ts that
cover the focus-tracker layer for the maximize-targeting scenario
(basic, natural-flow without explicit focus, real search tile open
with the manifest key). All currently green — they document the
contract and will catch regressions if the tracker layer ever drifts.

The user repro likely needs in-app diagnostic logging to pin down
whether the focus event isn't firing, the tracker is being overwritten,
or the cmd-panel-modal flow has a state issue. None of those are
addressable from these defensive fixes alone.

+160 -3
+5 -3
backend/electron/tile-api.d.ts
··· 178 178 setVisibleOnAllWorkspaces(id: number | undefined, visible: boolean, opts?: { visibleOnFullScreen?: boolean; skipTransformProcessType?: boolean }): Promise<{ success: boolean; error?: string }>; 179 179 180 180 /** 181 - * Get the id of the last-focused visible window. 182 - * Requires `window.query` capability. 181 + * Get the id of the last-focused visible window, or null if none is 182 + * tracked. The preload wrapper unwraps the underlying IPC envelope, so 183 + * callers receive the bare id (or null) — not a `{success, windowId}` 184 + * object. Requires `window.query` capability. 183 185 */ 184 - getFocusedVisibleWindowId(): Promise<{ success: boolean; windowId?: number; error?: string }>; 186 + getFocusedVisibleWindowId(): Promise<number | null>; 185 187 186 188 /** 187 189 * Set the overlay focus target for the calling tile's window.
+13
backend/electron/tile-ipc.ts
··· 2886 2886 } 2887 2887 2888 2888 try { 2889 + if (args.id !== undefined && typeof args.id !== 'number') { 2890 + return { success: false, error: `Invalid window id: ${typeof args.id}` }; 2891 + } 2889 2892 const win = typeof args.id === 'number' 2890 2893 ? BrowserWindow.fromId(args.id) 2891 2894 : BrowserWindow.fromWebContents(event.sender); ··· 2983 2986 } 2984 2987 2985 2988 try { 2989 + // Reject non-number ids loudly. The convention is "omit id for the 2990 + // calling window", not "pass an object and silently fall back" — 2991 + // a malformed caller (e.g. forgetting to unwrap a `{success, data}` 2992 + // envelope) would otherwise silently maximize the wrong window. 2993 + if (args.id !== undefined && typeof args.id !== 'number') { 2994 + return { success: false, error: `Invalid window id: ${typeof args.id}` }; 2995 + } 2986 2996 const win = typeof args.id === 'number' 2987 2997 ? BrowserWindow.fromId(args.id) 2988 2998 : BrowserWindow.fromWebContents(event.sender); ··· 3016 3026 } 3017 3027 3018 3028 try { 3029 + if (args.id !== undefined && typeof args.id !== 'number') { 3030 + return { success: false, error: `Invalid window id: ${typeof args.id}` }; 3031 + } 3019 3032 const win = typeof args.id === 'number' 3020 3033 ? BrowserWindow.fromId(args.id) 3021 3034 : BrowserWindow.fromWebContents(event.sender);
+142
tests/desktop/window-targeting.spec.ts
··· 26 26 if (app) await app.close(); 27 27 }); 28 28 29 + test('getFocusedVisibleWindowId tracks the most recently focused non-modal window', async () => { 30 + // Regression for: when a search window is open in front of a page-host 31 + // (e.g. example.com), the maximize command was hitting the page-host 32 + // *behind* the search window. The bug surfaces if either 33 + // getFocusedVisibleWindowId returns a stale id or the maximize IPC 34 + // silently falls back to event.sender on a bad arg. This test pins 35 + // the expected behavior at the tracker layer. 36 + const result = await bgWindow.evaluate(async () => { 37 + const api = (window as any).app; 38 + 39 + // 1. Open an http content window. This becomes the "page-host 40 + // behind" the search window in the user's repro. 41 + const back = await api.window.open('about:blank', { 42 + role: 'workspace', 43 + width: 600, 44 + height: 400, 45 + key: 'targeting-back', 46 + }); 47 + if (!back.success) return { stage: 'open back', error: back.error }; 48 + 49 + // Drive focus explicitly — opening a new window doesn't always 50 + // settle focus before we read the tracker in headless tests. 51 + await api.window.focus(back.id); 52 + await new Promise(r => setTimeout(r, 200)); 53 + 54 + // 2. Open a "search-like" workspace window on top. 55 + const front = await api.window.open('about:blank', { 56 + role: 'workspace', 57 + width: 700, 58 + height: 500, 59 + key: 'targeting-front', 60 + }); 61 + if (!front.success) return { stage: 'open front', error: front.error }; 62 + await api.window.focus(front.id); 63 + await new Promise(r => setTimeout(r, 200)); 64 + 65 + // 3. The tracker should now point at the front window. 66 + const tracked = await api.window.getFocusedVisibleWindowId(); 67 + 68 + // Cleanup 69 + await api.window.close(back.id); 70 + await api.window.close(front.id); 71 + 72 + return { backId: back.id, frontId: front.id, tracked }; 73 + }); 74 + 75 + expect(result.error).toBeUndefined(); 76 + expect(result.tracked).toBe(result.frontId); 77 + // And critically, NOT the back window: 78 + expect(result.tracked).not.toBe(result.backId); 79 + }); 80 + 81 + test('maximize cmd targets the most recently opened workspace, not the one behind it', async () => { 82 + // Tighter reproduction of the user's bug report: opening a "back" 83 + // window and then a "front" workspace window without explicitly 84 + // calling api.window.focus on either, then running maximize, must 85 + // grow the front window (not the back one). If the natural focus 86 + // flow on window.open doesn't update the tracker, this fails — and 87 + // any "maximize" command that uses getFocusedVisibleWindowId hits 88 + // the wrong window. 89 + const result = await bgWindow.evaluate(async () => { 90 + const api = (window as any).app; 91 + 92 + const back = await api.window.open('about:blank', { 93 + role: 'workspace', 94 + width: 600, 95 + height: 400, 96 + key: 'targeting-back-natural', 97 + }); 98 + if (!back.success) return { stage: 'open back', error: back.error }; 99 + await new Promise(r => setTimeout(r, 250)); 100 + 101 + const front = await api.window.open('about:blank', { 102 + role: 'workspace', 103 + width: 700, 104 + height: 500, 105 + key: 'targeting-front-natural', 106 + }); 107 + if (!front.success) return { stage: 'open front', error: front.error }; 108 + await new Promise(r => setTimeout(r, 250)); 109 + 110 + const tracked = await api.window.getFocusedVisibleWindowId(); 111 + return { backId: back.id, frontId: front.id, tracked }; 112 + }); 113 + 114 + expect(result.error).toBeUndefined(); 115 + // The freshly-opened front window should be tracked. If this fails, 116 + // the bug is at the focus-event-on-open layer — opening a window 117 + // doesn't reliably update lastFocusedVisibleWindowId, leaving stale 118 + // state from whatever was focused previously. 119 + expect(result.tracked).toBe(result.frontId); 120 + 121 + // Cleanup 122 + await bgWindow.evaluate(async (ids: { backId: number; frontId: number }) => { 123 + const api = (window as any).app; 124 + await api.window.close(ids.backId); 125 + await api.window.close(ids.frontId); 126 + }, { backId: result.backId as number, frontId: result.frontId as number }); 127 + }); 128 + 129 + test('opening the actual search tile after a content window updates the focus tracker', async () => { 130 + // Closer to the user repro: open a content window, then open the 131 + // search tile (peek://search/home.html with the manifest key 132 + // 'search-home'), and assert the tracker now points at search. 133 + // If the search tile's reuse flow or lazy-stub bootstrap doesn't 134 + // propagate focus correctly, this fails — explaining why the 135 + // 'maximize' command lands on the back content window. 136 + const result = await bgWindow.evaluate(async () => { 137 + const api = (window as any).app; 138 + 139 + const back = await api.window.open('about:blank', { 140 + role: 'workspace', 141 + width: 600, 142 + height: 400, 143 + key: 'targeting-search-back', 144 + }); 145 + if (!back.success) return { stage: 'open back', error: back.error }; 146 + await new Promise(r => setTimeout(r, 250)); 147 + 148 + const search = await api.window.open('peek://search/home.html', { 149 + role: 'workspace', 150 + width: 700, 151 + height: 500, 152 + key: 'search-home', 153 + }); 154 + if (!search.success) return { stage: 'open search', error: search.error }; 155 + await new Promise(r => setTimeout(r, 400)); 156 + 157 + const tracked = await api.window.getFocusedVisibleWindowId(); 158 + return { backId: back.id, searchId: search.id, tracked }; 159 + }); 160 + 161 + expect(result.error).toBeUndefined(); 162 + expect(result.tracked).toBe(result.searchId); 163 + 164 + await bgWindow.evaluate(async (ids: { backId: number; searchId: number }) => { 165 + const api = (window as any).app; 166 + await api.window.close(ids.backId); 167 + await api.window.close(ids.searchId); 168 + }, { backId: result.backId as number, searchId: result.searchId as number }); 169 + }); 170 + 29 171 test('setWindowColorScheme returns success with windowId', async () => { 30 172 // Test that setWindowColorScheme works and returns expected data 31 173 const result = await bgWindow.evaluate(async () => {