← back to Dw Signup Fulfillment
TK-11285: the race was bidirectional — stop the service clobbering too
f9de58c0a9684cb58a429ce682f80e107b9830d1 · 2026-09-10 08:02:42 -0700 · Steve Abrams
Cody (contrarian) returned FIX FIRST on my bef6f4f/f849a46 work and was right.
I fixed recover-stuck-apps.js clobbering the service, but not the service
clobbering recover-stuck-apps.js. Verified independently before acting.
checkpointApproval() re-read the FILE (so new applications survived) but `app`
is a pre-await snapshot of the ROW, and `{...latest[index], ...app}` re-asserted
every field it held. So while approve() awaited Shopify and George, a concurrent
link by recover-stuck-apps.js was reverted: the row ended up status:'approved'
with a valid shopify_customer_id but link_status:'unlinked' and a stale
link_error — a self-contradictory record, and the --send-only filter then skips
it, so a designer can silently never receive the activation letter.
Reproduced 3/3 with my own harness (verification/tk11285/reverse-race.js,
independent of Cody's), before: "CLAIM CONFIRMED", after: "CLAIM NOT REPRODUCED".
Fix merges only the 7 fields a decision actually owns (status, decision,
decided_at, approval_error, assigned_rep, shopify_customer_id,
approval_progress), enumerated exhaustively from every `app.X =` in approve()
and reject(). Explicit key test rather than a bare spread — the same
undefined-still-spreads trap I hit in f849a46. approve() and reject() both
funnel through this one function, so it is a single-point fix.
Regression after changing the service hot path:
trade-approval-test.js 27/27 PASS 0 FAIL (the accepted artifact, unchanged)
commit-rows-test.js 9/9 PASS
forward race 3/3 NO LOSS, control 3/3 NO LOSS
reverse race 3/3 NOT REPRODUCED
DRY_RUN selftest all pass
Known cosmetic residual: recover-stuck-apps.js does not clear a stale link_error
when a link later succeeds. Pre-existing, not introduced here, left alone.
Local artifact only — no deploy, no push, no live writes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEDp3MXiKVJ5GDKaQCHZon
Files touched
M lib/trade.jsM verification/tk11285/control.jsonM verification/tk11285/race.jsonA verification/tk11285/reverse-race-after.jsonA verification/tk11285/reverse-race-before.jsonA verification/tk11285/reverse-race.jsA verification/tk11285/stubs-inject.js
Diff
commit f9de58c0a9684cb58a429ce682f80e107b9830d1
Author: Steve Abrams <steve@designerwallcoverings.com>
Date: Thu Sep 10 08:02:42 2026 -0700
TK-11285: the race was bidirectional — stop the service clobbering too
Cody (contrarian) returned FIX FIRST on my bef6f4f/f849a46 work and was right.
I fixed recover-stuck-apps.js clobbering the service, but not the service
clobbering recover-stuck-apps.js. Verified independently before acting.
checkpointApproval() re-read the FILE (so new applications survived) but `app`
is a pre-await snapshot of the ROW, and `{...latest[index], ...app}` re-asserted
every field it held. So while approve() awaited Shopify and George, a concurrent
link by recover-stuck-apps.js was reverted: the row ended up status:'approved'
with a valid shopify_customer_id but link_status:'unlinked' and a stale
link_error — a self-contradictory record, and the --send-only filter then skips
it, so a designer can silently never receive the activation letter.
Reproduced 3/3 with my own harness (verification/tk11285/reverse-race.js,
independent of Cody's), before: "CLAIM CONFIRMED", after: "CLAIM NOT REPRODUCED".
Fix merges only the 7 fields a decision actually owns (status, decision,
decided_at, approval_error, assigned_rep, shopify_customer_id,
approval_progress), enumerated exhaustively from every `app.X =` in approve()
and reject(). Explicit key test rather than a bare spread — the same
undefined-still-spreads trap I hit in f849a46. approve() and reject() both
funnel through this one function, so it is a single-point fix.
Regression after changing the service hot path:
trade-approval-test.js 27/27 PASS 0 FAIL (the accepted artifact, unchanged)
commit-rows-test.js 9/9 PASS
forward race 3/3 NO LOSS, control 3/3 NO LOSS
reverse race 3/3 NOT REPRODUCED
DRY_RUN selftest all pass
Known cosmetic residual: recover-stuck-apps.js does not clear a stale link_error
when a link later succeeds. Pre-existing, not introduced here, left alone.
Local artifact only — no deploy, no push, no live writes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEDp3MXiKVJ5GDKaQCHZon
---
lib/trade.js | 18 +++++++++-
verification/tk11285/control.json | 2 +-
verification/tk11285/race.json | 2 +-
verification/tk11285/reverse-race-after.json | 21 +++++++++++
verification/tk11285/reverse-race-before.json | 21 +++++++++++
verification/tk11285/reverse-race.js | 52 +++++++++++++++++++++++++++
verification/tk11285/stubs-inject.js | 38 ++++++++++++++++++++
7 files changed, 151 insertions(+), 3 deletions(-)
diff --git a/lib/trade.js b/lib/trade.js
index 4afa8aa..d0cb433 100644
--- a/lib/trade.js
+++ b/lib/trade.js
@@ -181,11 +181,27 @@ function tagsIncludeTrade(tags) {
}
// Do not retain a pre-await snapshot of the entire file: new applications may
// arrive while Shopify or email is in flight.
+//
+// The row is a pre-await snapshot too. `app` was read before the Shopify and email
+// calls, so spreading all of it over the fresh row re-asserts whatever it held at
+// that moment and silently reverts anything another writer changed meanwhile -
+// scripts/recover-stuck-apps.js links a row and this writes link_status back to
+// 'unlinked', leaving status:'approved' with a valid customer id but a stale
+// link_error. Merge only the fields a decision actually owns.
+const DECISION_OWNED = ['status', 'decision', 'decided_at', 'approval_error',
+ 'assigned_rep', 'shopify_customer_id', 'approval_progress'];
+
function checkpointApproval(app) {
const latest = readAll();
const index = latest.findIndex(row => row.id === app.id);
if (index < 0 || latest[index].status !== 'pending') throw new Error('application_changed');
- latest[index] = { ...latest[index], ...app };
+ const patch = {};
+ // Explicit key test, never a bare spread: an undefined value still spreads and
+ // would erase the fresh row's field.
+ for (const field of DECISION_OWNED) {
+ if (app[field] !== undefined) patch[field] = app[field];
+ }
+ latest[index] = { ...latest[index], ...patch };
rewriteAll(latest);
}
diff --git a/verification/tk11285/control.json b/verification/tk11285/control.json
index 9d8c746..9033e3b 100644
--- a/verification/tk11285/control.json
+++ b/verification/tk11285/control.json
@@ -4,7 +4,7 @@
"approve_status_returned": "approved",
"rows_before": 6,
"rows_after": 7,
- "new_intake_id": "TRADE-20260910-85f58e",
+ "new_intake_id": "TRADE-20260910-0f4d53",
"new_intake_survived": true,
"seed1_status_after": "approved",
"seed1_decision_survived": true,
diff --git a/verification/tk11285/race.json b/verification/tk11285/race.json
index 1441c08..6694b99 100644
--- a/verification/tk11285/race.json
+++ b/verification/tk11285/race.json
@@ -4,7 +4,7 @@
"approve_status_returned": "approved",
"rows_before": 6,
"rows_after": 7,
- "new_intake_id": "TRADE-20260910-14b0e3",
+ "new_intake_id": "TRADE-20260910-b4b036",
"new_intake_survived": true,
"seed1_status_after": "approved",
"seed1_decision_survived": true,
diff --git a/verification/tk11285/reverse-race-after.json b/verification/tk11285/reverse-race-after.json
new file mode 100644
index 0000000..bc08c2c
--- /dev/null
+++ b/verification/tk11285/reverse-race-after.json
@@ -0,0 +1,21 @@
+{
+ "claim": "service (checkpointApproval) clobbers recover-owned fields with a pre-await row snapshot",
+ "approve_ok": true,
+ "recover_wrote": {
+ "link_status": "linked",
+ "recovery_emailed": true,
+ "recovered_at": "2026-09-10T00:00:00.000Z"
+ },
+ "final_row": {
+ "status": "approved",
+ "shopify_customer_id": "5551111",
+ "link_status": "linked",
+ "link_error": "shopify_create_failed",
+ "recovery_emailed": true,
+ "recovered_at": "2026-09-10T00:00:00.000Z"
+ },
+ "link_status_clobbered": false,
+ "receipt_clobbered": false,
+ "self_contradictory": false,
+ "VERDICT": "CLAIM NOT REPRODUCED"
+}
diff --git a/verification/tk11285/reverse-race-before.json b/verification/tk11285/reverse-race-before.json
new file mode 100644
index 0000000..02a8ef8
--- /dev/null
+++ b/verification/tk11285/reverse-race-before.json
@@ -0,0 +1,21 @@
+{
+ "claim": "service (checkpointApproval) clobbers recover-owned fields with a pre-await row snapshot",
+ "approve_ok": true,
+ "recover_wrote": {
+ "link_status": "linked",
+ "recovery_emailed": true,
+ "recovered_at": "2026-09-10T00:00:00.000Z"
+ },
+ "final_row": {
+ "status": "approved",
+ "shopify_customer_id": "5551111",
+ "link_status": "unlinked",
+ "link_error": "shopify_create_failed",
+ "recovery_emailed": true,
+ "recovered_at": "2026-09-10T00:00:00.000Z"
+ },
+ "link_status_clobbered": true,
+ "receipt_clobbered": false,
+ "self_contradictory": true,
+ "VERDICT": "CLAIM CONFIRMED — service clobbers"
+}
diff --git a/verification/tk11285/reverse-race.js b/verification/tk11285/reverse-race.js
new file mode 100644
index 0000000..6902236
--- /dev/null
+++ b/verification/tk11285/reverse-race.js
@@ -0,0 +1,52 @@
+'use strict';
+// Independent check of the red-team's claim: does the SERVICE clobber fields the
+// recover script owns? approve() mutates a row object captured BEFORE its awaits,
+// then checkpointApproval spreads that whole stale object over the fresh row.
+const fs = require('fs'), path = require('path'), { execFileSync } = require('child_process');
+const ROOT = path.join(__dirname, "..", "..");
+const trade = require(path.join(ROOT, 'lib', 'trade'));
+const APPS = trade.APPS_PATH;
+const read = () => fs.readFileSync(APPS, 'utf8').split('\n').filter(Boolean).map(l => JSON.parse(l));
+
+// A realistic failed-intake row: applyAndLink degraded gracefully, so it is unlinked.
+fs.writeFileSync(APPS, JSON.stringify({
+ id: 'TRADE-X', email: 'designer@example.com', business_name: 'Studio X',
+ status: 'pending', created_at: '2026-09-01T00:00:00.000Z',
+ shopify_customer_id: null, link_status: 'unlinked', link_error: 'shopify_create_failed',
+}) + '\n');
+
+let injected = false;
+// Interleave the recover script's OWN commitRows, from a separate process, during
+// the service's first await.
+global.__inject = () => {
+ if (injected) return; injected = true;
+ execFileSync(process.execPath, ['-e', `
+ const p=require(${JSON.stringify(path.join(ROOT, 'scripts', 'recover-stuck-apps.js'))});
+ const rows=p.readRows();
+ const r=rows.find(x=>x.id==='TRADE-X');
+ r.shopify_customer_id='777'; r.link_status='linked'; r.link_via='existing';
+ r.recovered_at='2026-09-10T00:00:00.000Z'; r.recovery_emailed=true;
+ p.commitRows([r]);
+ `], { cwd: ROOT, env: { ...process.env, DRY_RUN: '0' } });
+};
+
+(async () => {
+ const beforeApprove = read()[0];
+ const res = await trade.approve('TRADE-X');
+ const after = read()[0];
+ const out = {
+ claim: 'service (checkpointApproval) clobbers recover-owned fields with a pre-await row snapshot',
+ approve_ok: res.ok,
+ recover_wrote: { link_status: 'linked', recovery_emailed: true, recovered_at: '2026-09-10T00:00:00.000Z' },
+ final_row: {
+ status: after.status, shopify_customer_id: after.shopify_customer_id,
+ link_status: after.link_status, link_error: after.link_error,
+ recovery_emailed: after.recovery_emailed, recovered_at: after.recovered_at,
+ },
+ link_status_clobbered: after.link_status !== 'linked',
+ receipt_clobbered: after.recovery_emailed !== true,
+ self_contradictory: after.status === 'approved' && !!after.shopify_customer_id && after.link_status !== 'linked',
+ };
+ out.VERDICT = (out.link_status_clobbered || out.receipt_clobbered) ? 'CLAIM CONFIRMED — service clobbers' : 'CLAIM NOT REPRODUCED';
+ console.log(JSON.stringify(out, null, 2));
+})();
diff --git a/verification/tk11285/stubs-inject.js b/verification/tk11285/stubs-inject.js
new file mode 100644
index 0000000..25827d8
--- /dev/null
+++ b/verification/tk11285/stubs-inject.js
@@ -0,0 +1,38 @@
+'use strict';
+// Repro harness stub loader. Intercepts lib/email + lib/shopify so the REAL
+// production code paths run with ZERO external calls (no George send, no Shopify).
+// lib/trade.js and scripts/recover-stuck-apps.js are NOT stubbed - they are the
+// code under test and run verbatim.
+const Module = require('module');
+const path = require('path');
+const origLoad = Module._load;
+const SEND_MS = parseInt(process.env.REPRO_SEND_MS || '250', 10);
+const sleep = ms => new Promise(r => setTimeout(r, ms));
+
+const emailStub = {
+ // Simulates a REAL (non-dryRun) George send that takes time - this is the
+ // read->write window that the recover script holds its stale snapshot across.
+ async sendEmail() { await sleep(SEND_MS); return { ok: true, status: 200, dryRun: false }; },
+ designerAccountReadyEmail: () => ({ subject: 's', html: 'h' }),
+ tradeApplicationEmail: () => ({ subject: 's', html: 'h' }),
+ repNotifyEmail: () => ({ subject: 's', html: 'h' }),
+ tradeApprovedEmail: () => ({ subject: 's', html: 'h' }),
+ repAssignmentEmail: () => ({ subject: 's', html: 'h' }),
+ tradeRejectedEmail: () => ({ subject: 's', html: 'h' }),
+};
+const shopifyStub = {
+ async findCustomerByEmail() { if (global.__inject) global.__inject(); return "5551111"; },
+ async findOrCreateCustomer() { return { ok: true, id: '5551111', via: 'existing', created: false }; },
+ async addTags(id) { return { ok: true, status: 200, json: { customer: { id, tags: 'trade,trade_approved' } } }; },
+ async setCustomerMetafield(id, mf) { return { ok: true, status: 200, json: { metafield: { id: 'gid://shopify/Metafield/1', namespace: 'custom', key: 'assigned_rep', value: mf.value, type: mf.type, owner_id: id } } }; },
+ async getCustomer(id) { return { ok: true, status: 200, json: { customer: { id, tags: 'trade,trade_approved', metafields: [] } } }; },
+ async graphql() { return { ok: true, status: 200, json: { data: {} } }; },
+};
+Module._load = function (request, parent, isMain) {
+ const resolved = (() => { try { return Module._resolveFilename(request, parent, isMain); } catch { return ''; } })();
+ const base = path.basename(resolved);
+ const dir = path.basename(path.dirname(resolved));
+ if (dir === 'lib' && base === 'email.js') return emailStub;
+ if (dir === 'lib' && base === 'shopify.js') return shopifyStub;
+ return origLoad.apply(this, arguments);
+};
← 944582a TK-10836: contrarian overturns my Option D headline; correct
·
back to Dw Signup Fulfillment
·
TK-11285: correct my own overclaim in the f9de58c write-up 534d1f6 →