From 426bd983c946563bc70abbbb95daaa3dae1385af Mon Sep 17 00:00:00 2001 From: James Date: Mon, 11 May 2026 18:59:07 +0000 Subject: [PATCH] refactor(producer): address review feedback on planHash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `PLAN_HASH_SCHEMA_PREFIX = "hyperframes-plan-hash-v1\x00"` mixed into every digest. Future framing changes must bump the trailing integer; this makes cross-version mismatches visible at the wire format instead of producing silent collisions. Impossible to backfill later, easy to bake in now. - Hoist the `0x00` field delimiter to module scope (`FIELD_DELIMITER`). - Document the UTF-8 encoding contract for all string-typed input fields in the file-level JSDoc, so external verifiers know the encoding without reading the implementation. - Add a known-digest test for one fixed reference input (`995b4105...`). If the framing changes silently this test fails, forcing the developer to also bump the schema prefix. - Add an explicit `canonicalJsonStringify(undefined) → TypeError` test to pin the contract. No callers yet, so no behavior change in any code path. Review feedback addressed: vanceingalls on #717. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../services/render/stages/planHash.test.ts | 14 ++++++ .../src/services/render/stages/planHash.ts | 47 ++++++++++++++----- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/packages/producer/src/services/render/stages/planHash.test.ts b/packages/producer/src/services/render/stages/planHash.test.ts index f010676d92..2ecd5913f4 100644 --- a/packages/producer/src/services/render/stages/planHash.test.ts +++ b/packages/producer/src/services/render/stages/planHash.test.ts @@ -41,6 +41,16 @@ describe("computePlanHash", () => { expect(a).toMatch(/^[0-9a-f]{64}$/); }); + // Pin the schema-prefix-mixed-in result for one fixed input. If the + // framing of `computePlanHash` ever changes silently, this test must + // be updated (which means bumping `PLAN_HASH_SCHEMA_PREFIX` in the + // source too). Catches accidental drift across producer versions. + it("matches the known digest for a fixed reference input", () => { + expect(computePlanHash(makeInput())).toBe( + "995b4105a1a629965e85dc5d92c6aab9b888e39acdb14369d1bc781aa3247a94", + ); + }); + it("ignores asset order in the input array", () => { const ordered = computePlanHash( makeInput({ @@ -206,6 +216,10 @@ describe("canonicalJsonStringify", () => { expect(() => canonicalJsonStringify(Symbol("s"))).toThrow(TypeError); }); + it("rejects undefined at the top level", () => { + expect(() => canonicalJsonStringify(undefined)).toThrow(TypeError); + }); + it("produces equal output for semantically equal objects with different key order", () => { const a = canonicalJsonStringify({ encoder: "libx264-software", diff --git a/packages/producer/src/services/render/stages/planHash.ts b/packages/producer/src/services/render/stages/planHash.ts index e6d1b8eba1..effa2ea503 100644 --- a/packages/producer/src/services/render/stages/planHash.ts +++ b/packages/producer/src/services/render/stages/planHash.ts @@ -4,7 +4,8 @@ * See DISTRIBUTED-RENDERING-PLAN.md §4.2 for the contract: * * planHash = sha256( - * composition_html_bytes + * SCHEMA_PREFIX + * ⊕ composition_html_bytes * ⊕ asset_shas (sorted by relative path) * ⊕ font_snapshot_sha * ⊕ encoder_config_canonical_json @@ -17,12 +18,35 @@ * use this to short-circuit `plan()` on workflow replay and to detect * cross-version mismatches (§9.3 PLAN_HASH_MISMATCH). * - * Phase 1 ships the pure hashing utility only; no caller exists yet. The - * `services/distributed/plan.ts` PR in Phase 3 will compose it. + * Pure utility; no caller exists yet — the distributed-render + * `services/distributed/plan.ts` will compose it. + * + * ## Encoding contract + * + * Every string-typed component (`fontSnapshotSha`, + * `encoderConfigCanonicalJson`, `producerVersion`, `ffmpegVersion`, asset + * paths and shas, the dimensions tuple) is hashed as UTF-8. External + * verifiers must encode the same way. Binary fields (`compositionHtml`) + * are hashed verbatim. */ import { createHash } from "node:crypto"; +/** + * Schema-version prefix mixed into every digest. Bump the trailing version + * integer whenever the framing of `computePlanHash` changes (new fields, + * new delimiter, new field order, etc.) so every cached plan from an older + * producer is forced to mismatch and re-plan. This is impossible to + * backfill, so a deliberate bump is the only correct action. + */ +const PLAN_HASH_SCHEMA_PREFIX = "hyperframes-plan-hash-v1\x00"; + +/** + * 0x00 byte used to frame each `hash.update()` call. Hoisted to module + * scope so it's not reallocated on every `computePlanHash` invocation. + */ +const FIELD_DELIMITER = Buffer.from([0x00]); + /** * SHA-256 hex digest of an asset, paired with its plan-relative path. Sort * order across an asset list is by `path` (byte-wise ascending) to keep the @@ -75,29 +99,30 @@ export interface PlanHashInput { */ export function computePlanHash(input: PlanHashInput): string { const hash = createHash("sha256"); - const delimiter = Buffer.from([0x00]); + + hash.update(PLAN_HASH_SCHEMA_PREFIX, "utf8"); hash.update(input.compositionHtml); - hash.update(delimiter); + hash.update(FIELD_DELIMITER); const sortedAssets = [...input.assets].sort((a, b) => a.path < b.path ? -1 : a.path > b.path ? 1 : 0, ); for (const asset of sortedAssets) { hash.update(asset.path, "utf8"); - hash.update(delimiter); + hash.update(FIELD_DELIMITER); hash.update(asset.sha256, "utf8"); - hash.update(delimiter); + hash.update(FIELD_DELIMITER); } hash.update(input.fontSnapshotSha, "utf8"); - hash.update(delimiter); + hash.update(FIELD_DELIMITER); hash.update(input.encoderConfigCanonicalJson, "utf8"); - hash.update(delimiter); + hash.update(FIELD_DELIMITER); hash.update(input.producerVersion, "utf8"); - hash.update(delimiter); + hash.update(FIELD_DELIMITER); hash.update(input.ffmpegVersion, "utf8"); - hash.update(delimiter); + hash.update(FIELD_DELIMITER); const d = input.dimensions; hash.update(`${d.fpsNum}/${d.fpsDen}x${d.width}x${d.height}x${d.format}`, "utf8");