fix(canonical): stop the view layer calling an unhashed canonical string a digest

scripts/run-tests.mjs

@@ -36,6 +36,7 @@ if (scopeIndex !== -1 && !scope) {
 const scopeFiles = {
   unit: [
     'agent-interface-runtime-parity.test.js',
+    'canonical.test.js',
     'analysis-model-call-observability.test.js',
     'analysis-model-call-roundtrip.test.js',
     'application.test.js',

src/domain/canonical-json.ts

@@ -0,0 +1,53 @@
+function compareCodeUnits(left: string, right: string): number {
+  return left < right ? -1 : left > right ? 1 : 0
+}
+
+function canonicalValue(value: unknown, ancestors: Set<object>): unknown {
+  if (value === null || typeof value === 'string' || typeof value === 'boolean') return value
+  if (typeof value === 'number') {
+    if (!Number.isFinite(value)) throw new TypeError('Canonical JSON requires finite numbers')
+    return Object.is(value, -0) ? 0 : value
+  }
+  if (typeof value !== 'object') {
+    throw new TypeError(`Canonical JSON cannot represent ${typeof value}`)
+  }
+  if (ancestors.has(value)) throw new TypeError('Canonical JSON cannot represent cycles')
+  ancestors.add(value)
+  try {
+    if (Array.isArray(value)) {
+      return value.map((child) => {
+        if (child === undefined) {
+          throw new TypeError('Canonical JSON cannot represent undefined array entries')
+        }
+        return canonicalValue(child, ancestors)
+      })
+    }
+    const prototype = Object.getPrototypeOf(value)
+    if (prototype !== Object.prototype && prototype !== null) {
+      throw new TypeError('Canonical JSON requires plain objects')
+    }
+    return Object.fromEntries(
+      Object.entries(value)
+        .filter(([, child]) => child !== undefined)
+        .sort(([left], [right]) => compareCodeUnits(left, right))
+        .map(([key, child]) => [key, canonicalValue(child, ancestors)]),
+    )
+  } finally {
+    ancestors.delete(value)
+  }
+}
+
+/**
+ * The canonical text of a value: object keys ordered by code unit, `undefined`
+ * members dropped, and every value that has no faithful JSON form refused.
+ *
+ * Two values that mean different things must not produce the same text, so a
+ * non-finite number, a cycle, a class instance, and a bare `undefined` are
+ * refused rather than serialized as `null` or as their own enumerable fields.
+ * This module reaches for nothing outside the language, so the view layer can
+ * use it under the boundary rule that forbids `node:` imports there.
+ */
+export function canonicalJson(value: unknown): string {
+  if (value === undefined) throw new TypeError('Canonical JSON cannot represent undefined')
+  return JSON.stringify(canonicalValue(value, new Set()))
+}

src/domain/canonical.ts

@@ -1,50 +1,10 @@
 import { createHash } from 'node:crypto'
+import { canonicalJson } from './canonical-json.js'
 import { createDigest, type Digest } from './ids.js'
 
-function compareCodeUnits(left: string, right: string): number {
-  return left < right ? -1 : left > right ? 1 : 0
-}
-
-function canonicalValue(value: unknown, ancestors: Set<object>): unknown {
-  if (value === null || typeof value === 'string' || typeof value === 'boolean') return value
-  if (typeof value === 'number') {
-    if (!Number.isFinite(value)) throw new TypeError('Canonical JSON requires finite numbers')
-    return Object.is(value, -0) ? 0 : value
-  }
-  if (typeof value !== 'object') {
-    throw new TypeError(`Canonical JSON cannot represent ${typeof value}`)
-  }
-  if (ancestors.has(value)) throw new TypeError('Canonical JSON cannot represent cycles')
-  ancestors.add(value)
-  try {
-    if (Array.isArray(value)) {
-      return value.map((child) => {
-        if (child === undefined) {
-          throw new TypeError('Canonical JSON cannot represent undefined array entries')
-        }
-        return canonicalValue(child, ancestors)
-      })
-    }
-    const prototype = Object.getPrototypeOf(value)
-    if (prototype !== Object.prototype && prototype !== null) {
-      throw new TypeError('Canonical JSON requires plain objects')
-    }
-    return Object.fromEntries(
-      Object.entries(value)
-        .filter(([, child]) => child !== undefined)
-        .sort(([left], [right]) => compareCodeUnits(left, right))
-        .map(([key, child]) => [key, canonicalValue(child, ancestors)]),
-    )
-  } finally {
-    ancestors.delete(value)
-  }
-}
-
-export function canonicalJson(value: unknown): string {
-  if (value === undefined) throw new TypeError('Canonical JSON cannot represent undefined')
-  return JSON.stringify(canonicalValue(value, new Set()))
-}
+export { canonicalJson }
 
+/** The SHA-256 of a value's canonical text. */
 export function canonicalDigest(value: unknown): Digest {
   return createDigest(createHash('sha256').update(canonicalJson(value)).digest('hex'))
 }

src/views/headless/rpc-types.ts

@@ -9,7 +9,8 @@ export const RPC_REPLAY_MAX_ENTRIES = 256
 export const RPC_REPLAY_MAX_BYTES = 8 * 1024 * 1024
 
 export interface RequestRecord {
-  readonly digest: string
+  /** Canonical text of the request this identifier was first used with. */
+  readonly identity: string
   readonly responses: string[]
   bytes: number
   replayable: boolean

src/views/headless/rpc.ts

@@ -1,5 +1,5 @@
 import { boundedDrain } from '../../app/application-lifecycle.js'
-import { canonicalDigest } from '../shared/canonical.js'
+import { canonicalRequestIdentity } from '../shared/canonical.js'
 import type { BraidUiController, UiEvent } from '../shared/intents.js'
 import { redactSensitiveText, sanitizeTerminalText } from '../shared/sanitize.js'
 import { BoundedOutputQueue } from './bounded-output.js'
@@ -222,10 +222,10 @@ export async function runRpc(
       let requestRecord: RequestRecord | undefined
       try {
         const request = parseRequest(line)
-        const digest = canonicalDigest(request)
+        const identity = canonicalRequestIdentity(request)
         const previous = requests.get(request.requestId)
         if (previous) {
-          if (previous.digest !== digest) {
+          if (previous.identity !== identity) {
             await write(
               errorResponse(
                 new RpcParseError(
@@ -250,7 +250,7 @@ export async function runRpc(
           }
           continue
         }
-        requestRecord = { digest, responses: [], bytes: 0, replayable: true }
+        requestRecord = { identity, responses: [], bytes: 0, replayable: true }
         requests.set(request.requestId, requestRecord)
         trimReplayHistory()
         const respond = async (response: BraidResponse): Promise<void> => {

src/views/shared/canonical.ts

@@ -1,24 +1,16 @@
-function canonicalValue(value: unknown): unknown {
-  if (Array.isArray(value)) return value.map(canonicalValue)
-  if (value === null || typeof value !== 'object') return value
-
-  return Object.fromEntries(
-    Object.entries(value)
-      .filter(([, child]) => child !== undefined)
-      .sort(([left], [right]) => (left < right ? -1 : left > right ? 1 : 0))
-      .map(([key, child]) => [key, canonicalValue(child)]),
-  )
-}
+export { canonicalJson } from '../../domain/canonical-json.js'
+import { canonicalJson } from '../../domain/canonical-json.js'
 
 /**
- * Canonicalizes a protocol value for stable request identity.
- * The protocol layer owns this small value-only helper so views do not import
- * application or domain modules merely to detect duplicate JSONL requests.
+ * The identity of one protocol request, for recognizing a request identifier
+ * that arrives a second time carrying different input.
+ *
+ * This is the request's canonical text, not a digest of it. Two requests are
+ * the same request when their canonical texts are equal, and comparing the
+ * text needs no hash — which matters here, because the view layer may not
+ * import `node:crypto`. Anything that needs a fixed-width value should take
+ * `canonicalDigest` from the domain layer instead.
  */
-export function canonicalJson(value: unknown): string {
-  return JSON.stringify(canonicalValue(value))
-}
-
-export function canonicalDigest(value: unknown): string {
+export function canonicalRequestIdentity(value: unknown): string {
   return canonicalJson(value)
 }

test/canonical.test.ts

@@ -0,0 +1,47 @@
+import assert from 'node:assert/strict'
+import test from 'node:test'
+import { canonicalDigest, canonicalJson } from '../src/domain/canonical.js'
+import { canonicalRequestIdentity } from '../src/views/shared/canonical.js'
+
+test('request identity does not depend on the order members were written in', () => {
+  assert.equal(
+    canonicalRequestIdentity({ version: 1, method: 'state', requestId: 'req-1' }),
+    canonicalRequestIdentity({ requestId: 'req-1', method: 'state', version: 1 }),
+  )
+})
+
+test('request identity refuses a value with no faithful JSON form', () => {
+  // Each of these once produced an identity, and the first two produced the
+  // SAME identity as `null` - so a request identifier reused with different
+  // input read as a replay of the first request rather than a conflict.
+  for (const [label, value] of [
+    ['not a number', { params: { limit: Number.NaN } }],
+    ['infinite', { params: { limit: Number.POSITIVE_INFINITY } }],
+    [
+      'a class instance',
+      {
+        params: new (class Params {
+          limit = 1
+        })(),
+      },
+    ],
+    [
+      'a cycle',
+      (() => {
+        const request: Record<string, unknown> = { method: 'state' }
+        request.self = request
+        return request
+      })(),
+    ],
+    ['nothing', undefined],
+  ] as const) {
+    assert.throws(() => canonicalRequestIdentity(value), TypeError, label)
+  }
+})
+
+test('a request identity is the canonical text, and a domain digest is a hash of it', () => {
+  const request = { version: 1, method: 'state', requestId: 'req-1' }
+  assert.equal(canonicalRequestIdentity(request), canonicalJson(request))
+  assert.match(canonicalDigest(request), /^[0-9a-f]{64}$/)
+  assert.notEqual(canonicalDigest(request), canonicalRequestIdentity(request))
+})

test/scripts.test.ts

@@ -206,6 +206,7 @@ test('every scoped package alias forwards its declared file set', () => {
       'analysis-model-call-observability.test.js',
       'analysis-model-call-roundtrip.test.js',
       'application.test.js',
+      'canonical.test.js',
       'cli-startup.test.js',
       'conversations.test.js',
       'coordination.test.js',