From 2aaafe7591eadb97351e67e03f41a3999e091df7 Mon Sep 17 00:00:00 2001 From: Sutu Sebastian Date: Sun, 19 Jul 2026 12:40:03 +0300 Subject: [PATCH 1/2] fix(core): reject non-JSON-safe layer keys with LayerKeyError Fail closed before hashKey so undefined/NaN/null no longer collide via JSON.stringify, and document the JSON-safe key domain. --- .changeset/json-safe-layer-keys.md | 5 + apps/docs/content/adapters/core.mdx | 2 +- apps/docs/content/concepts/glossary.mdx | 2 +- .../content/concepts/identity-and-types.mdx | 18 ++- apps/docs/content/guides/error-handling.mdx | 4 + apps/docs/content/reference/core-api.mdx | 14 +++ docs/architecture.md | 2 +- docs/glossary.md | 2 +- packages/core/skills/layers/SKILL.md | 5 +- packages/core/src/errors.ts | 30 +++++ packages/core/src/index.ts | 14 ++- packages/core/src/layerStack.test.ts | 64 ++++++++++- packages/core/src/types.ts | 9 +- packages/core/src/utils.ts | 104 +++++++++++++++++- 14 files changed, 262 insertions(+), 13 deletions(-) create mode 100644 .changeset/json-safe-layer-keys.md diff --git a/.changeset/json-safe-layer-keys.md b/.changeset/json-safe-layer-keys.md new file mode 100644 index 0000000..0fcefe0 --- /dev/null +++ b/.changeset/json-safe-layer-keys.md @@ -0,0 +1,5 @@ +--- +"@stainless-code/layers": patch +--- + +Reject non-JSON-safe layer key segments with `LayerKeyError` before `hashKey`. Previously `JSON.stringify` could collide `[undefined]` / `[null]` / `[NaN]` as `"[null]"`. diff --git a/apps/docs/content/adapters/core.mdx b/apps/docs/content/adapters/core.mdx index 6d1d771..4a52adf 100644 --- a/apps/docs/content/adapters/core.mdx +++ b/apps/docs/content/adapters/core.mdx @@ -30,7 +30,7 @@ npm i @stainless-code/layers | `notifyManager` | `{ batch, batchCalls }` — coalesce notifications inside a tick | | `Register` | Module augmentation interface for app-wide `DefaultLayerError` | -The core also exports `Layer`, `Subscribable`, `ControlledPromise`, `PayloadValidationError`, `isPayloadValidationError`, `hashKey`, `keySignature`, `childStackId`, and the full type surface (`LayerState`, `LayerPhase`, `LayerCallContext`, `StackOptions`, …). See [Core API](/reference/core-api). +The core also exports `Layer`, `Subscribable`, `ControlledPromise`, `PayloadValidationError`, `isPayloadValidationError`, `LayerKeyError`, `isLayerKeyError`, `assertLayerKey`, `hashKey`, `keySignature`, `childStackId`, and the full type surface (`LayerState`, `LayerPhase`, `LayerCallContext`, `StackOptions`, …). See [Core API](/reference/core-api). ## Headless quick start diff --git a/apps/docs/content/concepts/glossary.mdx b/apps/docs/content/concepts/glossary.mdx index 91e163d..028ba61 100644 --- a/apps/docs/content/concepts/glossary.mdx +++ b/apps/docs/content/concepts/glossary.mdx @@ -17,7 +17,7 @@ From `LayerClient` down to `keySignature` — every term in code, docs, and issu - **StackNotifyEvent** — JSON-safe record of one observable stack transition (`action` + `active`/`queued` views + optional projected `payload`). Emitted by core; Devtools bridges to TanStack `EventClient`. - **subscribeNotify** — `LayerClient`: `(listener) => unsubscribe` for `StackNotifyEvent`s. Distinct from `subscribe` / `subscribeStacks` (snapshot / stack-created). - **seedNotify** — `LayerClient`: re-emit a `register` `StackNotifyEvent` for one stack, or all materialized stacks when omitted (late-subscriber / Devtools attach seed). - - **Key** — the **logical** identity of a layer (`find`/`upsert`/`gcTime` operate on `keySignature(key)`); multiple live layers may share a key in a `parallel` stack. + - **Key** — the **logical** identity of a layer (`find`/`upsert`/`gcTime` operate on `keySignature(key)`). Must be JSON-safe (`string` \| `boolean` \| `null` \| finite `number` \| plain objects/arrays of those); invalid segments throw `LayerKeyError`. Multiple live layers may share a key in a `parallel` stack. - **Instance id** (`LayerState.id`) — the **physical**, unique id of one `Layer` instance (`` `${hashKey(key)}#n` ``); used for rendering keys and instance lookup/removal (`getLayer`, `#remove`). - **Call context** (`LayerCallContext`) — imperative handle handed to a layer component: `end`/`dismiss` (async — resolve the caller's `await`; return `Promise`), `addBlocker`, `update` (patch payload live), `setRunning` (drives `actionStatus`), `settle` (drives `transition`), `ended`, `index`, `stackSize`, `root`, `stackId`, `layerId`. Built by `createCallContext`. - **Layer group** — a child stack owned by a parent layer and tied to its lifetime: created via `createLayerGroup` / adapter `useLayerGroup`; when the parent dismisses, the group's stack auto-drains. Same `LayerClient`, no second client. diff --git a/apps/docs/content/concepts/identity-and-types.mdx b/apps/docs/content/concepts/identity-and-types.mdx index 7b70369..91f21ee 100644 --- a/apps/docs/content/concepts/identity-and-types.mdx +++ b/apps/docs/content/concepts/identity-and-types.mdx @@ -11,13 +11,29 @@ Two opens with the same `key` share **logical identity**; each still gets a uniq | Concept | Role | | ------- | ---- | -| **Key** | Logical identity. `find`, `upsert`, and `gcTime` operate on `keySignature(key)`. Multiple live layers may share a key in a `parallel` stack. | +| **Key** | Logical identity. Must be **JSON-safe** (see below). `find`, `upsert`, and `gcTime` operate on `keySignature(key)`. Multiple live layers may share a key in a `parallel` stack. | | **Instance id** (`LayerState.id`) | Physical, unique id of one `Layer` instance (`` `${hashKey(key)}#n` ``). Used for rendering keys and instance lookup/removal (`getLayer`, `#remove`). | :::note[`find` returns the topmost match] `LayerStack.find(key)` walks the stack and returns the **last** layer with that key signature — the topmost instance when several share a key in a `parallel` stack. Wired handles route `dismiss`/`update` without `{ id }` through this same lookup. ::: +## JSON-safe key domain + +`hashKey` / `keySignature` serialize with sorted-object `JSON.stringify`. Keys must stay inside that domain or `open` / `find` / `cancelQueued` throw `LayerKeyError` synchronously (narrow with `isLayerKeyError`). + +| Allowed | Rejected | +| ------- | -------- | +| `string`, `boolean`, `null`, finite `number` | `undefined` (anywhere, including object props) | +| Plain objects / arrays of the above | `NaN`, `±Infinity`, `bigint`, `symbol`, functions | +| Property order in plain objects (sorted for equality) | Non-plain objects (`Date`, class instances), cycles | + +Prefer `null` or an explicit sentinel (`filterId ?? "none"`) over `undefined`. Omit optional object fields instead of setting them to `undefined`. + +:::note[Earlier collision behavior] +Before this assert, `JSON.stringify` made `[undefined]`, `[null]`, and `[NaN]` collide as `"[null]"`; object `undefined` props were omitted; BigInt/cycles threw opaque `TypeError`s from `JSON.stringify`. Invalid segments now fail closed as `LayerKeyError` before hash. +::: + ## Wired handle path Bind a declaration with `useLayer(options)` (adapter) or `createLayer(options, client)` (headless). Payload-only `open`/`upsert` infer `R` from the same `DataTag` on `options.key`: diff --git a/apps/docs/content/guides/error-handling.mdx b/apps/docs/content/guides/error-handling.mdx index fc9e251..8666d67 100644 --- a/apps/docs/content/guides/error-handling.mdx +++ b/apps/docs/content/guides/error-handling.mdx @@ -7,6 +7,10 @@ search: `open()` returns `Promise` — the direct-`await` contract. Resolution types flow through `DataTag`; **rejections do not** (a TypeScript limitation). Narrow errors in `catch` with runtime guards. +## Sync throws vs Promise rejects + +A non-JSON-safe `key` throws `LayerKeyError` **synchronously** from `open` / `find` / `cancelQueued` (before a promise is returned). Narrow with `isLayerKeyError`. That path is distinct from the Promise rejection cases below — wrap the `open` call itself if keys may be untrusted. + ## What rejects The promise rejects when: diff --git a/apps/docs/content/reference/core-api.mdx b/apps/docs/content/reference/core-api.mdx index 376b062..c39a013 100644 --- a/apps/docs/content/reference/core-api.mdx +++ b/apps/docs/content/reference/core-api.mdx @@ -173,6 +173,20 @@ function isPayloadValidationError( ): value is PayloadValidationError; ``` +## Layer key errors + +Non-JSON-safe keys throw synchronously from `hashKey` / `open` / `find` / `cancelQueued` (not a Promise rejection). See [Identity & types](/concepts/identity-and-types). + +```ts +class LayerKeyError extends Error { + readonly path: ReadonlyArray; +} + +function isLayerKeyError(value: unknown): value is LayerKeyError; + +function assertLayerKey(key: unknown): asserts key is LayerKey; +``` + ## createLayer Wire `layerOptions` + a `LayerClient` into a headless handle. Adapters wrap this in reactive `useLayer` / `injectLayer` / `createLayer`. diff --git a/docs/architecture.md b/docs/architecture.md index 30ccea3..014f7f3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -103,7 +103,7 @@ Delay `0` (default) flips the axis synchronously — no transition frame observe Layers are client-only today (React/Preact `getServerSnapshot` returns `[]`); when SSR hydration lands, initial `transition` must be `"settled"`. -**Identity:** each layer has a unique **instance id** (`LayerState.id`, `` `${hashKey(key)}#n` ``) for rendering keys and instance lookup/removal. The **key** is the logical identity (`find`/`upsert`/`gcTime` use `keySignature(key)`); multiple live layers may share a key in a `parallel` stack. +**Identity:** each layer has a unique **instance id** (`LayerState.id`, `` `${hashKey(key)}#n` ``) for rendering keys and instance lookup/removal. The **key** is the logical identity (`find`/`upsert`/`gcTime` use `keySignature(key)`); keys must be JSON-safe or `hashKey` throws `LayerKeyError`. Multiple live layers may share a key in a `parallel` stack. ## Blockers diff --git a/docs/glossary.md b/docs/glossary.md index 804ea78..026f6ec 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -9,7 +9,7 @@ Ubiquitous language for the `@stainless-code/layers` domain. Keep terms stable a - **StackNotifyEvent** — JSON-safe record of one observable stack transition (`action` + `active`/`queued` views + optional projected `payload`). Emitted by core; Devtools bridges to TanStack `EventClient`. - **subscribeNotify** — `LayerClient`: `(listener) => unsubscribe` for `StackNotifyEvent`s. Distinct from `subscribe` / `subscribeStacks` (snapshot / stack-created). - **seedNotify** — `LayerClient`: re-emit a `register` `StackNotifyEvent` for one stack, or all when omitted (Devtools attach / late subscribers). -- **Key** — the **logical** identity of a layer (`find`/`upsert`/`gcTime` operate on `keySignature(key)`); multiple live layers may share a key in a `parallel` stack. +- **Key** — the **logical** identity of a layer (`find`/`upsert`/`gcTime` operate on `keySignature(key)`). Must be JSON-safe (`string` \| `boolean` \| `null` \| finite `number` \| plain objects/arrays of those); invalid segments throw `LayerKeyError`. Multiple live layers may share a key in a `parallel` stack. - **Instance id** (`LayerState.id`) — the **physical**, unique id of one `Layer` instance (`` `${hashKey(key)}#n` ``); used for rendering keys and instance lookup/removal (`getLayer`, `#remove`). - **Phase** — resolution lifecycle axis: `pending` → `active` → `dismissed`; or `queued` (serial-waiting, not mounted); or `error`. Does **not** include `exiting` — animation is on `transition`. Distinct from `actionStatus` and `transition`. - **Transition** — animation axis (`entering` | `settled` | `exiting`), orthogonal to `phase` (like `actionStatus`). `dismissed + exiting` = playing exit anim; `dismissed + settled` = cached/done. diff --git a/packages/core/skills/layers/SKILL.md b/packages/core/skills/layers/SKILL.md index b06fdaa..5ae4eb1 100644 --- a/packages/core/skills/layers/SKILL.md +++ b/packages/core/skills/layers/SKILL.md @@ -151,7 +151,7 @@ const ok = await client.open({ `createCallContext` gives an active layer `end`, `dismiss`, `addBlocker`, `update`, `setRunning`, and `settle`, plus `ended`, `index`, `stackSize`, `root`, `stackId`, and `layerId`. `await call.end(response)` and `await call.dismiss(response)` request dismissal; when allowed, they resolve the caller and begin exit. Both return `Promise`, with `false` meaning a blocker vetoed dismissal. `update` patches payload, `setRunning` controls `actionStatus`, and `settle` completes the current enter or exit transition. -`key` is logical identity for `find`, `upsert`, and `gcTime`; every mount gets a unique instance `id` shaped as `${hashKey(key)}#`. `hashKey` and its alias `keySignature` compare keys structurally. Parallel stacks may contain multiple same-key instances. +`key` is logical identity for `find`, `upsert`, and `gcTime`; every mount gets a unique instance `id` shaped as `${hashKey(key)}#`. Keys must be JSON-safe (`string` | `boolean` | `null` | finite `number` | plain objects/arrays of those); `hashKey` / `keySignature` assert that domain and throw `LayerKeyError` otherwise. Parallel stacks may contain multiple same-key instances. ## Lifecycle @@ -271,11 +271,12 @@ The complete public API surface: | Engine classes | `LayerClient`, `LayerStack`, `Layer` | | Infrastructure classes | `Subscribable`, `ControlledPromise` | | Validation class | `PayloadValidationError` | +| Key errors | `LayerKeyError`, `isLayerKeyError`, `assertLayerKey` | | Declaration and inference | `layerOptions`, `layerKey`, `DataTag`, `InferDataTagResponse`, `InferDataTagError`, `ResponseOf`, `ErrorOf` | | Wired handles | `createLayer`, `LayerHandle`, `ValidatedLayerHandle` | | Rendering seam | `createCallContext`, `LayerCallContext`, `LayerComponentProps`, `LayerComponent` | | Nested layers | `createLayerGroup`, `childStackId`, `LayerGroupOptions`, `LayerGroupHandle` | -| Identity and notification values | `hashKey`, `keySignature`, `notifyManager` | +| Identity and notification values | `assertLayerKey`, `hashKey`, `keySignature`, `notifyManager` | | Validation API | `isPayloadValidationError`, `Validator`, `InferValidatorInput`, `InferValidatorOutput`, `StandardSchemaV1`, `ValidationIssue` | | Layer model types | `LayerKey`, `LayerPhase`, `LayerTransition`, `LayerActionStatus`, `LayerState`, `LayerOptions`, `OpenLayerOptions` | | Stack and client types | `StackOptions`, `StackDefaults`, `LayerClientOptions` | diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 8c5c1ff..3b77b5b 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -36,3 +36,33 @@ export function isPayloadValidationError( ): value is PayloadValidationError { return value instanceof PayloadValidationError; } + +/** Thrown when a layer key is not JSON-safe (see {@link assertLayerKey}). */ +export class LayerKeyError extends Error { + readonly path: ReadonlyArray; + constructor(message: string, path: ReadonlyArray = []) { + super(message); + this.name = "LayerKeyError"; + this.path = path; + } +} + +/** + * Narrows an unknown throw/rejection to {@link LayerKeyError}. + * + * @example + * ```ts + * import { isLayerKeyError } from "@stainless-code/layers"; + * + * try { + * client.open({ key: [maybeId], payload }); + * } catch (error) { + * if (isLayerKeyError(error)) { + * console.error(error.path, error.message); + * } + * } + * ``` + */ +export function isLayerKeyError(value: unknown): value is LayerKeyError { + return value instanceof LayerKeyError; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index bab5c47..640f485 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,5 +1,10 @@ export * from "./types"; -export { hashKey, keySignature, shallowArrayEqual } from "./utils"; +export { + assertLayerKey, + hashKey, + keySignature, + shallowArrayEqual, +} from "./utils"; export { Subscribable } from "./subscribable"; export { notifyManager } from "./notifyManager"; export { ControlledPromise } from "./controlledPromise"; @@ -28,5 +33,10 @@ export type { InferValidatorOutput, OpenValidatePayload, } from "./validators"; -export { PayloadValidationError, isPayloadValidationError } from "./errors"; +export { + PayloadValidationError, + isPayloadValidationError, + LayerKeyError, + isLayerKeyError, +} from "./errors"; export type { ValidationIssue } from "./errors"; diff --git a/packages/core/src/layerStack.test.ts b/packages/core/src/layerStack.test.ts index 7ad9f48..7aad1cf 100644 --- a/packages/core/src/layerStack.test.ts +++ b/packages/core/src/layerStack.test.ts @@ -1,11 +1,21 @@ import { describe, expect, it } from "bun:test"; -import { isPayloadValidationError, PayloadValidationError } from "./errors"; +import { + isLayerKeyError, + isPayloadValidationError, + LayerKeyError, + PayloadValidationError, +} from "./errors"; import { LayerClient } from "./layerClient"; import { layerOptions } from "./layerOptions"; import { LayerStack } from "./layerStack"; import { notifyManager } from "./notifyManager"; -import { hashKey, keySignature, shallowArrayEqual } from "./utils"; +import { + assertLayerKey, + hashKey, + keySignature, + shallowArrayEqual, +} from "./utils"; describe("hashKey / keySignature", () => { it("is stable across key-object key order", () => { @@ -16,6 +26,46 @@ describe("hashKey / keySignature", () => { keySignature(["confirm", "y"]), ); }); + it("accepts JSON-safe primitives, arrays, and plain objects", () => { + expect(() => + assertLayerKey(["modal", 1, true, null, { nested: ["a", 2] }]), + ).not.toThrow(); + expect(hashKey(["modal", 1, true, null])).toBe( + JSON.stringify(["modal", 1, true, null]), + ); + }); + it("rejects undefined, non-finite numbers, bigint, and non-plain objects", () => { + const cases: Array<{ key: unknown; pathHint: string }> = [ + { key: [undefined], pathHint: "key[0]" }, + { key: [{ a: undefined }], pathHint: "key[0].a" }, + { key: [NaN], pathHint: "key[0]" }, + { key: [Infinity], pathHint: "key[0]" }, + { key: [-Infinity], pathHint: "key[0]" }, + { key: [1n], pathHint: "key[0]" }, + { key: [new Date()], pathHint: "key[0]" }, + { key: [Symbol("x")], pathHint: "key[0]" }, + ]; + for (const { key, pathHint } of cases) { + expect(() => hashKey(key as never)).toThrow(LayerKeyError); + try { + hashKey(key as never); + } catch (error) { + expect(isLayerKeyError(error)).toBe(true); + if (isLayerKeyError(error)) { + expect(error.message).toContain(pathHint); + } + } + } + }); + it("rejects cyclic structures", () => { + const cyclic: Record = {}; + cyclic.self = cyclic; + expect(() => hashKey([cyclic])).toThrow(LayerKeyError); + }); + it("null is accepted; undefined throws (no longer collide)", () => { + expect(hashKey([null])).toBe(JSON.stringify([null])); + expect(() => hashKey([undefined])).toThrow(LayerKeyError); + }); }); describe("shallowArrayEqual", () => { @@ -28,6 +78,16 @@ describe("shallowArrayEqual", () => { }); describe("LayerStack — basics", () => { + it("open throws LayerKeyError synchronously for non-JSON-safe keys", () => { + const stack = new LayerStack("s"); + expect(() => + stack.open({ + key: [undefined], + payload: {}, + }), + ).toThrow(LayerKeyError); + }); + it("open pushes, indexes, and goes active without loadFn", () => { const stack = new LayerStack<{ n: number }, void>("s"); stack.open({ key: ["a"], payload: { n: 1 } }); diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 6d3e486..31e729c 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1,6 +1,13 @@ import type { Validator } from "./validators"; -/** Structurally comparable layer identity. */ +/** + * Stable identity for a layer instance. + * + * Must be JSON-safe: `string` | `boolean` | `null` | finite `number`, or + * plain objects / arrays of those. Compared via sorted `JSON.stringify` — + * two keys with the same values in a different object-property order are equal. + * Invalid keys throw {@link LayerKeyError} from {@link hashKey} / `open`. + */ export type LayerKey = ReadonlyArray; export type LayerPhase = diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 621d30a..136b1ed 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -1,7 +1,12 @@ +import { LayerKeyError } from "./errors"; import type { LayerKey } from "./types"; function isPlainObject(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; } function sortedObject(value: Record): Record { @@ -13,8 +18,105 @@ function sortedObject(value: Record): Record { return out; } +function formatKeyPath(path: ReadonlyArray): string { + if (path.length === 0) { + return "key"; + } + let out = "key"; + for (const segment of path) { + if (typeof segment === "number") { + out += `[${segment}]`; + } else { + out += `.${String(segment)}`; + } + } + return out; +} + +function walkLayerKey( + value: unknown, + path: PropertyKey[], + seen: WeakSet, +): void { + if (value === undefined) { + throw new LayerKeyError( + `${formatKeyPath(path)}: undefined is not JSON-safe`, + path, + ); + } + if (typeof value === "number") { + if (!Number.isFinite(value)) { + throw new LayerKeyError( + `${formatKeyPath(path)}: non-finite number is not JSON-safe`, + path, + ); + } + return; + } + if ( + typeof value === "string" || + typeof value === "boolean" || + value === null + ) { + return; + } + if (typeof value === "bigint") { + throw new LayerKeyError( + `${formatKeyPath(path)}: bigint is not JSON-safe`, + path, + ); + } + if (typeof value === "symbol" || typeof value === "function") { + throw new LayerKeyError( + `${formatKeyPath(path)}: ${typeof value} is not JSON-safe`, + path, + ); + } + if (typeof value !== "object") { + throw new LayerKeyError( + `${formatKeyPath(path)}: unsupported key segment`, + path, + ); + } + if (seen.has(value)) { + throw new LayerKeyError( + `${formatKeyPath(path)}: cyclic structure is not JSON-safe`, + path, + ); + } + seen.add(value); + + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i++) { + walkLayerKey(value[i], [...path, i], seen); + } + return; + } + if (!isPlainObject(value)) { + throw new LayerKeyError( + `${formatKeyPath(path)}: only plain objects are JSON-safe`, + path, + ); + } + for (const k of Object.keys(value)) { + walkLayerKey(value[k], [...path, k], seen); + } +} + +/** + * Ensures a layer key is JSON-safe for {@link hashKey}. + * Allowed: `string` | `boolean` | `null` | finite `number` | plain objects | arrays of those. + */ +export function assertLayerKey(key: unknown): asserts key is LayerKey { + if (!Array.isArray(key)) { + throw new LayerKeyError("key: must be an array", []); + } + walkLayerKey(key, [], new WeakSet()); +} + /** Serializes a key deterministically by sorting object properties recursively. */ export function hashKey(key: LayerKey): string { + assertLayerKey(key); return JSON.stringify(key, (_unused, val) => isPlainObject(val) ? sortedObject(val) : val, ); From 4e45ecdb790af743fe7b0e3d0cf0ab7d905657ef Mon Sep 17 00:00:00 2001 From: Sutu Sebastian Date: Sun, 19 Jul 2026 12:49:49 +0300 Subject: [PATCH 2/2] harden: fix DAG cycle false positive and __proto__ hash collision Backtrack WeakSet after walking children; sort into Object.create(null); tighten tests/docs/JSDoc for the JSON-safe key contract. --- .changeset/json-safe-layer-keys.md | 2 +- .../content/concepts/identity-and-types.mdx | 4 +-- apps/docs/content/guides/error-handling.mdx | 2 +- apps/docs/content/reference/core-api.mdx | 2 +- packages/core/skills/layers/SKILL.md | 2 +- packages/core/src/errors.ts | 4 +-- packages/core/src/layerStack.test.ts | 29 ++++++++++++++++++- packages/core/src/types.ts | 5 ++-- packages/core/src/utils.ts | 23 +++++++++++++-- 9 files changed, 59 insertions(+), 14 deletions(-) diff --git a/.changeset/json-safe-layer-keys.md b/.changeset/json-safe-layer-keys.md index 0fcefe0..09bedf3 100644 --- a/.changeset/json-safe-layer-keys.md +++ b/.changeset/json-safe-layer-keys.md @@ -2,4 +2,4 @@ "@stainless-code/layers": patch --- -Reject non-JSON-safe layer key segments with `LayerKeyError` before `hashKey`. Previously `JSON.stringify` could collide `[undefined]` / `[null]` / `[NaN]` as `"[null]"`. +Reject non-JSON-safe layer key segments with `LayerKeyError` from `hashKey` (before stringify). Previously `JSON.stringify` could collide `[undefined]` / `[null]` / `[NaN]` as `"[null]"`. diff --git a/apps/docs/content/concepts/identity-and-types.mdx b/apps/docs/content/concepts/identity-and-types.mdx index 91f21ee..1b208be 100644 --- a/apps/docs/content/concepts/identity-and-types.mdx +++ b/apps/docs/content/concepts/identity-and-types.mdx @@ -20,12 +20,12 @@ Two opens with the same `key` share **logical identity**; each still gets a uniq ## JSON-safe key domain -`hashKey` / `keySignature` serialize with sorted-object `JSON.stringify`. Keys must stay inside that domain or `open` / `find` / `cancelQueued` throw `LayerKeyError` synchronously (narrow with `isLayerKeyError`). +`hashKey` / `keySignature` serialize with sorted-object `JSON.stringify`. Keys must stay inside that domain or `hashKey`, `open` / `find` / `cancelQueued`, and `createLayer` (eager `keySignature`) throw `LayerKeyError` synchronously (narrow with `isLayerKeyError`). | Allowed | Rejected | | ------- | -------- | | `string`, `boolean`, `null`, finite `number` | `undefined` (anywhere, including object props) | -| Plain objects / arrays of the above | `NaN`, `±Infinity`, `bigint`, `symbol`, functions | +| Plain objects / arrays of the above (shared refs / DAGs ok) | `NaN`, `±Infinity`, `bigint`, `symbol`, functions | | Property order in plain objects (sorted for equality) | Non-plain objects (`Date`, class instances), cycles | Prefer `null` or an explicit sentinel (`filterId ?? "none"`) over `undefined`. Omit optional object fields instead of setting them to `undefined`. diff --git a/apps/docs/content/guides/error-handling.mdx b/apps/docs/content/guides/error-handling.mdx index 8666d67..1917aef 100644 --- a/apps/docs/content/guides/error-handling.mdx +++ b/apps/docs/content/guides/error-handling.mdx @@ -9,7 +9,7 @@ search: ## Sync throws vs Promise rejects -A non-JSON-safe `key` throws `LayerKeyError` **synchronously** from `open` / `find` / `cancelQueued` (before a promise is returned). Narrow with `isLayerKeyError`. That path is distinct from the Promise rejection cases below — wrap the `open` call itself if keys may be untrusted. +A non-JSON-safe `key` throws `LayerKeyError` **synchronously** (before any promise) — not a Promise rejection. Narrow with `isLayerKeyError`. Throw sites: [Identity & types](/concepts/identity-and-types). Wrap the call site when keys may be untrusted. ## What rejects diff --git a/apps/docs/content/reference/core-api.mdx b/apps/docs/content/reference/core-api.mdx index c39a013..de60f00 100644 --- a/apps/docs/content/reference/core-api.mdx +++ b/apps/docs/content/reference/core-api.mdx @@ -175,7 +175,7 @@ function isPayloadValidationError( ## Layer key errors -Non-JSON-safe keys throw synchronously from `hashKey` / `open` / `find` / `cancelQueued` (not a Promise rejection). See [Identity & types](/concepts/identity-and-types). +Non-JSON-safe keys throw `LayerKeyError` synchronously (not a Promise rejection). Throw sites: [Identity & types](/concepts/identity-and-types). ```ts class LayerKeyError extends Error { diff --git a/packages/core/skills/layers/SKILL.md b/packages/core/skills/layers/SKILL.md index 5ae4eb1..545e793 100644 --- a/packages/core/skills/layers/SKILL.md +++ b/packages/core/skills/layers/SKILL.md @@ -276,7 +276,7 @@ The complete public API surface: | Wired handles | `createLayer`, `LayerHandle`, `ValidatedLayerHandle` | | Rendering seam | `createCallContext`, `LayerCallContext`, `LayerComponentProps`, `LayerComponent` | | Nested layers | `createLayerGroup`, `childStackId`, `LayerGroupOptions`, `LayerGroupHandle` | -| Identity and notification values | `assertLayerKey`, `hashKey`, `keySignature`, `notifyManager` | +| Identity and notification values | `hashKey`, `keySignature`, `notifyManager` | | Validation API | `isPayloadValidationError`, `Validator`, `InferValidatorInput`, `InferValidatorOutput`, `StandardSchemaV1`, `ValidationIssue` | | Layer model types | `LayerKey`, `LayerPhase`, `LayerTransition`, `LayerActionStatus`, `LayerState`, `LayerOptions`, `OpenLayerOptions` | | Stack and client types | `StackOptions`, `StackDefaults`, `LayerClientOptions` | diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 3b77b5b..61a998b 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -37,7 +37,7 @@ export function isPayloadValidationError( return value instanceof PayloadValidationError; } -/** Thrown when a layer key is not JSON-safe (see {@link assertLayerKey}). */ +/** Thrown when a layer key is not JSON-safe (from `assertLayerKey` / `hashKey`). */ export class LayerKeyError extends Error { readonly path: ReadonlyArray; constructor(message: string, path: ReadonlyArray = []) { @@ -48,7 +48,7 @@ export class LayerKeyError extends Error { } /** - * Narrows an unknown throw/rejection to {@link LayerKeyError}. + * Narrows an unknown synchronous throw to {@link LayerKeyError}. * * @example * ```ts diff --git a/packages/core/src/layerStack.test.ts b/packages/core/src/layerStack.test.ts index 7aad1cf..999dc50 100644 --- a/packages/core/src/layerStack.test.ts +++ b/packages/core/src/layerStack.test.ts @@ -30,9 +30,31 @@ describe("hashKey / keySignature", () => { expect(() => assertLayerKey(["modal", 1, true, null, { nested: ["a", 2] }]), ).not.toThrow(); + expect(hashKey([])).toBe("[]"); + expect(hashKey([["a"]])).toBe(JSON.stringify([["a"]])); expect(hashKey(["modal", 1, true, null])).toBe( JSON.stringify(["modal", 1, true, null]), ); + const nullProto = Object.create(null) as Record; + nullProto.id = 1; + expect(hashKey([nullProto])).toBe(JSON.stringify([{ id: 1 }])); + }); + it("accepts shared object refs (DAGs), not only trees", () => { + const shared = { id: 1 }; + expect(hashKey([{ x: shared, y: shared }])).toBe( + JSON.stringify([{ x: { id: 1 }, y: { id: 1 } }]), + ); + expect(hashKey([shared, shared])).toBe( + JSON.stringify([{ id: 1 }, { id: 1 }]), + ); + }); + it("preserves own __proto__ keys in the hash (no prototype clobber)", () => { + const withProto = JSON.parse('{"__proto__":{"a":1}}') as Record< + string, + unknown + >; + expect(hashKey([withProto])).not.toBe(hashKey([{}])); + expect(hashKey([withProto])).toBe(JSON.stringify([withProto])); }); it("rejects undefined, non-finite numbers, bigint, and non-plain objects", () => { const cases: Array<{ key: unknown; pathHint: string }> = [ @@ -42,6 +64,7 @@ describe("hashKey / keySignature", () => { { key: [Infinity], pathHint: "key[0]" }, { key: [-Infinity], pathHint: "key[0]" }, { key: [1n], pathHint: "key[0]" }, + { key: [() => {}], pathHint: "key[0]" }, { key: [new Date()], pathHint: "key[0]" }, { key: [Symbol("x")], pathHint: "key[0]" }, ]; @@ -78,7 +101,7 @@ describe("shallowArrayEqual", () => { }); describe("LayerStack — basics", () => { - it("open throws LayerKeyError synchronously for non-JSON-safe keys", () => { + it("open/find/cancelQueued throw LayerKeyError synchronously for non-JSON-safe keys", () => { const stack = new LayerStack("s"); expect(() => stack.open({ @@ -86,6 +109,10 @@ describe("LayerStack — basics", () => { payload: {}, }), ).toThrow(LayerKeyError); + expect(() => stack.find([undefined])).toThrow(LayerKeyError); + expect(() => stack.cancelQueued([undefined], undefined)).toThrow( + LayerKeyError, + ); }); it("open pushes, indexes, and goes active without loadFn", () => { diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 31e729c..a319e9d 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1,12 +1,13 @@ import type { Validator } from "./validators"; /** - * Stable identity for a layer instance. + * Logical identity for a layer (`find` / `upsert` / `gcTime`). * * Must be JSON-safe: `string` | `boolean` | `null` | finite `number`, or * plain objects / arrays of those. Compared via sorted `JSON.stringify` — * two keys with the same values in a different object-property order are equal. - * Invalid keys throw {@link LayerKeyError} from {@link hashKey} / `open`. + * Invalid keys throw `LayerKeyError` from any path that hashes the key + * (`hashKey`, `open`, `find`, `cancelQueued`, `createLayer`). */ export type LayerKey = ReadonlyArray; diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 136b1ed..8823dc7 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -11,7 +11,8 @@ function isPlainObject(value: unknown): value is Record { function sortedObject(value: Record): Record { const keys = Object.keys(value).sort(); - const out: Record = {}; + // null prototype — own `__proto__` must not mutate [[Prototype]] / drop from hash + const out: Record = Object.create(null); for (const k of keys) { out[k] = value[k]; } @@ -90,6 +91,7 @@ function walkLayerKey( for (let i = 0; i < value.length; i++) { walkLayerKey(value[i], [...path, i], seen); } + seen.delete(value); return; } if (!isPlainObject(value)) { @@ -101,11 +103,20 @@ function walkLayerKey( for (const k of Object.keys(value)) { walkLayerKey(value[k], [...path, k], seen); } + seen.delete(value); } /** * Ensures a layer key is JSON-safe for {@link hashKey}. * Allowed: `string` | `boolean` | `null` | finite `number` | plain objects | arrays of those. + * Throws {@link LayerKeyError} when a segment is outside that domain. + * + * @example + * ```ts + * import { assertLayerKey } from "@stainless-code/layers"; + * + * assertLayerKey(["confirm", filterId ?? "none"]); + * ``` */ export function assertLayerKey(key: unknown): asserts key is LayerKey { if (!Array.isArray(key)) { @@ -114,7 +125,10 @@ export function assertLayerKey(key: unknown): asserts key is LayerKey { walkLayerKey(key, [], new WeakSet()); } -/** Serializes a key deterministically by sorting object properties recursively. */ +/** + * Serializes a key deterministically by sorting object properties recursively. + * Throws {@link LayerKeyError} when the key is not JSON-safe. + */ export function hashKey(key: LayerKey): string { assertLayerKey(key); return JSON.stringify(key, (_unused, val) => @@ -122,7 +136,10 @@ export function hashKey(key: LayerKey): string { ); } -/** Produces the canonical identity used to compare layer keys. */ +/** + * Produces the canonical identity used to compare layer keys. + * Throws {@link LayerKeyError} when the key is not JSON-safe (via {@link hashKey}). + */ export function keySignature(key: LayerKey): string { return hashKey(key); }