[object Object]

← back to Designerwallcoverings

TK-11307: fix silent-no-op rollback — reconstruct baseline earliest-wins, fail loud

5db825544d98618b0d1c6aae5a95a1e5439525cc · 2026-09-10 16:44:06 -0700 · Steve Abrams

The 4,728-product ShowroomOnly tag write fired on 2026-09-10 and its undo was
already non-functional.

--rollback read "the newest restore map", but a restore map is written by EVERY
prescan run INCLUDING a plain dry-run. A run started 141ms after the apply
finished recorded had_tag=true for all 4,728, became the newest map, and made
rollback compute "we added it to 0 products" -- removing nothing while still
reporting success. Measured on disk: 19:03 FALSE=4728, 19:52 FALSE=4728,
20:32 TRUE=4728 (poisoned). Reversibility is why that write was allowed to
fire, so this was a false green on the undo path.

Fix: resolveBaseline() merges every restore map oldest-first, keeping the
EARLIEST row per gid (later maps may only add unseen products, never overwrite
a known baseline), plus a fail-loud guard that aborts non-zero when it computes
0 removals while the tag is still live, plus a --map override.

Verified against the real maps: old code -> 0 to untag (no-op), new code ->
4,728 to untag with the poisoned map contributing 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011FAPArHdMKRiNHqJiorUFm

Files touched

Diff

commit 5db825544d98618b0d1c6aae5a95a1e5439525cc
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Thu Sep 10 16:44:06 2026 -0700

    TK-11307: fix silent-no-op rollback — reconstruct baseline earliest-wins, fail loud
    
    The 4,728-product ShowroomOnly tag write fired on 2026-09-10 and its undo was
    already non-functional.
    
    --rollback read "the newest restore map", but a restore map is written by EVERY
    prescan run INCLUDING a plain dry-run. A run started 141ms after the apply
    finished recorded had_tag=true for all 4,728, became the newest map, and made
    rollback compute "we added it to 0 products" -- removing nothing while still
    reporting success. Measured on disk: 19:03 FALSE=4728, 19:52 FALSE=4728,
    20:32 TRUE=4728 (poisoned). Reversibility is why that write was allowed to
    fire, so this was a false green on the undo path.
    
    Fix: resolveBaseline() merges every restore map oldest-first, keeping the
    EARLIEST row per gid (later maps may only add unseen products, never overwrite
    a known baseline), plus a fail-loud guard that aborts non-zero when it computes
    0 removals while the tag is still live, plus a --map override.
    
    Verified against the real maps: old code -> 0 to untag (no-op), new code ->
    4,728 to untag with the poisoned map contributing 0.
    
    Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_011FAPArHdMKRiNHqJiorUFm
---
 .../tk11307-showroom-tag/apply-showroomonly.mjs    | 73 +++++++++++++++++++---
 1 file changed, 65 insertions(+), 8 deletions(-)

diff --git a/scripts/tk11307-showroom-tag/apply-showroomonly.mjs b/scripts/tk11307-showroom-tag/apply-showroomonly.mjs
index 30ac583..8a1ac21 100644
--- a/scripts/tk11307-showroom-tag/apply-showroomonly.mjs
+++ b/scripts/tk11307-showroom-tag/apply-showroomonly.mjs
@@ -50,6 +50,12 @@ const args = process.argv.slice(2);
 const APPLY = args.includes('--apply');
 const ROLLBACK = args.includes('--rollback');
 const VERIFY = args.includes('--verify');
+// Explicit restore-map override for rollback. Use when the auto-reconstructed baseline is
+// unusable (see resolveBaseline). Accepts an absolute path or one relative to this script.
+const _mapIdx = args.indexOf('--map');
+const MAP_ARG = _mapIdx > -1 && args[_mapIdx + 1]
+  ? path.resolve(__dirname, args[_mapIdx + 1])
+  : null;
 const READ_BATCH = 250;   // nodes() max ids per query
 const WRITE_BATCH = 20;   // aliased tagsAdd per gql call
 const BATCH_PAUSE_MS = 300;
@@ -109,10 +115,44 @@ function writeRestoreMap(targets, byGid, runtag) {
   return { file, rows };
 }
 
-function newestRestoreMap() {
+function restoreMapFiles() {
+  // Chronological: the runtag in the filename is an ISO timestamp, so lexical sort == time sort.
   const files = fs.readdirSync(DATA).filter(f => /^restore-map-.*\.json$/.test(f)).sort();
   if (!files.length) throw new Error('no restore-map found in data/ — run --apply (or dry-run) first');
-  return path.join(DATA, files[files.length - 1]);
+  return files.map(f => path.join(DATA, f));
+}
+
+/**
+ * Reconstruct the TRUE pre-apply baseline for rollback.
+ *
+ * DO NOT use "the newest restore map". A restore map is written by EVERY prescan run —
+ * including a plain DRY-RUN — so any run that happens AFTER the tag has landed records
+ * had_tag===true for every product. If rollback trusted the newest map it would compute
+ * "we added it to 0 products", remove NOTHING, and still report success: a silent no-op
+ * that destroys the undo path for a 4,728-product live write. That exact poisoning has
+ * already happened once here (restore-map-2026-09-10T20-32-09-161Z.json, TRUE=4728).
+ *
+ * The authoritative pre-state of a product is its EARLIEST observation, so merge every
+ * map oldest-first and keep the first row seen for each gid. Later maps can only ADD
+ * products that earlier maps never saw; they can never overwrite a known baseline.
+ */
+function resolveBaseline(explicitMap) {
+  const files = explicitMap ? [explicitMap] : restoreMapFiles();
+  const byGid = new Map();
+  const used = [];
+  for (const file of files) {
+    let map;
+    try { map = JSON.parse(fs.readFileSync(file, 'utf8')); } catch { continue; }
+    if (!Array.isArray(map.rows)) continue;
+    let contributed = 0;
+    for (const r of map.rows) {
+      if (!r || !r.gid) continue;
+      if (byGid.has(r.gid)) continue;      // earliest wins — never overwrite a known baseline
+      byGid.set(r.gid, r); contributed++;
+    }
+    used.push({ file: path.basename(file), rows: map.rows.length, contributed });
+  }
+  return { byGid, used };
 }
 
 async function batchTagOp(op, gids) {
@@ -167,13 +207,30 @@ async function doVerify(targets) {
   if (VERIFY) { await doVerify(targets); return; }
 
   if (ROLLBACK) {
-    const file = newestRestoreMap();
-    console.log(`  reading restore map: ${file}`);
-    const map = JSON.parse(fs.readFileSync(file, 'utf8'));
-    // remove ONLY from products we added it to (had_tag === false)
-    const toRemove = map.rows.filter(r => r.present_in_store && r.had_tag === false).map(r => r.gid);
-    const preserved = map.rows.filter(r => r.had_tag === true).length;
+    const { byGid: baseline, used } = resolveBaseline(MAP_ARG);
+    console.log(MAP_ARG ? `  restore map (explicit --map): ${MAP_ARG}`
+                        : `  reconstructing baseline from ${used.length} restore map(s), earliest-wins:`);
+    for (const u of used) console.log(`    ${u.file}  rows=${u.rows}  baseline rows contributed=${u.contributed}`);
+    const rows = [...baseline.values()];
+    // remove ONLY from products we added it to (had_tag === false at their EARLIEST observation)
+    const toRemove = rows.filter(r => r.present_in_store && r.had_tag === false).map(r => r.gid);
+    const preserved = rows.filter(r => r.had_tag === true).length;
     console.log(`  will tagsRemove '${TAG}' from ${toRemove.length} products (preserving ${preserved} that had it before).`);
+
+    // FAIL LOUD: a rollback that would remove nothing while the tag is still live is the
+    // silent-no-op failure this guard exists to prevent. Never report success for it.
+    if (toRemove.length === 0) {
+      const live = await doVerify(targets);
+      if (live > 0) {
+        console.error(`\n  ROLLBACK ABORTED — computed 0 products to untag, but '${TAG}' is live on ${live} products.`);
+        console.error(`  The baseline is unusable (every restore map on disk was written AFTER the tag landed).`);
+        console.error(`  Do NOT treat this as a completed rollback. Pass the pre-apply map explicitly:`);
+        console.error(`    node apply-showroomonly.mjs --rollback --apply --map data/restore-map-<pre-apply>.json`);
+        process.exitCode = 1; return;
+      }
+      console.log(`\n  Nothing to remove and tag is live on 0 products — already rolled back. OK`);
+      return;
+    }
     if (!APPLY) { console.log('\n  ROLLBACK is a preview unless combined with --apply. Re-run: --rollback --apply'); return; }
     const res = await batchTagOp('tagsRemove', toRemove);
     const fails = res.filter(r => !r.ok);

← 68bf79a TK-11404: theme-fix spec for handoff (reorder dead; leak is  ·  back to Designerwallcoverings  ·  TK-11307: theme deploy targeted an UNPUBLISHED theme; rollba 0a6978b →