From 5adf0d955aa8e854d2f0dbbb5ff4bcd347d74186 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 23:59:54 -0700 Subject: [PATCH] fix(scripts): the soft-delete reconciler wrote a literal `archived` into boards that do not have one (#2999) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## A repair script that wrote a column the board does not have Under `--apply`, against an operator's live database: ```js await tx.execute(sql`UPDATE project."tasks" SET "column" = 'archived' WHERE id = ${row.id}`); ``` On a board that does not declare `archived`, that is not a mislabel — it parks the row in a column the workflow does not have, **manufacturing exactly the undeclared-column state this migration keeps repairing elsewhere**. The selection was wrong in the same direction, which made the write far worse. "Leaked" meant `column !== "archived"`, so on a renamed board **every** soft-deleted row looked leaked — including the ones resting correctly in that board's own archived lane. The repair then rewrote them. The tool's fix *was* the damage. ## Three changes, because fixing one would have left the others deciding **The SQL pre-filter carried the same literal** (`AND "column" != 'archived'`), so the query and the planner each imposed the legacy vocabulary independently. Dropped it — soft-deleted rows are a small set, so selecting them all and filtering in the pure planner costs nothing and leaves **one** place that decides what "archived" means. **The filter takes the set**; a row resting in *any* of the board's archived lanes is not leaked. **The write resolves per task**, because the destination must be that card's own lane, not a board-wide pick. A row whose archived lane cannot be resolved is **skipped and reported**, never written with a guessed id. A recovery script that declines to act on rows it does not understand is recoverable; one that writes a plausible wrong value is not. Verified rather than assumed — a store that answers nothing resolves to the default lifecycle: ``` lifecycle from unanswering store: {"intake":"todo",…,"archived":"archived"} ``` so a legacy board repairs exactly as before. ## Correcting myself On #2994 I wrote that this follow-up "needs the same `importCore` seam". It doesn't: `openBackend` already returns `{ core, store, … }` and this script already destructures `core`. No new plumbing was required. I posted that correction on #2994 too, since acting on it would have wasted someone's time. ## Revert proof ``` ✖ a soft-deleted row already in the board's RENAMED archived lane is not leaked ✖ a board with several archived lanes treats all of them as resting places ℹ pass 5 ℹ fail 2 ``` The other two new cases pass both ways by design — they guard the legacy meaning and the still-catches-a-real-leak direction — so I am not counting them as coverage of the defect. ## Verification (measured) - `node --test` across all three script suites — **17 passed / 0 failed** - `node --check`, `eslint` — clean - `check-sql-column-literals`, `lifecycle-column-census --strict`, `check-lane-wiring`, `check-fnxc-future-dates` — green No changeset: root `scripts/` is repo tooling, not part of the published package. ## Gate blind spot found while verifying this, NOT fixed here I removed a raw-SQL lane literal and expected `check-sql-column-literals` to drop from 22 — its own header says *"a LOWER count fails too so the baseline is ratcheted down"*. It stayed at **22 and green**, because it walks `PACKAGES` only and **never scans `scripts/`**. That is the same shape as the lane-wiring gap #2978 closed (it scanned neither `plugins` nor `dashboard/app`). `scripts/audit-branch-cross-contamination.mjs:185` still holds `WHERE … "column" IN ('triage','todo','in-progress','in-review')`, invisible to the gate. Left as a separate follow-up rather than bundled into a product fix. --- .../reconcile-leaked-soft-deletes.test.mjs | 53 +++++++++++++++++ scripts/reconcile-leaked-soft-deletes.mjs | 58 +++++++++++++++++-- 2 files changed, 105 insertions(+), 6 deletions(-) diff --git a/scripts/__tests__/reconcile-leaked-soft-deletes.test.mjs b/scripts/__tests__/reconcile-leaked-soft-deletes.test.mjs index e62c9bd1dd..0569814bc4 100644 --- a/scripts/__tests__/reconcile-leaked-soft-deletes.test.mjs +++ b/scripts/__tests__/reconcile-leaked-soft-deletes.test.mjs @@ -71,3 +71,56 @@ test("formatSummary renders dry-run and apply headers", () => { assert.match(formatSummary(summary, false), /^Mode: APPLY/); assert.match(formatSummary(summary, true), /FN-5130\tin-review\tfailed/); }); + +/* +FNXC:OperatorScriptLaneAssumptions 2026-07-30-23:50: +THE INVARIANT: "leaked" is measured against the board's OWN archived lanes, not the string "archived". + +Keyed on the literal, a soft-deleted row resting correctly in a renamed archived lane (`binned`) is +reported as leaked — and under `--apply` the repair then rewrites it, which is the damage rather than +the fix. The mirror case matters just as much: a genuinely leaked row must still be caught on that +same board. + +Reverted, the first case reports `FN-A` as leaked and the third stops seeing the legacy board's leak. +*/ +test("a soft-deleted row already in the board's RENAMED archived lane is not leaked", () => { + const rows = [{ id: "FN-A", column: "binned", status: null, deletedAt: "2026-01-01T00:00:00.000Z" }]; + + const summary = planReconcileLeakedSoftDeletes(rows, { runId: "r", archivedLanes: new Set(["binned"]) }); + + assert.deepEqual(summary.findings, []); +}); + +test("a soft-deleted row loose in a working lane IS leaked on that same renamed board", () => { + const rows = [{ id: "FN-B", column: "building", status: null, deletedAt: "2026-01-01T00:00:00.000Z" }]; + + const summary = planReconcileLeakedSoftDeletes(rows, { runId: "r", archivedLanes: new Set(["binned"]) }); + + assert.deepEqual(summary.findings.map((row) => row.id), ["FN-B"]); +}); + +test("unresolved lanes keep exactly the legacy meaning of leaked", () => { + const rows = [ + { id: "FN-C", column: "archived", status: null, deletedAt: "2026-01-01T00:00:00.000Z" }, + { id: "FN-D", column: "todo", status: null, deletedAt: "2026-01-01T00:00:00.000Z" }, + ]; + + const summary = planReconcileLeakedSoftDeletes(rows, { runId: "r" }); + + assert.deepEqual(summary.findings.map((row) => row.id), ["FN-D"]); +}); + +test("a board with several archived lanes treats all of them as resting places", () => { + const rows = [ + { id: "FN-E", column: "binned", status: null, deletedAt: "2026-01-01T00:00:00.000Z" }, + { id: "FN-F", column: "archived", status: null, deletedAt: "2026-01-01T00:00:00.000Z" }, + { id: "FN-G", column: "building", status: null, deletedAt: "2026-01-01T00:00:00.000Z" }, + ]; + + const summary = planReconcileLeakedSoftDeletes(rows, { + runId: "r", + archivedLanes: new Set(["binned", "archived"]), + }); + + assert.deepEqual(summary.findings.map((row) => row.id), ["FN-G"]); +}); diff --git a/scripts/reconcile-leaked-soft-deletes.mjs b/scripts/reconcile-leaked-soft-deletes.mjs index d1a6cfaf95..12d4426e11 100644 --- a/scripts/reconcile-leaked-soft-deletes.mjs +++ b/scripts/reconcile-leaked-soft-deletes.mjs @@ -33,9 +33,22 @@ export function parseArgs(argv = process.argv.slice(2)) { * Pure planning step: given task rows ({ id, column, status, deletedAt }), * report the leaked soft-deletes (deletedAt set but column != 'archived'). */ -export function planReconcileLeakedSoftDeletes(rows, { runId = `synthetic-reconcile-fn-5175-${Date.now()}` } = {}) { +/* +FNXC:OperatorScriptLaneAssumptions 2026-07-30-23:50: +`archivedLanes` is the board's OWN archived vocabulary, and it decides what counts as "leaked". + +Keyed on the literal, this filter calls a soft-deleted row leaked whenever its column is not the +string `archived` — so on a board whose archived lane is named anything else EVERY soft-deleted row +looks leaked, and `--apply` then rewrites all of them. The repair is the damage: see the write step. + +A row already resting in ANY of the board's archived lanes is not leaked, which is why the filter +takes the SET rather than a single id. The WRITE resolves per task instead — see `reconcileLeakedSoftDeletes`. +*/ +export function planReconcileLeakedSoftDeletes(rows, { runId = `synthetic-reconcile-fn-5175-${Date.now()}`, archivedLanes } = {}) { + /* DELIBERATE-LITERAL — the degraded default when the caller resolved no lanes. */ + const isArchivedLane = (column) => (archivedLanes ? archivedLanes.has(column) : column === "archived"); const findings = rows - .filter((row) => row.deletedAt != null && row.column !== "archived") + .filter((row) => row.deletedAt != null && !isArchivedLane(row.column)) .map((row) => ({ id: row.id, column: row.column, status: row.status ?? null, deletedAt: row.deletedAt })) .sort((a, b) => a.id.localeCompare(b.id)); return { @@ -47,13 +60,34 @@ export function planReconcileLeakedSoftDeletes(rows, { runId = `synthetic-reconc }; } +/* +FNXC:OperatorScriptLaneAssumptions 2026-07-30-23:50: +The repair writes each task's OWN archived lane, and REFUSES to guess when it cannot resolve one. + +This wrote the literal: `UPDATE project."tasks" SET "column" = 'archived'`. On a board that does not +declare `archived` that is not a mislabel — it parks the row in a column the workflow does not have, +manufacturing exactly the undeclared-column state this migration keeps repairing elsewhere, and it +did so under `--apply` against an operator's live database. + +The SQL pre-filter went with it. It carried the same literal (`AND "column" != 'archived'`), so the +query and the planner each imposed the legacy vocabulary and fixing only one would leave the other +silently deciding. Soft-deleted rows are a small set, so selecting them all and filtering in the +planner costs nothing and leaves ONE place that decides what "archived" means. + +A row whose own archived lane cannot be resolved is SKIPPED and reported, never written with a +guessed id. A recovery script that declines to act on the rows it does not understand is recoverable; +one that writes a plausible wrong value is not. + +Verified rather than assumed: a store that answers nothing resolves to the default lifecycle +(`archived: "archived"`), so a legacy board still repairs exactly as before. +*/ export async function reconcileLeakedSoftDeletes({ backend, dryRun = true, runId }) { - const { core, asyncLayer, sql } = backend; + const { core, store, asyncLayer, sql } = backend; const rows = rowsOf( await asyncLayer.db.execute(sql` SELECT id, "column", status, deleted_at AS "deletedAt" FROM project."tasks" - WHERE deleted_at IS NOT NULL AND "column" != 'archived' + WHERE deleted_at IS NOT NULL ORDER BY id `), ); @@ -61,7 +95,9 @@ export async function reconcileLeakedSoftDeletes({ backend, dryRun = true, runId await asyncLayer.db.execute(sql`SELECT count(*)::int AS count FROM project."tasks"`), )[0]?.count ?? rows.length; - const summary = planReconcileLeakedSoftDeletes(rows, runId ? { runId } : {}); + const archivedLanes = store && core.resolveArchivedLanes ? await core.resolveArchivedLanes(store) : undefined; + const summary = planReconcileLeakedSoftDeletes(rows, { ...(runId ? { runId } : {}), archivedLanes }); + summary.skipped = []; summary.rowsScanned = allCount; if (dryRun || summary.findings.length === 0) { @@ -69,8 +105,15 @@ export async function reconcileLeakedSoftDeletes({ backend, dryRun = true, runId } await asyncLayer.transactionImmediate(async (tx) => { + const irCache = new Map(); for (const row of summary.findings) { - await tx.execute(sql`UPDATE project."tasks" SET "column" = 'archived' WHERE id = ${row.id}`); + const target = (await core.resolveTaskLifecycleColumns(store, row.id, irCache))?.archived; + if (!target) { + /* Reported, not guessed — see the header. */ + summary.skipped.push({ id: row.id, column: row.column, reason: "unresolved-archived-lane" }); + continue; + } + await tx.execute(sql`UPDATE project."tasks" SET "column" = ${target} WHERE id = ${row.id}`); await core.recordRunAuditEventWithinTransaction(tx, { taskId: row.id, agentId: "system", @@ -100,6 +143,9 @@ export function formatSummary(summary, dryRun) { `Rows scanned: ${summary.rowsScanned}`, `Rows updated: ${summary.rowsUpdated}`, `Audit rows inserted: ${summary.auditRowsInserted}`, + ...(summary.skipped?.length + ? [`Skipped (no resolvable archived lane): ${summary.skipped.map((row) => row.id).join(", ")}`] + : []), ]; return lines.join("\n"); }