fix(onboarding-migration): deterministic survivor when two old ticks collapse onto one key (FIX C)

advance_paid/balance_paid (and the interim advance_payment/balance_payment)
all collapse onto the single new payment_received key, and the carry used
Map.set -- whichever row was visited second silently overwrote the first's
done_on/payment_id, counted as neither carried nor dropped, with the audit
`before` left `undefined` so the loss was unreconstructible.

pickSurvivingTick() now picks deterministically (prefers the tick with a
real linked payment_id; otherwise the later done_on), the losing tick is
counted in a new `collapsed` field, and the audit `before` payload is now
the full pre-migration row set instead of `undefined` -- any discarded tick
is reconstructible from the append-only log.

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

apps/hq/src/migrate-onboarding-templates.ts

@@ -32,6 +32,21 @@ interface OldRow {
   payment_id: string | null; document_id: string | null
 }
 
+/**
+ * When two+ old keys collapse onto the SAME new key (e.g. `advance_paid` + `balance_paid`
+ * both -> `payment_received`), pick which tick survives, deterministically: prefer whichever
+ * carries a real linked payment (`payment_id`) — that link is the more "meaningful" tick;
+ * if both or neither do, prefer the one with the LATEST `done_on` (the more recent progress).
+ * The loser is never silently overwritten — the caller counts it (`collapsed`) and the full
+ * pre-migration row set goes into the audit `before` payload, so it's always reconstructible.
+ */
+function pickSurvivingTick(a: OldRow, b: OldRow): OldRow {
+  const aHasPayment = a.payment_id !== null && a.payment_id !== ''
+  const bHasPayment = b.payment_id !== null && b.payment_id !== ''
+  if (aHasPayment !== bHasPayment) return aHasPayment ? a : b
+  return (a.done_on ?? '') >= (b.done_on ?? '') ? a : b
+}
+
 /**
  * Idempotent: for every client_module whose module has a per-module onboarding template
  * (a `project.milestone_template:<CODE>` setting — task 2), rebuild its milestone rows
@@ -44,12 +59,12 @@ interface OldRow {
  */
 export async function migrateOnboardingTemplates(
   db: DB,
-): Promise<{ projects: number; carried: number; dropped: number; preserved: number }> {
+): Promise<{ projects: number; carried: number; dropped: number; preserved: number; collapsed: number }> {
   // Which module codes have a per-module template setting?
   const codes = (await db.all<{ key: string }>(
     `SELECT key FROM setting WHERE key LIKE 'project.milestone_template:%'`))
     .map((r) => r.key.slice('project.milestone_template:'.length))
-  let projects = 0, carried = 0, dropped = 0, preserved = 0
+  let projects = 0, carried = 0, dropped = 0, preserved = 0, collapsed = 0
   for (const code of codes) {
     const template = await milestoneTemplate(db, code)
     const templateKeys = new Set(template.map((t) => t.key))
@@ -69,7 +84,17 @@ export async function migrateOnboardingTemplates(
         if (isCustom) { customRows.push(r); continue }
         const target = templateKeys.has(r.key) ? r.key : KEY_MAP[r.key]
         if (target !== undefined && templateKeys.has(target)) {
-          if (r.done === 1) carriedState.set(target, r)
+          if (r.done === 1) {
+            const existing = carriedState.get(target)
+            if (existing === undefined) {
+              carriedState.set(target, r)
+            } else {
+              // Two old keys collapse onto the same new key — deterministically pick a
+              // survivor (never a silent Map.set overwrite) and count the one that loses.
+              carriedState.set(target, pickSurvivingTick(existing, r))
+              collapsed++
+            }
+          }
         } else if (r.done === 1) {
           dropped++
         }
@@ -108,10 +133,13 @@ export async function migrateOnboardingTemplates(
           sort++
         }
         preserved += customRows.length
-        await writeAudit(db, 'system', 'migrate_onboarding', 'client_module', cmId, undefined,
+        // The full pre-migration row set is recorded as `before` (not just a summary) so
+        // ANY discarded tick — dropped, or the losing side of a collapse — is reconstructible
+        // from the append-only audit log, never silently unrecoverable (FIX C).
+        await writeAudit(db, 'system', 'migrate_onboarding', 'client_module', cmId, oldRows,
           { module: code, carried: [...carriedState.keys()], preserved: customRows.map((c) => c.key) })
       })
     }
   }
-  return { projects, carried, dropped, preserved }
+  return { projects, carried, dropped, preserved, collapsed }
 }

apps/hq/test/migrate-onboarding-templates.test.ts

@@ -321,4 +321,75 @@ describe('onboarding template migration', () => {
     const ms = await listProjectMilestones(db, cmId)
     expect(ms.some((m) => m.key === 'amc_start')).toBe(false)
   })
+
+  // Pre-ship audit FIX C: advance_paid AND balance_paid (and the interim advance_payment /
+  // balance_payment) all collapse onto the single new `payment_received` key. The old code
+  // used Map.set, so whichever row was visited last silently overwrote the other — its
+  // done_on AND payment_id lost, counted as neither `carried` nor `dropped`, and the audit
+  // `before` was `undefined` so the loss was unreconstructible.
+  it('when two old keys collapse onto the same new key, deterministically keeps the linked tick, counts the loser, and records the full pre-migration state in the audit before payload', async () => {
+    const cmId = await makeSmsClientModule()
+    const client = await db.get<{ client_id: string }>(`SELECT client_id FROM client_module WHERE id=?`, cmId)
+    const paymentId = 'pay-collapse-1'
+    await db.run(
+      `INSERT INTO payment (id, client_id, received_on, mode, amount_paise, created_by, created_at)
+       VALUES (?, ?, '2026-03-01', 'bank', 500000, 'u1', '2026-03-01T00:00:00.000Z')`,
+      paymentId, client!.client_id,
+    )
+    // advance_paid: ticked LATER, but with NO payment link.
+    await db.run(
+      `INSERT INTO project_milestone (id, client_module_id, key, label, done, done_on, sort)
+       VALUES (?,?,?,?,?,?,?)`,
+      'm0', cmId, 'advance_paid', 'advance_paid', 1, '2026-06-01', 0,
+    )
+    // balance_paid: ticked EARLIER, but WITH a real payment link — this one must survive.
+    await db.run(
+      `INSERT INTO project_milestone (id, client_module_id, key, label, done, done_on, payment_id, sort)
+       VALUES (?,?,?,?,?,?,?,?)`,
+      'm1', cmId, 'balance_paid', 'balance_paid', 1, '2026-03-01', paymentId, 1,
+    )
+
+    const res = await migrateOnboardingTemplates(db)
+    expect(res.collapsed).toBe(1)
+    expect(res.carried).toBe(1) // only ONE surviving tick lands on payment_received
+    expect(res.dropped).toBe(0) // the loser isn't "dropped" — it's "collapsed"
+
+    const ms = await listProjectMilestones(db, cmId)
+    const paymentReceived = ms.find((m) => m.key === 'payment_received')!
+    expect(paymentReceived.done).toBe(true)
+    expect(paymentReceived.paymentId).toBe(paymentId) // the LINKED tick wins, not the later one
+    expect(paymentReceived.doneOn).toBe('2026-03-01')
+
+    // The full pre-migration row set is in the audit `before` — both ticks reconstructible.
+    const audits = await db.all<{ before_json: string | null }>(
+      `SELECT before_json FROM audit_log WHERE action='migrate_onboarding' AND entity_id=?`, cmId,
+    )
+    expect(audits).toHaveLength(1)
+    expect(audits[0]!.before_json).not.toBeNull()
+    const before = JSON.parse(audits[0]!.before_json!) as
+      { key: string; done: number; done_on: string | null; payment_id: string | null }[]
+    const advancePaidBefore = before.find((r) => r.key === 'advance_paid')!
+    expect(advancePaidBefore).toMatchObject({ done: 1, done_on: '2026-06-01', payment_id: null })
+    const balancePaidBefore = before.find((r) => r.key === 'balance_paid')!
+    expect(balancePaidBefore).toMatchObject({ done: 1, done_on: '2026-03-01', payment_id: paymentId })
+  })
+
+  it('when neither collapsing tick has a payment link, keeps the LATEST done_on', async () => {
+    const cmId = await makeSmsClientModule()
+    await db.run(
+      `INSERT INTO project_milestone (id, client_module_id, key, label, done, done_on, sort)
+       VALUES (?,?,?,?,?,?,?)`,
+      'm0', cmId, 'advance_paid', 'advance_paid', 1, '2026-02-01', 0,
+    )
+    await db.run(
+      `INSERT INTO project_milestone (id, client_module_id, key, label, done, done_on, sort)
+       VALUES (?,?,?,?,?,?,?)`,
+      'm1', cmId, 'balance_paid', 'balance_paid', 1, '2026-05-15', 1,
+    )
+
+    const res = await migrateOnboardingTemplates(db)
+    expect(res.collapsed).toBe(1)
+    const ms = await listProjectMilestones(db, cmId)
+    expect(ms.find((m) => m.key === 'payment_received')!.doneOn).toBe('2026-05-15') // latest wins
+  })
 })