← Commit history

Dev skill final pass: remove contradictory input-heuristic re-park text, direct-API principle, self-heal + verified-badge sections, restart-kills-windows wart, position-based verify

John Lauer ·8c3af8ff59 · ·parent 96066ce
1 file changed +55−50
dev-skills/pup-bridge-dev/SKILL.md+55−50
@@ -275,47 +275,53 @@ tiny Win32 call, no PS window-hunting, no timers, no loops.** Lives in `tryDetac 6. Remaining AD gap (issue filed): a `send-to-back` window state; when AD ships it, delete    `osBottomUnlessForeground` and pup is 100% PS-free. -**The 4 techniques (still true, now mostly belt-and-suspenders):**+**Supporting principles (subordinate to the algorithm above — where older text conflicts, the+algorithm wins):**  1. **Launch OFF-SCREEN** → satisfies R1. Background windows launch with    `--window-position=-32000,-32000` (NOT `--start-maximized`; strip it when `!foreground`). The window is    created far past any monitor, so nothing ever appears on screen during startup. CDP renders regardless    of position, so screenshots/eval/record all still work.-2. **Park ON-SCREEN at the bottom of the z-order** → satisfies R2 + R3. On-screen-occluded (behind the-   user's windows, taskbar-clickable) is the ONLY resting state that keeps R3 — see the minimize dead-end.-   TWO details that took blood to get right:-   - **Resolve the REAL window by ENUMERATION, not `MainWindowHandle`.** `Get-Process(pid).MainWindowHandle`-     returns a HIDDEN Chrome helper window on some boxes → `SetWindowPos` moved a phantom, the real window-     stayed off-screen, and the "backgrounded" self-check passed against the phantom (a FALSE positive).-     Enumerate top-level windows and take the largest VISIBLE `Chrome_WidgetWin_1` owned by the pid/children-     (`WinBgP::BestWindow`). This was THE "I click the taskbar and nothing shows" bug (1.8.46).-   - **Deactivate FIRST, THEN move on-screen** (order matters). If you move it on-screen while Chrome still-     has it foreground, it flashes on TOP for a moment (an active window stays topmost until deactivated) =-     the "I saw it open" pop. So: hand focus back (below) while it is still OFF-SCREEN, and only then-     `SetWindowPos(pup, HWND_BOTTOM, 0, 0, scrW, scrH, SWP_NOACTIVATE)` — now it drops behind, invisible.-3. **Hand keyboard focus back via `AttachThreadInput`** → satisfies R4, and is the crux. Plain-   `SetForegroundWindow(usersWindow)` is BLOCKED by Windows' foreground lock. The workaround:-   - snapshot the user's window with `GetForegroundWindow()` **before** spawning the browser;-   - after the browser grabs focus: `AttachThreadInput(pupWindowThreadId, myThreadId, true)` (get the pup-     window's thread via `GetWindowThreadProcessId`), which shares the foreground privilege → then-     `SetForegroundWindow(usersWindow)` + `BringWindowToTop(usersWindow)` succeeds → detach with-     `AttachThreadInput(..., false)`.-   - **Keep the re-park window SHORT (~1.2s: 300/700/1200ms), not 2.5s+.** The re-passes only exist to-     counter Chrome re-grabbing foreground during startup. A LONG/aggressive loop FIGHTS the user: they-     click the window to view it and the next pass yanks it back to the background. After ~1.2s pup must-     NEVER auto-re-park. WITHIN the window, use `GetLastInputInfo`: pup-foreground + a physical input in the-     last ~1.5s = the USER (Chrome self-raise is programmatic, no input) → bring the window on-screen-     (centered ~80%) + latch `browser._userForeground` + cancel remaining passes; else re-park. See the-     "user↔AI balance" section below — this is the behavior John cares most about.-4. **Run all this PowerShell HIDDEN** → satisfies R5. Use `execFileSync('powershell.exe', [...args],-   {windowsHide:true})` + `-WindowStyle Hidden` (see `runPsHidden`/`runPsHiddenAsync`), NEVER-   `execSync('powershell …')` — the latter goes through cmd.exe and flashes a console window. (A cmd blip-   during a redeploy is different: that's AD respawning the node process, not our runtime.)+2. **The resting state is ON-SCREEN at the bottom of the z-order** → satisfies R2 + R3.+   On-screen-occluded (behind the user's windows, taskbar-clickable, still compositing) is the ONLY+   resting state that keeps screenshots working — see the minimize dead-end.+3. **DIRECT APIs, not PowerShell** (John: "why are you using powershell? that's heavy. why aren't you+   just calling direct api's?"). Geometry = CDP `Browser.setWindowBounds` by windowId (exact window,+   ~10ms). Window lookup = AD `desktop_find_window`. Foregrounding = AD `desktop_bring_to_front`.+   Flash/badge = AD `desktop_flash_window` / `desktop_taskbar`. The one residual PS+   (`osBottomUnlessForeground`) exists ONLY because z-order has no direct API yet; it also reads+   `GetForegroundWindow` — the ONE honest "did the user click it" signal. Never grow the PS surface.+   (`AttachThreadInput` focus-handback machinery still exists in the legacy helpers used by+   `browser_lower_os_window`; with `--no-startup-window` nothing steals focus, so new code never needs it.)+4. **What little PowerShell remains runs HIDDEN** → satisfies R5. `execFileSync('powershell.exe',+   [...args], {windowsHide:true})` + `-WindowStyle Hidden` (see `runPsHidden`), NEVER+   `execSync('powershell …')` — that goes through cmd.exe and flashes a console window. (A cmd blip+   during a redeploy is AD respawning the node process, not our runtime.) -**SELF-VERIFY, always.** The PS re-reads `GetForegroundWindow()` after the handback and returns-`backgrounded` | `still-foreground` | `no-window`; that surfaces as the `background` field in the-open response + `getSessionInfo`. Test on a box with a LIVE interactive desktop (an RDP-disconnected box-reports a degenerate/blank foreground and lies "backgrounded"). ADOMGPU with Fusion open is a good rig.+**SELF-VERIFY, always — and verify POSITION, not just focus.** The park's verdict comes from CDP+`getWindowBounds` (on-screen?) — a focus-only check once said "backgrounded" while the window sat+stranded off-screen. The verdict surfaces as the `background` field in the open response ++`getSessionInfo` ('backgrounded' | 'user-foreground' | 'park-failed' | 'no-window'). Test on a box+with a LIVE interactive desktop (an RDP-disconnected box reports a degenerate foreground and lies);+have the USER click taskbar buttons early and late — that's the test every automated check missed.++**SELF-HEAL (v1.8.57) — windows must not stay broken.** A window created under a buggy build (or any+future placement failure) used to sit stranded off-screen forever: its taskbar button "activated" an+invisible window and the badge was gone (this bit the adom-shotlog app's long-lived shared window).+Every natural touchpoint (bridge-restart reattach, `open_tab`, `switch_tab`) now runs+`healOffscreenWindow`: a cheap CDP bounds check, and ONLY if actually off-screen, re-park + re-badge.+Healthy or user-held windows are never touched.++**BADGE (v1.8.58) — check the result, debounce only success.** `badgePupTaskbar` verifies AD's+response and logs `[badge] applied/FAILED`; the tab-hot-path debounce (10 min) arms ONLY on success.+The old fire-and-forget + unconditional debounce meant one silent failure = a bare icon for 10++minutes (John: "there's no favicon on this window? wtf?").++**KNOWN WART (open):** AD's `restart_bridge`/`bridge_kill` can take the pup-spawned Chrome processes+down with the bridge — every pup window (including other apps' windows, e.g. shotlog's) vanishes on a+bridge update. Normal users only hit this on AD auto-updates; apps recover by reopening on next use.+If this starts mattering, file it against AD (process-tree kill) — do not "fix" it by re-parenting+Chrome in pup.  **`browser_raise_os_window` is the inverse:** move the window on-screen + maximize via CDP `Browser.setWindowBounds`, then foreground it. If you ever add a non-activatable style (see below), raise@@ -372,22 +378,21 @@ and use normally**. Both must hold at once. Get the handoff right: - **The moment the user takes a window, it becomes a normal foreground window** — visible, on-screen,   usable, and it **STAYS** that way. pup must never fight a deliberate user action. -**How pup tells a user-raise from Chrome's self-raise (both make the pup window foreground):**-`GetLastInputInfo` — a physical input (click/key) within ~1.5s while the pup window is foreground means the-**user** did it; Chrome's startup self-raise is programmatic (no input). See the `respectUser` path in-`osBackgroundWindowByPid` and the `_userForeground` latch in `tryDetachedLaunch`.--**When a user-raise is detected, the re-park must do TWO things (missing either is a bug we shipped):**-1. **Bring the window ON-SCREEN and SHOW it** (`SetWindowPos HWND_TOP … SWP_SHOWWINDOW`, centered ~80%).-   The window is still at its off-screen launch coords, so *just bailing out* activates an **invisible**-   window — the user clicks the taskbar button and sees NOTHING. (This was the exact v1.8.44 regression:-   the bail exited without moving it on-screen. Fixed in v1.8.45.)-2. **Latch `_userForeground`** so the remaining re-park passes AND the post-resize re-park leave it alone.-   Once the human has it, pup does not background it again for that session.+**How pup tells a user-raise from Chrome's self-raise — FINAL ANSWER (v1.8.51+): it doesn't have+to, because Chrome can no longer self-raise.** `--no-startup-window` makes "pup window is+OS-foreground" (`GetForegroundWindow`, read inside `osBottomUnlessForeground`) mean exactly ONE+thing: the user clicked it. No `GetLastInputInfo`, no input-timing, no heuristics — all of those+were shipped and all false-fired (see dead ends). -**Dead end (do NOT reintroduce):** detecting a user-raise and just `exit`-ing the re-park **without moving-the window on-screen** → the window stays parked off-screen, foreground but invisible, and the user's click-shows nothing. Always transition it to a real visible on-screen window.+**When the user has the window (at park time or ever after):**+1. **Bring it ON-SCREEN and SHOW it** (CDP place + AD `desktop_bring_to_front`). The window may+   still be at its off-screen launch coords, so *just bailing out* activates an **invisible**+   window — the user clicks the taskbar and sees NOTHING (the v1.8.44 regression, and again the+   v1.8.51 launch-gap bug).+2. **Leave it alone forever.** There are no re-park passes to latch off anymore — after the single+   park, nothing in pup can background a window the user holds. The only re-park paths left are the+   explicit `browser_lower_os_window` verb and the off-screen SELF-HEAL (which checks bounds first+   and never touches an on-screen window).  ## The taskbar flash — pulse vs. the persistent tint (clearing it programmatically)