fix(audit): interests/pipeline — M1 (outcome terminal guard), M3 (single-UPDATE + milestone gating), L1 (dead 'completed'), L2 (nurturing edge), L24 (deposit re-lock on refund)

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

src/lib/services/interests.service.ts

@@ -38,6 +38,7 @@ import {
   PIPELINE_STAGES,
   STAGE_LABELS,
   canTransitionStage,
+  canonicalizeStage,
   type PipelineStage,
 } from '@/lib/constants';
 import type {
@@ -1067,7 +1068,17 @@ export async function changeInterestStage(
   portId: string,
   data: ChangeStageInput,
   meta: AuditMeta,
+  // M3: distinguishes a manual/UI stage move (the /stage route + bulk route
+  // call this directly) from a lifecycle/signing-driven advance routed
+  // through advanceStageIfBehind (EOI sent/signed, deposit met, contract
+  // signed, reservation signed, custom-doc upload). For signing-driven
+  // advances the milestone date is owned by the doc-send/sign flow that
+  // already stamped the real event timestamp - auto-stamping `now` here on
+  // top of it back-dates "sent → signed" to ~0. So we only auto-populate
+  // milestone columns when `lifecycleDriven` is false (manual/UI move).
+  options?: { lifecycleDriven?: boolean },
 ) {
+  const lifecycleDriven = options?.lifecycleDriven ?? false;
   const existing = await db.query.interests.findFirst({
     where: eq(interests.id, id),
   });
@@ -1137,31 +1148,32 @@ export async function changeInterestStage(
 
   const oldStage = existing.pipelineStage;
 
-  const [updated] = await db
-    .update(interests)
-    .set({ pipelineStage: data.pipelineStage, updatedAt: new Date() })
-    .where(and(eq(interests.id, id), eq(interests.portId, portId)))
-    .returning();
-
   // BR-133: Auto-populate milestones based on stage. The rep can override the
   // stamp via `milestoneDate` when they're back-dating a real event (e.g.
   // "deposit landed yesterday"); we still default to now when omitted.
-  const milestoneDate = data.milestoneDate ? new Date(data.milestoneDate) : new Date();
+  //
+  // M3: only stamp milestone columns for manual/UI moves. Lifecycle/signing-
+  // driven advances (routed through advanceStageIfBehind) get their milestone
+  // dates from the doc-send/sign flow that already recorded the true event
+  // timestamp; auto-stamping `now` here on top of that back-dates intervals
+  // like "contract sent → signed" to ~0. Folding this into the same UPDATE as
+  // the stage change also removes the previous non-transactional double-write.
   const milestoneUpdates: Record<string, unknown> = {};
-  // For doc-bearing stages (eoi/reservation/contract) the milestone date is
-  // owned by the doc-send/sign flow, not the stage move - these only fire
-  // when the rep stamps a date manually via override.
-  if (data.pipelineStage === 'eoi') milestoneUpdates.dateEoiSent = milestoneDate;
-  if (data.pipelineStage === 'reservation') milestoneUpdates.dateReservationSigned = milestoneDate;
-  if (data.pipelineStage === 'deposit_paid') milestoneUpdates.dateDepositReceived = milestoneDate;
-  if (data.pipelineStage === 'contract') milestoneUpdates.dateContractSent = milestoneDate;
-  if (Object.keys(milestoneUpdates).length > 0) {
-    await db
-      .update(interests)
-      .set({ ...milestoneUpdates, updatedAt: new Date() })
-      .where(eq(interests.id, id));
+  if (!lifecycleDriven) {
+    const milestoneDate = data.milestoneDate ? new Date(data.milestoneDate) : new Date();
+    if (data.pipelineStage === 'eoi') milestoneUpdates.dateEoiSent = milestoneDate;
+    if (data.pipelineStage === 'reservation')
+      milestoneUpdates.dateReservationSigned = milestoneDate;
+    if (data.pipelineStage === 'deposit_paid') milestoneUpdates.dateDepositReceived = milestoneDate;
+    if (data.pipelineStage === 'contract') milestoneUpdates.dateContractSent = milestoneDate;
   }
 
+  const [updated] = await db
+    .update(interests)
+    .set({ pipelineStage: data.pipelineStage, ...milestoneUpdates, updatedAt: new Date() })
+    .where(and(eq(interests.id, id), eq(interests.portId, portId)))
+    .returning();
+
   void createAuditLog({
     userId: meta.userId,
     portId,
@@ -1280,7 +1292,14 @@ export async function advanceStageIfBehind(
     return false;
   }
 
-  await changeInterestStage(interestId, portId, { pipelineStage: target, reason }, meta);
+  // M3: this helper is the single funnel for every lifecycle/signing-driven
+  // advance (EOI sent/signed, deposit met, contract signed, reservation
+  // signed, custom-doc upload). Flag the move so changeInterestStage does not
+  // auto-stamp milestone dates - those are owned by the doc-send/sign flow,
+  // which already recorded the real event timestamp.
+  await changeInterestStage(interestId, portId, { pipelineStage: target, reason }, meta, {
+    lifecycleDriven: true,
+  });
   return true;
 }
 
@@ -1366,6 +1385,19 @@ export async function setInterestOutcome(
   });
   if (!existing) throw new NotFoundError('Interest');
 
+  // M1: terminal-state guard. Once an outcome is set this method must not run
+  // again - re-firing it (won→lost flip, double-submit, an idempotent webhook
+  // retry) would re-evaluate the berth rule, rename the document folder, write
+  // a duplicate audit row, re-emit the socket event and re-fire the Umami
+  // event. Mirrors clearInterestOutcome's `if (!existing.outcome)` guard:
+  // changing a recorded outcome requires clearing it first (which reopens the
+  // deal), so the side effects only ever run on a genuine close transition.
+  if (existing.outcome) {
+    throw new ConflictError(
+      'Interest already has an outcome. Clear the current outcome before setting a new one.',
+    );
+  }
+
   const oldOutcome = existing.outcome;
   const stageAtOutcome = existing.pipelineStage;
 
@@ -1453,16 +1485,17 @@ export async function clearInterestOutcome(
 
   // Reopen-stage logic:
   // - If the caller passed `data.reopenStage`, honor it (rep override).
-  // - Else if the current stage is the legacy 'completed' sentinel,
-  //   default to 'qualified' (closest analog of the pre-refactor
-  //   'in_communication' which would have lived there).
   // - Else preserve the current stage - post-refactor setOutcome stops
   //   touching pipelineStage, so the deal already knows where it was
   //   when the rep closed it. Reopening should drop the rep back into
   //   that same column on the kanban.
-  const reopenStage =
-    data.reopenStage ??
-    (existing.pipelineStage === 'completed' ? 'qualified' : existing.pipelineStage);
+  // L1: the dead `pipelineStage === 'completed' ? 'qualified'` branch is
+  // removed (the 'completed' sentinel was dropped in the 9→7 migration).
+  // Any legacy stage value still on the row is folded to its canonical
+  // 7-stage equivalent via canonicalizeStage so a pre-migration
+  // 'completed' row reopens to `contract` (its true pre-close stage) and
+  // never re-enters the kanban with a non-canonical value.
+  const reopenStage = data.reopenStage ?? canonicalizeStage(existing.pipelineStage);
   const now = new Date();
   await db
     .update(interests)