From e5a5aed5befbc424240b363fc0252e98256d9c18 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:28:02 -0700 Subject: [PATCH] test(web-fetch): count scans instead of timing them in the no-rescan guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The no-rescan test asserted that 1,000 refused lookups finish inside 50 ms. That measures the CI runner as much as the code — 62.47 ms on #1768 attempt 1, green on attempt 2, and red on #2446 too — and it also misses the regression it exists to catch: a resolve that re-derives from the stored texts once per instance passes the clock in 8 ms. The guarantee is a count, so count it. webAddressProvenance keeps a monotonic scan counter, read-only to callers and never read inside the module, and the test asserts three adds scan three times and that 1,000 refused lookups plus one accepted lookup scan nothing further. Fixes #2455 Co-Authored-By: Claude Opus 5 Co-authored-by: nova Agent-Model: claude/claude-opus-5/high --- .../services/webAddressProvenance.test.js | 25 ++++++++++++++----- backend/services/webAddressProvenance.ts | 14 +++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/backend/__tests__/unit/services/webAddressProvenance.test.js b/backend/__tests__/unit/services/webAddressProvenance.test.js index 13993e1dac..e2d3368e05 100644 --- a/backend/__tests__/unit/services/webAddressProvenance.test.js +++ b/backend/__tests__/unit/services/webAddressProvenance.test.js @@ -13,7 +13,7 @@ */ const { - addressKey, addressesIn, addressWasGiven, GivenAddresses, + addressKey, addressesIn, addressWasGiven, GivenAddresses, textScanCount, } = require('../../../services/webAddressProvenance'); describe('webAddressProvenance', () => { @@ -144,14 +144,27 @@ describe('webAddressProvenance', () => { expect(given.resolve('')).toBeNull(); }); + // Counted, not timed. This assertion was a 50 ms budget for 1,000 lookups, + // which measures the runner as much as the code: it flaked at 62.47 ms on + // #1768 and again on #2446 (TASK-347). A rescan is a count, so count it — + // ONE rescan fails this now, where the clock needed enough of them to + // exceed a budget, and a cold or loaded machine no longer fails it at all. test('each text is scanned once: refused attempts against three hostile pages cost no rescan', () => { const given = new GivenAddresses(); - const page = Array.from({ length: 400 }, (_, i) => `https://h${i}.example/p${'!'.repeat(50)}x`).join(' '); - [page, page, page].forEach((text) => given.add(text)); + const address = (i) => `https://h${i}.example/p${'!'.repeat(50)}x`; + const page = Array.from({ length: 400 }, (_, i) => address(i)).join(' '); - const started = process.hrtime.bigint(); - for (let i = 0; i < 1000; i += 1) given.has(`https://attacker.example/c?d=${i}`); - expect(Number(process.hrtime.bigint() - started) / 1e6).toBeLessThan(50); + const beforeAdds = textScanCount(); + [page, page, page].forEach((text) => given.add(text)); + expect(textScanCount() - beforeAdds).toBe(3); + + const afterAdds = textScanCount(); + for (let i = 0; i < 1000; i += 1) { + expect(given.has(`https://attacker.example/c?d=${i}`)).toBe(false); + } + // An accepted attempt is a lookup too, so it may not rescan either. + expect(given.has(address(7))).toBe(true); + expect(textScanCount()).toBe(afterAdds); }); }); }); diff --git a/backend/services/webAddressProvenance.ts b/backend/services/webAddressProvenance.ts index e629472f19..827612dcb9 100644 --- a/backend/services/webAddressProvenance.ts +++ b/backend/services/webAddressProvenance.ts @@ -75,8 +75,22 @@ const parseAddress = (raw: string): { key: string; url: string } | null => { /** The comparable form of one written address, or null when it is not one. */ export const addressKey = (raw: string): string | null => parseAddress(raw)?.key ?? null; +/** + * How many texts have been scanned for addresses, ever, in this process. + * + * Instrumentation, not behaviour: the no-rescan guarantee below ("each given + * text is scanned once, and a refused attempt costs no rescan") used to be + * asserted with a wall clock, which measures the shared CI runner as much as + * the code and flaked at 62 ms against a 50 ms budget (TASK-347). A rescan is + * a count, so count it: one rescan fails the test, and a slow machine does + * not. Monotonic and never read by anything here. + */ +let textScans = 0; +export const textScanCount = (): number => textScans; + /** Every address written in `text`, keyed by its comparable form, first writing kept. */ const writtenAddresses = (text: string): Map => { + textScans += 1; const found = new Map(); for (const token of String(text || '').split(TOKEN_SEPARATORS)) { if (!token.includes('.')) continue;