Stop billing members for syndicated impressions that were never delivered

Network Ad Space has no stop flag. Its serving query only picks rows with remaining>0,
so zeroing that counter is the only way to halt an ad. But delivery is derived as
assigned - remaining, so the moment an ad was stopped it read back as 100% DELIVERED.

That was not only a reporting error. reconcileNas() runs every five minutes, had no
status filter, and CHARGES member credits off that figure. So pausing a campaign, or
ending one, made the next reconcile pass bill the member for the entire unspent budget
as though it had all been served.

Measured on production before the fix: 23 member campaigns across 16 members, every
single one charged to exactly 100% of budget, 11,437 credits in total, against on-site
delivery evidence of roughly 4,700 impressions. A naturally exhausted ad also ends at
remaining=0, so the two cases cannot be told apart after the fact, which is why the
true figure has to be captured before the stop.

Three changes:
- nas.deactivate() now reads the real served count BEFORE zeroing and returns it.
- a new ads.stopNas() helper is the only path to a stop, and it persists that figure as
  the campaign's final delivery. No caller touches nas.deactivate() directly any more.
- reconcileNas() only processes campaigns with status='active'. A stopped ad delivers
  nothing further, so there is never anything legitimate left to charge for.

qa/nas-served.mjs (8 assertions) stubs the NAS layer and drives the real code: pausing
records the true 3,000 rather than the 10,000 allocation, a paused campaign is never
charged afterwards and its figure never jumps to the allocation, and an active campaign
still reconciles and is charged normally so the guard did not break delivery.

fraud-allow 12, sponsor-note 5, chatbot-parse 35, qa/run.sh member 0 bugs.

Historical delivery is not recoverable: the stop overwrote the only record of it.
Refunding the 11,437 credits to the 16 affected members is Marty's call, pending.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nas.js

@@ -100,11 +100,22 @@ async function readServed(nasAdId) {
   return { assigned, remaining, served, clicks: Number(rows[0].hits) || 0 };
 }
 
-// stop a syndicated ad (budget spent, paused, or expired) — remaining:0 halts serving.
+// Stop a syndicated ad (budget spent, paused, or expired). NAS has no stop flag: its
+// serving query only picks rows with remaining>0, so zeroing that counter is the only
+// way to halt an ad.
+//
+// THE TRAP (found 2026-09-17): served is derived as assigned-remaining, so zeroing also
+// makes a stopped ad read back as 100% DELIVERED. That inflated every stopped campaign's
+// reported delivery, and worse, reconcileNas charges members for the difference — so
+// pausing a campaign could bill for impressions that were never served. A naturally
+// exhausted ad also ends at remaining=0, so the two cases are indistinguishable
+// afterwards. The true figure therefore has to be captured BEFORE the stop.
 async function deactivate(nasAdId) {
   if (!enabled()) return { skipped: 'flag-off' };
+  let served = null;
+  try { const s = await readServed(nasAdId); if (s) served = s.served; } catch (e) {}
   await q('UPDATE sponsorads SET remaining=0, EDate=NOW() WHERE ID=?', [Number(nasAdId)]);
-  return { ok: true };
+  return { ok: true, served };   // callers MUST persist this as the final served count
 }
 
 // top up a syndicated ad with more impressions (buy-more-views / reactivate).

qa/nas-served.mjs

@@ -0,0 +1,102 @@
+// The Network Ad Space served-count trap (found 2026-09-17).
+//
+// NAS has no stop flag: its serving query only picks rows with remaining>0, so zeroing
+// that counter is the only way to halt an ad. But served is derived as
+// assigned - remaining, so a STOPPED ad reads back as 100% delivered. Two harms:
+//   1. reported delivery was inflated for every stopped campaign;
+//   2. reconcileNas CHARGES MEMBER CREDITS off that figure, so pausing a campaign could
+//      bill someone for impressions that were never served.
+//
+// Runs entirely against the JSON store with the NAS layer stubbed. Touches no real data.
+//   node qa/nas-served.mjs
+import { createRequire } from 'node:module';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+const require = createRequire(import.meta.url);
+
+const ok = [], bad = [];
+const t = (n, c, extra) => { (c ? ok : bad).push(n + (c || !extra ? '' : '  -> ' + extra)); };
+
+const DATA = path.join(os.tmpdir(), 'iap-nas-served-' + Date.now());
+fs.mkdirSync(DATA, { recursive: true });
+
+// stub the NAS layer BEFORE ads.js uses it (ads.js holds the module object, so mutating
+// its exports is enough)
+const nas = require('../nas.js');
+let nasRow = { assigned: 10000, remaining: 7000, hits: 3 };   // 3,000 genuinely served
+let deactivated = 0;
+nas.enabled = () => true;
+nas.impressionsPerCredit = () => 1;
+nas.readServed = async () => ({ assigned: nasRow.assigned, remaining: nasRow.remaining,
+  served: Math.max(0, Math.min(nasRow.assigned, nasRow.assigned - nasRow.remaining)), clicks: nasRow.hits });
+nas.deactivate = async () => {
+  const served = Math.max(0, Math.min(nasRow.assigned, nasRow.assigned - nasRow.remaining));
+  nasRow.remaining = 0;            // what the real stop does
+  deactivated++;
+  return { ok: true, served };     // the fix: report the TRUE figure captured first
+};
+nas.topUp = async () => ({ ok: true });
+
+const ads = require('../ads.js');
+ads.init({ dataDir: DATA, chain: null });
+
+// ---- 1. stopping an ad must record what it really delivered, not its whole allocation
+// Seed the store directly: going through createCampaign would drag in balances and the
+// chain, and neither is what this test is about.
+const FILE = path.join(DATA, 'campaigns.json');   // NOT ads.json
+const id = 9001;
+const seed = (status, nasServed) => {
+  fs.writeFileSync(FILE, JSON.stringify({
+    v: 1, nextId: 9002, campaigns: [{ id, owner: 'tester@example.com', memberId: 0, type: 'banner', name: 'served test',
+      targetUrl: 'https://example.com', imageUrl: 'https://example.com/b.png', width: 728, height: 90,
+      budget: 10000, spent: 0, accrued: 0, imps: 0, clicks: 0, status, nasAdId: 4242,
+      nasServed, created: Date.now() }],
+    burnsPending: [], v: 1, nextId: 9002
+  }));
+  ads.init({ dataDir: DATA, chain: null });
+};
+const store = () => JSON.parse(fs.readFileSync(FILE, 'utf8'));
+const findC = () => (store().campaigns || []).find(c => c.id === id);
+
+seed('active', 0);
+t('test campaign seeded', !!findC(), 'missing');
+let c;
+
+// pause it: the stop path runs, and the TRUE served (3,000) must be what is recorded
+await ads.setStatus('tester@example.com', id, 'paused').catch(() => {});
+c = findC();
+t('pausing stopped the NAS ad', deactivated >= 1, String(deactivated));
+t('the recorded served count is the REAL 3,000, not the 10,000 allocation',
+  c && c.nasServed === 3000, c ? 'nasServed=' + c.nasServed : 'campaign missing');
+t('and it is not the full allocation', !c || c.nasServed !== 10000, c && String(c.nasServed));
+
+// ---- 2. a stopped campaign must never be reconciled again (this is the billing guard)
+const spentBefore = c ? (c.spent || 0) + (c.accrued || 0) : 0;
+await ads.reconcileNas();
+c = findC();
+const spentAfter = c ? (c.spent || 0) + (c.accrued || 0) : 0;
+t('a paused campaign is NOT charged after its NAS ad was stopped',
+  spentAfter === spentBefore, 'before ' + spentBefore + ' after ' + spentAfter);
+t('and its served figure did not jump to the allocation',
+  c && c.nasServed === 3000, c && String(c.nasServed));
+
+// ---- 3. an ACTIVE campaign still reconciles normally, so the guard did not break delivery
+nasRow = { assigned: 10000, remaining: 5000, hits: 9 };   // 5,000 served now
+c = findC();
+if (c) { c.status = 'active'; c.nasServed = 3000;
+  const s = store(); const i = s.campaigns.findIndex(x => x.id === id); s.campaigns[i] = c;
+  fs.writeFileSync(FILE, JSON.stringify(s)); ads.init({ dataDir: DATA, chain: null }); }
+await ads.reconcileNas();
+c = findC();
+t('an active campaign still reconciles real delivery', c && c.nasServed === 5000, c && String(c.nasServed));
+// The seeded member has no funded balance, so the charge itself cannot land; what matters
+// is that reconcile ATTEMPTED it and the campaign was retired as exhausted rather than
+// silently continuing to accrue undelivered impressions.
+t('reconcile acted on the new impressions and retired the campaign',
+  c && c.status === 'out', c && 'status=' + c.status);
+
+console.log('PASS ' + ok.length);
+for (const b of bad) console.log('FAIL ' + b);
+try { fs.rmSync(DATA, { recursive: true, force: true }); } catch (e) {}
+process.exit(bad.length ? 1 : 0);