fix(snapshot): harden message id retention

packages/support/acp-snapshot/src/suite.ts

@@ -20,6 +20,7 @@
 import { readFile, readdir, rm, writeFile } from 'node:fs/promises'
 import { existsSync } from 'node:fs'
 import { join } from 'node:path'
+import { isSurfaceEligibleType } from '@deepseek-ai/dsh-session/surface'
 import { describe, expect, it } from 'vitest'
 import { type AgentUnderTest, type HarvestedLog, type InputScript, runScenario } from './harness.ts'
 import {
@@ -512,11 +513,11 @@ export function headerChangeCount(rawLog: string): number {
     .length
 }
 
-/** A literal string replacement used to carry an existing fixture value into fresh write-back. */
+/** A literal replacement from a fresh replay-run volatile to its existing fixture value. */
 export interface FixtureReplacement {
-  /** The fresh run's value to replace. */
+  /** The fresh replay run's volatile value. */
   from: string
-  /** The existing fixture value to keep. */
+  /** The existing fixture value retained during write-back. */
   to: string
 }
 
@@ -526,24 +527,51 @@ function parseJsonlRecords(text: string): Record<string, unknown>[] {
     .map(line => JSON.parse(line) as Record<string, unknown>)
 }
 
+/** Narrow one parsed value to the complete identified-message shape retained by fixtures. */
+function completeMessage(value: unknown): Record<string, unknown> | undefined {
+  if (
+    !isRecord(value)
+    || typeof value.id !== 'string'
+    || !UUID_RE.test(value.id)
+    || typeof value.role !== 'string'
+    || !Array.isArray(value.content)
+    || !isRecord(value.source)
+  ) return undefined
+  return value
+}
+
 /** Return the complete identified message carried by one surface event. */
-function eventMessage(record: Record<string, unknown>): Record<string, unknown> | undefined {
+function surfaceEventMessage(record: Record<string, unknown>): Record<string, unknown> | undefined {
+  const type = record.type
+  if (typeof type !== 'string' || !isSurfaceEligibleType(type)) return undefined
   const data = record.data
   if (!isRecord(data)) return undefined
-  const message = record.type === 'user/message'
-    ? data
-    : record.type === 'assistant/message' || record.type === 'tool/result' || record.type === 'steering/message'
-      ? data.message
-      : undefined
-  if (
-    !isRecord(message)
-    || typeof message.id !== 'string'
-    || !UUID_RE.test(message.id)
-    || typeof message.role !== 'string'
-    || !Array.isArray(message.content)
-    || !isRecord(message.source)
-  ) return undefined
-  return message
+  let message: unknown
+  switch (type) {
+    case 'user/message':
+      message = data
+      break
+    case 'assistant/message':
+    case 'tool/result':
+      message = data.message
+      break
+    /* v8 ignore next -- the authoritative predicate must fail loud when a new surface shape lands. */
+    default: throw new Error(`acp-snapshot: unsupported surface event type "${type}"`)
+  }
+  return completeMessage(message)
+}
+
+/** Return complete message identities structurally owned by one durable record. */
+function recordMessages(record: Record<string, unknown>): Record<string, unknown>[] {
+  const surfaceMessage = surfaceEventMessage(record)
+  if (surfaceMessage !== undefined) return [surfaceMessage]
+  if (record.type !== 'agent/inbox/spliced' || !isRecord(record.data) || !Array.isArray(record.data.inserted)) {
+    return []
+  }
+  return record.data.inserted.flatMap((value) => {
+    const message = completeMessage(value)
+    return message === undefined ? [] : [message]
+  })
 }
 
 /** Serialize parsed JSON by value rather than insertion order. */
@@ -555,42 +583,48 @@ function canonicalJson(value: unknown): string {
   return JSON.stringify(value)
 }
 
-/** Index each unambiguous identity-free message value by its sole message id. */
-function uniqueMessageIds(logs: readonly string[]): Map<string, string | undefined> {
-  const fingerprintsById = new Map<string, string | undefined>()
+/** Index identity-free message values whose ID and fingerprint are mutually unique. */
+function uniqueMessageIds(logs: readonly string[]): Map<string, string> {
+  const fingerprintsById = new Map<string, Set<string>>()
+  const idsByFingerprint = new Map<string, Set<string>>()
   for (const log of logs) {
     for (const record of parseJsonlRecords(log)) {
-      const message = eventMessage(record)
-      if (message === undefined) continue
-      const { id, ...withoutId } = message
-      const messageId = id as string
-      const fingerprint = canonicalJson(withoutId)
-      if (!fingerprintsById.has(messageId)) fingerprintsById.set(messageId, fingerprint)
-      else if (fingerprintsById.get(messageId) !== fingerprint) fingerprintsById.set(messageId, undefined)
+      for (const message of recordMessages(record)) {
+        const { id, ...withoutId } = message
+        const messageId = id as string
+        const fingerprint = canonicalJson(withoutId)
+        const fingerprints = fingerprintsById.get(messageId)
+        if (fingerprints === undefined) fingerprintsById.set(messageId, new Set([fingerprint]))
+        else fingerprints.add(fingerprint)
+        const ids = idsByFingerprint.get(fingerprint)
+        if (ids === undefined) idsByFingerprint.set(fingerprint, new Set([messageId]))
+        else ids.add(messageId)
+      }
     }
   }
 
-  const idsByFingerprint = new Map<string, string | undefined>()
-  for (const [id, fingerprint] of fingerprintsById) {
-    if (fingerprint === undefined) continue
-    if (!idsByFingerprint.has(fingerprint)) idsByFingerprint.set(fingerprint, id)
-    else idsByFingerprint.set(fingerprint, undefined)
+  const unique = new Map<string, string>()
+  for (const [id, fingerprints] of fingerprintsById) {
+    if (fingerprints.size !== 1) continue
+    const fingerprint = fingerprints.values().next().value as string
+    if (idsByFingerprint.get(fingerprint)?.size !== 1) continue
+    unique.set(fingerprint, id)
   }
-  return idsByFingerprint
+  return unique
 }
 
 /**
  * Match unchanged complete messages across a scenario's fresh and existing logs.
- * New, changed, repeated, or otherwise ambiguous messages keep their fresh ids.
+ * New, changed, duplicate-content, or otherwise ambiguous messages keep their fresh ids.
  */
-function fixtureMessageIdReplacements(logs: readonly string[], fixtures: readonly string[]): FixtureReplacement[] {
+function fixtureMessageIdReplacements(logs: readonly string[], fixtures: readonly string[]): Map<string, string> {
   const freshIds = uniqueMessageIds(logs)
   const existingIds = uniqueMessageIds(fixtures)
-  const replacements: FixtureReplacement[] = []
+  const replacements = new Map<string, string>()
   for (const [fingerprint, fresh] of freshIds) {
     const existing = existingIds.get(fingerprint)
-    if (fresh === undefined || existing === undefined || fresh === existing) continue
-    replacements.push({ from: fresh, to: existing })
+    if (existing === undefined || fresh === existing) continue
+    replacements.set(fresh, existing)
   }
   return replacements
 }
@@ -602,6 +636,22 @@ function applyFixtureReplacements(content: string, replacements: readonly Fixtur
   return stable
 }
 
+/** Rewrite only validated durable-message ID fields, leaving every other occurrence untouched. */
+function applyFixtureMessageIds(content: string, replacements: ReadonlyMap<string, string>): string {
+  return content.split('\n').map((line) => {
+    if (line.trim().length === 0) return line
+    const record = JSON.parse(line) as Record<string, unknown>
+    let changed = false
+    for (const message of recordMessages(record)) {
+      const replacement = replacements.get(message.id as string)
+      if (replacement === undefined) continue
+      message.id = replacement
+      changed = true
+    }
+    return changed ? JSON.stringify(record) : line
+  }).join('\n')
+}
+
 /**
  * Carry committed UUIDs into unchanged, unambiguous messages in fresh session fixtures.
  *
@@ -611,7 +661,7 @@ function applyFixtureReplacements(content: string, replacements: readonly Fixtur
  */
 export function stabilizeFixtureMessageIds(logs: readonly string[], fixtures: readonly string[]): string[] {
   const replacements = fixtureMessageIdReplacements(logs, fixtures)
-  return logs.map(log => applyFixtureReplacements(log, replacements))
+  return logs.map(log => applyFixtureMessageIds(log, replacements))
 }
 
 /** One packed row's member times, or `undefined` for an ordinary record. */
@@ -659,15 +709,15 @@ export function unknownToolCallIds(rawLog: string): string[] {
 }
 
 /**
- * Build refresh write-back replacements: scenario-wide unchanged message ids,
- * plus per-log session ids, cwd values, and spill paths.
+ * Build refresh write-back replacements for per-log session ids, cwd values,
+ * and spill paths. Durable message ids have a later structural owner.
  *
  * @param logs The freshly harvested logs, in fixture order.
  * @param fixtures The existing fixture contents, in matching order.
  * @returns Literal replacements from fresh values to the fixture's existing values.
  */
 export function refreshFixtureReplacements(logs: HarvestedLog[], fixtures: string[]): FixtureReplacement[] {
-  const replacements = fixtureMessageIdReplacements(logs.map(log => log.content), fixtures)
+  const replacements: FixtureReplacement[] = []
   for (let i = 0; i < logs.length; i++) {
     const fresh = parseJsonlRecords((logs[i] as HarvestedLog).content)[0]
     const existing = parseJsonlRecords(fixtures[i] ?? '')[0]
@@ -818,6 +868,7 @@ function collectNormalizedStringMappings(
   existing: unknown,
   normalizedFresh: unknown,
   normalizedExisting: unknown,
+  excludedStrings: ReadonlySet<string>,
   forward: Map<string, string>,
   reverse: Map<string, string>,
 ): boolean {
@@ -837,6 +888,7 @@ function collectNormalizedStringMappings(
       existing[index],
       normalizedFresh[index],
       normalizedExisting[index],
+      excludedStrings,
       forward,
       reverse,
     ))
@@ -856,6 +908,7 @@ function collectNormalizedStringMappings(
         existing[key],
         normalizedFresh[key],
         normalizedExisting[key],
+        excludedStrings,
         forward,
         reverse,
       ))
@@ -866,6 +919,8 @@ function collectNormalizedStringMappings(
     || typeof normalizedFresh !== 'string'
     || normalizedFresh !== normalizedExisting
     || fresh === existing
+    || excludedStrings.has(fresh)
+    || excludedStrings.has(existing)
   ) return true
   const freshKey = JSON.stringify([normalizedFresh, fresh])
   const existingKey = JSON.stringify([normalizedFresh, existing])
@@ -891,6 +946,10 @@ function normalizedStringMappings(
   freshContext: NormalizeContext,
   existingContext: NormalizeContext,
 ): Map<string, string> | undefined {
+  const excludedStrings = new Set<string>()
+  for (const record of [...freshRecords, ...existingRecords]) {
+    for (const message of recordMessages(record)) excludedStrings.add(message.id as string)
+  }
   const forward = new Map<string, string>()
   const reverse = new Map<string, string>()
   let existingIndex = 0
@@ -912,6 +971,7 @@ function normalizedStringMappings(
         existingRecord,
         normalizedRefreshRecord(freshRecords[recordIndex] as Record<string, unknown>, freshContext),
         normalizedRefreshRecord(existingRecord, existingContext),
+        excludedStrings,
         forward,
         reverse,
       )) return undefined
@@ -924,11 +984,13 @@ function normalizedStringMappings(
 /**
  * Rewrite a fresh replay-produced log so repeated refreshes do not churn
  * volatile fixture fields. Meaningful event payloads come from `fresh`; the
- * existing fixture lends normalized-equivalent values, including ids, paths,
+ * existing fixture lends normalized-equivalent values, including non-message ids, paths,
  * creation/event times, spill locators, and hook durations, only when the
  * complete record layout aligns and volatile strings form a consistent
- * bijection. Ambiguous layouts or mappings keep fresh strings. Packed timing
- * envelopes expand for alignment, so packing does not shift later records;
+ * bijection. Complete durable-message ids are excluded because the later
+ * fixture-ready structural pass owns them. Ambiguous layouts or mappings
+ * keep fresh strings. Packed timing envelopes expand for alignment, so
+ * packing does not shift later records;
  * fresh semantic values and fragment arrays remain authoritative.
  *
  * @param fresh The newly harvested session JSONL.
@@ -1158,17 +1220,15 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void {
           const refreshReplacements = REFRESHING
             ? refreshFixtureReplacements(result.sessionLogs, existingFixtures)
             : []
-          const outputFixtures = REFRESHING
+          const freshFixtures = REFRESHING
             ? result.sessionLogs.map((log, index) => scrub(portableFixture(stabilizeRefreshLog(
               log.content,
               existingFixtures[index] as string,
               refreshReplacements,
               ctx,
             ))))
-            : stabilizeFixtureMessageIds(
-              result.sessionLogs.map(log => scrub(portableFixture(log.content))),
-              existingFixtures,
-            )
+            : result.sessionLogs.map(log => scrub(portableFixture(log.content)))
+          const outputFixtures = stabilizeFixtureMessageIds(freshFixtures, existingFixtures)
           await Promise.all(outputFixtures.map((fixture, index) =>
             writeFile(join(dir, outputFixtureFiles[index] as string), fixture)))
           if (RECORDING) {