← Commit history

Add pup-cdp-discipline + pup-differential-diagnosis maintainer dev-skills (CDP event-flood handling rules + prove-it-against-nb technique)

John Lauer ·13d77fe0c9 ·2mo ago ·parent 02225bc
2 files changed +198
dev-skills/pup-cdp-discipline/SKILL.mdadded+99
@@ -0,0 +1,99 @@+---+name: pup-cdp-discipline+description: The hard rules for handling the Chrome DevTools Protocol (CDP) in the pup bridge WITHOUT choking. pup drives Chrome over CDP on a SINGLE Node event loop; the moment a page throws thousands of CDP events at a naive handler, the loop starves, the bridge stops answering AD on :64230, and it looks like "pup crashed." This has been the #1 recurring misery for MONTHS. READ THIS before you enable a CDP domain, add a page event listener, createCDPSession, wire a console/network/DOM/screencast handler, or do ANY per-event work. Never be lazy about CDP again.+---++# CDP discipline: never bring the bridge to a screeching halt again++John, verbatim and furious (and right): *"the moment you start dealing with thousands of events across+CDP you come to a screeching halt and can't handle it. it's been miserable. you have made my life awful+with how shitty you've handled CDP in the past. i never want you to be lazy about CDP handling again."*++This is the single longest-running class of pup bug. It is NOT Chrome's fault and NOT the page's fault+(see `pup-differential-diagnosis` — the same page always runs fine in nb / plain Chrome). It is pup+processing a flood of protocol events on one event loop with naive, synchronous, always-on handlers.+Internalize the model, then obey the rules.++## The model (why it dies)++- pup + puppeteer drive Chrome over ONE websocket, and every CDP message is dispatched on the bridge's+  SINGLE Node event loop — the same loop that serves AD's HTTP on `127.0.0.1:64230`.+- CDP domains are FIREHOSES. Enable `Debugger` and Chrome streams a `scriptParsed` for every script+  (thousands on a bundle). Enable `Network` and you get request/response/dataReceived/loadingFinished+  for every asset. `Log`, `DOM`, `CSS`, `Performance`, `Runtime.consoleAPICalled`, `Page.screencastFrame`+  — all high-volume on a real app.+- A heavy page (a Babylon.js 3D viewer, a busy SPA, a console-spammer) emits these by the THOUSAND, in+  bursts. If pup's handler for each is synchronous and unbounded, the loop spends all its time in+  handlers and NEVER gets back to the HTTP server. AD's health probe times out → `error sending request+  for url .../command` / `bridge_not_listening` → AD reaps the "hung" bridge. The process is usually+  still ALIVE (uncaughtException/unhandledRejection guards survive) — it is STARVED, not crashed.+- Concurrency multiplies it: N heavy pages open at once = N floods on the one loop. Single opens often+  survive; the batch/migration/audit-loop is what kills it.++## The rules (violate none of these)++### 1. Enable the MINIMUM domains, for the SHORTEST time. Then disable AND detach.+The default posture is: no extra CDP domain enabled. Turn one on only for the exact operation, then turn+it OFF and detach the session. **A chatty domain left enabled is a permanent tax on every future event.**+- The canonical scar: `neutralizeDebugger` (v1.9.341) enabled `Debugger` on every page and KEPT it on to+  skip `debugger;` pauses → `scriptParsed` flood + V8 de-opt → starvation under load. Fix (v1.9.347):+  enable → `Debugger.resume` (clear any inherited pause) → **`Debugger.disable`** → `detach()`. Immunity+  kept (a `debugger;` is a no-op with no client holding the domain), flood gone.+- `Network`, `Log`, `DOM`, `Performance`, `CSS`, `Runtime` (for consoleAPICalled) are the same hazard.+  If you enable one, you own turning it off.++### 2. Balance every `createCDPSession()` with a `.detach()`.+An attached session keeps delivering events even if you dropped the reference. Leaks accumulate per+tab/verb until the loop is buried. Grep must balance: `grep -c createCDPSession` ≈ `grep -c '\.detach('`.+The few intentional long-lived sessions (the recorder's screencast) are deliberate and bounded — every+OTHER session is short-lived: create, do the one thing, detach in a `finally`.++### 3. NO heavy synchronous work in an event handler.+An event handler runs ON the loop. Per-event `JSON.parse` of a big payload, a fat regex, a disk write,+a `sharp` composite, a synchronous AD round-trip — any of these, multiplied by thousands of events,+IS the stall. Handlers must be O(1)-ish: stash a value, bump a counter, set a dirty flag. Do the real+work later, once, off the hot path.++### 4. Debounce / throttle / coalesce EVERY burst-y event.+Bursts must collapse to one unit of work:+- `framenavigated` (SPA route spam, redirect chains) → `scheduleOverlayRefresh` / `scheduleCategoryRecheck`+  are DEBOUNCED (~2s). New handlers on nav MUST debounce too — never do work per navigation event.+- `console` / `Runtime.consoleAPICalled` → pup FLOOD-SUPPRESSES ("suppressed N flood messages in the last+  second"). Keep it. Never log/process every console line from a spammer.+- DOM mutation / animation-frame-driven events → sample, don't stream.+The overlay PAINT is deduped too (skip the AD call when the composed icon+tooltip is unchanged) — same+principle: one commit per real change, never a per-event heartbeat.++### 5. Bound and scope streams (screencast, tracing).+`Page.startScreencast` streams frames forever — cap the rate, scope it to the ONE target, and stop it the+instant recording ends. Never leave a screencast, tracing, or coverage session running "just in case."++### 6. Keep the loop free for the health server.+AD MUST be able to reach `:64230` at all times, or it reaps the bridge. Treat "the HTTP server can always+answer within a health-check timeout" as an invariant. If an operation is genuinely CPU-heavy and+unavoidable, chunk it / yield (`setImmediate`) so the loop breathes, or push it off-thread. `sharp` is+threadpool-backed (OK); genuinely synchronous CPU on the loop is not.++## The acceptance test (prove it, every time)+A CDP change is not done until it passes the **concurrent heavy-load stress test** from+`pup-differential-diagnosis`: fire 4+ heavy pages at once and poll `browser_readiness` throughout — it+must stay responsive 10/10, zero `error sending request`. If a single open works but the batch drops the+bridge, you have NOT fixed it.++## The month-after-month history (so it is never re-derived as "a heavy page")+- **The crashes/freezes reckoning (v1.9.185, minimal-touch).** John: *"you crash chrome with the way you+  control it… what are you doing that's so different trying to control chrome that you cause crashes and+  freezes?"* Answer: all the cosmetic shell + CDP machinery synchronizing with Chrome's UI thread. The+  measured verdict was to STOP doing the extra things (`pup-aumid-decisions`).+- **Overlay paint spam** — re-sending identical taskbar overlays every few seconds; fixed by dedup.+- **`Debugger.enable` kept on** (v1.9.341→347) — the scriptParsed flood documented above.+- **Startup blocking the bind** (#468) — heavy synchronous `require`s / session recovery blocking+  `server.listen`, so AD reaped before the port bound; fixed by deferring heavy init AFTER listen.+Every one of these is the same root disease: too much work on the one loop, driven by CDP volume.++## The rule to tattoo on your hand+**Design every CDP interaction assuming a hostile, chatty page emitting thousands of events.** Enable the+least, disable and detach fast, never work synchronously per event, debounce every burst, and keep the+loop free for the health server. Then prove it under concurrent load. A correct CDP client is unbothered+by any page; if pup chokes, pup is wrong. Related: `pup-differential-diagnosis` (prove it's you, not the+page), `pup-bridge-debug`, `pup-aumid-decisions`.
dev-skills/pup-differential-diagnosis/SKILL.mdadded+99
@@ -0,0 +1,99 @@+---+name: pup-differential-diagnosis+description: How to tell whether a "the page crashed/hung pup" problem is really pup's bug (almost always) and not the page or Chrome, by running the SAME page through nb (the Adom extension driving real Chrome) or a plain headful Chrome and watching it behave. A heavy or hostile page must NEVER crash, hang, or starve the pup bridge; if it does, the fault is in pup's CDP handling. READ THIS before you blame a page ("it's a giant Babylon.js bundle", "it floods the console", "it's a WebGL hog") for bridge instability. John's rule: prove it's your bridge first.+---++# Differential diagnosis: is it pup, or is it the page? (it's pup)++John, verbatim: *"a heavy page should NEVER crash your bridge... open it in nb chrome and see if that+crashes, and if it doesn't it shows you your bridge has issues you need to fix. i like the model of you+testing yourself against nb chrome so you never blame chrome and a heavy page — which i tend to think is+how you're handling the CDP interface."*++He is right, and this is the standing rule: **a page cannot crash a correct CDP client.** Chrome sandboxes+and isolates page code; the DevTools protocol is a read/drive channel. If a page takes down the *bridge*,+the bug is in how the bridge drives CDP, not in the page. Do not close a bug as "that page is just too+heavy." Localize it first with the differential test below.++## The differential test (do this before touching bridge code)++Both pup and the Adom browser extension (`nbrowser_*`) drive Chrome over CDP. So does a plain headful+Chrome you launch by hand. Run the SAME url through a second CDP driver:++1. **nb / real Chrome** — `nbrowser_open` the exact url in the user's real browser (see `driving-the-extension`).+2. **Plain headful Chrome** — launch Chrome yourself with `--user-data-dir=<scratch>` and open the url+   (no pup, no bridge). This is the cleanest control: it removes BOTH pup and the extension.++Then read the result:++| nb / plain Chrome | pup | Verdict |+|---|---|---|+| renders fine | crashes / hangs / drops | **pup's bug.** The page + Chrome + CDP are proven fine; the only variable left is pup's own CDP handling. Fix pup. |+| also crashes | crashes | rare — a genuine Chrome/GPU bug or an OOM. File upstream with the plain-Chrome repro; still check pup isn't amplifying it. |++In practice it is always the first row. The differential's job is to make you *stop guessing about the+page* and start reading your own CDP code.++## Read the failure shape: crash vs starvation (they are different)++The bridge has top-level guards (`process.on('uncaughtException')` / `'unhandledRejection')`) that log and+survive, so a stray CDP rejection ("Target closed") does NOT kill the process. That means the symptom you+actually see is almost never a hard crash — it is **event-loop starvation**:++- **Hard crash** (rare): the node process exits, its pid is gone, and a stack lands in+  `%USERPROFILE%\.adom\bridge-logs\puppeteer.log` stderr. Look for the stack.+- **Starvation** (the usual one): the node process is STILL ALIVE (same pid), but AD can't reach it —+  `error sending request for url http://127.0.0.1:64230/command`, `bridge_not_listening`, or verbs time+  out. The single event loop is blocked processing something, so HTTP on 64230 can't be served, and AD's+  health check reaps it. **This is what "the page crashed the bridge" almost always is.**++Confirm which you have via AD-core (works even when the bridge is unreachable): `shell_execute` a check+of the node pid + a tail of the bridge log. Same pid + no stack = starvation, hunt the event-loop blocker.++## The load multiplier: reproduce under CONCURRENCY, not a single open++A single open of the "bad" page often works — the starvation only shows when several heavy opens/closes+overlap (a migration, a batch, an audit loop). So the real repro is **concurrent load**:++```bash+# fire N heavy opens at once, then poll the bridge the whole time — it must stay responsive+for n in 1 2 3 4; do ( adom-desktop --target <host> --ai-thread stress browser_open_window \+  "{\"sessionId\":\"stress-$n\",\"url\":\"<heavy url>?s=$n\"}" & ); done+for i in $(seq 1 10); do adom-desktop --target <host> browser_readiness '{"reason":"probe"}'; sleep 3; done+```++If the probes stay `ready` through the load, the bridge is healthy under it. A `DROP` / `error sending+request` during the load is the bug reproducing. Fix, reship, and re-run this exact stress until it is+10/10 responsive. That is the acceptance test.++## What in pup's CDP handling actually starves the loop (the suspect list)++Everything here runs CDP traffic on the bridge's single event loop. On a heavy page, any of these can+flood it:++1. **`Debugger.enable` kept on (the one that bit us, v1.9.341→347).** `neutralizeDebugger` enabled the+   Debugger domain on every page and KEPT it enabled to skip `debugger;` pauses. Enabling Debugger makes+   Chrome stream a `Debugger.scriptParsed` event for EVERY script the page parses — thousands on a+   Babylon.js bundle — all processed on the loop, plus V8 runs de-optimized. Under concurrent heavy opens+   the loop starved → `error sending request`. FIX: enable → clear any inherited pause (`Debugger.resume`)+   → **disable again** → detach. A page's `debugger;` is a no-op with no client holding the domain, so+   immunity is kept without the flood. **Lesson: never leave a chatty CDP domain (`Debugger`, `Network`+   with bodies, `Log`, `Performance`) enabled longer than one op; disable + detach.**+2. **Undetached CDP sessions.** `createCDPSession` without a matching `.detach()` leaks a session per+   call; each keeps delivering events. Grep `createCDPSession` vs `.detach()` — they should balance+   (the intentional long-lived ones are few and deliberate).+3. **Console-event handling.** pup subscribes to page `console` and logs it; a page that spams the+   console spams the loop. pup already flood-suppresses ("suppressed N flood messages") — keep that, and+   never do heavy synchronous work (regex, JSON, disk) per console line.+4. **Screencast / `Page.startScreencast`** (recorder): frames stream continuously; bound the rate and+   scope to the target, never leave one running.+5. **Synchronous heavy work on the loop** — sharp compositing for overlays, the notch ICO build.+   sharp is threadpool-backed (fine), but any genuinely synchronous per-event work is not; defer/debounce.++## The rule to internalize++When a page "crashes pup": **prove it against nb or plain Chrome first.** It will render there. That+result is your evidence that the bug is yours, in the CDP layer — go find the domain you left enabled,+the session you didn't detach, or the event storm you processed synchronously. Then reship and pass the+concurrent-load stress test. Do not ship a "that page is too heavy" excuse; the correct bridge survives+any page.