diff --git a/docs/gate/compiler.md b/docs/gate/compiler.md index 4cc63c5..42b1188 100644 --- a/docs/gate/compiler.md +++ b/docs/gate/compiler.md @@ -4,7 +4,7 @@ doc_type: spec status: draft owner: B3 created: 2026-07-25 -updated: 2026-07-27 +updated: 2026-07-28 confidence: MED supersedes: null sources_verified: true @@ -128,6 +128,23 @@ listed and the fallback was useless. It is now genuinely visibility-filtered, an `page-state` run **one** enumeration (`src/shared/landmarks.ts`) rather than two that agreed only on markup with redundant `role=` attributes. +### `timeout_ms` is part of strength, not a performance knob + +Every synthesized assertion carries `DEFAULT_ASSERTION_TIMEOUT_MS` (5000 ms, +`src/compiler/assertions.ts`) — previously seven separate `5000` literals, now one named +constant, overridable per compile via `CompileOptions.assertionTimeoutMs`. + +**The value is deliberately unmoved.** A shorter timeout is a *stricter* check and a longer one +laxer, so "tuning it for speed" would move step-level replay-validity — the number PRD §9 gates +on — while looking like a perf change. That is the shape the assertion-immutability invariant +forbids. + +It is also the dominant term in worst-case replay latency, because +`src/runner/assertions.ts` spends the full budget on **failure**: a 12-step task with three +stale locators waits 3 × 5 s before repair even starts. That is a real cost worth revisiting — +but on evidence, after a measurement, not before one. `tests/unit/compiler.test.ts` pins the +emitted default so it cannot drift silently. + **Strength rule:** `strong` = unambiguous proof the step achieved its purpose; `weak` = consistent with success but also with several failures. Weak is allowed and **must stay labelled** (`strength` + `notes`). Silent promotion is forbidden diff --git a/docs/gate/runner.md b/docs/gate/runner.md index 1508d0e..8547c12 100644 --- a/docs/gate/runner.md +++ b/docs/gate/runner.md @@ -4,7 +4,7 @@ doc_type: spec status: draft owner: B4 created: 2026-07-24 -updated: 2026-07-27 +updated: 2026-07-28 confidence: MED supersedes: null sources_verified: true @@ -30,6 +30,60 @@ repairs **actions only** on failure (≤2 repairs/run by default), and emits | `replay.ts` | `ReplayRunner` — dry-run, repair loop, metrics emission | | `metrics/` | Sibling package: emitter + §9 aggregates | +## Bounded waits + +Every wait the runner performs has an explicit ceiling. One did not: a `wait` step with no +positive duration parameter called `page.waitForLoadState("networkidle")` with **no timeout**, +inheriting Playwright's 30s default — a number nobody here chose. If the page never goes quiet +for 500ms the step burned all 30s and then failed anyway: maximum latency for zero information. + +Now bounded by `NETWORK_IDLE_WAIT_MS` (5000ms, `src/runner/actions.ts`), overridable per run via +`ReplayRunnerOptions.networkIdleWaitMs`. Measured in `tests/unit/runner-bounded-wait.test.ts` +against a page that never reaches idle: + +| | unbounded (before) | bounded (after) | +| --- | --- | --- | +| default | 30.8 s | 5.0 s | +| 1s override | 30.0 s | 1.0 s | + +**Honest scope.** The seeded Grafana dashboard does *not* trigger this — `networkidle` settles +there in ~3 ms (measured on 11.0.0, `/d/paragent-seed`, 2026-07-28), because the seed dashboard +sets no refresh interval and TestData is generated client-side. This is a **latent** worst case, +reachable on any surface with continuous polling, streaming, or websockets — not a hang observed +on the current test-bed. Bounding it is cheap insurance taken before the gate runs, not a fix +for a live symptom. + +**It changes which steps pass.** A page that first goes quiet at 12 s held the step until it did +and does not now — at 5 s the step continues and the assertion decides on whatever is on screen. +Deliberate, and cheap *today* because no gate number exists (`gate:matrix` is dry-run only, +[#62](https://github.com/DevToolie/Paragent/issues/62)). After a published measurement it would +be an expensive silent shift. + +### Reaching the bound is not a step failure + +A parameterless `wait` is a settling **hint**. The step's post-condition is the assertion that +runs immediately after it, with its own `timeout_ms` budget. So when the bound elapses the step +**proceeds** and records `settled: false` (`ActionResult.settled`, surfaced as +`StepAttemptResult.notes`) — the same posture as the 250 ms idle probe in +`src/runner/page-state.ts`, where a timeout means *no claim* rather than failure. + +Classifying it as `TIMEOUT` would be worse than slow. `replay.ts` routes every non-`PASS` +outcome into the repair loop, so a never-quiet page would fail deterministically at the bound, +consume both repair attempts, and land on `REPAIR_EXHAUSTED` — and no `corrected_action` can +make a polling page go quiet. The run's `success_with_le_2_repairs` would then be reporting a +scaffolding condition as churn, which is the one thing the gate number must not do. On exactly +the surfaces this bound exists for (polling, streaming, websockets), the bound would otherwise +make a doomed step fail 6× faster without making it any less doomed. + +If the page really is broken, nothing is hidden: the assertion fails on its own evidence, and +*that* failure is worth a repair attempt. And a step that genuinely needs idle as its +post-condition can say so — `network-idle` is an assertion type +(`src/runner/assertions.ts`), where a timeout is a real failure because it was a real claim. + +`tests/unit/runner-bounded-wait.test.ts` pins both halves: the clock (bounded, not 30 s) and the +classification (`repair_count: 0` on a never-idle page, with the note still recorded). Reverting +either fails it. + ## Invariants 1. **Assertions are immutable in repair.** `deepFreeze` + `assertAssertionUnchanged` — proposals may only supply `corrected_action`. @@ -78,3 +132,12 @@ npm run gate:report - Whether walking eight versions changes anything the report can *conclude*. It does not — more rows over the same hand-written 2-step program is a better-shaped denominator, not a measurement. That waits on live execution ([#62](https://github.com/DevToolie/Paragent/issues/62)). +- **`settled: false` is recorded but not aggregated.** It reaches `StepAttemptResult.notes` in + memory and stops there: `metrics.schema.json` has no field for it, so nothing counts how often + a wait's hint went unanswered across a matrix run. Adding one is a contract change, and there + is no measurement yet to justify the shape. Until then a reader cannot tell "this run met a + never-quiet page eight times" from "never". +- Whether 5000 ms is the right *bound* rather than merely a chosen one. It equals + `DEFAULT_ASSERTION_TIMEOUT_MS` by coincidence, not by construction — two independent constants + in two packages, nothing enforcing the match. Neither number has been fitted to an observation + because no live run exists yet. diff --git a/src/compiler/assertions.ts b/src/compiler/assertions.ts index 130c16b..04e0a01 100644 --- a/src/compiler/assertions.ts +++ b/src/compiler/assertions.ts @@ -10,6 +10,32 @@ import type { } from "./types.js"; import { SCHEMA_VERSION } from "./types.js"; +/** + * `timeout_ms` written onto every synthesized assertion. + * + * Was seven separate `5000` literals. Naming it is the change; **the value is + * deliberately unmoved.** + * + * A timeout is part of an assertion's *strength*, not a performance knob: a + * shorter one is a stricter check, a longer one a laxer one. Lowering this to + * make replay feel faster would raise the failure rate and move step-level + * replay-validity — the one number PRD §9 gates on — while looking like a perf + * tweak. That is the shape `docs/architecture.md` invariant 1 forbids. + * + * The runner spends this budget on *failure* + * (`src/runner/assertions.ts`), so it is also the dominant term in + * worst-case latency: a 12-step task with three stale locators waits 3 × this + * before repair even starts. Changing it is therefore a real decision, and it + * should follow a measurement rather than precede one. Override per-compile via + * `SynthesizeAssertionOptions.timeoutMs` if you need to explore that. + */ +export const DEFAULT_ASSERTION_TIMEOUT_MS = 5000; + +export interface SynthesizeAssertionOptions { + /** Override for {@link DEFAULT_ASSERTION_TIMEOUT_MS}. */ + timeoutMs?: number; +} + const ASSERTION_TYPES = new Set([ "element-visible", "text-matches", @@ -89,7 +115,11 @@ interface SynthesisContext { } /** Synthesize one post-condition; expected values are templates with typed holes. */ -export function synthesizeAssertion(ctx: SynthesisContext): Assertion { +export function synthesizeAssertion( + ctx: SynthesisContext, + options: SynthesizeAssertionOptions = {}, +): Assertion { + const timeout_ms = options.timeoutMs ?? DEFAULT_ASSERTION_TIMEOUT_MS; const { trajectory, step, locatorChain } = ctx; const hint = step.assertion_hint; const primary = pickPrimaryLocator(locatorChain); @@ -133,7 +163,7 @@ export function synthesizeAssertion(ctx: SynthesisContext): Assertion { param_types: { success_message: "string" }, regex_template: templateToRegex(template), }, - timeout_ms: 5000, + timeout_ms, failure_classification: "assertion_failed", notes: "Recorder signalled toast/success copy. Expected text is a typed hole ({success_message}) — never a tenant/product-message literal. Strong when the runner binds a success-pattern allowlist; otherwise runtime bind quality is MED confidence.", @@ -157,7 +187,7 @@ export function synthesizeAssertion(ctx: SynthesisContext): Assertion { template: "{item_count}", param_types: { item_count: "integer" }, }, - timeout_ms: 5000, + timeout_ms, failure_classification: "assertion_failed", notes: "Count asserted via template hole. expected.count=0 is a schema placeholder until B2 emits structured counts in post_state; runner must bind the observed count. Labelled weak: absolute counts drift. Not a gate metric.", @@ -210,7 +240,7 @@ export function synthesizeAssertion(ctx: SynthesisContext): Assertion { strength: "strong", target: { locator: stripTenantFlagForTarget(primary) }, expected: { visible: false }, - timeout_ms: 5000, + timeout_ms, failure_classification: "assertion_failed", notes: "Recorder observed the acted-on control was no longer visible after the action (ADR-0007 post_action_target_visible=false). Asserts only that: the control is gone. Proves the step was not a no-op; proves nothing about downstream state.", @@ -278,7 +308,7 @@ export function synthesizeAssertion(ctx: SynthesisContext): Assertion { strength, target: { locator: stripTenantFlagForTarget(resolved) }, expected: { visible: true }, - timeout_ms: 5000, + timeout_ms, failure_classification: "assertion_failed", notes, }; @@ -305,7 +335,7 @@ export function synthesizeAssertion(ctx: SynthesisContext): Assertion { strength: strong ? "strong" : "weak", target: { url_template }, expected, - timeout_ms: 5000, + timeout_ms, failure_classification: "assertion_failed", notes: strong ? "URL template changed (or navigate completed); matching post_state.url_template is strong evidence the step reached the intended surface." @@ -319,7 +349,7 @@ export function synthesizeAssertion(ctx: SynthesisContext): Assertion { assertion_id: assertionId, type: "network-idle", strength: "weak", - timeout_ms: 5000, + timeout_ms, failure_classification: "timeout", notes: "network_idle in post_state is weakly consistent with success — idle also occurs on error pages and no-ops. Labelled weak.", diff --git a/src/compiler/compile.ts b/src/compiler/compile.ts index b8d8a0d..69bb5d2 100644 --- a/src/compiler/compile.ts +++ b/src/compiler/compile.ts @@ -1,4 +1,7 @@ -import { synthesizeAssertion } from "./assertions.js"; +import { + synthesizeAssertion, + type SynthesizeAssertionOptions, +} from "./assertions.js"; import { buildLocatorFallbackChain } from "./locators.js"; import { decidePoolEligibility } from "./pool.js"; import { @@ -50,13 +53,17 @@ export function compileStep( trajectory: Trajectory, step: TrajectoryStep, compiledAt: string, + assertionOptions: SynthesizeAssertionOptions = {}, ): CacheRow { const { action, topologyOnly } = buildCompiledAction(step); - const assertion = synthesizeAssertion({ - trajectory, - step, - locatorChain: action.locator_fallback_chain, - }); + const assertion = synthesizeAssertion( + { + trajectory, + step, + locatorChain: action.locator_fallback_chain, + }, + assertionOptions, + ); const pool = decidePoolEligibility({ chain: action.locator_fallback_chain, assertion, @@ -101,6 +108,12 @@ export interface CompileOptions { compiledAt?: string; inputPath?: string; notes?: string; + /** + * Override the `timeout_ms` written onto every synthesized assertion. + * Defaults to `DEFAULT_ASSERTION_TIMEOUT_MS`. Read the note on that constant + * before changing it — it is an assertion-strength knob, not a perf one. + */ + assertionTimeoutMs?: number; } export function compileTrajectory( @@ -117,9 +130,13 @@ export function compileTrajectory( } const compiledAt = options.compiledAt ?? new Date().toISOString(); + const assertionOptions: SynthesizeAssertionOptions = + options.assertionTimeoutMs === undefined + ? {} + : { timeoutMs: options.assertionTimeoutMs }; const rows = [...trajectory.steps] .sort((a, b) => a.step_index - b.step_index) - .map((step) => compileStep(trajectory, step, compiledAt)); + .map((step) => compileStep(trajectory, step, compiledAt, assertionOptions)); const bundle: CompiledTrajectoryBundle = { schema_version: SCHEMA_VERSION, diff --git a/src/compiler/index.ts b/src/compiler/index.ts index a88945f..cc62bf5 100644 --- a/src/compiler/index.ts +++ b/src/compiler/index.ts @@ -7,7 +7,12 @@ export type { Trajectory, } from "./types.js"; export { compileTrajectory, compileStep } from "./compile.js"; -export { synthesizeAssertion, templateToRegex } from "./assertions.js"; +export { + DEFAULT_ASSERTION_TIMEOUT_MS, + synthesizeAssertion, + templateToRegex, +} from "./assertions.js"; +export type { SynthesizeAssertionOptions } from "./assertions.js"; export { buildLocatorFallbackChain, orderLocatorCandidates, diff --git a/src/runner/actions.ts b/src/runner/actions.ts index 95d1140..e1e0b69 100644 --- a/src/runner/actions.ts +++ b/src/runner/actions.ts @@ -15,6 +15,62 @@ export interface ActionResult { ok: boolean; outcome?: "LOCATOR_NOT_FOUND" | "TIMEOUT" | "PAGE_ERROR"; message?: string; + /** + * `wait` steps only: whether the `networkidle` fallback actually fired. + * `false` means the bound elapsed and the step proceeded anyway — a settling + * hint that went unanswered, not a failure. See {@link NETWORK_IDLE_WAIT_MS}. + */ + settled?: boolean; +} + +/** + * Ceiling for the `networkidle` fallback of a parameterless `wait` step. + * + * This used to be an unbounded `page.waitForLoadState("networkidle")`, which + * inherits Playwright's 30s default — a number nobody in this repo chose. On a + * page that never goes quiet for 500ms, `networkidle` never fires, so the step + * burned the full 30s and then failed anyway: maximum latency for zero + * information. Measured at 30.0s in tests/unit/runner-bounded-wait.test.ts. + * + * **Honest scope of the risk.** The seeded Grafana dashboard does *not* trigger + * it — `networkidle` settles there in ~3ms (measured on 11.0.0, /d/paragent-seed, + * 2026-07-28), because the seed dashboard sets no refresh interval and TestData + * is generated client-side. So this is a latent worst case rather than one the + * current test-bed hits: it becomes reachable on any surface with continuous + * polling, streaming, or websockets. Bounding it is cheap insurance, not a fix + * for an observed test-bed hang. + * + * 5000ms happens to equal the assertion timeout the compiler emits + * (`DEFAULT_ASSERTION_TIMEOUT_MS` in src/compiler/assertions.ts), which keeps a + * step's wait and its post-condition the same order of magnitude. Nothing + * enforces the equality — they are independent constants in different packages, + * and either can move without the other. + * + * **Reaching the bound is not a step failure.** A parameterless `wait` is a + * settling *hint*; the post-condition is the assertion that runs immediately + * after it, with its own budget. So when the bound elapses the step proceeds + * and records `settled: false` — the same posture as the 250ms idle probe in + * page-state.ts, where a timeout means *no claim* rather than failure. + * + * Classifying it as `TIMEOUT` instead would route the step into the repair loop + * (replay.ts sends every non-PASS outcome there), spending repair budget twice + * on a condition no `corrected_action` can fix — "this page never goes quiet" + * is not a locator problem. `success_with_le_2_repairs` would then be reporting + * a scaffolding condition as churn, which is the one thing the gate number must + * not do. If the page really is broken, the assertion says so on its own and + * that failure *is* worth a repair attempt. + * + * **This still changes which steps pass.** A page that first goes quiet at, say, + * 12s used to hold the step until it did; now the step continues at 5s and the + * assertion decides on whatever is on screen. That is deliberate and is a good + * trade *now*, while no gate number exists — see docs/gate/runner.md. It would + * be a bad trade after a measurement had been published against the old value. + */ +export const NETWORK_IDLE_WAIT_MS = 5_000; + +export interface ExecuteActionOptions { + /** Override for {@link NETWORK_IDLE_WAIT_MS}. */ + networkIdleWaitMs?: number; } function isTimeoutError(err: unknown): boolean { @@ -39,6 +95,7 @@ export async function executeAction( page: Page, action: CompiledAction, params: ParamBindings = {}, + options: ExecuteActionOptions = {}, ): Promise { try { switch (action.type) { @@ -156,10 +213,23 @@ export async function executeAction( : 0; if (Number.isFinite(ms) && ms > 0) { await page.waitForTimeout(ms); - } else { - await page.waitForLoadState("networkidle"); + return { ok: true }; + } + const bound = options.networkIdleWaitMs ?? NETWORK_IDLE_WAIT_MS; + try { + await page.waitForLoadState("networkidle", { timeout: bound }); + return { ok: true, settled: true }; + } catch (err) { + // Only an unanswered settling hint is tolerated here. Anything else + // (a closed page, a navigation error) is a real failure and falls + // through to the outer handler. + if (!isTimeoutError(err)) throw err; + return { + ok: true, + settled: false, + message: `networkidle not reached within ${bound}ms; proceeded — the assertion is the post-condition`, + }; } - return { ok: true }; } case "upload": { diff --git a/src/runner/replay.ts b/src/runner/replay.ts index ad038e3..1d688f0 100644 --- a/src/runner/replay.ts +++ b/src/runner/replay.ts @@ -16,7 +16,7 @@ import { } from "../metrics/types.js"; import { addCost, measureWallClock, zeroCost } from "../metrics/cost.js"; import { MetricsEmitter } from "../metrics/emitter.js"; -import { executeAction } from "./actions.js"; +import { executeAction, NETWORK_IDLE_WAIT_MS } from "./actions.js"; import { evaluateAssertion } from "./assertions.js"; import { capturePageState, emptyPageState } from "./page-state.js"; import { @@ -51,6 +51,12 @@ export interface ReplayRunnerOptions { /** Fresh-reasoning baseline cost (measured separately). Defaults to zeros. */ costFresh?: Cost; page?: Page; + /** + * Ceiling for a parameterless `wait` step's `networkidle` fallback. + * Defaults to `NETWORK_IDLE_WAIT_MS`; see the note on that constant for why + * it is bounded at all. + */ + networkIdleWaitMs?: number; } function nowIso(): string { @@ -69,6 +75,7 @@ export class ReplayRunner { private readonly metrics: MetricsEmitter; private readonly costFresh: Cost; private readonly page?: Page; + private readonly networkIdleWaitMs: number; constructor(options: ReplayRunnerOptions = {}) { this.dryRun = options.dryRun ?? false; @@ -77,6 +84,7 @@ export class ReplayRunner { this.repairClient = options.repairClient ?? new StubRepairModelClient(); this.metrics = options.metrics ?? new MetricsEmitter(); this.costFresh = options.costFresh ?? zeroCost(); + this.networkIdleWaitMs = options.networkIdleWaitMs ?? NETWORK_IDLE_WAIT_MS; if (options.page !== undefined) this.page = options.page; } @@ -362,13 +370,19 @@ export class ReplayRunner { } const { result, wall_clock_ms } = await measureWallClock(async () => { - const actionResult = await executeAction(this.page!, action, params); + const actionResult = await executeAction(this.page!, action, params, { + networkIdleWaitMs: this.networkIdleWaitMs, + }); if (!actionResult.ok) { return { outcome: (actionResult.outcome ?? "PAGE_ERROR") as StepOutcome, message: actionResult.message, }; } + // A `wait` whose networkidle hint never fired is not a failure — it + // proceeds and says so, so the note has to survive a passing assertion. + const notes = + actionResult.settled === false ? actionResult.message : undefined; const assertionResult = await evaluateAssertion( this.page!, step.assertion, @@ -377,6 +391,7 @@ export class ReplayRunner { return { outcome: assertionResult.outcome, message: assertionResult.message, + notes, }; }); @@ -390,6 +405,7 @@ export class ReplayRunner { }; if (mode === "repair") out.repair_attempt = repairAttempt; if (result.message !== undefined) out.error_message = result.message; + if (result.notes !== undefined) out.notes = result.notes; return out; } diff --git a/src/runner/types.ts b/src/runner/types.ts index 08e6071..a10f89c 100644 --- a/src/runner/types.ts +++ b/src/runner/types.ts @@ -171,6 +171,15 @@ export interface StepAttemptResult { time_to_repair_ms?: number; assertion_strength?: AssertionStrength; error_message?: string; + /** + * Something worth recording about a step that did **not** fail — today only + * "the `networkidle` hint never fired, we proceeded anyway". Deliberately not + * `error_message`: this is not an error, and a step carrying it can be `PASS`. + * + * In-memory only. Adding it to `metrics.schema.json` is a contract change, + * and nothing aggregates it yet — see docs/gate/runner.md. + */ + notes?: string; } export interface RunResult { diff --git a/tests/unit/compiler.test.ts b/tests/unit/compiler.test.ts index dc314f8..bfcb514 100644 --- a/tests/unit/compiler.test.ts +++ b/tests/unit/compiler.test.ts @@ -3,6 +3,7 @@ import { readFile } from "node:fs/promises"; import path from "node:path"; import { compileTrajectory, + DEFAULT_ASSERTION_TIMEOUT_MS, looksLikeTenantLiteral, orderLocatorCandidates, PACKAGE, @@ -160,6 +161,68 @@ describe("click assertion target", () => { }); }); +describe("assertion timeout policy", () => { + // `timeout_ms` is an assertion-STRENGTH knob, not a perf one: a shorter + // timeout is a stricter check. The runner spends it on failure, so it is also + // the dominant term in worst-case replay latency. Nothing pinned the emitted + // value before, which meant it could be "tuned" for speed and silently move + // step-level replay-validity — the one number PRD §9 gates on. + const trajectory = (): Trajectory => ({ + schema_version: "1.0.0", + trajectory_id: "traj-timeout", + site_key: "fixture@local", + task_key: "timeout-task", + recorded_at: "2026-07-25T00:00:00.000Z", + base_url_template: "http://{host}:{port}/app", + provenance: { + recorder: "test", + agent_model: "human", + testbed_version: "fixture-v1", + }, + parameters: { host: "string", port: "integer" }, + steps: [ + { + step_index: 0, + intent: "Open the app", + action: { type: "navigate" as const, url_template: "http://{host}:{port}/app" }, + locator_candidates: [], + pre_state: { + url_template: "about:blank", + title_template: "", + dom_digest: "d0", + visible_landmarks: [], + network_idle: false, + }, + post_state: { + url_template: "http://{host}:{port}/app", + title_template: "App", + dom_digest: "d1", + visible_landmarks: ["main"], + network_idle: true, + }, + timing_ms: { started_offset_ms: 0, duration_ms: 5 }, + }, + ], + }); + + it("emits the documented default on every assertion", () => { + expect(DEFAULT_ASSERTION_TIMEOUT_MS).toBe(5000); + const bundle = compileTrajectory(trajectory()); + for (const row of bundle.rows) { + expect(row.assertion.timeout_ms).toBe(DEFAULT_ASSERTION_TIMEOUT_MS); + } + }); + + it("can be overridden per compile without touching the default", () => { + const bundle = compileTrajectory(trajectory(), { assertionTimeoutMs: 1234 }); + for (const row of bundle.rows) { + expect(row.assertion.timeout_ms).toBe(1234); + } + // The override must not leak into the module-level policy. + expect(DEFAULT_ASSERTION_TIMEOUT_MS).toBe(5000); + }); +}); + describe("compileTrajectory example", () => { it("emits one asserted cache-row per step with no tenant literals", async () => { const trajPath = path.join( diff --git a/tests/unit/runner-bounded-wait.test.ts b/tests/unit/runner-bounded-wait.test.ts new file mode 100644 index 0000000..128c28b --- /dev/null +++ b/tests/unit/runner-bounded-wait.test.ts @@ -0,0 +1,179 @@ +/** + * A parameterless `wait` step used to call `page.waitForLoadState("networkidle")` + * with no timeout, inheriting Playwright's 30s default — a number nobody in this + * repo chose. On a page that never goes quiet the step burned the full 30s and + * then failed: maximum latency for zero information. + * + * The seeded Grafana dashboard is **not** such a page — `networkidle` settles on + * `/d/paragent-seed` in ~3ms (measured on 11.0.0, 2026-07-28), because the seed + * sets no refresh interval and TestData is generated client-side. The worst case + * is latent, reachable on any surface with continuous polling, streaming or + * websockets, so these tests build one on purpose rather than borrowing the + * test-bed. + * + * Two things are under test, and they need different instruments: + * - the bound is real — only the clock separates "bounded" from "unbounded"; + * - reaching the bound does not fail the step, and therefore does not spend + * repair budget on a condition no `corrected_action` can fix. + */ + +import { createServer, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { chromium, type Browser, type Page } from "playwright"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + executeAction, + NETWORK_IDLE_WAIT_MS, +} from "../../src/runner/actions.js"; +import { ReplayRunner } from "../../src/runner/replay.js"; +import type { + CompiledAction, + CompiledProgram, +} from "../../src/runner/types.js"; + +/** A page whose in-flight request never resolves, so networkidle never fires. */ +async function startNeverIdleServer(): Promise<{ server: Server; baseUrl: string }> { + const server = createServer((req, res) => { + if ((req.url ?? "/").startsWith("/hang")) { + // Deliberately never responds and never ends the socket. + return; + } + res.writeHead(200, { "content-type": "text/html" }); + res.end( + `

busy

+ `, + ); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as AddressInfo; + return { server, baseUrl: `http://127.0.0.1:${port}` }; +} + +const waitAction: CompiledAction = { + type: "wait", + locator_fallback_chain: [], +}; + +describe("bounded networkidle wait", () => { + let browser: Browser; + let server: Server; + let baseUrl: string; + let page: Page; + + beforeAll(async () => { + browser = await chromium.launch({ headless: true }); + ({ server, baseUrl } = await startNeverIdleServer()); + page = await browser.newPage(); + await page.goto(baseUrl, { waitUntil: "domcontentloaded" }); + }, 60_000); + + afterAll(async () => { + await browser?.close(); + await new Promise((resolve) => server?.close(() => resolve())); + }); + + it("gives up at the configured bound instead of Playwright's 30s default", async () => { + const started = Date.now(); + const result = await executeAction(page, waitAction, {}, { + networkIdleWaitMs: 1_000, + }); + const elapsed = Date.now() - started; + + // Proceeds rather than failing: the hint went unanswered, which is not the + // same as the step being wrong. The assertion after it is the post-condition. + expect(result.ok).toBe(true); + expect(result.settled).toBe(false); + expect(result.message).toMatch(/networkidle not reached/); + // The load-bearing assertion. Unbounded, this takes ~30s; the generous + // ceiling here still fails loudly if the bound is ever removed. + expect(elapsed).toBeLessThan(10_000); + }, 60_000); + + it("honours the default bound when no override is supplied", async () => { + const started = Date.now(); + const result = await executeAction(page, waitAction); + const elapsed = Date.now() - started; + + expect(result.settled).toBe(false); + expect(elapsed).toBeLessThan(NETWORK_IDLE_WAIT_MS + 5_000); + }, 60_000); + + it("pins the default bound", () => { + // Changing this changes which steps pass — see the note on the constant. + expect(NETWORK_IDLE_WAIT_MS).toBe(5_000); + }); + + it("reports settled=true when the page does go quiet", async () => { + // Positive control: without this, `settled: false` could be a constant. + const quiet = await browser.newPage(); + try { + await quiet.setContent("

quiet

"); + const result = await executeAction(quiet, waitAction, {}, { + networkIdleWaitMs: 5_000, + }); + expect(result.ok).toBe(true); + expect(result.settled).toBe(true); + expect(result.message).toBeUndefined(); + } finally { + await quiet.close(); + } + }, 60_000); + + it("does not spend repair budget on a page that never goes quiet", async () => { + // The reason the classification matters. Routed through the repair loop, + // this step would burn both attempts and land on REPAIR_EXHAUSTED — and no + // corrected_action can make a polling page go quiet, so the run's + // success_with_le_2_repairs would be reporting scaffolding, not churn. + const program: CompiledProgram = { + schema_version: "1.0.0", + program_id: "bounded-wait-probe", + site_key: "local-never-idle", + task_key: "wait-then-assert", + testbed_version: "n/a", + steps: [ + { + step_index: 0, + compiled_action: waitAction, + assertion: { + schema_version: "1.0.0", + assertion_id: "a0", + type: "element-visible", + strength: "strong", + target: { locator: { strategy: "text", text: "busy" } }, + expected: { visible: true }, + timeout_ms: 5_000, + failure_classification: "assertion_failed", + }, + }, + ], + }; + + const runner = new ReplayRunner({ page, networkIdleWaitMs: 1_000 }); + const result = await runner.run(program); + + expect(result.repair_count).toBe(0); + expect(result.task_success).toBe(true); + expect(result.steps_replay_valid).toBe(1); + expect(result.step_results[0]?.outcome).toBe("PASS"); + // Passing is not the same as pretending it settled. + expect(result.step_results[0]?.notes).toMatch(/networkidle not reached/); + }, 60_000); + + it("still uses a plain sleep when the step carries a positive duration", async () => { + const started = Date.now(); + const result = await executeAction( + page, + { ...waitAction, param_refs: ["ms"] }, + { ms: 150 }, + ); + const elapsed = Date.now() - started; + + // Never touches networkidle, so the never-idle page is irrelevant here. + expect(result.ok).toBe(true); + expect(elapsed).toBeLessThan(3_000); + }, 30_000); +});