diff --git a/scripts/run-tests.mjs b/scripts/run-tests.mjs index ce0c8ff..4ae027f 100644 --- a/scripts/run-tests.mjs +++ b/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', diff --git a/src/domain/canonical-json.ts b/src/domain/canonical-json.ts new file mode 100644 index 0000000..59ab7fc --- /dev/null +++ b/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): 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())) +} diff --git a/src/domain/canonical.ts b/src/domain/canonical.ts index 22a386b..96a8196 100644 --- a/src/domain/canonical.ts +++ b/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): 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')) } diff --git a/src/views/headless/rpc-types.ts b/src/views/headless/rpc-types.ts index 16038cb..bcfa8a9 100644 --- a/src/views/headless/rpc-types.ts +++ b/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 diff --git a/src/views/headless/rpc.ts b/src/views/headless/rpc.ts index bd97555..5adcab2 100644 --- a/src/views/headless/rpc.ts +++ b/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 => { diff --git a/src/views/shared/canonical.ts b/src/views/shared/canonical.ts index 9505f24..524216f 100644 --- a/src/views/shared/canonical.ts +++ b/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) } diff --git a/test/canonical.test.ts b/test/canonical.test.ts new file mode 100644 index 0000000..3cfa14e --- /dev/null +++ b/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 = { 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)) +}) diff --git a/test/scripts.test.ts b/test/scripts.test.ts index a3a66de..5dda38e 100644 --- a/test/scripts.test.ts +++ b/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',