From 1503c7d07962117799f5a7bab334ffa8690a3e26 Mon Sep 17 00:00:00 2001 From: Timur Mashara <62305878+tmashara@users.noreply.github.com> Date: Sat, 24 Oct 2026 16:05:28 +0200 Subject: [PATCH] --cache-right: never replay a hang, keep POST bodies, flag live misses Drops cancelled (-1) entries along with errors, keys entries by request body too, and lists requests the recording lacks as a failed asset on the right so a half-live reference is never clean or reused. Blocks service workers on both sides while caching, writes the recording 0600 with a .gitignore, and prints both right: lines when a rerun replays and records. Co-Authored-By: Claude Opus 5.5 --- docs/how-it-works.md | 13 +++++++++---- skills/renderdiff/reference.md | 3 ++- src/cache.ts | 15 +++++++++------ src/run.ts | 18 +++++++++++++++++- tests/unit/cache.test.ts | 22 +++++++++++++++++++++- 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/docs/how-it-works.md b/docs/how-it-works.md index b7eabeb..8ece5f2 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -49,10 +49,15 @@ honestly. This is what renderdiff does to make sure of that. the run, and was made with the same right origin, right storage state, blocked URLs, pinned context options, renderdiff version and browser. Otherwise the right side loads live and is recorded again, replacing any older recording; with `--rerun-failed` it covers only the - rerun shots. A request the recording lacks goes to the network. Error responses aren't - recorded, nor are shots that errored or failed an asset, so an error is never replayed. A shot - measured against a replay is marked `rightReplayedFrom`, and `--rerun-failed` reuses it only - in a run that would replay it too. The run says when it replayed (`right:` line, + rerun shots. Only responses from 200 to 399 are recorded, one per method, URL and request + body, so an error or a cancelled request is never replayed; a shot that errored or failed an + asset doesn't count toward coverage. A request the recording lacks goes to the network, and + the shot lists it as failed (`N request(s) not in the recording, loaded live`), so a half-live + reference is never clean. Service workers are blocked on both sides while `--cache-right` is + on, since a HAR can't see what they serve. The recording holds the right side's cookies and + auth headers: it's written 0600 with a `.gitignore` beside it. A shot measured against a + replay is marked `rightReplayedFrom`, and `--rerun-failed` reuses it only in a run that would + replay it too. The run says when it replayed (`right:` line, `meta.rightReplayedFrom`), because a recording can't see a deploy to the reference. - **Stitches tall pages.** A single screenshot past 16,384px can come out truncated or wrapped (Chrome repeats the top of the page; recent headless Chromium does not), so taller diff --git a/skills/renderdiff/reference.md b/skills/renderdiff/reference.md index cc0fe12..982888c 100644 --- a/skills/renderdiff/reference.md +++ b/skills/renderdiff/reference.md @@ -111,7 +111,8 @@ cookie script ran wedges the renderer). renders live, so `dom` and the refusals still hold, but a deploy to the reference goes unseen until the recording expires. A `right: replayed from …` line says so. Never give a verdict from a replayed run alone. A `--rerun-failed --cache-right` re-record covers only the rerun - shots. + shots. `failed right: N request(s) not in the recording, loaded live` means the reference was + part replay, part live: drop the flag rather than trust or accept that shot. - **The out dir is wiped at the start of every run** (except `--rerun-failed`). To keep a previous run's evidence next to a new one, pass `--out renderdiff-before` / `--out renderdiff-after`. A directory holding files renderdiff didn't write is refused (exit 2). diff --git a/src/cache.ts b/src/cache.ts index c763379..dec67c2 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -25,7 +25,7 @@ export function cacheFile(cfg: ResolvedConfig, browser: { name: string; version: } interface Entry { - request: { method: string; url: string }; + request: { method: string; url: string; postData?: { text?: string } }; response: { status: number }; } @@ -54,12 +54,13 @@ export function saveRecording(file: string, parts: string[], shots: string[], re if (!hars.length || !shots.length) return; const seen = new Set(); const har = hars[0]!; - // Replay serves the first match, so a one-off error would answer every later load. + // Errors would answer every later load; a cancelled request (-1) replays as a hang. har.log.entries = hars .flatMap((h) => h.log.entries) .filter((e) => { - const key = `${e.request.method} ${e.request.url}`; - if (e.response.status >= 400 || seen.has(key)) return false; + const key = JSON.stringify([e.request.method, e.request.url, e.request.postData?.text]); + const ok = e.response.status >= 200 && e.response.status < 400; + if (!ok || seen.has(key)) return false; seen.add(key); return true; }); @@ -67,9 +68,11 @@ export function saveRecording(file: string, parts: string[], shots: string[], re for (const f of fs.readdirSync(dir)) { if (f.startsWith("right-")) fs.rmSync(path.join(dir, f), { force: true }); } - fs.writeFileSync(file, JSON.stringify(har)); + // It holds the right side's cookies and auth headers. + fs.writeFileSync(path.join(dir, ".gitignore"), "*\n"); + fs.writeFileSync(file, JSON.stringify(har), { mode: 0o600 }); // Written last: without it the recording is never replayed. - fs.writeFileSync(`${file}.json`, JSON.stringify({ recordedAt, shots })); + fs.writeFileSync(`${file}.json`, JSON.stringify({ recordedAt, shots }), { mode: 0o600 }); } finally { for (const p of parts) fs.rmSync(p, { force: true }); } diff --git a/src/run.ts b/src/run.ts index de6dc78..a166abf 100644 --- a/src/run.ts +++ b/src/run.ts @@ -248,14 +248,26 @@ export async function run({ const worker = async (browser: Browser, i: number) => { // One context per origin so cookies/localStorage never cross between them. + // A service worker answers requests a HAR never sees; blocked on both sides to stay symmetric. + const sw = har ? { serviceWorkers: "block" as const } : {}; const leftCtx = await browser.newContext({ ...CONTEXT_OPTIONS, + ...sw, storageState: cfg.storageState.left ?? undefined, }); const rightCtx = await browser.newContext({ ...CONTEXT_OPTIONS, + ...sw, storageState: cfg.storageState.right ?? undefined, }); + // Registered before the HAR route, so it sees only what the recording lacks. + let missed = 0; + if (replayedAt) { + await rightCtx.route("**", (r) => { + missed++; + return r.fallback(); + }); + } if (har) { await rightCtx.routeFromHAR( replayedAt ? har : part(i), @@ -278,6 +290,7 @@ export async function run({ // Its own line: parse.ts reads the route on the row as one \S+ token. const sel = route.selector ? `selector ${selectorText(route.selector)}` : ""; try { + missed = 0; // allSettled, not all: a page must not start the next job while still loading this one. const settled = await Promise.allSettled([ shoot(leftPage, cfg.left + route.left, route, "left", width, cfg, now), @@ -296,6 +309,8 @@ export async function run({ `(left: ${leftShot.blank}; right: ${rightShot.blank}) at ${key}`, ); } + // A replay that went live in part compared against two different moments of the reference. + if (missed) rightShot.failed.push(`${missed} request(s) not in the recording, loaded live`); const unstablePixels = await cancelUnstable(leftShot, rightShot); const { diffPng, spots, ...metrics } = await compare(leftShot, rightShot); @@ -468,7 +483,8 @@ export async function run({ } if (meta.rightReplayedFrom) { console.log(`right: replayed from a recording of ${meta.rightReplayedFrom} (--cache-right)`); - } else if (har) { + } + if (har && !replayedAt) { console.log( unsaved ? `right: loaded live; recording not saved: ${unsaved}` diff --git a/tests/unit/cache.test.ts b/tests/unit/cache.test.ts index 1146bd5..b36599f 100644 --- a/tests/unit/cache.test.ts +++ b/tests/unit/cache.test.ts @@ -59,12 +59,32 @@ describe("replayable", () => { assert.deepEqual(JSON.parse(fs.readFileSync(file, "utf8")).log.entries, [entry("/a")]); }); + test("a cancelled request (-1) is dropped, and a good copy of the same URL stays", () => { + const { file } = record([a], now, [[entry("/a.css", -1)], [entry("/a.css")]]); + assert.deepEqual(JSON.parse(fs.readFileSync(file, "utf8")).log.entries, [entry("/a.css")]); + }); + + test("POSTs to one URL with different bodies are all kept", () => { + const post = (text: string) => ({ + request: { method: "POST", url: "/api", postData: { text } }, + response: { status: 200 }, + }); + const { file } = record([a], now, [[post("a"), post("b"), post("a")]]); + assert.deepEqual(JSON.parse(fs.readFileSync(file, "utf8")).log.entries, [post("a"), post("b")]); + }); + + test("the recording is private to the user and ignored by git", () => { + const { file } = record([a], now); + assert.equal(fs.statSync(file).mode & 0o777, 0o600); + assert.equal(fs.readFileSync(path.join(path.dirname(file), ".gitignore"), "utf8"), "*\n"); + }); + test("a new recording replaces every other one beside it", () => { const dir = tempDir(); fs.writeFileSync(path.join(dir, "right-old.har"), "{}"); fs.writeFileSync(path.join(dir, "right-old.har.json"), "{}"); record([a], now, undefined, dir); - assert.deepEqual(fs.readdirSync(dir).sort(), ["right-x.har", "right-x.har.json"]); + assert.deepEqual(fs.readdirSync(dir).sort(), [".gitignore", "right-x.har", "right-x.har.json"]); }); test("a shot it lacks, an expired or future recording, or none at all loads live", () => {