diff --git a/CHANGELOG.md b/CHANGELOG.md index 5976514..13200ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,18 @@ match exactly, or the workflow fails the publish. *Behavior change:* a drive that needs the terminal must now opt in with `--allow-shell`, and one that reaches a second origin must allowlist it. +- **"Verified" now means verified.** A drive is finished only when the model + explicitly calls `finish` — a model that stops acting is an honest give-up, + recorded to failure-learning, never a success. A session that asserted + nothing is reported unverified without compiling (and the emitter refuses + assertion-free sessions). Emitted role/text/label locators match **exactly** + on both record and replay, so a later DOM addition can't silently re-target + a committed test. A numeric graph `schema_version` can no longer bypass the + compatibility guard, an empty Playwright report raises an actionable error + ("no tests matched — check testDir") instead of a misleading "unstable" + quarantine, and a target project's configured retries count each test's + final attempt as its outcome. + ## 2026.07.1 — the "any model" release The release that makes **bring-your-own-model** mean *any* model — and proves Proofkeeper on itself. Everything since the first cut: diff --git a/lore-proofkeeper/designs/design-verified-semantics.md b/lore-proofkeeper/designs/design-verified-semantics.md new file mode 100644 index 0000000..29933f2 --- /dev/null +++ b/lore-proofkeeper/designs/design-verified-semantics.md @@ -0,0 +1,99 @@ +--- +schema_version: 1 +id: PK-KWFVXA7HJBYQ +type: design +--- +# Verified Semantics — Finish, Assert, Match Exactly, Refuse Ambiguity + +## Context + +A review of the loop found the "verified" claim could be produced without +verification: completion was inferred from a field both adapters always set, +assertion-free sessions gated green, locators matched by substring, and two +contract edges (numeric `schema_version`, empty or retried Playwright reports) +were misread. This design closes each gap at its narrowest choke point. + +## User Need + +A reviewer merging a `## Verified By` link — and a maintainer reading +`proofkeeper qa` output — must be able to trust that "verified" means: the +model explicitly finished, at least one observable outcome was asserted, the +committed locators mean the same thing on replay, and contract anomalies were +refused rather than guessed at. + +## Design + +- **Finish is explicit.** `DriveResult` gains `stopReason: + "finished" | "gave_up" | "step_budget"` (plus `gaveUpText`); `finished` is + true only for an explicit `finish` tool call. The vacuous + `done !== undefined` check is gone. `runQa` records give-ups and budget + exhaustion to the learning store with distinct reasons. +- **Assertions are required.** `sessionAssertsOutcome` (compiler IR) names the + assertion action types; `runQa` skips compile/gate for assertion-free + sessions (`QaResult.loop` becomes optional, `unverifiedReason` says why), and + `emitSpec` refuses them as it already refused empty sessions. +- **Exact locators, both sides.** The emitter renders `{ exact: true }` for + role-name, text, and label locators, and the Recorder resolves with the same + exactness — record/replay agreement is the invariant, so the change is made + in both places in the same commit. The locator guidance tells the model to + copy names verbatim. +- **Contract edges refuse, don't guess.** `parseGraph` stringifies a numeric + `schema_version` before comparing, so `2` is refused and `1` accepted. + `reduceReport` groups results per test and takes each test's final attempt + (retries append attempts); zero results raises `ReportParseError` naming the + testDir/testMatch cause instead of returning "failed". + +## Constraints + +- `QaResult.loop` optional is the only public-shape change; CLI rendering + guards it and prints the unverified reason. +- Emitted specs remain byte-deterministic; the exact-matching change alters + emitted bytes once, uniformly. +- No new dependency. + +## Rationale + +Each fix lands at the single point every caller flows through: the loop's stop +handling, the emitter's refusal, the one locator-resolution seam per side, the +one graph parser, the one report reducer. The QA loop refuses to compile +assertion-free sessions *and* the emitter refuses them — the loop gives the +honest verdict, the emitter makes the invariant unconditional for library +callers. + +## Alternatives + +- **Nudge the model on a no-tool-call turn instead of stopping.** Deferred: a + retry prompt spends tokens to mask a model that has already disengaged; + honest give-up plus failure-learning steers the next attempt instead. +- **Treat an empty report as a distinct RunStatus.** Rejected: every consumer + would need to handle a fourth status; an exception with a diagnostic is the + existing infra-failure channel. +- **Substring locators with strict-mode suppression.** Rejected: it trades a + loud record-time failure for a silent wrong-element match on replay. + +## Accessibility + +Not applicable — loop semantics and parser behavior; user surface is CLI text, +which now names why a capability is unverified. + +## Style Guidance + +Unverified reasons are complete sentences a maintainer can act on ("drive gave +up after 4 step(s): …", "no tests matched — check testDir"). + +## Open Questions + +- Whether a single retry nudge on give-up earns its token cost. Measure + give-up rates from the learning store first. + +## Related Requirements + +- req-verified-semantics + +## Related Roadmaps + +- autonomous-qa-enhancements + +## Status + +Accepted diff --git a/lore-proofkeeper/requirements/req-verified-semantics.md b/lore-proofkeeper/requirements/req-verified-semantics.md new file mode 100644 index 0000000..61af0d4 --- /dev/null +++ b/lore-proofkeeper/requirements/req-verified-semantics.md @@ -0,0 +1,63 @@ +--- +schema_version: 1 +id: PK-KWFVX9H4FDSE +type: requirement +--- +# "Verified" Means Verified + +## Problem + +Proofkeeper's product promise is the word "verified", and four gaps let it be +claimed without being earned. A drive that stopped issuing tool calls was +scored as finished (the completion check was vacuously true for both bundled +adapters), so give-ups looked like successes and never reached the failure +learning store. A session that asserted nothing compiled to a trivially-green +spec and passed the fidelity gate. Emitted locators matched by substring, so a +later DOM addition could silently re-target a committed test. And two contract +edges misreported: a numeric graph `schema_version` bypassed the compatibility +guard as "omitted", and a Playwright report with zero results — or with +configured retries — was mislabelled instead of surfaced. + +## Requirements + +- [REQ-001] A drive is finished only when the model explicitly calls `finish`; a turn with no tool calls is a give-up, distinguished from the step budget, and recorded to the failure-learning store with the model's final text. +- [REQ-002] A session with no recorded assertions is never compiled or gated: the QA loop reports it unverified with the reason, and the emitter refuses assertion-free sessions outright. +- [REQ-003] Emitted role-name, text, and label locators match exactly, and the recorder resolves with the same exactness — an assertion that held at record time means the same thing on replay. +- [REQ-004] A numeric graph `schema_version` counts as present: a supported value is accepted, an unsupported one is refused — it can never pass the guard as "omitted". +- [REQ-005] A Playwright report with no test results is refused with a diagnostic naming the likely cause (spec outside the config's testDir), never silently mapped to "failed"; when the target project configures retries, each test's final attempt is its outcome. + +## Success Metrics + +- A scripted give-up drive produces `verified: false` with a give-up reason and + a learning record, without compiling. +- An assertion-free session is refused by the emitter and reported unverified + by the QA loop. +- A flaky-then-green retried test reduces to "passed"; an empty report raises + an actionable error. + +## Risks + +- Exact matching is stricter: a model that asserts a text fragment now fails at + record time. Mitigation: the locator guidance tells the model matching is + exact and to copy names verbatim; the recorder rejects at record time, so + nothing weaker is ever committed. +- Give-up semantics depend on models using the `finish` tool. Mitigation: the + system prompt instructs it, and a non-finishing drive degrades to an honest + "unverified", never a false "verified". + +## Assumptions + +- The `finish` tool remains the completion signal for every adapter. +- Playwright's JSON report continues to append one result per retry attempt, + final attempt last. + +## Related Roadmaps + +- autonomous-qa-enhancements + +## Verified By + +- `tests/drive-loop.test.ts` +- `tests/compiler-emit.test.ts` +- `tests/playwright-report.test.ts` +- `tests/coverage.test.ts` diff --git a/lore-proofkeeper/roadmaps/autonomous-qa-enhancements.md b/lore-proofkeeper/roadmaps/autonomous-qa-enhancements.md index 99af8b2..76db5c8 100644 --- a/lore-proofkeeper/roadmaps/autonomous-qa-enhancements.md +++ b/lore-proofkeeper/roadmaps/autonomous-qa-enhancements.md @@ -79,6 +79,13 @@ explicit operator opt-in, navigate/request egress is allowlisted to the product under test, and observation side channels are redacted before reaching the model provider. Serves the trust outcome that underwrites every committed test. +### Verified semantics + +Make "verified" unearnable without verification: completion only via an explicit +finish, assertion-free sessions refused, exact locator matching on record and +replay, and contract anomalies (schema versions, empty or retried reports) +refused rather than guessed at. Serves the faithful-tests outcome at its core. + ## Success Measures - A pull request shows exactly one Proofkeeper QA comment regardless of how many diff --git a/src/agent/drive.ts b/src/agent/drive.ts index 0dfe07c..e102775 100644 --- a/src/agent/drive.ts +++ b/src/agent/drive.ts @@ -78,8 +78,16 @@ export interface DriveOptions { export interface DriveResult { /** The recorded session, ready to compile. */ session: RecordedSession; - /** True if the model signalled completion (vs hitting the step budget). */ + /** True only when the model explicitly called `finish`. */ finished: boolean; + /** + * Why the drive ended: `finished` — the model called `finish`; `gave_up` — + * the model stopped issuing tool calls without finishing; `step_budget` — + * the turn cap was hit. Only `finished` may lead to "verified". + */ + stopReason: "finished" | "gave_up" | "step_budget"; + /** The model's final text when it gave up, for the failure-learning record. */ + gaveUpText?: string; /** Number of model turns taken. */ steps: number; /** The Markdown test plan, when a planning turn ran. */ @@ -297,6 +305,8 @@ export class AutonomousDriver { const maxSteps = this.options.maxSteps ?? DEFAULT_MAX_STEPS; let finished = false; + let stopReason: DriveResult["stopReason"] = "step_budget"; + let gaveUpText: string | undefined; let steps = 0; while (steps < maxSteps) { @@ -305,8 +315,11 @@ export class AutonomousDriver { const calls = response.toolCalls ?? []; if (calls.length === 0) { - // The model stopped acting; treat a `done` message as completion. - finished = response.done !== undefined; + // The model stopped issuing tool calls without calling `finish`: that is + // a give-up, not completion. (Adapters return text in `done` whenever + // there are no tool calls, so its mere presence proves nothing.) + stopReason = "gave_up"; + gaveUpText = response.done?.trim() || undefined; break; } @@ -318,6 +331,7 @@ export class AutonomousDriver { const result = await dispatch(recorder, call, policy); if (result.finished) { finished = true; + stopReason = "finished"; stop = true; break; } @@ -335,7 +349,14 @@ export class AutonomousDriver { monitor.dispose(); const session = recorder.recording(); if (plan !== undefined) session.plan = plan; - return { session, finished, steps, ...(plan !== undefined ? { plan } : {}) }; + return { + session, + finished, + stopReason, + ...(gaveUpText !== undefined ? { gaveUpText } : {}), + steps, + ...(plan !== undefined ? { plan } : {}), + }; } } diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 9dd5e9e..6bf3df4 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -164,6 +164,8 @@ export const LOCATOR_GUIDANCE = "A locator is an object { strategy: 'role'|'testId'|'text'|'label'|'css', ... }: " + "role needs { role, name? }, testId needs { testId }, text needs { text }, " + "label needs { label }, css needs { selector }. Prefer role, testId, or text over css. " + + "Name, text, and label matching is EXACT — copy the accessible name or visible text " + + "verbatim from the observation, not a fragment of it. " + "A locator may be passed as the `locator` field or inline on the arguments."; /** Guidance for the terminal tools, folded into the system prompt. */ diff --git a/src/cli.ts b/src/cli.ts index af57236..442231f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -517,13 +517,25 @@ async function runQaCommand(argv: string[]): Promise { } function renderQaResult(result: Awaited>): string { - const v = result.loop.verdict; + const stop = + result.drive.stopReason === "finished" + ? "finished" + : result.drive.stopReason === "gave_up" + ? "gave up" + : "stopped at step budget"; const lines = [ `Capability: ${result.capability.id} — ${result.capability.title}`, - `Drive: ${result.drive.steps} step(s), ${result.drive.finished ? "finished" : "stopped at step budget"}`, - `Compiled: ${result.loop.candidate.specPath}`, - `Fidelity: ${v.passed}/${v.attempts} re-runs green — ${v.stable ? "stable" : "unstable, quarantined"}`, + `Drive: ${result.drive.steps} step(s), ${stop}`, ]; + if (result.loop) { + const v = result.loop.verdict; + lines.push( + `Compiled: ${result.loop.candidate.specPath}`, + `Fidelity: ${v.passed}/${v.attempts} re-runs green — ${v.stable ? "stable" : "unstable, quarantined"}`, + ); + } else { + lines.push(`Not compiled: ${result.unverifiedReason ?? "nothing to verify"}`); + } if (result.writeBack) { lines.push( result.writeBack.status === "proposed" diff --git a/src/compiler/actions.ts b/src/compiler/actions.ts index 9086460..080b024 100644 --- a/src/compiler/actions.ts +++ b/src/compiler/actions.ts @@ -41,6 +41,25 @@ export type Action = | { type: "expectStatus"; status: number } | { type: "expectJson"; path: string; equals: string | number | boolean }; +/** The action types that assert an observable outcome. */ +const ASSERTION_TYPES: readonly Action["type"][] = [ + "expectText", + "expectVisible", + "expectOutput", + "expectExit", + "expectStatus", + "expectJson", +]; + +/** + * Whether a session asserted at least one observable outcome. A session of + * bare navigation/clicks compiles to a trivially-green spec that verifies + * nothing — callers use this to refuse to call such a session "verified". + */ +export function sessionAssertsOutcome(session: RecordedSession): boolean { + return session.actions.some((a) => ASSERTION_TYPES.includes(a.type)); +} + /** A captured drive session: its entry point and the actions recorded from it. */ export interface RecordedSession { /** The capability this session exercises (threads to the write-back). */ diff --git a/src/compiler/emit.ts b/src/compiler/emit.ts index eac9d0a..a6c9e9a 100644 --- a/src/compiler/emit.ts +++ b/src/compiler/emit.ts @@ -11,7 +11,7 @@ * (dev, prod) while still being runnable standalone. */ -import type { Action, Locator, RecordedSession } from "./actions.js"; +import { sessionAssertsOutcome, type Action, type Locator, type RecordedSession } from "./actions.js"; /** Single-quoted string literal with deterministic escaping. */ function lit(value: string): string { @@ -24,18 +24,23 @@ function lit(value: string): string { return `'${escaped}'`; } +// Name/text matching is exact: Playwright's default is substring and +// case-insensitive, so a later DOM addition containing the same substring would +// silently break (strict-mode violation) or mis-target a recorded locator on +// replay. The Recorder resolves with the same exactness, so an assertion that +// held at record time means the same thing at run time. function locatorExpr(loc: Locator): string { switch (loc.kind) { case "role": return loc.name !== undefined - ? `page.getByRole(${lit(loc.role)}, { name: ${lit(loc.name)} })` + ? `page.getByRole(${lit(loc.role)}, { name: ${lit(loc.name)}, exact: true })` : `page.getByRole(${lit(loc.role)})`; case "testId": return `page.getByTestId(${lit(loc.testId)})`; case "text": - return `page.getByText(${lit(loc.text)})`; + return `page.getByText(${lit(loc.text)}, { exact: true })`; case "label": - return `page.getByLabel(${lit(loc.label)})`; + return `page.getByLabel(${lit(loc.label)}, { exact: true })`; case "css": return `page.locator(${lit(loc.selector)})`; } @@ -136,13 +141,20 @@ function usesHttp(session: RecordedSession): boolean { /** * Emit Playwright `.spec.ts` source for a recorded session. * - * @throws {Error} when the session recorded no actions — an empty test - * verifies nothing and must not be emitted. + * @throws {Error} when the session recorded no actions, or recorded no + * assertions — a test that asserts nothing verifies nothing and must not + * be emitted (it would pass the fidelity gate trivially). */ export function emitSpec(session: RecordedSession): string { if (session.actions.length === 0) { throw new Error("refusing to emit a test from a session with no recorded actions"); } + if (!sessionAssertsOutcome(session)) { + throw new Error( + "refusing to emit a test from a session with no recorded assertions — " + + "a spec that asserts nothing verifies nothing", + ); + } const provenance = session.capabilityId ? ` for capability ${session.capabilityId}` diff --git a/src/compiler/recorder.ts b/src/compiler/recorder.ts index 3b013d6..09aff7e 100644 --- a/src/compiler/recorder.ts +++ b/src/compiler/recorder.ts @@ -37,16 +37,20 @@ export class Recorder { private readonly options: RecorderOptions, ) {} + // Exact name/text matching, mirroring the emitter's locatorExpr — an action + // that resolved at record time must mean the same thing on replay. private resolve(loc: Locator) { switch (loc.kind) { case "role": - return this.page.getByRole(loc.role as Parameters[0], { name: loc.name }); + return this.page.getByRole(loc.role as Parameters[0], { + ...(loc.name !== undefined ? { name: loc.name, exact: true } : {}), + }); case "testId": return this.page.getByTestId(loc.testId); case "text": - return this.page.getByText(loc.text); + return this.page.getByText(loc.text, { exact: true }); case "label": - return this.page.getByLabel(loc.label); + return this.page.getByLabel(loc.label, { exact: true }); case "css": return this.page.locator(loc.selector); } diff --git a/src/coverage/graph.ts b/src/coverage/graph.ts index 5e50770..8a58338 100644 --- a/src/coverage/graph.ts +++ b/src/coverage/graph.ts @@ -121,8 +121,12 @@ export function parseGraph(json: string): Graph { throw new GraphParseError("graph export is missing an `edges` array"); } // Tolerate a missing schema_version (older/loose inputs), but refuse a - // present-but-unsupported one rather than emit possibly-wrong coverage. - const schemaVersion = typeof raw["schema_version"] === "string" ? raw["schema_version"] : ""; + // present-but-unsupported one rather than emit possibly-wrong coverage. A + // numeric version counts as present — stringify it so `2` cannot slip past + // the guard as "omitted". + const rawVersion = raw["schema_version"]; + const schemaVersion = + typeof rawVersion === "string" ? rawVersion : typeof rawVersion === "number" ? String(rawVersion) : ""; if (schemaVersion !== "" && schemaVersion !== SUPPORTED_GRAPH_SCHEMA) { throw new GraphParseError( `unsupported rac graph schema_version '${schemaVersion}' ` + diff --git a/src/qa/run-qa.ts b/src/qa/run-qa.ts index 3f05eb6..bf43e82 100644 --- a/src/qa/run-qa.ts +++ b/src/qa/run-qa.ts @@ -17,6 +17,7 @@ import type { Graph } from "../coverage/graph.js"; import { computeCoverage, type CapabilityCoverage } from "../coverage/model.js"; import type { Compiler } from "../compiler/types.js"; +import { sessionAssertsOutcome } from "../compiler/actions.js"; import { summarizeSession } from "../compiler/summary.js"; import type { Runner, RunTarget } from "../runner/types.js"; import { runAgentLoop, type AgentLoopResult } from "../agent/loop.js"; @@ -102,9 +103,17 @@ export interface QaOptions { export interface QaResult { capability: CapabilityCoverage; drive: DriveResult; - loop: AgentLoopResult; - /** True iff the compiled test passed the fidelity gate. */ + /** + * Compile → fidelity → run outcome. Absent when the drive produced nothing + * worth compiling (it gave up, hit the step budget, or asserted nothing) — + * a session that asserts nothing would pass the gate trivially, so it is + * never compiled. + */ + loop?: AgentLoopResult; + /** True iff the drive finished AND the compiled test passed the fidelity gate. */ verified: boolean; + /** Why the capability is not verified, when it is not. */ + unverifiedReason?: string; /** Present only when a write-back was attempted (stable + proposer + propose). */ writeBack?: WriteBackResult; } @@ -136,6 +145,31 @@ export async function runQa(deps: QaDeps, options: QaOptions): Promise }; const drive = await deps.drive(driveOptions); + // "Verified" must mean verified: the drive must have explicitly finished AND + // asserted at least one observable outcome. A gave-up or assertion-free + // session is unverified without compiling — it would pass the gate trivially. + const unverifiedReason = !drive.finished + ? drive.stopReason === "gave_up" + ? `drive gave up after ${drive.steps} step(s) without finishing` + + (drive.gaveUpText !== undefined ? `: ${drive.gaveUpText}` : "") + : `drive did not finish within the step budget (${drive.steps} steps)` + : !sessionAssertsOutcome(drive.session) + ? "drive finished but recorded no assertions — nothing observable was verified" + : undefined; + + if (unverifiedReason !== undefined) { + const result: QaResult = { capability, drive, verified: false, unverifiedReason }; + if (deps.learning) { + await deps.learning.recordFailure({ + capabilityId: capability.id, + goal, + reason: unverifiedReason, + steps: drive.steps, + }); + } + return result; + } + const loop = await runAgentLoop( { compiler: deps.compiler, runner: deps.runner }, { @@ -147,6 +181,9 @@ export async function runQa(deps: QaDeps, options: QaOptions): Promise const verified = loop.verdict.stable; const result: QaResult = { capability, drive, loop, verified }; + if (!verified) { + result.unverifiedReason = `compiled test was unstable: ${loop.verdict.passed}/${loop.verdict.attempts} re-runs green`; + } // Propose the write-back only for a stable test (ADR-065: a human reviews it). // The proposal carries the readable step summary so a reviewer can read the @@ -168,13 +205,11 @@ export async function runQa(deps: QaDeps, options: QaOptions): Promise } // Remember a failure so the next attempt avoids it (failure-learning). - if (deps.learning && (!verified || !drive.finished)) { + if (deps.learning && !verified) { await deps.learning.recordFailure({ capabilityId: capability.id, goal, - reason: !drive.finished - ? `drive did not finish within the step budget (${drive.steps} steps)` - : `compiled test was unstable: ${loop.verdict.passed}/${loop.verdict.attempts} re-runs green`, + reason: result.unverifiedReason ?? "unverified", steps: drive.steps, }); } diff --git a/src/runner/playwright-report.ts b/src/runner/playwright-report.ts index bd3821f..8037538 100644 --- a/src/runner/playwright-report.ts +++ b/src/runner/playwright-report.ts @@ -58,16 +58,14 @@ function mapStatus(raw: string | undefined): RunStatus { } } -/** Depth-first walk of the nested suite tree, yielding every result. */ -function collectResults(suites: PwSuite[] | undefined): PwResult[] { - const out: PwResult[] = []; +/** Depth-first walk of the nested suite tree, yielding every test. */ +function collectTests(suites: PwSuite[] | undefined): PwTest[] { + const out: PwTest[] = []; for (const suite of suites ?? []) { for (const spec of suite.specs ?? []) { - for (const test of spec.tests ?? []) { - out.push(...(test.results ?? [])); - } + out.push(...(spec.tests ?? [])); } - out.push(...collectResults(suite.suites)); + out.push(...collectTests(suite.suites)); } return out; } @@ -85,19 +83,32 @@ function firstTracePath(results: PwResult[]): string | undefined { * Reduce a parsed report to a single {@link RunResult} for one (test, target). * * A spec file may contain several `test(...)` blocks; we aggregate them: the - * run passed iff every result passed, the duration is the sum, and the trace is - * the first trace attachment found. An empty report (no results) is a failure — - * a spec that ran nothing did not verify anything. + * run passed iff every test's FINAL attempt passed (a target project may + * configure retries, and Playwright appends one result per attempt — the last + * one is the test's outcome), the duration is the sum of final attempts, and + * the trace is the first trace attachment found. + * + * @throws {ReportParseError} when the report contains no test results — a spec + * that ran nothing did not verify anything, and silently calling that + * "failed" would quarantine the capability with a misleading reason. The + * usual cause is a spec path outside the Playwright config's `testDir`. */ export function reduceReport(report: PwReport, testId: string, target: string): RunResult { - const results = collectResults(report.suites); + const tests = collectTests(report.suites); + // One result per test: its final attempt (retries append earlier attempts). + const finals = tests + .map((t) => t.results?.[t.results.length - 1]) + .filter((r): r is PwResult => r !== undefined); - if (results.length === 0) { - return { testId, target, status: "failed", durationMs: 0 }; + if (finals.length === 0) { + throw new ReportParseError( + `the Playwright report for '${testId}' contains no test results — no tests matched. ` + + "Check that the spec path is inside the target Playwright config's testDir/testMatch.", + ); } - const statuses = results.map((r) => mapStatus(r.status)); - const durationMs = results.reduce((sum, r) => sum + (r.duration ?? 0), 0); + const statuses = finals.map((r) => mapStatus(r.status)); + const durationMs = finals.reduce((sum, r) => sum + (r.duration ?? 0), 0); const allPassed = statuses.every((s) => s === "passed"); // Surface the most informative non-pass status when not all passed. @@ -106,7 +117,9 @@ export function reduceReport(report: PwReport, testId: string, target: string): : (statuses.find((s) => s === "timedout") ?? statuses.find((s) => s === "failed") ?? "failed"); const result: RunResult = { testId, target, status, durationMs }; - const tracePath = firstTracePath(results); + // Prefer a trace from a final attempt; fall back to any attempt's trace. + const tracePath = + firstTracePath(finals) ?? firstTracePath(tests.flatMap((t) => t.results ?? [])); if (tracePath) result.tracePath = tracePath; return result; } diff --git a/tests/compiler-emit.test.ts b/tests/compiler-emit.test.ts index 4514ab2..4303af4 100644 --- a/tests/compiler-emit.test.ts +++ b/tests/compiler-emit.test.ts @@ -23,8 +23,8 @@ describe("emitSpec", () => { expect(src).toContain(`import { expect, test } from "@playwright/test";`); expect(src).toContain(`test('user can log in', async ({ page }) => {`); expect(src).toContain(`await page.goto(BASE);`); - expect(src).toContain(`await page.getByLabel('Email').fill('a@b.com');`); - expect(src).toContain(`await page.getByRole('button', { name: 'Log in' }).click();`); + expect(src).toContain(`await page.getByLabel('Email', { exact: true }).fill('a@b.com');`); + expect(src).toContain(`await page.getByRole('button', { name: 'Log in', exact: true }).click();`); expect(src).toContain(`await expect(page.getByTestId('status')).toHaveText('Signed in');`); expect(src).toContain(`await expect(page.locator('.dashboard')).toBeVisible();`); }); @@ -51,6 +51,19 @@ describe("emitSpec", () => { expect(() => emitSpec({ ...session, actions: [] })).toThrow(/no recorded actions/); }); + it("refuses to emit a test from a session with no assertions", () => { + // Navigation and clicks alone compile to a trivially-green spec that + // verifies nothing — such a session must never reach the fidelity gate. + const assertionFree: RecordedSession = { + ...session, + actions: [ + { type: "goto", url: "http://x/" }, + { type: "click", locator: { kind: "testId", testId: "go" } }, + ], + }; + expect(() => emitSpec(assertionFree)).toThrow(/no recorded assertions/); + }); + it("does not switch to extension mode for a normal session", () => { const src = emitSpec(session); expect(src).toContain(`async ({ page }) => {`); diff --git a/tests/coverage.test.ts b/tests/coverage.test.ts index 7f575bd..3c8ad1d 100644 --- a/tests/coverage.test.ts +++ b/tests/coverage.test.ts @@ -57,6 +57,16 @@ describe("parseGraph", () => { const graph = parseGraph(JSON.stringify({ source: "x", nodes: [], edges: [] })); expect(graph.schema_version).toBe(""); }); + + it("treats a numeric schema_version as present — a number cannot bypass the guard", () => { + // A supported version emitted as a number is accepted… + const graph = parseGraph(JSON.stringify({ schema_version: 1, source: "x", nodes: [], edges: [] })); + expect(graph.schema_version).toBe("1"); + // …and an unsupported one is refused, not silently tolerated as "omitted". + expect(() => + parseGraph(JSON.stringify({ schema_version: 2, source: "x", nodes: [], edges: [] })), + ).toThrow(/unsupported rac graph schema_version '2'/); + }); }); describe("computeCoverage", () => { diff --git a/tests/drive-loop.test.ts b/tests/drive-loop.test.ts new file mode 100644 index 0000000..9cba934 --- /dev/null +++ b/tests/drive-loop.test.ts @@ -0,0 +1,127 @@ +/** + * The drive loop's control logic, unit-tested with a fake page and scripted + * models — no browser. Pins the semantics the product promise rests on: + * "finished" means the model called `finish` (a give-up is never completion), + * the step budget is honoured, and the trust boundary shapes both the + * advertised tools and dispatch. + */ + +import { describe, it, expect } from "vitest"; +import type { Page } from "@playwright/test"; + +import { AutonomousDriver } from "../src/agent/drive.js"; +import type { ModelClient, ModelRequest, ModelResponse } from "../src/agent/model.js"; +import { SHELL_TOOL_NAMES } from "../src/agent/policy.js"; + +/** A page double good enough for goto + observation; locators never resolve. */ +function fakePage(): Page { + const body = { + innerText: () => Promise.resolve("Fake page body"), + ariaSnapshot: () => Promise.resolve("- document"), + }; + return { + goto: () => Promise.resolve(null), + url: () => "http://x/", + title: () => Promise.resolve("Fake"), + locator: () => body, + on: () => undefined, + off: () => undefined, + } as unknown as Page; +} + +/** A model that replays a fixed sequence of responses, capturing each request. */ +class ScriptedModel implements ModelClient { + requests: ModelRequest[] = []; + private turn = 0; + constructor(private readonly responses: ModelResponse[]) {} + complete(request: ModelRequest): Promise { + this.requests.push(request); + const response = this.responses[Math.min(this.turn, this.responses.length - 1)]; + this.turn++; + return Promise.resolve(response ?? { done: "" }); + } +} + +const OPTIONS = { title: "t", startUrl: "http://x/", goal: "verify the thing" }; + +/** All user-role feedback in a request's transcript (the array is shared/mutable). */ +function userMessages(request: ModelRequest): string { + return request.transcript + .filter((m) => m.role === "user") + .map((m) => m.content) + .join("\n"); +} + +describe("drive finished semantics", () => { + it("a no-tool-call turn is a give-up, never completion", async () => { + const model = new ScriptedModel([{ done: "I cannot find the button, giving up." }]); + const result = await new AutonomousDriver(fakePage(), model, OPTIONS).drive(); + + expect(result.finished).toBe(false); + expect(result.stopReason).toBe("gave_up"); + expect(result.gaveUpText).toBe("I cannot find the button, giving up."); + expect(result.steps).toBe(1); + }); + + it("an explicit finish call is the only completion", async () => { + const model = new ScriptedModel([{ toolCalls: [{ name: "finish", arguments: {} }] }]); + const result = await new AutonomousDriver(fakePage(), model, OPTIONS).drive(); + + expect(result.finished).toBe(true); + expect(result.stopReason).toBe("finished"); + expect(result.gaveUpText).toBeUndefined(); + }); + + it("exhausting the step budget reports step_budget", async () => { + // A model that keeps issuing a (refused) navigate forever. + const model = new ScriptedModel([ + { toolCalls: [{ name: "navigate", arguments: { url: "https://elsewhere.example/" } }] }, + ]); + const result = await new AutonomousDriver(fakePage(), model, { ...OPTIONS, maxSteps: 3 }).drive(); + + expect(result.finished).toBe(false); + expect(result.stopReason).toBe("step_budget"); + expect(result.steps).toBe(3); + }); +}); + +describe("drive trust boundary in the loop", () => { + it("never advertises the terminal tools without allowShell, and refuses a run_command", async () => { + const model = new ScriptedModel([ + { toolCalls: [{ name: "run_command", arguments: { command: "cat /etc/passwd" } }] }, + { done: "stopping" }, + ]); + await new AutonomousDriver(fakePage(), model, { ...OPTIONS, maxSteps: 2 }).drive(); + + for (const request of model.requests) { + const names = request.tools.map((t) => t.name); + for (const shellTool of SHELL_TOOL_NAMES) expect(names).not.toContain(shellTool); + } + // The refusal came back as a failed action, naming the opt-in. + const feedback = userMessages(model.requests.at(-1)!); + expect(feedback).toContain("ERROR run_command"); + expect(feedback).toContain("--allow-shell"); + }); + + it("advertises the full catalog with allowShell", async () => { + const model = new ScriptedModel([{ toolCalls: [{ name: "finish", arguments: {} }] }]); + await new AutonomousDriver(fakePage(), model, { ...OPTIONS, allowShell: true }).drive(); + + const names = model.requests[0]!.tools.map((t) => t.name); + for (const shellTool of SHELL_TOOL_NAMES) expect(names).toContain(shellTool); + }); + + it("refuses navigate to a non-allowlisted origin and feeds the reason back", async () => { + const model = new ScriptedModel([ + { toolCalls: [{ name: "navigate", arguments: { url: "http://169.254.169.254/latest/" } }] }, + { toolCalls: [{ name: "finish", arguments: {} }] }, + ]); + const result = await new AutonomousDriver(fakePage(), model, OPTIONS).drive(); + + const feedback = userMessages(model.requests.at(-1)!); + expect(feedback).toContain("ERROR navigate"); + expect(feedback).toContain("not allowed"); + // The refused navigation was never recorded. + expect(result.session.actions).toEqual([{ type: "goto", url: "http://x/" }]); + }); +}); diff --git a/tests/http-emit.test.ts b/tests/http-emit.test.ts index fe5e605..8cc89f1 100644 --- a/tests/http-emit.test.ts +++ b/tests/http-emit.test.ts @@ -46,7 +46,11 @@ describe("emitSpec — HTTP actions", () => { const browserOnly: RecordedSession = { title: "browser", startUrl: "http://x/", - actions: [{ type: "goto", url: "http://x/" }, { type: "click", locator: { kind: "testId", testId: "go" } }], + actions: [ + { type: "goto", url: "http://x/" }, + { type: "click", locator: { kind: "testId", testId: "go" } }, + { type: "expectVisible", locator: { kind: "testId", testId: "go" } }, + ], }; expect(emitSpec(browserOnly)).not.toContain("httpRequest"); }); diff --git a/tests/playwright-report.test.ts b/tests/playwright-report.test.ts index 7385f4e..8494ed2 100644 --- a/tests/playwright-report.test.ts +++ b/tests/playwright-report.test.ts @@ -53,9 +53,36 @@ describe("reduceReport", () => { expect(reduceReport(nested, "t", "dev").status).toBe("passed"); }); - it("treats a report with no results as a failure", () => { - expect(reduceReport({ suites: [] }, "t", "dev").status).toBe("failed"); - expect(reduceReport({}, "t", "dev").status).toBe("failed"); + it("refuses a report with no results — no tests matched is not 'failed'", () => { + expect(() => reduceReport({ suites: [] }, "t", "dev")).toThrow(/no tests matched/); + expect(() => reduceReport({}, "t", "dev")).toThrow(/testDir/); + }); + + it("uses each test's final attempt when the target project configures retries", () => { + const flakyThenGreen = { + suites: [ + { + specs: [ + { + tests: [ + // Playwright appends one result per attempt; the last is the outcome. + { results: [{ status: "failed", duration: 5 }, { status: "passed", duration: 7 }] }, + ], + }, + ], + }, + ], + }; + const result = reduceReport(flakyThenGreen, "t", "dev"); + expect(result.status).toBe("passed"); + expect(result.durationMs).toBe(7); + + const retriedAndStillFailing = { + suites: [ + { specs: [{ tests: [{ results: [{ status: "failed" }, { status: "failed" }] }] }] }, + ], + }; + expect(reduceReport(retriedAndStillFailing, "t", "dev").status).toBe("failed"); }); it("passes only when every result in the spec passed", () => { diff --git a/tests/qa-command.test.ts b/tests/qa-command.test.ts index 2dcc7d4..0ff2902 100644 --- a/tests/qa-command.test.ts +++ b/tests/qa-command.test.ts @@ -64,12 +64,18 @@ function fakeDrive(captured: { options?: DriveOptions }): QaDeps["drive"] { ...(options.capabilityId !== undefined ? { capabilityId: options.capabilityId } : {}), title: options.title, startUrl: options.startUrl, - actions: [{ type: "goto", url: options.startUrl }], + actions: [ + { type: "goto", url: options.startUrl }, + // A verifiable session asserts an observable outcome (assertion-free + // sessions are unverified without compiling). + { type: "expectText", locator: { kind: "testId", testId: "status" }, text: "ok" }, + ], ...(options.plan ? { plan: "1. Navigate to the page\n2. Assert the outcome" } : {}), }; return Promise.resolve({ session, finished: true, + stopReason: "finished", steps: 2, ...(options.plan ? { plan: "1. Navigate to the page\n2. Assert the outcome" } : {}), } satisfies DriveResult); @@ -121,6 +127,46 @@ describe("runQa", () => { expect(captured.options?.allowedHosts).toEqual(["api.example.com"]); }); + it("marks a give-up unverified without compiling, and records the reason", async () => { + const learning = new InMemoryLearningStore(); + const gaveUpDrive: QaDeps["drive"] = (options) => + Promise.resolve({ + session: { title: options.title, startUrl: options.startUrl, actions: [{ type: "goto", url: options.startUrl }] }, + finished: false, + stopReason: "gave_up", + gaveUpText: "cannot find the checkout button", + steps: 4, + } satisfies DriveResult); + const deps: QaDeps = { drive: gaveUpDrive, compiler: new FakeCompiler(), runner: new FakeRunner("passed"), learning }; + + const result = await runQa(deps, { graph: GRAPH, startUrl: "http://x/", target: TARGET, n: 3 }); + + expect(result.verified).toBe(false); + expect(result.loop).toBeUndefined(); // never compiled, never gated + expect(result.unverifiedReason).toMatch(/gave up after 4 step\(s\).*cannot find the checkout button/); + const failures = await learning.priorFailures("REQ-B"); + expect(failures.map((f) => f.reason).join()).toContain("gave up"); + }); + + it("marks a finished-but-assertion-free drive unverified — nothing observable was verified", async () => { + const learning = new InMemoryLearningStore(); + const assertionFreeDrive: QaDeps["drive"] = (options) => + Promise.resolve({ + session: { title: options.title, startUrl: options.startUrl, actions: [{ type: "goto", url: options.startUrl }] }, + finished: true, + stopReason: "finished", + steps: 2, + } satisfies DriveResult); + const deps: QaDeps = { drive: assertionFreeDrive, compiler: new FakeCompiler(), runner: new FakeRunner("passed"), learning }; + + const result = await runQa(deps, { graph: GRAPH, startUrl: "http://x/", target: TARGET, n: 3 }); + + expect(result.verified).toBe(false); + expect(result.loop).toBeUndefined(); + expect(result.unverifiedReason).toMatch(/no assertions/); + expect((await learning.priorFailures("REQ-B")).length).toBe(1); + }); + it("drives the selected capability with a derived title and goal", async () => { const captured: { options?: DriveOptions } = {}; const deps: QaDeps = { drive: fakeDrive(captured), compiler: new FakeCompiler(), runner: new FakeRunner("passed") }; @@ -199,7 +245,7 @@ describe("runQa", () => { n: 1, propose: { targetPath: "rac/b.md" }, }); - expect(proposer.input?.steps).toEqual(["Navigate to http://x/"]); + expect(proposer.input?.steps).toEqual(["Navigate to http://x/", 'Expect [status] to read "ok"']); }); it("runs a planning turn and threads the plan into the write-back when enabled", async () => { diff --git a/tests/scoped-qa.test.ts b/tests/scoped-qa.test.ts index 38bcf68..400634e 100644 --- a/tests/scoped-qa.test.ts +++ b/tests/scoped-qa.test.ts @@ -65,9 +65,12 @@ function fakeDrive(driven: string[]): QaDeps["drive"] { ...(options.capabilityId !== undefined ? { capabilityId: options.capabilityId } : {}), title: options.title, startUrl: options.startUrl, - actions: [{ type: "goto", url: options.startUrl }], + actions: [ + { type: "goto", url: options.startUrl }, + { type: "expectText", locator: { kind: "testId", testId: "status" }, text: "ok" }, + ], }; - return Promise.resolve({ session, finished: true, steps: 1 } satisfies DriveResult); + return Promise.resolve({ session, finished: true, stopReason: "finished", steps: 1 } satisfies DriveResult); }; } @@ -125,9 +128,12 @@ describe("runScopedQa", () => { ...(options.capabilityId !== undefined ? { capabilityId: options.capabilityId } : {}), title: options.title, startUrl: options.startUrl, - actions: [{ type: "goto", url: options.startUrl }], + actions: [ + { type: "goto", url: options.startUrl }, + { type: "expectText", locator: { kind: "testId", testId: "status" }, text: "ok" }, + ], }; - return Promise.resolve({ session, finished: true, steps: 1 } satisfies DriveResult); + return Promise.resolve({ session, finished: true, stopReason: "finished", steps: 1 } satisfies DriveResult); }; const config: ProofkeeperConfig = { capabilities: [{ id: "REQ-B", paths: ["src/b/**"], environment: "production" }], @@ -153,9 +159,12 @@ describe("runScopedQa", () => { ...(options.capabilityId !== undefined ? { capabilityId: options.capabilityId } : {}), title: options.title, startUrl: options.startUrl, - actions: [{ type: "goto", url: options.startUrl }], + actions: [ + { type: "goto", url: options.startUrl }, + { type: "expectText", locator: { kind: "testId", testId: "status" }, text: "ok" }, + ], }; - return Promise.resolve({ session, finished: true, steps: 1 } satisfies DriveResult); + return Promise.resolve({ session, finished: true, stopReason: "finished", steps: 1 } satisfies DriveResult); }; const config: ProofkeeperConfig = { capabilities: [{ id: "REQ-B", paths: ["src/b/**"], url: "http://b/" }], @@ -176,9 +185,12 @@ describe("runScopedQa", () => { ...(options.capabilityId !== undefined ? { capabilityId: options.capabilityId } : {}), title: options.title, startUrl: options.startUrl, - actions: [{ type: "goto", url: options.startUrl }], + actions: [ + { type: "goto", url: options.startUrl }, + { type: "expectText", locator: { kind: "testId", testId: "status" }, text: "ok" }, + ], }; - return Promise.resolve({ session, finished: true, steps: 1 } satisfies DriveResult); + return Promise.resolve({ session, finished: true, stopReason: "finished", steps: 1 } satisfies DriveResult); }; const config: ProofkeeperConfig = { capabilities: [{ id: "REQ-B", paths: ["src/b/**"], url: "http://b/", persona: "viewer" }], @@ -241,9 +253,12 @@ describe("runScopedQa", () => { ...(options.capabilityId !== undefined ? { capabilityId: options.capabilityId } : {}), title: options.title, startUrl: options.startUrl, - actions: [{ type: "goto", url: options.startUrl }], + actions: [ + { type: "goto", url: options.startUrl }, + { type: "expectText", locator: { kind: "testId", testId: "status" }, text: "ok" }, + ], }; - return { session, finished: true, steps: 1 } satisfies DriveResult; + return { session, finished: true, stopReason: "finished", steps: 1 } satisfies DriveResult; }; const deps: ScopedQaDeps = { drive: slowDrive, makeCompiler: () => new FakeCompiler(), makeRunner: () => new FakeRunner("passed") }; const result = await runScopedQa(deps, { @@ -271,6 +286,7 @@ describe("runScopedQa", () => { return { session: { ...(options.capabilityId !== undefined ? { capabilityId: options.capabilityId } : {}), title: options.title, startUrl: options.startUrl, actions: [{ type: "goto", url: options.startUrl }] }, finished: true, + stopReason: "finished", steps: 1, } satisfies DriveResult; }; diff --git a/tests/terminal-emit.test.ts b/tests/terminal-emit.test.ts index b161925..a8b1a53 100644 --- a/tests/terminal-emit.test.ts +++ b/tests/terminal-emit.test.ts @@ -30,7 +30,11 @@ describe("emitSpec — terminal actions", () => { const browserOnly: RecordedSession = { title: "browser only", startUrl: "http://x/", - actions: [{ type: "goto", url: "http://x/" }, { type: "click", locator: { kind: "testId", testId: "go" } }], + actions: [ + { type: "goto", url: "http://x/" }, + { type: "click", locator: { kind: "testId", testId: "go" } }, + { type: "expectVisible", locator: { kind: "testId", testId: "go" } }, + ], }; const src = emitSpec(browserOnly); expect(src).not.toContain("node:child_process");