From e611eea0ecd58c69f01bf151a56cb12a8baf355f Mon Sep 17 00:00:00 2001 From: Manuel Fittko Date: Sat, 11 Jul 2026 02:43:34 +0200 Subject: [PATCH 1/4] feat(loop): drive-session row tagging so ui_review teardown drops drive-created rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Stage-2 drive now advertises a per-run drive-session id on the X-UI-Review-Drive-Session request header and emits a session-stamped row manifest (one record per mutating step) in its result envelope. Stage-5 teardown consumes the manifest and, on --confirm, deletes exactly the tagged rows via the project's uiReview.run.rowTeardown.deleteCommand (dev DB only). An absent/untagged manifest or a missing recipe still fails closed to a drop failure or may-remain-untagged — never a guess or a wrong-scope delete. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0177JAWNT67FY6d48qQJeDwo --- docs/ui-review-recipe-contract.md | 28 ++- packages/core/src/config/config.mjs | 21 ++- packages/core/src/loop/ui-review-drive.mjs | 27 ++- packages/core/src/loop/ui-review-teardown.mjs | 13 +- scripts/loop/ui-review-drive.mjs | 15 +- scripts/loop/ui-review-teardown.mjs | 66 ++++++- test/docs/ui-review-recipe-doc.test.mjs | 2 +- test/loop/ui-review-drive.test.mjs | 35 ++++ test/loop/ui-review-teardown.test.mjs | 172 ++++++++++++++++++ 9 files changed, 354 insertions(+), 25 deletions(-) diff --git a/docs/ui-review-recipe-contract.md b/docs/ui-review-recipe-contract.md index 77d38a7c..d3774d31 100644 --- a/docs/ui-review-recipe-contract.md +++ b/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` diff --git a/packages/core/src/config/config.mjs b/packages/core/src/config/config.mjs index 448c10dd..b69209d9 100644 --- a/packages/core/src/config/config.mjs +++ b/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,6 +1756,10 @@ 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(), @@ -1749,6 +1767,7 @@ export function resolveUiReviewRunRecipe(config) { readyIntervalMs: Number.isInteger(run.readyIntervalMs) ? run.readyIntervalMs : 1000, cwd: typeof run.cwd === "string" && run.cwd.trim().length > 0 ? run.cwd.trim() : null, migrate, + rowTeardown, }; } diff --git a/packages/core/src/loop/ui-review-drive.mjs b/packages/core/src/loop/ui-review-drive.mjs index 01ac60f8..c202bd9c 100644 --- a/packages/core/src/loop/ui-review-drive.mjs +++ b/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} 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(), }; } diff --git a/packages/core/src/loop/ui-review-teardown.mjs b/packages/core/src/loop/ui-review-teardown.mjs index 49c23efa..34aed6cc 100644 --- a/packages/core/src/loop/ui-review-teardown.mjs +++ b/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({ diff --git a/scripts/loop/ui-review-drive.mjs b/scripts/loop/ui-review-drive.mjs index 17d579ea..0b6b32a0 100644 --- a/scripts/loop/ui-review-drive.mjs +++ b/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). @@ -371,9 +374,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 +393,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 }), diff --git a/scripts/loop/ui-review-teardown.mjs b/scripts/loop/ui-review-teardown.mjs index cb841340..83387420 100644 --- a/scripts/loop/ui-review-teardown.mjs +++ b/scripts/loop/ui-review-teardown.mjs @@ -19,6 +19,7 @@ import { readFileSync, statSync } from "node:fs"; import { parseArgs } from "node:util"; import { buildParseError, formatCliError, isDirectCliRun } from "../_core-helpers.mjs"; import { requireTokenValue, runChild } from "../_cli-primitives.mjs"; +import { loadDevLoopConfig, resolveUiReviewRunRecipe } from "@dev-loops/core/config"; import { teardown } from "@dev-loops/core/loop/ui-review-teardown"; import { cleanupWorktree } from "./cleanup-worktree.mjs"; import { JQ_OUTPUT_PARSE_OPTIONS, JQ_OUTPUT_USAGE, emitResult, matchJqOutputToken } from "../lib/jq-output.mjs"; @@ -229,6 +230,54 @@ async function deleteGist({ id }, { run = runChild } = {}) { return { ok: false, detail: `gh gist delete exit ${res.code}: ${res.stderr?.trim() || "no stderr"}` }; } +/** The single drive-session id every manifest row shares, or null. Fails closed + * on an absent OR mixed session — the delete targets one session, so an ambiguous + * or untagged manifest must never drop (better a "may remain" than a wrong scope). */ +function sessionFromManifest(rows) { + const ids = new Set(); + for (const r of rows ?? []) { + const s = typeof r?.session === "string" ? r.session.trim() : ""; + if (s.length > 0) ids.add(s); + } + return ids.size === 1 ? [...ids][0] : null; +} + +/** Drop the dev-DB rows a drive tagged with its session, via the project's + * `uiReview.run.rowTeardown.deleteCommand`. The shared session id is passed in + * UI_REVIEW_DRIVE_SESSION and the command runs in the provisioned worktree (dev + * DB). Fail closed: no delete recipe, or a manifest with no single session, drops + * nothing and says why — the core maps that to a drop failure, never a silent + * no-op or a wrong-scope delete. */ +async function dropRows({ rows }, { deleteCommand, cwd, run = runChild }) { + if (!deleteCommand) { + return { ok: false, dropped: 0, detail: "no uiReview.run.rowTeardown.deleteCommand configured; cannot drop tagged rows" }; + } + const session = sessionFromManifest(rows); + if (!session) { + return { ok: false, dropped: 0, detail: "manifest rows carry no single drive-session tag; refusing to drop (ambiguous target)" }; + } + const env = { ...process.env, UI_REVIEW_DRIVE_SESSION: session }; + // POSIX single-quote the cwd — it is interpolated into a shell `cd`, and a + // git-branch-derived worktree path may carry `$()`/backtick command- + // substitution chars that would otherwise execute during a confirmed teardown. + const quotedCwd = `'${cwd.replace(/'/g, "'\\''")}'`; + const res = await run("sh", ["-c", `cd ${quotedCwd} && ${deleteCommand}`], env); + if (res.code === 0) { + // `dropped` is the number of manifested mutating steps requested for deletion, + // not a confirmed DB-row count — the by-session deleteCommand returns none. + return { ok: true, dropped: rows.length, detail: `requested deletion of rows tagged ${session} (${rows.length} manifested step(s)) via rowTeardown.deleteCommand` }; + } + return { ok: false, dropped: 0, detail: `rowTeardown.deleteCommand exit ${res.code}: ${res.stderr?.trim() || "no stderr"}` }; +} + +/** Resolve the project's row-delete command from the (worktree) config. Loaded + * lazily — only when a confirmed manifest actually needs dropping — so a teardown + * with no rows never reads config. */ +async function resolveDeleteCommand(repoRoot) { + const { config } = await loadDevLoopConfig({ repoRoot }); + return resolveUiReviewRunRecipe(config)?.rowTeardown?.deleteCommand ?? null; +} + export async function runCli(argv = process.argv.slice(2), { stdout = process.stdout, stderr = process.stderr, run = runChild } = {}) { const options = parseUiReviewTeardownCliArgs(argv); if (options.help) { stdout.write(`${USAGE}\n`); return; } @@ -238,6 +287,12 @@ export async function runCli(argv = process.argv.slice(2), { stdout = process.st const reportResult = readResultJson(options.reportResult); const rowManifest = readRowManifest(options.rowManifest); + // The row-delete command lives in the branch's (worktree) config; fall back to + // the primary checkout when no worktree path was captured. The dev DB the + // command targets is the worktree's, so the command runs there. + const rawWorktreePath = provisionResult?.worktreePath; + const dropCwd = typeof rawWorktreePath === "string" && rawWorktreePath.trim().length > 0 ? rawWorktreePath : options.repoRoot; + const result = await teardown( { provisionResult, @@ -250,12 +305,11 @@ export async function runCli(argv = process.argv.slice(2), { stdout = process.st { killProcess, deleteGist: (a) => deleteGist(a, { run }), - // No real drop seam yet: Stage 2 does not tag rows, so an actual manifest - // never arrives today and the ledger reports "may remain (untagged)". A - // manifest handed in is dropped by whatever project mechanism supplies it; - // until one exists, a confirmed manifest is a hard fail-closed error rather - // than a silent no-op, so a caller can never think rows were dropped. - dropRows: () => Promise.resolve({ ok: false, dropped: 0, detail: "no row-drop mechanism wired: dev-DB rows are untagged (Stage 2 does not tag rows); cannot drop a manifest safely" }), + // The Stage-2 drive stamps each mutating step with its drive-session id and + // emits a manifest; this seam deletes exactly the rows the app tagged with + // that session via the project's rowTeardown.deleteCommand. The delete + // command is resolved lazily (only a confirmed manifest reaches this seam). + dropRows: async (a) => dropRows(a, { deleteCommand: await resolveDeleteCommand(dropCwd), cwd: dropCwd, run }), removeWorktree: (a) => removeWorktree(options.repoRoot, a), log: (msg) => stderr.write(`[ui-review-teardown] ${msg}\n`), }, diff --git a/test/docs/ui-review-recipe-doc.test.mjs b/test/docs/ui-review-recipe-doc.test.mjs index 01747c53..325297e5 100644 --- a/test/docs/ui-review-recipe-doc.test.mjs +++ b/test/docs/ui-review-recipe-doc.test.mjs @@ -114,7 +114,7 @@ test("recipe doc config keys match the shipped uiReview/worktree schema", async // Anchor the count so a wrapped-then-dropped key (see below) can't pass by // matching a doc that dropped the same key. - assert.equal(schema.size, 34, "expected 34 uiReview/worktree schema leaves"); + assert.equal(schema.size, 35, "expected 35 uiReview/worktree schema leaves"); }); test("collectKeys descends through effects/preprocess wrappers at nested levels", () => { diff --git a/test/loop/ui-review-drive.test.mjs b/test/loop/ui-review-drive.test.mjs index 902ae3c6..189f725e 100644 --- a/test/loop/ui-review-drive.test.mjs +++ b/test/loop/ui-review-drive.test.mjs @@ -379,6 +379,41 @@ test("driveUiReview: caps a flow at maxStepsPerFlow and logs the truncation", as assert.ok(logs.some((m) => /maxStepsPerFlow cap \(2\)/.test(m)), "the step truncation is logged"); }); +test("driveUiReview: emits a session-stamped row manifest for the mutating steps it drove (goto/fill excluded)", async () => { + const r = await driveUiReview( + { + appUrl: "http://app", + login: {}, + driveSession: "sess-abc", + flows: [{ + name: "create widget", + steps: [ + { name: "open", action: "goto", path: "/widgets/new" }, // navigation — not a mutation + { name: "name", action: "fill", selector: "#name" }, // typing — not a mutation + { name: "save", action: "click", selector: "button[type=submit]" }, // create + { name: "logo", action: "upload", selector: "#logo", value: "/f.png" }, // upload + ], + }], + }, + baseSeams(), + ); + assert.equal(r.driveSession, "sess-abc"); + // Exactly the two mutating steps are manifested, each stamped with the session. + assert.deepEqual(r.rowManifest, [ + { session: "sess-abc", flow: "create widget", step: "save", action: "click" }, + { session: "sess-abc", flow: "create widget", step: "logo", action: "upload" }, + ]); +}); + +test("driveUiReview: no driveSession => driveSession null and an empty manifest (nothing to drop)", async () => { + const r = await driveUiReview( + { appUrl: "http://app", login: {}, flows: [{ name: "f", steps: [{ name: "save", action: "click", selector: "#s" }] }] }, + baseSeams(), + ); + assert.equal(r.driveSession, null); + assert.deepEqual(r.rowManifest, []); +}); + test("driveUiReview: collates a swallowed error response from the injected listener + log tail", async () => { const r = await driveUiReview( { appUrl: "http://app", login: {}, flows: [{ name: "f", steps: [{ name: "save", action: "click" }] }], serverLogExceptionPattern: DEFAULT_SERVER_LOG_EXCEPTION_PATTERN }, diff --git a/test/loop/ui-review-teardown.test.mjs b/test/loop/ui-review-teardown.test.mjs index 8681f134..7d94bbbc 100644 --- a/test/loop/ui-review-teardown.test.mjs +++ b/test/loop/ui-review-teardown.test.mjs @@ -6,6 +6,7 @@ import { join } from "node:path"; import test from "node:test"; import { teardown, ROW_STATUS, WORKTREE_STATUS, PROCESS_STATUS, GIST_STATUS } from "@dev-loops/core/loop/ui-review-teardown"; +import { driveUiReview } from "@dev-loops/core/loop/ui-review-drive"; import { parseUiReviewTeardownCliArgs, killProcess, runCli } from "../../scripts/loop/ui-review-teardown.mjs"; // A Stage-1 provision result: a booted app (pid), applied migrations, a worktree. @@ -429,6 +430,177 @@ test("runCli: prunes the report-result gist via gh gist delete (confirmed), no l assert.deepEqual(ghCalls[0], ["gh", "gist", "delete", "abc123def456"]); }); +// AC end-to-end: a drive that creates rows emits a session-stamped manifest, and +// teardown drops EXACTLY those rows on confirm — no real DB (the drop seam is +// mocked to record the rows it was handed). +test("drive-emitted manifest drops exactly the drive-created rows on confirm (ledger reports dropped with the count)", async () => { + const drive = await driveUiReview( + { + appUrl: "http://app", + login: {}, + driveSession: "sess-42", + flows: [{ + name: "create widget", + steps: [ + { name: "open", action: "goto", path: "/widgets/new" }, + { name: "save", action: "click", selector: "button[type=submit]" }, + { name: "logo", action: "upload", selector: "#logo", value: "/f.png" }, + ], + }], + }, + { + authenticate: async () => ({ ok: true, detail: "ok" }), + runStep: async ({ step }) => ({ ok: true, screenshotPath: `/o/${step.name}.png` }), + getCapturedEvents: () => ({ responses: [], requestFailures: [], pageErrors: [] }), + }, + ); + // The drive emitted a manifest for exactly the two mutating steps. + assert.equal(drive.rowManifest.length, 2); + + let droppedRows = null; + const { seams } = makeSeams({ + dropRows: async ({ rows }) => { droppedRows = rows; return { ok: true, dropped: rows.length, detail: "dropped" }; }, + }); + const res = await teardown( + { provisionResult: PROVISION, driveResult: drive, rowManifest: drive.rowManifest, confirm: true }, + seams, + ); + + // Teardown dropped EXACTLY the manifested rows the drive created. + assert.deepEqual(droppedRows, drive.rowManifest); + assert.equal(res.ledger.rows.status, ROW_STATUS.DROPPED); + assert.equal(res.ledger.rows.dropped, 2, "ledger reports the count, not may-remain-untagged"); + assert.notEqual(res.ledger.rows.status, ROW_STATUS.MAY_REMAIN_UNTAGGED); +}); + +// The CLI drop seam is real: it deletes by drive-session via the project's +// rowTeardown.deleteCommand, in the worktree, with the session in the env — no +// live DB (the command runner is mocked). +test("runCli: confirmed manifest deletes by session via rowTeardown.deleteCommand (mocked runner, dropped count in the ledger)", async () => { + const dir = mkdtempSync(join(tmpdir(), "teardown-rowdrop-")); + writeFileSync(join(dir, ".devloops.json"), JSON.stringify({ + version: 1, + uiReview: { run: { command: "bin/app", readyUrl: "http://127.0.0.1:4000/healthz", rowTeardown: { deleteCommand: "bin/cleanup-rows" } } }, + })); + const provisionPath = join(dir, "provision.json"); + writeFileSync(provisionPath, JSON.stringify({ ...PROVISION, worktreePath: dir, boot: {} })); + const manifestPath = join(dir, "manifest.json"); + writeFileSync(manifestPath, JSON.stringify([ + { session: "sess-xyz", flow: "create widget", step: "save", action: "click" }, + { session: "sess-xyz", flow: "create widget", step: "logo", action: "upload" }, + ])); + + const runCalls = []; + const run = async (cmd, args, env) => { runCalls.push({ cmd, args, env }); return { code: 0, stdout: "", stderr: "" }; }; + + let out = ""; + const stdout = { write: (s) => { out += s; return true; } }; + const stderr = { write: () => true }; + const argv = ["--repo-root", dir, "--provision-result", provisionPath, "--row-manifest", manifestPath, "--confirm", "--no-stop-app"]; + await runCli(argv, { stdout, stderr, run }); + + // The delete command ran in the worktree with the drive session in the env. + assert.equal(runCalls.length, 1); + assert.equal(runCalls[0].cmd, "sh"); + assert.match(runCalls[0].args[1], /bin\/cleanup-rows/); + assert.match(runCalls[0].args[1], new RegExp(dir)); + assert.equal(runCalls[0].env.UI_REVIEW_DRIVE_SESSION, "sess-xyz"); + // The ledger reports the drop with its count. + const result = JSON.parse(out); + assert.equal(result.ledger.rows.status, ROW_STATUS.DROPPED); + assert.equal(result.ledger.rows.dropped, 2); +}); + +// Fail closed: a confirmed manifest with no rowTeardown recipe drops nothing and +// is recorded as a drop failure — never a silent no-op or a wrong-scope delete. +test("runCli: confirmed manifest but no rowTeardown recipe fails closed (DROP_FAILED, ok:false), runner never invoked", async () => { + const dir = mkdtempSync(join(tmpdir(), "teardown-norecipe-")); + writeFileSync(join(dir, ".devloops.json"), JSON.stringify({ + version: 1, + uiReview: { run: { command: "bin/app", readyUrl: "http://127.0.0.1:4000/healthz" } }, + })); + const provisionPath = join(dir, "provision.json"); + writeFileSync(provisionPath, JSON.stringify({ ...PROVISION, worktreePath: dir, boot: {} })); + const manifestPath = join(dir, "manifest.json"); + writeFileSync(manifestPath, JSON.stringify([{ session: "sess-xyz", step: "save", action: "click" }])); + + const runCalls = []; + const run = async (...a) => { runCalls.push(a); return { code: 0, stdout: "", stderr: "" }; }; + let out = ""; + const stdout = { write: (s) => { out += s; return true; } }; + const stderr = { write: () => true }; + const argv = ["--repo-root", dir, "--provision-result", provisionPath, "--row-manifest", manifestPath, "--confirm", "--no-stop-app"]; + await runCli(argv, { stdout, stderr, run }); + process.exitCode = 0; // runCli sets exitCode=1 on the (expected) fail-closed drop; don't leak it to the runner + + assert.equal(runCalls.length, 0, "no delete recipe => the runner is never invoked"); + const result = JSON.parse(out); + assert.equal(result.ledger.rows.status, ROW_STATUS.DROP_FAILED); + assert.match(result.ledger.rows.detail, /no uiReview\.run\.rowTeardown\.deleteCommand/i); + assert.equal(result.ok, false); +}); + +// Fail closed: a configured recipe + a manifest with no single shared session +// (mixed sessions here) must refuse — the delete targets one session, so an +// ambiguous manifest never runs the command (guards a wrong-scope delete). +test("runCli: configured recipe but ambiguous manifest session fails closed (DROP_FAILED, runner never invoked)", async () => { + const dir = mkdtempSync(join(tmpdir(), "teardown-ambig-")); + writeFileSync(join(dir, ".devloops.json"), JSON.stringify({ + version: 1, + uiReview: { run: { command: "bin/app", readyUrl: "http://127.0.0.1:4000/healthz", rowTeardown: { deleteCommand: "bin/cleanup-rows" } } }, + })); + const provisionPath = join(dir, "provision.json"); + writeFileSync(provisionPath, JSON.stringify({ ...PROVISION, worktreePath: dir, boot: {} })); + const manifestPath = join(dir, "manifest.json"); + writeFileSync(manifestPath, JSON.stringify([ + { session: "sess-a", step: "save", action: "click" }, + { session: "sess-b", step: "logo", action: "upload" }, // different session => ambiguous target + ])); + + const runCalls = []; + const run = async (...a) => { runCalls.push(a); return { code: 0, stdout: "", stderr: "" }; }; + let out = ""; + const stdout = { write: (s) => { out += s; return true; } }; + const stderr = { write: () => true }; + const argv = ["--repo-root", dir, "--provision-result", provisionPath, "--row-manifest", manifestPath, "--confirm", "--no-stop-app"]; + await runCli(argv, { stdout, stderr, run }); + process.exitCode = 0; // fail-closed drop sets exitCode=1; don't leak it to the runner + + assert.equal(runCalls.length, 0, "ambiguous session => the runner is never invoked"); + const result = JSON.parse(out); + assert.equal(result.ledger.rows.status, ROW_STATUS.DROP_FAILED); + assert.equal(result.ledger.rows.dropped, 0); + assert.match(result.ledger.rows.detail, /no single drive-session tag|ambiguous/i); +}); + +// A real non-zero exit from the delete command is a drop failure, not a silent +// success — guards the exit-code check in the real dropRows seam. +test("runCli: rowTeardown.deleteCommand non-zero exit => DROP_FAILED (ok:false), exit surfaced in the ledger detail", async () => { + const dir = mkdtempSync(join(tmpdir(), "teardown-exit1-")); + writeFileSync(join(dir, ".devloops.json"), JSON.stringify({ + version: 1, + uiReview: { run: { command: "bin/app", readyUrl: "http://127.0.0.1:4000/healthz", rowTeardown: { deleteCommand: "bin/cleanup-rows" } } }, + })); + const provisionPath = join(dir, "provision.json"); + writeFileSync(provisionPath, JSON.stringify({ ...PROVISION, worktreePath: dir, boot: {} })); + const manifestPath = join(dir, "manifest.json"); + writeFileSync(manifestPath, JSON.stringify([{ session: "sess-xyz", step: "save", action: "click" }])); + + const run = async () => ({ code: 1, stdout: "", stderr: "connection refused" }); + let out = ""; + const stdout = { write: (s) => { out += s; return true; } }; + const stderr = { write: () => true }; + const argv = ["--repo-root", dir, "--provision-result", provisionPath, "--row-manifest", manifestPath, "--confirm", "--no-stop-app"]; + await runCli(argv, { stdout, stderr, run }); + process.exitCode = 0; // fail-closed drop sets exitCode=1; don't leak it to the runner + + const result = JSON.parse(out); + assert.equal(result.ledger.rows.status, ROW_STATUS.DROP_FAILED); + assert.equal(result.ledger.rows.dropped, 0); + assert.match(result.ledger.rows.detail, /rowTeardown\.deleteCommand exit 1/); + assert.equal(result.ok, false); +}); + test("parseUiReviewTeardownCliArgs: requires --repo-root and --provision-result", () => { assert.throws(() => parseUiReviewTeardownCliArgs(["--provision-result", "/p.json"]), /repo-root/); assert.throws(() => parseUiReviewTeardownCliArgs(["--repo-root", "/r"]), /provision-result/); From 03bdb97bce5d14ebbd9a68e1af3200c2d61a6050 Mon Sep 17 00:00:00 2001 From: Manuel Fittko Date: Sat, 11 Jul 2026 03:00:43 +0200 Subject: [PATCH 2/4] fix(loop): refuse a partially-untagged row manifest in teardown sessionFromManifest only collected non-empty sessions, so a manifest with one tagged row and the rest untagged resolved to a single session and dropped it while the untagged rows went unaccounted-for but were reported dropped. Any untagged row now fails closed (refuse, DROP_FAILED), never a partial delete misreported as complete. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0177JAWNT67FY6d48qQJeDwo --- scripts/loop/ui-review-teardown.mjs | 10 ++++++-- test/loop/ui-review-teardown.test.mjs | 33 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/scripts/loop/ui-review-teardown.mjs b/scripts/loop/ui-review-teardown.mjs index 83387420..bab6630b 100644 --- a/scripts/loop/ui-review-teardown.mjs +++ b/scripts/loop/ui-review-teardown.mjs @@ -234,10 +234,16 @@ async function deleteGist({ id }, { run = runChild } = {}) { * on an absent OR mixed session — the delete targets one session, so an ambiguous * or untagged manifest must never drop (better a "may remain" than a wrong scope). */ function sessionFromManifest(rows) { + const list = rows ?? []; + if (list.length === 0) return null; const ids = new Set(); - for (const r of rows ?? []) { + for (const r of list) { const s = typeof r?.session === "string" ? r.session.trim() : ""; - if (s.length > 0) ids.add(s); + // Any untagged row means part of the manifest can't be scoped to a session: + // refuse rather than delete only the tagged subset and misreport the rest as + // dropped. Every row must carry the same non-empty session. + if (s.length === 0) return null; + ids.add(s); } return ids.size === 1 ? [...ids][0] : null; } diff --git a/test/loop/ui-review-teardown.test.mjs b/test/loop/ui-review-teardown.test.mjs index 7d94bbbc..317d154b 100644 --- a/test/loop/ui-review-teardown.test.mjs +++ b/test/loop/ui-review-teardown.test.mjs @@ -573,6 +573,39 @@ test("runCli: configured recipe but ambiguous manifest session fails closed (DRO assert.match(result.ledger.rows.detail, /no single drive-session tag|ambiguous/i); }); +// A PARTIALLY-untagged manifest (some rows tagged, some untagged) must also refuse: +// deleting only the tagged subset would leave the untagged rows while reporting +// DROPPED. Any untagged row => refuse, runner never invoked. +test("runCli: partially-untagged manifest fails closed (DROP_FAILED, runner never invoked)", async () => { + const dir = mkdtempSync(join(tmpdir(), "teardown-partial-")); + writeFileSync(join(dir, ".devloops.json"), JSON.stringify({ + version: 1, + uiReview: { run: { command: "bin/app", readyUrl: "http://127.0.0.1:4000/healthz", rowTeardown: { deleteCommand: "bin/cleanup-rows" } } }, + })); + const provisionPath = join(dir, "provision.json"); + writeFileSync(provisionPath, JSON.stringify({ ...PROVISION, worktreePath: dir, boot: {} })); + const manifestPath = join(dir, "manifest.json"); + writeFileSync(manifestPath, JSON.stringify([ + { session: "sess-a", step: "save", action: "click" }, + { session: "", step: "logo", action: "upload" }, // untagged row => can't be scoped + ])); + + const runCalls = []; + const run = async (...a) => { runCalls.push(a); return { code: 0, stdout: "", stderr: "" }; }; + let out = ""; + const stdout = { write: (s) => { out += s; return true; } }; + const stderr = { write: () => true }; + const argv = ["--repo-root", dir, "--provision-result", provisionPath, "--row-manifest", manifestPath, "--confirm", "--no-stop-app"]; + await runCli(argv, { stdout, stderr, run }); + process.exitCode = 0; + + assert.equal(runCalls.length, 0, "a partially-untagged manifest => the runner is never invoked"); + const result = JSON.parse(out); + assert.equal(result.ledger.rows.status, ROW_STATUS.DROP_FAILED); + assert.equal(result.ledger.rows.dropped, 0); + assert.match(result.ledger.rows.detail, /no single drive-session tag|ambiguous/i); +}); + // A real non-zero exit from the delete command is a drop failure, not a silent // success — guards the exit-code check in the real dropRows seam. test("runCli: rowTeardown.deleteCommand non-zero exit => DROP_FAILED (ok:false), exit surfaced in the ledger detail", async () => { From a602362c73a21c7d1b20bc5b73cdb0636250c7fb Mon Sep 17 00:00:00 2001 From: Manuel Fittko Date: Sat, 11 Jul 2026 03:10:56 +0200 Subject: [PATCH 3/4] fix(loop): keep the drive envelope shape stable on the no-recipe early return The fail-closed return when no uiReview.login recipe is declared omitted the documented driveSession/rowManifest keys (they were only set once a session was generated later), so the CLI output shape depended on the failure mode. Emit driveSession:null + rowManifest:[] there too, and pin it with a runCli test. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0177JAWNT67FY6d48qQJeDwo --- scripts/loop/ui-review-drive.mjs | 4 ++++ test/loop/ui-review-drive.test.mjs | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/scripts/loop/ui-review-drive.mjs b/scripts/loop/ui-review-drive.mjs index 0b6b32a0..e9926f26 100644 --- a/scripts/loop/ui-review-drive.mjs +++ b/scripts/loop/ui-review-drive.mjs @@ -365,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 }); diff --git a/test/loop/ui-review-drive.test.mjs b/test/loop/ui-review-drive.test.mjs index 189f725e..b738a5a7 100644 --- a/test/loop/ui-review-drive.test.mjs +++ b/test/loop/ui-review-drive.test.mjs @@ -20,6 +20,7 @@ import { makeRunStep, openServerLogTail, toPerStateConsolePayload, + runCli, } from "../../scripts/loop/ui-review-drive.mjs"; // A fake page for the real makeRunStep wiring: the emitter interface @@ -414,6 +415,28 @@ test("driveUiReview: no driveSession => driveSession null and an empty manifest assert.deepEqual(r.rowManifest, []); }); +test("runCli: no uiReview.login recipe still emits the stable envelope shape (driveSession null, empty manifest)", async () => { + const dir = mkdtempSync(path.join(tmpdir(), "drive-norecipe-")); + after(() => rmSync(dir, { recursive: true, force: true })); + writeFileSync( + path.join(dir, ".devloops.json"), + JSON.stringify({ version: 1, uiReview: { run: { command: "bin/app", readyUrl: "http://127.0.0.1:4000/healthz" } } }), + ); + let out = ""; + const stdout = { write: (s) => { out += s; return true; } }; + const stderr = { write: () => true }; + await runCli(["--repo-root", dir, "--app-url", "http://127.0.0.1:4000", "--output-dir", path.join(dir, "out")], { stdout, stderr }); + process.exitCode = 0; + + const r = JSON.parse(out); + assert.equal(r.ok, false); + assert.equal(r.stopped, true); + // The documented envelope keys are present on the fail-closed early return too, + // so a downstream consumer sees a stable shape regardless of failure mode. + assert.equal(r.driveSession, null); + assert.deepEqual(r.rowManifest, []); +}); + test("driveUiReview: collates a swallowed error response from the injected listener + log tail", async () => { const r = await driveUiReview( { appUrl: "http://app", login: {}, flows: [{ name: "f", steps: [{ name: "save", action: "click" }] }], serverLogExceptionPattern: DEFAULT_SERVER_LOG_EXCEPTION_PATTERN }, From 9958c529e9d52f02639f51d1444d56c48fc9e9d2 Mon Sep 17 00:00:00 2001 From: Manuel Fittko Date: Sat, 11 Jul 2026 03:19:53 +0200 Subject: [PATCH 4/4] fix(loop): fail closed on a malformed provision worktreePath before the row delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A present-but-non-string worktreePath signals a corrupted provision result; the old fallback silently used the primary checkout as the delete cwd, risking a wrong-scope destructive delete. Resolve a malformed path to null and refuse the drop (DROP_FAILED) — distinct from an absent/empty path, which still legitimately falls back to the repo root. The cwd guard runs first so the refusal is explicit. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0177JAWNT67FY6d48qQJeDwo --- scripts/loop/ui-review-teardown.mjs | 18 ++++++++++++---- test/loop/ui-review-teardown.test.mjs | 30 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/scripts/loop/ui-review-teardown.mjs b/scripts/loop/ui-review-teardown.mjs index bab6630b..46965167 100644 --- a/scripts/loop/ui-review-teardown.mjs +++ b/scripts/loop/ui-review-teardown.mjs @@ -255,6 +255,12 @@ function sessionFromManifest(rows) { * nothing and says why — the core maps that to a drop failure, never a silent * no-op or a wrong-scope delete. */ async function dropRows({ rows }, { deleteCommand, cwd, run = runChild }) { + if (typeof cwd !== "string" || cwd.trim().length === 0) { + // No verified dev-scope directory to run the delete in (a malformed provision + // worktreePath resolves to null upstream). Refuse rather than run the + // destructive command in an unverified/wrong scope. + return { ok: false, dropped: 0, detail: "no verified worktree cwd for the row delete (malformed provision worktreePath); refusing to run in an unverified scope" }; + } if (!deleteCommand) { return { ok: false, dropped: 0, detail: "no uiReview.run.rowTeardown.deleteCommand configured; cannot drop tagged rows" }; } @@ -293,11 +299,15 @@ export async function runCli(argv = process.argv.slice(2), { stdout = process.st const reportResult = readResultJson(options.reportResult); const rowManifest = readRowManifest(options.rowManifest); - // The row-delete command lives in the branch's (worktree) config; fall back to - // the primary checkout when no worktree path was captured. The dev DB the - // command targets is the worktree's, so the command runs there. + // The row-delete command lives in the branch's (worktree) config and runs in the + // worktree (its dev DB). Fall back to the primary checkout only when NO worktree + // path was captured (absent/empty). A present-but-malformed (non-string) + // worktreePath signals a corrupted provision result — resolve to null so the + // delete fails closed rather than silently running in the wrong scope. const rawWorktreePath = provisionResult?.worktreePath; - const dropCwd = typeof rawWorktreePath === "string" && rawWorktreePath.trim().length > 0 ? rawWorktreePath : options.repoRoot; + const dropCwd = rawWorktreePath != null && typeof rawWorktreePath !== "string" + ? null + : (typeof rawWorktreePath === "string" && rawWorktreePath.trim().length > 0 ? rawWorktreePath : options.repoRoot); const result = await teardown( { diff --git a/test/loop/ui-review-teardown.test.mjs b/test/loop/ui-review-teardown.test.mjs index 317d154b..88aef59d 100644 --- a/test/loop/ui-review-teardown.test.mjs +++ b/test/loop/ui-review-teardown.test.mjs @@ -634,6 +634,36 @@ test("runCli: rowTeardown.deleteCommand non-zero exit => DROP_FAILED (ok:false), assert.equal(result.ok, false); }); +// A present-but-malformed (non-string) worktreePath is a corrupted provision +// result — the destructive delete must NOT silently fall back to the primary +// checkout scope. Refuse (runner never invoked), distinct from an absent path. +test("runCli: malformed (non-string) provision worktreePath fails closed (DROP_FAILED, runner never invoked)", async () => { + const dir = mkdtempSync(join(tmpdir(), "teardown-badwt-")); + writeFileSync(join(dir, ".devloops.json"), JSON.stringify({ + version: 1, + uiReview: { run: { command: "bin/app", readyUrl: "http://127.0.0.1:4000/healthz", rowTeardown: { deleteCommand: "bin/cleanup-rows" } } }, + })); + const provisionPath = join(dir, "provision.json"); + writeFileSync(provisionPath, JSON.stringify({ ...PROVISION, worktreePath: { bad: true }, boot: {} })); + const manifestPath = join(dir, "manifest.json"); + writeFileSync(manifestPath, JSON.stringify([{ session: "sess-xyz", step: "save", action: "click" }])); + + const runCalls = []; + const run = async (...a) => { runCalls.push(a); return { code: 0, stdout: "", stderr: "" }; }; + let out = ""; + const stdout = { write: (s) => { out += s; return true; } }; + const stderr = { write: () => true }; + const argv = ["--repo-root", dir, "--provision-result", provisionPath, "--row-manifest", manifestPath, "--confirm", "--no-stop-app"]; + await runCli(argv, { stdout, stderr, run }); + process.exitCode = 0; + + assert.equal(runCalls.length, 0, "a malformed worktreePath => the runner is never invoked"); + const result = JSON.parse(out); + assert.equal(result.ledger.rows.status, ROW_STATUS.DROP_FAILED); + assert.equal(result.ledger.rows.dropped, 0); + assert.match(result.ledger.rows.detail, /malformed provision worktreePath|unverified scope/i); +}); + test("parseUiReviewTeardownCliArgs: requires --repo-root and --provision-result", () => { assert.throws(() => parseUiReviewTeardownCliArgs(["--provision-result", "/p.json"]), /repo-root/); assert.throws(() => parseUiReviewTeardownCliArgs(["--repo-root", "/r"]), /provision-result/);