fix(onboarding-migration): idempotency check must ignore ad-hoc custom_* rows

alreadyNew was set false for EVERY row whose key wasn't a template key,
including custom_* rows that the same function deliberately preserves
verbatim. So a project with even one ad-hoc "+ Add step" entry was
never "identical" and got DELETE-and-rebuilt on every single run (new
row ids, a fresh audit row each time) — contradicting the documented
"safe to re-run / a second run does nothing".

Idempotency is now computed over non-custom rows only, using the same
custom_* discriminator addProjectMilestone's key generation relies on.

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

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

@@ -77,11 +77,9 @@ export async function migrateOnboardingTemplates(
       // Build the carried tick-state keyed by NEW key, and set aside ad-hoc custom rows.
       const carriedState = new Map<string, OldRow>()
       const customRows: OldRow[] = []
-      let alreadyNew = true
       for (const r of oldRows) {
-        if (!templateKeys.has(r.key)) alreadyNew = false
-        const isCustom = !templateKeys.has(r.key) && KEY_MAP[r.key] === undefined && !KNOWN_LEGACY_KEYS.has(r.key)
-        if (isCustom) { customRows.push(r); continue }
+        const isAdHocCustom = !templateKeys.has(r.key) && KEY_MAP[r.key] === undefined && !KNOWN_LEGACY_KEYS.has(r.key)
+        if (isAdHocCustom) { 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) {
@@ -99,10 +97,17 @@ export async function migrateOnboardingTemplates(
           dropped++
         }
       }
-      // Idempotency: if the project's keys already ARE exactly the template, skip.
-      const oldKeySet = new Set(oldRows.map((r) => r.key))
-      const identical = alreadyNew && oldKeySet.size === templateKeys.size
-        && [...templateKeys].every((k) => oldKeySet.has(k))
+      // Idempotency: if the project's keys already ARE exactly the template, skip. Computed
+      // over NON-CUSTOM rows only (regression fix): the old check flagged `alreadyNew = false`
+      // for ANY row whose key isn't a template key — including deliberately-preserved `custom_*`
+      // rows (the `addProjectMilestone` "+ Add step" key pattern) — so a project with even one
+      // ad-hoc step was never "identical" and got DELETE-and-rebuilt on every single run (new
+      // row ids, a fresh audit row each time), contradicting "safe to re-run / a second run
+      // does nothing". Using the same discriminator the preservation logic above relies on.
+      const isCustom = (k: string): boolean => k.startsWith('custom_')
+      const templateRowKeys = new Set(oldRows.filter((r) => !isCustom(r.key)).map((r) => r.key))
+      const identical = templateRowKeys.size === templateKeys.size
+        && [...templateKeys].every((k) => templateRowKeys.has(k))
       if (identical) continue
       projects++
       await db.transaction(async () => {

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

@@ -313,6 +313,48 @@ describe('onboarding template migration', () => {
     expect(audits[0]!.after_json).toContain('custom_ef56gh78')
   })
 
+  // Regression (FIX 3): the idempotency check used to flag `alreadyNew = false` for ANY row
+  // whose key wasn't a template key — including the deliberately-preserved `custom_*` ad-hoc
+  // steps — so a project carrying one was never "identical" and got DELETE-and-rebuilt on
+  // EVERY run (new row ids, a fresh audit row each time), contradicting "a second run does
+  // nothing". Idempotency must be computed over non-custom rows only.
+  it('is idempotent on a project carrying a custom_* ad-hoc step (second run is a true no-op)', async () => {
+    const cmId = await makeSmsClientModule()
+    await seedOldGenericRows(cmId)
+    await db.run(
+      `INSERT INTO project_milestone (id, client_module_id, key, label, done, done_on, sort)
+       VALUES (?,?,?,?,?,?,?)`,
+      'custom-1', cmId, 'custom_ab12cd34', 'AWS region confirmed', 1, '2026-05-01', 10,
+    )
+
+    const first = await migrateOnboardingTemplates(db)
+    expect(first.projects).toBe(1)
+    const afterFirst = await listProjectMilestones(db, cmId)
+    const rowIdsAfterFirst = (await db.all<{ id: string }>(
+      `SELECT id FROM project_milestone WHERE client_module_id=? ORDER BY sort`, cmId,
+    )).map((r) => r.id)
+
+    const second = await migrateOnboardingTemplates(db)
+    expect(second.projects).toBe(0) // true no-op, not a DELETE-and-rebuild
+    expect(second.carried).toBe(0)
+    expect(second.dropped).toBe(0)
+    expect(second.preserved).toBe(0)
+
+    const afterSecond = await listProjectMilestones(db, cmId)
+    expect(afterSecond).toEqual(afterFirst)
+    const rowIdsAfterSecond = (await db.all<{ id: string }>(
+      `SELECT id FROM project_milestone WHERE client_module_id=? ORDER BY sort`, cmId,
+    )).map((r) => r.id)
+    // Same row ids — proof the second run did not delete-and-reinsert.
+    expect(rowIdsAfterSecond).toEqual(rowIdsAfterFirst)
+
+    // No second audit row written for this client_module on the no-op pass.
+    const audits = await db.all<{ n: number }>(
+      `SELECT COUNT(*) AS n FROM audit_log WHERE action='migrate_onboarding' AND entity_id=?`, cmId,
+    )
+    expect((audits[0] as unknown as { n: number }).n).toBe(1)
+  })
+
   it('a known old-generic key with no forward mapping (amc_start) is still dropped, not treated as custom', async () => {
     const cmId = await makeSmsClientModule()
     await seedOldGenericRows(cmId) // includes ticked 'installed'/'go_live' and un-ticked 'amc_start'