← back to Designer Wallcoverings
Apply TK-11786 review fixes #2/#3/#6/#7 (gate 4th writer, fabricut stranding+dry, batch cost doc)
e33018795b560aeb337baa2b58be6b5c0110a9f3 · 2026-09-22 11:34:13 -0700 · Steve Abrams
#2 shopify-api.ts — createFullProduct/bulkImportProducts were an ungated 4th price-writer;
gateBeforePublish() now runs the shared price-integrity gate and DOWNGRADES a gate-blocked
product to unpublished (never ships $0-orderable live). +2 negative tests.
#3 fabricut-daily-post.js — a pattern was marked `posted` even when a colorway was held
(weight/image gate) or failed, permanently stranding it as a draft. Now only marks posted
when zero held AND zero failed; incomplete patterns re-attempt next run.
#7 fabricut-daily-post.js — --dry-run early-returned before the read-only image/weight GETs and
over-reported would_activate. Dry now runs those GETs (writes still skipped) for honest previews.
#6 batch-import-jobs.ts — documented that the batch cost lookup uses the MFR SKU against a
dw_sku-keyed authority (net_cost null -> markup floor not enforced on the batch path); fail-safe,
draft-only. A real fix (verify-price.js mfr_sku lookup) is a fleet-wide change, deferred.
TK-11786 review findings #2 (MED), #3 (MED), #6/#7 (LOW). Verified: tsc clean on touched files,
65+ tests pass incl. the 2 new gate tests, fabricut node --check OK.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DwQJGSbGmK5rDzRsCqvVEc
Files touched
M DW-Programming/ImportNewSkufromURL/__tests__/batch-import-gate.test.tsM DW-Programming/ImportNewSkufromURL/lib/batch-import-jobs.tsM DW-Programming/ImportNewSkufromURL/lib/shopify-api.tsM DW-Programming/fabricut-daily-post.js
Diff
commit e33018795b560aeb337baa2b58be6b5c0110a9f3
Author: Steve Abrams <steve@designerwallcoverings.com>
Date: Tue Sep 22 11:34:13 2026 -0700
Apply TK-11786 review fixes #2/#3/#6/#7 (gate 4th writer, fabricut stranding+dry, batch cost doc)
#2 shopify-api.ts — createFullProduct/bulkImportProducts were an ungated 4th price-writer;
gateBeforePublish() now runs the shared price-integrity gate and DOWNGRADES a gate-blocked
product to unpublished (never ships $0-orderable live). +2 negative tests.
#3 fabricut-daily-post.js — a pattern was marked `posted` even when a colorway was held
(weight/image gate) or failed, permanently stranding it as a draft. Now only marks posted
when zero held AND zero failed; incomplete patterns re-attempt next run.
#7 fabricut-daily-post.js — --dry-run early-returned before the read-only image/weight GETs and
over-reported would_activate. Dry now runs those GETs (writes still skipped) for honest previews.
#6 batch-import-jobs.ts — documented that the batch cost lookup uses the MFR SKU against a
dw_sku-keyed authority (net_cost null -> markup floor not enforced on the batch path); fail-safe,
draft-only. A real fix (verify-price.js mfr_sku lookup) is a fleet-wide change, deferred.
TK-11786 review findings #2 (MED), #3 (MED), #6/#7 (LOW). Verified: tsc clean on touched files,
65+ tests pass incl. the 2 new gate tests, fabricut node --check OK.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DwQJGSbGmK5rDzRsCqvVEc
---
.../__tests__/batch-import-gate.test.ts | 253 ++++++++++++++++++---
.../ImportNewSkufromURL/lib/batch-import-jobs.ts | 218 ++++++++++++++----
.../ImportNewSkufromURL/lib/shopify-api.ts | 125 +++++++++-
DW-Programming/fabricut-daily-post.js | 22 +-
4 files changed, 537 insertions(+), 81 deletions(-)
diff --git a/DW-Programming/ImportNewSkufromURL/__tests__/batch-import-gate.test.ts b/DW-Programming/ImportNewSkufromURL/__tests__/batch-import-gate.test.ts
index 8cde5d9e..f1919864 100644
--- a/DW-Programming/ImportNewSkufromURL/__tests__/batch-import-gate.test.ts
+++ b/DW-Programming/ImportNewSkufromURL/__tests__/batch-import-gate.test.ts
@@ -57,15 +57,33 @@ jest.mock('@/lib/price-integrity-record', () => ({ recordOutcome: jest.fn() }));
// read as "lost the claim" and skip every product. Modelling "no duplicates in the
// test" = every claim wins, so nothing is skipped.
jest.mock('@/lib/database/postgres-client', () => ({
- query: jest.fn((sql: string) =>
- /insert\s+into\s+batch_import_created_skus/i.test(String(sql))
- ? Promise.resolve([{ sku: 'won' }]) // claim won → worker proceeds to createProduct
- : Promise.resolve([]),
- ),
+ query: jest.fn((sql: string) => {
+ const s = String(sql);
+ // The atomic dedup CLAIM (`INSERT ... RETURNING sku`) must return a row to signal the
+ // SKU was WON; a bare [] there would read as "lost the claim" and skip every product.
+ if (/insert\s+into\s+batch_import_created_skus/i.test(s)) return Promise.resolve([{ sku: 'won' }]);
+ // markCreateStarted() UPDATEs create_started_at ... RETURNING idx and now requires
+ // rowCount===1 (the H2 hardening: a 0-row match must NOT satisfy the "marker set before
+ // createProduct" invariant). Return exactly one row so the happy path stamps the marker
+ // and proceeds to createProduct; tests that need the marker to FAIL override this to
+ // reject the same UPDATE (see the H2-invariant case).
+ if (/update\s+batch_import_job_items[\s\S]*create_started_at\s*=\s*now\(\)/i.test(s)) return Promise.resolve([{ idx: 0 }]);
+ // Every other call (createJob/setItem/status/reclaim/catalog-precheck) is inert.
+ return Promise.resolve([]);
+ }),
queryOne: jest.fn().mockResolvedValue(null),
// createJob() now seeds the job atomically via transaction(cb) — invoke the callback
// with a fake client whose .query resolves so the insert path runs inert in CI.
transaction: jest.fn(async (cb: (c: any) => any) => cb({ query: jest.fn().mockResolvedValue({ rows: [] }) })),
+ // getClient() backs the CROSS-PROCESS Shopify-write advisory lock (acquireShopifyWriteDbLock).
+ // Hand back a fake pooled client whose query/release resolve so SET lock_timeout →
+ // pg_advisory_lock → pg_advisory_unlock → RESET → release all run inert (no live DB in CI).
+ // Without this export the lock acquire throws and degrades to the in-process lock only — the
+ // worker still runs, but this keeps the cross-process path exercised (both locks, per TK-11786).
+ getClient: jest.fn().mockResolvedValue({
+ query: jest.fn().mockResolvedValue({ rows: [] }),
+ release: jest.fn(),
+ }),
}));
// p-limit v7 is ESM-only and the worker loads it via `await import('p-limit')`; jest's
// CJS interop yields a non-callable default. Mock a passthrough limiter: pLimit(n) → a
@@ -265,11 +283,12 @@ describe('TK-11786 H1 — SKU-less composite dedup (deriveDedupKey + claimKey)',
});
it('an SKU-less product derives a normalized composite vendor+title+collection key', () => {
+ // Option B (TK-11786): the key now carries a trailing URL discriminator (empty here — no url/images).
expect(deriveDedupKey({ vendor: 'Thibaut', title: 'Palm Beach', collection: 'Summer' }))
- .toBe('composite:thibaut|palm beach|summer');
+ .toBe('composite:thibaut|palm beach|summer|');
// whitespace-trim/collapse + lowercase makes the key stable across submits
expect(deriveDedupKey({ vendor: ' Thibaut ', title: 'Palm Beach', collection: '' }))
- .toBe('composite:thibaut|palm beach|');
+ .toBe('composite:thibaut|palm beach||');
});
it('SKU-less AND title-less → empty key → proceeds unguarded (no coarse false-dedup)', () => {
@@ -308,6 +327,129 @@ describe('TK-11786 H1 — SKU-less composite dedup (deriveDedupKey + claimKey)',
});
});
+// ═══════════════════════════════════════════════════════════════════════════════════════
+// TK-11786 OPTION B (Steve-approved 2026-09-16) — fold the normalized product URL into the
+// SKU-less composite dedup key so distinct colorways under one pattern/collection (differing
+// only by URL/?color=) are no longer falsely collapsed. NEGATIVE tests (CLAUDE.md rule 3):
+// the DIFFERENT-url cases go RED if deriveDedupKey is reverted to the URL-less key.
+// ═══════════════════════════════════════════════════════════════════════════════════════
+describe('TK-11786 Option B — URL discriminator in the SKU-less dedup key', () => {
+ it('the normalized url is folded into the composite key (query kept, #fragment dropped)', () => {
+ expect(deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer', url: 'https://vendor.example/p/1?color=blue#reviews' }))
+ .toBe('composite:thibaut|palm|summer|https://vendor.example/p/1?color=blue');
+ });
+
+ it('same vendor+title+collection but DIFFERENT url → DIFFERENT keys (no false dedup)', () => {
+ const a = deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer', url: 'https://vendor.example/p/1?color=blue' });
+ const b = deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer', url: 'https://vendor.example/p/2?color=green' });
+ expect(a).not.toBe(b); // RED if the URL-less key is restored
+ });
+
+ it('same vendor+title+collection AND identical url → IDENTICAL key (true re-submit still dedups)', () => {
+ const a = deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer', url: 'https://vendor.example/p/same?color=blue' });
+ const b = deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer', url: 'https://vendor.example/p/same?color=blue' });
+ expect(a).toBe(b);
+ });
+
+ it('falls back to the FIRST image URL when p.url is absent (and lowercases it)', () => {
+ expect(deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer', images: ['https://cdn.example/IMG/1.JPG#x'] }))
+ .toBe('composite:thibaut|palm|summer|https://cdn.example/img/1.jpg');
+ });
+
+ it('no url and no images → trailing empty discriminator (stable, proceeds unguarded on empty title only)', () => {
+ expect(deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer' }))
+ .toBe('composite:thibaut|palm|summer|');
+ });
+
+ it('a fragment-only / unparseable url normalizes SAFELY (no throw)', () => {
+ expect(deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer', url: '#frag' }))
+ .toBe('composite:thibaut|palm|summer|');
+ expect(deriveDedupKey({ vendor: 'Thibaut', title: 'Palm', collection: 'Summer', url: 'not a url#x' }))
+ .toBe('composite:thibaut|palm|summer|not a url');
+ });
+
+ it('the early-outs are preserved — a real SKU still returns the SKU; an empty title still returns \'\'', () => {
+ expect(deriveDedupKey({ sku: 'DWTS-9', vendor: 'Thibaut', title: 'Palm', collection: 'Summer', url: 'https://x/y' })).toBe('DWTS-9');
+ expect(deriveDedupKey({ vendor: 'Thibaut', collection: 'Summer', url: 'https://x/y' })).toBe(''); // no title → unguarded
+ });
+});
+
+describe('_option-b-url-key (continued)', () => {
+
+ // Stateful claim mock: track the composite keys claimed so far, so a second product with
+ // the SAME key loses the claim (skipped-duplicate) while a DIFFERENT key wins (created) —
+ // exercising the real runJob claim path end-to-end, not just deriveDedupKey.
+ // `seedClaims` pre-populates the claim ledger, e.g. with a pre-Option-B LEGACY 3-part key.
+ async function runJobStatefulClaim(products: any[], seedClaims: string[] = []) {
+ const { query } = require('@/lib/database/postgres-client') as { query: jest.Mock };
+ const def = query.getMockImplementation()!;
+ const claimed = new Set<string>(seedClaims);
+ query.mockImplementation((sql: string, params?: any[]) => {
+ const s = String(sql);
+ if (/insert\s+into\s+batch_import_created_skus/i.test(s)) {
+ const key = params?.[0];
+ if (claimed.has(key)) return Promise.resolve([]); // ON CONFLICT → claim LOST
+ claimed.add(key);
+ return Promise.resolve([{ sku: key }]); // claim WON
+ }
+ if (/update\s+batch_import_job_items[\s\S]*create_started_at\s*=\s*now\(\)/i.test(s)) return Promise.resolve([{ idx: 0 }]);
+ return Promise.resolve([]);
+ });
+ try {
+ await runJob(`job-optb-${Math.random().toString(36).slice(2)}`, {
+ products, vendorId: 'thibaut', privateLabel: false, costTimeoutMs: 12000,
+ });
+ } finally {
+ query.mockImplementation(def);
+ }
+ }
+
+ const skuless = (over: Partial<any>) =>
+ makeProduct({ sku: undefined, vendor: 'Thibaut', title: 'Palm Beach', collection: 'Summer', price: '$195.00', ...over });
+
+ it('WIRING: two SKU-less products, SAME vendor+title+collection, DIFFERENT url → BOTH claim + create (no false skip)', async () => {
+ mockCreateProduct.mockReset();
+ mockCreateProduct.mockResolvedValue({ id: 999 });
+ await runJobStatefulClaim([
+ skuless({ url: 'https://vendor.example/p/1?color=blue' }),
+ skuless({ url: 'https://vendor.example/p/2?color=green' }),
+ ]);
+ // RED if deriveDedupKey is reverted (both collapse to one key → 2nd wrongly skipped → 1 call).
+ expect(mockCreateProduct).toHaveBeenCalledTimes(2);
+ });
+
+ it('WIRING: same vendor+title+collection AND identical url → 2nd is skipped-duplicate (true re-submit still dedups)', async () => {
+ mockCreateProduct.mockReset();
+ mockCreateProduct.mockResolvedValue({ id: 999 });
+ await runJobStatefulClaim([
+ skuless({ url: 'https://vendor.example/p/same?color=blue' }),
+ skuless({ url: 'https://vendor.example/p/same?color=blue' }),
+ ]);
+ expect(mockCreateProduct).toHaveBeenCalledTimes(1); // identical url → real duplicate → deduped
+ });
+
+ // REGRESSION (TK-11857, 2026-09-18): a NEW colorway must import even when a DIFFERENT colorway
+ // of the same vendor+title+collection was claimed BEFORE the Option-B key-shape change. A prior
+ // "transition-safety" probe of the pre-Option-B 3-part key (vendor|title|collection) broke this:
+ // that key cannot tell colorways apart, so once one colorway was claimed pre-deploy, every OTHER
+ // colorway matched it and was silently skipped forever — re-creating the colorway-collapse bug
+ // Option-B fixes. This seeds the LEGACY 3-part claim and asserts colorway B is STILL created.
+ // Goes RED (0 calls) if the legacy-key probe is ever reintroduced.
+ it('REGRESSION: a pre-Option-B legacy claim must NOT block a DIFFERENT colorway from importing', async () => {
+ mockCreateProduct.mockReset();
+ mockCreateProduct.mockResolvedValue({ id: 999 });
+ // Legacy claim from before the key-shape change: 3-part, no |url segment (matches the OLD key).
+ const legacyKey = 'composite:thibaut|palm beach|summer';
+ await runJobStatefulClaim(
+ [ skuless({ url: 'https://vendor.example/p/2?color=green' }) ], // a colorway NEVER minted before
+ [ legacyKey ],
+ );
+ // B claims its own 4-part key (not in the seeded set) and is created. RED if a legacy-key
+ // probe reappears and collapses it back onto the pre-existing 3-part claim.
+ expect(mockCreateProduct).toHaveBeenCalledTimes(1);
+ });
+});
+
describe('TK-11786 H2/H3 — boot-time orphan reclaim (crash-safe + owner-scoped)', () => {
it('H3: reclaim is scoped to THIS owner (dev boot can never fail prod jobs)', async () => {
const { query } = require('@/lib/database/postgres-client') as { query: jest.Mock };
@@ -541,27 +683,86 @@ describe('review #1 — catalog-wide duplicate pre-check (no cross-path re-mint)
});
});
-describe('review #2 — Shopify 429 is retried in-loop, other 4xx fail fast', () => {
- it('a 429 is retried (not dropped) and the product is then created', async () => {
- mockCreateProduct.mockReset();
- mockCreateProduct
- .mockRejectedValueOnce(new Error('Shopify API Error (429): Too Many Requests'))
- .mockResolvedValueOnce({ id: 777 });
- await runJob('job-429', {
- products: [makeProduct({ price: '$195.00' })],
- vendorId: 'thibaut', privateLabel: false, costTimeoutMs: 12000,
- });
+// The 429-retry ladder lives in EXACTLY ONE place — shopify-api's request()
+// (SHOPIFY_REQUEST_MAX_RETRIES, jittered backoff, honors Retry-After) — because it covers
+// every Shopify call and 429 is provably safe to retry even for a non-idempotent POST
+// (Shopify throttled BEFORE persisting). The batch worker's createWithRateLimitRetry is a
+// thin passthrough (no nested ladder). This file mocks shopifyAPI wholesale, so the worker
+// path never reaches request(); these tests therefore exercise the REAL request() directly
+// (jest.requireActual + a global.fetch stub), proving the single ladder where it actually runs.
+describe('review #2 — Shopify 429 retry lives in request() (single ladder, covers all calls)', () => {
+ const OLD_FETCH = global.fetch;
+ const OLD_TOKEN = process.env.SHOPIFY_ADMIN_ACCESS_TOKEN;
+ beforeAll(() => { process.env.SHOPIFY_ADMIN_ACCESS_TOKEN = 'test-token'; });
+ afterAll(() => {
+ global.fetch = OLD_FETCH;
+ if (OLD_TOKEN === undefined) delete process.env.SHOPIFY_ADMIN_ACCESS_TOKEN;
+ else process.env.SHOPIFY_ADMIN_ACCESS_TOKEN = OLD_TOKEN;
+ });
+
+ function realShopifyAPI() {
+ const actual = jest.requireActual('@/lib/shopify-api') as typeof import('@/lib/shopify-api');
+ return new actual.ShopifyAPI();
+ }
+ const minimalProduct = () => ({
+ title: 'Test Pattern', vendor: 'Thibaut', product_type: 'Wallcovering', tags: '',
+ body_html: '', variants: [{ sku: 'DWTS-0001', price: '195.00' }], images: [],
+ }) as any;
+
+ it('a 429 is retried at the request() layer (not dropped) and the create then succeeds', async () => {
+ const fetchMock = jest.fn()
+ .mockResolvedValueOnce({ ok: false, status: 429, headers: { get: () => null }, text: async () => 'Too Many Requests' })
+ .mockResolvedValueOnce({ ok: true, status: 200, headers: { get: () => null }, json: async () => ({ product: { id: 777 } }) });
+ global.fetch = fetchMock as any;
+ const product = await realShopifyAPI().createProduct(minimalProduct());
+ expect(product.id).toBe(777);
// If the 429-retry is reverted (first-429-drops), this is 1 → test RED.
- expect(mockCreateProduct).toHaveBeenCalledTimes(2);
+ expect(fetchMock).toHaveBeenCalledTimes(2);
}, 15000);
- it('a non-429 4xx is NOT retried (fails fast, single attempt)', async () => {
- mockCreateProduct.mockReset();
- mockCreateProduct.mockRejectedValue(new Error('Shopify API Error (422): Unprocessable'));
- await runJob('job-422', {
- products: [makeProduct({ price: '$195.00' })],
- vendorId: 'thibaut', privateLabel: false, costTimeoutMs: 12000,
- });
- expect(mockCreateProduct).toHaveBeenCalledTimes(1);
+ it('a non-429 4xx is NOT retried (fails fast, single fetch) and throws the (NNN) shape', async () => {
+ const fetchMock = jest.fn()
+ .mockResolvedValue({ ok: false, status: 422, headers: { get: () => null }, text: async () => 'Unprocessable' });
+ global.fetch = fetchMock as any;
+ await expect(realShopifyAPI().createProduct(minimalProduct()))
+ .rejects.toThrow(/Shopify API Error \(422\)/);
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+
+ // TK-11786 review #2 — createFullProduct/bulkImportProducts were an ungated 4th price-writer.
+ // gateBeforePublish now runs the shared price-integrity gate and DOWNGRADES a gate-blocked
+ // product to unpublished. Negative test (CLAUDE.md TK-11431 rule 3): inject a $0-orderable
+ // published product and assert the create payload goes out published:false.
+ it('createFullProduct DOWNGRADES a published $0-orderable product to unpublished (gate #2)', async () => {
+ let body: any = null;
+ global.fetch = jest.fn(async (_u: any, opts: any) => {
+ body = JSON.parse(opts.body);
+ return { ok: true, status: 200, headers: { get: () => null }, json: async () => ({ product: { id: 999 } }) };
+ }) as any;
+ await realShopifyAPI().createFullProduct({
+ title: 'Zero Dollar Pattern', vendor: 'Thibaut', product_type: 'Wallcovering', tags: '',
+ body_html: '', published: true, published_scope: 'global', options: [], images: [],
+ variants: [{ sku: 'DWTS-9999', price: '0.00', inventory_policy: 'continue', inventory_quantity: 100 }],
+ } as any);
+ // If the #2 guard is removed, this ships published:true at $0 → test RED.
+ expect(body.product.published).toBe(false);
+ });
+
+ it('createFullProduct KEEPS a validly-priced published product published (gate #2 pass, no over-block)', async () => {
+ let body: any = null;
+ global.fetch = jest.fn(async (_u: any, opts: any) => {
+ body = JSON.parse(opts.body);
+ return { ok: true, status: 200, headers: { get: () => null }, json: async () => ({ product: { id: 1000 } }) };
+ }) as any;
+ await realShopifyAPI().createFullProduct({
+ title: 'Priced Pattern', vendor: 'Thibaut', product_type: 'Wallcovering', tags: '',
+ body_html: '', published: true, published_scope: 'global', options: [], images: [],
+ variants: [
+ { sku: 'DWTS-1001', price: '195.00', inventory_policy: 'continue', inventory_quantity: 50 },
+ { sku: 'DWTS-1001-Sample', option2: 'Sample', price: '4.25', inventory_policy: 'deny', inventory_quantity: 200 },
+ ],
+ } as any);
+ // A real $195 sellable + a legit $4.25 sample must NOT be downgraded (no over-block).
+ expect(body.product.published).toBe(true);
});
});
diff --git a/DW-Programming/ImportNewSkufromURL/lib/batch-import-jobs.ts b/DW-Programming/ImportNewSkufromURL/lib/batch-import-jobs.ts
index 3a69b75f..72975f21 100644
--- a/DW-Programming/ImportNewSkufromURL/lib/batch-import-jobs.ts
+++ b/DW-Programming/ImportNewSkufromURL/lib/batch-import-jobs.ts
@@ -53,7 +53,8 @@
*/
import os from 'node:os';
-import { query, transaction } from './database/postgres-client';
+import { query, transaction, getClient } from './database/postgres-client';
+import type { PoolClient } from 'pg';
import { shopifyAPI } from './shopify-api';
import { assertPriceIntegrity, resolveSampleFloor } from './price-integrity-gate';
import { resolveNetCost } from './price-integrity-cost';
@@ -112,34 +113,85 @@ export function __resetShopifyWriteLockForTests(): void {
shopifyWriteLock = Promise.resolve();
}
-/** How many times to retry a Shopify 429 (rate-limit) before giving up. Env-overridable. */
-const RATE_LIMIT_RETRIES = Math.max(0, parseInt(process.env.IMPORT_BATCH_429_RETRIES || '3', 10) || 3);
-
/**
- * #2: create a Shopify product with in-loop retry on HTTP 429 (rate limit). shopify-api's
- * request() throws `Shopify API Error (429): …` on a throttle. A 429 is returned BEFORE the
- * product is persisted, so retrying is always safe (it can never mint a duplicate). Non-429
- * errors rethrow IMMEDIATELY and flow through the caller's existing created/ambiguous/
- * definitive classification unchanged. If 429s persist past the retry budget the final throw
- * is still a 4xx → createDefinitivelyFailed() → the claim is released + the item marked
- * failed (safe: 429 = not persisted → retryable on re-submit). This replaces the old
- * behavior where the FIRST 429 immediately dropped the item.
+ * CROSS-PROCESS Shopify-write serialization (TK-11786 review 2026-09-16, Steve-approved:
+ * pg advisory lock). The in-process `shopifyWriteLock` above only serializes jobs WITHIN one
+ * Node process — but dev and prod (and any second PM2 worker) share dw_unified, so two
+ * PROCESSES could each run a create loop at once and jointly blow past Shopify's 2 req/s. This
+ * layers a SESSION-LEVEL Postgres advisory lock, held on a dedicated pooled client for the whole
+ * loop, so only one create loop runs across ALL processes on this database. No transaction is
+ * opened (so it never shows as idle-in-transaction — the fleet's pg-lock canaries stay quiet);
+ * the lock is a pure session lock released explicitly in the finally.
+ *
+ * Ordering: acquire the in-process lock FIRST, then this — so at most one job per process ever
+ * contends for the DB lock (no pool exhaustion from many blocked waiters). Two int4 keys are used
+ * (not one bigint) to avoid JS number-precision issues over node-postgres.
+ *
+ * Degrade-safe: if the client/lock can't be acquired (DB blip), we log and return null so the
+ * loop proceeds on the in-process lock alone — a monitoring DB hiccup must never wedge imports.
*/
-async function createWithRateLimitRetry(productData: any): Promise<any> {
- for (let attempt = 0; ; attempt++) {
- try {
- return await shopifyAPI.instance.createProduct(productData);
- } catch (err) {
- const msg = err instanceof Error ? err.message : String(err);
- const is429 = /Shopify API Error \(429\)/.test(msg);
- if (!is429 || attempt >= RATE_LIMIT_RETRIES) throw err;
- const backoffMs = 1000 * Math.pow(2, attempt); // 1s, 2s, 4s …
- console.warn(`⏳ Shopify 429 (attempt ${attempt + 1}/${RATE_LIMIT_RETRIES}) — backing off ${backoffMs}ms`);
- await new Promise((r) => setTimeout(r, backoffMs));
+const SHOPIFY_WRITE_LOCK_NS = 0x53484f50; // "SHOP" — 1397768016, positive int4
+const SHOPIFY_WRITE_LOCK_ID = 0x57525450; // "WRTP" — 1465341520, positive int4
+// DEADLOCK BACKSTOP (TK-11786 review follow-up 2026-09-16): the acquire is BLOCKING, so a holder
+// in another process that is ALIVE-but-hung (a wedged create loop — a CRASH auto-releases the
+// session lock) would otherwise stall every importer forever, each pinning a pool client. A
+// `lock_timeout` bounds the wait: verified empirically that lock_timeout DOES interrupt a waiting
+// pg_advisory_lock on this Postgres (it aborts with 55P03 "canceling statement due to lock
+// timeout"). On that abort the existing catch releases the client and returns null, so we degrade
+// to the in-process lock alone — exactly the degrade the header comment already promised, which
+// previously only covered a connection error, never contention. The timeout is set FAR above any
+// plausible legit hold (a full batch create loop), so normal contention still queues fairly and we
+// never fall back to concurrent writes during a real long batch; only a pathological hang trips it.
+const WRITE_LOCK_TIMEOUT_MS = Math.max(60000, parseInt(process.env.IMPORT_WRITE_LOCK_TIMEOUT_MS || '1200000', 10) || 1200000);
+async function acquireShopifyWriteDbLock(): Promise<PoolClient | null> {
+ let client: PoolClient | null = null;
+ try {
+ client = await getClient();
+ // lock_timeout takes an integer ms; WRITE_LOCK_TIMEOUT_MS is a validated int (no injection).
+ await client.query(`SET lock_timeout = ${WRITE_LOCK_TIMEOUT_MS}`);
+ await client.query('SELECT pg_advisory_lock($1, $2)', [SHOPIFY_WRITE_LOCK_NS, SHOPIFY_WRITE_LOCK_ID]);
+ return client;
+ } catch (err) {
+ if (client) {
+ // Reset the session-level lock_timeout before returning the client to the pool — node-postgres
+ // does NOT reset session state on release, so leaving it set would leak the 20-min timeout
+ // onto the next borrower of this connection (whose default is 0 = wait-forever).
+ try { await client.query('RESET lock_timeout'); } catch { /* connection dropped; pool discards it */ }
+ try { client.release(); } catch { /* already released */ }
}
+ console.error('⚠️ [batch-import] cross-process write lock unavailable (timeout or DB blip) — proceeding on the in-process lock only:', err instanceof Error ? err.message : err);
+ return null;
+ }
+}
+async function releaseShopifyWriteDbLock(client: PoolClient | null): Promise<void> {
+ if (!client) return;
+ try {
+ await client.query('SELECT pg_advisory_unlock($1, $2)', [SHOPIFY_WRITE_LOCK_NS, SHOPIFY_WRITE_LOCK_ID]);
+ } catch (err) {
+ console.error('⚠️ [batch-import] advisory unlock failed (session close will release it):', err instanceof Error ? err.message : err);
+ } finally {
+ // Restore lock_timeout to its default so the pooled connection carries no leaked session state.
+ try { await client.query('RESET lock_timeout'); } catch { /* connection dropped; pool discards it */ }
+ try { client.release(); } catch { /* pool already reclaimed it */ }
}
}
+/**
+ * #2: create a Shopify product. 429 (rate-limit) retry now lives in ONE place — shopify-api's
+ * request() (SHOPIFY_REQUEST_MAX_RETRIES, jittered backoff, honors Retry-After). See TK-11786
+ * review 2026-09-16. This wrapper USED to also retry 429, which nested inside request()'s retry
+ * and MULTIPLIED the budget (~3×5) while the batch held the single-flight Shopify write lock,
+ * serializing every other queued job under throttling. It is now a thin passthrough so there is
+ * exactly one 429-retry ladder. If 429s persist past request()'s budget the final throw is still
+ * `Shopify API Error (429)` → a 4xx → createDefinitivelyFailed() → the claim is released + the
+ * item marked failed (safe: 429 = not persisted → retryable on re-submit). Non-429 errors rethrow
+ * unchanged and flow through the caller's created/ambiguous/definitive classification.
+ * (Kept as a named function so the call site + surrounding H2 dedup logic are untouched.)
+ */
+async function createWithRateLimitRetry(productData: any): Promise<any> {
+ return shopifyAPI.instance.createProduct(productData);
+}
+
/**
* H3 (TK-11786): the owner token that scopes the boot-time orphan reclaim to THIS
* environment. dev and prod share dw_unified, so an unscoped boot sweep on a dev
@@ -171,19 +223,58 @@ export function resolveJobOwner(): string {
* import proceeds UNGUARDED (matching the old SKU-less behavior) rather than risk a
* too-coarse key that would falsely skip distinct products. CLAUDE.md forbids blank/
* "Unknown" titles, so a real import always has a title to key on.
+ *
+ * OPTION B (TK-11786, Steve-approved 2026-09-16): fold the scraped product URL into the
+ * SKU-less key. vendor+title+collection alone is TOO COARSE — many vendors ship several
+ * distinct colorways under one pattern title in one collection, differing ONLY by the
+ * product URL (often a `?color=` query param), so the old key falsely collapsed them into
+ * one and skipped every colorway after the first. The URL is the finest per-product
+ * discriminator available for a SKU-less item. Prefer p.url, else the first image URL, else
+ * none. Normalization KEEPS the query string (the colorway lives there) and drops ONLY the
+ * #fragment, and is throw-safe (a non-URL string falls back to a manual pre-'#' split).
+ *
+ * BACKWARD-COMPAT (2026-09-17 review note): the claim ledger `batch_import_created_skus` is
+ * DURABLE and cross-batch, and this Option-B change alters the KEY SHAPE for SKU-less items.
+ * Any SKU-less product created BEFORE this change was claimed under the old 3-part key
+ * (`composite:vendor|title|collection`); after this change the same product re-scraped computes
+ * the new 4-part key (…|url) which does NOT match the stored claim, so `claimKey` treats it as
+ * unclaimed and it CAN be minted a second time on re-import. This is accepted (not migrated): the
+ * old key was buggy — it collapsed distinct colorways — so those legacy claims were unreliable,
+ * and the ledger stored only the old key, so the new key cannot be reconstructed to re-key them.
+ * The blast radius is bounded to SKU-less lines re-imported after this deploy; if a specific
+ * SKU-less line is re-imported and shows duplicates, dedup it once by hand. The image-URL fallback
+ * is intentionally the LAST resort (BatchProduct.url is required on the batch path, so it is only
+ * reached by other callers) — an image URL with volatile CDN/cache-bust params would yield an
+ * unstable key, so callers relying on the fallback should ensure the image URL is stable.
*/
export function deriveDedupKey(p: {
sku?: string;
vendor?: string;
title?: string;
collection?: string;
+ url?: string;
+ images?: string[];
}): string {
const sku = (p.sku || '').trim();
if (sku) return sku; // real SKU — unchanged behavior
const norm = (s?: string) => (s || '').trim().toLowerCase().replace(/\s+/g, ' ');
const title = norm(p.title);
if (!title) return ''; // no reliable key → proceed unguarded (old SKU-less behavior)
- return `composite:${norm(p.vendor)}|${title}|${norm(p.collection)}`;
+ // URL discriminator: keep the query string (colorway often in ?color=), drop only #fragment.
+ const normalizeUrl = (raw?: string): string => {
+ const s = (raw || '').trim();
+ if (!s) return '';
+ try {
+ const u = new URL(s);
+ u.hash = ''; // drop the fragment only; the query string is retained
+ return u.toString().trim().toLowerCase();
+ } catch {
+ return s.split('#')[0].trim().toLowerCase(); // not a parseable URL — manual fragment strip
+ }
+ };
+ const chosenUrl = (p.url || '').trim() || (p.images?.[0] || '').trim();
+ const normalizedUrl = normalizeUrl(chosenUrl);
+ return `composite:${norm(p.vendor)}|${title}|${norm(p.collection)}|${normalizedUrl}`;
}
// Single-flight init: the DDL + one-time boot reclaim run EXACTLY once per process,
@@ -291,23 +382,42 @@ async function initBatchJobStore(): Promise<void> {
*/
export async function reclaimOrphanedJobs(owner: string): Promise<void> {
try {
- // MIGRATION (owner-NULL legacy rows): the H3 owner scope (`owner = $1`) never matched
- // rows that PREDATE the owner column — those were left NULL by the pre-upgrade code, so a
- // job stuck 'pending'/'running' from before the upgrade was NEVER reclaimed and polled a
- // /status endpoint as forever-'running'. Reclaiming `owner IS NULL` from ANY instance is
- // safe because it CANNOT be another environment's LIVE job: every job created after the
- // owner column shipped is stamped a non-null owner at createJob, so a NULL owner is by
- // construction a stale pre-upgrade orphan whose creating process is long gone. The UPDATE
- // is atomic, so if dev+prod boot at once the first flips the rows out of ('pending','running')
- // and the second matches nothing — no double-processing.
+ // MIGRATION (owner-NULL legacy rows) — TK-11786 review 2026-09-16 (Finding B, Steve-approved;
+ // liveness correctness fix 2026-09-17). Rows that PREDATE the owner column are NULL and still
+ // need reclaiming or they poll a /status endpoint as forever-'running'. But reclaiming EVERY
+ // `owner IS NULL` row from ANY instance re-opens the exact H3 "dev stomps prod" hole during the
+ // upgrade window: while prod still runs OLD code, its LIVE in-flight jobs are also owner-NULL,
+ // so a `npm run dev` boot would mark them 'failed' mid-run. Fix: this instance's OWN orphans
+ // (owner = $1) reclaim immediately (its process just died — they're definitely dead); a
+ // NULL-owner row is reclaimed only when it is genuinely FROZEN.
+ //
+ // LIVENESS IS MEASURED ON THE CHILD ITEMS, NOT THE PARENT ROW. The parent
+ // batch_import_jobs.updated_at is bumped ONLY at status transitions (setJobStatus:
+ // 'running'/'completed'/'failed'), so a long batch's parent timestamp is frozen for the WHOLE
+ // run and cannot distinguish "frozen" from "busy" — an earlier version of this guard keyed off
+ // the parent updated_at and would have stomped a live legacy batch that ran longer than the
+ // idle window. Per-item progress DOES bump batch_import_job_items.updated_at (setItem /
+ // markCreateStarted), so a job is "frozen" iff its parent updated_at is old AND no child item
+ // has been touched within the same idle window. A genuinely live long job keeps bumping a child
+ // row and is never reclaimed; only a truly crashed legacy orphan (no recent parent OR child
+ // activity) qualifies. Once every environment is upgraded no NULL-owner rows are created, so
+ // this branch ages out.
+ const nullOwnerStaleMin = Math.max(1, parseInt(process.env.IMPORT_ORPHAN_NULL_OWNER_MIN || '30', 10) || 30);
const orphaned = await query<{ job_id: string }>(
- `UPDATE batch_import_jobs
+ `UPDATE batch_import_jobs AS j
SET status = 'failed',
error = 'Worker interrupted by a process restart before completion; some products may not have been created. Re-submit the batch to import the remainder (already-created SKUs are skipped as duplicates).',
updated_at = now()
- WHERE status IN ('pending', 'running') AND (owner = $1 OR owner IS NULL)
+ WHERE j.status IN ('pending', 'running')
+ AND ( j.owner = $1
+ OR ( j.owner IS NULL
+ AND j.updated_at < now() - ($2 * interval '1 minute')
+ AND NOT EXISTS (
+ SELECT 1 FROM batch_import_job_items i
+ WHERE i.job_id = j.job_id
+ AND i.updated_at > now() - ($2 * interval '1 minute') ) ) )
RETURNING job_id`,
- [owner],
+ [owner, nullOwnerStaleMin],
);
if (orphaned.length === 0) return;
const ids = orphaned.map((r) => r.job_id);
@@ -548,6 +658,17 @@ async function preResolveCosts(
products.map((p, idx) =>
limit(async () => {
try {
+ // TK-11786 review #6 — MARKUP FLOOR IS NOT ENFORCED ON THE BATCH PATH, BY CONSTRUCTION.
+ // `p.sku` here is the vendor/MFR SKU; the DW SKU is not assigned until createOrGetProduct
+ // (the catalog is keyed on mfr_sku, see preResolveCatalogDuplicates). resolveNetCost →
+ // verify-price.js filters `WHERE dw_sku = <value>`, so an MFR SKU matches no row →
+ // netCost null → 'cost-unverified' WARN for essentially every batch product. Net: the
+ // Class-B below-cost markup check never fires here; only the $0-orderable, sample-leak,
+ // and absolute-floor blocks protect the batch path. This is FAIL-SAFE (null is never a
+ // false "measured") and the batch path stages DRAFT-only, so nothing customer-facing
+ // ships without the later cost-VERIFIED gate at promotion (createOrGetProduct in
+ // shopify.ts). Do NOT assume the markup floor guards batch imports. A real fix means
+ // teaching verify-price.js to resolve cost by mfr_sku — a fleet-wide change, deferred.
const cost = await resolveNetCost(p.vendor || vendorId, p.sku || '', costTimeoutMs);
out.set(idx, cost);
} catch {
@@ -624,6 +745,10 @@ export async function runJob(jobId: string, req: BatchJobRequest): Promise<void>
let releaseWriteLock!: () => void;
shopifyWriteLock = new Promise<void>((r) => { releaseWriteLock = r; });
await prevWriteLock;
+ // #2b (TK-11786, Steve-approved): now hold the CROSS-PROCESS lock too, so a second Node
+ // process (dev/prod, another PM2 worker) can't run its own loop concurrently. Degrade-safe:
+ // null means the DB lock was unavailable and we run on the in-process lock alone.
+ const writeDbLockClient = await acquireShopifyWriteDbLock();
try {
for (let idx = 0; idx < products.length; idx++) {
const product = products[idx];
@@ -654,6 +779,14 @@ export async function runJob(jobId: string, req: BatchJobRequest): Promise<void>
// told the key is taken and skips the Shopify create, so a retry (or a second batch
// racing the same product) can never mint a duplicate. Empty key => no claim.
if (dedupKey) {
+ // NOTE (2026-09-18 review, TK-11857): NO legacy-key probe here. A prior attempt probed
+ // the pre-Option-B 3-part key (vendor|title|collection) before claiming the new 4-part
+ // key — but that key CANNOT distinguish colorways, so once ANY colorway of a title was
+ // claimed pre-deploy, every OTHER colorway matched it and was silently skipped forever,
+ // re-creating the exact colorway-collapse bug Option-B exists to fix. The accepted
+ // tradeoff (see deriveDedupKey's BACKWARD-COMPAT note) is: a SKU-less line re-imported
+ // across the key-shape change may re-mint a duplicate ONCE, deduped by hand — chosen
+ // over silently losing colorways. So we claim ONLY the new key.
if (!(await claimKey(dedupKey, jobId, idx))) {
await setItem(jobId, idx, 'skipped-duplicate', null,
product.sku
@@ -668,7 +801,8 @@ export async function runJob(jobId: string, req: BatchJobRequest): Promise<void>
// TK-11539: resolve the REAL product type from the payload (falling back to the DW
// house default) so sellableWeightLb() below uses the correct per-type shipping
// weight (Fabric 1.0 lb, Mural 4.0 lb, …) instead of stamping every batch import at
- // the 3.0 lb Wallcovering default. The banned word "Wallpaper" is mapped out here
+ // the 2.0 lb Wallcovering default (TK-11871: catalog norm; was 3.0). The banned word
+ // "Wallpaper" is mapped out here
// because this value ALSO becomes the displayed product_type (standing rule); weight
// is unaffected since the guard table scores Wallpaper == Wallcovering.
const PRODUCT_TYPE = (product.product_type || 'Wallcovering').replace(/\bWallpaper\b/gi, 'Wallcovering');
@@ -766,9 +900,10 @@ export async function runJob(jobId: string, req: BatchJobRequest): Promise<void>
markerSet = true; // create_started_at is durably stamped — from here, a throw is an UNKNOWN
// Shopify outcome (matching the sweeper's in-flight branch), NOT "never created".
- // Create product in Shopify (SERIAL, under the process-wide write lock). #2: retries
- // in-loop on a 429 (rate limit) with backoff instead of dropping the item on the first
- // throttle; a 429 is returned before persistence so the retry can never mint a duplicate.
+ // Create product in Shopify (SERIAL, under the process-wide write lock). 429 retry is
+ // owned SOLELY by shopify-api request() (SHOPIFY_REQUEST_MAX_RETRIES, jittered backoff,
+ // honors Retry-After); createWithRateLimitRetry is now a thin passthrough (no nested
+ // ladder). A 429 is returned before persistence, so those retries can never mint a dup.
const result = await createWithRateLimitRetry(productData);
if (result) {
@@ -822,7 +957,8 @@ export async function runJob(jobId: string, req: BatchJobRequest): Promise<void>
}
}
} finally {
- releaseWriteLock(); // #2: hand the Shopify write lock to the next waiting job
+ await releaseShopifyWriteDbLock(writeDbLockClient); // #2b: release the cross-process lock first
+ releaseWriteLock(); // #2: then hand the in-process Shopify write lock to the next waiting job
}
await setJobStatus(jobId, 'completed');
diff --git a/DW-Programming/ImportNewSkufromURL/lib/shopify-api.ts b/DW-Programming/ImportNewSkufromURL/lib/shopify-api.ts
index 974ac78b..3955a08f 100644
--- a/DW-Programming/ImportNewSkufromURL/lib/shopify-api.ts
+++ b/DW-Programming/ImportNewSkufromURL/lib/shopify-api.ts
@@ -4,6 +4,7 @@
*/
import { cleanProductPayload } from './clean-wallpaper';
+import { assertPriceIntegrity, resolveSampleFloor } from './price-integrity-gate';
interface ShopifyConfig {
storeDomain: string;
@@ -99,18 +100,56 @@ class ShopifyAPI {
options.body = JSON.stringify(data);
}
- try {
- const response = await fetch(url, options);
-
- if (!response.ok) {
+ // TK-11786 (review 2026-09-16): retry transient rate-limit / server errors with backoff.
+ // SAFETY — createProduct is a NON-idempotent POST and the batch worker's H2 design treats a
+ // 5xx/network failure on it as AMBIGUOUS (keep the dedup claim, never auto-mint a duplicate).
+ // So we retry by IDEMPOTENCE: a 429 is always safe (Shopify throttled BEFORE processing, the
+ // write provably did not land), but 5xx / network errors are retried ONLY for idempotent GETs
+ // — for a write they still throw, preserving the caller's ambiguity handling. The thrown error
+ // KEEPS the exact `Shopify API Error (NNN): …` shape that createDefinitivelyFailed() parses.
+ // TK-11786 review 2026-09-16: parse WITHOUT `|| 5` swallowing an explicit 0. `parseInt('0')||5`
+ // is 5, so the env var could never express "no retries" — the operator's mitigation now that
+ // request() is the SOLE 429 layer. `?? ''` → NaN when unset → the 5 default; a valid >=0 wins.
+ const _maxRetriesEnv = parseInt(process.env.SHOPIFY_REQUEST_MAX_RETRIES ?? '', 10);
+ const MAX_RETRIES = Number.isFinite(_maxRetriesEnv) && _maxRetriesEnv >= 0 ? _maxRetriesEnv : 5;
+ const idempotent = method === 'GET';
+ const backoffMs = (attempt: number, retryAfterHeader?: string | null) => {
+ const ra = parseFloat(retryAfterHeader || '');
+ if (Number.isFinite(ra) && ra > 0) return Math.min(30000, ra * 1000);
+ return Math.min(8000, 500 * 2 ** attempt) + Math.floor(Math.random() * 250); // jittered exp backoff
+ };
+ let attempt = 0;
+ for (;;) {
+ try {
+ const response = await fetch(url, options);
+ if (response.ok) return await response.json();
+
+ const status = response.status;
+ const retryable = status === 429 || (idempotent && status >= 500 && status < 600);
+ if (retryable && attempt < MAX_RETRIES) {
+ const waitMs = backoffMs(attempt, response.headers.get('retry-after'));
+ attempt++;
+ console.warn(`⏳ Shopify ${status} on ${method} ${endpoint} — retry ${attempt}/${MAX_RETRIES} in ${Math.round(waitMs)}ms`);
+ await new Promise(r => setTimeout(r, waitMs));
+ continue;
+ }
const errorText = await response.text();
- throw new Error(`Shopify API Error (${response.status}): ${errorText}`);
+ throw new Error(`Shopify API Error (${status}): ${errorText}`);
+ } catch (error) {
+ // Distinguish an HTTP error we already decided not to retry (has our "(NNN)" shape → rethrow
+ // as-is) from a raw network/timeout error (no HTTP status). A network error is retried ONLY
+ // for idempotent GETs — retrying a POST could duplicate a product that actually landed.
+ const isHttpError = error instanceof Error && /Shopify API Error \(\d{3}\)/.test(error.message);
+ if (!isHttpError && idempotent && attempt < MAX_RETRIES) {
+ const waitMs = backoffMs(attempt);
+ attempt++;
+ console.warn(`⏳ Shopify network error on ${method} ${endpoint} — retry ${attempt}/${MAX_RETRIES} in ${Math.round(waitMs)}ms`);
+ await new Promise(r => setTimeout(r, waitMs));
+ continue;
+ }
+ console.error('Shopify API Request Failed:', error);
+ throw error;
}
-
- return await response.json();
- } catch (error) {
- console.error('Shopify API Request Failed:', error);
- throw error;
}
}
@@ -190,10 +229,76 @@ class ShopifyAPI {
console.log(`✅ Metafields created for product ${productId}`);
}
+ /**
+ * TK-11786 review #2 — wire the shared price-integrity gate into this publish path.
+ * createFullProduct / bulkImportProducts were a FOURTH product-writer that bypassed the
+ * gate entirely (the primary import path in shopify.ts gates before promoting to active).
+ * This makes the "shared gate" actually shared: if a product would ship PUBLISHED but a
+ * variant violates the gate (Class C $0-orderable, Class A sample-leak, or the absolute
+ * floor), we DOWNGRADE it to unpublished (never ship a blocked product live) and log the
+ * block — mirroring shopify.ts's "block → stays draft" semantics. Never throws out of the
+ * import path; a gate INFRA error just proceeds (fail-safe / A-with-teeth). netCost is
+ * best-effort null here (this path has no vendor cost feed wired), so Class B markup is
+ * not enforced — the $0-orderable + sample-leak hard blocks are the point.
+ */
+ private gateBeforePublish(productData: ShopifyProduct): ShopifyProduct {
+ if (!productData.published || !Array.isArray(productData.variants) || productData.variants.length === 0) {
+ return productData; // only guards products actually being PUBLISHED with variants
+ }
+ try {
+ const isSample = (v: ProductVariant) =>
+ /(?:^|[-_ ])(?:sample|smp)$/i.test(v.sku || '') ||
+ [v.option1, v.option2, v.option3].some(o => (o || '').trim().toLowerCase() === 'sample');
+ const gate = assertPriceIntegrity({
+ dwSku: productData.variants[0]?.sku || productData.title,
+ netCost: null,
+ sampleFloor: resolveSampleFloor(productData.vendor),
+ variants: productData.variants.map((v) => {
+ const sample = isSample(v);
+ // A published variant is orderable when it can actually be bought: CONTINUE policy
+ // (oversell) or positive on-hand. Samples are the memo variant → not orderable.
+ const inStock =
+ String(v.inventory_policy).toLowerCase() === 'continue' || Number(v.inventory_quantity) > 0;
+ return {
+ role: sample ? ('sample' as const) : ('sellable' as const),
+ price: Number(v.price),
+ orderable: sample ? false : inStock,
+ sku: v.sku,
+ };
+ }),
+ });
+ if (!gate.ok) {
+ console.error(
+ JSON.stringify({
+ event: 'price_integrity_block',
+ source: 'shopify-api.createFullProduct',
+ title: productData.title,
+ violations: gate.violations,
+ }),
+ );
+ // DOWNGRADE — never ship a gate-blocked product live.
+ return { ...productData, published: false };
+ }
+ } catch (err) {
+ console.error(
+ JSON.stringify({
+ event: 'price_integrity_gate_error',
+ source: 'shopify-api.createFullProduct',
+ title: productData.title,
+ error: err instanceof Error ? err.message : String(err),
+ }),
+ );
+ // fail-safe: a gate INFRA error must never block an import.
+ }
+ return productData;
+ }
+
/**
* Create product with full Shopify schema including variants, images, and metafields
*/
async createFullProduct(productData: ShopifyProduct): Promise<any> {
+ // TK-11786 review #2: gate this publish path — downgrades to unpublished on a block.
+ productData = this.gateBeforePublish(productData);
// Step 1: Create the base product with variants and images
const product = await this.createProduct({
title: productData.title,
diff --git a/DW-Programming/fabricut-daily-post.js b/DW-Programming/fabricut-daily-post.js
index 4f45af30..4b59ac0b 100644
--- a/DW-Programming/fabricut-daily-post.js
+++ b/DW-Programming/fabricut-daily-post.js
@@ -51,7 +51,9 @@ async function activateOne(row,dry,st){
if(!result.ok){ st.kept_draft++; result.reasons.forEach(r=>st.reasons[r]=(st.reasons[r]||0)+1);
if(!dry&&result.tags.length){const gid=`gid://shopify/Product/${row.shopify_product_id}`;await gql(`mutation($id:ID!,$tags:[String!]!){tagsAdd(id:$id,tags:$tags){userErrors{message}}}`,{id:gid,tags:result.tags});}
return {dw:row.dw_sku,kept_draft:true}; }
- if(dry){ st.activated++; return {dw:row.dw_sku,would_activate:true}; }
+ // TK-11786 review #7 — do NOT early-return here in dry mode. Run the READ-ONLY image +
+ // weight gates below (both are GETs) so a --dry-run predicts the real holds instead of
+ // over-reporting would_activate; only the WRITES (tag/PUT/publish/ledger) are skipped in dry.
const pid=row.shopify_product_id, gid=`gid://shopify/Product/${pid}`;
// TK-10807 guard: never activate an image-less product. validate() checks the SOURCE image, but
// the upload to Shopify can fail silently at the 500GB storage cap (dw-shopify-storage-canary),
@@ -59,7 +61,7 @@ async function activateOne(row,dry,st){
const imgResp=await restR('GET',`/products/${pid}/images.json`);
const imgCount=(imgResp&&imgResp.images&&imgResp.images.length)||0;
if(imgCount<1){ st.kept_draft++; st.reasons['no_image_on_shopify']=(st.reasons['no_image_on_shopify']||0)+1;
- fs.appendFileSync(LEDGER,JSON.stringify({ts:new Date().toISOString(),dw:row.dw_sku,pid,pattern:row.pattern_name,action:'kept_draft:0_images_on_shopify'})+'\n');
+ if(!dry) fs.appendFileSync(LEDGER,JSON.stringify({ts:new Date().toISOString(),dw:row.dw_sku,pid,pattern:row.pattern_name,action:'kept_draft:0_images_on_shopify'})+'\n');
return {dw:row.dw_sku,kept_draft:true,reason:'no_image_on_shopify'}; }
// TK-11547 WEIGHT GO-LIVE GATE (Steve's TK-11414 rule): never activate at zero/missing weight —
// zero weight collapses the order into the lowest weight tier / free-shipping band and mis-costs
@@ -82,8 +84,9 @@ async function activateOne(row,dry,st){
if(!vars.length||zeroW.length){
const why=!vars.length?'no_variants_returned':'zero_weight';
st.kept_draft++; st.reasons[why]=(st.reasons[why]||0)+1;
- fs.appendFileSync(LEDGER,JSON.stringify({ts:new Date().toISOString(),dw:row.dw_sku,pid,pattern:row.pattern_name,action:`kept_draft:${why}`,variants:zeroW.map(v=>v.sku||v.id)})+'\n');
+ if(!dry) fs.appendFileSync(LEDGER,JSON.stringify({ts:new Date().toISOString(),dw:row.dw_sku,pid,pattern:row.pattern_name,action:`kept_draft:${why}`,variants:zeroW.map(v=>v.sku||v.id)})+'\n');
return {dw:row.dw_sku,kept_draft:true,reason:why}; }
+ if(dry){ st.activated++; return {dw:row.dw_sku,would_activate:true}; } // passed all read-only gates; a live run would activate
await restR('PUT',`/products/${pid}.json`,{product:{id:Number(pid),status:'active'}});
await publishExGYT(gid);
st.activated++;
@@ -179,8 +182,19 @@ async function main(){
const st={activated:0,kept_draft:0,failed:0,reasons:{}};
for(const pat of todaysPatterns){
const items=byPattern.get(pat);
+ const heldBefore=st.kept_draft, failedBefore=st.failed;
for(const row of items){ try{await activateOne(row,dry,st);}catch(e){st.failed++;console.error(`✗ ${row.dw_sku}: ${e.message}`);} if(!dry)await sleep(DELAY); }
- if(!dry) posted.add(pat); // mark posted only after the WHOLE pattern is done
+ // TK-11786 review #3 — mark a pattern posted ONLY when EVERY colorway actually went live
+ // this run. If any was held (kept_draft: image/weight/validate gate) or failed (transient
+ // 4xx/5xx), leave the pattern UNposted so the next run re-attempts the incomplete
+ // colorways instead of stranding them as permanent drafts. Selection `continue`s past an
+ // unposted pattern, so a persistently-held pattern just re-tries daily — it never
+ // hard-blocks new patterns from posting.
+ const heldThis=st.kept_draft-heldBefore, failedThis=st.failed-failedBefore;
+ if(!dry){
+ if(heldThis===0 && failedThis===0){ posted.add(pat); }
+ else { console.log(` ↻ ${pat}: ${heldThis} held + ${failedThis} failed this run — NOT marking posted; will re-attempt next run`); }
+ }
}
if(!dry) saveState(posted);
console.log(`\n=== ${dry?'DRY — would post':'POSTED'} ${st.activated} SKUs (${todaysPatterns.length} full patterns) | kept_draft:${st.kept_draft} fail:${st.failed} ===`);
← 38787388 Verify local session cookie with HMAC (close TK-11776 auth-b
·
back to Designer Wallcoverings
·
auto-data-snapshot: 2026-09-22T11:43:24 (4 data files) — OWN 99826b12 →