← back to Designerwallcoverings
TK-11076: Option B — undo DELETEs a metafield the forward run created (absent != empty)
f47fd6c874bcbc62a66e5d38d15a1f6a778aded4 · 2026-09-25 11:51:14 -0700 · Steve Abrams
- forward records metafield_preexisted (presence, not value), first-write-wins
- rollback deletes via metafieldsDelete only when preexisted===false AND the live
value is still exactly the forward-written value (never deletes a foreign edit);
legacy records without the flag keep write-empty
- delete-mode verify polls (<=15s) and demands ABSENT; empty residue reads FAIL
- forward gains --max-fail circuit breaker (default 3) + non-zero exit on any failure
- both scripts ship --self-test with negative tests proven RED on injected faults
Reviewed: local qwen (points refuted), Kimi k3 (2 real holes folded in).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H5X9Z41EQourT4mYoq57Np
Files touched
M scripts/colorway-title-anchor/tk11076-colorway-retag.mjsM scripts/colorway-title-anchor/tk11076-rollback.mjs
Diff
commit f47fd6c874bcbc62a66e5d38d15a1f6a778aded4
Author: Steve Abrams <steve@designerwallcoverings.com>
Date: Fri Sep 25 11:51:14 2026 -0700
TK-11076: Option B — undo DELETEs a metafield the forward run created (absent != empty)
- forward records metafield_preexisted (presence, not value), first-write-wins
- rollback deletes via metafieldsDelete only when preexisted===false AND the live
value is still exactly the forward-written value (never deletes a foreign edit);
legacy records without the flag keep write-empty
- delete-mode verify polls (<=15s) and demands ABSENT; empty residue reads FAIL
- forward gains --max-fail circuit breaker (default 3) + non-zero exit on any failure
- both scripts ship --self-test with negative tests proven RED on injected faults
Reviewed: local qwen (points refuted), Kimi k3 (2 real holes folded in).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H5X9Z41EQourT4mYoq57Np
---
.../tk11076-colorway-retag.mjs | 65 ++++++++++++-
scripts/colorway-title-anchor/tk11076-rollback.mjs | 103 +++++++++++++++++++--
2 files changed, 157 insertions(+), 11 deletions(-)
diff --git a/scripts/colorway-title-anchor/tk11076-colorway-retag.mjs b/scripts/colorway-title-anchor/tk11076-colorway-retag.mjs
index 165e0f8..e581e9b 100644
--- a/scripts/colorway-title-anchor/tk11076-colorway-retag.mjs
+++ b/scripts/colorway-title-anchor/tk11076-colorway-retag.mjs
@@ -32,12 +32,48 @@ import { fileURLToPath } from 'node:url';
import { execSync } from 'node:child_process';
import { parseColorway, titleCase } from './parse-colorway.mjs';
+// PRESENCE, not value. Shopify returns `null` for a metafield that does not exist and an object
+// (possibly `{value:""}`) for one that does. Collapsing both to '' is what made the undo blank a
+// field it had CREATED instead of deleting it. This is the single source of that distinction.
+export function metafieldExists(node) { return node !== null && node !== undefined; }
+
+if (process.argv.includes('--self-test')) {
+ // NEGATIVE TEST (CLAUDE.md TK-11431 amendment 3): prove the presence capture goes RED on the
+ // fault it exists to prevent — an ABSENT metafield must never be recorded as pre-existing,
+ // and an EXISTING-BUT-EMPTY one must never be recorded as absent.
+ const checks = [
+ ['absent metafield (null) -> NOT pre-existing', metafieldExists(null) === false],
+ ['absent metafield (undefined) -> NOT pre-existing', metafieldExists(undefined) === false],
+ ['existing metafield with a value -> pre-existing', metafieldExists({ value: 'Oyster' }) === true],
+ ['existing but EMPTY metafield -> still pre-existing (empty != absent)', metafieldExists({ value: '' }) === true],
+ ['existing with null value -> still pre-existing (the node exists)', metafieldExists({ value: null }) === true],
+ ];
+ let bad = 0;
+ for (const [n, ok] of checks) { console.log(` ${ok ? 'PASS' : 'FAIL'} ${n}`); if (!ok) bad++; }
+ console.log(bad ? `\nSELF-TEST FAILED (${bad})` : '\nSELF-TEST PASSED');
+ process.exit(bad ? 1 : 0);
+}
+
+
const HERE = path.dirname(fileURLToPath(import.meta.url));
const OUT = path.join(HERE, 'out');
fs.mkdirSync(OUT, { recursive: true });
const DB = 'postgresql:///dw_unified?host=/tmp';
const STORE = process.env.SHOPIFY_STORE || 'designer-laboratory-sandbox.myshopify.com';
const TOKEN = process.env.SHOPIFY_ADMIN_TOKEN;
+// CIRCUIT BREAKER (TK-11076, 2026-09-25 — Cody's surviving hole on the REMEASURED-HOLD memo).
+// The apply loop used to `fail++` and PUSH THROUGH every userError/VERIFY-FAIL, surfacing the
+// count only in the final tally after the whole batch had already fired. Now the run ABORTS once
+// total failures reach --max-fail (default 3). Aborting is the conservative direction: every item
+// fired before the abort is still individually snapshotted and revertible, and the operator sees
+// the failure while there is still a batch left to save. --max-fail=0 restores push-through.
+const MAX_FAIL = (() => {
+ const raw = (process.argv.find(a => a.startsWith('--max-fail=')) || '').split('=')[1];
+ if (raw === undefined) return 3;
+ const n = Number(raw);
+ if (!Number.isInteger(n) || n < 0) { console.error(`FATAL: --max-fail must be a non-negative integer, got ${JSON.stringify(raw)}`); process.exit(1); }
+ return n; // 0 = never abort (legacy push-through)
+})();
const API = process.env.SHOPIFY_API_VERSION || '2024-10';
const APPLY = process.argv.includes('--apply');
const arg = k => (process.argv.find(a => a.startsWith(`--${k}=`)) || '').split('=').slice(1).join('=');
@@ -164,6 +200,10 @@ function planProduct(p) {
}
if (!newTags.some(t => norm(t) === norm(facet))) newTags.push(facet);
+ // ABSENT != EMPTY. `p.cw` is null when the metafield does not exist at all; `(p.cw||{}).value`
+ // collapses both to ''. The undo needs the distinction: a field we CREATED must be DELETED on
+ // revert, not blanked, or every undo leaves a structurally-present empty metafield behind.
+ const metafieldPreexisted = metafieldExists(p.cw);
const oldCw = (p.cw && p.cw.value) || '';
const metafieldChange = norm(oldCw) !== norm(colorway);
const tagsChanged = JSON.stringify([...oldTags].map(norm).sort()) !== JSON.stringify([...newTags].map(norm).sort());
@@ -172,7 +212,7 @@ function planProduct(p) {
return { plan: {
id: p.id, handle: p.handle, status: p.status, vendor: p.vendor, title: p.title,
colorway, facetVal, form: parsed.form, oldTags, newTags, strippedFacet, strippedPalette,
- oldColorwayMeta: oldCw, newColorwayMeta: colorway,
+ oldColorwayMeta: oldCw, newColorwayMeta: colorway, metafieldPreexisted,
} };
}
@@ -302,7 +342,7 @@ async function run() {
stamp();
fs.writeFileSync(ROLLBACK, JSON.stringify(rollback, null, 2)); // persist the re-stamp even on a zero-plan run
- let done = 0, pass = 0, fail = 0, batch = 0;
+ let done = 0, pass = 0, fail = 0, batch = 0, aborted = false;
for (const pl of plans) {
const had = rollback[pl.handle];
rollback[pl.handle] = {
@@ -312,6 +352,10 @@ async function run() {
// would restore the mutation it was meant to reverse — verifying clean the whole way.
old_tags: had && had.old_tags ? had.old_tags : pl.oldTags,
old_metafield: had && 'old_metafield' in had ? had.old_metafield : pl.oldColorwayMeta,
+ // FIRST-WRITE-WINS too: after our first apply the field EXISTS, so re-capturing on a repeat
+ // run would record true and the undo would blank-instead-of-delete. Absent key on a legacy
+ // record means "not measured" -> the rollback keeps its old write-empty behaviour (fail-safe).
+ metafield_preexisted: had && 'metafield_preexisted' in had ? had.metafield_preexisted : pl.metafieldPreexisted,
...(had ? { reapplied_at: new Date().toISOString(), first_captured: had.ts } : {}),
};
stamp();
@@ -322,7 +366,11 @@ async function run() {
{ input: { id: pl.id, tags: pl.newTags,
metafields: [{ namespace: CW_META_NS, key: CW_META_KEY, type: 'single_line_text_field', value: pl.newColorwayMeta }] } });
const ue = d && d.productUpdate && d.productUpdate.userErrors;
- if (ue && ue.length) { console.error(` userErrors ${pl.handle}: ${JSON.stringify(ue)}`); fail++; continue; }
+ if (ue && ue.length) {
+ console.error(` userErrors ${pl.handle}: ${JSON.stringify(ue)}`); fail++;
+ if (MAX_FAIL && fail >= MAX_FAIL) { aborted = true; break; }
+ continue;
+ }
const v = await gql(`query($id:ID!){ product(id:$id){ tags m:metafield(namespace:"${CW_META_NS}",key:"${CW_META_KEY}"){ value } } }`, { id: pl.id });
const lt = (v && v.product && v.product.tags) || [];
@@ -331,6 +379,7 @@ async function run() {
const ok = facetOk && metaOk;
ok ? pass++ : (fail++, console.error(` VERIFY FAIL ${pl.handle}: facetOk=${facetOk} metaOk=${metaOk}`));
done++; console.log(`[${done}/${plans.length}] ${pl.handle} ${ok ? '✓' : 'VERIFY-FAIL'}`);
+ if (MAX_FAIL && fail >= MAX_FAIL) { aborted = true; break; }
try { execSync(`node "${path.join(process.env.HOME, '.claude/yolo-queue/executed-reversible/log-exec.mjs')}" ` +
`--agent vp-dw-commerce --ticket TK-11076 --action "colorway retag ${pl.handle}: color:${pl.colorway}" --blast 1 ` +
@@ -342,6 +391,16 @@ async function run() {
if (batch % 25 === 0 && done < plans.length) { console.log(' …90s inter-batch gap…'); await sleep(90000); }
else await sleep(700);
}
+ const remaining = plans.length - done;
console.log(`\nDONE ${VENDOR}: ${done} · verify-pass ${pass} · verify-fail ${fail} · rollback=${ROLLBACK}`);
+ if (aborted) {
+ console.error(`\nABORTED by circuit breaker: ${fail} failure(s) reached --max-fail=${MAX_FAIL}. ` +
+ `${remaining} planned product(s) were NOT touched. Every fired item is snapshotted in the ` +
+ `rollback file — undo with: node tk11076-rollback.mjs --vendor="${VENDOR}" --apply`);
+ process.exit(1);
+ }
+ // A run that fired and left failures behind must not exit 0 — a green exit code on a partly
+ // failed batch is the false-green this whole ticket keeps re-learning.
+ if (fail) { console.error(`\n${fail} item(s) did NOT verify. Do not treat this batch as applied.`); process.exit(1); }
}
run();
diff --git a/scripts/colorway-title-anchor/tk11076-rollback.mjs b/scripts/colorway-title-anchor/tk11076-rollback.mjs
index b077324..bf9ca0a 100644
--- a/scripts/colorway-title-anchor/tk11076-rollback.mjs
+++ b/scripts/colorway-title-anchor/tk11076-rollback.mjs
@@ -16,6 +16,14 @@
* an unmeasured input is never a pass.
* - userErrors were discarded and "rollback applied." printed unconditionally. Now every revert
* is verified by read-back and the run exits non-zero if any item did not come back.
+ *
+ * TK-11076 (2026-09-25) Option-B fix — TRUE structural revert of a CREATED metafield:
+ * - Writing value:'' to a metafield the forward run CREATED leaves a structurally-present empty
+ * field where there was none. Empty != absent for anything keying on presence. When the
+ * forward run recorded metafield_preexisted:false, the undo now DELETES the metafield
+ * (metafieldsDelete) and verifies by read-back that it is absent, not empty.
+ * - Fail-safe on legacy records: an entry with NO metafield_preexisted key was never measured,
+ * so it keeps the original write-empty behaviour. Only an explicit `false` triggers a delete.
*/
import fs from 'node:fs';
import path from 'node:path';
@@ -41,6 +49,27 @@ const isProductEntry = ([, r]) =>
!!r && typeof r === 'object' && !Array.isArray(r) &&
typeof r.id === 'string' && r.id.startsWith('gid://shopify/Product/');
+// Pure verify predicate, extracted so the self-test can prove the dangerous false-green — an
+// EMPTY metafield in delete-mode must read FAIL, not pass. `m` is the read-back metafield node.
+const metaReverted = (mode, m, wantMeta) =>
+ mode === 'delete'
+ ? (m === null || m === undefined)
+ : norm((m && m.value) || '') === norm(wantMeta);
+
+// Three states, never two. A legacy record that never measured presence is NOT-MEASURED and must
+// keep the old behaviour rather than guess a delete — deleting a metafield we did not create is
+// the one irreversible direction here.
+// Pure: may the undo delete this live metafield node? Only when it is still exactly our write.
+const deleteGuard = (liveNode, forwardWrote) =>
+ (liveNode === null || liveNode === undefined) ? 'absent'
+ : (typeof forwardWrote === 'string' && forwardWrote !== '' && liveNode.value === forwardWrote) ? 'delete'
+ : 'foreign';
+
+const metaMode = r =>
+ !('metafield_preexisted' in r) ? 'not-measured'
+ : r.metafield_preexisted === false ? 'delete'
+ : 'restore';
+
function classify(map, only) {
const all = Object.entries(map);
const products = all.filter(isProductEntry);
@@ -73,6 +102,27 @@ if (SELFTEST) {
['--only flags a handle with no record', b.missing.length === 1 && b.missing[0] === 'never-written-handle'],
['tag verify rejects a stray tag', sameTagSet(['A', 'B'], ['A', 'B']) && !sameTagSet(['A', 'A'], ['A', 'B'])],
['tag verify is case/whitespace-insensitive', sameTagSet([' Beige'], ['beige'])],
+ // NEGATIVE TEST for the 2026-09-25 delete-on-absent fix: prove each mode is selected, and
+ // prove the dangerous direction (deleting a field we did not create) cannot be reached by
+ // omission — an unmeasured legacy record must NOT resolve to 'delete'.
+ ['created field -> DELETE on undo', metaMode({ metafield_preexisted: false }) === 'delete'],
+ ['pre-existing field -> RESTORE on undo', metaMode({ metafield_preexisted: true }) === 'restore'],
+ ['legacy record with no flag -> NOT-MEASURED, never delete',
+ metaMode({ old_metafield: 'Oyster' }) === 'not-measured' && metaMode({ old_metafield: 'Oyster' }) !== 'delete'],
+ ['a truthy non-false value never means delete', metaMode({ metafield_preexisted: 'false' }) !== 'delete'],
+ ['delete-mode passes ONLY on an absent metafield', metaReverted('delete', null, '') === true],
+ ['delete-mode REJECTS an empty metafield (the residue false-green)',
+ metaReverted('delete', { value: '' }, '') === false],
+ ['delete-mode REJECTS a surviving value', metaReverted('delete', { value: 'Indigo' }, '') === false],
+ ['restore-mode passes on the matching value', metaReverted('restore', { value: 'Oyster' }, 'Oyster') === true],
+ ['restore-mode REJECTS a wrong value', metaReverted('restore', { value: 'Pewter' }, 'Oyster') === false],
+ ['guard: our own write -> delete', deleteGuard({ value: 'Indigo' }, 'Indigo') === 'delete'],
+ ['guard: already absent -> skip, no delete', deleteGuard(null, 'Indigo') === 'absent'],
+ ['guard: someone changed it after us -> REFUSE', deleteGuard({ value: 'Navy' }, 'Indigo') === 'foreign'],
+ ['guard: no recorded forward value -> REFUSE (never delete blind)', deleteGuard({ value: 'Indigo' }, undefined) === 'foreign'],
+ ['guard: empty live value is not our write -> REFUSE', deleteGuard({ value: '' }, '') === 'foreign'],
+ ['not-measured keeps write-empty semantics (absent still passes as empty)',
+ metaReverted('not-measured', null, '') === true],
];
let bad = 0;
for (const [name, ok] of checks) { console.log(` ${ok ? 'PASS' : 'FAIL'} ${name}`); if (!ok) bad++; }
@@ -100,20 +150,57 @@ if (missing.length) console.error(` WARN: ${missing.length} requested handle(s)
let ok = 0, fail = 0;
for (const [h, r] of entries) {
- console.log(` ${h}: tags -> [${(r.old_tags || []).join(', ')}] real_color_name -> "${r.old_metafield || '(empty)'}"`);
+ const mode = metaMode(r);
+ const metaPlan = mode === 'delete'
+ ? 'DELETE (forward run created it; absent != empty)'
+ : `-> "${r.old_metafield || '(empty)'}"${mode === 'not-measured' ? ' [legacy record: presence not measured, writing empty]' : ''}`;
+ console.log(` ${h}: tags -> [${(r.old_tags || []).join(', ')}] real_color_name ${metaPlan}`);
if (!APPLY) continue;
const want = { tags: r.old_tags || [], meta: r.old_metafield || '' };
- const d = await gql(`mutation($i:ProductInput!){ productUpdate(input:$i){ userErrors{ field message } } }`,
- { i: { id: r.id, tags: want.tags, metafields: [{ namespace: 'custom', key: 'real_color_name', type: 'single_line_text_field', value: want.meta }] } });
+
+ // Tags always revert via productUpdate. The metafield rides along ONLY when we are restoring a
+ // value; in delete-mode it must not be written first (that would create the field we are about
+ // to remove, and a failed delete would then leave the empty residue this fix exists to avoid).
+ const input = { id: r.id, tags: want.tags };
+ if (mode !== 'delete') input.metafields = [{ namespace: 'custom', key: 'real_color_name', type: 'single_line_text_field', value: want.meta }];
+ const d = await gql(`mutation($i:ProductInput!){ productUpdate(input:$i){ userErrors{ field message } } }`, { i: input });
const ue = (d && d.productUpdate && d.productUpdate.userErrors) || [];
if (d.__http || d.__nodata || ue.length) { fail++; console.error(` REVERT FAIL ${h}: ${d.__http ? 'HTTP ' + d.__http : d.__nodata ? 'no data' : JSON.stringify(ue)}`); await sleep(500); continue; }
+
+ if (mode === 'delete') {
+ // POSITIVE EVIDENCE before the one irreversible direction (Kimi review, 2026-09-25):
+ // `metafield_preexisted:false` is recorded BEFORE the forward mutation, so a failed forward or a
+ // later edit by someone else would otherwise get deleted. Delete ONLY if the live value is still
+ // exactly what the forward run wrote. Absent -> already reverted (skip). Different -> someone
+ // changed it after us: do NOT delete, count as a FAILURE so the run cannot read green.
+ const pre = await gql(`query($id:ID!){ product(id:$id){ m:metafield(namespace:"custom",key:"real_color_name"){ value } } }`, { id: r.id });
+ if (pre.__http || pre.__nodata || !pre.product) { fail++; console.error(` PRE-DELETE READ UNREADABLE ${h} — not deleting`); await sleep(500); continue; }
+ const guard = deleteGuard(pre.product.m, r.new_metafield);
+ if (guard === 'foreign') { fail++; console.error(` REFUSING DELETE ${h}: live value ${JSON.stringify(pre.product.m.value)} != forward-written ${JSON.stringify(r.new_metafield)} — changed after our write; left in place`); await sleep(500); continue; }
+ if (guard === 'delete') {
+ const dd = await gql(`mutation($m:[MetafieldIdentifierInput!]!){ metafieldsDelete(metafields:$m){ deletedMetafields{ key namespace ownerId } userErrors{ field message } } }`,
+ { m: [{ ownerId: r.id, namespace: 'custom', key: 'real_color_name' }] });
+ const de = (dd && dd.metafieldsDelete && dd.metafieldsDelete.userErrors) || [];
+ if (dd.__http || dd.__nodata || de.length) { fail++; console.error(` METAFIELD DELETE FAIL ${h}: ${dd.__http ? 'HTTP ' + dd.__http : dd.__nodata ? 'no data' : JSON.stringify(de)}`); await sleep(500); continue; }
+ }
+ }
+
// Verify by read-back — a mutation that returned clean is not proof the state came back.
- const v = await gql(`query($id:ID!){ product(id:$id){ tags m:metafield(namespace:"custom",key:"real_color_name"){ value } } }`, { id: r.id });
- const lt = (v && v.product && v.product.tags) || [];
+ let v;
+ for (let attempt = 0; attempt < (mode === 'delete' ? 6 : 1); attempt++) {
+ if (attempt) await sleep(1000 * attempt); // 1+2+3+4+5 = 15s max; delete may be processed async
+ v = await gql(`query($id:ID!){ product(id:$id){ tags m:metafield(namespace:"custom",key:"real_color_name"){ value } } }`, { id: r.id });
+ if (v.__http || v.__nodata || !v.product) break;
+ if (metaReverted(mode, v.product.m, r.old_metafield || '')) break;
+ }
+ if (v.__http || v.__nodata || !v.product) { fail++; console.error(` VERIFY UNREADABLE ${h} — not counted as reverted`); await sleep(500); continue; }
+ const lt = (v.product.tags) || [];
const tagsOk = sameTagSet(want.tags, lt);
- const metaOk = norm((v && v.product && v.product.m && v.product.m.value) || '') === norm(want.meta);
- if (tagsOk && metaOk) { ok++; console.log(` ✓ reverted + verified`); }
- else { fail++; console.error(` VERIFY FAIL ${h}: tagsOk=${tagsOk} metaOk=${metaOk}`); }
+ // delete-mode demands ABSENT (metafield node === null). An empty string here is the exact
+ // residue this fix removes, so it must read as a FAILURE, not a pass.
+ const metaOk = metaReverted(mode, v.product.m, want.meta);
+ if (tagsOk && metaOk) { ok++; console.log(` ✓ reverted + verified${mode === 'delete' ? ' (metafield ABSENT)' : ''}`); }
+ else { fail++; console.error(` VERIFY FAIL ${h}: tagsOk=${tagsOk} metaOk=${metaOk}${mode === 'delete' ? ' (expected metafield ABSENT)' : ''}`); }
await sleep(500);
}
← 590af0c TK-10484: word-boundary settlement matcher + negative test +
·
back to Designerwallcoverings
·
TK-12267: tripwire enumerate uses read-only ADMIN-first seam 820535f →