fix(onboarding-migration): preserve ad-hoc custom steps across the template swap

Final whole-branch review FIX 3.

migrateOnboardingTemplates previously DELETEd every project_milestone row and
reinserted only template keys, so a project-specific step added via
addProjectMilestone ("+ Add step", custom_* keys) vanished with no trace if
un-ticked, or was miscounted as `dropped` if ticked.

Old rows are now partitioned before the DELETE into template-mapped vs
ad-hoc custom (not a template key, not in KEY_MAP, and not a known old
generic FRONT_FALLBACK/TAIL_FALLBACK key like amc_start — those keep their
existing drop behavior). Custom rows are re-inserted unchanged after the
template block, sort continuing past it, preserving
label/done/done_on/payment_id/document_id. The return type and the audit
`after` payload both gained a `preserved` count/list.

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

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

@@ -13,7 +13,7 @@ async function main() {
 
   const res = await migrateOnboardingTemplates(db)
   // eslint-disable-next-line no-console
-  console.log(`Onboarding template migration: ${res.projects} project(s) rebuilt, ${res.carried} tick(s) carried, ${res.dropped} dropped.`)
+  console.log(`Onboarding template migration: ${res.projects} project(s) rebuilt, ${res.carried} tick(s) carried, ${res.dropped} dropped, ${res.preserved} custom step(s) preserved.`)
 }
 
 main().catch((e) => { console.error(e); process.exit(1) })

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

@@ -1,10 +1,13 @@
 // apps/hq/src/migrate-onboarding-templates.ts — Client Detail redesign task 5: one-time
 // migration swapping seeded modules (SMS/RTGS/MobileApp) off the old generic onboarding
 // checklist onto their new per-module template (task 2), carrying mapped ticks
-// (done + done_on + payment_id + document_id) so onboarding history is not lost.
+// (done + done_on + payment_id + document_id) so onboarding history is not lost. Ad-hoc
+// project-specific steps ("+ Add step" / `addProjectMilestone`, `custom_*` keys) are never
+// template-mapped — they are preserved verbatim, appended after the template block, so a
+// staffer's own checklist entries survive the swap too (see `preserved`, below).
 import { uuidv7 } from '@sims/domain'
 import { writeAudit } from './audit'
-import { milestoneTemplate } from './repos-milestones'
+import { FRONT_FALLBACK, TAIL_FALLBACK, milestoneTemplate } from './repos-milestones'
 import type { DB } from './db'
 
 /** old generic key -> new template key (only applied if the new template has that key). */
@@ -15,26 +18,38 @@ const KEY_MAP: Record<string, string> = {
   setup_done: 'setup_configuration',
 }
 
+/**
+ * Old generic (pre-redesign) keys that intentionally have NO forward mapping — e.g.
+ * `amc_start` (AMC/renewal is tracked elsewhere now). These are code-level defaults from
+ * FRONT_FALLBACK/TAIL_FALLBACK, not ad-hoc project-specific steps (those always use
+ * `addProjectMilestone`'s `custom_*` key pattern), so a ticked one still counts as `dropped`
+ * — never `preserved` — same as before this fix.
+ */
+const KNOWN_LEGACY_KEYS = new Set([...FRONT_FALLBACK, ...TAIL_FALLBACK].map((t) => t.key))
+
 interface OldRow {
-  key: string; done: number; done_on: string | null; payment_id: string | null; document_id: string | null
+  key: string; label: string; done: number; done_on: string | null
+  payment_id: string | null; document_id: string | null
 }
 
 /**
  * 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
  * from that template, carrying mapped ticks (done/done_on/payment_id/document_id) forward under their
- * new key. Unmapped old ticks (e.g. `amc_start`, or `setup_done` for a module whose new
- * template has no matching step) are dropped (and counted). Client_modules whose module
- * has no per-module template are left completely untouched.
+ * new key. Unmapped old GENERIC ticks (e.g. `amc_start`, or `setup_done` for a module whose
+ * new template has no matching step) are dropped (and counted). Ad-hoc project-specific rows
+ * (any key that isn't a template key, isn't in KEY_MAP, and isn't a known old generic key) are
+ * preserved unchanged and re-appended after the template block (counted in `preserved`).
+ * Client_modules whose module has no per-module template are left completely untouched.
  */
 export async function migrateOnboardingTemplates(
   db: DB,
-): Promise<{ projects: number; carried: number; dropped: number }> {
+): Promise<{ projects: number; carried: number; dropped: number; preserved: 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
+  let projects = 0, carried = 0, dropped = 0, preserved = 0
   for (const code of codes) {
     const template = await milestoneTemplate(db, code)
     const templateKeys = new Set(template.map((t) => t.key))
@@ -42,12 +57,16 @@ export async function migrateOnboardingTemplates(
       `SELECT cm.id FROM client_module cm JOIN module m ON m.id = cm.module_id WHERE m.code=?`, code)
     for (const { id: cmId } of cms) {
       const oldRows = await db.all<OldRow>(
-        `SELECT key, done, done_on, payment_id, document_id FROM project_milestone WHERE client_module_id=?`, cmId)
-      // Build the carried tick-state keyed by NEW key.
+        `SELECT key, label, done, done_on, payment_id, document_id FROM project_milestone
+          WHERE client_module_id=? ORDER BY sort`, cmId)
+      // 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 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)
@@ -76,10 +95,23 @@ export async function migrateOnboardingTemplates(
           if (c !== undefined) carried++
           sort++
         }
+        // Ad-hoc project-specific steps survive the swap unchanged, appended after the
+        // template (sort continues past it) so this project's own checklist entries — and
+        // whatever tick state they carry — are never silently lost.
+        for (const c of customRows) {
+          await db.run(
+            `INSERT INTO project_milestone
+               (id, client_module_id, key, label, done, done_on, payment_id, document_id, sort)
+             VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+            uuidv7(), cmId, c.key, c.label, c.done, c.done_on, c.payment_id, c.document_id, sort,
+          )
+          sort++
+        }
+        preserved += customRows.length
         await writeAudit(db, 'system', 'migrate_onboarding', 'client_module', cmId, undefined,
-          { module: code, carried: [...carriedState.keys()] })
+          { module: code, carried: [...carriedState.keys()], preserved: customRows.map((c) => c.key) })
       })
     }
   }
-  return { projects, carried, dropped }
+  return { projects, carried, dropped, preserved }
 }

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

@@ -272,4 +272,53 @@ describe('onboarding template migration', () => {
     const after = await listProjectMilestones(db, cm.id)
     expect(after).toEqual(before)
   })
+
+  // Final whole-branch review FIX 3: an ad-hoc project-specific step (`+ Add step` /
+  // addProjectMilestone, `custom_*` key) must survive the swap — ticked or not — instead of
+  // being silently deleted because it's in neither templateKeys nor KEY_MAP.
+  it('preserves ad-hoc custom steps (both ticked and un-ticked), appended after the template, with state intact', async () => {
+    const cmId = await makeSmsClientModule()
+    await seedOldGenericRows(cmId) // baseline so the project isn't already "identical" (forces a rebuild)
+    await db.run(
+      `INSERT INTO project_milestone (id, client_module_id, key, label, done, done_on, sort)
+       VALUES (?,?,?,?,?,?,?)`,
+      'custom-ticked', cmId, 'custom_ab12cd34', 'AWS region confirmed', 1, '2026-05-01', 10,
+    )
+    await db.run(
+      `INSERT INTO project_milestone (id, client_module_id, key, label, done, done_on, sort)
+       VALUES (?,?,?,?,?,?,?)`,
+      'custom-unticked', cmId, 'custom_ef56gh78', 'Client-specific step', 0, null, 11,
+    )
+
+    const res = await migrateOnboardingTemplates(db)
+    expect(res.preserved).toBe(2)
+    // Neither custom row counts as dropped, even though the ticked one is done=1.
+    expect(res.dropped).toBe(0)
+
+    const ms = await listProjectMilestones(db, cmId)
+    const ticked = ms.find((m) => m.key === 'custom_ab12cd34')
+    expect(ticked).toMatchObject({ label: 'AWS region confirmed', done: true, doneOn: '2026-05-01' })
+    const unticked = ms.find((m) => m.key === 'custom_ef56gh78')
+    expect(unticked).toMatchObject({ label: 'Client-specific step', done: false, doneOn: null })
+    // Appended after the whole template block, sort continuing past it (deterministic order:
+    // the ticked one was inserted before the un-ticked one).
+    expect(ms.slice(-2).map((m) => m.key)).toEqual(['custom_ab12cd34', 'custom_ef56gh78'])
+
+    // Reflected in the audit's `after` payload.
+    const audits = await db.all<{ after_json: string | null }>(
+      `SELECT after_json FROM audit_log WHERE action='migrate_onboarding' AND entity_id=?`, cmId,
+    )
+    expect(audits).toHaveLength(1)
+    expect(audits[0]!.after_json).toContain('custom_ab12cd34')
+    expect(audits[0]!.after_json).toContain('custom_ef56gh78')
+  })
+
+  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'
+    const res = await migrateOnboardingTemplates(db)
+    expect(res.preserved).toBe(0)
+    const ms = await listProjectMilestones(db, cmId)
+    expect(ms.some((m) => m.key === 'amc_start')).toBe(false)
+  })
 })