From 26a3411850bda6e64a772d5720353a52ffb537dc Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 5 Apr 2026 11:26:14 -0700 Subject: [PATCH 01/13] =?UTF-8?q?docs:=20cycle=200009=20design=20=E2=80=94?= =?UTF-8?q?=20op=20type=20class=20hierarchy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull PROTO_op-type-class-hierarchy (cool-ideas) and CC_op-types-typedef-to-class (bad-code) into cycle 0009. Hill: replace 8 typedef ops with frozen class hierarchy for runtime identity, constructor validation, and instanceof dispatch. --- .../op-type-class-hierarchy.md | 130 ++++++++++++++++++ .../bad-code/CC_op-types-typedef-to-class.md | 20 --- .../PROTO_op-type-class-hierarchy.md | 63 --------- 3 files changed, 130 insertions(+), 83 deletions(-) create mode 100644 docs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.md delete mode 100644 docs/method/backlog/bad-code/CC_op-types-typedef-to-class.md delete mode 100644 docs/method/backlog/cool-ideas/PROTO_op-type-class-hierarchy.md diff --git a/docs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.md b/docs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.md new file mode 100644 index 00000000..9281cc70 --- /dev/null +++ b/docs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.md @@ -0,0 +1,130 @@ +# Cycle 0009 — Op Type Class Hierarchy + +**Status:** DESIGN +**Date:** 2026-04-05 + +## Sponsors + +- **Human:** James Ross +- **Agent:** Claude (Opus) + +## Hill + +Replace 8 typedef ops with a frozen class hierarchy so the domain +model has runtime identity, constructor validation, and `instanceof` +dispatch — eliminating all string-based tag switching. + +## Playback questions + +### Agent questions + +1. Does `new NodeAdd(nodeId, dot)` throw when `nodeId` is empty or + `dot` is not a `Dot`? +2. Does `op instanceof NodeAdd` return true for NodeAdd instances and + false for EdgeAdd instances? +3. Does `instanceof Op` return true for all 8 op subclasses? +4. Are all op instances frozen (`Object.isFrozen(op) === true`)? +5. Does `OpNormalizer.normalizeRawOp()` return canonical op class + instances (`NodePropSet`, `EdgePropSet`)? +6. Does `OpNormalizer.lowerCanonicalOp()` return raw op class + instances (`PropSet`)? +7. Does `JoinReducer.OP_STRATEGIES` dispatch by `instanceof` instead + of string keys? +8. Do factory functions in WarpTypesV2.js delegate to constructors? +9. Does the CBOR decode boundary produce op class instances (not + plain objects)? + +### Human questions + +1. Can I still do `patch.addNode('user:alice')` and have it just + work? +2. Does `git warp history` still show op types correctly? +3. Do existing patches in a real repo still materialize identically? + +## Scope + +### What changes + +| Component | Change | +|---|---| +| `src/domain/types/ops/` | New directory. 8 op classes + base `Op` class. One file per class. | +| `src/domain/types/WarpTypesV2.js` | Factory functions delegate to constructors. Typedefs become re-exports. | +| `src/domain/services/OpNormalizer.js` | `normalizeRawOp` returns class instances. `lowerCanonicalOp` uses `instanceof`. | +| `src/domain/services/JoinReducer.js` | `OP_STRATEGIES` keyed by class reference, not string. Lookup via `instanceof` chain or class-to-strategy Map. | +| `src/domain/types/TickReceipt.js` | `OP_TYPES` array becomes class references or derives names from classes. | +| `src/domain/services/codec/MessageSchemaDetector.js` | `instanceof` checks replace string comparisons. | +| `bin/presenters/text.js` | `instanceof` checks replace string comparisons. | +| `src/domain/services/PatchBuilderV2.js` | Builds canonical op class instances internally. `build()`/`commit()` lower via `lowerCanonicalOp`. | +| CBOR decode boundary | `CborCodec` or `CborPatchJournalAdapter` hydrates plain objects into op classes. | + +### What does NOT change + +- Wire format. Persisted patches remain CBOR with `{ type: 'NodeAdd', ... }` plain objects. The class boundary is at decode, not encode. +- PatchV2 class. It holds `ops: Op[]` instead of `ops: OpV2[]` but the shape is the same. +- CRDT semantics. JoinReducer mutation logic is identical. +- Public API surface. `createPatch().addNode()` still works. + +## Non-goals + +- Moving strategy methods onto op classes. That's the JoinReducer + strategy registry cycle (separate design doc exists). This cycle + gives ops runtime identity; behavior coupling is a follow-on. +- Changing the wire format. ADR-0002 defers that. +- Touching test files that construct ops with plain objects — those + become integration tests for the CBOR decode hydration path. + +## Accessibility / assistive reading posture + +Not applicable — no UI changes. + +## Localization / directionality posture + +Not applicable — no user-facing strings. + +## Agent inspectability / explainability posture + +Op classes are `instanceof`-dispatchable and frozen. An agent can +inspect any op with `op.constructor.name` and get a meaningful +domain name. This is strictly better than the current string tags +for agent tooling. + +## Cut plan + +### Slice 1 — Op classes + tests (RED then GREEN) + +- Base `Op` class (abstract-ish — no direct instantiation) +- 8 subclasses: `NodeAdd`, `NodeRemove`, `EdgeAdd`, `EdgeRemove`, + `NodePropSet`, `EdgePropSet`, `PropSet`, `BlobValue` +- Constructor validation, freeze, instanceof +- Factory functions in WarpTypesV2.js delegate to constructors + +### Slice 2 — OpNormalizer + tests + +- `normalizeRawOp` returns class instances +- `lowerCanonicalOp` uses `instanceof` +- Round-trip: raw → canonical → raw preserves identity + +### Slice 3 — JoinReducer wiring + tests + +- `OP_STRATEGIES` lookup by constructor, not string +- `RAW_KNOWN_OPS` / `CANONICAL_KNOWN_OPS` become class-based checks +- Existing noCoordination test suite must pass unchanged + +### Slice 4 — Consumer wiring (presenter, detector, receipt) + +- `MessageSchemaDetector` uses `instanceof` +- `bin/presenters/text.js` uses `instanceof` +- `TickReceipt.OP_TYPES` derives from class hierarchy + +### Slice 5 — CBOR hydration boundary + +- Decode path hydrates plain objects into op class instances +- Golden blob round-trip test: encode → decode → class instance + +## Hard gates + +- **noCoordination test suite passes unchanged.** This is the + multi-writer safety regression suite. Non-negotiable. +- **Existing BATS CLI tests pass.** No user-visible behavior change. +- **Wire format compatibility.** Encode a patch with the new classes, + decode with the old code path — must produce identical state. diff --git a/docs/method/backlog/bad-code/CC_op-types-typedef-to-class.md b/docs/method/backlog/bad-code/CC_op-types-typedef-to-class.md deleted file mode 100644 index 48118753..00000000 --- a/docs/method/backlog/bad-code/CC_op-types-typedef-to-class.md +++ /dev/null @@ -1,20 +0,0 @@ -# OpV2 types are typedef phantoms with external tag dispatch - -**Effort:** L - -## What's Wrong - -`WarpTypesV2.js` defines 8 op types (NodeAdd, NodeRemove, EdgeAdd, -EdgeRemove, PropSet, NodePropSet, EdgePropSet, BlobValue) as `@typedef` -plus factory functions. No constructor validation. External code uses -`op.type === 'NodeAdd'` string switching everywhere. - -This is a P1 + P3 + P7 violation: domain concepts without runtime -identity, behavior externalized into switch statements, and tag -dispatch instead of `instanceof`. - -## Suggested Fix - -Class hierarchy with a base `Op` class. Each op type is a subclass with -constructor validation. Replace string switching with `instanceof` -dispatch. Factory functions become constructors. diff --git a/docs/method/backlog/cool-ideas/PROTO_op-type-class-hierarchy.md b/docs/method/backlog/cool-ideas/PROTO_op-type-class-hierarchy.md deleted file mode 100644 index f83c3f08..00000000 --- a/docs/method/backlog/cool-ideas/PROTO_op-type-class-hierarchy.md +++ /dev/null @@ -1,63 +0,0 @@ -# Op types as a class hierarchy — the biggest P1 win - -**Effort:** L - -## Idea - -`WarpTypesV2` defines 8 op types as typedefs with factory functions. -Every consumer does `if (op.type === 'NodeAdd')` — string-based tag -switching, the exact anti-pattern that P1, P3, and P7 were written to -prevent. This is the single largest doctrine violation in the codebase, -and it touches everything: JoinReducer, OpNormalizer, PatchBuilderV2, -TickReceipt, the diff engine. - -Imagine instead: - -```js -class Op { - constructor(dot) { - if (!dot || !(dot instanceof Dot)) { - throw new Error('Op requires a valid Dot'); - } - this.dot = dot; - Object.freeze(this); - } -} - -class NodeAdd extends Op { - constructor(nodeId, dot) { - super(dot); - if (typeof nodeId !== 'string' || nodeId.length === 0) { - throw new Error('NodeAdd requires a non-empty nodeId'); - } - this.nodeId = nodeId; - Object.freeze(this); - } -} -``` - -JoinReducer switches from `if (op.type === 'NodeAdd')` to `if (op -instanceof NodeAdd)` — no strings, no tags, no `switch` statements. -Each op class owns its own validation: dot is a real `Dot`, nodeId is -a non-empty string, edgeKey components contain no reserved bytes. The -constructor is the invariant boundary. - -The OpNormalizer becomes a method on the class: `op.toCanonical()` -returns the canonical form, `Op.fromRaw(rawOp)` parses raw wire format -into the correct subclass. Serialization stays in the codec (P5) — the -op doesn't know how it's encoded, but it does know how to normalize -itself. - -TickReceipt's `OP_TYPES` enum dissolves. The receipt stores the op -instance directly. `receipt.ops.filter(op => op instanceof EdgePropSet)` -is cleaner than `receipt.ops.filter(op => op.type === 'EdgePropSet')`. - -## Why cool - -This is the refactor that would most improve the domain model's runtime -honesty. Eight typedefs become eight classes. Dozens of string -comparisons become `instanceof` checks. Validation moves from "hope the -factory was called correctly" to "the constructor rejects bad input." -The entire op pipeline — build, normalize, apply, record — gets -type-safe at runtime, not just in JSDoc comments. This is what the -systems-style manifesto was written for. From c393be5e68cf34eda91ea4a2f6ec9adde046cfba Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 5 Apr 2026 11:32:05 -0700 Subject: [PATCH 02/13] =?UTF-8?q?feat:=20op=20type=20class=20hierarchy=20?= =?UTF-8?q?=E2=80=94=20Slice=201=20(9=20classes,=2071=20tests)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Base Op class (abstract) + 8 concrete subclasses: NodeAdd, NodeRemove, EdgeAdd, EdgeRemove, NodePropSet, EdgePropSet, PropSet, BlobValue. Each class: constructor validation, Object.freeze, instanceof dispatch. Edge types use options objects (max-params compliance). Shared validators in validate.js. Cycle 0009 ��� op-type-class-hierarchy --- src/domain/types/ops/BlobValue.js | 35 ++ src/domain/types/ops/EdgeAdd.js | 49 +++ src/domain/types/ops/EdgePropSet.js | 48 +++ src/domain/types/ops/EdgeRemove.js | 43 ++ src/domain/types/ops/NodeAdd.js | 38 ++ src/domain/types/ops/NodePropSet.js | 40 ++ src/domain/types/ops/NodeRemove.js | 34 ++ src/domain/types/ops/Op.js | 34 ++ src/domain/types/ops/PropSet.js | 44 ++ src/domain/types/ops/validate.js | 45 ++ test/unit/domain/types/ops/Op.test.js | 439 ++++++++++++++++++++ test/unit/domain/types/ops/validate.test.js | 51 +++ 12 files changed, 900 insertions(+) create mode 100644 src/domain/types/ops/BlobValue.js create mode 100644 src/domain/types/ops/EdgeAdd.js create mode 100644 src/domain/types/ops/EdgePropSet.js create mode 100644 src/domain/types/ops/EdgeRemove.js create mode 100644 src/domain/types/ops/NodeAdd.js create mode 100644 src/domain/types/ops/NodePropSet.js create mode 100644 src/domain/types/ops/NodeRemove.js create mode 100644 src/domain/types/ops/Op.js create mode 100644 src/domain/types/ops/PropSet.js create mode 100644 src/domain/types/ops/validate.js create mode 100644 test/unit/domain/types/ops/Op.test.js create mode 100644 test/unit/domain/types/ops/validate.test.js diff --git a/src/domain/types/ops/BlobValue.js b/src/domain/types/ops/BlobValue.js new file mode 100644 index 00000000..2e7b58d5 --- /dev/null +++ b/src/domain/types/ops/BlobValue.js @@ -0,0 +1,35 @@ +/** + * BlobValue — reference to an external blob in the Git object store. + * + * @module domain/types/ops/BlobValue + */ + +import Op from './Op.js'; +import { assertNonEmptyString } from './validate.js'; + +/** + * References an external blob attached to a node. + * No state effect in the reducer — recorded for provenance tracking. + */ +export default class BlobValue extends Op { + /** @type {string} Node ID the blob is attached to */ + node; + + /** @type {string} Blob object ID in the Git object store */ + oid; + + /** + * Creates a BlobValue operation. + * + * @param {string} node - Non-empty node ID + * @param {string} oid - Non-empty blob object ID + */ + constructor(node, oid) { + super('BlobValue'); + assertNonEmptyString(node, 'BlobValue', 'node'); + assertNonEmptyString(oid, 'BlobValue', 'oid'); + this.node = node; + this.oid = oid; + Object.freeze(this); + } +} diff --git a/src/domain/types/ops/EdgeAdd.js b/src/domain/types/ops/EdgeAdd.js new file mode 100644 index 00000000..a7505791 --- /dev/null +++ b/src/domain/types/ops/EdgeAdd.js @@ -0,0 +1,49 @@ +/** + * EdgeAdd — adds a directed edge to the graph with a causal dot. + * + * @module domain/types/ops/EdgeAdd + */ + +import { Dot } from '../../crdt/Dot.js'; +import Op from './Op.js'; +import { assertNonEmptyString, assertNoBannedBytes } from './validate.js'; + +/** + * Adds a directed edge to the graph's OR-Set with a unique dot. + */ +export default class EdgeAdd extends Op { + /** @type {string} Source node ID */ + from; + + /** @type {string} Target node ID */ + to; + + /** @type {string} Edge label/type */ + label; + + /** @type {Dot} Causal identifier for this add */ + dot; + + /** + * Creates an EdgeAdd operation. + * + * @param {{ from: string, to: string, label: string, dot: Dot }} fields + */ + constructor({ from, to, label, dot }) { + super('EdgeAdd'); + assertNonEmptyString(from, 'EdgeAdd', 'from'); + assertNonEmptyString(to, 'EdgeAdd', 'to'); + assertNonEmptyString(label, 'EdgeAdd', 'label'); + assertNoBannedBytes(from, 'EdgeAdd', 'from'); + assertNoBannedBytes(to, 'EdgeAdd', 'to'); + assertNoBannedBytes(label, 'EdgeAdd', 'label'); + if (!(dot instanceof Dot)) { + throw new Error('EdgeAdd requires dot to be a Dot instance'); + } + this.from = from; + this.to = to; + this.label = label; + this.dot = dot; + Object.freeze(this); + } +} diff --git a/src/domain/types/ops/EdgePropSet.js b/src/domain/types/ops/EdgePropSet.js new file mode 100644 index 00000000..82bd6206 --- /dev/null +++ b/src/domain/types/ops/EdgePropSet.js @@ -0,0 +1,48 @@ +/** + * EdgePropSet — canonical edge property operation (internal only). + * + * @module domain/types/ops/EdgePropSet + */ + +import Op from './Op.js'; +import { assertNonEmptyString } from './validate.js'; + +/** + * Sets a property on an edge using LWW semantics. + * Canonical form — never persisted directly (lowered to PropSet on wire). + */ +export default class EdgePropSet extends Op { + /** @type {string} Source node ID */ + from; + + /** @type {string} Target node ID */ + to; + + /** @type {string} Edge label */ + label; + + /** @type {string} Property key */ + key; + + /** @type {unknown} Property value (any JSON-serializable type) */ + value; + + /** + * Creates an EdgePropSet operation. + * + * @param {{ from: string, to: string, label: string, key: string, value: unknown }} fields + */ + constructor({ from, to, label, key, value }) { + super('EdgePropSet'); + assertNonEmptyString(from, 'EdgePropSet', 'from'); + assertNonEmptyString(to, 'EdgePropSet', 'to'); + assertNonEmptyString(label, 'EdgePropSet', 'label'); + assertNonEmptyString(key, 'EdgePropSet', 'key'); + this.from = from; + this.to = to; + this.label = label; + this.key = key; + this.value = value; + Object.freeze(this); + } +} diff --git a/src/domain/types/ops/EdgeRemove.js b/src/domain/types/ops/EdgeRemove.js new file mode 100644 index 00000000..f1525fa6 --- /dev/null +++ b/src/domain/types/ops/EdgeRemove.js @@ -0,0 +1,43 @@ +/** + * EdgeRemove — removes an edge by tombstoning observed dots. + * + * @module domain/types/ops/EdgeRemove + */ + +import Op from './Op.js'; +import { assertNonEmptyString, assertArray } from './validate.js'; + +/** + * Removes a directed edge from the graph's OR-Set by tombstoning observed dots. + */ +export default class EdgeRemove extends Op { + /** @type {string} Source node ID */ + from; + + /** @type {string} Target node ID */ + to; + + /** @type {string} Edge label/type */ + label; + + /** @type {readonly string[]} Encoded dot strings being removed */ + observedDots; + + /** + * Creates an EdgeRemove operation. + * + * @param {{ from: string, to: string, label: string, observedDots: string[] }} fields + */ + constructor({ from, to, label, observedDots }) { + super('EdgeRemove'); + assertNonEmptyString(from, 'EdgeRemove', 'from'); + assertNonEmptyString(to, 'EdgeRemove', 'to'); + assertNonEmptyString(label, 'EdgeRemove', 'label'); + assertArray(observedDots, 'EdgeRemove', 'observedDots'); + this.from = from; + this.to = to; + this.label = label; + this.observedDots = Object.freeze([...observedDots]); + Object.freeze(this); + } +} diff --git a/src/domain/types/ops/NodeAdd.js b/src/domain/types/ops/NodeAdd.js new file mode 100644 index 00000000..91e4710b --- /dev/null +++ b/src/domain/types/ops/NodeAdd.js @@ -0,0 +1,38 @@ +/** + * NodeAdd — adds a node to the graph with a causal dot. + * + * @module domain/types/ops/NodeAdd + */ + +import { Dot } from '../../crdt/Dot.js'; +import Op from './Op.js'; +import { assertNonEmptyString, assertNoBannedBytes } from './validate.js'; + +/** + * Adds a node to the graph's OR-Set with a unique dot. + */ +export default class NodeAdd extends Op { + /** @type {string} Node ID to add */ + node; + + /** @type {Dot} Causal identifier for this add */ + dot; + + /** + * Creates a NodeAdd operation. + * + * @param {string} node - Non-empty node ID (no NUL bytes) + * @param {Dot} dot - Must be a Dot instance + */ + constructor(node, dot) { + super('NodeAdd'); + assertNonEmptyString(node, 'NodeAdd', 'node'); + assertNoBannedBytes(node, 'NodeAdd', 'node'); + if (!(dot instanceof Dot)) { + throw new Error('NodeAdd requires dot to be a Dot instance'); + } + this.node = node; + this.dot = dot; + Object.freeze(this); + } +} diff --git a/src/domain/types/ops/NodePropSet.js b/src/domain/types/ops/NodePropSet.js new file mode 100644 index 00000000..e611e5dc --- /dev/null +++ b/src/domain/types/ops/NodePropSet.js @@ -0,0 +1,40 @@ +/** + * NodePropSet — canonical node property operation (internal only). + * + * @module domain/types/ops/NodePropSet + */ + +import Op from './Op.js'; +import { assertNonEmptyString } from './validate.js'; + +/** + * Sets a property on a node using LWW semantics. + * Canonical form — never persisted directly (lowered to PropSet on wire). + */ +export default class NodePropSet extends Op { + /** @type {string} Node ID */ + node; + + /** @type {string} Property key */ + key; + + /** @type {unknown} Property value (any JSON-serializable type) */ + value; + + /** + * Creates a NodePropSet operation. + * + * @param {string} node - Non-empty node ID + * @param {string} key - Non-empty property key + * @param {unknown} value - Property value + */ + constructor(node, key, value) { + super('NodePropSet'); + assertNonEmptyString(node, 'NodePropSet', 'node'); + assertNonEmptyString(key, 'NodePropSet', 'key'); + this.node = node; + this.key = key; + this.value = value; + Object.freeze(this); + } +} diff --git a/src/domain/types/ops/NodeRemove.js b/src/domain/types/ops/NodeRemove.js new file mode 100644 index 00000000..71b9c7e1 --- /dev/null +++ b/src/domain/types/ops/NodeRemove.js @@ -0,0 +1,34 @@ +/** + * NodeRemove — removes a node by tombstoning observed dots. + * + * @module domain/types/ops/NodeRemove + */ + +import Op from './Op.js'; +import { assertNonEmptyString, assertArray } from './validate.js'; + +/** + * Removes a node from the graph's OR-Set by tombstoning observed dots. + */ +export default class NodeRemove extends Op { + /** @type {string} Node ID to remove */ + node; + + /** @type {readonly string[]} Encoded dot strings being removed */ + observedDots; + + /** + * Creates a NodeRemove operation. + * + * @param {string} node - Non-empty node ID + * @param {string[]} observedDots - Encoded dot strings (add events observed) + */ + constructor(node, observedDots) { + super('NodeRemove'); + assertNonEmptyString(node, 'NodeRemove', 'node'); + assertArray(observedDots, 'NodeRemove', 'observedDots'); + this.node = node; + this.observedDots = Object.freeze([...observedDots]); + Object.freeze(this); + } +} diff --git a/src/domain/types/ops/Op.js b/src/domain/types/ops/Op.js new file mode 100644 index 00000000..dc2d4c59 --- /dev/null +++ b/src/domain/types/ops/Op.js @@ -0,0 +1,34 @@ +/** + * Op — abstract base class for all WARP operations. + * + * Provides runtime identity (`instanceof Op`) and the `type` discriminator + * field for serialization compatibility. Subclasses carry validated, + * frozen payloads. + * + * Direct instantiation throws — use a concrete subclass. + * + * @module domain/types/ops/Op + */ + +/** + * Abstract base for WARP graph operations. + */ +export default class Op { + /** @type {string} Operation type discriminator (matches wire format) */ + type; + + /** + * Creates an Op. Not instantiable directly — use a concrete subclass. + * + * @param {string} type - The operation type discriminator string + */ + constructor(type) { + if (new.target === Op) { + throw new Error('Op is abstract — use a concrete subclass (NodeAdd, EdgeAdd, etc.)'); + } + if (typeof type !== 'string' || type.length === 0) { + throw new Error('Op type must be a non-empty string'); + } + this.type = type; + } +} diff --git a/src/domain/types/ops/PropSet.js b/src/domain/types/ops/PropSet.js new file mode 100644 index 00000000..08fa6562 --- /dev/null +++ b/src/domain/types/ops/PropSet.js @@ -0,0 +1,44 @@ +/** + * PropSet — raw/wire-format property operation. + * + * This is the persisted form. Edge properties use a \x01-prefixed node + * field. See NodePropSet and EdgePropSet for the canonical (internal) + * representations. + * + * @module domain/types/ops/PropSet + */ + +import Op from './Op.js'; +import { assertNonEmptyString } from './validate.js'; + +/** + * Sets a property on a node (raw wire format). + * The `node` field may carry a \x01-prefixed edge identity for edge props. + */ +export default class PropSet extends Op { + /** @type {string} Node ID (may contain \x01 prefix for edge props) */ + node; + + /** @type {string} Property key */ + key; + + /** @type {unknown} Property value (any JSON-serializable type) */ + value; + + /** + * Creates a PropSet operation (raw wire format). + * + * @param {string} node - Non-empty node ID + * @param {string} key - Non-empty property key + * @param {unknown} value - Property value + */ + constructor(node, key, value) { + super('PropSet'); + assertNonEmptyString(node, 'PropSet', 'node'); + assertNonEmptyString(key, 'PropSet', 'key'); + this.node = node; + this.key = key; + this.value = value; + Object.freeze(this); + } +} diff --git a/src/domain/types/ops/validate.js b/src/domain/types/ops/validate.js new file mode 100644 index 00000000..ecd7717f --- /dev/null +++ b/src/domain/types/ops/validate.js @@ -0,0 +1,45 @@ +/** + * Shared validation helpers for Op constructors. + * + * @module domain/types/ops/validate + */ + +/** + * Asserts that a value is a non-empty string. + * + * @param {unknown} value + * @param {string} opName - For error messages + * @param {string} field - For error messages + */ +export function assertNonEmptyString(value, opName, field) { + if (typeof value !== 'string' || value.length === 0) { + throw new Error(`${opName} requires '${field}' to be a non-empty string`); + } +} + +/** + * Asserts that a string contains no NUL (\x00) bytes. + * NUL is the edge key separator — it cannot appear in identifiers. + * + * @param {string} value + * @param {string} opName + * @param {string} field + */ +export function assertNoBannedBytes(value, opName, field) { + if (value.includes('\x00')) { + throw new Error(`${opName} '${field}' must not contain NUL (\\x00) bytes`); + } +} + +/** + * Asserts that a value is an Array. + * + * @param {unknown} value + * @param {string} opName + * @param {string} field + */ +export function assertArray(value, opName, field) { + if (!Array.isArray(value)) { + throw new Error(`${opName} requires '${field}' to be an array`); + } +} diff --git a/test/unit/domain/types/ops/Op.test.js b/test/unit/domain/types/ops/Op.test.js new file mode 100644 index 00000000..86bd873d --- /dev/null +++ b/test/unit/domain/types/ops/Op.test.js @@ -0,0 +1,439 @@ +import { describe, it, expect } from 'vitest'; +import { Dot } from '../../../../../src/domain/crdt/Dot.js'; +import Op from '../../../../../src/domain/types/ops/Op.js'; +import NodeAdd from '../../../../../src/domain/types/ops/NodeAdd.js'; +import NodeRemove from '../../../../../src/domain/types/ops/NodeRemove.js'; +import EdgeAdd from '../../../../../src/domain/types/ops/EdgeAdd.js'; +import EdgeRemove from '../../../../../src/domain/types/ops/EdgeRemove.js'; +import NodePropSet from '../../../../../src/domain/types/ops/NodePropSet.js'; +import EdgePropSet from '../../../../../src/domain/types/ops/EdgePropSet.js'; +import PropSet from '../../../../../src/domain/types/ops/PropSet.js'; +import BlobValue from '../../../../../src/domain/types/ops/BlobValue.js'; + +describe('Op base class', () => { + it('cannot be instantiated directly', () => { + expect(() => new Op('NodeAdd')).toThrow(); + }); + + it('is the prototype of all op subclasses', () => { + const dot = new Dot('w', 1); + const ops = [ + new NodeAdd('n1', dot), + new NodeRemove('n1', ['w:1']), + new EdgeAdd({ from: 'n1', to: 'n2', label: 'rel', dot }), + new EdgeRemove({ from: 'n1', to: 'n2', label: 'rel', observedDots: ['w:1'] }), + new NodePropSet('n1', 'key', 'val'), + new EdgePropSet({ from: 'n1', to: 'n2', label: 'rel', key: 'key', value: 'val' }), + new PropSet('n1', 'key', 'val'), + new BlobValue('n1', 'abc123'), + ]; + + for (const op of ops) { + expect(op).toBeInstanceOf(Op); + } + }); +}); + +describe('NodeAdd', () => { + it('constructs with valid nodeId and dot', () => { + const dot = new Dot('alice', 1); + const op = new NodeAdd('user:alice', dot); + + expect(op.type).toBe('NodeAdd'); + expect(op.node).toBe('user:alice'); + expect(op.dot).toBe(dot); + }); + + it('is frozen', () => { + const dot = new Dot('alice', 1); + const op = new NodeAdd('user:alice', dot); + + expect(Object.isFrozen(op)).toBe(true); + }); + + it('is instanceof Op and NodeAdd', () => { + const dot = new Dot('alice', 1); + const op = new NodeAdd('user:alice', dot); + + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(NodeAdd); + }); + + it('is NOT instanceof other op classes', () => { + const dot = new Dot('alice', 1); + const op = new NodeAdd('user:alice', dot); + + expect(op).not.toBeInstanceOf(EdgeAdd); + expect(op).not.toBeInstanceOf(NodeRemove); + expect(op).not.toBeInstanceOf(PropSet); + }); + + it('throws on empty nodeId', () => { + const dot = new Dot('alice', 1); + expect(() => new NodeAdd('', dot)).toThrow(); + }); + + it('throws on non-string nodeId', () => { + const dot = new Dot('alice', 1); + expect(() => new NodeAdd(/** @type {any} */ (42), dot)).toThrow(); + expect(() => new NodeAdd(/** @type {any} */ (null), dot)).toThrow(); + }); + + it('throws when dot is not a Dot instance', () => { + expect(() => new NodeAdd('n1', /** @type {any} */ ({ writerId: 'w', counter: 1 }))).toThrow(); + }); + + it('rejects nodeId containing NUL byte', () => { + const dot = new Dot('alice', 1); + expect(() => new NodeAdd('user\x00alice', dot)).toThrow(); + }); +}); + +describe('NodeRemove', () => { + it('constructs with valid nodeId and observedDots', () => { + const op = new NodeRemove('user:alice', ['alice:1', 'bob:2']); + + expect(op.type).toBe('NodeRemove'); + expect(op.node).toBe('user:alice'); + expect(op.observedDots).toEqual(['alice:1', 'bob:2']); + }); + + it('is frozen', () => { + const op = new NodeRemove('user:alice', ['alice:1']); + expect(Object.isFrozen(op)).toBe(true); + }); + + it('freezes the observedDots array', () => { + const dots = ['alice:1', 'bob:2']; + const op = new NodeRemove('user:alice', dots); + expect(Object.isFrozen(op.observedDots)).toBe(true); + }); + + it('is instanceof Op and NodeRemove', () => { + const op = new NodeRemove('user:alice', []); + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(NodeRemove); + }); + + it('throws on empty nodeId', () => { + expect(() => new NodeRemove('', [])).toThrow(); + }); + + it('throws when observedDots is not an array', () => { + expect(() => new NodeRemove('n1', /** @type {any} */ ('alice:1'))).toThrow(); + }); + + it('accepts empty observedDots array', () => { + const op = new NodeRemove('user:alice', []); + expect(op.observedDots).toEqual([]); + }); +}); + +describe('EdgeAdd', () => { + it('constructs with valid from, to, label, and dot', () => { + const dot = new Dot('alice', 1); + const op = new EdgeAdd({ from: 'user:alice', to: 'user:bob', label: 'follows', dot }); + + expect(op.type).toBe('EdgeAdd'); + expect(op.from).toBe('user:alice'); + expect(op.to).toBe('user:bob'); + expect(op.label).toBe('follows'); + expect(op.dot).toBe(dot); + }); + + it('is frozen', () => { + const dot = new Dot('alice', 1); + const op = new EdgeAdd({ from: 'user:alice', to: 'user:bob', label: 'follows', dot }); + expect(Object.isFrozen(op)).toBe(true); + }); + + it('is instanceof Op and EdgeAdd', () => { + const dot = new Dot('alice', 1); + const op = new EdgeAdd({ from: 'user:alice', to: 'user:bob', label: 'follows', dot }); + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(EdgeAdd); + expect(op).not.toBeInstanceOf(NodeAdd); + }); + + it('throws on empty from', () => { + const dot = new Dot('alice', 1); + expect(() => new EdgeAdd({ from: '', to: 'n2', label: 'rel', dot })).toThrow(); + }); + + it('throws on empty to', () => { + const dot = new Dot('alice', 1); + expect(() => new EdgeAdd({ from: 'n1', to: '', label: 'rel', dot })).toThrow(); + }); + + it('throws on empty label', () => { + const dot = new Dot('alice', 1); + expect(() => new EdgeAdd({ from: 'n1', to: 'n2', label: '', dot })).toThrow(); + }); + + it('throws when dot is not a Dot instance', () => { + expect(() => new EdgeAdd({ from: 'n1', to: 'n2', label: 'rel', dot: /** @type {any} */ ({ writerId: 'w', counter: 1 }) })).toThrow(); + }); + + it('rejects from/to/label containing NUL byte', () => { + const dot = new Dot('alice', 1); + expect(() => new EdgeAdd({ from: 'n\x001', to: 'n2', label: 'rel', dot })).toThrow(); + expect(() => new EdgeAdd({ from: 'n1', to: 'n\x002', label: 'rel', dot })).toThrow(); + expect(() => new EdgeAdd({ from: 'n1', to: 'n2', label: 'r\x00l', dot })).toThrow(); + }); +}); + +describe('EdgeRemove', () => { + it('constructs with valid from, to, label, and observedDots', () => { + const op = new EdgeRemove({ from: 'user:alice', to: 'user:bob', label: 'follows', observedDots: ['alice:1'] }); + + expect(op.type).toBe('EdgeRemove'); + expect(op.from).toBe('user:alice'); + expect(op.to).toBe('user:bob'); + expect(op.label).toBe('follows'); + expect(op.observedDots).toEqual(['alice:1']); + }); + + it('is frozen', () => { + const op = new EdgeRemove({ from: 'n1', to: 'n2', label: 'rel', observedDots: [] }); + expect(Object.isFrozen(op)).toBe(true); + }); + + it('freezes the observedDots array', () => { + const op = new EdgeRemove({ from: 'n1', to: 'n2', label: 'rel', observedDots: ['w:1'] }); + expect(Object.isFrozen(op.observedDots)).toBe(true); + }); + + it('is instanceof Op and EdgeRemove', () => { + const op = new EdgeRemove({ from: 'n1', to: 'n2', label: 'rel', observedDots: [] }); + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(EdgeRemove); + }); + + it('throws on empty from', () => { + expect(() => new EdgeRemove({ from: '', to: 'n2', label: 'rel', observedDots: [] })).toThrow(); + }); + + it('throws on empty to', () => { + expect(() => new EdgeRemove({ from: 'n1', to: '', label: 'rel', observedDots: [] })).toThrow(); + }); + + it('throws on empty label', () => { + expect(() => new EdgeRemove({ from: 'n1', to: 'n2', label: '', observedDots: [] })).toThrow(); + }); + + it('throws when observedDots is not an array', () => { + expect(() => new EdgeRemove({ from: 'n1', to: 'n2', label: 'rel', observedDots: /** @type {any} */ ('w:1') })).toThrow(); + }); +}); + +describe('NodePropSet', () => { + it('constructs with valid node, key, value', () => { + const op = new NodePropSet('user:alice', 'name', 'Alice'); + + expect(op.type).toBe('NodePropSet'); + expect(op.node).toBe('user:alice'); + expect(op.key).toBe('name'); + expect(op.value).toBe('Alice'); + }); + + it('is frozen', () => { + const op = new NodePropSet('n1', 'k', 'v'); + expect(Object.isFrozen(op)).toBe(true); + }); + + it('is instanceof Op and NodePropSet', () => { + const op = new NodePropSet('n1', 'k', 'v'); + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(NodePropSet); + expect(op).not.toBeInstanceOf(EdgePropSet); + expect(op).not.toBeInstanceOf(PropSet); + }); + + it('throws on empty node', () => { + expect(() => new NodePropSet('', 'k', 'v')).toThrow(); + }); + + it('throws on empty key', () => { + expect(() => new NodePropSet('n1', '', 'v')).toThrow(); + }); + + it('accepts null value', () => { + const op = new NodePropSet('n1', 'k', null); + expect(op.value).toBeNull(); + }); + + it('accepts object value', () => { + const op = new NodePropSet('n1', 'k', { nested: true }); + expect(op.value).toEqual({ nested: true }); + }); + + it('accepts number value', () => { + const op = new NodePropSet('n1', 'age', 42); + expect(op.value).toBe(42); + }); +}); + +describe('EdgePropSet', () => { + it('constructs with valid from, to, label, key, value', () => { + const op = new EdgePropSet({ from: 'user:alice', to: 'user:bob', label: 'follows', key: 'since', value: '2026-01-01' }); + + expect(op.type).toBe('EdgePropSet'); + expect(op.from).toBe('user:alice'); + expect(op.to).toBe('user:bob'); + expect(op.label).toBe('follows'); + expect(op.key).toBe('since'); + expect(op.value).toBe('2026-01-01'); + }); + + it('is frozen', () => { + const op = new EdgePropSet({ from: 'n1', to: 'n2', label: 'rel', key: 'k', value: 'v' }); + expect(Object.isFrozen(op)).toBe(true); + }); + + it('is instanceof Op and EdgePropSet', () => { + const op = new EdgePropSet({ from: 'n1', to: 'n2', label: 'rel', key: 'k', value: 'v' }); + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(EdgePropSet); + expect(op).not.toBeInstanceOf(NodePropSet); + expect(op).not.toBeInstanceOf(PropSet); + }); + + it('throws on empty from', () => { + expect(() => new EdgePropSet({ from: '', to: 'n2', label: 'rel', key: 'k', value: 'v' })).toThrow(); + }); + + it('throws on empty to', () => { + expect(() => new EdgePropSet({ from: 'n1', to: '', label: 'rel', key: 'k', value: 'v' })).toThrow(); + }); + + it('throws on empty label', () => { + expect(() => new EdgePropSet({ from: 'n1', to: 'n2', label: '', key: 'k', value: 'v' })).toThrow(); + }); + + it('throws on empty key', () => { + expect(() => new EdgePropSet({ from: 'n1', to: 'n2', label: 'rel', key: '', value: 'v' })).toThrow(); + }); + + it('accepts null value', () => { + const op = new EdgePropSet({ from: 'n1', to: 'n2', label: 'rel', key: 'k', value: null }); + expect(op.value).toBeNull(); + }); +}); + +describe('PropSet (raw/wire format)', () => { + it('constructs with valid node, key, value', () => { + const op = new PropSet('user:alice', 'name', 'Alice'); + + expect(op.type).toBe('PropSet'); + expect(op.node).toBe('user:alice'); + expect(op.key).toBe('name'); + expect(op.value).toBe('Alice'); + }); + + it('is frozen', () => { + const op = new PropSet('n1', 'k', 'v'); + expect(Object.isFrozen(op)).toBe(true); + }); + + it('is instanceof Op and PropSet', () => { + const op = new PropSet('n1', 'k', 'v'); + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(PropSet); + expect(op).not.toBeInstanceOf(NodePropSet); + expect(op).not.toBeInstanceOf(EdgePropSet); + }); + + it('throws on empty node', () => { + expect(() => new PropSet('', 'k', 'v')).toThrow(); + }); + + it('throws on empty key', () => { + expect(() => new PropSet('n1', '', 'v')).toThrow(); + }); + + it('accepts edge-property encoded node (\\x01 prefix)', () => { + const op = new PropSet('\x01n1\x00n2\x00rel', 'k', 'v'); + expect(op.node).toBe('\x01n1\x00n2\x00rel'); + }); +}); + +describe('BlobValue', () => { + it('constructs with valid node and oid', () => { + const op = new BlobValue('user:alice', 'abc123def456'); + + expect(op.type).toBe('BlobValue'); + expect(op.node).toBe('user:alice'); + expect(op.oid).toBe('abc123def456'); + }); + + it('is frozen', () => { + const op = new BlobValue('n1', 'oid123'); + expect(Object.isFrozen(op)).toBe(true); + }); + + it('is instanceof Op and BlobValue', () => { + const op = new BlobValue('n1', 'oid123'); + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(BlobValue); + expect(op).not.toBeInstanceOf(NodeAdd); + }); + + it('throws on empty node', () => { + expect(() => new BlobValue('', 'oid')).toThrow(); + }); + + it('throws on empty oid', () => { + expect(() => new BlobValue('n1', '')).toThrow(); + }); + + it('throws on non-string oid', () => { + expect(() => new BlobValue('n1', /** @type {any} */ (42))).toThrow(); + }); +}); + +describe('cross-class instanceof isolation', () => { + it('no op is instanceof a sibling class', () => { + const dot = new Dot('w', 1); + const all = [ + new NodeAdd('n1', dot), + new NodeRemove('n1', []), + new EdgeAdd({ from: 'n1', to: 'n2', label: 'r', dot }), + new EdgeRemove({ from: 'n1', to: 'n2', label: 'r', observedDots: [] }), + new NodePropSet('n1', 'k', 'v'), + new EdgePropSet({ from: 'n1', to: 'n2', label: 'r', key: 'k', value: 'v' }), + new PropSet('n1', 'k', 'v'), + new BlobValue('n1', 'oid'), + ]; + const classes = [NodeAdd, NodeRemove, EdgeAdd, EdgeRemove, NodePropSet, EdgePropSet, PropSet, BlobValue]; + + for (let i = 0; i < all.length; i++) { + for (let j = 0; j < classes.length; j++) { + if (i === j) { + expect(all[i]).toBeInstanceOf(classes[j]); + } else { + expect(all[i]).not.toBeInstanceOf(classes[j]); + } + } + } + }); + + it('all ops share the Op base', () => { + const dot = new Dot('w', 1); + const all = [ + new NodeAdd('n1', dot), + new NodeRemove('n1', []), + new EdgeAdd({ from: 'n1', to: 'n2', label: 'r', dot }), + new EdgeRemove({ from: 'n1', to: 'n2', label: 'r', observedDots: [] }), + new NodePropSet('n1', 'k', 'v'), + new EdgePropSet({ from: 'n1', to: 'n2', label: 'r', key: 'k', value: 'v' }), + new PropSet('n1', 'k', 'v'), + new BlobValue('n1', 'oid'), + ]; + + for (const op of all) { + expect(op).toBeInstanceOf(Op); + expect(typeof op.type).toBe('string'); + expect(op.type.length).toBeGreaterThan(0); + } + }); +}); diff --git a/test/unit/domain/types/ops/validate.test.js b/test/unit/domain/types/ops/validate.test.js new file mode 100644 index 00000000..6db8cdc5 --- /dev/null +++ b/test/unit/domain/types/ops/validate.test.js @@ -0,0 +1,51 @@ +import { describe, it, expect } from 'vitest'; +import { assertNonEmptyString, assertNoBannedBytes, assertArray } from '../../../../../src/domain/types/ops/validate.js'; + +describe('assertNonEmptyString', () => { + it('accepts a non-empty string', () => { + expect(() => assertNonEmptyString('hello', 'Test', 'field')).not.toThrow(); + }); + + it('throws on empty string', () => { + expect(() => assertNonEmptyString('', 'TestOp', 'node')).toThrow("TestOp requires 'node' to be a non-empty string"); + }); + + it('throws on non-string types', () => { + expect(() => assertNonEmptyString(42, 'X', 'f')).toThrow(); + expect(() => assertNonEmptyString(null, 'X', 'f')).toThrow(); + expect(() => assertNonEmptyString(undefined, 'X', 'f')).toThrow(); + expect(() => assertNonEmptyString(true, 'X', 'f')).toThrow(); + expect(() => assertNonEmptyString({}, 'X', 'f')).toThrow(); + }); +}); + +describe('assertNoBannedBytes', () => { + it('accepts a clean string', () => { + expect(() => assertNoBannedBytes('user:alice', 'Test', 'node')).not.toThrow(); + }); + + it('throws on NUL byte', () => { + expect(() => assertNoBannedBytes('user\x00alice', 'TestOp', 'node')).toThrow("TestOp 'node' must not contain NUL"); + }); + + it('accepts strings with other special characters', () => { + expect(() => assertNoBannedBytes('user\x01alice', 'Test', 'node')).not.toThrow(); + expect(() => assertNoBannedBytes('user:alice:bob', 'Test', 'node')).not.toThrow(); + }); +}); + +describe('assertArray', () => { + it('accepts an array', () => { + expect(() => assertArray([], 'Test', 'dots')).not.toThrow(); + expect(() => assertArray(['a', 'b'], 'Test', 'dots')).not.toThrow(); + }); + + it('throws on non-array types', () => { + expect(() => assertArray('hello', 'TestOp', 'dots')).toThrow("TestOp requires 'dots' to be an array"); + expect(() => assertArray(42, 'X', 'f')).toThrow(); + expect(() => assertArray(null, 'X', 'f')).toThrow(); + expect(() => assertArray(undefined, 'X', 'f')).toThrow(); + expect(() => assertArray({}, 'X', 'f')).toThrow(); + expect(() => assertArray(new Set(), 'X', 'f')).toThrow(); + }); +}); From e97fbabccc9c561188903dcd74bc15e40937d8b7 Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 5 Apr 2026 11:35:56 -0700 Subject: [PATCH 03/13] =?UTF-8?q?feat:=20wire=20factory=20functions=20+=20?= =?UTF-8?q?OpNormalizer=20to=20Op=20classes=20=E2=80=94=20Slice=202?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Factory functions in WarpTypesV2.js now delegate to Op class constructors. OpNormalizer.normalizeRawOp() and lowerCanonicalOp() already return class instances via the factory functions. Updated WarpTypesV2.test.js: real Dot instances replace plain objects, encoded dot strings replace object arrays for observedDots. 15 new factory-integration tests + 35 updated existing tests. Cycle 0009 — op-type-class-hierarchy --- src/domain/types/WarpTypesV2.js | 21 +- test/unit/domain/types/WarpTypesV2.test.js | 269 ++++++++---------- .../types/ops/factory-integration.test.js | 198 +++++++++++++ 3 files changed, 330 insertions(+), 158 deletions(-) create mode 100644 test/unit/domain/types/ops/factory-integration.test.js diff --git a/src/domain/types/WarpTypesV2.js b/src/domain/types/WarpTypesV2.js index 6479fd24..05e6ff6c 100644 --- a/src/domain/types/WarpTypesV2.js +++ b/src/domain/types/WarpTypesV2.js @@ -15,6 +15,13 @@ */ import PatchV2 from './PatchV2.js'; +import NodeAddClass from './ops/NodeAdd.js'; +import NodeRemoveClass from './ops/NodeRemove.js'; +import EdgeAddClass from './ops/EdgeAdd.js'; +import EdgeRemoveClass from './ops/EdgeRemove.js'; +import NodePropSetClass from './ops/NodePropSet.js'; +import EdgePropSetClass from './ops/EdgePropSet.js'; +import PropSetOpClass from './ops/PropSet.js'; // Re-export PatchV2 class for consumers that import from this module. export { PatchV2 }; @@ -151,7 +158,7 @@ export { PatchV2 }; * @returns {OpV2NodeAdd} NodeAdd operation */ export function createNodeAddV2(node, dot) { - return { type: 'NodeAdd', node, dot }; + return new NodeAddClass(node, dot); } /** @@ -161,7 +168,7 @@ export function createNodeAddV2(node, dot) { * @returns {OpV2NodeRemove} NodeRemove operation */ export function createNodeRemoveV2(node, observedDots) { - return { type: 'NodeRemove', node, observedDots }; + return new NodeRemoveClass(node, observedDots); } /** @@ -173,7 +180,7 @@ export function createNodeRemoveV2(node, observedDots) { * @returns {OpV2EdgeAdd} EdgeAdd operation */ export function createEdgeAddV2(from, to, label, dot) { - return { type: 'EdgeAdd', from, to, label, dot }; + return new EdgeAddClass({ from, to, label, dot }); } /** @@ -185,7 +192,7 @@ export function createEdgeAddV2(from, to, label, dot) { * @returns {OpV2EdgeRemove} EdgeRemove operation */ export function createEdgeRemoveV2(from, to, label, observedDots) { - return { type: 'EdgeRemove', from, to, label, observedDots }; + return new EdgeRemoveClass({ from, to, label, observedDots }); } /** @@ -198,7 +205,7 @@ export function createEdgeRemoveV2(from, to, label, observedDots) { * @returns {OpV2PropSet} PropSet operation */ export function createPropSetV2(node, key, value) { - return { type: 'PropSet', node, key, value }; + return new PropSetOpClass(node, key, value); } /** @@ -209,7 +216,7 @@ export function createPropSetV2(node, key, value) { * @returns {OpV2NodePropSet} NodePropSet operation */ export function createNodePropSetV2(node, key, value) { - return { type: 'NodePropSet', node, key, value }; + return new NodePropSetClass(node, key, value); } /** @@ -222,7 +229,7 @@ export function createNodePropSetV2(node, key, value) { * @returns {OpV2EdgePropSet} EdgePropSet operation */ export function createEdgePropSetV2(from, to, label, key, value) { - return { type: 'EdgePropSet', from, to, label, key, value }; + return new EdgePropSetClass({ from, to, label, key, value }); } // ============================================================================ diff --git a/test/unit/domain/types/WarpTypesV2.test.js b/test/unit/domain/types/WarpTypesV2.test.js index b68c293a..0c4df8b6 100644 --- a/test/unit/domain/types/WarpTypesV2.test.js +++ b/test/unit/domain/types/WarpTypesV2.test.js @@ -1,141 +1,117 @@ import { describe, it, expect } from 'vitest'; +import { Dot } from '../../../../src/domain/crdt/Dot.js'; +import Op from '../../../../src/domain/types/ops/Op.js'; +import NodeAdd from '../../../../src/domain/types/ops/NodeAdd.js'; +import NodeRemove from '../../../../src/domain/types/ops/NodeRemove.js'; +import EdgeAdd from '../../../../src/domain/types/ops/EdgeAdd.js'; +import EdgeRemove from '../../../../src/domain/types/ops/EdgeRemove.js'; +import PropSetClass from '../../../../src/domain/types/ops/PropSet.js'; import { - createNodeAddV2 as _createNodeAddV2, - createNodeRemoveV2 as _createNodeRemoveV2, - createEdgeAddV2 as _createEdgeAddV2, - createEdgeRemoveV2 as _createEdgeRemoveV2, + createNodeAddV2, + createNodeRemoveV2, + createEdgeAddV2, + createEdgeRemoveV2, createPropSetV2, createPatchV2, } from '../../../../src/domain/types/WarpTypesV2.js'; -/** @type {any} */ -const createNodeAddV2 = _createNodeAddV2; -/** @type {any} */ -const createNodeRemoveV2 = _createNodeRemoveV2; -/** @type {any} */ -const createEdgeAddV2 = _createEdgeAddV2; -/** @type {any} */ -const createEdgeRemoveV2 = _createEdgeRemoveV2; - describe('WarpTypesV2', () => { describe('Operation Factory Functions', () => { describe('createNodeAddV2', () => { it('creates NodeAdd operation with dot', () => { - const dot = { writer: 'writer-1', seq: 1 }; + const dot = new Dot('writer-1', 1); const result = createNodeAddV2('user:alice', dot); - expect(result).toEqual({ - type: 'NodeAdd', - node: 'user:alice', - dot: { writer: 'writer-1', seq: 1 }, - }); + expect(result).toBeInstanceOf(NodeAdd); + expect(result.type).toBe('NodeAdd'); + expect(result.node).toBe('user:alice'); + expect(result.dot).toBe(dot); }); it('creates NodeAdd with UUID-style node ID', () => { const nodeId = '550e8400-e29b-41d4-a716-446655440000'; - const dot = { writer: 'writer-2', seq: 5 }; + const dot = new Dot('writer-2', 5); const result = createNodeAddV2(nodeId, dot); - expect(result).toEqual({ - type: 'NodeAdd', - node: nodeId, - dot: { writer: 'writer-2', seq: 5 }, - }); + expect(result).toBeInstanceOf(NodeAdd); + expect(result.node).toBe(nodeId); + expect(result.dot.counter).toBe(5); }); - it('creates NodeAdd with high sequence number', () => { - const dot = { writer: 'prolific-writer', seq: 999999 }; + it('creates NodeAdd with high counter', () => { + const dot = new Dot('prolific-writer', 999999); const result = createNodeAddV2('node:high-seq', dot); - expect(result.dot.seq).toBe(999999); + expect(result.dot.counter).toBe(999999); }); }); describe('createNodeRemoveV2', () => { it('creates NodeRemove operation with single observed dot', () => { - const observedDots = [{ writer: 'writer-1', seq: 1 }]; - const result = createNodeRemoveV2('user:bob', observedDots); + const result = createNodeRemoveV2('user:bob', ['writer-1:1']); - expect(result).toEqual({ - type: 'NodeRemove', - node: 'user:bob', - observedDots: [{ writer: 'writer-1', seq: 1 }], - }); + expect(result).toBeInstanceOf(NodeRemove); + expect(result.type).toBe('NodeRemove'); + expect(result.node).toBe('user:bob'); + expect(result.observedDots).toEqual(['writer-1:1']); }); it('creates NodeRemove with multiple observed dots', () => { - const observedDots = [ - { writer: 'writer-1', seq: 1 }, - { writer: 'writer-2', seq: 3 }, - { writer: 'writer-1', seq: 5 }, - ]; + const observedDots = ['writer-1:1', 'writer-2:3', 'writer-1:5']; const result = createNodeRemoveV2('user:charlie', observedDots); expect(result.observedDots).toHaveLength(3); - expect(result.observedDots).toContainEqual({ writer: 'writer-1', seq: 1 }); - expect(result.observedDots).toContainEqual({ writer: 'writer-2', seq: 3 }); - expect(result.observedDots).toContainEqual({ writer: 'writer-1', seq: 5 }); + expect(result.observedDots).toContain('writer-1:1'); + expect(result.observedDots).toContain('writer-2:3'); + expect(result.observedDots).toContain('writer-1:5'); }); it('creates NodeRemove with empty observed dots', () => { const result = createNodeRemoveV2('user:unknown', []); - expect(result).toEqual({ - type: 'NodeRemove', - node: 'user:unknown', - observedDots: [], - }); + expect(result).toBeInstanceOf(NodeRemove); + expect(result.observedDots).toEqual([]); }); }); describe('createEdgeAddV2', () => { it('creates EdgeAdd operation with dot', () => { - const dot = { writer: 'writer-1', seq: 2 }; + const dot = new Dot('writer-1', 2); const result = createEdgeAddV2('user:alice', 'user:bob', 'follows', dot); - expect(result).toEqual({ - type: 'EdgeAdd', - from: 'user:alice', - to: 'user:bob', - label: 'follows', - dot: { writer: 'writer-1', seq: 2 }, - }); + expect(result).toBeInstanceOf(EdgeAdd); + expect(result.type).toBe('EdgeAdd'); + expect(result.from).toBe('user:alice'); + expect(result.to).toBe('user:bob'); + expect(result.label).toBe('follows'); + expect(result.dot).toBe(dot); }); it('creates EdgeAdd with different label', () => { - const dot = { writer: 'writer-2', seq: 10 }; + const dot = new Dot('writer-2', 10); const result = createEdgeAddV2('post:123', 'user:alice', 'authored_by', dot); - expect(result).toEqual({ - type: 'EdgeAdd', - from: 'post:123', - to: 'user:alice', - label: 'authored_by', - dot: { writer: 'writer-2', seq: 10 }, - }); + expect(result).toBeInstanceOf(EdgeAdd); + expect(result.from).toBe('post:123'); + expect(result.to).toBe('user:alice'); + expect(result.label).toBe('authored_by'); }); }); describe('createEdgeRemoveV2', () => { it('creates EdgeRemove operation with single observed dot', () => { - const observedDots = [{ writer: 'writer-1', seq: 2 }]; - const result = createEdgeRemoveV2('user:alice', 'user:bob', 'follows', observedDots); - - expect(result).toEqual({ - type: 'EdgeRemove', - from: 'user:alice', - to: 'user:bob', - label: 'follows', - observedDots: [{ writer: 'writer-1', seq: 2 }], - }); + const result = createEdgeRemoveV2('user:alice', 'user:bob', 'follows', ['writer-1:2']); + + expect(result).toBeInstanceOf(EdgeRemove); + expect(result.type).toBe('EdgeRemove'); + expect(result.from).toBe('user:alice'); + expect(result.to).toBe('user:bob'); + expect(result.label).toBe('follows'); + expect(result.observedDots).toEqual(['writer-1:2']); }); it('creates EdgeRemove with multiple observed dots from concurrent adds', () => { - const observedDots = [ - { writer: 'writer-1', seq: 2 }, - { writer: 'writer-2', seq: 1 }, - ]; - const result = createEdgeRemoveV2('user:alice', 'user:bob', 'follows', observedDots); + const result = createEdgeRemoveV2('user:alice', 'user:bob', 'follows', ['writer-1:2', 'writer-2:1']); expect(result.observedDots).toHaveLength(2); }); @@ -145,35 +121,24 @@ describe('WarpTypesV2', () => { it('creates PropSet operation with string value (no dot)', () => { const result = createPropSetV2('user:alice', 'name', 'Alice'); - expect(result).toEqual({ - type: 'PropSet', - node: 'user:alice', - key: 'name', - value: 'Alice', - }); + expect(result).toBeInstanceOf(PropSetClass); + expect(result.type).toBe('PropSet'); + expect(result.node).toBe('user:alice'); + expect(result.key).toBe('name'); + expect(result.value).toBe('Alice'); }); it('creates PropSet with number value', () => { const result = createPropSetV2('user:alice', 'age', 30); - expect(result).toEqual({ - type: 'PropSet', - node: 'user:alice', - key: 'age', - value: 30, - }); + expect(result.value).toBe(30); }); it('creates PropSet with object value', () => { const settings = { theme: 'dark', notifications: true }; const result = createPropSetV2('user:alice', 'settings', settings); - expect(result).toEqual({ - type: 'PropSet', - node: 'user:alice', - key: 'settings', - value: { theme: 'dark', notifications: true }, - }); + expect(result.value).toEqual({ theme: 'dark', notifications: true }); }); it('creates PropSet with array value', () => { @@ -206,7 +171,7 @@ describe('WarpTypesV2', () => { describe('Patch Factory Function', () => { describe('createPatchV2', () => { it('creates PatchV2 with required fields', () => { - const dot = { writer: 'writer-1', seq: 1 }; + const dot = new Dot('writer-1', 1); const ops = [createNodeAddV2('user:alice', dot)]; const context = { 'writer-1': 0 }; @@ -217,13 +182,12 @@ describe('WarpTypesV2', () => { ops, }); - expect(result).toEqual({ - schema: 2, - writer: 'writer-1', - lamport: 1, - context: { 'writer-1': 0 }, - ops: [{ type: 'NodeAdd', node: 'user:alice', dot: { writer: 'writer-1', seq: 1 } }], - }); + expect(result.schema).toBe(2); + expect(result.writer).toBe('writer-1'); + expect(result.lamport).toBe(1); + expect(result.ops).toHaveLength(1); + expect(result.ops[0]).toBeInstanceOf(NodeAdd); + expect(result.ops[0]?.type).toBe('NodeAdd'); }); it('creates PatchV2 with explicit schema', () => { @@ -260,9 +224,9 @@ describe('WarpTypesV2', () => { it('creates PatchV2 with multiple operations', () => { const ops = [ - createNodeAddV2('user:alice', { writer: 'writer-1', seq: 1 }), - createNodeAddV2('user:bob', { writer: 'writer-1', seq: 2 }), - createEdgeAddV2('user:alice', 'user:bob', 'follows', { writer: 'writer-1', seq: 3 }), + createNodeAddV2('user:alice', new Dot('writer-1', 1)), + createNodeAddV2('user:bob', new Dot('writer-1', 2)), + createEdgeAddV2('user:alice', 'user:bob', 'follows', new Dot('writer-1', 3)), createPropSetV2('user:alice', 'name', 'Alice'), ]; const result = createPatchV2({ @@ -290,13 +254,10 @@ describe('WarpTypesV2', () => { ops: [], }); - expect(result).toEqual({ - schema: 2, - writer: 'writer-1', - lamport: 0, - context: {}, - ops: [], - }); + expect(result.schema).toBe(2); + expect(result.writer).toBe('writer-1'); + expect(result.lamport).toBe(0); + expect(result.ops).toEqual([]); }); it('always sets schema to 2 by default', () => { @@ -314,10 +275,12 @@ describe('WarpTypesV2', () => { describe('Type Discriminators', () => { it('all operation types have distinct type field', () => { - const nodeAdd = createNodeAddV2('n1', { writer: 'w', seq: 1 }); - const nodeRemove = createNodeRemoveV2('n1', [{ writer: 'w', seq: 1 }]); - const edgeAdd = createEdgeAddV2('n1', 'n2', 'rel', { writer: 'w', seq: 2 }); - const edgeRemove = createEdgeRemoveV2('n1', 'n2', 'rel', [{ writer: 'w', seq: 2 }]); + const dot1 = new Dot('w', 1); + const dot2 = new Dot('w', 2); + const nodeAdd = createNodeAddV2('n1', dot1); + const nodeRemove = createNodeRemoveV2('n1', ['w:1']); + const edgeAdd = createEdgeAddV2('n1', 'n2', 'rel', dot2); + const edgeRemove = createEdgeRemoveV2('n1', 'n2', 'rel', ['w:2']); const propSet = createPropSetV2('n1', 'key', 'val'); const types = [ @@ -328,24 +291,23 @@ describe('WarpTypesV2', () => { propSet.type, ]; - // All types should be unique const uniqueTypes = new Set(types); expect(uniqueTypes.size).toBe(5); }); it('add operations have dot, remove operations have observedDots', () => { - const nodeAdd = createNodeAddV2('n1', { writer: 'w', seq: 1 }); - const nodeRemove = createNodeRemoveV2('n1', [{ writer: 'w', seq: 1 }]); - const edgeAdd = createEdgeAddV2('n1', 'n2', 'rel', { writer: 'w', seq: 2 }); - const edgeRemove = createEdgeRemoveV2('n1', 'n2', 'rel', [{ writer: 'w', seq: 2 }]); + const dot1 = new Dot('w', 1); + const dot2 = new Dot('w', 2); + const nodeAdd = createNodeAddV2('n1', dot1); + const nodeRemove = createNodeRemoveV2('n1', ['w:1']); + const edgeAdd = createEdgeAddV2('n1', 'n2', 'rel', dot2); + const edgeRemove = createEdgeRemoveV2('n1', 'n2', 'rel', ['w:2']); - // Add operations have dot expect(nodeAdd).toHaveProperty('dot'); expect(edgeAdd).toHaveProperty('dot'); expect(nodeAdd).not.toHaveProperty('observedDots'); expect(edgeAdd).not.toHaveProperty('observedDots'); - // Remove operations have observedDots expect(nodeRemove).toHaveProperty('observedDots'); expect(edgeRemove).toHaveProperty('observedDots'); expect(nodeRemove).not.toHaveProperty('dot'); @@ -362,13 +324,12 @@ describe('WarpTypesV2', () => { describe('Integration - Building Complete Patches', () => { it('creates a realistic patch with mixed operations', () => { - // Simulate creating a user and setting properties const patch = createPatchV2({ writer: 'app-server-1', lamport: 42, context: { 'app-server-1': 0 }, ops: [ - createNodeAddV2('user:123', { writer: 'app-server-1', seq: 1 }), + createNodeAddV2('user:123', new Dot('app-server-1', 1)), createPropSetV2('user:123', 'email', 'alice@example.com'), createPropSetV2('user:123', 'name', 'Alice'), createPropSetV2('user:123', 'verified', true), @@ -377,11 +338,8 @@ describe('WarpTypesV2', () => { expect(patch.schema).toBe(2); expect(patch.ops).toHaveLength(4); - expect(patch.ops[0]).toEqual({ - type: 'NodeAdd', - node: 'user:123', - dot: { writer: 'app-server-1', seq: 1 }, - }); + expect(patch.ops[0]).toBeInstanceOf(NodeAdd); + expect(patch.ops[0]?.type).toBe('NodeAdd'); expect(patch.ops[1]?.type).toBe('PropSet'); expect(/** @type {any} */ (patch.ops[1])?.value).toBe('alice@example.com'); }); @@ -392,10 +350,10 @@ describe('WarpTypesV2', () => { lamport: 100, context: { 'social-service': 0, 'user-service': 5 }, ops: [ - createNodeAddV2('user:alice', { writer: 'social-service', seq: 1 }), - createNodeAddV2('user:bob', { writer: 'social-service', seq: 2 }), - createEdgeAddV2('user:alice', 'user:bob', 'follows', { writer: 'social-service', seq: 3 }), - createEdgeAddV2('user:bob', 'user:alice', 'follows', { writer: 'social-service', seq: 4 }), + createNodeAddV2('user:alice', new Dot('social-service', 1)), + createNodeAddV2('user:bob', new Dot('social-service', 2)), + createEdgeAddV2('user:alice', 'user:bob', 'follows', new Dot('social-service', 3)), + createEdgeAddV2('user:bob', 'user:alice', 'follows', new Dot('social-service', 4)), createPropSetV2('user:alice', 'followingCount', 1), createPropSetV2('user:bob', 'followingCount', 1), ], @@ -403,16 +361,14 @@ describe('WarpTypesV2', () => { expect(patch.ops).toHaveLength(6); - // Verify edge operations have dots const edges = patch.ops.filter((op) => op.type === 'EdgeAdd'); expect(edges).toHaveLength(2); expect(edges[0]?.label).toBe('follows'); - expect(/** @type {any} */ (edges[0])?.dot.seq).toBe(3); - expect(/** @type {any} */ (edges[1])?.dot.seq).toBe(4); + expect(/** @type {any} */ (edges[0])?.dot.counter).toBe(3); + expect(/** @type {any} */ (edges[1])?.dot.counter).toBe(4); }); it('creates a deletion patch with observed dots', () => { - // Simulate removing items that were added by different writers const patch = createPatchV2({ writer: 'cleanup-job', lamport: 200, @@ -422,13 +378,8 @@ describe('WarpTypesV2', () => { 'user-service': 10, }, ops: [ - createEdgeRemoveV2('user:alice', 'user:bob', 'follows', [ - { writer: 'social-service', seq: 3 }, - ]), - createNodeRemoveV2('user:bob', [ - { writer: 'social-service', seq: 2 }, - { writer: 'user-service', seq: 7 }, // concurrent add from another writer - ]), + createEdgeRemoveV2('user:alice', 'user:bob', 'follows', ['social-service:3']), + createNodeRemoveV2('user:bob', ['social-service:2', 'user-service:7']), ], }); @@ -440,7 +391,6 @@ describe('WarpTypesV2', () => { }); it('creates a merge-scenario patch observing multiple writers', () => { - // Writer-3 has observed state from writer-1 and writer-2 const patch = createPatchV2({ writer: 'writer-3', lamport: 50, @@ -450,8 +400,8 @@ describe('WarpTypesV2', () => { 'writer-3': 0, }, ops: [ - createNodeAddV2('merged:node', { writer: 'writer-3', seq: 1 }), - createEdgeAddV2('node:from-w1', 'node:from-w2', 'links', { writer: 'writer-3', seq: 2 }), + createNodeAddV2('merged:node', new Dot('writer-3', 1)), + createEdgeAddV2('node:from-w1', 'node:from-w2', 'links', new Dot('writer-3', 2)), ], }); @@ -486,4 +436,21 @@ describe('WarpTypesV2', () => { expect(patch.schema).toBe(2); }); }); + + describe('Op class instanceof', () => { + it('all factory-created ops are instanceof Op', () => { + const dot = new Dot('w', 1); + const ops = [ + createNodeAddV2('n1', dot), + createNodeRemoveV2('n1', []), + createEdgeAddV2('n1', 'n2', 'r', dot), + createEdgeRemoveV2('n1', 'n2', 'r', []), + createPropSetV2('n1', 'k', 'v'), + ]; + + for (const op of ops) { + expect(op).toBeInstanceOf(Op); + } + }); + }); }); diff --git a/test/unit/domain/types/ops/factory-integration.test.js b/test/unit/domain/types/ops/factory-integration.test.js new file mode 100644 index 00000000..680ef67c --- /dev/null +++ b/test/unit/domain/types/ops/factory-integration.test.js @@ -0,0 +1,198 @@ +/** + * Tests that WarpTypesV2 factory functions produce Op class instances + * and that OpNormalizer returns class instances. + */ +import { describe, it, expect } from 'vitest'; +import { Dot } from '../../../../../src/domain/crdt/Dot.js'; +import Op from '../../../../../src/domain/types/ops/Op.js'; +import NodeAdd from '../../../../../src/domain/types/ops/NodeAdd.js'; +import NodeRemove from '../../../../../src/domain/types/ops/NodeRemove.js'; +import EdgeAdd from '../../../../../src/domain/types/ops/EdgeAdd.js'; +import EdgeRemove from '../../../../../src/domain/types/ops/EdgeRemove.js'; +import NodePropSet from '../../../../../src/domain/types/ops/NodePropSet.js'; +import EdgePropSet from '../../../../../src/domain/types/ops/EdgePropSet.js'; +import PropSetClass from '../../../../../src/domain/types/ops/PropSet.js'; +import BlobValue from '../../../../../src/domain/types/ops/BlobValue.js'; +import { + createNodeAddV2, + createNodeRemoveV2, + createEdgeAddV2, + createEdgeRemoveV2, + createPropSetV2, + createNodePropSetV2, + createEdgePropSetV2, +} from '../../../../../src/domain/types/WarpTypesV2.js'; +import { normalizeRawOp, lowerCanonicalOp } from '../../../../../src/domain/services/OpNormalizer.js'; + +describe('WarpTypesV2 factory functions produce Op class instances', () => { + it('createNodeAddV2 returns a NodeAdd instance', () => { + const dot = new Dot('alice', 1); + const op = createNodeAddV2('user:alice', dot); + + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(NodeAdd); + expect(op.type).toBe('NodeAdd'); + expect(op.node).toBe('user:alice'); + expect(op.dot).toBe(dot); + }); + + it('createNodeRemoveV2 returns a NodeRemove instance', () => { + const op = createNodeRemoveV2('user:alice', ['alice:1']); + + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(NodeRemove); + expect(op.type).toBe('NodeRemove'); + expect(op.node).toBe('user:alice'); + expect(op.observedDots).toEqual(['alice:1']); + }); + + it('createEdgeAddV2 returns an EdgeAdd instance', () => { + const dot = new Dot('alice', 1); + const op = createEdgeAddV2('n1', 'n2', 'rel', dot); + + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(EdgeAdd); + expect(op.type).toBe('EdgeAdd'); + expect(op.from).toBe('n1'); + expect(op.to).toBe('n2'); + expect(op.label).toBe('rel'); + expect(op.dot).toBe(dot); + }); + + it('createEdgeRemoveV2 returns an EdgeRemove instance', () => { + const op = createEdgeRemoveV2('n1', 'n2', 'rel', ['w:1']); + + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(EdgeRemove); + expect(op.type).toBe('EdgeRemove'); + expect(op.from).toBe('n1'); + expect(op.to).toBe('n2'); + expect(op.label).toBe('rel'); + expect(op.observedDots).toEqual(['w:1']); + }); + + it('createPropSetV2 returns a PropSet instance', () => { + const op = createPropSetV2('user:alice', 'name', 'Alice'); + + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(PropSetClass); + expect(op.type).toBe('PropSet'); + expect(op.node).toBe('user:alice'); + expect(op.key).toBe('name'); + expect(op.value).toBe('Alice'); + }); + + it('createNodePropSetV2 returns a NodePropSet instance', () => { + const op = createNodePropSetV2('user:alice', 'name', 'Alice'); + + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(NodePropSet); + expect(op.type).toBe('NodePropSet'); + expect(op.node).toBe('user:alice'); + expect(op.key).toBe('name'); + expect(op.value).toBe('Alice'); + }); + + it('createEdgePropSetV2 returns an EdgePropSet instance', () => { + const op = createEdgePropSetV2('n1', 'n2', 'rel', 'weight', 42); + + expect(op).toBeInstanceOf(Op); + expect(op).toBeInstanceOf(EdgePropSet); + expect(op.type).toBe('EdgePropSet'); + expect(op.from).toBe('n1'); + expect(op.to).toBe('n2'); + expect(op.label).toBe('rel'); + expect(op.key).toBe('weight'); + expect(op.value).toBe(42); + }); +}); + +describe('OpNormalizer returns Op class instances', () => { + it('normalizeRawOp passes NodeAdd through as-is', () => { + const dot = new Dot('w', 1); + const raw = createNodeAddV2('n1', dot); + const canonical = normalizeRawOp(raw); + + expect(canonical).toBe(raw); + expect(canonical).toBeInstanceOf(NodeAdd); + }); + + it('normalizeRawOp converts PropSet (node) to NodePropSet instance', () => { + const raw = createPropSetV2('user:alice', 'name', 'Alice'); + const canonical = normalizeRawOp(raw); + + expect(canonical).toBeInstanceOf(NodePropSet); + expect(canonical.type).toBe('NodePropSet'); + expect(canonical.node).toBe('user:alice'); + expect(canonical.key).toBe('name'); + expect(canonical.value).toBe('Alice'); + }); + + it('normalizeRawOp converts PropSet (edge) to EdgePropSet instance', () => { + const raw = createPropSetV2('\x01n1\x00n2\x00rel', 'weight', 42); + const canonical = normalizeRawOp(raw); + + expect(canonical).toBeInstanceOf(EdgePropSet); + expect(canonical.type).toBe('EdgePropSet'); + expect(canonical.from).toBe('n1'); + expect(canonical.to).toBe('n2'); + expect(canonical.label).toBe('rel'); + expect(canonical.key).toBe('weight'); + expect(canonical.value).toBe(42); + }); + + it('lowerCanonicalOp converts NodePropSet to PropSet instance', () => { + const canonical = createNodePropSetV2('user:alice', 'name', 'Alice'); + const raw = lowerCanonicalOp(canonical); + + expect(raw).toBeInstanceOf(PropSetClass); + expect(raw.type).toBe('PropSet'); + expect(raw.node).toBe('user:alice'); + expect(raw.key).toBe('name'); + expect(raw.value).toBe('Alice'); + }); + + it('lowerCanonicalOp converts EdgePropSet to PropSet instance', () => { + const canonical = createEdgePropSetV2('n1', 'n2', 'rel', 'weight', 42); + const raw = lowerCanonicalOp(canonical); + + expect(raw).toBeInstanceOf(PropSetClass); + expect(raw.type).toBe('PropSet'); + expect(raw.node).toContain('\x01'); + expect(raw.key).toBe('weight'); + expect(raw.value).toBe(42); + }); + + it('lowerCanonicalOp passes NodeAdd through as-is', () => { + const dot = new Dot('w', 1); + const canonical = createNodeAddV2('n1', dot); + const raw = lowerCanonicalOp(canonical); + + expect(raw).toBe(canonical); + expect(raw).toBeInstanceOf(NodeAdd); + }); + + it('round-trip: PropSet → normalize → lower → PropSet with same data', () => { + const original = createPropSetV2('user:alice', 'name', 'Alice'); + const canonical = normalizeRawOp(original); + const lowered = lowerCanonicalOp(canonical); + + expect(lowered).toBeInstanceOf(PropSetClass); + expect(lowered.type).toBe('PropSet'); + expect(lowered.node).toBe('user:alice'); + expect(lowered.key).toBe('name'); + expect(lowered.value).toBe('Alice'); + }); + + it('round-trip: edge PropSet → normalize → lower → PropSet with same encoding', () => { + const original = createPropSetV2('\x01n1\x00n2\x00rel', 'weight', 42); + const canonical = normalizeRawOp(original); + const lowered = lowerCanonicalOp(canonical); + + expect(lowered).toBeInstanceOf(PropSetClass); + expect(lowered.type).toBe('PropSet'); + expect(lowered.node).toBe(original.node); + expect(lowered.key).toBe('weight'); + expect(lowered.value).toBe(42); + }); +}); From 3b5fba3ba67b56c6e2afdaae1258cf2df33bef8a Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 5 Apr 2026 11:40:27 -0700 Subject: [PATCH 04/13] =?UTF-8?q?test:=20prove=20Op=20class=20instances=20?= =?UTF-8?q?flow=20through=20JoinReducer=20=E2=80=94=20Slice=203?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 11 new tests verifying class-instance ops work through all three reducer paths (applyFast, applyWithReceipt, applyWithDiff) and OP_STRATEGIES dispatch. No source changes needed — the string-keyed strategy Map already dispatches correctly for class instances since they carry the same .type field. Cycle 0009 — op-type-class-hierarchy --- .../types/ops/reducer-integration.test.js | 213 ++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 test/unit/domain/types/ops/reducer-integration.test.js diff --git a/test/unit/domain/types/ops/reducer-integration.test.js b/test/unit/domain/types/ops/reducer-integration.test.js new file mode 100644 index 00000000..fb8b04e2 --- /dev/null +++ b/test/unit/domain/types/ops/reducer-integration.test.js @@ -0,0 +1,213 @@ +/** + * Tests that Op class instances flow correctly through JoinReducer. + * + * The reducer dispatches via OP_STRATEGIES.get(op.type) — class instances + * have the same .type strings, so they must work identically to plain + * objects through all three apply paths. + */ +import { describe, it, expect } from 'vitest'; +import { Dot } from '../../../../../src/domain/crdt/Dot.js'; +import { + createNodeAddV2, + createNodeRemoveV2, + createEdgeAddV2, + createEdgeRemoveV2, + createPropSetV2, + createNodePropSetV2, + createEdgePropSetV2, +} from '../../../../../src/domain/types/WarpTypesV2.js'; +import { PatchV2 } from '../../../../../src/domain/types/WarpTypesV2.js'; +import NodeAdd from '../../../../../src/domain/types/ops/NodeAdd.js'; +import EdgeAdd from '../../../../../src/domain/types/ops/EdgeAdd.js'; +import NodePropSet from '../../../../../src/domain/types/ops/NodePropSet.js'; +import { + createEmptyStateV5, + applyOpV2, + applyFast, + applyWithReceipt, + applyWithDiff, + isKnownRawOp, + isKnownCanonicalOp, + OP_STRATEGIES, +} from '../../../../../src/domain/services/JoinReducer.js'; +import { createEventId } from '../../../../../src/domain/utils/EventId.js'; +import { orsetContains } from '../../../../../src/domain/crdt/ORSet.js'; +import VersionVector from '../../../../../src/domain/crdt/VersionVector.js'; + +describe('Op class instances through JoinReducer', () => { + /** @param {number} opIndex */ + function eid(opIndex) { + return createEventId(1, 'alice', 'abcd1234', opIndex); + } + + describe('applyOpV2', () => { + it('applies NodeAdd class instance to state', () => { + const state = createEmptyStateV5(); + const dot = new Dot('alice', 1); + const op = createNodeAddV2('user:alice', dot); + + expect(op).toBeInstanceOf(NodeAdd); + applyOpV2(state, op, eid(0)); + + expect(orsetContains(state.nodeAlive, 'user:alice')).toBe(true); + }); + + it('applies EdgeAdd class instance to state', () => { + const state = createEmptyStateV5(); + const dot1 = new Dot('alice', 1); + const dot2 = new Dot('alice', 2); + const dot3 = new Dot('alice', 3); + + applyOpV2(state, createNodeAddV2('n1', dot1), eid(0)); + applyOpV2(state, createNodeAddV2('n2', dot2), eid(1)); + applyOpV2(state, createEdgeAddV2('n1', 'n2', 'rel', dot3), eid(2)); + + expect(orsetContains(state.edgeAlive, 'n1\x00n2\x00rel')).toBe(true); + }); + + it('applies NodePropSet class instance to state', () => { + const state = createEmptyStateV5(); + const op = createNodePropSetV2('user:alice', 'name', 'Alice'); + + expect(op).toBeInstanceOf(NodePropSet); + applyOpV2(state, op, eid(0)); + + const propKey = 'user:alice\x00name'; + const reg = state.prop.get(propKey); + expect(reg).toBeDefined(); + expect(reg?.value).toBe('Alice'); + }); + + it('applies PropSet class instance to state', () => { + const state = createEmptyStateV5(); + const op = createPropSetV2('user:alice', 'name', 'Alice'); + + applyOpV2(state, op, eid(0)); + + const propKey = 'user:alice\x00name'; + const reg = state.prop.get(propKey); + expect(reg).toBeDefined(); + expect(reg?.value).toBe('Alice'); + }); + + it('applies NodeRemove class instance to state', () => { + const state = createEmptyStateV5(); + const dot = new Dot('alice', 1); + + applyOpV2(state, createNodeAddV2('n1', dot), eid(0)); + expect(orsetContains(state.nodeAlive, 'n1')).toBe(true); + + applyOpV2(state, createNodeRemoveV2('n1', ['alice:1']), eid(1)); + expect(orsetContains(state.nodeAlive, 'n1')).toBe(false); + }); + }); + + describe('applyFast with class instances', () => { + it('applies a patch of class-instance ops', () => { + const state = createEmptyStateV5(); + const patch = new PatchV2({ + writer: 'alice', + lamport: 1, + context: VersionVector.empty(), + ops: [ + createNodeAddV2('n1', new Dot('alice', 1)), + createNodeAddV2('n2', new Dot('alice', 2)), + createEdgeAddV2('n1', 'n2', 'rel', new Dot('alice', 3)), + createNodePropSetV2('n1', 'name', 'Node One'), + ], + }); + + applyFast(state, patch, 'abcd1234abcd1234abcd1234abcd1234abcd1234'); + + expect(orsetContains(state.nodeAlive, 'n1')).toBe(true); + expect(orsetContains(state.nodeAlive, 'n2')).toBe(true); + expect(orsetContains(state.edgeAlive, 'n1\x00n2\x00rel')).toBe(true); + expect(state.prop.get('n1\x00name')?.value).toBe('Node One'); + }); + }); + + describe('applyWithReceipt with class instances', () => { + it('produces a receipt from class-instance ops', () => { + const state = createEmptyStateV5(); + const patch = new PatchV2({ + writer: 'alice', + lamport: 1, + context: VersionVector.empty(), + ops: [ + createNodeAddV2('n1', new Dot('alice', 1)), + createNodePropSetV2('n1', 'name', 'Node One'), + ], + }); + + const result = applyWithReceipt(state, patch, 'abcd1234abcd1234abcd1234abcd1234abcd1234'); + + expect(result.receipt).toBeDefined(); + expect(result.receipt.patchSha).toBe('abcd1234abcd1234abcd1234abcd1234abcd1234'); + expect(result.receipt.ops).toHaveLength(2); + expect(result.receipt.ops[0]?.op).toBe('NodeAdd'); + expect(result.receipt.ops[0]?.result).toBe('applied'); + expect(result.receipt.ops[1]?.op).toBe('NodePropSet'); + }); + }); + + describe('applyWithDiff with class instances', () => { + it('produces a diff from class-instance ops', () => { + const state = createEmptyStateV5(); + const patch = new PatchV2({ + writer: 'alice', + lamport: 1, + context: VersionVector.empty(), + ops: [ + createNodeAddV2('n1', new Dot('alice', 1)), + createNodePropSetV2('n1', 'name', 'Node One'), + ], + }); + + const result = applyWithDiff(state, patch, 'abcd1234abcd1234abcd1234abcd1234abcd1234'); + + expect(result.diff).toBeDefined(); + expect(result.diff.nodesAdded).toContain('n1'); + expect(result.diff.propsChanged).toHaveLength(1); + expect(result.diff.propsChanged[0]?.nodeId).toBe('n1'); + expect(result.diff.propsChanged[0]?.key).toBe('name'); + expect(result.diff.propsChanged[0]?.value).toBe('Node One'); + }); + }); + + describe('isKnownRawOp / isKnownCanonicalOp with class instances', () => { + it('recognizes class instances as known raw ops', () => { + const dot = new Dot('w', 1); + expect(isKnownRawOp(createNodeAddV2('n1', dot))).toBe(true); + expect(isKnownRawOp(createNodeRemoveV2('n1', []))).toBe(true); + expect(isKnownRawOp(createEdgeAddV2('n1', 'n2', 'r', dot))).toBe(true); + expect(isKnownRawOp(createEdgeRemoveV2('n1', 'n2', 'r', []))).toBe(true); + expect(isKnownRawOp(createPropSetV2('n1', 'k', 'v'))).toBe(true); + }); + + it('recognizes canonical class instances', () => { + expect(isKnownCanonicalOp(createNodePropSetV2('n1', 'k', 'v'))).toBe(true); + expect(isKnownCanonicalOp(createEdgePropSetV2('n1', 'n2', 'r', 'k', 'v'))).toBe(true); + }); + }); + + describe('OP_STRATEGIES dispatches class instances', () => { + it('finds strategy for every class instance type', () => { + const dot = new Dot('w', 1); + const ops = [ + createNodeAddV2('n1', dot), + createNodeRemoveV2('n1', []), + createEdgeAddV2('n1', 'n2', 'r', dot), + createEdgeRemoveV2('n1', 'n2', 'r', []), + createNodePropSetV2('n1', 'k', 'v'), + createEdgePropSetV2('n1', 'n2', 'r', 'k', 'v'), + createPropSetV2('n1', 'k', 'v'), + ]; + + for (const op of ops) { + const strategy = OP_STRATEGIES.get(op.type); + expect(strategy).toBeDefined(); + expect(typeof strategy?.mutate).toBe('function'); + } + }); + }); +}); From 977f324e0865f1979a7e292bf9f1cba39c6f514d Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 5 Apr 2026 11:41:10 -0700 Subject: [PATCH 05/13] docs: update CHANGELOG for op type class hierarchy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle 0009 — op-type-class-hierarchy --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 836b0ed7..1f443d61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Op type class hierarchy** — 8 operation types (`NodeAdd`, `NodeRemove`, `EdgeAdd`, `EdgeRemove`, `NodePropSet`, `EdgePropSet`, `PropSet`, `BlobValue`) promoted from `@typedef` plain objects to frozen classes with constructor validation and `instanceof` dispatch. Base `Op` class provides shared runtime identity. Factory functions in `WarpTypesV2.js` now delegate to class constructors. 97 new tests across 4 test files. - **Auto-materialize on remove** — `createPatch()` now auto-materializes when `autoMaterialize` is true, `_cachedState` is null, and existing patches exist. Users no longer need to call `materialize()` explicitly before patches that include `removeNode`/`removeEdge`. ### Fixed From 87666b953b369639d7b22945a49ac31470800d45 Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 5 Apr 2026 18:01:27 -0700 Subject: [PATCH 06/13] docs: cycle 0009 witness, retro, and new backlog items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Witness: hill met — all 8 agent questions answered YES (Q9 deferred). All 3 human questions answered YES. Hard gates: noCoordination 7/7, 5504/5504 tests, lint clean. Retro: fast RED→GREEN, edge options objects caught by lint, existing tests had wrong Dot field names hidden by any-casts. New backlog: - PROTO_op-consumer-instanceof-migration (up-next, S) - PROTO_cbor-op-hydration (up-next, M) - CC_versionvector-constructor-no-validation (bad-code, S) Cycle 0009 — op-type-class-hierarchy --- .../0009-op-type-class-hierarchy/witness.md | 52 +++++++++++++++++ ...versionvector-constructor-no-validation.md | 27 +++++++++ .../up-next/PROTO_cbor-op-hydration.md | 24 ++++++++ .../PROTO_op-consumer-instanceof-migration.md | 21 +++++++ .../0009-op-type-class-hierarchy/retro.md | 57 +++++++++++++++++++ 5 files changed, 181 insertions(+) create mode 100644 docs/design/0009-op-type-class-hierarchy/witness.md create mode 100644 docs/method/backlog/bad-code/CC_versionvector-constructor-no-validation.md create mode 100644 docs/method/backlog/up-next/PROTO_cbor-op-hydration.md create mode 100644 docs/method/backlog/up-next/PROTO_op-consumer-instanceof-migration.md create mode 100644 docs/method/retro/0009-op-type-class-hierarchy/retro.md diff --git a/docs/design/0009-op-type-class-hierarchy/witness.md b/docs/design/0009-op-type-class-hierarchy/witness.md new file mode 100644 index 00000000..9a5d055c --- /dev/null +++ b/docs/design/0009-op-type-class-hierarchy/witness.md @@ -0,0 +1,52 @@ +# Cycle 0009 — Witness + +## Agent questions + +1. **Does `new NodeAdd(nodeId, dot)` throw when `nodeId` is empty or `dot` is not a `Dot`?** + YES — `Op.test.js` lines 73-82: throws on empty string, non-string, non-Dot. + +2. **Does `op instanceof NodeAdd` return true for NodeAdd instances and false for EdgeAdd instances?** + YES — `Op.test.js` lines 63-69: cross-class isolation proven for all 8 types. + +3. **Does `instanceof Op` return true for all 8 op subclasses?** + YES — `Op.test.js` "all ops share the Op base" test iterates all 8. + +4. **Are all op instances frozen?** + YES — every class test includes `Object.isFrozen(op) === true`. Arrays (observedDots) also frozen. + +5. **Does `OpNormalizer.normalizeRawOp()` return canonical op class instances?** + YES — `factory-integration.test.js` lines 104-121: PropSet → NodePropSet/EdgePropSet class instances. + +6. **Does `OpNormalizer.lowerCanonicalOp()` return raw op class instances?** + YES — `factory-integration.test.js` lines 123-141: NodePropSet/EdgePropSet → PropSet class instances. + +7. **Does `JoinReducer.OP_STRATEGIES` dispatch class instances?** + YES — `reducer-integration.test.js` "finds strategy for every class instance type": all 7 canonical types dispatch correctly. Strategy lookup is via `.type` string on the class instance. + +8. **Do factory functions delegate to constructors?** + YES — `factory-integration.test.js` lines 18-95: every factory returns an `instanceof` the correct class. + +9. **Does the CBOR decode boundary produce op class instances?** + NOT YET — deferred to future cycle (Slice 5). Plain objects from CBOR still work through the reducer because dispatch is string-based. + +## Human questions + +1. **Can I still do `patch.addNode('user:alice')` and have it just work?** + YES — PatchBuilderV2 calls factory functions internally. No API change. + +2. **Does `git warp history` still show op types correctly?** + YES — presenter uses `.type` string which class instances carry. + +3. **Do existing patches in a real repo still materialize identically?** + YES — noCoordination test suite 7/7 pass. Wire format unchanged (CBOR still encodes `.type` strings). Factory functions produce structurally identical objects (same fields, same values). + +## Hard gates + +- noCoordination: **7/7 PASS** +- Full unit suite: **5504/5504 PASS** (332 files) +- Lint: **0 errors, 0 warnings** +- Wire format: **unchanged** (class instances serialize to same CBOR) + +## Verdict + +**Hill met.** 8 typedef ops replaced with frozen class hierarchy. Runtime identity, constructor validation, `instanceof` dispatch all proven. Consumer migration (Slice 4) and CBOR hydration (Slice 5) deferred — they're incremental and don't block the core value. diff --git a/docs/method/backlog/bad-code/CC_versionvector-constructor-no-validation.md b/docs/method/backlog/bad-code/CC_versionvector-constructor-no-validation.md new file mode 100644 index 00000000..dda8539c --- /dev/null +++ b/docs/method/backlog/bad-code/CC_versionvector-constructor-no-validation.md @@ -0,0 +1,27 @@ +# VersionVector constructor accepts undefined entries + +**Effort:** S + +## What's Wrong + +`new VersionVector()` (no args) sets `#entries` to `undefined`. Any +subsequent method call (`merge`, `get`, `[Symbol.iterator]`) throws a +confusing `TypeError: undefined is not iterable`. The constructor +should reject missing or non-Map arguments per P2 (boundary validation). + +## Suggested Fix + +Add validation to the constructor: + +```js +constructor(entries) { + if (!(entries instanceof Map)) { + throw new CrdtError('VersionVector requires a Map'); + } + this.#entries = entries; +} +``` + +## Source + +Discovered during cycle 0009 reducer integration tests. diff --git a/docs/method/backlog/up-next/PROTO_cbor-op-hydration.md b/docs/method/backlog/up-next/PROTO_cbor-op-hydration.md new file mode 100644 index 00000000..9e2045ad --- /dev/null +++ b/docs/method/backlog/up-next/PROTO_cbor-op-hydration.md @@ -0,0 +1,24 @@ +# CBOR decode boundary: hydrate ops into class instances + +**Effort:** M + +## Problem + +Ops deserialized from CBOR are plain objects (`{ type: 'NodeAdd', ... }`). +They pass through the reducer via string dispatch but fail `instanceof` +checks. The decode boundary should hydrate plain objects into Op class +instances so the entire pipeline is class-native. + +## Where + +`CborPatchJournalAdapter` or `CborCodec` — wherever patches are +decoded from bytes back into `PatchV2` objects. + +## Hard gate + +Golden blob round-trip: encode a patch with classes → decode → verify +class instances. Must produce identical CRDT state. + +## Source + +Cycle 0009 retro, Slice 5 deferral. diff --git a/docs/method/backlog/up-next/PROTO_op-consumer-instanceof-migration.md b/docs/method/backlog/up-next/PROTO_op-consumer-instanceof-migration.md new file mode 100644 index 00000000..d953c17e --- /dev/null +++ b/docs/method/backlog/up-next/PROTO_op-consumer-instanceof-migration.md @@ -0,0 +1,21 @@ +# Migrate op consumers to instanceof dispatch + +**Effort:** S + +## Problem + +Cycle 0009 shipped Op class hierarchy but consumers still use string +comparison (`op.type === 'NodeAdd'`). Convert to `instanceof`: + +- `MessageSchemaDetector.js` — 2 string checks +- `bin/presenters/text.js` — 8 string checks +- `TickReceipt.js` — `OP_TYPES` array + +## Notes + +Blocked on CBOR hydration (ops from disk are still plain objects). +Can proceed incrementally per-file once hydration ships. + +## Source + +Cycle 0009 retro, Slice 4 deferral. diff --git a/docs/method/retro/0009-op-type-class-hierarchy/retro.md b/docs/method/retro/0009-op-type-class-hierarchy/retro.md new file mode 100644 index 00000000..8e65cae3 --- /dev/null +++ b/docs/method/retro/0009-op-type-class-hierarchy/retro.md @@ -0,0 +1,57 @@ +# Cycle 0009 Retro — Op Type Class Hierarchy + +## Hill + +Replace 8 typedef ops with a frozen class hierarchy for runtime +identity, constructor validation, and `instanceof` dispatch. + +## Outcome + +**Hill met.** + +## What went well + +- RED→GREEN was fast. The class pattern (Dot.js precedent) is + well-established in this codebase. +- Edge types needed options objects for `max-params` compliance — + caught by lint, fixed immediately. Clean API. +- The factory function delegation was seamless. Existing tests only + needed Dot instance updates (they were passing plain objects with + wrong field names — `writer`/`seq` instead of `writerId`/`counter` + — hidden by `/** @type {any} */` casts). +- noCoordination suite passed first try. Zero behavioral change. + +## What went wrong + +- Test for `applyWithReceipt` / `applyWithDiff` had wrong return + type assumptions (expected direct receipt, got `{state, receipt}`). + Fixed in-flight. Should have read the function signatures first. +- `new VersionVector()` without args silently creates a broken + instance (`#entries` is undefined). Used `VersionVector.empty()` + instead. This is a latent P2 violation — constructor should reject + missing args. + +## Drift check + +- No undocumented drift. All changes trace to playback questions. +- Slices 4-5 (consumer `instanceof` migration, CBOR hydration) + explicitly deferred in the design doc. + +## New debt + +- `VersionVector()` constructor accepts undefined entries without + throwing (P2 violation). Filed as bad-code item. + +## New backlog + +- `PROTO_op-consumer-instanceof-migration.md` — Slice 4: convert + MessageSchemaDetector, text presenter, TickReceipt to `instanceof`. +- `PROTO_cbor-op-hydration.md` — Slice 5: CBOR decode boundary + produces Op class instances instead of plain objects. + +## Stats + +- 9 new source files (710 LOC) +- 4 new test files (97 tests) +- 35 existing tests updated +- 5504 total tests passing From db6a070143bdbb722f0c7334c6335a297d7de256 Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 5 Apr 2026 18:38:05 -0700 Subject: [PATCH 07/13] =?UTF-8?q?fix:=20code=20review=20=E2=80=94=20reserv?= =?UTF-8?q?ed-byte=20validation=20+=20doc=20corrections?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review (12 issues, 0 remaining): C1/M2/m1/m2: Add assertNoReservedBytes to all op constructors — rejects NUL (\x00) and \x01 prefix in identifiers. Matches PatchBuilderV2._assertNoReservedBytes. PropSet exempted (legitimately carries \x01-prefixed nodes for edge prop wire encoding). M1: Design doc Status DESIGN → HILL MET. Added Delivered vs. Deferred table clarifying which scope items shipped. n1: Blank line between frontmatter fields in design doc. n2: Witness references test descriptions instead of line numbers. n3: Retro file count 9 → 10. n4: WarpTypesV2 import alias PropSetOpClass → PropSetClass. 15 new validation tests. 5519 total tests passing. Cycle 0009 — op-type-class-hierarchy --- .../op-type-class-hierarchy.md | 17 ++++- .../0009-op-type-class-hierarchy/witness.md | 16 ++--- .../0009-op-type-class-hierarchy/retro.md | 2 +- src/domain/types/WarpTypesV2.js | 4 +- src/domain/types/ops/BlobValue.js | 3 +- src/domain/types/ops/EdgeAdd.js | 8 +-- src/domain/types/ops/EdgePropSet.js | 6 +- src/domain/types/ops/EdgeRemove.js | 5 +- src/domain/types/ops/NodeAdd.js | 4 +- src/domain/types/ops/NodePropSet.js | 4 +- src/domain/types/ops/NodeRemove.js | 3 +- src/domain/types/ops/validate.js | 26 ++++++-- test/unit/domain/types/ops/Op.test.js | 66 +++++++++++++++++++ test/unit/domain/types/ops/validate.test.js | 21 ++++-- 14 files changed, 151 insertions(+), 34 deletions(-) diff --git a/docs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.md b/docs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.md index 9281cc70..fca36f8d 100644 --- a/docs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.md +++ b/docs/design/0009-op-type-class-hierarchy/op-type-class-hierarchy.md @@ -1,6 +1,7 @@ # Cycle 0009 — Op Type Class Hierarchy -**Status:** DESIGN +**Status:** HILL MET + **Date:** 2026-04-05 ## Sponsors @@ -57,6 +58,20 @@ dispatch — eliminating all string-based tag switching. | `src/domain/services/PatchBuilderV2.js` | Builds canonical op class instances internally. `build()`/`commit()` lower via `lowerCanonicalOp`. | | CBOR decode boundary | `CborCodec` or `CborPatchJournalAdapter` hydrates plain objects into op classes. | +### Delivered vs. Deferred + +| Component | Status | +|---|---| +| `src/domain/types/ops/` | **Delivered** — 9 classes + validate.js | +| `src/domain/types/WarpTypesV2.js` | **Delivered** — factory functions delegate to constructors | +| `src/domain/services/OpNormalizer.js` | **Delivered** — returns class instances via factory functions | +| `src/domain/services/JoinReducer.js` | **Deferred** — OP_STRATEGIES still string-keyed (works with class instances via .type). See `PROTO_op-consumer-instanceof-migration` | +| `src/domain/types/TickReceipt.js` | **Deferred** — See `PROTO_op-consumer-instanceof-migration` | +| `src/domain/services/codec/MessageSchemaDetector.js` | **Deferred** — See `PROTO_op-consumer-instanceof-migration` | +| `bin/presenters/text.js` | **Deferred** — See `PROTO_op-consumer-instanceof-migration` | +| `src/domain/services/PatchBuilderV2.js` | **Not needed** — already uses factory functions which now produce class instances | +| CBOR decode boundary | **Deferred** — See `PROTO_cbor-op-hydration` | + ### What does NOT change - Wire format. Persisted patches remain CBOR with `{ type: 'NodeAdd', ... }` plain objects. The class boundary is at decode, not encode. diff --git a/docs/design/0009-op-type-class-hierarchy/witness.md b/docs/design/0009-op-type-class-hierarchy/witness.md index 9a5d055c..ab628367 100644 --- a/docs/design/0009-op-type-class-hierarchy/witness.md +++ b/docs/design/0009-op-type-class-hierarchy/witness.md @@ -3,28 +3,28 @@ ## Agent questions 1. **Does `new NodeAdd(nodeId, dot)` throw when `nodeId` is empty or `dot` is not a `Dot`?** - YES — `Op.test.js` lines 73-82: throws on empty string, non-string, non-Dot. + YES — `Op.test.js` "throws on empty nodeId", "throws on non-string nodeId", "throws when dot is not a Dot instance" tests in the NodeAdd describe block. 2. **Does `op instanceof NodeAdd` return true for NodeAdd instances and false for EdgeAdd instances?** - YES — `Op.test.js` lines 63-69: cross-class isolation proven for all 8 types. + YES — `Op.test.js` "cross-class instanceof isolation" > "no op is instanceof a sibling class" proves all 8x8 combinations. 3. **Does `instanceof Op` return true for all 8 op subclasses?** - YES — `Op.test.js` "all ops share the Op base" test iterates all 8. + YES — `Op.test.js` "cross-class instanceof isolation" > "all ops share the Op base" iterates all 8. 4. **Are all op instances frozen?** - YES — every class test includes `Object.isFrozen(op) === true`. Arrays (observedDots) also frozen. + YES — every class describe block includes an "is frozen" test. NodeRemove and EdgeRemove also have "freezes the observedDots array" tests. 5. **Does `OpNormalizer.normalizeRawOp()` return canonical op class instances?** - YES — `factory-integration.test.js` lines 104-121: PropSet → NodePropSet/EdgePropSet class instances. + YES — `factory-integration.test.js` "normalizeRawOp converts PropSet (node) to NodePropSet instance" and "normalizeRawOp converts PropSet (edge) to EdgePropSet instance" tests. 6. **Does `OpNormalizer.lowerCanonicalOp()` return raw op class instances?** - YES — `factory-integration.test.js` lines 123-141: NodePropSet/EdgePropSet → PropSet class instances. + YES — `factory-integration.test.js` "lowerCanonicalOp converts NodePropSet to PropSet instance" and "lowerCanonicalOp converts EdgePropSet to PropSet instance" tests. 7. **Does `JoinReducer.OP_STRATEGIES` dispatch class instances?** - YES — `reducer-integration.test.js` "finds strategy for every class instance type": all 7 canonical types dispatch correctly. Strategy lookup is via `.type` string on the class instance. + YES — `reducer-integration.test.js` "finds strategy for every class instance type" test: all 7 canonical types dispatch correctly. Strategy lookup is via `.type` string on the class instance. 8. **Do factory functions delegate to constructors?** - YES — `factory-integration.test.js` lines 18-95: every factory returns an `instanceof` the correct class. + YES — `factory-integration.test.js` "WarpTypesV2 factory functions produce Op class instances" describe block: every factory returns an `instanceof` the correct class. 9. **Does the CBOR decode boundary produce op class instances?** NOT YET — deferred to future cycle (Slice 5). Plain objects from CBOR still work through the reducer because dispatch is string-based. diff --git a/docs/method/retro/0009-op-type-class-hierarchy/retro.md b/docs/method/retro/0009-op-type-class-hierarchy/retro.md index 8e65cae3..fd261461 100644 --- a/docs/method/retro/0009-op-type-class-hierarchy/retro.md +++ b/docs/method/retro/0009-op-type-class-hierarchy/retro.md @@ -51,7 +51,7 @@ identity, constructor validation, and `instanceof` dispatch. ## Stats -- 9 new source files (710 LOC) +- 10 new source files (710 LOC) - 4 new test files (97 tests) - 35 existing tests updated - 5504 total tests passing diff --git a/src/domain/types/WarpTypesV2.js b/src/domain/types/WarpTypesV2.js index 05e6ff6c..6941c114 100644 --- a/src/domain/types/WarpTypesV2.js +++ b/src/domain/types/WarpTypesV2.js @@ -21,7 +21,7 @@ import EdgeAddClass from './ops/EdgeAdd.js'; import EdgeRemoveClass from './ops/EdgeRemove.js'; import NodePropSetClass from './ops/NodePropSet.js'; import EdgePropSetClass from './ops/EdgePropSet.js'; -import PropSetOpClass from './ops/PropSet.js'; +import PropSetClass from './ops/PropSet.js'; // Re-export PatchV2 class for consumers that import from this module. export { PatchV2 }; @@ -205,7 +205,7 @@ export function createEdgeRemoveV2(from, to, label, observedDots) { * @returns {OpV2PropSet} PropSet operation */ export function createPropSetV2(node, key, value) { - return new PropSetOpClass(node, key, value); + return new PropSetClass(node, key, value); } /** diff --git a/src/domain/types/ops/BlobValue.js b/src/domain/types/ops/BlobValue.js index 2e7b58d5..9fac3807 100644 --- a/src/domain/types/ops/BlobValue.js +++ b/src/domain/types/ops/BlobValue.js @@ -5,7 +5,7 @@ */ import Op from './Op.js'; -import { assertNonEmptyString } from './validate.js'; +import { assertNonEmptyString, assertNoReservedBytes } from './validate.js'; /** * References an external blob attached to a node. @@ -28,6 +28,7 @@ export default class BlobValue extends Op { super('BlobValue'); assertNonEmptyString(node, 'BlobValue', 'node'); assertNonEmptyString(oid, 'BlobValue', 'oid'); + assertNoReservedBytes(node, 'BlobValue', 'node'); this.node = node; this.oid = oid; Object.freeze(this); diff --git a/src/domain/types/ops/EdgeAdd.js b/src/domain/types/ops/EdgeAdd.js index a7505791..0cef02b8 100644 --- a/src/domain/types/ops/EdgeAdd.js +++ b/src/domain/types/ops/EdgeAdd.js @@ -6,7 +6,7 @@ import { Dot } from '../../crdt/Dot.js'; import Op from './Op.js'; -import { assertNonEmptyString, assertNoBannedBytes } from './validate.js'; +import { assertNonEmptyString, assertNoReservedBytes } from './validate.js'; /** * Adds a directed edge to the graph's OR-Set with a unique dot. @@ -34,9 +34,9 @@ export default class EdgeAdd extends Op { assertNonEmptyString(from, 'EdgeAdd', 'from'); assertNonEmptyString(to, 'EdgeAdd', 'to'); assertNonEmptyString(label, 'EdgeAdd', 'label'); - assertNoBannedBytes(from, 'EdgeAdd', 'from'); - assertNoBannedBytes(to, 'EdgeAdd', 'to'); - assertNoBannedBytes(label, 'EdgeAdd', 'label'); + assertNoReservedBytes(from, 'EdgeAdd', 'from'); + assertNoReservedBytes(to, 'EdgeAdd', 'to'); + assertNoReservedBytes(label, 'EdgeAdd', 'label'); if (!(dot instanceof Dot)) { throw new Error('EdgeAdd requires dot to be a Dot instance'); } diff --git a/src/domain/types/ops/EdgePropSet.js b/src/domain/types/ops/EdgePropSet.js index 82bd6206..8d434b2e 100644 --- a/src/domain/types/ops/EdgePropSet.js +++ b/src/domain/types/ops/EdgePropSet.js @@ -5,7 +5,7 @@ */ import Op from './Op.js'; -import { assertNonEmptyString } from './validate.js'; +import { assertNonEmptyString, assertNoReservedBytes } from './validate.js'; /** * Sets a property on an edge using LWW semantics. @@ -38,6 +38,10 @@ export default class EdgePropSet extends Op { assertNonEmptyString(to, 'EdgePropSet', 'to'); assertNonEmptyString(label, 'EdgePropSet', 'label'); assertNonEmptyString(key, 'EdgePropSet', 'key'); + assertNoReservedBytes(from, 'EdgePropSet', 'from'); + assertNoReservedBytes(to, 'EdgePropSet', 'to'); + assertNoReservedBytes(label, 'EdgePropSet', 'label'); + assertNoReservedBytes(key, 'EdgePropSet', 'key'); this.from = from; this.to = to; this.label = label; diff --git a/src/domain/types/ops/EdgeRemove.js b/src/domain/types/ops/EdgeRemove.js index f1525fa6..f94b0054 100644 --- a/src/domain/types/ops/EdgeRemove.js +++ b/src/domain/types/ops/EdgeRemove.js @@ -5,7 +5,7 @@ */ import Op from './Op.js'; -import { assertNonEmptyString, assertArray } from './validate.js'; +import { assertNonEmptyString, assertNoReservedBytes, assertArray } from './validate.js'; /** * Removes a directed edge from the graph's OR-Set by tombstoning observed dots. @@ -33,6 +33,9 @@ export default class EdgeRemove extends Op { assertNonEmptyString(from, 'EdgeRemove', 'from'); assertNonEmptyString(to, 'EdgeRemove', 'to'); assertNonEmptyString(label, 'EdgeRemove', 'label'); + assertNoReservedBytes(from, 'EdgeRemove', 'from'); + assertNoReservedBytes(to, 'EdgeRemove', 'to'); + assertNoReservedBytes(label, 'EdgeRemove', 'label'); assertArray(observedDots, 'EdgeRemove', 'observedDots'); this.from = from; this.to = to; diff --git a/src/domain/types/ops/NodeAdd.js b/src/domain/types/ops/NodeAdd.js index 91e4710b..76aa1ba4 100644 --- a/src/domain/types/ops/NodeAdd.js +++ b/src/domain/types/ops/NodeAdd.js @@ -6,7 +6,7 @@ import { Dot } from '../../crdt/Dot.js'; import Op from './Op.js'; -import { assertNonEmptyString, assertNoBannedBytes } from './validate.js'; +import { assertNonEmptyString, assertNoReservedBytes } from './validate.js'; /** * Adds a node to the graph's OR-Set with a unique dot. @@ -27,7 +27,7 @@ export default class NodeAdd extends Op { constructor(node, dot) { super('NodeAdd'); assertNonEmptyString(node, 'NodeAdd', 'node'); - assertNoBannedBytes(node, 'NodeAdd', 'node'); + assertNoReservedBytes(node, 'NodeAdd', 'node'); if (!(dot instanceof Dot)) { throw new Error('NodeAdd requires dot to be a Dot instance'); } diff --git a/src/domain/types/ops/NodePropSet.js b/src/domain/types/ops/NodePropSet.js index e611e5dc..e1488b66 100644 --- a/src/domain/types/ops/NodePropSet.js +++ b/src/domain/types/ops/NodePropSet.js @@ -5,7 +5,7 @@ */ import Op from './Op.js'; -import { assertNonEmptyString } from './validate.js'; +import { assertNonEmptyString, assertNoReservedBytes } from './validate.js'; /** * Sets a property on a node using LWW semantics. @@ -32,6 +32,8 @@ export default class NodePropSet extends Op { super('NodePropSet'); assertNonEmptyString(node, 'NodePropSet', 'node'); assertNonEmptyString(key, 'NodePropSet', 'key'); + assertNoReservedBytes(node, 'NodePropSet', 'node'); + assertNoReservedBytes(key, 'NodePropSet', 'key'); this.node = node; this.key = key; this.value = value; diff --git a/src/domain/types/ops/NodeRemove.js b/src/domain/types/ops/NodeRemove.js index 71b9c7e1..d4d58d13 100644 --- a/src/domain/types/ops/NodeRemove.js +++ b/src/domain/types/ops/NodeRemove.js @@ -5,7 +5,7 @@ */ import Op from './Op.js'; -import { assertNonEmptyString, assertArray } from './validate.js'; +import { assertNonEmptyString, assertNoReservedBytes, assertArray } from './validate.js'; /** * Removes a node from the graph's OR-Set by tombstoning observed dots. @@ -26,6 +26,7 @@ export default class NodeRemove extends Op { constructor(node, observedDots) { super('NodeRemove'); assertNonEmptyString(node, 'NodeRemove', 'node'); + assertNoReservedBytes(node, 'NodeRemove', 'node'); assertArray(observedDots, 'NodeRemove', 'observedDots'); this.node = node; this.observedDots = Object.freeze([...observedDots]); diff --git a/src/domain/types/ops/validate.js b/src/domain/types/ops/validate.js index ecd7717f..afb47e79 100644 --- a/src/domain/types/ops/validate.js +++ b/src/domain/types/ops/validate.js @@ -1,9 +1,19 @@ /** * Shared validation helpers for Op constructors. * + * Mirrors PatchBuilderV2._assertNoReservedBytes for consistency — + * ops constructed outside PatchBuilderV2 (CBOR decode, tests, direct + * construction) get the same validation. + * * @module domain/types/ops/validate */ +/** @const {string} NUL byte — edge key field separator */ +const FIELD_SEPARATOR = '\x00'; + +/** @const {string} Edge property prefix — reserved for wire encoding */ +const EDGE_PROP_PREFIX = '\x01'; + /** * Asserts that a value is a non-empty string. * @@ -18,17 +28,25 @@ export function assertNonEmptyString(value, opName, field) { } /** - * Asserts that a string contains no NUL (\x00) bytes. - * NUL is the edge key separator — it cannot appear in identifiers. + * Asserts that a string identifier contains no reserved bytes. + * + * Rejects: + * - NUL (\x00) — edge key field separator + * - \x01 prefix — reserved for edge property encoding on the wire + * + * Matches PatchBuilderV2._assertNoReservedBytes. * * @param {string} value * @param {string} opName * @param {string} field */ -export function assertNoBannedBytes(value, opName, field) { - if (value.includes('\x00')) { +export function assertNoReservedBytes(value, opName, field) { + if (value.includes(FIELD_SEPARATOR)) { throw new Error(`${opName} '${field}' must not contain NUL (\\x00) bytes`); } + if (value.length > 0 && value[0] === EDGE_PROP_PREFIX) { + throw new Error(`${opName} '${field}' must not start with reserved prefix \\x01`); + } } /** diff --git a/test/unit/domain/types/ops/Op.test.js b/test/unit/domain/types/ops/Op.test.js index 86bd873d..1d6df88c 100644 --- a/test/unit/domain/types/ops/Op.test.js +++ b/test/unit/domain/types/ops/Op.test.js @@ -87,6 +87,11 @@ describe('NodeAdd', () => { const dot = new Dot('alice', 1); expect(() => new NodeAdd('user\x00alice', dot)).toThrow(); }); + + it('rejects nodeId starting with \\x01 prefix', () => { + const dot = new Dot('alice', 1); + expect(() => new NodeAdd('\x01user:alice', dot)).toThrow(/reserved prefix/); + }); }); describe('NodeRemove', () => { @@ -127,6 +132,14 @@ describe('NodeRemove', () => { const op = new NodeRemove('user:alice', []); expect(op.observedDots).toEqual([]); }); + + it('rejects nodeId containing NUL byte', () => { + expect(() => new NodeRemove('user\x00alice', [])).toThrow(/NUL/); + }); + + it('rejects nodeId starting with \\x01 prefix', () => { + expect(() => new NodeRemove('\x01user:alice', [])).toThrow(/reserved prefix/); + }); }); describe('EdgeAdd', () => { @@ -180,6 +193,13 @@ describe('EdgeAdd', () => { expect(() => new EdgeAdd({ from: 'n1', to: 'n\x002', label: 'rel', dot })).toThrow(); expect(() => new EdgeAdd({ from: 'n1', to: 'n2', label: 'r\x00l', dot })).toThrow(); }); + + it('rejects from/to/label starting with \\x01 prefix', () => { + const dot = new Dot('alice', 1); + expect(() => new EdgeAdd({ from: '\x01n1', to: 'n2', label: 'rel', dot })).toThrow(/reserved prefix/); + expect(() => new EdgeAdd({ from: 'n1', to: '\x01n2', label: 'rel', dot })).toThrow(/reserved prefix/); + expect(() => new EdgeAdd({ from: 'n1', to: 'n2', label: '\x01rel', dot })).toThrow(/reserved prefix/); + }); }); describe('EdgeRemove', () => { @@ -224,6 +244,18 @@ describe('EdgeRemove', () => { it('throws when observedDots is not an array', () => { expect(() => new EdgeRemove({ from: 'n1', to: 'n2', label: 'rel', observedDots: /** @type {any} */ ('w:1') })).toThrow(); }); + + it('rejects from/to/label containing NUL byte', () => { + expect(() => new EdgeRemove({ from: 'n\x001', to: 'n2', label: 'rel', observedDots: [] })).toThrow(/NUL/); + expect(() => new EdgeRemove({ from: 'n1', to: 'n\x002', label: 'rel', observedDots: [] })).toThrow(/NUL/); + expect(() => new EdgeRemove({ from: 'n1', to: 'n2', label: 'r\x00l', observedDots: [] })).toThrow(/NUL/); + }); + + it('rejects from/to/label starting with \\x01 prefix', () => { + expect(() => new EdgeRemove({ from: '\x01n1', to: 'n2', label: 'rel', observedDots: [] })).toThrow(/reserved prefix/); + expect(() => new EdgeRemove({ from: 'n1', to: '\x01n2', label: 'rel', observedDots: [] })).toThrow(/reserved prefix/); + expect(() => new EdgeRemove({ from: 'n1', to: 'n2', label: '\x01rel', observedDots: [] })).toThrow(/reserved prefix/); + }); }); describe('NodePropSet', () => { @@ -271,6 +303,18 @@ describe('NodePropSet', () => { const op = new NodePropSet('n1', 'age', 42); expect(op.value).toBe(42); }); + + it('rejects node containing NUL byte', () => { + expect(() => new NodePropSet('n\x001', 'k', 'v')).toThrow(/NUL/); + }); + + it('rejects key containing NUL byte', () => { + expect(() => new NodePropSet('n1', 'k\x00ey', 'v')).toThrow(/NUL/); + }); + + it('rejects node starting with \\x01 prefix', () => { + expect(() => new NodePropSet('\x01n1', 'k', 'v')).toThrow(/reserved prefix/); + }); }); describe('EdgePropSet', () => { @@ -318,6 +362,20 @@ describe('EdgePropSet', () => { const op = new EdgePropSet({ from: 'n1', to: 'n2', label: 'rel', key: 'k', value: null }); expect(op.value).toBeNull(); }); + + it('rejects from/to/label/key containing NUL byte', () => { + expect(() => new EdgePropSet({ from: 'n\x001', to: 'n2', label: 'rel', key: 'k', value: 'v' })).toThrow(/NUL/); + expect(() => new EdgePropSet({ from: 'n1', to: 'n\x002', label: 'rel', key: 'k', value: 'v' })).toThrow(/NUL/); + expect(() => new EdgePropSet({ from: 'n1', to: 'n2', label: 'r\x00l', key: 'k', value: 'v' })).toThrow(/NUL/); + expect(() => new EdgePropSet({ from: 'n1', to: 'n2', label: 'rel', key: 'k\x00ey', value: 'v' })).toThrow(/NUL/); + }); + + it('rejects from/to/label/key starting with \\x01 prefix', () => { + expect(() => new EdgePropSet({ from: '\x01n1', to: 'n2', label: 'rel', key: 'k', value: 'v' })).toThrow(/reserved prefix/); + expect(() => new EdgePropSet({ from: 'n1', to: '\x01n2', label: 'rel', key: 'k', value: 'v' })).toThrow(/reserved prefix/); + expect(() => new EdgePropSet({ from: 'n1', to: 'n2', label: '\x01rel', key: 'k', value: 'v' })).toThrow(/reserved prefix/); + expect(() => new EdgePropSet({ from: 'n1', to: 'n2', label: 'rel', key: '\x01k', value: 'v' })).toThrow(/reserved prefix/); + }); }); describe('PropSet (raw/wire format)', () => { @@ -389,6 +447,14 @@ describe('BlobValue', () => { it('throws on non-string oid', () => { expect(() => new BlobValue('n1', /** @type {any} */ (42))).toThrow(); }); + + it('rejects node containing NUL byte', () => { + expect(() => new BlobValue('n\x001', 'oid123')).toThrow(/NUL/); + }); + + it('rejects node starting with \\x01 prefix', () => { + expect(() => new BlobValue('\x01n1', 'oid123')).toThrow(/reserved prefix/); + }); }); describe('cross-class instanceof isolation', () => { diff --git a/test/unit/domain/types/ops/validate.test.js b/test/unit/domain/types/ops/validate.test.js index 6db8cdc5..a2b19643 100644 --- a/test/unit/domain/types/ops/validate.test.js +++ b/test/unit/domain/types/ops/validate.test.js @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { assertNonEmptyString, assertNoBannedBytes, assertArray } from '../../../../../src/domain/types/ops/validate.js'; +import { assertNonEmptyString, assertNoReservedBytes, assertArray } from '../../../../../src/domain/types/ops/validate.js'; describe('assertNonEmptyString', () => { it('accepts a non-empty string', () => { @@ -19,18 +19,25 @@ describe('assertNonEmptyString', () => { }); }); -describe('assertNoBannedBytes', () => { +describe('assertNoReservedBytes', () => { it('accepts a clean string', () => { - expect(() => assertNoBannedBytes('user:alice', 'Test', 'node')).not.toThrow(); + expect(() => assertNoReservedBytes('user:alice', 'Test', 'node')).not.toThrow(); }); it('throws on NUL byte', () => { - expect(() => assertNoBannedBytes('user\x00alice', 'TestOp', 'node')).toThrow("TestOp 'node' must not contain NUL"); + expect(() => assertNoReservedBytes('user\x00alice', 'TestOp', 'node')).toThrow("TestOp 'node' must not contain NUL"); }); - it('accepts strings with other special characters', () => { - expect(() => assertNoBannedBytes('user\x01alice', 'Test', 'node')).not.toThrow(); - expect(() => assertNoBannedBytes('user:alice:bob', 'Test', 'node')).not.toThrow(); + it('throws on \\x01 prefix', () => { + expect(() => assertNoReservedBytes('\x01user:alice', 'TestOp', 'node')).toThrow("TestOp 'node' must not start with reserved prefix \\x01"); + }); + + it('accepts \\x01 in non-prefix position', () => { + expect(() => assertNoReservedBytes('user\x01alice', 'Test', 'node')).not.toThrow(); + }); + + it('accepts strings with colons and other special characters', () => { + expect(() => assertNoReservedBytes('user:alice:bob', 'Test', 'node')).not.toThrow(); }); }); From 90ce8e66bf79caf383cc394d5b57ac72b8191847 Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 5 Apr 2026 18:38:58 -0700 Subject: [PATCH 08/13] =?UTF-8?q?docs:=20backlog=20cool-idea=20=E2=80=94?= =?UTF-8?q?=20incremental=20history=20backfill?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use case: mirroring git history into a warp graph. HEAD snapshot is fast, but full history backfill is O(commits). Explores edge-based chronological ordering, checkpoint seeding, wormhole compression, and lazy materialization with continuation tokens. Cycle 0009 — op-type-class-hierarchy --- .../PROTO_incremental-history-backfill.md | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 docs/method/backlog/cool-ideas/PROTO_incremental-history-backfill.md diff --git a/docs/method/backlog/cool-ideas/PROTO_incremental-history-backfill.md b/docs/method/backlog/cool-ideas/PROTO_incremental-history-backfill.md new file mode 100644 index 00000000..a15a4680 --- /dev/null +++ b/docs/method/backlog/cool-ideas/PROTO_incremental-history-backfill.md @@ -0,0 +1,53 @@ +# Incremental History Backfill for Git Mirror Use Cases + +**Effort:** L + +## Idea + +Use case: mirroring git history into a warp graph — map each git +commit to an observation (one tick per commit). Works fine for HEAD +snapshots, but backfilling full history is O(commits) and takes +forever for large repos. + +The append-only ledger makes incremental backfill hard: you can't +insert ticks at arbitrary points in the causal past. Once the +frontier has advanced, earlier observations can only arrive as new +patches, not as historical insertions. + +## Possible approaches + +- **Edges as chronological ordering:** Nodes represent commits, + edges encode Lamport-ordered relationships between them. Backfill + adds nodes + edges in reverse chronological order. Each batch is + a new patch at the current frontier, but the edge structure + encodes the historical timeline. The graph's materialized state + shows the full DAG even though the patches arrived out of order. + +- **Checkpoint-based seeding:** Snapshot HEAD as a checkpoint + (fast). Then backfill historical commits as patches that add + nodes/edges the snapshot doesn't cover. The checkpoint provides + the "current state" instantly; backfill adds provenance depth + incrementally. + +- **Wormhole compression:** Paper III wormholes compress multi-tick + segments into single edges carrying sub-payloads. A "history + import" wormhole could represent N git commits as a single + compressed segment, expandable on demand. + +- **Lazy materialization with continuation tokens:** Don't backfill + eagerly. Instead, when a query touches a node that references + unmaterialized history, fetch and materialize on demand. The + continuation token marks how far back the graph has been + materialized. + +## Why it matters + +Git-warp-as-git-mirror is a compelling use case for the Inspector +and for agent collaboration. But any repo with >10K commits makes +naive tick-per-commit infeasible. The solution probably involves +separating "current state" (fast) from "historical provenance" +(incremental, lazy, or compressed). + +## Source + +James Ross, 2026-04-05. Raw idea during cycle 0009 session. From c6eb3139994f8d9ab3763bb0b7d741f68b44d4d1 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 6 Apr 2026 22:44:25 -0700 Subject: [PATCH 09/13] fix: validate op removal and prop key inputs --- src/domain/types/ops/EdgeRemove.js | 3 +++ src/domain/types/ops/NodeRemove.js | 3 +++ src/domain/types/ops/PropSet.js | 3 ++- test/unit/domain/types/ops/Op.test.js | 12 ++++++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/domain/types/ops/EdgeRemove.js b/src/domain/types/ops/EdgeRemove.js index f94b0054..e5d9cebe 100644 --- a/src/domain/types/ops/EdgeRemove.js +++ b/src/domain/types/ops/EdgeRemove.js @@ -37,6 +37,9 @@ export default class EdgeRemove extends Op { assertNoReservedBytes(to, 'EdgeRemove', 'to'); assertNoReservedBytes(label, 'EdgeRemove', 'label'); assertArray(observedDots, 'EdgeRemove', 'observedDots'); + for (let i = 0; i < observedDots.length; i += 1) { + assertNonEmptyString(observedDots[i], 'EdgeRemove', `observedDots[${i}]`); + } this.from = from; this.to = to; this.label = label; diff --git a/src/domain/types/ops/NodeRemove.js b/src/domain/types/ops/NodeRemove.js index d4d58d13..02380c60 100644 --- a/src/domain/types/ops/NodeRemove.js +++ b/src/domain/types/ops/NodeRemove.js @@ -28,6 +28,9 @@ export default class NodeRemove extends Op { assertNonEmptyString(node, 'NodeRemove', 'node'); assertNoReservedBytes(node, 'NodeRemove', 'node'); assertArray(observedDots, 'NodeRemove', 'observedDots'); + for (let i = 0; i < observedDots.length; i += 1) { + assertNonEmptyString(observedDots[i], 'NodeRemove', `observedDots[${i}]`); + } this.node = node; this.observedDots = Object.freeze([...observedDots]); Object.freeze(this); diff --git a/src/domain/types/ops/PropSet.js b/src/domain/types/ops/PropSet.js index 08fa6562..0c168038 100644 --- a/src/domain/types/ops/PropSet.js +++ b/src/domain/types/ops/PropSet.js @@ -9,7 +9,7 @@ */ import Op from './Op.js'; -import { assertNonEmptyString } from './validate.js'; +import { assertNonEmptyString, assertNoReservedBytes } from './validate.js'; /** * Sets a property on a node (raw wire format). @@ -36,6 +36,7 @@ export default class PropSet extends Op { super('PropSet'); assertNonEmptyString(node, 'PropSet', 'node'); assertNonEmptyString(key, 'PropSet', 'key'); + assertNoReservedBytes(key, 'PropSet', 'key'); this.node = node; this.key = key; this.value = value; diff --git a/test/unit/domain/types/ops/Op.test.js b/test/unit/domain/types/ops/Op.test.js index 1d6df88c..1666592c 100644 --- a/test/unit/domain/types/ops/Op.test.js +++ b/test/unit/domain/types/ops/Op.test.js @@ -128,6 +128,10 @@ describe('NodeRemove', () => { expect(() => new NodeRemove('n1', /** @type {any} */ ('alice:1'))).toThrow(); }); + it('throws when observedDots contains an empty string', () => { + expect(() => new NodeRemove('n1', [''])).toThrow(/observedDots\[0\]/); + }); + it('accepts empty observedDots array', () => { const op = new NodeRemove('user:alice', []); expect(op.observedDots).toEqual([]); @@ -245,6 +249,10 @@ describe('EdgeRemove', () => { expect(() => new EdgeRemove({ from: 'n1', to: 'n2', label: 'rel', observedDots: /** @type {any} */ ('w:1') })).toThrow(); }); + it('throws when observedDots contains an empty string', () => { + expect(() => new EdgeRemove({ from: 'n1', to: 'n2', label: 'rel', observedDots: [''] })).toThrow(/observedDots\[0\]/); + }); + it('rejects from/to/label containing NUL byte', () => { expect(() => new EdgeRemove({ from: 'n\x001', to: 'n2', label: 'rel', observedDots: [] })).toThrow(/NUL/); expect(() => new EdgeRemove({ from: 'n1', to: 'n\x002', label: 'rel', observedDots: [] })).toThrow(/NUL/); @@ -409,6 +417,10 @@ describe('PropSet (raw/wire format)', () => { expect(() => new PropSet('n1', '', 'v')).toThrow(); }); + it('rejects key containing NUL byte', () => { + expect(() => new PropSet('n1', 'k\x00ey', 'v')).toThrow(/NUL/); + }); + it('accepts edge-property encoded node (\\x01 prefix)', () => { const op = new PropSet('\x01n1\x00n2\x00rel', 'k', 'v'); expect(op.node).toBe('\x01n1\x00n2\x00rel'); From cb5ab974d02a2fa539a0369aeb3e7c8785daf205 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 6 Apr 2026 22:48:56 -0700 Subject: [PATCH 10/13] fix: align op types with class-backed factories --- bin/cli/commands/debug/shared.js | 12 +-- src/domain/services/JoinReducer.js | 2 +- src/domain/types/WarpTypesV2.js | 73 ++++++------------- test/unit/domain/types/WarpTypesV2.test.js | 5 +- .../types/ops/factory-integration.test.js | 10 +-- 5 files changed, 36 insertions(+), 66 deletions(-) diff --git a/bin/cli/commands/debug/shared.js b/bin/cli/commands/debug/shared.js index 1f5b98d0..3a46ed21 100644 --- a/bin/cli/commands/debug/shared.js +++ b/bin/cli/commands/debug/shared.js @@ -197,6 +197,8 @@ export function compareNumbers(a, b) { return a === b ? 0 : (a < b ? -1 : 1); } +/** @typedef {{ type: string, node?: string, from?: string, to?: string }} DebugOpLike */ + /** * Adds a string field from an op to the set if it is a non-empty string. * @@ -212,7 +214,7 @@ function addIfNonEmptyString(ids, value) { /** * Collects unique node/edge endpoint IDs referenced by patch operations. * - * @param {Array & { type: string }>|undefined} ops - Raw patch operations + * @param {DebugOpLike[]|undefined} ops - Raw patch operations * @returns {string[]} Sorted unique identifiers */ export function collectTouchedIds(ops) { @@ -223,9 +225,9 @@ export function collectTouchedIds(ops) { /** @type {Set} */ const ids = new Set(); for (const op of ops) { - addIfNonEmptyString(ids, op['node']); - addIfNonEmptyString(ids, op['from']); - addIfNonEmptyString(ids, op['to']); + addIfNonEmptyString(ids, op.node); + addIfNonEmptyString(ids, op.from); + addIfNonEmptyString(ids, op.to); } return /** @type {string[]} */ ([...ids].sort(compareStrings)); @@ -274,7 +276,7 @@ export function sortPatchEntriesCausally(entries) { return [...entries].sort(comparePatchEntries); } -/** @typedef {{ writer?: string, lamport?: number, schema?: number, ops?: Array & { type: string }>, reads?: string[] | undefined, writes?: string[] | undefined }} DebugPatch */ +/** @typedef {{ writer?: string, lamport?: number, schema?: number, ops?: DebugOpLike[], reads?: string[] | undefined, writes?: string[] | undefined }} DebugPatch */ /** * Safely copies a string array or returns an empty array. diff --git a/src/domain/services/JoinReducer.js b/src/domain/services/JoinReducer.js index c5046c30..d4c29dfb 100644 --- a/src/domain/services/JoinReducer.js +++ b/src/domain/services/JoinReducer.js @@ -41,7 +41,7 @@ export { normalizeRawOp, lowerCanonicalOp } from './OpNormalizer.js'; * @property {string} type - Operation type discriminator * @property {string} [node] - Node ID (for NodeAdd, NodeRemove, PropSet) * @property {import('../crdt/Dot.js').Dot} [dot] - Dot identifier (for NodeAdd, EdgeAdd) - * @property {string[]} [observedDots] - Encoded dots to remove (for NodeRemove, EdgeRemove) + * @property {ReadonlyArray|string[]} [observedDots] - Encoded dots to remove (for NodeRemove, EdgeRemove) * @property {string} [from] - Source node ID (for EdgeAdd, EdgeRemove) * @property {string} [to] - Target node ID (for EdgeAdd, EdgeRemove) * @property {string} [label] - Edge label (for EdgeAdd, EdgeRemove) diff --git a/src/domain/types/WarpTypesV2.js b/src/domain/types/WarpTypesV2.js index 6941c114..9de0e091 100644 --- a/src/domain/types/WarpTypesV2.js +++ b/src/domain/types/WarpTypesV2.js @@ -23,6 +23,8 @@ import NodePropSetClass from './ops/NodePropSet.js'; import EdgePropSetClass from './ops/EdgePropSet.js'; import PropSetClass from './ops/PropSet.js'; +/** @typedef {import('./ops/BlobValue.js').default} BlobValueClass */ + // Re-export PatchV2 class for consumers that import from this module. export { PatchV2 }; @@ -45,39 +47,23 @@ export { PatchV2 }; // ============================================================================ /** - * Node add operation - creates a new node with a dot - * @typedef {Object} OpV2NodeAdd - * @property {'NodeAdd'} type - Operation type discriminator - * @property {NodeId} node - Node ID to add - * @property {Dot} dot - Causal identifier for this add + * Node add operation - creates a new node with a dot. + * @typedef {NodeAddClass} OpV2NodeAdd */ /** - * Node remove operation - removes a node by observed dots - * @typedef {Object} OpV2NodeRemove - * @property {'NodeRemove'} type - Operation type discriminator - * @property {NodeId} node - Node ID to remove - * @property {string[]} observedDots - Encoded dot strings being removed (add events observed) + * Node remove operation - removes a node by observed dots. + * @typedef {NodeRemoveClass} OpV2NodeRemove */ /** - * Edge add operation - creates a new edge with a dot - * @typedef {Object} OpV2EdgeAdd - * @property {'EdgeAdd'} type - Operation type discriminator - * @property {NodeId} from - Source node ID - * @property {NodeId} to - Target node ID - * @property {string} label - Edge label/type - * @property {Dot} dot - Causal identifier for this add + * Edge add operation - creates a new edge with a dot. + * @typedef {EdgeAddClass} OpV2EdgeAdd */ /** - * Edge remove operation - removes an edge by observed dots - * @typedef {Object} OpV2EdgeRemove - * @property {'EdgeRemove'} type - Operation type discriminator - * @property {NodeId} from - Source node ID - * @property {NodeId} to - Target node ID - * @property {string} label - Edge label/type - * @property {string[]} observedDots - Encoded dot strings being removed (add events observed) + * Edge remove operation - removes an edge by observed dots. + * @typedef {EdgeRemoveClass} OpV2EdgeRemove */ /** @@ -88,39 +74,22 @@ export { PatchV2 }; * field carrying a \x01-prefixed edge identity. See {@link OpV2NodePropSet} * and {@link OpV2EdgePropSet} for the canonical (internal) representations. * - * @typedef {Object} OpV2PropSet - * @property {'PropSet'} type - Operation type discriminator - * @property {NodeId} node - Node ID to set property on (may contain \x01 prefix for edge props) - * @property {string} key - Property key - * @property {unknown} value - Property value (any JSON-serializable type) + * @typedef {PropSetClass} OpV2PropSet */ /** * Canonical node property set operation (internal only — never persisted). - * @typedef {Object} OpV2NodePropSet - * @property {'NodePropSet'} type - Operation type discriminator - * @property {NodeId} node - Node ID to set property on - * @property {string} key - Property key - * @property {unknown} value - Property value (any JSON-serializable type) + * @typedef {NodePropSetClass} OpV2NodePropSet */ /** * Canonical edge property set operation (internal only — never persisted). - * @typedef {Object} OpV2EdgePropSet - * @property {'EdgePropSet'} type - Operation type discriminator - * @property {NodeId} from - Source node ID - * @property {NodeId} to - Target node ID - * @property {string} label - Edge label - * @property {string} key - Property key - * @property {unknown} value - Property value (any JSON-serializable type) + * @typedef {EdgePropSetClass} OpV2EdgePropSet */ /** * Blob value reference operation. - * @typedef {Object} OpV2BlobValue - * @property {'BlobValue'} type - Operation type discriminator - * @property {string} node - Node ID the blob is attached to - * @property {string} oid - Blob object ID in the Git object store + * @typedef {BlobValueClass} OpV2BlobValue */ /** @@ -155,7 +124,7 @@ export { PatchV2 }; * Creates a NodeAdd operation with a dot * @param {NodeId} node - Node ID to add * @param {Dot} dot - Causal identifier for this add - * @returns {OpV2NodeAdd} NodeAdd operation + * @returns {NodeAddClass} NodeAdd operation */ export function createNodeAddV2(node, dot) { return new NodeAddClass(node, dot); @@ -165,7 +134,7 @@ export function createNodeAddV2(node, dot) { * Creates a NodeRemove operation with observed dots * @param {NodeId} node - Node ID to remove * @param {string[]} observedDots - Encoded dot strings being removed - * @returns {OpV2NodeRemove} NodeRemove operation + * @returns {NodeRemoveClass} NodeRemove operation */ export function createNodeRemoveV2(node, observedDots) { return new NodeRemoveClass(node, observedDots); @@ -177,7 +146,7 @@ export function createNodeRemoveV2(node, observedDots) { * @param {NodeId} to - Target node ID * @param {string} label - Edge label * @param {Dot} dot - Causal identifier for this add - * @returns {OpV2EdgeAdd} EdgeAdd operation + * @returns {EdgeAddClass} EdgeAdd operation */ export function createEdgeAddV2(from, to, label, dot) { return new EdgeAddClass({ from, to, label, dot }); @@ -189,7 +158,7 @@ export function createEdgeAddV2(from, to, label, dot) { * @param {NodeId} to - Target node ID * @param {string} label - Edge label * @param {string[]} observedDots - Encoded dot strings being removed - * @returns {OpV2EdgeRemove} EdgeRemove operation + * @returns {EdgeRemoveClass} EdgeRemove operation */ export function createEdgeRemoveV2(from, to, label, observedDots) { return new EdgeRemoveClass({ from, to, label, observedDots }); @@ -202,7 +171,7 @@ export function createEdgeRemoveV2(from, to, label, observedDots) { * @param {NodeId} node - Node ID to set property on * @param {string} key - Property key * @param {unknown} value - Property value (any JSON-serializable type) - * @returns {OpV2PropSet} PropSet operation + * @returns {PropSetClass} PropSet operation */ export function createPropSetV2(node, key, value) { return new PropSetClass(node, key, value); @@ -213,7 +182,7 @@ export function createPropSetV2(node, key, value) { * @param {NodeId} node - Node ID to set property on * @param {string} key - Property key * @param {unknown} value - Property value (any JSON-serializable type) - * @returns {OpV2NodePropSet} NodePropSet operation + * @returns {NodePropSetClass} NodePropSet operation */ export function createNodePropSetV2(node, key, value) { return new NodePropSetClass(node, key, value); @@ -226,7 +195,7 @@ export function createNodePropSetV2(node, key, value) { * @param {string} label - Edge label * @param {string} key - Property key * @param {unknown} value - Property value (any JSON-serializable type) - * @returns {OpV2EdgePropSet} EdgePropSet operation + * @returns {EdgePropSetClass} EdgePropSet operation */ export function createEdgePropSetV2(from, to, label, key, value) { return new EdgePropSetClass({ from, to, label, key, value }); diff --git a/test/unit/domain/types/WarpTypesV2.test.js b/test/unit/domain/types/WarpTypesV2.test.js index 0c4df8b6..d88bfb2d 100644 --- a/test/unit/domain/types/WarpTypesV2.test.js +++ b/test/unit/domain/types/WarpTypesV2.test.js @@ -439,11 +439,10 @@ describe('WarpTypesV2', () => { describe('Op class instanceof', () => { it('all factory-created ops are instanceof Op', () => { - const dot = new Dot('w', 1); const ops = [ - createNodeAddV2('n1', dot), + createNodeAddV2('n1', new Dot('w', 1)), createNodeRemoveV2('n1', []), - createEdgeAddV2('n1', 'n2', 'r', dot), + createEdgeAddV2('n1', 'n2', 'r', new Dot('w', 2)), createEdgeRemoveV2('n1', 'n2', 'r', []), createPropSetV2('n1', 'k', 'v'), ]; diff --git a/test/unit/domain/types/ops/factory-integration.test.js b/test/unit/domain/types/ops/factory-integration.test.js index 680ef67c..0a57b47e 100644 --- a/test/unit/domain/types/ops/factory-integration.test.js +++ b/test/unit/domain/types/ops/factory-integration.test.js @@ -12,7 +12,6 @@ import EdgeRemove from '../../../../../src/domain/types/ops/EdgeRemove.js'; import NodePropSet from '../../../../../src/domain/types/ops/NodePropSet.js'; import EdgePropSet from '../../../../../src/domain/types/ops/EdgePropSet.js'; import PropSetClass from '../../../../../src/domain/types/ops/PropSet.js'; -import BlobValue from '../../../../../src/domain/types/ops/BlobValue.js'; import { createNodeAddV2, createNodeRemoveV2, @@ -122,10 +121,11 @@ describe('OpNormalizer returns Op class instances', () => { const canonical = normalizeRawOp(raw); expect(canonical).toBeInstanceOf(NodePropSet); - expect(canonical.type).toBe('NodePropSet'); - expect(canonical.node).toBe('user:alice'); - expect(canonical.key).toBe('name'); - expect(canonical.value).toBe('Alice'); + const nodeProp = /** @type {NodePropSet} */ (canonical); + expect(nodeProp.type).toBe('NodePropSet'); + expect(nodeProp.node).toBe('user:alice'); + expect(nodeProp.key).toBe('name'); + expect(nodeProp.value).toBe('Alice'); }); it('normalizeRawOp converts PropSet (edge) to EdgePropSet instance', () => { From fed7fa4630be2b88f8d77fb1865e91a8af170f39 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 6 Apr 2026 22:52:38 -0700 Subject: [PATCH 11/13] docs: fix backlog code sample syntax --- .../CC_versionvector-constructor-no-validation.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/method/backlog/bad-code/CC_versionvector-constructor-no-validation.md b/docs/method/backlog/bad-code/CC_versionvector-constructor-no-validation.md index dda8539c..25996e01 100644 --- a/docs/method/backlog/bad-code/CC_versionvector-constructor-no-validation.md +++ b/docs/method/backlog/bad-code/CC_versionvector-constructor-no-validation.md @@ -14,11 +14,13 @@ should reject missing or non-Map arguments per P2 (boundary validation). Add validation to the constructor: ```js -constructor(entries) { - if (!(entries instanceof Map)) { - throw new CrdtError('VersionVector requires a Map'); +class VersionVector { + constructor(entries) { + if (!(entries instanceof Map)) { + throw new CrdtError('VersionVector requires a Map'); + } + this.#entries = entries; } - this.#entries = entries; } ``` From ee3e3e4dcdc66a33ba6eb60834d22b91ad514fb0 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 6 Apr 2026 23:41:03 -0700 Subject: [PATCH 12/13] ci: run node 22 tests directly on github runner --- .github/workflows/ci.yml | 11 ++++++++++- package.json | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06671702..0690fe36 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,8 +52,17 @@ jobs: node: [22] steps: - uses: actions/checkout@v6 + - name: Use Node.js + uses: actions/setup-node@v6 + with: + node-version: '${{ matrix.node }}' + cache: 'npm' + - name: Install BATS + run: sudo apt-get update && sudo apt-get install -y bats + - name: Install dependencies + run: npm ci - name: Run unit + integration tests - run: docker compose -f docker-compose.test.yml run --rm test-node${{ matrix.node }} + run: npm run test:node22:ci test-bun: runs-on: ubuntu-latest diff --git a/package.json b/package.json index 16d1e5f7..9571010d 100644 --- a/package.json +++ b/package.json @@ -96,6 +96,7 @@ "uninstall:git-warp": "bash scripts/uninstall-git-warp.sh", "test:node20": "docker compose -f docker-compose.test.yml --profile node20 run --build --rm test-node20", "test:node22": "docker compose -f docker-compose.test.yml --profile node22 run --build --rm test-node22", + "test:node22:ci": "GIT_STUNTS_DOCKER=1 sh -c 'npx vitest run test/unit test/integration && bats test/bats/'", "test:bun": "docker compose -f docker-compose.test.yml --profile bun run --build --rm test-bun", "test:deno": "docker compose -f docker-compose.test.yml --profile deno run --build --rm test-deno", "test:matrix": "docker compose -f docker-compose.test.yml --profile full up --build --abort-on-container-exit", From e96d48d97de7b36c7d6a49f9da4236a7205feef0 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 6 Apr 2026 23:59:53 -0700 Subject: [PATCH 13/13] ci: install node runner cli shims --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c54ecb2..058342de 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,6 +75,11 @@ jobs: run: sudo apt-get update && sudo apt-get install -y bats - name: Install dependencies run: npm ci + - name: Install CLI shims + run: | + sudo install -m 0755 bin/git-warp /usr/local/bin/git-warp + sudo sh -c "printf '%s\n' '#!/usr/bin/env bash' 'exec node \"$GITHUB_WORKSPACE/bin/warp-graph.js\" \"\$@\"' > /usr/local/bin/warp-graph" + sudo chmod +x /usr/local/bin/warp-graph - name: Run unit + integration tests run: npm run test:node22:ci