diff --git a/README.md b/README.md index d24e5e6..505dd6f 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,9 @@ Conductor). - **Shared agent sessions** — one isolated browser session per Herdr workspace. - **Live push streaming** — frames, URL/title changes, console messages, and page errors arrive over WebSocket, with transparent polling fallback. +- **Failed network requests** — 4xx/5xx and no-response xhr/fetch/document + requests appear in the console region as `✖ 404 GET ` lines, in both + streaming and polling modes. - **Pane-aware layout** — the browser viewport fits the pane without stretching or changing its responsive width; the console opens only when output exists. - **Real interaction** — clicks use Chrome mouse events rather than DOM selector @@ -197,8 +200,17 @@ viewport width—and therefore its responsive breakpoint—while fitting only th height to the pane's image area. The frame fills that area without stretching. On a quiet page, the image uses all rows between the header and controls. The -console region appears only after a console message or page error arrives; the -viewport then refits to the remaining image area. +console region appears only after a console message, page error, or failed +network request arrives; the viewport then refits to the remaining image area. + +Failed network requests paint as `✖ 404 GET ` (HTTP 400–599) or +`✖ no response GET ` (connection-level failures, detected after ~15 +seconds without a status). Only xhr, fetch, and document requests are watched — +images, stylesheets, and held-open streams (SSE, WebSocket) stay out. Failures +from before the pane attached are intentionally not replayed, repeated +identical failures are collapsed within a 60-second window, and on very long +sessions the feed turns itself off with a one-time note once the daemon's +request log outgrows the pane's read buffer. ## Session model diff --git a/bin/renderer.mjs b/bin/renderer.mjs index f912c36..18dd1ec 100644 --- a/bin/renderer.mjs +++ b/bin/renderer.mjs @@ -312,6 +312,85 @@ export function viewportForPane(frameWidth, { cols, imageRows }) { return { w, h }; } +// Failed-request diffing over `network requests --json`. The daemon's log is +// append-only from the pane's perspective but can be wiped without notice +// (browser relaunch, external --clear), and pid.N-format requestIds can +// restart after a relaunch — so membership sets are pruned to the ids present +// in the current log every poll instead of trusting counts or id uniqueness. +// Seen-set membership means "already reported OR resolved to a non-failure +// status": an in-flight request must stay classifiable until its status +// lands, or a 500 arriving one poll late would be swallowed forever. +export function newNetworkState() { + return { + seen: new Set(), // requestIds reported or resolved 2xx/3xx + recent: new Map(), // dedupe key -> last emit/suppress time (ms) + }; +} + +export function diffNetworkFailures(state, entries, nowMs, opts = {}) { + const ageThresholdMs = opts.ageThresholdMs ?? 15_000; + const dedupeWindowMs = opts.dedupeWindowMs ?? 60_000; + const maxPerPoll = opts.maxPerPoll ?? 5; + const currentIds = new Set(); + const candidates = []; + for (const e of entries) { + const id = e?.requestId; + if (typeof id !== "string" || !id) continue; + currentIds.add(id); + if (opts.baseline) { + state.seen.add(id); + continue; + } + if (state.seen.has(id)) continue; + const status = typeof e.status === "number" ? e.status : null; + if (status !== null) { + state.seen.add(id); + if (status >= 400 && status <= 599) + candidates.push({ method: e.method ?? "GET", url: e.url ?? "", status }); + continue; + } + // Null status = still in flight OR failed at the connection level; the + // daemon drops loadingFailed detail, so entry age past the threshold is + // the only failure signal — and it must be entry age, not poll count: + // pollDelay stretches ticks. Unaged ids stay out of `seen` so a status + // arriving on a later poll is still classified. + if (typeof e.timestamp === "number" && nowMs - e.timestamp > ageThresholdMs) { + state.seen.add(id); + candidates.push({ + method: e.method ?? "GET", + url: e.url ?? "", + status: null, + }); + } + } + // Prune to the live log so wipes and id reuse stay harmless. + for (const id of state.seen) if (!currentIds.has(id)) state.seen.delete(id); + for (const [k, t] of state.recent) + if (nowMs - t > dedupeWindowMs) state.recent.delete(k); + // Chrome retries a failed navigation with fresh requestIds and app retry + // loops mint new ids per attempt — dedupe by shape, refreshing the window + // on suppressed hits so a steady loop paints once, not once per minute. + const failures = []; + let overflow = 0; + for (const c of candidates) { + const key = `${c.method} ${c.url} ${c.status ?? "no response"}`; + const last = state.recent.get(key); + state.recent.set(key, nowMs); + if (last !== undefined && nowMs - last <= dedupeWindowMs) continue; + if (failures.length < maxPerPoll) failures.push(c); + else overflow++; + } + return { failures, overflow }; +} + +// Display text for one failure ("✖ " comes from pushConsole's error prefix). +// URLs are page-controlled and unbounded (data: URLs carry payloads) — +// sanitize and hard-cap before the line enters the 500-line store. +export function formatNetworkFailure({ method, url, status }) { + const shownUrl = truncate(sanitizeText(url), 200); + return `${status ?? "no response"} ${method} ${shownUrl}`; +} + // --- agent-browser access --- export function makeBrowser(session, bin = "agent-browser") { @@ -421,6 +500,20 @@ export function makeBrowser(session, bin = "agent-browser") { } }, streamStatus: async () => run("stream", "status"), + // Failed-request source (agent-browser >= 0.33). Read-only: never pass + // --clear — external agents share the daemon's request log. --type + // bounds payload (data: URLs and SSE/WS noise stay out) and is the + // signal filter: failed xhr/fetch/document is what a dev wants to see. + network: async () => { + const data = await run( + "network", + "requests", + "--type", + "xhr,fetch,document", + ); + if (Array.isArray(data?.requests)) return data.requests; + return Array.isArray(data) ? data : []; + }, sessionExists: async () => { try { const { stdout } = await pExecFile(bin, ["session", "list", "--json"], { @@ -505,6 +598,18 @@ export class Renderer { `shot-${safeWsId(env.HERDR_WORKSPACE_ID)}.jpg`, ); this.lastLiveCheck = 0; + // Failed-request feed (see pollNetwork). The baseline flag makes the + // first read after an attach swallow pre-attach history silently; the + // off latch stops polling for the rest of the attach once the daemon's + // unbounded log outgrows the exec limits — retrying a known-fatal + // multi-MiB read every tick would waste CPU forever with no output. + this.networkState = newNetworkState(); + this.networkBaselinePending = true; + this.networkOff = false; + this.networkPollBusy = false; + this.networkPollErrors = 0; + this.networkTimer = null; // live-mode cadence (see goLive/dropLive) + this.networkIdleTicks = 0; this.kittyAnon = false; // chafa emitted anonymous kitty placements this.lastImageDims = null; this.lastViewportRequest = ""; @@ -563,6 +668,96 @@ export class Renderer { } } + // Shared failure-feed poll for both modes (tick calls it after a healthy + // snapshot; the live-mode timer calls it directly). Returns true when it + // painted failures, so the live timer can hold its base cadence on a page + // whose only activity is failing requests. The in-flight guard prevents a + // timer poll and a tick poll from racing the same state across the + // dropLive transition. Never throws. + async pollNetwork(baseline = false) { + if (this.networkOff || this.networkPollBusy || !this.attached) + return false; + if (typeof this.browser.network !== "function") return false; // test doubles / older engines + this.networkPollBusy = true; + try { + const entries = await this.browser.network(); + const { failures, overflow } = diffNetworkFailures( + this.networkState, + entries, + Date.now(), + { baseline: baseline || this.networkBaselinePending }, + ); + this.networkBaselinePending = false; + this.networkPollErrors = 0; + if (!failures.length) return false; + const hadConsole = this.consoleLines.length > 0; + const lines = failures.map((f) => ({ + text: formatNetworkFailure(f), + type: "error", + })); + if (overflow) + lines.push({ + text: `…and ${overflow} more failed requests`, + type: "error", + }); + this.pushConsole(lines, false); + this.queueConsolePaint(hadConsole); + return true; + } catch (err) { + // The daemon log is unbounded and --clear is not ours to send: once + // the payload exceeds maxBuffer or the exec timeout, every retry is + // guaranteed to fail the same way. Latch off with one visible line. + const fatal = + /maxBuffer/i.test(err?.message ?? "") || err?.killed === true; + if (fatal) { + this.networkOff = true; + const hadConsole = this.consoleLines.length > 0; + this.pushConsole( + [{ text: "network reporting off — request log too large", type: "error" }], + false, + ); + this.queueConsolePaint(hadConsole); + } else { + this.networkPollErrors++; + } + return false; + } finally { + this.networkPollBusy = false; + } + } + + // Live mode has no snapshot tick and the push stream carries no network + // events, so failures need their own low-cadence poll. pollDelay-style + // backoff keeps an unwatched live pane near-free; the idle counter resets + // on stream activity AND on painted failures — a background retry loop on + // a visually static page produces neither frames nor console entries, so + // the failures themselves must hold the base cadence. + startNetworkTimer(baseMs = 4_000) { + if (this.networkTimer || typeof this.browser.network !== "function") + return; + const fire = async () => { + this.networkTimer = null; + if (!this.live || !this.attached || this.networkOff) return; + if (await this.pollNetwork()) this.networkIdleTicks = 0; + else this.networkIdleTicks++; + if (!this.live || this.networkOff) return; // dropped or latched mid-poll + this.networkTimer = setTimeout( + fire, + pollDelay(baseMs, this.networkIdleTicks), + ); + this.networkTimer.unref?.(); + }; + this.networkTimer = setTimeout(fire, baseMs); + this.networkTimer.unref?.(); + } + + stopNetworkTimer() { + if (this.networkTimer) { + clearTimeout(this.networkTimer); + this.networkTimer = null; + } + } + queueConsolePaint(hadConsole) { const layoutChanged = this.mode !== "text" && !hadConsole && this.consoleLines.length > 0; @@ -774,6 +969,11 @@ export class Renderer { this.banner = ""; this.streamCooldownUntil = 0; // try the live stream right away this.lastViewportRequest = ""; // fit the new session once + // A (re-)attach may be a different session under the same name: + // start the failure feed from a clean silent baseline every time. + this.networkState = newNetworkState(); + this.networkBaselinePending = true; + this.networkOff = false; } if (this.live) { // Event-driven: the stream paints everything; the poll loop only @@ -851,6 +1051,9 @@ export class Renderer { failed = true; this.failures++; } + // Best-effort by design: a broken failure feed must never take the + // frame/console paint path down with it (pollNetwork never throws). + if (!failed) await this.pollNetwork(); if (failed && this.failures >= 3) { if (await this.browser.sessionExists()) { this.banner = "agent-browser not responding — retrying"; @@ -1116,10 +1319,15 @@ export class Renderer { ws.onclose = drop; ws.onerror = drop; this.banner = ""; + this.networkIdleTicks = 0; + this.startNetworkTimer(); return true; } dropLive(note) { + // First: a timer poll must not fire into the poll-mode transition and + // race tick's poll over the same diff state. + this.stopNetworkTimer(); const wasLive = !!this.live; if (this.live) { try { @@ -1159,6 +1367,7 @@ export class Renderer { } this.shotFormat = "jpg"; this.frameSeq++; + this.networkIdleTicks = 0; // page activity: keep failure polls prompt this.enqueue(async () => { await this.renderImage(); await this.fitViewport(dims.w, dims.h); @@ -1166,6 +1375,7 @@ export class Renderer { break; } case "console": { + this.networkIdleTicks = 0; // page activity: keep failure polls prompt const hadConsole = this.consoleLines.length > 0; this.pushConsole( [{ text: m.text ?? "", type: m.level ?? "log" }], @@ -1260,6 +1470,22 @@ export class Renderer { // and ownership is claimed only after the open actually succeeds, so a // failed navigate can never make us kill someone else's session. const existed = await this.browser.sessionExists(); + // Failure-feed baseline splits on the same check: an existing session + // gets a real silent-baseline read before open (so only the navigation's + // own failures paint), while a not-yet-existing session gets state-only + // seeding — a pre-open network read would auto-create the session and + // break the ownership rule below. Fresh daemons arm request tracking at + // spawn, so the navigation's failures are in the log for the first + // post-attach poll either way. + this.networkState = newNetworkState(); + this.networkOff = false; + if (existed) { + this.networkBaselinePending = false; + this.attached = true; // pollNetwork requires it; the session exists + await this.pollNetwork(true); + } else { + this.networkBaselinePending = false; // nothing to swallow: empty log + } await this.browser.open(u); if (!existed) this.selfCreated = true; this.attached = true; // the user is explicitly starting/driving the session @@ -1298,6 +1524,7 @@ export class Renderer { cleanup() { if (this.mode === "kitty") process.stdout.write(KITTY_DELETE_ALL); + this.stopNetworkTimer(); try { this.live?.ws.close(); } catch { diff --git a/docs/plans/2026-08-03-001-feat-network-failures-console-plan.md b/docs/plans/2026-08-03-001-feat-network-failures-console-plan.md new file mode 100644 index 0000000..3247026 --- /dev/null +++ b/docs/plans/2026-08-03-001-feat-network-failures-console-plan.md @@ -0,0 +1,186 @@ +--- +title: "feat: Surface failed network requests in the console region" +type: feat +date: 2026-08-03 +--- + +# feat: Surface failed network requests in the console region + +## Summary + +Closes issue #1: failed network requests (4xx/5xx and no-response failures) appear in the pane's console region with the `✖ ` error prefix, in both poll and live-stream modes. Implemented as a requestId-set diff over `agent-browser network requests --json` — agent-browser 0.33.2's push stream carries no network event, so polling is the only path. + +## Problem Frame + +The console region shows console API output and page errors but never failed network requests — a 404 or connection-refused fetch is visible in `agent-browser network requests` yet invisible in the pane. The data already exists in the shared daemon session; it just isn't painted. The pane's core audience (localhost dev debugging) hits this constantly. + +--- + +## Requirements + +**Display** + +- R1. HTTP failures (status 400–599) on xhr/fetch/document requests appear in the console region as `✖ ` within one poll interval. +- R2. Connection-level failures (entries whose status never arrives) appear as `✖ no response ` once the entry is older than the age threshold. +- R3. Failure lines appear in live-stream mode too, via a low-cadence dedicated poll, since the WebSocket stream has no network event type. + +**Passivity and safety** + +- R4. The pane issues no network call before `attached` is true, never passes `--clear`, and never causes session auto-creation — the existing passivity contract holds unchanged. +- R5. URLs are display-sanitized (`sanitizeText`) and hard-truncated before storage; failure lines flow through `pushConsole` so the 500-line cap, lazy region open, and repaint gating keep working. + +**Noise control** + +- R6. Attaching to a session with existing failures in the log produces no replay wall — the first read seeds a silent baseline. Re-attach after session death re-baselines. +- R7. Repeated identical failures (Chrome nav retries, app retry loops) are deduped, and per-poll output is capped with a `…and N more failed requests` summary line. + +**Degradation** + +- R8. A failing or unsupported `network` subcommand degrades only network reporting — screenshots and console keep painting, and older agent-browser versions without the command leave the pane fully functional. +- R9. A failed navigation the pane itself initiated still surfaces (the baseline must not swallow the user's own 404). + +--- + +## Key Technical Decisions + +- **Poll-diff, not stream.** Verified against the vercel-labs/agent-browser 0.33.2 source: the per-session WebSocket emits only `url`, `frame`, `console`, `page_error`, `tabs`. Issue approach 1 (consume a network stream event) is impossible today; a follow-up upstream feature request is deferred work. +- **Separate exec, not the snapshot batch.** The snapshot batch uses `--bail` and throws on a short result array — an agent-browser version lacking the `network` subcommand would brick every tick (no frames, no console) if the command joined the batch. A separate `network requests --type xhr,fetch,document --json` exec degrades independently, and live mode needs a standalone call anyway. `--type` also bounds payload (spike observed `data:` URLs with full base64 payloads in the log; `maxBuffer` is 16 MiB). +- **Optional duck-typed `network()` method on `makeBrowser`.** Every call site guards `typeof this.browser.network === "function"` — the `streamEnable` precedent (`bin/renderer.mjs` `goLive()`) — so the dozens of existing object-literal test fakes stay green. +- **Diff by requestId with prune-to-current-log, not count-based reconcile.** A live spike on 0.33.2 observed the request log wiped without `--clear` (browser relaunch after a failed navigation recreated the page target), so `reconcileConsole`-style count/tail matching is unsafe here. Seen-set membership = ids already reported OR resolved to a non-failure status (2xx/3xx); unreported null-status ids live only in a pending-candidates state and are reclassified every poll — report immediately when status lands at 400–599, drop at 200–399, report as "no response" past the age threshold. This closes the slow-failure hole where a request observed in-flight at poll N would swallow its 500 arriving at poll N+1. Both sets are pruned to ids present in the current log (bounds growth; `pid.N`-format requestIds can restart after relaunch, so pruning makes stale collisions harmless). +- **"Failed" = status 400–599, plus null-status entries older than 15 s.** Spike-verified: entries carry a ms-epoch `timestamp`, and null-status entries have no error field anywhere (the daemon drops `Network.loadingFailed` detail), so lines can only say "no response". Age-based detection beats poll-count heuristics because `pollDelay` backoff stretches ticks up to 30×. The `--type xhr,fetch,document` filter excludes EventSource/WebSocket, whose held-open connections would otherwise false-positive every localhost HMR/SSE stream. +- **Silent baseline on attach — a deliberate divergence from console behavior.** The console region replays full history on attach (`reconcileConsole` from `count: 0`); the network feed starts silent instead, because stale failures from an hours-old agent session are noise, not signal. Re-baseline everywhere `attached` flips. On the pane's own `navigate()` path (which sets `attached` directly), split by the existing `existed` check: if the session already exists, run a real baseline read before `open` (safe — no auto-creation); if it does not exist yet, seed state-only (empty seen-set, baseline marked consumed, **no CLI call** — a pre-`open` read would auto-create a session and break the ownership invariant). R9 holds on the fresh-session path because streaming daemons arm request tracking at spawn, so the navigation's own failure is in the log for the first post-attach poll. +- **One shared poll function for both modes, with an in-flight guard.** Poll mode calls it from `tick()` after a successful snapshot; live mode runs it on its own timer with `pollDelay`-style backoff. A single guard prevents the dropLive-transition race where a timer poll and a tick poll diff the same seen-set concurrently. The live timer is cleared as the first statement of `dropLive()` and each firing checks `this.live && this.attached` before executing (R4). +- **Zero new dependencies, Node 20 floor.** Repo policy — only `node:` builtins, `node --test` runner. + +--- + +## High-Level Technical Design + +Directional guidance, not implementation specification. + +```mermaid +flowchart TB + subgraph shared [Shared] + PN[pollNetwork - in-flight guarded] + DIFF[diffNetworkFailures - pure, exported] + PC[pushConsole with error prefix] + PN --> DIFF --> PC + end + TICK[poll mode: tick after snapshot ok] --> PN + TIMER[live mode: timer with backoff] --> PN + ATTACH[attached flips true] -->|reset seen-set, set baseline-pending| PN + DROP[dropLive] -->|clear timer first| TIMER + NAV[navigate: real baseline read if session existed, state-only seed if creating] --> ATTACH +``` + +Line format: `✖ 404 GET https://api.example.com/users` / `✖ no response GET http://localhost:3000/api` — URL sanitized and truncated to ~200 chars before storage; per-poll overflow collapses into `✖ …and N more failed requests`. + +--- + +## Implementation Units + +### U1. Pure diff and formatting helpers + +- **Goal:** The diff/classification/dedupe/format logic exists as exported pure functions, fully unit-testable without a Renderer. +- **Requirements:** R1, R2, R6, R7 +- **Dependencies:** none +- **Files:** `bin/renderer.mjs` (helpers band), `tests/renderer.test.mjs` +- **Approach:** A `diffNetworkFailures(state, entries, nowMs, opts)`-shaped function taking the previous state (seen-set of reported-or-resolved-OK ids, pending null-status candidates, recent-failure memory) and the current entry list, returning new failure descriptors plus the next state (both id sets pruned to the current log). Classification per entry: status 400–599 → report unless already reported; status 200–399 → move to seen, never report; null status → pending, reported as "no response" only when `nowMs - timestamp > ageThreshold` (default 15 000 ms), and reclassified on every poll so a late-arriving status wins. Dedupe by `method + url + (status ?? "no response")`: within the returned batch, and across polls via a 60 s suppression window refreshed on each suppressed hit (a steady retry loop paints once, not every poll). Cap output at N (~5) per poll with an overflow count. A small formatter builds the display line (sanitize + truncate URL ~200 chars). +- **Patterns to follow:** `reconcileConsole` / `pollDelay` — exported pure helpers in the labeled helpers band; tabs, double quotes, why-not-what comments naming the failure mode. +- **Test scenarios:** + - New 404 entry → one `✖ 404 GET ` descriptor; same entry next poll → nothing (seen). + - Entry with null status and age 5 s → nothing; same entry at 20 s → `no response` descriptor; entry that gains status 200 before threshold → never reported. + - Entry with null status at poll 1, status 500 at poll 2 → exactly one `✖ 500` descriptor, at poll 2 (late-arriving failure status is not swallowed by the seen-set). + - Same failing `method + url + status` key on 5 consecutive polls → one line total (60 s cross-poll suppression window). + - Log wipe (current entries no longer contain seen ids) → seen-set pruned, no replay of surviving-but-already-seen ids re-added later with same id. + - Chrome nav retry shape: 3 entries, distinct requestIds, same URL + null status → one line, not three. + - 12 distinct failures in one poll with cap 5 → 5 lines + `…and 7 more failed requests`. + - Baseline call (`state` empty, `baseline: true` or equivalent) → zero descriptors, seen-set populated with all current ids. + - URL with ANSI escape bytes and 5 000 chars → sanitized, truncated to ~200 chars. +- **Verification:** `npm test` green; every scenario above has a direct assertion on the returned descriptors/state. + +### U2. `network()` method on the browser wrapper + +- **Goal:** `makeBrowser` exposes an optional `network()` returning the parsed request list, with the CLI contract pinned by stub tests. +- **Requirements:** R4, R8 +- **Dependencies:** none +- **Files:** `bin/renderer.mjs` (agent-browser access band), `tests/renderer.test.mjs` +- **Approach:** `network: async () => run("network", "requests", "--type", "xhr,fetch,document")`, normalizing the result to an array. No `--clear`, ever. +- **Patterns to follow:** the `streamEnable`/`run` wrappers; bash-stub CLI-contract tests in the existing batch-shape test style. +- **Test scenarios:** + - Stub returns `{requests: [...]}` → method resolves to the array; malformed JSON → rejects (caller degrades). + - Stub logs argv → assert exact args include `--type xhr,fetch,document` and `--json`, and assert `--clear` never appears (extend the existing never-destructive contract test). +- **Verification:** `npm test` green; argv assertions pass against the stub. + +### U3. Poll-mode integration in `tick()` + +- **Goal:** Poll mode paints failure lines; a broken network poll never affects frame/console painting. +- **Requirements:** R1, R2, R4, R5, R6, R8, R9 +- **Dependencies:** U1, U2 +- **Files:** `bin/renderer.mjs` (Renderer: constructor state, `tick()`, attach transition, `navigate()`), `tests/renderer.test.mjs` +- **Approach:** New constructor state (`networkState`, baseline-pending flag) following the `` naming convention. In `tick()`, after the snapshot succeeds, run the shared poll: duck-type guard, own try/catch that swallows into a counter (no banner — the feature is best-effort), baseline consumed on first read after any `attached` flip. On a maxBuffer/timeout-class failure from `network()`, stop polling for the rest of the attach and push a one-time `✖ network reporting off — request log too large` line — the daemon log is unbounded (verified: append-only Vec, `--clear` is the only eviction), so retrying a known-fatal multi-MiB exec every tick is pure waste. Push descriptors via `pushConsole([{text, type: "error"}], false)` so `consolePushes`/`sig()` backoff-reset work unchanged; use the tick's existing console layout dance for the first-line region open. In `navigate()`, split baseline seeding by the `existed` check per the KTD: real read before `open` when the session existed, state-only seed (no CLI call) when the pane is creating it. +- **Patterns to follow:** `tick()`'s swallow-and-degrade error idiom (counter, no unguarded paths); `suppressConsoleOnce` one-shot consumption shape (but a separate flag — do not reuse the console's). +- **Test scenarios:** + - Fake browser with `network` returning a 404 entry → after two ticks (baseline, then report… first tick baselines silently, entry present at baseline is NOT reported; a new failing id on tick 2 → `✖ 404` line appears in `consoleLines`). + - Fake browser without a `network` method (existing object-literal fakes) → all current tick tests pass unmodified. + - `network()` rejecting every tick → screenshots/console still paint, no banner, no uncaught rejection. + - Session death → `attached` false → recreation → re-attach → failures from before re-attach not replayed; new ones are. + - `network()` rejecting with a maxBuffer/timeout-class error → polling stops for the rest of the attach, exactly one `request log too large` line; a fresh re-attach resumes polling. + - `navigate()` to a refused port on the self-created path → no `network` call before `open` (call-log assertion), and the navigation's own failure line appears on the first post-attach poll. + - `navigate()` into an already-existing session → baseline read fires before `open`; prior failures not replayed, the navigation's own failure reports. + - Update `"tick stays passive when the session is missing"` — call sequence stays `["sessionExists"]` (no network call while unattached). + - Network lines in `consoleLines` do not perturb console reconcile: console entries still diff correctly afterward (guards the `consoleState`-vs-display separation). +- **Verification:** `npm test` green including the two deliberately-updated passivity tests; manual check with a linked pane (close/reopen the pane after edits — a running pane keeps the old renderer). + +### U4. Live-mode timer + +- **Goal:** Failure lines appear while the WebSocket stream is active, without violating passivity or the unwatched-pane-costs-nothing principle. +- **Requirements:** R3, R4, R7 +- **Dependencies:** U3 +- **Files:** `bin/renderer.mjs` (`goLive()`, `dropLive()`, live branch of `tick()` or a dedicated timer), `tests/renderer.test.mjs` +- **Approach:** Start a timer on successful `goLive()` (base ~4 s) that fires the shared poll with `pollDelay`-style backoff, reset by frame/console stream activity or by a poll that returned failure descriptors (a silently failing background fetch on a static page produces neither frames nor console entries — the failures themselves must hold the cadence). Guards: clear the timer as the first statement of `dropLive()`; each firing checks `this.live && this.attached`; shared in-flight guard prevents a timer poll and a tick poll racing across the dropLive transition. Paint via `queueConsolePaint(hadConsole)` (handles the console-region-opens layout change in live mode). +- **Patterns to follow:** `streamCooldownUntil`/`lastLiveCheck` cadence state; stream handlers' `queueConsolePaint` usage. +- **Test scenarios:** + - Live mode faked (`r.live = {ws: ...}`) with a failing entry appearing in `network()` → line lands in `consoleLines` after the timer fires (drive the timer hook directly rather than real time). + - `dropLive()` → timer cleared; an in-flight poll resolving after the drop pushes nothing twice (in-flight guard) and the post-drop tick does not re-report entries the timer already showed. + - Update `"live tick only watches liveness"` deliberately for the new call pattern. + - Timer firing after session death (`sessionExists` false path) → no session-creating call. +- **Test expectation note:** backoff-cadence exactness is not asserted (timing-flaky); assert the reset-on-activity state transitions instead. +- **Verification:** `npm test` green; a live-mode manual session shows failure lines within a few seconds. + +### U5. README documentation + +- **Goal:** The console-region docs describe failure lines and their limits. +- **Requirements:** R1, R2, R3 +- **Dependencies:** U3, U4 +- **Files:** `README.md` +- **Approach:** Extend the console-region description (Highlights + relevant Troubleshooting notes): what shows (`✖ 404 GET …`, `✖ no response …`), the xhr/fetch/document scope, the ~15 s delay on no-response detection, that history before pane attach is intentionally not replayed, and that failure reporting turns itself off on very long sessions once the daemon's unbounded request log outgrows the read buffer. +- **Test scenarios:** Test expectation: none — documentation only. +- **Verification:** README reads accurately against shipped behavior. + +--- + +## Scope Boundaries + +**In scope:** failure display in the console region, both modes, tests, README. + +**Not in scope:** + +- Successful-request display, request detail drill-down, HAR anything. +- Merging failure lines chronologically with console entries — polling makes interleaving inherently approximate; accepted. +- The pane calling `network requests --clear` to bound daemon log growth — the pane is passive and external agents may depend on the log. + +**Deferred to follow-up work:** + +- Upstream feature request to vercel-labs/agent-browser for a `network` stream event (read their CONTRIBUTING and templates first), which would replace the live-mode timer entirely. +- Upstream issue for `tracked_requests` growth: verified unbounded in 0.33.2 (append-only Vec, `--clear` is the only eviction, armed at daemon spawn on streaming daemons) — request a cap or eviction policy. + +--- + +## Risks & Dependencies + +- **agent-browser flag churn.** A `-labs` project; every CLI fact here was verified against 0.33.2 (installed) via the upstream source and a live spike. The separate-exec + duck-type design means a future breaking change degrades network reporting only. Re-verify `network requests` argv against the installed version at implementation time. +- **Arming semantics (verified in 0.33.2 source).** Daemons started with a stream server arm request tracking at startup (`new_with_stream` sets `request_tracking = true`); non-stream daemons lazy-arm on the first `network requests` call. herdr sessions run streaming daemons, which is why the spike saw pre-existing requests tracked. Both paths are handled by the unconditional baseline read. +- **Unbounded daemon request log.** On a long-lived, request-heavy session the `network requests` payload grows monotonically toward the 16 MiB `maxBuffer` / 10 s exec timeout; when either trips, the pane disables network reporting for the rest of the attach with a one-time console note (U3). The real fix is upstream (deferred follow-up). +- **Long-poll XHR false positives.** An XHR held open past 15 s with no response reports as failed. The type filter removes the worst offenders (SSE/WS); dedupe and the per-poll cap bound the residual noise. Revisit the threshold if real-world use complains. +- **Console-line eviction.** A burst of failures (dead API server) can evict genuine console lines from the 500-line ring; the per-poll cap is the mitigation, accepted as sufficient. diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index 718b393..3278d19 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -24,6 +24,9 @@ import { safeWsId, kittyImageSequence, viewportForPane, + newNetworkState, + diffNetworkFailures, + formatNetworkFailure, } from "../bin/renderer.mjs"; const repoRoot = path.resolve( @@ -1489,3 +1492,442 @@ test("empty console gives its rows to the browser until output arrives", () => { assert.ok(visible.consoleRows >= 4); assert.equal(visible.imageRows, empty.imageRows - visible.consoleRows); }); + +// --- Wave 3: failed network requests in the console region --- + +const req = (id, over = {}) => ({ + requestId: id, + url: `https://api.test/${id}`, + method: "GET", + resourceType: "Fetch", + timestamp: 1_000_000, + ...over, +}); +const T0 = 1_000_000; + +test("network diff: new 404 reported once, then seen", () => { + const st = newNetworkState(); + const first = diffNetworkFailures(st, [req("a", { status: 404 })], T0 + 10); + assert.equal(first.failures.length, 1); + assert.equal(first.failures[0].status, 404); + const second = diffNetworkFailures(st, [req("a", { status: 404 })], T0 + 20); + assert.equal(second.failures.length, 0, "same entry must not re-report"); +}); + +test("network diff: null status ages into no-response, 200 never reports", () => { + const st = newNetworkState(); + const young = diffNetworkFailures(st, [req("a")], T0 + 5_000); + assert.equal(young.failures.length, 0, "5s old in-flight is not a failure"); + const aged = diffNetworkFailures(st, [req("a")], T0 + 20_000); + assert.equal(aged.failures.length, 1); + assert.equal(aged.failures[0].status, null); + const st2 = newNetworkState(); + diffNetworkFailures(st2, [req("b")], T0 + 5_000); + const ok = diffNetworkFailures(st2, [req("b", { status: 200 })], T0 + 9_000); + assert.equal(ok.failures.length, 0); + const later = diffNetworkFailures(st2, [req("b", { status: 200 })], T0 + 60_000); + assert.equal(later.failures.length, 0, "resolved-OK id stays swallowed"); +}); + +test("network diff: failure status arriving one poll late still reports", () => { + const st = newNetworkState(); + const inflight = diffNetworkFailures(st, [req("a")], T0 + 1_000); + assert.equal(inflight.failures.length, 0); + const landed = diffNetworkFailures(st, [req("a", { status: 500 })], T0 + 3_000); + assert.equal(landed.failures.length, 1, "late 500 must not be swallowed"); + assert.equal(landed.failures[0].status, 500); +}); + +test("network diff: log wipe prunes state without replay", () => { + const st = newNetworkState(); + diffNetworkFailures(st, [req("a", { status: 404 })], T0 + 10); + assert.ok(st.seen.has("a")); + // Wipe: log now holds only a fresh id; 'a' evaporates from state. + const after = diffNetworkFailures(st, [req("z", { status: 200 })], T0 + 20); + assert.equal(after.failures.length, 0); + assert.ok(!st.seen.has("a"), "seen pruned to current log"); + // Reused id after relaunch is a new request, judged on its own status — + // dedupe by shape (not id) is what suppresses the repeat line. + const reused = diffNetworkFailures( + st, + [req("z", { status: 200 }), req("a", { status: 404 })], + T0 + 30, + ); + assert.equal(reused.failures.length, 0, "same shape within window dedupes"); + assert.ok(st.seen.has("a"), "reused id still classified and tracked"); +}); + +test("network diff: nav-retry burst dedupes to one line", () => { + const st = newNetworkState(); + const out = diffNetworkFailures( + st, + [ + req("r1", { url: "https://x.invalid/", timestamp: T0 - 60_000 }), + req("r2", { url: "https://x.invalid/", timestamp: T0 - 60_000 }), + req("r3", { url: "https://x.invalid/", timestamp: T0 - 60_000 }), + ], + T0, + ); + assert.equal(out.failures.length, 1, "3 retry entries paint one line"); + assert.equal(out.overflow, 0, "deduped entries are not overflow"); +}); + +test("network diff: cross-poll retry loop stays suppressed within window", () => { + const st = newNetworkState(); + let lines = 0; + for (let i = 0; i < 5; i++) { + const out = diffNetworkFailures( + st, + [req(`try${i}`, { url: "https://api.test/beacon", status: 502 })], + T0 + i * 5_000, + ); + lines += out.failures.length; + } + assert.equal(lines, 1, "steady 5s retry loop paints once inside 60s window"); +}); + +test("network diff: per-poll cap emits overflow count", () => { + const st = newNetworkState(); + const entries = []; + for (let i = 0; i < 12; i++) + entries.push(req(`e${i}`, { url: `https://api.test/${i}`, status: 500 })); + const out = diffNetworkFailures(st, entries, T0); + assert.equal(out.failures.length, 5); + assert.equal(out.overflow, 7); +}); + +test("network diff: baseline swallows everything silently", () => { + const st = newNetworkState(); + const out = diffNetworkFailures( + st, + [req("a", { status: 404 }), req("b"), req("c", { status: 500 })], + T0, + { baseline: true }, + ); + assert.equal(out.failures.length, 0); + const next = diffNetworkFailures( + st, + [req("a", { status: 404 }), req("b"), req("c", { status: 500 })], + T0 + 1_000, + ); + assert.equal(next.failures.length, 0, "baselined ids never replay"); +}); + +test("network format: sanitizes and hard-caps page-controlled URLs", () => { + const nasty = `https://api.test/${"\x1b[2J"}${"x".repeat(5000)}`; + const line = formatNetworkFailure({ method: "GET", url: nasty, status: 404 }); + assert.ok(line.startsWith("404 GET https://api.test/")); + assert.ok(!line.includes("\x1b"), "escape bytes stripped"); + assert.ok(line.length <= 220, "stored line is capped"); + assert.equal( + formatNetworkFailure({ method: "POST", url: "http://l:3000/a", status: null }), + "no response POST http://l:3000/a", + ); +}); + +test("makeBrowser.network passes the type filter, never --clear", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "hb-net-")); + const logf = path.join(dir, "log"); + const stub = path.join(dir, "ab-stub"); + fs.writeFileSync( + stub, + `#!/usr/bin/env bash +echo "$@" >> "${logf}" +printf '%s' '{"success":true,"data":{"requests":[{"requestId":"r1","url":"https://x/a","method":"GET","status":404,"timestamp":1000,"resourceType":"Fetch"}]}}' +`, + ); + fs.chmodSync(stub, 0o755); + const reqs = await makeBrowser("s", stub).network(); + assert.equal(reqs.length, 1); + assert.equal(reqs[0].requestId, "r1"); + const logged = fs.readFileSync(logf, "utf8"); + assert.match( + logged, + /--session s network requests --type xhr,fetch,document --json/, + ); + assert.ok(!logged.includes("--clear"), "pane must never clear the shared log"); +}); + +test("makeBrowser.network rejects on malformed output so callers degrade", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "hb-netbad-")); + const stub = path.join(dir, "ab-stub"); + fs.writeFileSync(stub, `#!/usr/bin/env bash\nprintf 'not json'\n`); + fs.chmodSync(stub, 0o755); + await assert.rejects(makeBrowser("s", stub).network(), /non-JSON/); +}); + +// U3: poll-mode failure feed integration. + +const netEntry = (id, over = {}) => ({ + requestId: id, + url: `https://api.test/${id}`, + method: "GET", + resourceType: "Fetch", + timestamp: Date.now(), + ...over, +}); +const pollFake = (netQueue, calls = []) => ({ + sessionExists: async () => { + calls.push("sessionExists"); + return true; + }, + snapshot: async (f) => { + calls.push("snapshot"); + fs.writeFileSync(f, PNG_1PX); + return { url: "https://x/", title: "T", entries: [] }; + }, + network: async () => { + calls.push("network"); + const next = netQueue.length > 1 ? netQueue.shift() : netQueue[0]; + if (next instanceof Error) throw next; + return next; + }, +}); +const quietPoll = (r) => { + quiet(r); + r.redrawAll = async () => {}; + r.fitViewport = async () => false; + r.streamCooldownUntil = Number.MAX_SAFE_INTEGER; // stay in poll mode + return r; +}; + +test("tick: first poll baselines silently, later failure paints with ✖", async () => { + const r = quietPoll(mkRenderer()); + r.browser = pollFake([ + [netEntry("old", { status: 404 })], + [netEntry("old", { status: 404 }), netEntry("fresh", { status: 500 })], + ]); + await r.tick(); // attach + baseline: 'old' swallowed + await flush(); + assert.deepEqual(r.consoleLines, [], "baseline paints nothing"); + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /^✖ 500 GET https:\/\/api\.test\/fresh/); +}); + +test("tick: broken network feed degrades silently, pane keeps painting", async () => { + const r = quietPoll(mkRenderer()); + r.browser = pollFake([new Error("weird transient failure")]); + await r.tick(); + await r.tick(); + await flush(); + assert.deepEqual(r.consoleLines, []); + assert.equal(r.banner, "", "no banner for a best-effort feature"); + assert.equal(r.failures, 0, "tick failure counter untouched"); + assert.ok(r.networkPollErrors >= 2); +}); + +test("tick: maxBuffer-class failure latches the feed off with one line", async () => { + const r = quietPoll(mkRenderer()); + const calls = []; + r.browser = pollFake( + [new Error("stdout maxBuffer length exceeded")], + calls, + ); + await r.tick(); + await r.tick(); + await r.tick(); + await flush(); + assert.equal(r.networkOff, true); + assert.deepEqual( + r.consoleLines, + ["✖ network reporting off — request log too large"], + "exactly one visible off note", + ); + assert.equal( + calls.filter((c) => c === "network").length, + 1, + "no retries after the latch", + ); +}); + +test("tick: re-attach re-baselines instead of replaying", async () => { + const r = quietPoll(mkRenderer()); + let alive = true; + const netQueue = [[netEntry("preexisting", { status: 503 })]]; + r.browser = { + ...pollFake(netQueue), + sessionExists: async () => alive, + }; + await r.tick(); // attach + baseline + await flush(); + // Session dies: three failed snapshots detach the pane. + const goodSnapshot = r.browser.snapshot; + r.browser.snapshot = async () => { + throw new Error("session gone"); + }; + alive = false; + await r.tick(); + await r.tick(); + await r.tick(); + assert.equal(r.attached, false, "death detaches"); + // Session comes back under the same name with old failures in its log. + alive = true; + r.browser.snapshot = goodSnapshot; + await r.tick(); // re-attach: baseline swallows 'preexisting' again + await flush(); + assert.deepEqual(r.consoleLines, [], "no replay across re-attach"); + netQueue[0] = [netEntry("preexisting", { status: 503 }), netEntry("new1", { status: 404 })]; + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /404 GET https:\/\/api\.test\/new1/); +}); + +test("navigate: existing session baselines before open; nav failure still reports", async () => { + const r = quietPoll(mkRenderer()); + const calls = []; + const netQueue = [[netEntry("stale", { status: 500 })]]; + r.browser = { + ...pollFake(netQueue, calls), + open: async () => { + calls.push("open"); + }, + }; + await r.navigate("https://localhost:3000/"); + assert.ok( + calls.indexOf("network") < calls.indexOf("open"), + "baseline read fires before open on an existing session", + ); + assert.deepEqual(r.consoleLines, [], "stale failure swallowed"); + netQueue[0] = [netEntry("stale", { status: 500 }), netEntry("nav", { status: 404 })]; + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /404 GET https:\/\/api\.test\/nav/); +}); + +test("navigate: fresh session gets no pre-open network call, nav failure reports", async () => { + const r = quietPoll(mkRenderer()); + const calls = []; + let exists = false; + const netQueue = [[]]; + r.browser = { + ...pollFake(netQueue, calls), + sessionExists: async () => { + calls.push("sessionExists"); + return exists; + }, + open: async () => { + calls.push("open"); + exists = true; + }, + }; + await r.navigate("https://localhost:3000/"); + assert.ok( + !calls.slice(0, calls.indexOf("open")).includes("network"), + "no session-creating read before open", + ); + assert.equal(r.selfCreated, true); + netQueue[0] = [netEntry("nav", { status: null, timestamp: Date.now() - 20_000 })]; + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /^✖ no response GET/); +}); + +test("network lines in consoleLines do not perturb console reconcile", async () => { + const r = quietPoll(mkRenderer()); + let consoleEntries = []; + const netQueue = [[]]; + r.browser = { + ...pollFake(netQueue), + snapshot: async (f) => { + fs.writeFileSync(f, PNG_1PX); + return { url: "https://x/", title: "T", entries: consoleEntries }; + }, + }; + await r.tick(); // baseline + netQueue[0] = [netEntry("bad", { status: 500 })]; + await r.tick(); // paints the failure line + await flush(); + assert.equal(r.consoleLines.length, 1); + consoleEntries = [{ text: "page says hi", type: "log" }]; + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 2, "console entry appended once"); + assert.equal(r.consoleLines.at(-1), " page says hi"); + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 2, "no duplicate on the next tick"); +}); + +// U4: live-mode network timer. + +test("live timer: fires the shared poll and paints while streaming", async () => { + const r = quietPoll(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.networkBaselinePending = false; + const calls = []; + r.browser = pollFake([[netEntry("bad", { status: 500 })]], calls); + r.startNetworkTimer(5); + await new Promise((res) => setTimeout(res, 60)); + r.stopNetworkTimer(); + await flush(); + assert.ok(calls.includes("network"), "timer polled the daemon"); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /^✖ 500 GET/); +}); + +test("live timer: painted polls hold base cadence, empty polls back off", async () => { + const r = quietPoll(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.browser = { network: async () => [] }; + let painted = true; + r.pollNetwork = async () => painted; + r.startNetworkTimer(5); + await new Promise((res) => setTimeout(res, 40)); + assert.equal(r.networkIdleTicks, 0, "painted failures reset the counter"); + painted = false; + await new Promise((res) => setTimeout(res, 40)); + r.stopNetworkTimer(); + assert.ok(r.networkIdleTicks > 0, "quiet polls accumulate idle ticks"); +}); + +test("live timer: dropLive clears it first; no fire after drop", async () => { + const r = quietPoll(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.networkBaselinePending = false; + const calls = []; + r.browser = pollFake([[]], calls); + r.startNetworkTimer(20); + r.dropLive(); + assert.equal(r.networkTimer, null, "timer cleared on drop"); + await new Promise((res) => setTimeout(res, 60)); + assert.ok(!calls.includes("network"), "no poll after the stream dropped"); +}); + +test("live timer: in-flight guard collapses concurrent polls", async () => { + const r = quietPoll(mkRenderer()); + r.attached = true; + r.networkBaselinePending = false; + let netCalls = 0; + let release; + r.browser = { + network: async () => { + netCalls++; + await new Promise((res) => { + release = res; + }); + return []; + }, + }; + const p1 = r.pollNetwork(); + const p2 = r.pollNetwork(); + release([]); + const [r1, r2] = await Promise.all([p1, p2]); + assert.equal(netCalls, 1, "second poll skipped while one is in flight"); + assert.equal(r2, false); + assert.equal(r1, false); +}); + +test("live timer: never starts for browsers without network()", () => { + const r = quietPoll(mkRenderer()); + r.browser = { sessionExists: async () => true }; + r.startNetworkTimer(5); + assert.equal(r.networkTimer, null); +});