feat(loop): drive-session row tagging so ui_review teardown drops exactly the drive-created rows

docs/ui-review-recipe-contract.md

@@ -39,17 +39,22 @@ drive your app.
 2. **Auth + drive** — launches one headless WebKit context, authenticates
    through your dev-login recipe, dismisses declared interstitials once, then
    walks the changed UI flows, capturing a screenshot + `state.json` + `snapshot.json` + `axe.json` + `console.json` per step.
+   Each run carries a unique **drive-session id** on the
+   `X-UI-Review-Drive-Session` request header and emits a **row manifest** — one
+   session-stamped record per mutating step — so teardown can drop exactly the
+   dev-DB rows the walk created (see `uiReview.run.rowTeardown`).
 3. **Diagnose** — maps each captured failure to a source line and then to a PR
    diff line so a finding can anchor on a real changed line.
 4. **Report** — posts a head-pinned **pending** PR review and produces a
    self-contained HTML artifact.
 5. **Teardown** — stops the booted app and, only with explicit confirmation,
-   removes the worktree (the working destructive step) and prunes the Stage-4
-   hosting gist (`gh gist delete`, when a gist id is recorded in the report
-   result). Dev-DB row-drop currently **fails closed** — the tagged-drop seam is
-   not wired, so a confirmed manifest is recorded as a drop failure and untagged
-   rows may remain (a real tagged drop is a tracked follow-up). A side-effect
-   ledger is ALWAYS emitted, enumerating what was done vs. left behind.
+   drops the drive-tagged dev-DB rows (`uiReview.run.rowTeardown.deleteCommand`,
+   dev DB only), removes the worktree, and prunes the Stage-4 hosting gist
+   (`gh gist delete`, when a gist id is recorded in the report result). Row-drop
+   consumes the drive's emitted manifest and deletes exactly the rows tagged with
+   that run's drive-session id; an absent manifest, an untagged one, or a missing
+   `rowTeardown` recipe **fails closed** (rows may remain) rather than guessing. A
+   side-effect ledger is ALWAYS emitted, enumerating what was done vs. left behind.
 
 ## Guardrails (non-negotiable)
 
@@ -107,6 +112,14 @@ worktree:
     destructive guard is inert — set a `destructivePattern` matching your own
     status format (e.g. a `destructive`/`down` marker), or make `statusCommand`
     emit the destructive SQL/marker.
+- `uiReview.run.rowTeardown` — optional dev-DB row-teardown sub-recipe. The drive
+  advertises a per-run drive-session id on the `X-UI-Review-Drive-Session` request
+  header; instrument your create/edit/upload paths to tag the rows they persist
+  with that header value so teardown can remove exactly them.
+  - `uiReview.run.rowTeardown.deleteCommand` — shell command that deletes the rows
+    tagged with a drive session. Teardown runs it in the provisioned worktree (dev
+    DB) with the session id in the `UI_REVIEW_DRIVE_SESSION` env var, only on
+    `--confirm`. Absent recipe means row-drop fails closed (rows may remain).
 
 ```yaml
 uiReview:
@@ -119,6 +132,8 @@ uiReview:
     migrate:
       statusCommand: "bin/example-migrate status"    # example
       applyCommand: "bin/example-migrate up"         # example
+    rowTeardown:
+      deleteCommand: "bin/example-cleanup --session \"$UI_REVIEW_DRIVE_SESSION\""  # example
 ```
 
 ## `uiReview.login` — dev-login recipe
@@ -278,6 +293,7 @@ from the schema or if the schema gains a key not listed here.
 - `uiReview.run.migrate.statusCommand`
 - `uiReview.run.migrate.applyCommand`
 - `uiReview.run.migrate.destructivePattern`
+- `uiReview.run.rowTeardown.deleteCommand`
 - `uiReview.login.loginUrl`
 - `uiReview.login.usernameSelector`
 - `uiReview.login.usernameValue`

packages/core/src/config/config.mjs

@@ -281,6 +281,18 @@ const UiReviewMigrateConfig = z.strictObject({
     .optional(),
 });
 
+/**
+ * Per-project dev-DB row-teardown recipe (Stage 5). The drive stamps each
+ * mutating step it drives with a drive-session id (advertised to the app on the
+ * DRIVE_SESSION_HEADER request header); this `deleteCommand` deletes exactly the
+ * rows the app tagged with that session — the id is passed in the
+ * UI_REVIEW_DRIVE_SESSION env var and the command runs in the provisioned
+ * worktree (dev DB only). Teardown runs it only on explicit confirmation.
+ */
+const UiReviewRowTeardownConfig = z.strictObject({
+  deleteCommand: z.string().trim().min(1),
+});
+
 /**
  * Per-project boot recipe: a shell `command` that starts the branch's app and a
  * `readyUrl` an HTTP readiness probe polls until the app is up (never a fixed
@@ -305,6 +317,7 @@ const UiReviewRunConfig = z.strictObject({
   readyIntervalMs: z.number().int().min(1).max(60000).default(1000),
   cwd: z.string().trim().min(1).optional(),
   migrate: UiReviewMigrateConfig.optional(),
+  rowTeardown: UiReviewRowTeardownConfig.optional(),
 });
 
 /**
@@ -1729,7 +1742,8 @@ export const DEFAULT_DESTRUCTIVE_MIGRATION_PATTERN =
  * @param {DevLoopConfig} config
  * @returns {null | { command: string, readyUrl: string, readyTimeoutMs: number,
  *   readyIntervalMs: number, cwd: string|null,
- *   migrate: null | { statusCommand: string, applyCommand: string, destructivePattern: string } }}
+ *   migrate: null | { statusCommand: string, applyCommand: string, destructivePattern: string },
+ *   rowTeardown: null | { deleteCommand: string } }}
  */
 export function resolveUiReviewRunRecipe(config) {
   const run = config?.uiReview?.run;
@@ -1742,13 +1756,18 @@ export function resolveUiReviewRunRecipe(config) {
         destructivePattern: run.migrate.destructivePattern ?? DEFAULT_DESTRUCTIVE_MIGRATION_PATTERN,
       }
     : null;
+  const rowTeardown =
+    run.rowTeardown && typeof run.rowTeardown.deleteCommand === "string" && run.rowTeardown.deleteCommand.trim().length > 0
+      ? { deleteCommand: run.rowTeardown.deleteCommand.trim() }
+      : null;
   return {
     command: run.command.trim(),
     readyUrl: run.readyUrl.trim(),
     readyTimeoutMs: Number.isInteger(run.readyTimeoutMs) ? run.readyTimeoutMs : 60000,
     readyIntervalMs: Number.isInteger(run.readyIntervalMs) ? run.readyIntervalMs : 1000,
     cwd: typeof run.cwd === "string" && run.cwd.trim().length > 0 ? run.cwd.trim() : null,
     migrate,
+    rowTeardown,
   };
 }
 

packages/core/src/loop/ui-review-drive.mjs

@@ -26,6 +26,16 @@
 
 const MUST_FIX = "must-fix";
 
+/** Request header the drive advertises its drive-session id on, so a cooperating
+ * app can tag the dev-DB rows a create/edit/upload persists during the walk.
+ * Stage-5 teardown deletes exactly those tagged rows from an emitted manifest. */
+export const DRIVE_SESSION_HEADER = "X-UI-Review-Drive-Session";
+
+/** Step actions that can persist dev-DB state (a create/edit/reorder/upload/
+ * toggle). `goto` is navigation and `fill` only types into a field before a
+ * submit, so neither is recorded as a row-creating mutation in the manifest. */
+const MUTATING_ACTIONS = new Set(["click", "select", "upload", "dispatch"]);
+
 /** The one owner of the error-response threshold: an error response is anything
  * outside 2xx/3xx. 3xx redirects are normal navigation (login/canonical), not
  * errors, so they are not flagged. Shared by the CLI listener's pre-filter (for
@@ -212,6 +222,9 @@ export function classifyFailures({
  * @param {object} input
  * @param {string} input.appUrl - The arbitrary running-app URL from Stage 1.
  * @param {object} input.login - Resolved dev-login recipe (loginUrl + selectors).
+ * @param {string|null} [input.driveSession] - Unique id advertised to the app on
+ *   DRIVE_SESSION_HEADER; stamps the emitted row manifest so Stage-5 teardown can
+ *   drop exactly the rows a mutating step created. Null => no manifest is emitted.
  * @param {object[]} [input.flows] - Allowlisted changed-flow definitions.
  * @param {object[]} [input.interstitials] - Config-declared dismiss selectors.
  * @param {string[]} [input.changedPaths] - Changed file paths (drives selection).
@@ -227,7 +240,7 @@ export function classifyFailures({
  * @returns {Promise<object>} Result envelope (steps, captures, failures, caps, logs).
  */
 export async function driveUiReview(
-  { appUrl, login, flows = [], interstitials = [], changedPaths = [], serverLogExceptionPattern, caps = {} },
+  { appUrl, login, flows = [], interstitials = [], changedPaths = [], serverLogExceptionPattern, caps = {}, driveSession = null },
   {
     authenticate,
     dismissInterstitials = async () => ({ dismissed: [] }),
@@ -246,7 +259,8 @@ export async function driveUiReview(
   // No-retry is a fixed policy — log it every run so the bound is never implicit.
   record(`caps: maxScreenshots=${resolvedCaps.maxScreenshots}, maxFlows=${resolvedCaps.maxFlows}, maxStepsPerFlow=${resolvedCaps.maxStepsPerFlow}, retries=${resolvedCaps.retries} (no-retry)`);
 
-  const base = () => ({ appUrl: appUrl ?? null, logs });
+  const session = typeof driveSession === "string" && driveSession.trim().length > 0 ? driveSession.trim() : null;
+  const base = () => ({ appUrl: appUrl ?? null, logs, driveSession: session });
 
   // 1. Authenticate as the target role. Fail closed: no session -> STOP, drive
   //    nothing (a review that never reached the app is worthless, not empty).
@@ -262,6 +276,7 @@ export async function driveUiReview(
       captures: [],
       failures: [{ kind: "auth-failed", severity: MUST_FIX, message: stopReason }],
       caps: resolvedCaps,
+      rowManifest: [],
       ...base(),
     };
   }
@@ -281,6 +296,10 @@ export async function driveUiReview(
   //    moves on — deterministic, bounded, never re-run.
   const steps = [];
   const captures = [];
+  // Row manifest: one session-tagged record per mutating step driven, so Stage-5
+  // teardown can drop exactly the dev-DB rows this walk created. Only built when a
+  // session is present (no session => nothing to tag => no manifest to drop).
+  const rowManifest = [];
   let screenshots = 0;
   let screensSkipped = 0;
   for (const flow of selected) {
@@ -314,6 +333,9 @@ export async function driveUiReview(
       };
       steps.push(entry);
       if (entry.screenshotPath) captures.push({ flow: flow.name, step: entry.step, screenshotPath: entry.screenshotPath, statePath: entry.statePath });
+      if (session && MUTATING_ACTIONS.has(step.action)) {
+        rowManifest.push({ session, flow: flow.name, step: entry.step, action: step.action });
+      }
       if (!ok) record(`step failed (no retry): ${flow.name} / ${entry.step}: ${entry.detail ?? "unknown"}`);
     }
   }
@@ -344,6 +366,7 @@ export async function driveUiReview(
     failures,
     caps: resolvedCaps,
     screensSkipped,
+    rowManifest,
     ...base(),
   };
 }

packages/core/src/loop/ui-review-teardown.mjs

@@ -2,7 +2,7 @@
  * Teardown + side-effect ledger orchestrator for the ui_review route (Stage 5).
  *
  * Terminal cleanup for a running-app review: stop the app booted in Stage 1,
- * drop the dev-DB rows the Stage-2 drive created, and remove the provisioned
+ * drop the dev-DB rows the Stage-2 drive tagged, and remove the provisioned
  * worktree. The core safety property of this stage is that a side-effect ledger
  * is ALWAYS emitted — enumerating every migration applied, row created/dropped,
  * the worktree path, and any process left running — so nothing the loop touched
@@ -28,11 +28,12 @@
  */
 
 /**
- * The honest row-drop reality: Stage 2 does NOT tag the dev-DB rows it creates
- * with a session id or row manifest. So unless an explicit row manifest is
- * handed in (and confirmed), this stage CANNOT know which rows to drop and MUST
- * NOT guess. When the drive ran mutating flows without a manifest, the ledger
- * reports rows "may remain (untagged)" rather than dropping anything.
+ * Row-drop model: Stage 2 stamps each mutating step with a drive-session id and
+ * emits a session-tagged row manifest. Given that manifest (and confirmation),
+ * this stage drops exactly the rows tagged with that session. Only the fallback
+ * case — a drive that mutated but handed in no manifest — CANNOT know which rows
+ * to drop and MUST NOT guess: the ledger reports rows "may remain (untagged)"
+ * rather than dropping anything.
  */
 
 const ROW_STATUS = Object.freeze({

scripts/loop/ui-review-drive.mjs

@@ -21,6 +21,7 @@
  * (packages/core/src/loop/ui-review-drive.mjs). The decision logic (flow
  * selection, cap enforcement, failure classification) lives in core.
  */
+import { randomUUID } from "node:crypto";
 import { statSync } from "node:fs";
 import { open } from "node:fs/promises";
 import path from "node:path";
@@ -29,7 +30,7 @@ import { webkit } from "@playwright/test";
 import { buildParseError, formatCliError, isDirectCliRun } from "../_core-helpers.mjs";
 import { requireTokenValue } from "../_cli-primitives.mjs";
 import { loadDevLoopConfig, resolveUiReviewDriveRecipe } from "@dev-loops/core/config";
-import { driveUiReview, isErrorResponseStatus, PAGE_ERROR_STACK_MAX_CHARS } from "@dev-loops/core/loop/ui-review-drive";
+import { driveUiReview, isErrorResponseStatus, PAGE_ERROR_STACK_MAX_CHARS, DRIVE_SESSION_HEADER } from "@dev-loops/core/loop/ui-review-drive";
 import { captureNamedUiState } from "../../test/playwright/harness/webkit-smoke-harness.mjs";
 import { JQ_OUTPUT_PARSE_OPTIONS, JQ_OUTPUT_USAGE, emitResult, matchJqOutputToken } from "../lib/jq-output.mjs";
 
@@ -47,7 +48,9 @@ Optional:
 Output (stdout, JSON):
   { "ok": bool, "stopped": bool, "stopReason": string|null,
     "steps": [...], "captures": [...], "failures": [...], "caps": {...},
-    "appUrl": string|null, "logs": [...] }
+    "appUrl": string|null, "driveSession": string|null, "rowManifest": [...], "logs": [...] }
+  driveSession is advertised to the app on the ${DRIVE_SESSION_HEADER} header and stamps
+  each rowManifest record (a mutating step), so Stage-5 teardown can drop exactly those rows.
   On the walk (non-stopped) path the envelope also carries "screensSkipped": number
   (steps dropped past the maxScreenshots cap).
 
@@ -362,6 +365,10 @@ export async function runCli(argv = process.argv.slice(2), { stdout = process.st
       failures: [{ kind: "drive-recipe-missing", severity: "must-fix", message: "declare uiReview.login (loginUrl + submitSelector + successSelector) in .devloops" }],
       caps: {},
       appUrl: options.appUrl ?? null,
+      // Keep the envelope shape stable across every return path: no drive ran, so
+      // there is no session and nothing to tag.
+      driveSession: null,
+      rowManifest: [],
       logs: [],
     };
     process.exitCode = emitResult(result, { jq: options.jq, silent: options.silent, stdout, stderr });
@@ -371,9 +378,14 @@ export async function runCli(argv = process.argv.slice(2), { stdout = process.st
   const serverLogPath = recipe.serverLogPath ? path.resolve(options.repoRoot, recipe.serverLogPath) : null;
   const logTail = openServerLogTail(serverLogPath, { log: (msg) => stderr.write(`[ui-review-drive] ${msg}\n`) });
 
+  // One id per drive run: advertised to the app on every request via the session
+  // header so a cooperating app can tag the dev-DB rows a mutating step persists,
+  // and stamped onto the emitted row manifest so teardown drops exactly those.
+  const driveSession = randomUUID();
+
   const browser = await webkit.launch({ headless: true });
   try {
-    const context = await browser.newContext();
+    const context = await browser.newContext({ extraHTTPHeaders: { [DRIVE_SESSION_HEADER]: driveSession } });
     const page = await context.newPage();
     const { getCapturedEvents, sliceCapturedEvents } = attachPageListeners(page);
     const result = await driveUiReview(
@@ -385,6 +397,7 @@ export async function runCli(argv = process.argv.slice(2), { stdout = process.st
         changedPaths: options.changedPaths,
         serverLogExceptionPattern: recipe.serverLogExceptionPattern,
         caps: recipe.caps,
+        driveSession,
       },
       {
         authenticate: () => authenticate({ page, login: recipe.login }),