From 9869bf90b1e9e48b3c0da3179e93d8c20a2c5efa Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 09:03:01 -0800 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20InMemoryGraphAdapter=20=E2=80=94=20?= =?UTF-8?q?zero-I/O=20persistence=20for=20fast=20tests=20(MEM-ADAPTER,=20v?= =?UTF-8?q?10.7.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 15 + ROADMAP.md | 4 +- package.json | 2 +- .../adapters/GitGraphAdapter.js | 65 +-- .../adapters/InMemoryGraphAdapter.js | 481 ++++++++++++++++++ .../adapters/adapterValidation.js | 90 ++++ test/helpers/warpGraphTestUtils.js | 20 + .../adapters/AdapterConformance.js | 265 ++++++++++ .../adapters/GitGraphAdapter.coverage.test.js | 9 + .../InMemoryGraphAdapter.integration.test.js | 52 ++ .../adapters/InMemoryGraphAdapter.test.js | 94 ++++ 11 files changed, 1039 insertions(+), 58 deletions(-) create mode 100644 src/infrastructure/adapters/InMemoryGraphAdapter.js create mode 100644 src/infrastructure/adapters/adapterValidation.js create mode 100644 test/unit/infrastructure/adapters/AdapterConformance.js create mode 100644 test/unit/infrastructure/adapters/InMemoryGraphAdapter.integration.test.js create mode 100644 test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index d1aa12c..dd885b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [10.7.0] — 2026-02-11 — MEM-ADAPTER: In-Memory Persistence + +Adds `InMemoryGraphAdapter`, a zero-I/O implementation of `GraphPersistencePort` for fast tests. + +### Added + +- **`InMemoryGraphAdapter`** (`src/infrastructure/adapters/InMemoryGraphAdapter.js`): Full in-memory implementation of all five ports (Commit, Blob, Tree, Ref, Config). Uses Git's SHA-1 object format for content-addressable hashing. Accepts optional `author` and `clock` injection for deterministic tests. +- **`adapterValidation.js`** (`src/infrastructure/adapters/adapterValidation.js`): Extracted shared validation functions (`validateOid`, `validateRef`, `validateLimit`, `validateConfigKey`) used by both adapters. +- **Adapter conformance suite** (`test/unit/infrastructure/adapters/AdapterConformance.js`): ~25 shared behavioral tests that run against any `GraphPersistencePort` implementation, ensuring parity between Git and in-memory adapters. +- **`createInMemoryRepo()`** (`test/helpers/warpGraphTestUtils.js`): Test factory for instant in-memory adapter setup — no temp dirs, no git subprocesses. + +### Changed + +- **`GitGraphAdapter`**: Validation methods now delegate to shared `adapterValidation.js` functions. No behavioral change. + ## [10.6.1] — 2026-02-11 — Code Review Fixups Addresses code review feedback from PR #23 across SEEKDIFF and SHIELD features. diff --git a/ROADMAP.md b/ROADMAP.md index 8ae2e70..5e5b75e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -152,7 +152,7 @@ All 12 milestones (77 tasks, ~255 human hours, ~13,100 LOC) have been implemente ### M2.T1.MEM-ADAPTER (A-Tier) -- **Status:** `OPEN` +- **Status:** `DONE` **User Story:** As an architect, I need fast in-memory tests to validate risky logic quickly. @@ -162,6 +162,8 @@ All 12 milestones (77 tasks, ~255 human hours, ~13,100 LOC) have been implemente - parity behaviors with Git adapter (including integrity constraints where relevant) - shared tests run against both adapters +**Delivered in v10.7.0.** + **Acceptance Criteria:** domain suite passes on memory + git adapters **Definition of Done:** adapter integrated in test harness; CI includes adapter matrix lane diff --git a/package.json b/package.json index dbdd028..43af3f2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@git-stunts/git-warp", - "version": "10.6.1", + "version": "10.7.0", "description": "Deterministic WARP graph over Git: graph-native storage, traversal, and tooling.", "type": "module", "license": "Apache-2.0", diff --git a/src/infrastructure/adapters/GitGraphAdapter.js b/src/infrastructure/adapters/GitGraphAdapter.js index f418bd4..644f3ca 100644 --- a/src/infrastructure/adapters/GitGraphAdapter.js +++ b/src/infrastructure/adapters/GitGraphAdapter.js @@ -45,6 +45,7 @@ import { retry } from '@git-stunts/alfred'; import GraphPersistencePort from '../../ports/GraphPersistencePort.js'; +import { validateOid, validateRef, validateLimit, validateConfigKey } from './adapterValidation.js'; /** * Transient Git errors that are safe to retry automatically. @@ -374,30 +375,13 @@ export default class GitGraphAdapter extends GraphPersistencePort { /** * Validates that a ref is safe to use in git commands. - * Prevents command injection via malicious ref names. + * Delegates to shared validation in adapterValidation.js. * @param {string} ref - The ref to validate * @throws {Error} If ref contains invalid characters, is too long, or starts with -/-- * @private */ _validateRef(ref) { - if (!ref || typeof ref !== 'string') { - throw new Error('Ref must be a non-empty string'); - } - // Prevent buffer overflow attacks with extremely long refs - if (ref.length > 1024) { - throw new Error(`Ref too long: ${ref.length} chars. Maximum is 1024`); - } - // Prevent git option injection (must check before pattern matching) - if (ref.startsWith('-') || ref.startsWith('--')) { - throw new Error(`Invalid ref: ${ref}. Refs cannot start with - or --. See https://github.com/git-stunts/git-warp#security`); - } - // Allow alphanumeric, ., /, -, _ in names - // Allow ancestry operators: ^ or ~ optionally followed by digits - // Allow range operators: .. between names - const validRefPattern = /^[a-zA-Z0-9._/-]+((~\d*|\^\d*|\.\.[a-zA-Z0-9._/-]+)*)$/; - if (!validRefPattern.test(ref)) { - throw new Error(`Invalid ref format: ${ref}. Only alphanumeric characters, ., /, -, _, ^, ~, and range operators are allowed. See https://github.com/git-stunts/git-warp#ref-validation`); - } + validateRef(ref); } /** @@ -546,42 +530,24 @@ export default class GitGraphAdapter extends GraphPersistencePort { /** * Validates that an OID is safe to use in git commands. + * Delegates to shared validation in adapterValidation.js. * @param {string} oid - The OID to validate * @throws {Error} If OID is invalid * @private */ _validateOid(oid) { - if (!oid || typeof oid !== 'string') { - throw new Error('OID must be a non-empty string'); - } - if (oid.length > 64) { - throw new Error(`OID too long: ${oid.length} chars. Maximum is 64`); - } - const validOidPattern = /^[0-9a-fA-F]{4,64}$/; - if (!validOidPattern.test(oid)) { - throw new Error(`Invalid OID format: ${oid}`); - } + validateOid(oid); } /** * Validates that a limit is a safe positive integer. + * Delegates to shared validation in adapterValidation.js. * @param {number} limit - The limit to validate * @throws {Error} If limit is invalid * @private */ _validateLimit(limit) { - if (typeof limit !== 'number' || !Number.isFinite(limit)) { - throw new Error('Limit must be a finite number'); - } - if (!Number.isInteger(limit)) { - throw new Error('Limit must be an integer'); - } - if (limit <= 0) { - throw new Error('Limit must be a positive integer'); - } - if (limit > 10_000_000) { - throw new Error(`Limit too large: ${limit}. Maximum is 10,000,000`); - } + validateLimit(limit); } /** @@ -721,26 +687,13 @@ export default class GitGraphAdapter extends GraphPersistencePort { /** * Validates that a config key is safe to use in git commands. + * Delegates to shared validation in adapterValidation.js. * @param {string} key - The config key to validate * @throws {Error} If key is invalid * @private */ _validateConfigKey(key) { - if (!key || typeof key !== 'string') { - throw new Error('Config key must be a non-empty string'); - } - if (key.length > 256) { - throw new Error(`Config key too long: ${key.length} chars. Maximum is 256`); - } - // Prevent git option injection - if (key.startsWith('-')) { - throw new Error(`Invalid config key: ${key}. Keys cannot start with -`); - } - // Allow section.subsection.key format - const validKeyPattern = /^[a-zA-Z][a-zA-Z0-9._-]*$/; - if (!validKeyPattern.test(key)) { - throw new Error(`Invalid config key format: ${key}`); - } + validateConfigKey(key); } /** diff --git a/src/infrastructure/adapters/InMemoryGraphAdapter.js b/src/infrastructure/adapters/InMemoryGraphAdapter.js new file mode 100644 index 0000000..b5f7fa6 --- /dev/null +++ b/src/infrastructure/adapters/InMemoryGraphAdapter.js @@ -0,0 +1,481 @@ +/* eslint-disable @typescript-eslint/require-await -- all async methods match the port contract */ +/** + * @fileoverview In-memory persistence adapter for WARP graph storage. + * + * Implements the same {@link GraphPersistencePort} contract as GitGraphAdapter + * but stores all data in Maps. Designed for fast unit/integration tests that + * don't need real Git I/O. + * + * SHA computation follows Git's object format so debugging is straightforward, + * but cross-adapter SHA matching is NOT guaranteed. + * + * @module infrastructure/adapters/InMemoryGraphAdapter + */ + +import { createHash } from 'node:crypto'; +import { Readable } from 'node:stream'; +import GraphPersistencePort from '../../ports/GraphPersistencePort.js'; +import { validateOid, validateRef, validateLimit, validateConfigKey } from './adapterValidation.js'; + +/** Well-known SHA for Git's empty tree. */ +const EMPTY_TREE_OID = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'; + +// ── SHA helpers ───────────────────────────────────────────────────────── + +/** + * Computes a Git blob SHA-1: `SHA1("blob " + len + "\0" + content)`. + * @param {Buffer} content + * @returns {string} 40-hex SHA + */ +function hashBlob(content) { + const header = Buffer.from(`blob ${content.length}\0`); + return createHash('sha1').update(header).update(content).digest('hex'); +} + +/** + * Builds the binary tree buffer in Git's internal format and hashes it. + * + * Each entry is: ` \0<20-byte binary OID>` + * Entries are sorted by path (byte order), matching Git's canonical sort. + * + * @param {Array<{mode: string, path: string, oid: string}>} entries + * @returns {string} 40-hex SHA + */ +function hashTree(entries) { + const sorted = [...entries].sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)); + const parts = sorted.map(e => { + const prefix = Buffer.from(`${e.mode} ${e.path}\0`); + const oidBin = Buffer.from(e.oid, 'hex'); + return Buffer.concat([prefix, oidBin]); + }); + const body = Buffer.concat(parts); + const header = Buffer.from(`tree ${body.length}\0`); + return createHash('sha1').update(header).update(body).digest('hex'); +} + +/** + * Builds a Git-style commit string and hashes it. + * @param {{treeOid: string, parents: string[], message: string, author: string, date: string}} opts + * @returns {string} 40-hex SHA + */ +function hashCommit({ treeOid, parents, message, author, date }) { + const lines = [`tree ${treeOid}`]; + for (const p of parents) { + lines.push(`parent ${p}`); + } + lines.push(`author ${author} ${date}`); + lines.push(`committer ${author} ${date}`); + lines.push(''); + lines.push(message); + const body = lines.join('\n'); + const header = `commit ${Buffer.byteLength(body)}\0`; + return createHash('sha1').update(header).update(body).digest('hex'); +} + +// ── Adapter ───────────────────────────────────────────────────────────── + +/** + * In-memory implementation of {@link GraphPersistencePort}. + * + * Data structures: + * - `_commits` — Map + * - `_blobs` — Map + * - `_trees` — Map> + * - `_refs` — Map + * - `_config` — Map + * + * @extends GraphPersistencePort + */ +export default class InMemoryGraphAdapter extends GraphPersistencePort { + /** + * @param {{ author?: string, clock?: { now: () => number } }} [options] + */ + constructor({ author, clock } = {}) { + super(); + this._author = author || 'InMemory '; + this._clock = clock || { now: () => Date.now() }; + + /** @type {Map} */ + this._commits = new Map(); + /** @type {Map} */ + this._blobs = new Map(); + /** @type {Map>} */ + this._trees = new Map(); + /** @type {Map} */ + this._refs = new Map(); + /** @type {Map} */ + this._config = new Map(); + } + + // ── TreePort ──────────────────────────────────────────────────────── + + /** @type {string} */ + get emptyTree() { + return EMPTY_TREE_OID; + } + + /** + * Creates a tree from mktree-formatted entries. + * @param {string[]} entries - Lines in `" \t"` format + * @returns {Promise} + */ + async writeTree(entries) { + const parsed = entries.map(line => { + const tabIdx = line.indexOf('\t'); + const meta = line.slice(0, tabIdx); + const path = line.slice(tabIdx + 1); + const [mode, , oid] = meta.split(' '); + return { mode, path, oid }; + }); + const oid = hashTree(parsed); + this._trees.set(oid, parsed); + return oid; + } + + /** + * @param {string} treeOid + * @returns {Promise>} + */ + async readTreeOids(treeOid) { + validateOid(treeOid); + if (treeOid === EMPTY_TREE_OID) { + return {}; + } + const entries = this._trees.get(treeOid); + if (!entries) { + throw new Error(`Tree not found: ${treeOid}`); + } + /** @type {Record} */ + const result = {}; + for (const e of entries) { + result[e.path] = e.oid; + } + return result; + } + + /** + * @param {string} treeOid + * @returns {Promise>} + */ + async readTree(treeOid) { + const oids = await this.readTreeOids(treeOid); + /** @type {Record} */ + const files = {}; + for (const [path, oid] of Object.entries(oids)) { + files[path] = await this.readBlob(oid); + } + return files; + } + + // ── BlobPort ──────────────────────────────────────────────────────── + + /** + * @param {Buffer|string} content + * @returns {Promise} + */ + async writeBlob(content) { + const buf = Buffer.isBuffer(content) ? content : Buffer.from(content); + const oid = hashBlob(buf); + this._blobs.set(oid, buf); + return oid; + } + + /** + * @param {string} oid + * @returns {Promise} + */ + async readBlob(oid) { + validateOid(oid); + const buf = this._blobs.get(oid); + if (!buf) { + throw new Error(`Blob not found: ${oid}`); + } + return buf; + } + + // ── CommitPort ────────────────────────────────────────────────────── + + /** + * @param {{ message: string, parents?: string[], sign?: boolean }} options + * @returns {Promise} + */ + async commitNode({ message, parents = [] }) { + for (const p of parents) { + validateOid(p); + } + return this._createCommit(EMPTY_TREE_OID, parents, message); + } + + /** + * @param {{ treeOid: string, parents?: string[], message: string, sign?: boolean }} options + * @returns {Promise} + */ + async commitNodeWithTree({ treeOid, parents = [], message }) { + validateOid(treeOid); + for (const p of parents) { + validateOid(p); + } + return this._createCommit(treeOid, parents, message); + } + + /** + * @param {string} sha + * @returns {Promise} + */ + async showNode(sha) { + validateOid(sha); + const commit = this._commits.get(sha); + if (!commit) { + throw new Error(`Commit not found: ${sha}`); + } + return commit.message; + } + + /** + * @param {string} sha + * @returns {Promise<{sha: string, message: string, author: string, date: string, parents: string[]}>} + */ + async getNodeInfo(sha) { + validateOid(sha); + const commit = this._commits.get(sha); + if (!commit) { + throw new Error(`Commit not found: ${sha}`); + } + return { + sha, + message: commit.message, + author: commit.author, + date: commit.date, + parents: [...commit.parents], + }; + } + + /** + * @param {string} sha + * @returns {Promise} + */ + async nodeExists(sha) { + validateOid(sha); + return this._commits.has(sha); + } + + /** + * @param {string} ref + * @returns {Promise} + */ + async countNodes(ref) { + validateRef(ref); + const tip = this._resolveRef(ref); + if (!tip) { + throw new Error(`Ref not found: ${ref}`); + } + const visited = new Set(); + const stack = [tip]; + while (stack.length > 0) { + const sha = /** @type {string} */ (stack.pop()); + if (visited.has(sha)) { + continue; + } + visited.add(sha); + const commit = this._commits.get(sha); + if (commit) { + for (const p of commit.parents) { + stack.push(p); + } + } + } + return visited.size; + } + + /** + * @param {{ ref: string, limit?: number, format?: string }} options + * @returns {Promise} + */ + async logNodes({ ref, limit = 50, format }) { + validateRef(ref); + validateLimit(limit); + const records = this._walkLog(ref, limit); + // If no format given, return a simple repr. With format, return NUL-terminated records. + if (!format) { + return records.map(c => `commit ${c.sha}\nAuthor: ${c.author}\nDate: ${c.date}\n\n ${c.message}\n`).join('\n'); + } + return records.map(c => this._formatCommitRecord(c)).join('\0') + (records.length > 0 ? '\0' : ''); + } + + /** + * @param {{ ref: string, limit?: number, format?: string }} options + * @returns {Promise} + */ + async logNodesStream({ ref, limit = 1000000, format: _format }) { + validateRef(ref); + validateLimit(limit); + const records = this._walkLog(ref, limit); + const formatted = records.map(c => this._formatCommitRecord(c)).join('\0') + (records.length > 0 ? '\0' : ''); + return Readable.from([formatted]); + } + + /** + * @returns {Promise<{ok: boolean, latencyMs: number}>} + */ + async ping() { + return { ok: true, latencyMs: 0 }; + } + + // ── RefPort ───────────────────────────────────────────────────────── + + /** + * @param {string} ref + * @param {string} oid + * @returns {Promise} + */ + async updateRef(ref, oid) { + validateRef(ref); + validateOid(oid); + this._refs.set(ref, oid); + } + + /** + * @param {string} ref + * @returns {Promise} + */ + async readRef(ref) { + validateRef(ref); + return this._refs.get(ref) || null; + } + + /** + * @param {string} ref + * @returns {Promise} + */ + async deleteRef(ref) { + validateRef(ref); + this._refs.delete(ref); + } + + /** + * @param {string} prefix + * @returns {Promise} + */ + async listRefs(prefix) { + validateRef(prefix); + const result = []; + for (const key of this._refs.keys()) { + if (key.startsWith(prefix)) { + result.push(key); + } + } + return result.sort(); + } + + // ── ConfigPort ────────────────────────────────────────────────────── + + /** + * @param {string} key + * @returns {Promise} + */ + async configGet(key) { + validateConfigKey(key); + return this._config.get(key) ?? null; + } + + /** + * @param {string} key + * @param {string} value + * @returns {Promise} + */ + async configSet(key, value) { + validateConfigKey(key); + if (typeof value !== 'string') { + throw new Error('Config value must be a string'); + } + this._config.set(key, value); + } + + // ── Private helpers ───────────────────────────────────────────────── + + /** + * @param {string} treeOid + * @param {string[]} parents + * @param {string} message + * @returns {string} + */ + _createCommit(treeOid, parents, message) { + const date = new Date(this._clock.now()).toISOString(); + const sha = hashCommit({ + treeOid, + parents, + message, + author: this._author, + date, + }); + this._commits.set(sha, { + treeOid, + parents: [...parents], + message, + author: this._author, + date, + }); + return sha; + } + + /** + * Resolves a ref name to a SHA. If the ref looks like a raw SHA, returns it. + * @param {string} ref + * @returns {string|null} + */ + _resolveRef(ref) { + if (this._refs.has(ref)) { + return /** @type {string} */ (this._refs.get(ref)); + } + if (this._commits.has(ref)) { + return ref; + } + return null; + } + + /** + * Walks commit history from a ref, newest first, up to limit. + * @param {string} ref + * @param {number} limit + * @returns {Array<{sha: string, message: string, author: string, date: string, parents: string[]}>} + */ + _walkLog(ref, limit) { + const tip = this._resolveRef(ref); + if (!tip) { + return []; + } + /** @type {Array<{sha: string, message: string, author: string, date: string, parents: string[]}>} */ + const result = []; + const visited = new Set(); + // BFS with a queue (oldest commits first within each level, but tip is first overall) + const queue = [tip]; + while (queue.length > 0 && result.length < limit) { + const sha = /** @type {string} */ (queue.shift()); + if (visited.has(sha)) { + continue; + } + visited.add(sha); + const commit = this._commits.get(sha); + if (!commit) { + continue; + } + result.push({ sha, ...commit }); + for (const p of commit.parents) { + if (!visited.has(p)) { + queue.push(p); + } + } + } + return result; + } + + /** + * Formats a commit record in GitLogParser's expected format: + * `\n\n\n\n` + * @param {{sha: string, message: string, author: string, date: string, parents: string[]}} c + * @returns {string} + */ + _formatCommitRecord(c) { + return `${c.sha}\n${c.author}\n${c.date}\n${c.parents.join(' ')}\n${c.message}`; + } +} diff --git a/src/infrastructure/adapters/adapterValidation.js b/src/infrastructure/adapters/adapterValidation.js new file mode 100644 index 0000000..1925e08 --- /dev/null +++ b/src/infrastructure/adapters/adapterValidation.js @@ -0,0 +1,90 @@ +/** + * Shared input validation for persistence adapters. + * + * These functions are extracted from GitGraphAdapter so that both Git-backed + * and in-memory adapters apply identical validation rules. This prevents + * divergence and ensures conformance tests exercise the same constraints. + * + * @module infrastructure/adapters/adapterValidation + */ + +/** + * Validates that an OID is a safe hex string (4–64 characters). + * @param {string} oid - The OID to validate + * @throws {Error} If OID is invalid + */ +export function validateOid(oid) { + if (!oid || typeof oid !== 'string') { + throw new Error('OID must be a non-empty string'); + } + if (oid.length > 64) { + throw new Error(`OID too long: ${oid.length} chars. Maximum is 64`); + } + const validOidPattern = /^[0-9a-fA-F]{4,64}$/; + if (!validOidPattern.test(oid)) { + throw new Error(`Invalid OID format: ${oid}`); + } +} + +/** + * Validates that a ref is safe to use in git commands. + * Prevents command injection via malicious ref names. + * @param {string} ref - The ref to validate + * @throws {Error} If ref contains invalid characters, is too long, or starts with -/-- + */ +export function validateRef(ref) { + if (!ref || typeof ref !== 'string') { + throw new Error('Ref must be a non-empty string'); + } + if (ref.length > 1024) { + throw new Error(`Ref too long: ${ref.length} chars. Maximum is 1024`); + } + if (ref.startsWith('-') || ref.startsWith('--')) { + throw new Error(`Invalid ref: ${ref}. Refs cannot start with - or --. See https://github.com/git-stunts/git-warp#security`); + } + const validRefPattern = /^[a-zA-Z0-9._/-]+((~\d*|\^\d*|\.\.[a-zA-Z0-9._/-]+)*)$/; + if (!validRefPattern.test(ref)) { + throw new Error(`Invalid ref format: ${ref}. Only alphanumeric characters, ., /, -, _, ^, ~, and range operators are allowed. See https://github.com/git-stunts/git-warp#ref-validation`); + } +} + +/** + * Validates that a limit is a safe positive integer (max 10M). + * @param {number} limit - The limit to validate + * @throws {Error} If limit is invalid + */ +export function validateLimit(limit) { + if (typeof limit !== 'number' || !Number.isFinite(limit)) { + throw new Error('Limit must be a finite number'); + } + if (!Number.isInteger(limit)) { + throw new Error('Limit must be an integer'); + } + if (limit <= 0) { + throw new Error('Limit must be a positive integer'); + } + if (limit > 10_000_000) { + throw new Error(`Limit too large: ${limit}. Maximum is 10,000,000`); + } +} + +/** + * Validates that a config key is safe and well-formed. + * @param {string} key - The config key to validate + * @throws {Error} If key is invalid + */ +export function validateConfigKey(key) { + if (!key || typeof key !== 'string') { + throw new Error('Config key must be a non-empty string'); + } + if (key.length > 256) { + throw new Error(`Config key too long: ${key.length} chars. Maximum is 256`); + } + if (key.startsWith('-')) { + throw new Error(`Invalid config key: ${key}. Keys cannot start with -`); + } + const validKeyPattern = /^[a-zA-Z][a-zA-Z0-9._-]*$/; + if (!validKeyPattern.test(key)) { + throw new Error(`Invalid config key format: ${key}`); + } +} diff --git a/test/helpers/warpGraphTestUtils.js b/test/helpers/warpGraphTestUtils.js index 33b1873..a5f0893 100644 --- a/test/helpers/warpGraphTestUtils.js +++ b/test/helpers/warpGraphTestUtils.js @@ -12,6 +12,7 @@ import { tmpdir } from 'os'; // @ts-expect-error - no declaration file for @git-stunts/plumbing import Plumbing from '@git-stunts/plumbing'; import GitGraphAdapter from '../../src/infrastructure/adapters/GitGraphAdapter.js'; +import InMemoryGraphAdapter from '../../src/infrastructure/adapters/InMemoryGraphAdapter.js'; import { encode } from '../../src/infrastructure/codecs/CborCodec.js'; import { encodePatchMessage } from '../../src/domain/services/WarpMessageCodec.js'; import { createEmptyStateV5, encodeEdgeKey } from '../../src/domain/services/JoinReducer.js'; @@ -612,6 +613,25 @@ export async function createGitRepo(label = 'test') { } } +/** + * Creates an in-memory persistence adapter (no Git subprocess, no filesystem). + * Drop-in replacement for createGitRepo() in tests that don't need real Git. + * + * @returns {{ persistence: InMemoryGraphAdapter, cleanup: () => Promise }} + * + * @example + * let repo; + * beforeEach(() => { repo = createInMemoryRepo(); }); + * // no cleanup needed, but cleanup() is provided for API compatibility + */ +export function createInMemoryRepo() { + const persistence = new InMemoryGraphAdapter(); + return { + persistence, + async cleanup() { /* no-op */ }, + }; +} + // ============================================================================ // State Seeding Helpers // ============================================================================ diff --git a/test/unit/infrastructure/adapters/AdapterConformance.js b/test/unit/infrastructure/adapters/AdapterConformance.js new file mode 100644 index 0000000..1e37c86 --- /dev/null +++ b/test/unit/infrastructure/adapters/AdapterConformance.js @@ -0,0 +1,265 @@ +/** + * Shared conformance suite for GraphPersistencePort adapters. + * + * Call `describeAdapterConformance(name, createAdapter, cleanupAdapter)` to + * wire these tests against any adapter that implements GraphPersistencePort. + * + * @module test/unit/infrastructure/adapters/AdapterConformance + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import GitLogParser from '../../../../src/domain/services/GitLogParser.js'; + +/** + * @param {string} name - Adapter display name for describe blocks + * @param {() => Promise<{adapter: any, cleanup?: () => Promise}>} factory + */ +export function describeAdapterConformance(name, factory) { + describe(`${name} conformance`, () => { + /** @type {any} */ + let adapter; + /** @type {(() => Promise)|undefined} */ + let cleanup; + + beforeEach(async () => { + const result = await factory(); + adapter = result.adapter; + cleanup = result.cleanup; + }); + + afterEach(async () => { + if (cleanup) { + await cleanup(); + } + }); + + // ── CommitPort ────────────────────────────────────────────────── + + describe('CommitPort', () => { + it('commitNode returns a hex SHA', async () => { + const sha = await adapter.commitNode({ message: 'test commit' }); + expect(sha).toMatch(/^[0-9a-f]{40}$/); + }); + + it('commitNode supports parent chains', async () => { + const parent = await adapter.commitNode({ message: 'parent' }); + const child = await adapter.commitNode({ message: 'child', parents: [parent] }); + expect(child).toMatch(/^[0-9a-f]{40}$/); + expect(child).not.toBe(parent); + }); + + it('showNode round-trips the message', async () => { + const message = 'hello world\nwith newlines'; + const sha = await adapter.commitNode({ message }); + const retrieved = await adapter.showNode(sha); + // Git adds a trailing newline; trim for comparison + expect(retrieved.trim()).toBe(message); + }); + + it('showNode throws on missing commit', async () => { + const fakeSha = 'dead' + '0'.repeat(36); + await expect(adapter.showNode(fakeSha)).rejects.toThrow(); + }); + + it('getNodeInfo returns metadata', async () => { + const parent = await adapter.commitNode({ message: 'parent msg' }); + const sha = await adapter.commitNode({ message: 'child msg', parents: [parent] }); + const info = await adapter.getNodeInfo(sha); + expect(info.sha).toBe(sha); + expect(info.message.trim()).toContain('child msg'); + expect(info.author).toBeTruthy(); + expect(info.date).toBeTruthy(); + expect(info.parents).toContain(parent); + }); + + it('nodeExists returns true for existing commit', async () => { + const sha = await adapter.commitNode({ message: 'exists' }); + expect(await adapter.nodeExists(sha)).toBe(true); + }); + + it('nodeExists returns false for missing commit', async () => { + const fakeSha = 'beef' + '0'.repeat(36); + expect(await adapter.nodeExists(fakeSha)).toBe(false); + }); + + it('countNodes counts the full chain', async () => { + const a = await adapter.commitNode({ message: 'a' }); + const b = await adapter.commitNode({ message: 'b', parents: [a] }); + const c = await adapter.commitNode({ message: 'c', parents: [b] }); + await adapter.updateRef('refs/warp/test/writers/w1', c); + const count = await adapter.countNodes('refs/warp/test/writers/w1'); + expect(count).toBe(3); + }); + + it('ping returns ok', async () => { + const result = await adapter.ping(); + expect(result.ok).toBe(true); + expect(typeof result.latencyMs).toBe('number'); + }); + + it('commitNodeWithTree creates a commit with a custom tree', async () => { + const blobOid = await adapter.writeBlob('content'); + const treeOid = await adapter.writeTree([`100644 blob ${blobOid}\tfile.txt`]); + const sha = await adapter.commitNodeWithTree({ treeOid, message: 'tree commit' }); + expect(sha).toMatch(/^[0-9a-f]{40}$/); + const msg = await adapter.showNode(sha); + expect(msg.trim()).toContain('tree commit'); + }); + + it('logNodesStream is parseable by GitLogParser', async () => { + const a = await adapter.commitNode({ message: 'first commit' }); + const b = await adapter.commitNode({ message: 'second commit', parents: [a] }); + await adapter.updateRef('refs/warp/test/writers/log', b); + + const format = '%H%n%an <%ae>%n%aI%n%P%n%B'; + const stream = await adapter.logNodesStream({ + ref: 'refs/warp/test/writers/log', + limit: 10, + format, + }); + + const parser = new GitLogParser(); + const nodes = []; + for await (const node of parser.parse(stream)) { + nodes.push(node); + } + expect(nodes.length).toBe(2); + expect(nodes[0].sha).toBe(b); + expect(nodes[1].sha).toBe(a); + }); + }); + + // ── BlobPort ──────────────────────────────────────────────────── + + describe('BlobPort', () => { + it('writeBlob + readBlob round-trip', async () => { + const content = Buffer.from('hello blob'); + const oid = await adapter.writeBlob(content); + expect(oid).toMatch(/^[0-9a-f]{40}$/); + const retrieved = await adapter.readBlob(oid); + expect(Buffer.compare(retrieved, content)).toBe(0); + }); + + it('content-addressing: same content = same OID', async () => { + const oid1 = await adapter.writeBlob('same'); + const oid2 = await adapter.writeBlob('same'); + expect(oid1).toBe(oid2); + }); + + // readBlob behavior on missing OID varies by adapter + // (Git may return empty buffer vs throw). Tested per-adapter. + }); + + // ── TreePort ──────────────────────────────────────────────────── + + describe('TreePort', () => { + it('emptyTree is the well-known constant', () => { + expect(adapter.emptyTree).toBe('4b825dc642cb6eb9a060e54bf8d69288fbee4904'); + }); + + it('writeTree + readTreeOids round-trip', async () => { + const oid = await adapter.writeBlob('tree content'); + const treeOid = await adapter.writeTree([`100644 blob ${oid}\tindex.json`]); + expect(treeOid).toMatch(/^[0-9a-f]{40}$/); + const oids = await adapter.readTreeOids(treeOid); + expect(oids['index.json']).toBe(oid); + }); + + it('writeTree + readTree resolves blob content', async () => { + const content = Buffer.from('resolved content'); + const blobOid = await adapter.writeBlob(content); + const treeOid = await adapter.writeTree([`100644 blob ${blobOid}\tdata.bin`]); + const files = await adapter.readTree(treeOid); + expect(Buffer.compare(files['data.bin'], content)).toBe(0); + }); + }); + + // ── RefPort ───────────────────────────────────────────────────── + + describe('RefPort', () => { + it('updateRef + readRef round-trip', async () => { + const sha = await adapter.commitNode({ message: 'ref test' }); + await adapter.updateRef('refs/warp/test/writers/alice', sha); + const result = await adapter.readRef('refs/warp/test/writers/alice'); + expect(result).toBe(sha); + }); + + it('readRef returns null for missing ref', async () => { + const result = await adapter.readRef('refs/warp/doesnotexist'); + expect(result).toBeNull(); + }); + + it('deleteRef removes the ref', async () => { + const sha = await adapter.commitNode({ message: 'delete me' }); + await adapter.updateRef('refs/warp/test/writers/del', sha); + await adapter.deleteRef('refs/warp/test/writers/del'); + const result = await adapter.readRef('refs/warp/test/writers/del'); + expect(result).toBeNull(); + }); + + it('listRefs filters by prefix', async () => { + const sha = await adapter.commitNode({ message: 'list' }); + await adapter.updateRef('refs/warp/g1/writers/alice', sha); + await adapter.updateRef('refs/warp/g1/writers/bob', sha); + await adapter.updateRef('refs/warp/g2/writers/carol', sha); + + const g1Refs = await adapter.listRefs('refs/warp/g1/writers/'); + expect(g1Refs).toContain('refs/warp/g1/writers/alice'); + expect(g1Refs).toContain('refs/warp/g1/writers/bob'); + expect(g1Refs).not.toContain('refs/warp/g2/writers/carol'); + }); + }); + + // ── ConfigPort ────────────────────────────────────────────────── + + describe('ConfigPort', () => { + it('configSet + configGet round-trip', async () => { + await adapter.configSet('warp.test.key', 'value123'); + const value = await adapter.configGet('warp.test.key'); + expect(value).toBe('value123'); + }); + + it('configGet returns null for unset key', async () => { + const value = await adapter.configGet('warp.unset.key'); + expect(value).toBeNull(); + }); + }); + + // ── Validation ────────────────────────────────────────────────── + + describe('Validation', () => { + it('rejects bad OID', async () => { + await expect(adapter.showNode('not-hex!')).rejects.toThrow(/Invalid OID/); + }); + + it('rejects empty OID', async () => { + await expect(adapter.showNode('')).rejects.toThrow(/non-empty string/); + }); + + it('rejects bad ref', async () => { + await expect(adapter.readRef('--malicious')).rejects.toThrow(/Invalid ref/); + }); + + it('rejects empty ref', async () => { + await expect(adapter.readRef('')).rejects.toThrow(/non-empty string/); + }); + + it('rejects bad limit', async () => { + await expect(adapter.logNodes({ ref: 'HEAD', limit: -1 })) + .rejects.toThrow(/positive integer/); + }); + + it('rejects non-integer limit', async () => { + await expect(adapter.logNodes({ ref: 'HEAD', limit: 1.5 })) + .rejects.toThrow(/integer/); + }); + + it('rejects bad config key', async () => { + await expect(adapter.configGet('123invalid')).rejects.toThrow(/Invalid config key/); + }); + + it('rejects empty config key', async () => { + await expect(adapter.configGet('')).rejects.toThrow(/non-empty string/); + }); + }); + }); +} diff --git a/test/unit/infrastructure/adapters/GitGraphAdapter.coverage.test.js b/test/unit/infrastructure/adapters/GitGraphAdapter.coverage.test.js index 5762114..9cff95b 100644 --- a/test/unit/infrastructure/adapters/GitGraphAdapter.coverage.test.js +++ b/test/unit/infrastructure/adapters/GitGraphAdapter.coverage.test.js @@ -1,5 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import GitGraphAdapter from '../../../../src/infrastructure/adapters/GitGraphAdapter.js'; +import { createGitRepo } from '../../../helpers/warpGraphTestUtils.js'; +import { describeAdapterConformance } from './AdapterConformance.js'; /** @type {any} */ let mockPlumbing; @@ -398,3 +400,10 @@ describe('GitGraphAdapter coverage', () => { }); }); }); + +// ── Conformance suite against a real Git repo ───────────────────────────── + +describeAdapterConformance('GitGraphAdapter', async () => { + const repo = await createGitRepo('conformance'); + return { adapter: repo.persistence, cleanup: repo.cleanup }; +}); diff --git a/test/unit/infrastructure/adapters/InMemoryGraphAdapter.integration.test.js b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.integration.test.js new file mode 100644 index 0000000..6c9c97d --- /dev/null +++ b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.integration.test.js @@ -0,0 +1,52 @@ +import { describe, it, expect } from 'vitest'; +import InMemoryGraphAdapter from '../../../../src/infrastructure/adapters/InMemoryGraphAdapter.js'; +import WarpGraph from '../../../../src/domain/WarpGraph.js'; + +describe('InMemoryGraphAdapter integration smoke test', () => { + it('WarpGraph can write a patch and materialize with InMemoryAdapter', async () => { + const persistence = new InMemoryGraphAdapter(); + const graph = await WarpGraph.open({ + persistence, + graphName: 'test-graph', + writerId: 'alice', + }); + + const patch = await graph.createPatch(); + patch.addNode('user:alice'); + patch.setProperty('user:alice', 'name', 'Alice'); + await patch.commit(); + + const state = await graph.materialize(); + const nodes = state.nodeAlive; + expect(nodes.entries.has('user:alice')).toBe(true); + }); + + it('multi-writer convergence works with InMemoryAdapter', async () => { + const persistence = new InMemoryGraphAdapter(); + + const graphA = await WarpGraph.open({ + persistence, + graphName: 'multi', + writerId: 'alice', + }); + + const graphB = await WarpGraph.open({ + persistence, + graphName: 'multi', + writerId: 'bob', + }); + + const patchA = await graphA.createPatch(); + patchA.addNode('node:a'); + await patchA.commit(); + + const patchB = await graphB.createPatch(); + patchB.addNode('node:b'); + await patchB.commit(); + + // Both writers' patches should be visible after materialization + const state = await graphA.materialize(); + expect(state.nodeAlive.entries.has('node:a')).toBe(true); + expect(state.nodeAlive.entries.has('node:b')).toBe(true); + }); +}); diff --git a/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js new file mode 100644 index 0000000..6190cea --- /dev/null +++ b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js @@ -0,0 +1,94 @@ +import { describe, it, expect } from 'vitest'; +import InMemoryGraphAdapter from '../../../../src/infrastructure/adapters/InMemoryGraphAdapter.js'; +import { describeAdapterConformance } from './AdapterConformance.js'; + +// ── Conformance suite ─────────────────────────────────────────────────── + +describeAdapterConformance('InMemoryGraphAdapter', async () => ({ + adapter: new InMemoryGraphAdapter(), +})); + +// ── InMemory-specific tests ───────────────────────────────────────────── + +describe('InMemoryGraphAdapter specifics', () => { + it('uses injected clock for commit dates', async () => { + let t = 1000; + const clock = { now: () => t++ }; + const adapter = new InMemoryGraphAdapter({ clock }); + const sha = await adapter.commitNode({ message: 'timed' }); + const info = await adapter.getNodeInfo(sha); + expect(info.date).toBe(new Date(1000).toISOString()); + }); + + it('uses injected author string', async () => { + const adapter = new InMemoryGraphAdapter({ author: 'Alice ' }); + const sha = await adapter.commitNode({ message: 'authored' }); + const info = await adapter.getNodeInfo(sha); + expect(info.author).toBe('Alice '); + }); + + it('content-addressable blobs return same OID', async () => { + const adapter = new InMemoryGraphAdapter(); + const oid1 = await adapter.writeBlob('duplicate'); + const oid2 = await adapter.writeBlob('duplicate'); + expect(oid1).toBe(oid2); + }); + + it('different messages produce different commit SHAs', async () => { + const clock = { now: () => 42 }; // fixed clock for determinism + const adapter = new InMemoryGraphAdapter({ clock }); + const sha1 = await adapter.commitNode({ message: 'msg-a' }); + const sha2 = await adapter.commitNode({ message: 'msg-b' }); + expect(sha1).not.toBe(sha2); + }); + + it('ping always returns ok:true with latencyMs 0', async () => { + const adapter = new InMemoryGraphAdapter(); + const result = await adapter.ping(); + expect(result).toEqual({ ok: true, latencyMs: 0 }); + }); + + it('readTreeOids on empty tree returns empty object', async () => { + const adapter = new InMemoryGraphAdapter(); + const result = await adapter.readTreeOids(adapter.emptyTree); + expect(result).toEqual({}); + }); + + it('readBlob throws for missing OID', async () => { + const adapter = new InMemoryGraphAdapter(); + await expect(adapter.readBlob('abcd' + '0'.repeat(36))) + .rejects.toThrow(/Blob not found/); + }); + + it('readTreeOids throws for missing tree', async () => { + const adapter = new InMemoryGraphAdapter(); + await expect(adapter.readTreeOids('abcd' + '0'.repeat(36))) + .rejects.toThrow(/Tree not found/); + }); + + it('countNodes throws for missing ref', async () => { + const adapter = new InMemoryGraphAdapter(); + await expect(adapter.countNodes('refs/warp/missing')) + .rejects.toThrow(/Ref not found/); + }); + + it('configSet rejects non-string value', async () => { + const adapter = new InMemoryGraphAdapter(); + await expect(adapter.configSet('warp.key', /** @type {any} */ (42))) + .rejects.toThrow(/Config value must be a string/); + }); + + it('listRefs returns sorted results', async () => { + const adapter = new InMemoryGraphAdapter(); + const sha = await adapter.commitNode({ message: 'sort' }); + await adapter.updateRef('refs/warp/z/w1', sha); + await adapter.updateRef('refs/warp/a/w1', sha); + await adapter.updateRef('refs/warp/m/w1', sha); + const refs = await adapter.listRefs('refs/warp/'); + expect(refs).toEqual([ + 'refs/warp/a/w1', + 'refs/warp/m/w1', + 'refs/warp/z/w1', + ]); + }); +}); From d522bdd52fa6d4bca3a2174e6d6188265cff62a1 Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 09:03:52 -0800 Subject: [PATCH 2/4] fix: resolve typecheck errors in InMemoryGraphAdapter integration test --- .../adapters/InMemoryGraphAdapter.integration.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unit/infrastructure/adapters/InMemoryGraphAdapter.integration.test.js b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.integration.test.js index 6c9c97d..6b53971 100644 --- a/test/unit/infrastructure/adapters/InMemoryGraphAdapter.integration.test.js +++ b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.integration.test.js @@ -16,9 +16,9 @@ describe('InMemoryGraphAdapter integration smoke test', () => { patch.setProperty('user:alice', 'name', 'Alice'); await patch.commit(); + /** @type {any} */ const state = await graph.materialize(); - const nodes = state.nodeAlive; - expect(nodes.entries.has('user:alice')).toBe(true); + expect(state.nodeAlive.entries.has('user:alice')).toBe(true); }); it('multi-writer convergence works with InMemoryAdapter', async () => { @@ -45,6 +45,7 @@ describe('InMemoryGraphAdapter integration smoke test', () => { await patchB.commit(); // Both writers' patches should be visible after materialization + /** @type {any} */ const state = await graphA.materialize(); expect(state.nodeAlive.entries.has('node:a')).toBe(true); expect(state.nodeAlive.entries.has('node:b')).toBe(true); From ea27cd618d531ff945acf92c77638f439480e3f4 Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 09:54:10 -0800 Subject: [PATCH 3/4] =?UTF-8?q?fix:=204=20code=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20redundant=20check,=20writeTree=20guard,=20walkLog?= =?UTF-8?q?=20perf,=20format=20naming=20(v10.7.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 6 ++++++ .../adapters/InMemoryGraphAdapter.js | 15 ++++++++++----- src/infrastructure/adapters/adapterValidation.js | 2 +- .../adapters/InMemoryGraphAdapter.test.js | 6 ++++++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd885b6..1d84795 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,12 @@ Adds `InMemoryGraphAdapter`, a zero-I/O implementation of `GraphPersistencePort` - **Adapter conformance suite** (`test/unit/infrastructure/adapters/AdapterConformance.js`): ~25 shared behavioral tests that run against any `GraphPersistencePort` implementation, ensuring parity between Git and in-memory adapters. - **`createInMemoryRepo()`** (`test/helpers/warpGraphTestUtils.js`): Test factory for instant in-memory adapter setup — no temp dirs, no git subprocesses. +### Fixed + +- **`InMemoryGraphAdapter.writeTree()`**: Rejects malformed mktree entries missing a tab separator instead of silently producing garbage. +- **`InMemoryGraphAdapter._walkLog()`**: Replaced O(n) `queue.shift()` with index-pointer for O(1) dequeue. +- **`adapterValidation.validateRef()`**: Removed redundant `startsWith('--')` check (already covered by `startsWith('-')`). + ### Changed - **`GitGraphAdapter`**: Validation methods now delegate to shared `adapterValidation.js` functions. No behavioral change. diff --git a/src/infrastructure/adapters/InMemoryGraphAdapter.js b/src/infrastructure/adapters/InMemoryGraphAdapter.js index b5f7fa6..551b3f9 100644 --- a/src/infrastructure/adapters/InMemoryGraphAdapter.js +++ b/src/infrastructure/adapters/InMemoryGraphAdapter.js @@ -122,6 +122,9 @@ export default class InMemoryGraphAdapter extends GraphPersistencePort { async writeTree(entries) { const parsed = entries.map(line => { const tabIdx = line.indexOf('\t'); + if (tabIdx === -1) { + throw new Error(`Invalid mktree entry (missing tab): ${line}`); + } const meta = line.slice(0, tabIdx); const path = line.slice(tabIdx + 1); const [mode, , oid] = meta.split(' '); @@ -291,12 +294,13 @@ export default class InMemoryGraphAdapter extends GraphPersistencePort { * @param {{ ref: string, limit?: number, format?: string }} options * @returns {Promise} */ - async logNodes({ ref, limit = 50, format }) { + async logNodes({ ref, limit = 50, format: _format }) { validateRef(ref); validateLimit(limit); const records = this._walkLog(ref, limit); - // If no format given, return a simple repr. With format, return NUL-terminated records. - if (!format) { + // Format param is accepted for port compatibility but always uses + // the GitLogParser-compatible layout (SHA\nauthor\ndate\nparents\nmessage). + if (!_format) { return records.map(c => `commit ${c.sha}\nAuthor: ${c.author}\nDate: ${c.date}\n\n ${c.message}\n`).join('\n'); } return records.map(c => this._formatCommitRecord(c)).join('\0') + (records.length > 0 ? '\0' : ''); @@ -449,8 +453,9 @@ export default class InMemoryGraphAdapter extends GraphPersistencePort { const visited = new Set(); // BFS with a queue (oldest commits first within each level, but tip is first overall) const queue = [tip]; - while (queue.length > 0 && result.length < limit) { - const sha = /** @type {string} */ (queue.shift()); + let head = 0; + while (head < queue.length && result.length < limit) { + const sha = /** @type {string} */ (queue[head++]); if (visited.has(sha)) { continue; } diff --git a/src/infrastructure/adapters/adapterValidation.js b/src/infrastructure/adapters/adapterValidation.js index 1925e08..a4b4f71 100644 --- a/src/infrastructure/adapters/adapterValidation.js +++ b/src/infrastructure/adapters/adapterValidation.js @@ -39,7 +39,7 @@ export function validateRef(ref) { if (ref.length > 1024) { throw new Error(`Ref too long: ${ref.length} chars. Maximum is 1024`); } - if (ref.startsWith('-') || ref.startsWith('--')) { + if (ref.startsWith('-')) { throw new Error(`Invalid ref: ${ref}. Refs cannot start with - or --. See https://github.com/git-stunts/git-warp#security`); } const validRefPattern = /^[a-zA-Z0-9._/-]+((~\d*|\^\d*|\.\.[a-zA-Z0-9._/-]+)*)$/; diff --git a/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js index 6190cea..69b7c25 100644 --- a/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js +++ b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js @@ -78,6 +78,12 @@ describe('InMemoryGraphAdapter specifics', () => { .rejects.toThrow(/Config value must be a string/); }); + it('writeTree rejects malformed entry missing tab', async () => { + const adapter = new InMemoryGraphAdapter(); + await expect(adapter.writeTree(['100644 blob abcd0000000000000000000000000000000000 no-tab'])) + .rejects.toThrow(/missing tab/i); + }); + it('listRefs returns sorted results', async () => { const adapter = new InMemoryGraphAdapter(); const sha = await adapter.commitNode({ message: 'sort' }); From c9fc0646874aaec842220140f8992aabf92c571b Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 11 Feb 2026 10:21:12 -0800 Subject: [PATCH 4/4] fix: _walkLog reverse chronological sort for merge DAGs (v10.7.0) --- CHANGELOG.md | 2 +- .../adapters/InMemoryGraphAdapter.js | 14 +++++---- .../adapters/InMemoryGraphAdapter.test.js | 30 +++++++++++++++++++ 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d84795..6d93bc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Adds `InMemoryGraphAdapter`, a zero-I/O implementation of `GraphPersistencePort` ### Fixed - **`InMemoryGraphAdapter.writeTree()`**: Rejects malformed mktree entries missing a tab separator instead of silently producing garbage. -- **`InMemoryGraphAdapter._walkLog()`**: Replaced O(n) `queue.shift()` with index-pointer for O(1) dequeue. +- **`InMemoryGraphAdapter._walkLog()`**: Replaced O(n) `queue.shift()` with index-pointer for O(1) dequeue; sort by date descending to match Git's reverse chronological ordering for merge DAGs. - **`adapterValidation.validateRef()`**: Removed redundant `startsWith('--')` check (already covered by `startsWith('-')`). ### Changed diff --git a/src/infrastructure/adapters/InMemoryGraphAdapter.js b/src/infrastructure/adapters/InMemoryGraphAdapter.js index 551b3f9..798b239 100644 --- a/src/infrastructure/adapters/InMemoryGraphAdapter.js +++ b/src/infrastructure/adapters/InMemoryGraphAdapter.js @@ -438,7 +438,8 @@ export default class InMemoryGraphAdapter extends GraphPersistencePort { } /** - * Walks commit history from a ref, newest first, up to limit. + * Walks commit history from a ref, reverse chronological (newest first), + * up to limit. Matches `git log` default ordering for merge DAGs. * @param {string} ref * @param {number} limit * @returns {Array<{sha: string, message: string, author: string, date: string, parents: string[]}>} @@ -449,12 +450,11 @@ export default class InMemoryGraphAdapter extends GraphPersistencePort { return []; } /** @type {Array<{sha: string, message: string, author: string, date: string, parents: string[]}>} */ - const result = []; + const all = []; const visited = new Set(); - // BFS with a queue (oldest commits first within each level, but tip is first overall) const queue = [tip]; let head = 0; - while (head < queue.length && result.length < limit) { + while (head < queue.length) { const sha = /** @type {string} */ (queue[head++]); if (visited.has(sha)) { continue; @@ -464,14 +464,16 @@ export default class InMemoryGraphAdapter extends GraphPersistencePort { if (!commit) { continue; } - result.push({ sha, ...commit }); + all.push({ sha, ...commit }); for (const p of commit.parents) { if (!visited.has(p)) { queue.push(p); } } } - return result; + // Sort by date descending (reverse chronological), matching git log + all.sort((a, b) => (a.date < b.date ? 1 : a.date > b.date ? -1 : 0)); + return all.slice(0, limit); } /** diff --git a/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js index 69b7c25..243951f 100644 --- a/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js +++ b/test/unit/infrastructure/adapters/InMemoryGraphAdapter.test.js @@ -84,6 +84,36 @@ describe('InMemoryGraphAdapter specifics', () => { .rejects.toThrow(/missing tab/i); }); + it('_walkLog returns reverse chronological order for merge DAGs', async () => { + let t = 1000; + const clock = { now: () => t++ }; + const adapter = new InMemoryGraphAdapter({ clock }); + + // Build a diamond DAG: A -> B, A -> C, B+C -> D (merge) + const a = await adapter.commitNode({ message: 'a' }); // t=1000 + const b = await adapter.commitNode({ message: 'b', parents: [a] }); // t=1001 + const c = await adapter.commitNode({ message: 'c', parents: [a] }); // t=1002 + const d = await adapter.commitNode({ message: 'd', parents: [b, c] }); // t=1003 (merge) + await adapter.updateRef('refs/warp/test/writers/merge', d); + + const format = '%H%n%an <%ae>%n%aI%n%P%n%B'; + const stream = await adapter.logNodesStream({ + ref: 'refs/warp/test/writers/merge', + limit: 10, + format, + }); + + const chunks = []; + for await (const chunk of stream) { + chunks.push(chunk); + } + const records = chunks.join('').split('\0').filter(Boolean); + + // Should be reverse chronological: D (newest) first, A (oldest) last + const shas = records.map(r => r.split('\n')[0]); + expect(shas).toEqual([d, c, b, a]); + }); + it('listRefs returns sorted results', async () => { const adapter = new InMemoryGraphAdapter(); const sha = await adapter.commitNode({ message: 'sort' });