From ce33ed436a140b01640543e19eba5cf57f94c8fe Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:47:33 -0400 Subject: [PATCH 1/8] fix(get_neighbors): omitted depth now honors the schema default (0 = immediate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug_DepthDefaultMismatch_2026_08_23: three depth numberings coexist — C hop-count (1 = immediate), MCP schema 0-indexed (default 0), op_bench d0. The TS boundary bridges with depth+1, but the handler defaulted omitted depth to 1 (and getNeighbors' signature to 1), so schema-default calls silently returned TWO-hop neighborhoods. Canonical semantics: the 0-indexed schema numbering. C hop-count is internal; the single +1 translation lives at the TS boundary. New test pins the OMITTED-depth path specifically — explicit-depth tests cannot catch a default divergence. Behavioral change: default get_neighbors walks shrink from 2-hop to the schema-promised 1-hop set. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017GGvxNSpbpc5TVyMYYYzBr --- server.ts | 4 ++-- tests/memory-server.test.ts | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/server.ts b/server.ts index 0603987..963cde2 100644 --- a/server.ts +++ b/server.ts @@ -822,7 +822,7 @@ export class KnowledgeGraphManager { async getNeighbors( entityName: string, - depth: number = 1, + depth: number = 0, sortBy?: EntitySortField, sortDir?: SortDirection, direction: 'forward' | 'backward' | 'any' = 'forward' @@ -1690,7 +1690,7 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p return { content: [{ type: "text", text: JSON.stringify(paginateGraph(graph, args.entityCursor as number ?? 0, args.relationCursor as number ?? 0)) }] }; } case "get_neighbors": { - const neighbors = await knowledgeGraphManager.getNeighbors(args.entityName as string, args.depth as number ?? 1, args.sortBy as EntitySortField | undefined, args.sortDir as SortDirection | undefined, (args.direction as 'forward' | 'backward' | 'any') ?? 'forward'); + const neighbors = await knowledgeGraphManager.getNeighbors(args.entityName as string, args.depth as number ?? 0, args.sortBy as EntitySortField | undefined, args.sortDir as SortDirection | undefined, (args.direction as 'forward' | 'backward' | 'any') ?? 'forward'); // Record walker visits for returned neighbors knowledgeGraphManager.recordWalkerVisits(neighbors.map(n => n.name)); return { content: [{ type: "text", text: JSON.stringify(paginateItems(neighbors, args.cursor as number ?? 0)) }] }; diff --git a/tests/memory-server.test.ts b/tests/memory-server.test.ts index 52a2e76..00e9259 100644 --- a/tests/memory-server.test.ts +++ b/tests/memory-server.test.ts @@ -613,6 +613,22 @@ describe('MCP Memory Server E2E Tests', () => { expect(names).toContain('Child2'); }); + it('omitted depth must equal the schema default (0 = immediate only)', async () => { + // Bug_DepthDefaultMismatch_2026_08_23: the handler defaulted to 1 while + // the tool schema advertises 0, so omitted-depth calls silently returned + // TWO-hop neighborhoods. This pins the default path itself — the + // explicit-depth tests above cannot catch a default divergence. + const result = await callTool(client, 'get_neighbors', { + entityName: 'Root' + }) as PaginatedResult; + + expect(result.items).toHaveLength(2); + const names = result.items.map(n => n.name); + expect(names).toContain('Child1'); + expect(names).toContain('Child2'); + expect(names).not.toContain('Grandchild'); + }); + it('should get neighbors at depth 1 (includes neighbors of neighbors)', async () => { const result = await callTool(client, 'get_neighbors', { entityName: 'Root', From a8dd29e4b553066a8913af41b01fadb9ecc85d8f Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:25:09 -0400 Subject: [PATCH 2/8] feat(random_walk): avoidCycles - self-avoiding walks (no node revisits) - C graph_random_walk: optional visited filter; the path prefix doubles as the visited set, stops early when every neighbor is already on the path - plumbed end-to-end: binding argv[6] -> Store -> manager -> dispatch; schema, README, and kb.walker.avoid_cycles span attribute added - tests: forced chain walk (LoopA->B->C->D), 2-cycle early stop (Ping<->Pong), default-walk revisit proof, seeded reproducibility - full jest suite 174/174 incl. fuzz (13.7k random_walk calls, clean) Co-Authored-By: Claude Code --- README.md | 1 + native/graph.c | 13 ++++++- native/graph.h | 6 ++-- native/graphbind.c | 4 +-- server.ts | 10 +++++- src/store.ts | 6 ++-- tests/memory-server.test.ts | 71 +++++++++++++++++++++++++++++++++++++ 7 files changed, 102 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 8a39b7a..a81828c 100644 --- a/README.md +++ b/README.md @@ -241,6 +241,7 @@ The eigenvector is computed via sparse power iteration with teleportation dampin - `depth` (number, default: 3): Number of hops to take - `seed` (string, optional): Seed for reproducible walks - `direction` (string, optional): Edge direction filter (`forward`, `backward`, `any`). Default: `forward` + - `avoidCycles` (boolean, optional): Self-avoiding walk — never revisits a node; stops early when every neighbor is already on the path. Default: `false` - Neighbors are selected proportional to their MERW eigenvector component ψ - Falls back to uniform sampling if ψ has not been computed - Returns the terminal entity name and the path taken diff --git a/native/graph.c b/native/graph.c index d82555e..bcea550 100644 --- a/native/graph.c +++ b/native/graph.c @@ -866,7 +866,7 @@ u32 graph_compute_merw_psi(graph_t *g, double alpha, u32 max_iter, double tol) { } u32 graph_random_walk(graph_t *g, u64 start, u32 depth, u32 direction, int merw_mode, - u64 seed, u64 *out_path, u32 max_path) { + u64 seed, int avoid_cycles, u64 *out_path, u32 max_path) { u64 st = seed ? seed : g_rng; u32 plen = 0; if (max_path >= 1) out_path[plen] = start; @@ -881,6 +881,17 @@ u32 graph_random_walk(graph_t *g, u64 start, u32 depth, u32 direction, int merw_ for (u32 k = 0; k < ec; k++) { if (!dir_match(direction, es[k].direction)) continue; u64 t = es[k].target_offset; if (t == cur) continue; + if (avoid_cycles) { + /* Self-avoiding walk: skip any node already on the path. The + * path prefix doubles as the visited set (callers pass + * max_path >= depth+1, so plen never exceeds max_path and the + * prefix in out_path is complete). All neighbors visited -> + * nc == 0 below -> stops early. */ + u32 vmax = plen < max_path ? plen : max_path; + int seen = 0; + for (u32 j = 0; j < vmax; j++) if (out_path[j] == t) { seen = 1; break; } + if (seen) continue; + } double p = rdf64(g->mf, t + E_PSI); int found = 0; for (u32 j = 0; j < nc; j++) if (cand[j] == t) { if (p > cpsi[j]) cpsi[j] = p; found = 1; break; } diff --git a/native/graph.h b/native/graph.h index 8f16fee..6521fd9 100644 --- a/native/graph.h +++ b/native/graph.h @@ -132,8 +132,10 @@ void graph_set_entity_fields(graph_t *g, u64 off, u64 mtime, u64 obs_mtime, void graph_set_totals(graph_t *g, u64 structural_total, u64 walker_total); u32 graph_structural_sample(graph_t *g, u32 iterations, double damping); /* MC pagerank; total visits */ u32 graph_compute_merw_psi(graph_t *g, double alpha, u32 max_iter, double tol); /* iters run */ -/* random walk; mode: 1=merw (weighted by psi), 0=uniform; seed 0 = use global rng. Returns path node count. */ +/* random walk; mode: 1=merw (weighted by psi), 0=uniform; seed 0 = use global rng. + * avoid_cycles: 1 = self-avoiding (never revisits a node; stops early when every + * neighbor is already on the path). Returns path node count. */ u32 graph_random_walk(graph_t *g, u64 start, u32 depth, u32 direction, int merw_mode, - u64 seed, u64 *out_path, u32 max_path); + u64 seed, int avoid_cycles, u64 *out_path, u32 max_path); #endif /* GRAPH_H */ diff --git a/native/graphbind.c b/native/graphbind.c index 54cf394..25ddbc7 100644 --- a/native/graphbind.c +++ b/native/graphbind.c @@ -243,8 +243,8 @@ static napi_value n_structural_sample(napi_env env, napi_callback_info info){ AR static napi_value n_merw(napi_env env, napi_callback_info info) { ARGS(4); STORE; return mkU32(env, graph_compute_merw_psi(s->g, getF64(env, argv[1]), getU32(env, argv[2]), getF64(env, argv[3]))); } static napi_value n_seed(napi_env env, napi_callback_info info) { ARGS(2); (void)unwrap(env, argv[0]); graph_seed_rng(getU64(env, argv[1])); return NULL; } static napi_value n_random_walk(napi_env env, napi_callback_info info) { - ARGS(6); STORE; u32 depth = getU32(env, argv[2]); u32 cap = depth + 1; u64 *out = malloc((size_t)cap * 8); - u32 n = graph_random_walk(s->g, getU64(env, argv[1]), depth, getU32(env, argv[3]), getU32(env, argv[4]), getU64(env, argv[5]), out, cap); + ARGS(7); STORE; u32 depth = getU32(env, argv[2]); u32 cap = depth + 1; u64 *out = malloc((size_t)cap * 8); + u32 n = graph_random_walk(s->g, getU64(env, argv[1]), depth, getU32(env, argv[3]), getU32(env, argv[4]), getU64(env, argv[5]), getU32(env, argv[6]), out, cap); napi_value r = u64arr(env, out, n < cap ? n : cap); free(out); return r; } diff --git a/server.ts b/server.ts index 0603987..0413a6a 100644 --- a/server.ts +++ b/server.ts @@ -1122,6 +1122,7 @@ export class KnowledgeGraphManager { seed?: string, direction: 'forward' | 'backward' | 'any' = 'forward', mode: RandomWalkMode = 'merw', + avoidCycles: boolean = false, ): Promise<{ entity: string; path: string[] }> { return traced( 'kb.random_walk', @@ -1130,6 +1131,7 @@ export class KnowledgeGraphManager { 'kb.traversal.direction': direction, 'kb.walker.mode': mode, 'kb.walker.seeded': seed !== undefined, + 'kb.walker.avoid_cycles': avoidCycles, }, (span) => this.withReadLock(() => { const startOffset = this.db.lookup(start); @@ -1141,7 +1143,7 @@ export class KnowledgeGraphManager { // seed of 0 means "use the global RNG" (unseeded), so hashSeed (never // 0) keeps seeded walks reproducible. const seedU64 = seed !== undefined ? BigInt(this.hashSeed(seed) >>> 0) : 0n; - const pathOffsets = this.db.randomWalk(startOffset, depth, direction, mode === 'merw', seedU64); + const pathOffsets = this.db.randomWalk(startOffset, depth, direction, mode === 'merw', seedU64, avoidCycles); const pathNames = pathOffsets.map(o => this.db.entityName(o)); span.setAttribute('kb.walker.steps_taken', pathNames.length - 1); @@ -1565,6 +1567,11 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { description: "Transition policy. 'merw' (default) weights each step by ψ (the cached Maximum-Entropy Random Walk eigenvector), biasing the walk toward structurally important nodes; falls back to uniform if ψ is not yet computed. 'uniform' samples each eligible neighbor with equal probability — useful for unbiased exploration or as a baseline for comparison.", }, + avoidCycles: { + type: "boolean", + description: + "If true, the walk never revisits a node (self-avoiding — the path contains no cycles) and stops early when every neighbor is already on the path. Default: false", + }, }, required: ["start"], }, @@ -1765,6 +1772,7 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p args.seed as string | undefined, (args.direction as 'forward' | 'backward' | 'any') ?? 'forward', (args.mode as RandomWalkMode) ?? 'merw', + (args.avoidCycles as boolean) ?? false, ); return { content: [{ type: "text", text: JSON.stringify(result) }] }; } diff --git a/src/store.ts b/src/store.ts index fa4651f..4dd0076 100644 --- a/src/store.ts +++ b/src/store.ts @@ -82,7 +82,7 @@ interface NativeStore { structuralSample(h: unknown, iterations: number, damping: number): number; computeMerwPsi(h: unknown, alpha: number, maxIter: number, tol: number): number; seedRng(h: unknown, seed: bigint): void; - randomWalk(h: unknown, start: bigint, depth: number, direction: number, merwMode: number, seed: bigint): bigint[]; + randomWalk(h: unknown, start: bigint, depth: number, direction: number, merwMode: number, seed: bigint, avoidCycles: number): bigint[]; validateObs(h: unknown): { offset: bigint; count: number; oversize: number }[]; validateDangling(h: unknown): { src: bigint; target: bigint }[]; setEntityFields(h: unknown, off: bigint, mtime: bigint, obsMtime: bigint, structuralVisits: bigint, walkerVisits: bigint, psi: number): void; @@ -171,8 +171,8 @@ export class Store { structuralSample(iterations: number, damping: number): number { return native.structuralSample(this.h, iterations, damping); } computeMerwPsi(alpha: number, maxIter: number, tol: number): number { return native.computeMerwPsi(this.h, alpha, maxIter, tol); } seedRng(seed: bigint): void { native.seedRng(this.h, seed); } - randomWalk(start: bigint, depth: number, direction: Direction, merwMode: boolean, seed: bigint): bigint[] { - return native.randomWalk(this.h, start, depth, dirCode(direction), merwMode ? 1 : 0, seed); + randomWalk(start: bigint, depth: number, direction: Direction, merwMode: boolean, seed: bigint, avoidCycles: boolean): bigint[] { + return native.randomWalk(this.h, start, depth, dirCode(direction), merwMode ? 1 : 0, seed, avoidCycles ? 1 : 0); } // validate diff --git a/tests/memory-server.test.ts b/tests/memory-server.test.ts index 52a2e76..fdfb07a 100644 --- a/tests/memory-server.test.ts +++ b/tests/memory-server.test.ts @@ -1277,6 +1277,12 @@ describe('MCP Memory Server E2E Tests', () => { { name: 'South', entityType: 'Node', observations: ['South node'] }, { name: 'East', entityType: 'Node', observations: ['East node'] }, { name: 'Isolated', entityType: 'Node', observations: ['No connections'] }, + { name: 'Ping', entityType: 'Node', observations: ['2-cycle pair'] }, + { name: 'Pong', entityType: 'Node', observations: ['2-cycle pair'] }, + { name: 'LoopA', entityType: 'Node', observations: ['Chain head'] }, + { name: 'LoopB', entityType: 'Node', observations: ['Chain cycle'] }, + { name: 'LoopC', entityType: 'Node', observations: ['Chain cycle'] }, + { name: 'LoopD', entityType: 'Node', observations: ['Chain tail'] }, ] }); await callTool(client, 'create_relations', { @@ -1285,6 +1291,17 @@ describe('MCP Memory Server E2E Tests', () => { { from: 'Center', to: 'South', relationType: 'connects' }, { from: 'Center', to: 'East', relationType: 'connects' }, { from: 'North', to: 'South', relationType: 'connects' }, + // Cycle fixtures for the avoidCycles tests: Ping<->Pong is a 2-cycle; + // LoopA->LoopB->LoopC->LoopD with back-edges B->A and C->B, so an + // unguarded walk can cycle but a self-avoiding one is forced along + // the chain. + { from: 'Ping', to: 'Pong', relationType: 'connects' }, + { from: 'Pong', to: 'Ping', relationType: 'connects' }, + { from: 'LoopA', to: 'LoopB', relationType: 'connects' }, + { from: 'LoopB', to: 'LoopA', relationType: 'connects' }, + { from: 'LoopB', to: 'LoopC', relationType: 'connects' }, + { from: 'LoopC', to: 'LoopB', relationType: 'connects' }, + { from: 'LoopC', to: 'LoopD', relationType: 'connects' }, ] }); }); @@ -1385,6 +1402,60 @@ describe('MCP Memory Server E2E Tests', () => { expect(visited.has('South')).toBe(true); expect(visited.has('East')).toBe(true); }); + + it('avoidCycles=true produces a simple, cycle-free path', async () => { + const result = await callTool(client, 'random_walk', { + start: 'LoopA', + depth: 6, + avoidCycles: true, + }) as { entity: string; path: string[] }; + + // LoopA -> LoopB -> LoopC -> LoopD: back-edges are blocked by the visited + // set, so the walk is forced along the chain and then stops at the tail. + expect(result.path).toEqual(['LoopA', 'LoopB', 'LoopC', 'LoopD']); + expect(new Set(result.path).size).toBe(result.path.length); + }); + + it('avoidCycles=true stops when every neighbor is already on the path', async () => { + const result = await callTool(client, 'random_walk', { + start: 'Ping', + depth: 5, + avoidCycles: true, + }) as { entity: string; path: string[] }; + + // Ping <-> Pong is a 2-cycle; the self-avoiding walk takes one step and + // stops rather than bouncing. + expect(result.path).toEqual(['Ping', 'Pong']); + expect(result.entity).toBe('Pong'); + }); + + it('without avoidCycles the walk may still revisit nodes (default unchanged)', async () => { + const result = await callTool(client, 'random_walk', { + start: 'Ping', + depth: 5, + }) as { entity: string; path: string[] }; + + // Single candidate at every step, so the default walk alternates + // deterministically — proof that revisits are still permitted. + expect(result.path).toEqual(['Ping', 'Pong', 'Ping', 'Pong', 'Ping', 'Pong']); + }); + + it('avoidCycles=true walks stay reproducible under a seed', async () => { + const r1 = await callTool(client, 'random_walk', { + start: 'Center', + depth: 3, + seed: 'cycle-seed', + avoidCycles: true, + }) as { entity: string; path: string[] }; + const r2 = await callTool(client, 'random_walk', { + start: 'Center', + depth: 3, + seed: 'cycle-seed', + avoidCycles: true, + }) as { entity: string; path: string[] }; + + expect(r1.path).toEqual(r2.path); + }); }); describe('Sorting', () => { From 87693497fda1edcadda115078f9354e342c015ad Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Thu, 8 Oct 2026 01:10:33 -0400 Subject: [PATCH 3/8] fix(ci): repair op-bench after graph_random_walk signature change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit native/op_bench.c: pass the new avoid_cycles arg (0) — the harness keeps measuring the same op, so base/head results stay comparable. The benchmark job went red on #244 because the runner's merge-ref (new store sources + old harness) failed to compile. Co-Authored-By: Claude Code --- native/op_bench.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/native/op_bench.c b/native/op_bench.c index db48a39..04668bb 100644 --- a/native/op_bench.c +++ b/native/op_bench.c @@ -166,7 +166,7 @@ int main(int argc, char **argv) { ADAPT("find_path_d6", sink32 = graph_find_path(g, off[idx[_k]], off[idx2[_k]], 6, DIR_ANY, out, cap)); ADAPT("search", sink32 = graph_search(g, pat[_k], out, cap)); ADAPT("entities_by_type", sink32 = graph_entities_by_type(g, (const u8 *)tynames[tpix[_k]], tylen[tpix[_k]], out, cap)); - ADAPT("random_walk_d5", sink32 = graph_random_walk(g, off[idx[_k]], 5, DIR_ANY, 1, seed[_k], out, 8)); + ADAPT("random_walk_d5", sink32 = graph_random_walk(g, off[idx[_k]], 5, DIR_ANY, 1, seed[_k], 0, out, 8)); ADAPT("inc_walker_visit", graph_inc_walker_visit(g, off[idx[_k]])); ADAPT("structural_sample", graph_structural_sample(g, 1, 0.85)); ADAPT("compute_merw_psi", graph_compute_merw_psi(g, 0.85, 100, 1e-8)); From e218b39614dfba0342aa1699bfb42011c782b133 Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Thu, 8 Oct 2026 01:11:38 -0400 Subject: [PATCH 4/8] ci(benchmark): surface head-build failures instead of hiding make output The head-build step sent make's output to /dev/null, which made the #244 red run undebuggable. Print the build-log tail on failure. Co-Authored-By: Claude Code --- .github/workflows/benchmark.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 0bfe8ed..14aca96 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -27,7 +27,11 @@ jobs: - name: Build head benchmark run: | - make -C native bench >/dev/null 2>&1 + if ! make -C native bench >/tmp/head_build.log 2>&1; then + echo "Head bench build/run failed:" + tail -50 /tmp/head_build.log + exit 1 + fi cp /tmp/mf_test_bench /tmp/bench_head - name: Build base benchmark (head harness, base store sources) From 5534f3b1afd49d5309e6285116a1a4ce449fc447 Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Thu, 8 Oct 2026 01:48:08 -0400 Subject: [PATCH 5/8] feat(api): error envelope + validate-first atomicity (policy R1/R2/R4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements docs/api-error-policy.md: errors are signal. - ToolError + stable codes; the request handler converts them into visible structured tool errors (isError + JSON envelope) — protocol errors are now reserved for genuine server faults (R1) - validate-first, all-or-nothing batches for create_entities, create_relations, add_observations, sequentialthinking (R2; fixes the mid-loop partial application in add_observations) - silent drops become errors (R4): missing relation endpoints (ENDPOINT_MISSING), all-missing open_nodes, unknown type, unknown get_neighbors/random_walk starts, invalid regex, kb_load file errors, unlinked thoughts - search NL-guard returns the same envelope (NO_MATCHES + suggestion) - tests: 13 protocol-error assertions converted to envelope checks; new policy block (atomicity proofs, envelope shape); fuzz now shows protoErr=0 with every rejection visible as toolErr - full suite 183/183 Co-Authored-By: Claude Code --- docs/api-error-policy.md | 89 +++++++++++ server.ts | 288 +++++++++++++++++++++++++++------ tests/memory-server.test.ts | 309 ++++++++++++++++++++++++------------ tests/test-utils.ts | 30 ++++ 4 files changed, 568 insertions(+), 148 deletions(-) create mode 100644 docs/api-error-policy.md diff --git a/docs/api-error-policy.md b/docs/api-error-policy.md new file mode 100644 index 0000000..72eec41 --- /dev/null +++ b/docs/api-error-policy.md @@ -0,0 +1,89 @@ +# API Error & Result Policy + +Status: **adopted 2026-10-08** (implementation ongoing — see [Status](#status)). + +This document is the contract for how every MCP tool reports failure and +success. It replaces the original "never fail, funnel the caller into +unfailing completion" approach: with RL-era models, errors are signal — a +silent no-op or a hidden protocol error teaches the caller nothing and lets +wrong beliefs persist (KB: `BGS_API_ErrorInconsistency`, +`Inventory_APISilentDrops_2026_10_08`). + +## Rules + +**R1 — Visible failures.** +Every failure the caller can act on is returned as a tool-level error: +`isError: true` with a JSON envelope body: + +```json +{ + "error": { + "tool": "create_relations", + "code": "ENDPOINT_MISSING", + "message": "2 relation(s) reference nonexistent entities; nothing was created (details below).", + "details": [ + { "from": "A", "to": "Ghost", "relationType": "USES", "code": "ENDPOINT_MISSING", + "missing": ["to"], "message": "endpoint(s) not found: to" } + ] + } +} +``` + +Stable codes: `ENTITY_NOT_FOUND`, `TYPE_NOT_FOUND`, `ENDPOINT_MISSING`, +`COLLISION`, `LIMIT_EXCEEDED`, `INVALID_REGEX`, `INVALID_FILE`, +`NO_MATCHES`, `VALIDATION_FAILED`. JSON-RPC protocol errors are reserved for +genuine server faults and are never used for caller-actionable failures. + +**R2 — Validate-first, atomic.** +A mutation validates the entire request under the write lock before touching +state, and is all-or-nothing: a batch containing a failing item changes +nothing, and the error lists every offending item in one round-trip. + +**R3 — Per-item ledger.** +Every mutating success enumerates per-item outcomes +(`created` / `existing` / `skippedDuplicates` / `notFound` / `deleted` / +`added` / `alreadyPresent`). There is no bare "successfully" response. + +**R4 — No silent drops.** +An item that cannot be satisfied errors (R1) unless the caller's intent is +provably satisfied — duplicates are "already there", deleting an absent +thing is "already absent". Those cases succeed **and are reported** (R3). + +**R5 — Pagination integrity.** +Cursors identify the result set they came from (a fingerprint now; a store +txid once the owner daemon lands). A stale/mismatched cursor returns a +`CURSOR_STALE` error instead of a silently wrong page. + +## Per-op policy + +| Tool | On failure | On success | +|---|---|---| +| `create_entities` | same-name-different-data → `COLLISION` (atomic, all offenders); limit breaches → `LIMIT_EXCEEDED` | `{created, existing}` — exact duplicates are success | +| `create_relations` | any endpoint missing → `ENDPOINT_MISSING` (atomic, all offenders) | `{created, skippedDuplicates}` | +| `add_observations` | unknown entity → `ENTITY_NOT_FOUND`; limit breaches → `LIMIT_EXCEEDED` (atomic, all offenders) | per-entity `{entityName, addedObservations, alreadyPresent}` | +| `delete_entities` / `delete_relations` / `delete_observations` | nothing errors — absence is `notFound` (R4) | `{deleted, notFound}` ledgers | +| `sequentialthinking` | unknown `previousCtxId` → `ENTITY_NOT_FOUND` (chain integrity); limits → `LIMIT_EXCEEDED` | `{ctxId, linkedTo}` | +| `kb_load` | bad extension / unreadable file → `INVALID_FILE`; entity & relation stages per their rows | per-stage ledger (document, stats, entity/relation outcomes) | +| `open_nodes` | ALL requested names missing → `ENTITY_NOT_FOUND` | graph result + `missing[]` for partial misses | +| `get_neighbors` | unknown start → `ENTITY_NOT_FOUND` | neighbors (depth 0 = immediate) | +| `search_nodes` | invalid ERE → `INVALID_REGEX`; literal query, zero matches → `NO_MATCHES` | paginated graph | +| `get_entities_by_type` | type absent from the data-derived type set → `TYPE_NOT_FOUND` | entities of that type | +| `random_walk` | unknown start → `ENTITY_NOT_FOUND` | `{entity, path, modeUsed, stopReason}` | +| `find_path` | — (β-contract already reports `targetReached` / `budgetExhausted` / `note`) | unchanged — the reference pattern for R1-style reporting | +| reads (`get_entity_types`, `get_relation_types`, `get_stats`, `get_orphaned_entities`, `validate_graph`, `decode_timestamp`) | unchanged | unchanged | + +Decisions of record (Lev, 2026-10-08): duplicates are ledger reports, not +errors; the unified surface is `isError` with protocol errors reserved for +faults; Q1 = atomic-fail on missing endpoints; Q2/Q3 = error. + +## Status + +- **This change (envelope PR):** R1 (envelope + codes + dispatch conversion), + R2 (validate-first atomicity for `create_entities`, `create_relations`, + `add_observations`, `sequentialthinking`), R4 (drops→errors: + `ENDPOINT_MISSING`, all-missing `open_nodes`, `TYPE_NOT_FOUND`, unknown + starts), `INVALID_FILE`, `INVALID_REGEX`, `NO_MATCHES` envelope. +- **Ledger PR (next):** R3 result ledgers + report fields (`missing[]`, + `modeUsed`, `alreadyPresent`), R5 cursor fingerprinting. +- **v4 line (`feat/libsegstore`):** inherits this policy at reconciliation; + kbd4's batch=txn=commit model is the natural home for R2 atomicity. diff --git a/server.ts b/server.ts index 5054455..e2731a0 100644 --- a/server.ts +++ b/server.ts @@ -19,14 +19,48 @@ import { toolDurationHistogram, traced, tracer } from './src/tracing.js'; /** * Result envelope for a single tool dispatch. Mirrors the MCP CallToolResult * shape we return from every case; `isError: true` signals a tool-level error - * that the LLM should see (vs. a thrown error which becomes a hidden JSON-RPC - * protocol error). + * that the LLM should see. Caller-actionable failures are thrown as + * {@link ToolError} and converted into that visible shape by the request + * handler; any other exception propagates as a protocol error and is reserved + * for genuine server faults. */ type ToolDispatchResult = { content: Array<{ type: 'text'; text: string }>; isError?: boolean; }; +/** + * Stable, machine-readable error codes carried in the tool-error envelope. + * This list is part of the public contract (docs/api-error-policy.md). + */ +export type ToolErrorCode = + | 'ENTITY_NOT_FOUND' + | 'TYPE_NOT_FOUND' + | 'ENDPOINT_MISSING' + | 'COLLISION' + | 'LIMIT_EXCEEDED' + | 'INVALID_REGEX' + | 'INVALID_FILE' + | 'NO_MATCHES' + | 'VALIDATION_FAILED'; + +/** + * A failure the CALLER can act on. The request handler converts these into a + * VISIBLE, structured tool error (`isError: true` with a JSON envelope) — + * never a protocol error. Protocol errors stay reserved for genuine server + * faults. See docs/api-error-policy.md (R1). + */ +export class ToolError extends Error { + constructor( + public code: ToolErrorCode, + message: string, + public details?: unknown, + ) { + super(message); + this.name = 'ToolError'; + } +} + /** * True if the query contains any regex metacharacter. Used to gate the * `search_nodes` natural-language warning: a pure literal that returns no @@ -581,22 +615,43 @@ export class KnowledgeGraphManager { } async createEntities(entities: Entity[]): Promise { - // Validate observation limits (can do outside lock) + // Validate observation limits across the WHOLE batch first, collecting + // every violation rather than stopping at the first (R2: validate-first; + // the caller gets all offenders in one round-trip). + const limitViolations: { entity: string; code: ToolErrorCode; message: string }[] = []; for (const entity of entities) { if (entity.observations.length > 2) { - throw new Error(`Entity "${entity.name}" has ${entity.observations.length} observations. Maximum allowed is 2.`); + limitViolations.push({ + entity: entity.name, + code: 'LIMIT_EXCEEDED', + message: `has ${entity.observations.length} observations; maximum allowed is 2`, + }); } - for (const obs of entity.observations) { + entity.observations.forEach((obs, index) => { if (obs.length > 140) { - throw new Error(`Observation in entity "${entity.name}" exceeds 140 characters (${obs.length} chars): "${obs.substring(0, 50)}..."`); + limitViolations.push({ + entity: entity.name, + code: 'LIMIT_EXCEEDED', + message: `observation #${index + 1} is ${obs.length} chars (140 max, ${obs.length - 140} over)`, + }); } - } + }); + } + if (limitViolations.length > 0) { + throw new ToolError( + 'LIMIT_EXCEEDED', + `${limitViolations.length} limit violation(s) in the batch; nothing was created (details below).`, + limitViolations, + ); } return this.withWriteLock(() => { const now = BigInt(Date.now()); - const newEntities: Entity[] = []; + // Pass 1 — classify every entity before writing anything. A collision + // fails the whole batch (R2: atomic) and lists every offender. + const toCreate: Entity[] = []; + const collisions: { entity: string; code: ToolErrorCode; message: string }[] = []; for (const e of entities) { const existingOffset = this.db.lookup(e.name); if (existingOffset !== 0n) { @@ -604,10 +659,27 @@ export class KnowledgeGraphManager { const sameType = existing.entityType === e.entityType; const sameObs = existing.observations.length === e.observations.length && existing.observations.every((o, i) => o === e.observations[i]); - if (sameType && sameObs) continue; - throw new Error(`Entity "${e.name}" already exists with different data (type: "${existing.entityType}" vs "${e.entityType}", observations: ${existing.observations.length} vs ${e.observations.length})`); + if (sameType && sameObs) continue; // exact duplicate: intent satisfied + collisions.push({ + entity: e.name, + code: 'COLLISION', + message: `exists with different data (type: "${existing.entityType}" vs "${e.entityType}", observations: ${existing.observations.length} vs ${e.observations.length})`, + }); + } else { + toCreate.push(e); } + } + if (collisions.length > 0) { + throw new ToolError( + 'COLLISION', + `${collisions.length} entity name collision(s); nothing was created (details below).`, + collisions, + ); + } + // Pass 2 — create. + const newEntities: Entity[] = []; + for (const e of toCreate) { const offset = this.db.createEntity(e.name, e.entityType, now); for (const obs of e.observations) { this.db.addObservation(offset, obs, now); @@ -628,12 +700,40 @@ export class KnowledgeGraphManager { async createRelations(relations: Relation[]): Promise { return this.withWriteLock(() => { const now = BigInt(Date.now()); - const newRelations: Relation[] = []; + // Pass 1 — every endpoint must exist; missing endpoints fail the whole + // batch (R2/R4: a relation to a nonexistent entity is a state desync, + // never a silent drop). + const missing: { from: string; to: string; relationType: string; code: ToolErrorCode; missing: string[]; message: string }[] = []; + for (const r of relations) { + const missingEnds: string[] = []; + if (this.db.lookup(r.from) === 0n) missingEnds.push('from'); + if (this.db.lookup(r.to) === 0n) missingEnds.push('to'); + if (missingEnds.length > 0) { + missing.push({ + from: r.from, + to: r.to, + relationType: r.relationType, + code: 'ENDPOINT_MISSING', + missing: missingEnds, + message: `endpoint(s) not found: ${missingEnds.join(', ')}`, + }); + } + } + if (missing.length > 0) { + throw new ToolError( + 'ENDPOINT_MISSING', + `${missing.length} relation(s) reference nonexistent entities; nothing was created (details below).`, + missing, + ); + } + + // Pass 2 — create. Duplicates are already-satisfied intent and are + // skipped (they become part of the result ledger under policy R3). + const newRelations: Relation[] = []; for (const r of relations) { const fromOffset = this.db.lookup(r.from); const toOffset = this.db.lookup(r.to); - if (fromOffset === 0n || toOffset === 0n) continue; const isDuplicate = this.db.edges(fromOffset).some(e => e.direction === DIR_FORWARD && e.target === toOffset && e.relType === r.relationType @@ -651,33 +751,58 @@ export class KnowledgeGraphManager { async addObservations(observations: { entityName: string; contents: string[] }[]): Promise<{ entityName: string; addedObservations: string[] }[]> { return this.withWriteLock(() => { - const results: { entityName: string; addedObservations: string[] }[] = []; + // Pass 1 — validate the WHOLE batch before touching state (R2: atomic; + // the old code validated inside the write loop, so a late failure left + // earlier items applied). Collect every violation for one round-trip. + const pending: { entityName: string; offset: bigint; newObservations: string[] }[] = []; + const violations: { entityName: string; code: ToolErrorCode; message: string }[] = []; for (const o of observations) { const offset = this.db.lookup(o.entityName); if (offset === 0n) { - throw new Error(`Entity with name ${o.entityName} not found`); + violations.push({ entityName: o.entityName, code: 'ENTITY_NOT_FOUND', message: 'entity does not exist' }); + continue; } - for (const obs of o.contents) { + o.contents.forEach((obs, index) => { if (obs.length > 140) { - throw new Error(`Observation for "${o.entityName}" exceeds 140 characters (${obs.length} chars): "${obs.substring(0, 50)}..."`); + violations.push({ + entityName: o.entityName, + code: 'LIMIT_EXCEEDED', + message: `observation #${index + 1} is ${obs.length} chars (140 max, ${obs.length - 140} over)`, + }); } - } + }); const existingObs = this.db.readEntity(offset).observations; const newObservations = o.contents.filter(content => !existingObs.includes(content)); - if (existingObs.length + newObservations.length > 2) { - throw new Error(`Adding ${newObservations.length} observations to "${o.entityName}" would exceed limit of 2 (currently has ${existingObs.length}).`); + violations.push({ + entityName: o.entityName, + code: 'LIMIT_EXCEEDED', + message: `adding ${newObservations.length} observation(s) would give ${existingObs.length + newObservations.length} total; maximum is 2`, + }); } + pending.push({ entityName: o.entityName, offset, newObservations }); + } - const now = BigInt(Date.now()); - for (const obs of newObservations) { - this.db.addObservation(offset, obs, now); - } + if (violations.length > 0) { + const codes = new Set(violations.map(v => v.code)); + throw new ToolError( + codes.size === 1 ? violations[0].code : 'VALIDATION_FAILED', + `${violations.length} observation(s) failed validation; nothing was written (details below).`, + violations, + ); + } - results.push({ entityName: o.entityName, addedObservations: newObservations }); + // Pass 2 — apply. + const now = BigInt(Date.now()); + const results: { entityName: string; addedObservations: string[] }[] = []; + for (const p of pending) { + for (const obs of p.newObservations) { + this.db.addObservation(p.offset, obs, now); + } + results.push({ entityName: p.entityName, addedObservations: p.newObservations }); } return results; @@ -742,7 +867,7 @@ export class KnowledgeGraphManager { // query is never rejected by a divergent JS RegExp dialect, and an invalid one // is rejected consistently. Preserves the "Invalid regex pattern" contract. if (!this.db.regexValid(query)) { - throw new Error(`Invalid regex pattern: ${query}`); + throw new ToolError('INVALID_REGEX', `Invalid regex pattern: ${query}`); } return traced( @@ -789,6 +914,17 @@ export class KnowledgeGraphManager { offsetByName.set(name, offset); } + // Q2 (docs/api-error-policy.md): when NONE of the requested names exist, + // the request is a typo/desync — error instead of returning an empty + // graph (a partial miss is reported via the R3 ledger). + if (names.length > 0 && filteredEntities.length === 0) { + throw new ToolError( + 'ENTITY_NOT_FOUND', + `None of the ${names.length} requested entities exist.`, + { missing: names }, + ); + } + const filteredEntityNames = new Set(filteredEntities.map(e => e.name)); const filteredRelations: Relation[] = []; @@ -838,7 +974,7 @@ export class KnowledgeGraphManager { const startOffset = this.db.lookup(entityName); if (startOffset === 0n) { span.setAttribute('kb.traversal.start_found', false); - return []; + throw new ToolError('ENTITY_NOT_FOUND', `Start entity not found: ${entityName}`); } span.setAttribute('kb.traversal.start_found', true); @@ -989,7 +1125,17 @@ export class KnowledgeGraphManager { async getEntitiesByType(entityType: string, sortBy?: EntitySortField, sortDir?: SortDirection): Promise { return this.withReadLock(() => { - const filtered = this.getAllEntities().filter(e => e.entityType === entityType); + const all = this.getAllEntities(); + // Q3 (docs/api-error-policy.md): the type set is data-derived, so an + // absent type can never have entities — a miss is a typo/desync, not an + // empty result. + if (!all.some(e => e.entityType === entityType)) { + throw new ToolError( + 'TYPE_NOT_FOUND', + `Entity type "${entityType}" is not present in the graph. See get_entity_types for the full list.`, + ); + } + const filtered = all.filter(e => e.entityType === entityType); const rankMaps = this.getRankMapsUnlocked(); return sortEntities(filtered, sortBy, sortDir, rankMaps); }); @@ -1136,7 +1282,7 @@ export class KnowledgeGraphManager { (span) => this.withReadLock(() => { const startOffset = this.db.lookup(start); if (startOffset === 0n) { - throw new Error(`Start entity not found: ${start}`); + throw new ToolError('ENTITY_NOT_FOUND', `Start entity not found: ${start}`); } // Seeded walk: hash the string seed to a u64 the C RNG can use. A @@ -1203,33 +1349,45 @@ export class KnowledgeGraphManager { } async addThought(observations: string[], previousCtxId?: string): Promise<{ ctxId: string }> { - // Validate observations (can do outside lock) + // Validate observation limits across the whole request first. + const violations: { code: ToolErrorCode; message: string }[] = []; if (observations.length > 2) { - throw new Error(`Thought has ${observations.length} observations. Maximum allowed is 2.`); + violations.push({ code: 'LIMIT_EXCEEDED', message: `Thought has ${observations.length} observations; maximum allowed is 2` }); } - for (const obs of observations) { + observations.forEach((obs, index) => { if (obs.length > 140) { - throw new Error(`Observation exceeds 140 characters (${obs.length} chars): "${obs.substring(0, 50)}..."`); + violations.push({ code: 'LIMIT_EXCEEDED', message: `observation #${index + 1} is ${obs.length} chars (140 max, ${obs.length - 140} over)` }); } + }); + if (violations.length > 0) { + throw new ToolError('LIMIT_EXCEEDED', `${violations.length} limit violation(s); nothing was created (details below).`, violations); } return this.withWriteLock(() => { const now = BigInt(Date.now()); - const ctxId = randomBytes(12).toString('hex'); + // Validate the chain link BEFORE creating the thought (R2: atomic; a + // silently-unlinked thought is exactly the FORGOT_EVIDENCE failure the + // chain exists to prevent). + let prevOffset = 0n; + if (previousCtxId) { + prevOffset = this.db.lookup(previousCtxId); + if (prevOffset === 0n) { + throw new ToolError('ENTITY_NOT_FOUND', `previousCtxId "${previousCtxId}" does not exist; refusing to create an unlinked thought.`); + } + } + + const ctxId = randomBytes(12).toString('hex'); const offset = this.db.createEntity(ctxId, 'Thought', now); for (const obs of observations) { this.db.addObservation(offset, obs, now); } if (previousCtxId) { - const prevOffset = this.db.lookup(previousCtxId); - if (prevOffset !== 0n) { - // prev --follows--> new, and new --preceded_by--> prev. C creates - // both directed edges per relation and owns the refcounts. - this.db.createRelation(prevOffset, offset, 'follows', now); - this.db.createRelation(offset, prevOffset, 'preceded_by', now); - } + // prev --follows--> new, and new --preceded_by--> prev. C creates + // both directed edges per relation and owns the refcounts. + this.db.createRelation(prevOffset, offset, 'follows', now); + this.db.createRelation(offset, prevOffset, 'preceded_by', now); } return { ctxId }; @@ -1629,9 +1787,12 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p /** * Dispatch a single tool call. Extracted from the request handler so the * handler can wrap it with span/metric instrumentation without duplicating - * the per-tool logic. Returns the MCP `CallToolResult` shape; thrown errors - * become JSON-RPC protocol errors (hidden from the model), while - * `{ isError: true }` returns are visible tool-level errors. + * the per-tool logic. Returns the MCP `CallToolResult` shape. + * + * Error policy (docs/api-error-policy.md): caller-actionable failures are + * thrown as `ToolError`s — the request handler converts them into VISIBLE + * structured tool errors (`isError: true` + JSON envelope). Any other throw + * remains a JSON-RPC protocol error and is reserved for server faults. */ async function dispatch(name: string, args: Record): Promise { switch (name) { @@ -1677,11 +1838,16 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p const suggestion = suggested && suggested !== query ? ` For multiple terms try ${JSON.stringify(suggested)}.` : ''; + const envelope = { + error: { + tool: 'search_nodes', + code: 'NO_MATCHES', + message: `No matches for ${JSON.stringify(query)}. search_nodes uses POSIX Extended Regular Expressions (ERE), case-sensitive — not natural language, and not JS/PCRE regex (use [0-9] not \\d, [[:alpha:]] not \\w; no lookahead or backreferences).${suggestion} You can also browse with get_entities_by_type, get_neighbors, or random_walk.`, + ...(suggestion && { details: { suggestedRegex: suggested } }), + }, + }; return { - content: [{ - type: "text", - text: `No matches for ${JSON.stringify(query)}. search_nodes uses POSIX Extended Regular Expressions (ERE), case-sensitive — not natural language, and not JS/PCRE regex (use [0-9] not \\d, [[:alpha:]] not \\w; no lookahead or backreferences).${suggestion} You can also browse with get_entities_by_type, get_neighbors, or random_walk.`, - }], + content: [{ type: "text", text: JSON.stringify(envelope, null, 2) }], isError: true, }; } @@ -1787,14 +1953,18 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p const filePath = args.filePath as string; // Validate extension - validateExtension(filePath); + try { + validateExtension(filePath); + } catch (err: unknown) { + throw new ToolError('INVALID_FILE', err instanceof Error ? err.message : String(err)); + } // Read file let text: string; try { text = fs.readFileSync(filePath, 'utf-8'); } catch (err: unknown) { - throw new Error(`Failed to read file: ${err instanceof Error ? err.message : String(err)}`); + throw new ToolError('INVALID_FILE', `Failed to read file: ${err instanceof Error ? err.message : String(err)}`); } // Derive title @@ -1887,6 +2057,26 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p } return result; } catch (err) { + if (err instanceof ToolError) { + // Policy R1 (docs/api-error-policy.md): caller-actionable failures + // surface as VISIBLE tool errors with a structured envelope — + // never as protocol errors. + errorType = `tool_error:${err.code}`; + span.setAttribute('error.type', errorType); + span.setStatus({ code: SpanStatusCode.ERROR, message: err.message }); + const envelope = { + error: { + tool: name, + code: err.code, + message: err.message, + ...(err.details !== undefined && { details: err.details }), + }, + }; + return { + content: [{ type: "text", text: JSON.stringify(envelope, null, 2) }], + isError: true, + }; + } errorType = (err as Error)?.constructor?.name ?? 'Error'; span.setAttribute('error.type', errorType); span.recordException(err as Error); diff --git a/tests/memory-server.test.ts b/tests/memory-server.test.ts index 5017c5a..52ff6b5 100644 --- a/tests/memory-server.test.ts +++ b/tests/memory-server.test.ts @@ -3,7 +3,7 @@ import { promises as fs } from 'fs'; import path from 'path'; import os from 'os'; import { createServer, type Entity, type Relation, type Neighbor } from '../server.js'; -import { createTestClient, callTool, callToolRaw, type PaginatedGraph, type PaginatedResult, type FindPathResult } from './test-utils.js'; +import { createTestClient, callTool, callToolRaw, parseToolError, type PaginatedGraph, type PaginatedResult, type FindPathResult } from './test-utils.js'; describe('MCP Memory Server E2E Tests', () => { let testDir: string; @@ -64,49 +64,49 @@ describe('MCP Memory Server E2E Tests', () => { expect(result[0].name).toBe('Bob'); }); - it('should error on duplicate name with different data', async () => { + it('should error on duplicate name with different data (COLLISION, visible)', async () => { await callTool(client, 'create_entities', { entities: [{ name: 'Alice', entityType: 'Person', observations: ['First'] }] }); - // Same name, different type — should error - await expect( - callTool(client, 'create_entities', { - entities: [{ name: 'Alice', entityType: 'Organization', observations: ['First'] }] - }) - ).rejects.toThrow(/already exists/); + // Same name, different type — visible tool error (policy R1) + const rawType = await callToolRaw(client, 'create_entities', { + entities: [{ name: 'Alice', entityType: 'Organization', observations: ['First'] }] + }); + expect(rawType.isError).toBe(true); + expect(parseToolError(rawType).code).toBe('COLLISION'); - // Same name, different observations — should error - await expect( - callTool(client, 'create_entities', { - entities: [{ name: 'Alice', entityType: 'Person', observations: ['Different'] }] - }) - ).rejects.toThrow(/already exists/); + // Same name, different observations — also visible + const rawObs = await callToolRaw(client, 'create_entities', { + entities: [{ name: 'Alice', entityType: 'Person', observations: ['Different'] }] + }); + expect(rawObs.isError).toBe(true); + expect(parseToolError(rawObs).code).toBe('COLLISION'); }); - it('should reject entities with more than 2 observations', async () => { - await expect( - callTool(client, 'create_entities', { - entities: [{ - name: 'TooMany', - entityType: 'Test', - observations: ['One', 'Two', 'Three'] - }] - }) - ).rejects.toThrow(/Maximum allowed is 2/); + it('should reject entities with more than 2 observations (LIMIT_EXCEEDED, visible)', async () => { + const raw = await callToolRaw(client, 'create_entities', { + entities: [{ + name: 'TooMany', + entityType: 'Test', + observations: ['One', 'Two', 'Three'] + }] + }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('LIMIT_EXCEEDED'); }); - it('should reject observations longer than 140 characters', async () => { + it('should reject observations longer than 140 characters (LIMIT_EXCEEDED, visible)', async () => { const longObservation = 'x'.repeat(141); - await expect( - callTool(client, 'create_entities', { - entities: [{ - name: 'LongObs', - entityType: 'Test', - observations: [longObservation] - }] - }) - ).rejects.toThrow(/exceeds 140 characters/); + const raw = await callToolRaw(client, 'create_entities', { + entities: [{ + name: 'LongObs', + entityType: 'Test', + observations: [longObservation] + }] + }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('LIMIT_EXCEEDED'); }); it('should delete entities and their relations', async () => { @@ -158,16 +158,16 @@ describe('MCP Memory Server E2E Tests', () => { expect(result[0].addedObservations).toEqual(['New']); }); - it('should reject adding observations that would exceed limit', async () => { + it('should reject adding observations that would exceed limit (LIMIT_EXCEEDED, visible)', async () => { await callTool(client, 'add_observations', { observations: [{ entityName: 'TestEntity', contents: ['One', 'Two'] }] }); - await expect( - callTool(client, 'add_observations', { - observations: [{ entityName: 'TestEntity', contents: ['Three'] }] - }) - ).rejects.toThrow(/would exceed limit of 2/); + const raw = await callToolRaw(client, 'add_observations', { + observations: [{ entityName: 'TestEntity', contents: ['Three'] }] + }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('LIMIT_EXCEEDED'); }); it('should delete specific observations', async () => { @@ -300,10 +300,10 @@ describe('MCP Memory Server E2E Tests', () => { expect(result.entities.items[0].name).toBe('TypeScript'); }); - it('should reject invalid regex', async () => { - await expect( - callTool(client, 'search_nodes', { query: '[invalid' }) - ).rejects.toThrow(/Invalid regex pattern/); + it('should reject invalid regex (INVALID_REGEX, visible)', async () => { + const raw = await callToolRaw(client, 'search_nodes', { query: '[invalid' }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('INVALID_REGEX'); }); it('accepts ERE-valid patterns that JS RegExp rejects (validator uses the C ERE engine)', async () => { @@ -315,11 +315,11 @@ describe('MCP Memory Server E2E Tests', () => { }); it('rejects ERE-invalid patterns even when JS RegExp accepts them', async () => { - // '(?:x)' is a valid JS non-capturing group but invalid POSIX ERE — must throw, - // not silently return zero matches. - await expect( - callTool(client, 'search_nodes', { query: '(?:x)' }) - ).rejects.toThrow(/Invalid regex pattern/); + // '(?:x)' is a valid JS non-capturing group but invalid POSIX ERE — must + // be a visible error, not silently zero matches. + const raw = await callToolRaw(client, 'search_nodes', { query: '(?:x)' }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('INVALID_REGEX'); }); it('trigram path: regex-extractable queries return the same results as before', async () => { @@ -495,11 +495,12 @@ describe('MCP Memory Server E2E Tests', () => { }); expect(raw.isError).toBe(true); - const text = raw.content[0]?.text ?? ''; - expect(text).toContain('knowledge graph features'); - expect(text).toContain('regex'); + const err = parseToolError(raw); + expect(err.code).toBe('NO_MATCHES'); + expect(err.message).toContain('knowledge graph features'); + expect(err.message).toContain('regex'); // Auto-suggested |-joined regex should appear. - expect(text).toContain('knowledge|graph|features'); + expect(err.message).toContain('knowledge|graph|features'); }); it('does NOT flag a regex query with anchors that simply misses', async () => { @@ -539,11 +540,12 @@ describe('MCP Memory Server E2E Tests', () => { }); expect(raw.isError).toBe(true); - const text = raw.content[0]?.text ?? ''; - expect(text).toContain('Slef'); - expect(text).toContain('regex'); + const err = parseToolError(raw); + expect(err.code).toBe('NO_MATCHES'); + expect(err.message).toContain('Slef'); + expect(err.message).toContain('regex'); // No multi-term suggestion because there's only one term. - expect(text).not.toContain('"Slef|'); + expect(err.message).not.toContain('"Slef|'); }); // Note: a "walker bias" test is intentionally omitted because the guard @@ -1201,29 +1203,25 @@ describe('MCP Memory Server E2E Tests', () => { expect(neighbors.items.some(n => n.name === second.ctxId)).toBe(true); }); - it('should ignore invalid previousCtxId gracefully', async () => { - const result = await callTool(client, 'sequentialthinking', { + it('should refuse an invalid previousCtxId (visible ENTITY_NOT_FOUND)', async () => { + // Policy R4: a silently-unlinked thought is exactly the evidence-loss + // failure the chain exists to prevent — an unknown link is an error. + const raw = await callToolRaw(client, 'sequentialthinking', { previousCtxId: 'nonexistent_thought', observations: ['Orphaned thought'] - }) as { ctxId: string }; - - expect(result.ctxId).toMatch(/^[0-9a-f]{24}$/); - - // Verify no neighbors (no valid relations were created) - const neighbors = await callTool(client, 'get_neighbors', { - entityName: result.ctxId, - depth: 1 - }) as PaginatedResult; - - expect(neighbors.items).toHaveLength(0); + }); + expect(raw.isError).toBe(true); + const err = parseToolError(raw); + expect(err.code).toBe('ENTITY_NOT_FOUND'); + expect(err.message).toContain('nonexistent_thought'); }); - it('should enforce observation limits on thoughts', async () => { - await expect( - callTool(client, 'sequentialthinking', { - observations: ['One', 'Two', 'Three'] - }) - ).rejects.toThrow(/Maximum allowed is 2/); + it('should enforce observation limits on thoughts (LIMIT_EXCEEDED, visible)', async () => { + const raw = await callToolRaw(client, 'sequentialthinking', { + observations: ['One', 'Two', 'Three'] + }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('LIMIT_EXCEEDED'); }); it('should set mtime and obsMtime on thought entities', async () => { @@ -1360,10 +1358,10 @@ describe('MCP Memory Server E2E Tests', () => { expect(result1.path).toEqual(result2.path); }); - it('should throw on non-existent start entity', async () => { - await expect( - callTool(client, 'random_walk', { start: 'NonExistent', depth: 2 }) - ).rejects.toThrow(/not found/); + it('should return a visible ENTITY_NOT_FOUND for a non-existent start entity', async () => { + const raw = await callToolRaw(client, 'random_walk', { start: 'NonExistent', depth: 2 }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('ENTITY_NOT_FOUND'); }); it('should accept mode=uniform and produce a valid walk', async () => { @@ -1965,28 +1963,30 @@ describe('MCP Memory Server E2E Tests', () => { docFile = path.join(testDir, 'test-doc.txt'); }); - it('should reject non-plaintext extensions', async () => { + it('should reject non-plaintext extensions (INVALID_FILE, visible)', async () => { const pdfPath = path.join(testDir, 'test.pdf'); await fs.writeFile(pdfPath, 'fake pdf content'); - await expect( - callTool(client, 'kb_load', { filePath: pdfPath }) - ).rejects.toThrow(/Unsupported file extension/); + const raw = await callToolRaw(client, 'kb_load', { filePath: pdfPath }); + expect(raw.isError).toBe(true); + const err = parseToolError(raw); + expect(err.code).toBe('INVALID_FILE'); + expect(err.message).toContain('Unsupported file extension'); }); - it('should reject files with no extension', async () => { + it('should reject files with no extension (INVALID_FILE, visible)', async () => { const noExtPath = path.join(testDir, 'noext'); await fs.writeFile(noExtPath, 'some content'); - await expect( - callTool(client, 'kb_load', { filePath: noExtPath }) - ).rejects.toThrow(/no extension/); + const raw = await callToolRaw(client, 'kb_load', { filePath: noExtPath }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('INVALID_FILE'); }); - it('should reject missing files', async () => { - await expect( - callTool(client, 'kb_load', { filePath: path.join(testDir, 'nonexistent.txt') }) - ).rejects.toThrow(/Failed to read file/); + it('should reject missing files (INVALID_FILE, visible)', async () => { + const raw = await callToolRaw(client, 'kb_load', { filePath: path.join(testDir, 'nonexistent.txt') }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('INVALID_FILE'); }); it('should load a small document and create entities + relations', async () => { @@ -2095,14 +2095,14 @@ describe('MCP Memory Server E2E Tests', () => { await fs.writeFile(docFile, 'Short doc for dedup testing purposes here.'); await callTool(client, 'kb_load', { filePath: docFile }); - // Second load with different content but same title — Document entity - // already exists with entityType 'Document' and no observations, - // so it gets silently skipped. But the index entities already exist - // with different observations, so it should error. + // Second load with different content but same title: the Document entity + // is an exact duplicate (intent satisfied), but the index entities collide + // with different observations → visible COLLISION (policy R1), and the + // failed load is atomic per op. await fs.writeFile(docFile, 'Completely different content for dedup testing now.'); - await expect( - callTool(client, 'kb_load', { filePath: docFile }) - ).rejects.toThrow(/already exists/); + const raw = await callToolRaw(client, 'kb_load', { filePath: docFile }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('COLLISION'); }); it('should enforce observation length limits', async () => { @@ -2137,4 +2137,115 @@ describe('MCP Memory Server E2E Tests', () => { } }); }); + + describe('API error policy (R1/R2/R4)', () => { + it('create_entities collision is atomic: nothing in the batch is created', async () => { + await callTool(client, 'create_entities', { + entities: [{ name: 'PolicyDup', entityType: 'PolicyTag', observations: ['base'] }], + }); + + const raw = await callToolRaw(client, 'create_entities', { + entities: [ + { name: 'PolicyFresh', entityType: 'PolicyTag', observations: ['new'] }, + { name: 'PolicyDup', entityType: 'PolicyTag', observations: ['changed'] }, + ], + }); + + expect(raw.isError).toBe(true); + const err = parseToolError(raw); + expect(err.code).toBe('COLLISION'); + expect(Array.isArray(err.details)).toBe(true); + + // Atomicity: PolicyFresh must NOT exist. Anchored regex → plain empty + // result (a literal query would route through the NL guard instead). + const fresh = await callTool(client, 'search_nodes', { query: '^PolicyFresh$' }) as PaginatedGraph; + expect(fresh.entities.items).toEqual([]); + }); + + it('create_relations with a missing endpoint errors and writes nothing', async () => { + await callTool(client, 'create_entities', { + entities: [ + { name: 'PolicyX', entityType: 'PolicyTag', observations: [] }, + { name: 'PolicyY', entityType: 'PolicyTag', observations: [] }, + ], + }); + + const raw = await callToolRaw(client, 'create_relations', { + relations: [ + { from: 'PolicyX', to: 'PolicyY', relationType: 'LINKS' }, + { from: 'PolicyX', to: 'PolicyGhost', relationType: 'LINKS' }, + ], + }); + + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('ENDPOINT_MISSING'); + + // Atomicity: the valid relation in the same batch was not created. + const after = await callTool(client, 'open_nodes', { names: ['PolicyX'] }) as PaginatedGraph; + expect(after.relations.items).toEqual([]); + }); + + it('add_observations validates the whole batch first (atomic)', async () => { + await callTool(client, 'create_entities', { + entities: [{ name: 'PolicyObs', entityType: 'PolicyTag', observations: [] }], + }); + + const raw = await callToolRaw(client, 'add_observations', { + observations: [ + { entityName: 'PolicyObs', contents: ['would-have-been-added'] }, + { entityName: 'PolicyGhost', contents: ['x'] }, + ], + }); + + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('ENTITY_NOT_FOUND'); + + // Atomicity: the first item must not have been applied. + const after = await callTool(client, 'open_nodes', { names: ['PolicyObs'] }) as PaginatedGraph; + expect(after.entities.items[0]?.observations).toEqual([]); + }); + + it('add_observations limit errors carry exact char counts', async () => { + await callTool(client, 'create_entities', { + entities: [{ name: 'PolicyObs2', entityType: 'PolicyTag', observations: [] }], + }); + + const raw = await callToolRaw(client, 'add_observations', { + observations: [{ entityName: 'PolicyObs2', contents: ['y'.repeat(151)] }], + }); + + expect(raw.isError).toBe(true); + const err = parseToolError(raw); + expect(err.code).toBe('LIMIT_EXCEEDED'); + // Precise counts live in the per-item details: "151 chars (…11 over)". + expect(JSON.stringify(err.details)).toContain('151'); + }); + + it('open_nodes errors when ALL requested names are missing', async () => { + const raw = await callToolRaw(client, 'open_nodes', { names: ['NoSuchOne', 'NoSuchTwo'] }); + expect(raw.isError).toBe(true); + const err = parseToolError(raw); + expect(err.code).toBe('ENTITY_NOT_FOUND'); + expect(err.details).toEqual({ missing: ['NoSuchOne', 'NoSuchTwo'] }); + }); + + it('get_entities_by_type errors on an unknown type', async () => { + const raw = await callToolRaw(client, 'get_entities_by_type', { entityType: 'NoSuchType' }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('TYPE_NOT_FOUND'); + }); + + it('get_neighbors errors on an unknown start entity', async () => { + const raw = await callToolRaw(client, 'get_neighbors', { entityName: 'NoSuchEntity' }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('ENTITY_NOT_FOUND'); + }); + + it('error envelopes carry the tool name and a message naming the offender', async () => { + const raw = await callToolRaw(client, 'get_neighbors', { entityName: 'NoSuchEntity' }); + const err = parseToolError(raw); + expect(err.tool).toBe('get_neighbors'); + expect(err.message).toContain('NoSuchEntity'); + }); + }); }); diff --git a/tests/test-utils.ts b/tests/test-utils.ts index 60e3961..ea99ebe 100644 --- a/tests/test-utils.ts +++ b/tests/test-utils.ts @@ -147,3 +147,33 @@ export async function callToolRaw( isError: (result as { isError?: boolean }).isError, }; } + +/** + * The structured body of a tool-level error, per policy R1 + * (docs/api-error-policy.md): `{ tool, code, message, details? }`. + */ +export interface ToolErrorEnvelope { + tool: string; + code: string; + message: string; + details?: unknown; +} + +/** + * Parse the JSON envelope out of a `callToolRaw` result. Throws if the result + * isn't a tool-error envelope, so tests fail loudly when a call unexpectedly + * succeeds or returns something unstructured. + */ +export function parseToolError(raw: RawToolResult): ToolErrorEnvelope { + const text = raw.content[0]?.text ?? ''; + let parsed: { error?: ToolErrorEnvelope }; + try { + parsed = JSON.parse(text) as { error?: ToolErrorEnvelope }; + } catch { + throw new Error(`Expected a JSON tool-error envelope, got non-JSON: ${text.slice(0, 200)}`); + } + if (!parsed.error) { + throw new Error(`Expected a tool-error envelope, got: ${text.slice(0, 200)}`); + } + return parsed.error; +} From f20e343b5571befa3ddb6b4f0266b71c4b6bf51d Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Thu, 8 Oct 2026 02:23:43 -0400 Subject: [PATCH 6/8] feat(api): result ledgers + report fields (policy R3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the ledger half of docs/api-error-policy.md. - create_entities -> {created, existing}; create_relations -> {created, skippedDuplicates}; add_observations gains alreadyPresent - deletes return {deleted, notFound}: entities by name; relations with reason entity|relation; observations with reason entity|observation - open_nodes carries missing[] for partial misses; sequentialthinking returns linkedTo - random_walk reports modeUsed (merw | uniform | merw+fallback) and fallbackSteps — the C walk now counts merw-requested steps that fell back to uniform because psi weighting was unavailable (honest labeling, cf. D_NoUsageClaimWithoutTheRealThing) - kb_load returns per-stage counts: entities {created, existing}, relations {created, skippedDuplicates} - tests: new ledger block (9 cases) + shape updates; suite 192/192 Co-Authored-By: Claude Code --- docs/api-error-policy.md | 22 +++-- native/graph.c | 6 +- native/graph.h | 7 +- native/graphbind.c | 8 +- native/op_bench.c | 2 +- server.ts | 124 +++++++++++++++--------- src/store.ts | 4 +- tests/memory-server.test.ts | 181 ++++++++++++++++++++++++++++++++---- tests/test-utils.ts | 2 + 9 files changed, 279 insertions(+), 77 deletions(-) diff --git a/docs/api-error-policy.md b/docs/api-error-policy.md index 72eec41..c4975d9 100644 --- a/docs/api-error-policy.md +++ b/docs/api-error-policy.md @@ -63,12 +63,12 @@ txid once the owner daemon lands). A stale/mismatched cursor returns a | `add_observations` | unknown entity → `ENTITY_NOT_FOUND`; limit breaches → `LIMIT_EXCEEDED` (atomic, all offenders) | per-entity `{entityName, addedObservations, alreadyPresent}` | | `delete_entities` / `delete_relations` / `delete_observations` | nothing errors — absence is `notFound` (R4) | `{deleted, notFound}` ledgers | | `sequentialthinking` | unknown `previousCtxId` → `ENTITY_NOT_FOUND` (chain integrity); limits → `LIMIT_EXCEEDED` | `{ctxId, linkedTo}` | -| `kb_load` | bad extension / unreadable file → `INVALID_FILE`; entity & relation stages per their rows | per-stage ledger (document, stats, entity/relation outcomes) | +| `kb_load` | bad extension / unreadable file → `INVALID_FILE`; entity & relation stages per their rows | counts per stage: `entities {created, existing}`, `relations {created, skippedDuplicates}` | | `open_nodes` | ALL requested names missing → `ENTITY_NOT_FOUND` | graph result + `missing[]` for partial misses | | `get_neighbors` | unknown start → `ENTITY_NOT_FOUND` | neighbors (depth 0 = immediate) | | `search_nodes` | invalid ERE → `INVALID_REGEX`; literal query, zero matches → `NO_MATCHES` | paginated graph | | `get_entities_by_type` | type absent from the data-derived type set → `TYPE_NOT_FOUND` | entities of that type | -| `random_walk` | unknown start → `ENTITY_NOT_FOUND` | `{entity, path, modeUsed, stopReason}` | +| `random_walk` | unknown start → `ENTITY_NOT_FOUND` | `{entity, path, modeUsed, fallbackSteps}` (`modeUsed` = `merw` \| `uniform` \| `merw+fallback`) | | `find_path` | — (β-contract already reports `targetReached` / `budgetExhausted` / `note`) | unchanged — the reference pattern for R1-style reporting | | reads (`get_entity_types`, `get_relation_types`, `get_stats`, `get_orphaned_entities`, `validate_graph`, `decode_timestamp`) | unchanged | unchanged | @@ -78,12 +78,16 @@ faults; Q1 = atomic-fail on missing endpoints; Q2/Q3 = error. ## Status -- **This change (envelope PR):** R1 (envelope + codes + dispatch conversion), - R2 (validate-first atomicity for `create_entities`, `create_relations`, - `add_observations`, `sequentialthinking`), R4 (drops→errors: - `ENDPOINT_MISSING`, all-missing `open_nodes`, `TYPE_NOT_FOUND`, unknown - starts), `INVALID_FILE`, `INVALID_REGEX`, `NO_MATCHES` envelope. -- **Ledger PR (next):** R3 result ledgers + report fields (`missing[]`, - `modeUsed`, `alreadyPresent`), R5 cursor fingerprinting. +- **Envelope PR (#247, merged):** R1 (envelope + codes + dispatch + conversion), R2 (validate-first atomicity for `create_entities`, + `create_relations`, `add_observations`, `sequentialthinking`), R4 + (drops→errors: `ENDPOINT_MISSING`, all-missing `open_nodes`, + `TYPE_NOT_FOUND`, unknown starts), `INVALID_FILE`, `INVALID_REGEX`, + `NO_MATCHES` envelope. +- **Ledger PR:** R3 result ledgers + report fields + (`created`/`existing`/`skippedDuplicates`/`notFound`/`alreadyPresent`, + `open_nodes.missing[]`, `random_walk.modeUsed`/`fallbackSteps`, + `sequentialthinking.linkedTo`). +- **Next:** R5 cursor fingerprinting (`CURSOR_STALE`). - **v4 line (`feat/libsegstore`):** inherits this policy at reconciliation; kbd4's batch=txn=commit model is the natural home for R2 atomicity. diff --git a/native/graph.c b/native/graph.c index bcea550..c3fa687 100644 --- a/native/graph.c +++ b/native/graph.c @@ -866,9 +866,11 @@ u32 graph_compute_merw_psi(graph_t *g, double alpha, u32 max_iter, double tol) { } u32 graph_random_walk(graph_t *g, u64 start, u32 depth, u32 direction, int merw_mode, - u64 seed, int avoid_cycles, u64 *out_path, u32 max_path) { + u64 seed, int avoid_cycles, u64 *out_path, u32 max_path, + u32 *out_uniform_steps) { u64 st = seed ? seed : g_rng; u32 plen = 0; + u32 uniform_steps = 0; if (max_path >= 1) out_path[plen] = start; plen = 1; u64 cur = start; @@ -905,6 +907,7 @@ u32 graph_random_walk(graph_t *g, u64 start, u32 depth, u32 direction, int merw_ double r = rng_d(&st) * total_psi, cum = 0; chosen = cand[nc - 1]; for (u32 j = 0; j < nc; j++) { cum += cpsi[j]; if (r <= cum) { chosen = cand[j]; break; } } } else { + if (merw_mode) uniform_steps++; /* psi unavailable at this step: fallback */ u32 ix = (u32)(rng_d(&st) * nc); if (ix >= nc) ix = nc - 1; chosen = cand[ix]; } free(cand); free(cpsi); @@ -913,6 +916,7 @@ u32 graph_random_walk(graph_t *g, u64 start, u32 depth, u32 direction, int merw_ plen++; } if (!seed) g_rng = st; + if (out_uniform_steps) *out_uniform_steps = uniform_steps; return plen; } diff --git a/native/graph.h b/native/graph.h index 6521fd9..5b99add 100644 --- a/native/graph.h +++ b/native/graph.h @@ -134,8 +134,11 @@ u32 graph_structural_sample(graph_t *g, u32 iterations, double damping); /* u32 graph_compute_merw_psi(graph_t *g, double alpha, u32 max_iter, double tol); /* iters run */ /* random walk; mode: 1=merw (weighted by psi), 0=uniform; seed 0 = use global rng. * avoid_cycles: 1 = self-avoiding (never revisits a node; stops early when every - * neighbor is already on the path). Returns path node count. */ + * neighbor is already on the path). Returns path node count. + * out_uniform_steps (optional): number of merw-requested steps that fell back to + * uniform sampling because psi weighting was unavailable at that step. */ u32 graph_random_walk(graph_t *g, u64 start, u32 depth, u32 direction, int merw_mode, - u64 seed, int avoid_cycles, u64 *out_path, u32 max_path); + u64 seed, int avoid_cycles, u64 *out_path, u32 max_path, + u32 *out_uniform_steps); #endif /* GRAPH_H */ diff --git a/native/graphbind.c b/native/graphbind.c index 25ddbc7..0fe492d 100644 --- a/native/graphbind.c +++ b/native/graphbind.c @@ -244,8 +244,12 @@ static napi_value n_merw(napi_env env, napi_callback_info info) { ARG static napi_value n_seed(napi_env env, napi_callback_info info) { ARGS(2); (void)unwrap(env, argv[0]); graph_seed_rng(getU64(env, argv[1])); return NULL; } static napi_value n_random_walk(napi_env env, napi_callback_info info) { ARGS(7); STORE; u32 depth = getU32(env, argv[2]); u32 cap = depth + 1; u64 *out = malloc((size_t)cap * 8); - u32 n = graph_random_walk(s->g, getU64(env, argv[1]), depth, getU32(env, argv[3]), getU32(env, argv[4]), getU64(env, argv[5]), getU32(env, argv[6]), out, cap); - napi_value r = u64arr(env, out, n < cap ? n : cap); free(out); return r; + u32 uniform_steps = 0; + u32 n = graph_random_walk(s->g, getU64(env, argv[1]), depth, getU32(env, argv[3]), getU32(env, argv[4]), getU64(env, argv[5]), getU32(env, argv[6]), out, cap, &uniform_steps); + napi_value r; napi_create_object(env, &r); + napi_set_named_property(env, r, "path", u64arr(env, out, n < cap ? n : cap)); + napi_set_named_property(env, r, "uniformSteps", mkU32(env, uniform_steps)); + free(out); return r; } /* ---- validate ---- */ diff --git a/native/op_bench.c b/native/op_bench.c index 04668bb..f446142 100644 --- a/native/op_bench.c +++ b/native/op_bench.c @@ -166,7 +166,7 @@ int main(int argc, char **argv) { ADAPT("find_path_d6", sink32 = graph_find_path(g, off[idx[_k]], off[idx2[_k]], 6, DIR_ANY, out, cap)); ADAPT("search", sink32 = graph_search(g, pat[_k], out, cap)); ADAPT("entities_by_type", sink32 = graph_entities_by_type(g, (const u8 *)tynames[tpix[_k]], tylen[tpix[_k]], out, cap)); - ADAPT("random_walk_d5", sink32 = graph_random_walk(g, off[idx[_k]], 5, DIR_ANY, 1, seed[_k], 0, out, 8)); + ADAPT("random_walk_d5", sink32 = graph_random_walk(g, off[idx[_k]], 5, DIR_ANY, 1, seed[_k], 0, out, 8, NULL)); ADAPT("inc_walker_visit", graph_inc_walker_visit(g, off[idx[_k]])); ADAPT("structural_sample", graph_structural_sample(g, 1, 0.85)); ADAPT("compute_merw_psi", graph_compute_merw_psi(g, 0.85, 100, 1e-8)); diff --git a/server.ts b/server.ts index e2731a0..4e0e903 100644 --- a/server.ts +++ b/server.ts @@ -614,7 +614,7 @@ export class KnowledgeGraphManager { }; } - async createEntities(entities: Entity[]): Promise { + async createEntities(entities: Entity[]): Promise<{ created: Entity[]; existing: string[] }> { // Validate observation limits across the WHOLE batch first, collecting // every violation rather than stopping at the first (R2: validate-first; // the caller gets all offenders in one round-trip). @@ -651,6 +651,7 @@ export class KnowledgeGraphManager { // Pass 1 — classify every entity before writing anything. A collision // fails the whole batch (R2: atomic) and lists every offender. const toCreate: Entity[] = []; + const existingNames: string[] = []; const collisions: { entity: string; code: ToolErrorCode; message: string }[] = []; for (const e of entities) { const existingOffset = this.db.lookup(e.name); @@ -659,7 +660,7 @@ export class KnowledgeGraphManager { const sameType = existing.entityType === e.entityType; const sameObs = existing.observations.length === e.observations.length && existing.observations.every((o, i) => o === e.observations[i]); - if (sameType && sameObs) continue; // exact duplicate: intent satisfied + if (sameType && sameObs) { existingNames.push(e.name); continue; } // exact duplicate: intent satisfied (R3 ledger) collisions.push({ entity: e.name, code: 'COLLISION', @@ -693,11 +694,11 @@ export class KnowledgeGraphManager { newEntities.push(newEntity); } - return newEntities; + return { created: newEntities, existing: existingNames }; }); } - async createRelations(relations: Relation[]): Promise { + async createRelations(relations: Relation[]): Promise<{ created: Relation[]; skippedDuplicates: Relation[] }> { return this.withWriteLock(() => { const now = BigInt(Date.now()); @@ -731,6 +732,7 @@ export class KnowledgeGraphManager { // Pass 2 — create. Duplicates are already-satisfied intent and are // skipped (they become part of the result ledger under policy R3). const newRelations: Relation[] = []; + const skippedDuplicates: Relation[] = []; for (const r of relations) { const fromOffset = this.db.lookup(r.from); const toOffset = this.db.lookup(r.to); @@ -738,23 +740,23 @@ export class KnowledgeGraphManager { const isDuplicate = this.db.edges(fromOffset).some(e => e.direction === DIR_FORWARD && e.target === toOffset && e.relType === r.relationType ); - if (isDuplicate) continue; + if (isDuplicate) { skippedDuplicates.push(r); continue; } // C owns the bidirectional edges + relType interning/refcounts. this.db.createRelation(fromOffset, toOffset, r.relationType, now); newRelations.push({ ...r, mtime: Number(now) }); } - return newRelations; + return { created: newRelations, skippedDuplicates }; }); } - async addObservations(observations: { entityName: string; contents: string[] }[]): Promise<{ entityName: string; addedObservations: string[] }[]> { + async addObservations(observations: { entityName: string; contents: string[] }[]): Promise<{ entityName: string; addedObservations: string[]; alreadyPresent: string[] }[]> { return this.withWriteLock(() => { // Pass 1 — validate the WHOLE batch before touching state (R2: atomic; // the old code validated inside the write loop, so a late failure left // earlier items applied). Collect every violation for one round-trip. - const pending: { entityName: string; offset: bigint; newObservations: string[] }[] = []; + const pending: { entityName: string; offset: bigint; newObservations: string[]; alreadyPresent: string[] }[] = []; const violations: { entityName: string; code: ToolErrorCode; message: string }[] = []; for (const o of observations) { @@ -776,6 +778,7 @@ export class KnowledgeGraphManager { const existingObs = this.db.readEntity(offset).observations; const newObservations = o.contents.filter(content => !existingObs.includes(content)); + const alreadyPresent = o.contents.filter(content => existingObs.includes(content)); if (existingObs.length + newObservations.length > 2) { violations.push({ entityName: o.entityName, @@ -783,7 +786,7 @@ export class KnowledgeGraphManager { message: `adding ${newObservations.length} observation(s) would give ${existingObs.length + newObservations.length} total; maximum is 2`, }); } - pending.push({ entityName: o.entityName, offset, newObservations }); + pending.push({ entityName: o.entityName, offset, newObservations, alreadyPresent }); } if (violations.length > 0) { @@ -797,53 +800,78 @@ export class KnowledgeGraphManager { // Pass 2 — apply. const now = BigInt(Date.now()); - const results: { entityName: string; addedObservations: string[] }[] = []; + const results: { entityName: string; addedObservations: string[]; alreadyPresent: string[] }[] = []; for (const p of pending) { for (const obs of p.newObservations) { this.db.addObservation(p.offset, obs, now); } - results.push({ entityName: p.entityName, addedObservations: p.newObservations }); + results.push({ entityName: p.entityName, addedObservations: p.newObservations, alreadyPresent: p.alreadyPresent }); } return results; }); } - async deleteEntities(entityNames: string[]): Promise { - this.withWriteLock(() => { + async deleteEntities(entityNames: string[]): Promise<{ deleted: string[]; notFound: string[] }> { + return this.withWriteLock(() => { + const deleted: string[] = []; + const notFound: string[] = []; for (const name of entityNames) { const offset = this.db.lookup(name); - if (offset === 0n) continue; + if (offset === 0n) { notFound.push(name); continue; } // C deletes the record + adjacency, drops mirror edges, and releases // every string ref (name/type/obs + relType per edge + mirror). this.db.deleteEntity(offset); + deleted.push(name); } + return { deleted, notFound }; }); } - async deleteObservations(deletions: { entityName: string; observations: string[] }[]): Promise { - this.withWriteLock(() => { + async deleteObservations(deletions: { entityName: string; observations: string[] }[]): Promise<{ + deleted: { entityName: string; observations: string[] }[]; + notFound: { entityName: string; observation?: string; reason: 'entity' | 'observation' }[]; + }> { + return this.withWriteLock(() => { const now = BigInt(Date.now()); + const deleted: { entityName: string; observations: string[] }[] = []; + const notFound: { entityName: string; observation?: string; reason: 'entity' | 'observation' }[] = []; for (const d of deletions) { const offset = this.db.lookup(d.entityName); - if (offset === 0n) continue; - + if (offset === 0n) { notFound.push({ entityName: d.entityName, reason: 'entity' }); continue; } + const existing = this.db.readEntity(offset).observations; + const removed: string[] = []; for (const obs of d.observations) { + if (!existing.includes(obs)) { notFound.push({ entityName: d.entityName, observation: obs, reason: 'observation' }); continue; } this.db.removeObservation(offset, obs, now); + removed.push(obs); } + if (removed.length > 0) deleted.push({ entityName: d.entityName, observations: removed }); } + return { deleted, notFound }; }); } - async deleteRelations(relations: Relation[]): Promise { - this.withWriteLock(() => { + async deleteRelations(relations: Relation[]): Promise<{ deleted: Relation[]; notFound: { from: string; to: string; relationType: string; reason: 'entity' | 'relation' }[] }> { + return this.withWriteLock(() => { + const deleted: Relation[] = []; + const notFound: { from: string; to: string; relationType: string; reason: 'entity' | 'relation' }[] = []; for (const r of relations) { const fromOffset = this.db.lookup(r.from); const toOffset = this.db.lookup(r.to); - if (fromOffset === 0n || toOffset === 0n) continue; + if (fromOffset === 0n || toOffset === 0n) { + notFound.push({ from: r.from, to: r.to, relationType: r.relationType, reason: 'entity' }); + continue; + } + const exists = this.db.edges(fromOffset).some(e => + e.direction === DIR_FORWARD && e.target === toOffset && e.relType === r.relationType + ); + if (!exists) { notFound.push({ from: r.from, to: r.to, relationType: r.relationType, reason: 'relation' }); continue; } // C removes both directed edges and releases the two relType refs. this.db.deleteRelation(fromOffset, toOffset, r.relationType); + deleted.push(r); } + return { deleted, notFound }; }); } @@ -903,13 +931,14 @@ export class KnowledgeGraphManager { ); } - async openNodes(names: string[], direction: 'forward' | 'backward' | 'any' = 'forward'): Promise { + async openNodes(names: string[], direction: 'forward' | 'backward' | 'any' = 'forward'): Promise { return this.withReadLock(() => { const filteredEntities: Entity[] = []; + const missing: string[] = []; const offsetByName = new Map(); for (const name of names) { const offset = this.db.lookup(name); - if (offset === 0n) continue; + if (offset === 0n) { missing.push(name); continue; } filteredEntities.push(this.recordToEntity(this.db.readEntity(offset))); offsetByName.set(name, offset); } @@ -952,7 +981,7 @@ export class KnowledgeGraphManager { } } - return { entities: filteredEntities, relations: filteredRelations }; + return { entities: filteredEntities, relations: filteredRelations, missing }; }); } @@ -1269,7 +1298,7 @@ export class KnowledgeGraphManager { direction: 'forward' | 'backward' | 'any' = 'forward', mode: RandomWalkMode = 'merw', avoidCycles: boolean = false, - ): Promise<{ entity: string; path: string[] }> { + ): Promise<{ entity: string; path: string[]; modeUsed: 'merw' | 'uniform' | 'merw+fallback'; fallbackSteps: number }> { return traced( 'kb.random_walk', { @@ -1289,12 +1318,18 @@ export class KnowledgeGraphManager { // seed of 0 means "use the global RNG" (unseeded), so hashSeed (never // 0) keeps seeded walks reproducible. const seedU64 = seed !== undefined ? BigInt(this.hashSeed(seed) >>> 0) : 0n; - const pathOffsets = this.db.randomWalk(startOffset, depth, direction, mode === 'merw', seedU64, avoidCycles); - const pathNames = pathOffsets.map(o => this.db.entityName(o)); + const walk = this.db.randomWalk(startOffset, depth, direction, mode === 'merw', seedU64, avoidCycles); + const pathNames = walk.path.map(o => this.db.entityName(o)); span.setAttribute('kb.walker.steps_taken', pathNames.length - 1); span.setAttribute('kb.walker.truncated', pathNames.length - 1 < depth); - return { entity: pathNames[pathNames.length - 1], path: pathNames }; + span.setAttribute('kb.walker.fallback_steps', walk.uniformSteps); + // R3 report (docs/api-error-policy.md): which policy actually ran. + // 'merw+fallback' = psi weighting was unavailable at some steps, so + // those steps sampled uniformly instead. + const modeUsed: 'merw' | 'uniform' | 'merw+fallback' = + mode === 'uniform' ? 'uniform' : (walk.uniformSteps === 0 ? 'merw' : 'merw+fallback'); + return { entity: pathNames[pathNames.length - 1], path: pathNames, modeUsed, fallbackSteps: walk.uniformSteps }; }), ); } @@ -1348,7 +1383,7 @@ export class KnowledgeGraphManager { return result; } - async addThought(observations: string[], previousCtxId?: string): Promise<{ ctxId: string }> { + async addThought(observations: string[], previousCtxId?: string): Promise<{ ctxId: string; linkedTo: string | null }> { // Validate observation limits across the whole request first. const violations: { code: ToolErrorCode; message: string }[] = []; if (observations.length > 2) { @@ -1390,7 +1425,7 @@ export class KnowledgeGraphManager { this.db.createRelation(offset, prevOffset, 'preceded_by', now); } - return { ctxId }; + return { ctxId, linkedTo: previousCtxId ?? null }; }); } @@ -1808,17 +1843,20 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p } case "add_observations": return { content: [{ type: "text", text: JSON.stringify(await knowledgeGraphManager.addObservations(args.observations as { entityName: string; contents: string[] }[]), null, 2) }] }; - case "delete_entities": - await knowledgeGraphManager.deleteEntities(args.entityNames as string[]); + case "delete_entities": { + const result = await knowledgeGraphManager.deleteEntities(args.entityNames as string[]); knowledgeGraphManager.resample(); // Re-run structural sampling after graph mutation - return { content: [{ type: "text", text: "Entities deleted successfully" }] }; - case "delete_observations": - await knowledgeGraphManager.deleteObservations(args.deletions as { entityName: string; observations: string[] }[]); - return { content: [{ type: "text", text: "Observations deleted successfully" }] }; - case "delete_relations": - await knowledgeGraphManager.deleteRelations(args.relations as Relation[]); + return { content: [{ type: "text", text: JSON.stringify(result, null, 2) }] }; + } + case "delete_observations": { + const result = await knowledgeGraphManager.deleteObservations(args.deletions as { entityName: string; observations: string[] }[]); + return { content: [{ type: "text", text: JSON.stringify(result, null, 2) }] }; + } + case "delete_relations": { + const result = await knowledgeGraphManager.deleteRelations(args.relations as Relation[]); knowledgeGraphManager.resample(); // Re-run structural sampling after graph mutation - return { content: [{ type: "text", text: "Relations deleted successfully" }] }; + return { content: [{ type: "text", text: JSON.stringify(result, null, 2) }] }; + } case "search_nodes": { const query = args.query as string; const graph = await knowledgeGraphManager.searchNodes( @@ -1860,7 +1898,9 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p const graph = await knowledgeGraphManager.openNodes(args.names as string[], (args.direction as 'forward' | 'backward' | 'any') ?? 'forward'); // Record walker visits for opened nodes knowledgeGraphManager.recordWalkerVisits(graph.entities.map(e => e.name)); - return { content: [{ type: "text", text: JSON.stringify(paginateGraph(graph, args.entityCursor as number ?? 0, args.relationCursor as number ?? 0)) }] }; + // R3: report partial misses alongside the (paginated) graph. + const paginated = paginateGraph(graph, args.entityCursor as number ?? 0, args.relationCursor as number ?? 0); + return { content: [{ type: "text", text: JSON.stringify({ ...paginated, missing: graph.missing }) }] }; } case "get_neighbors": { const neighbors = await knowledgeGraphManager.getNeighbors(args.entityName as string, args.depth as number ?? 0, args.sortBy as EntitySortField | undefined, args.sortDir as SortDirection | undefined, (args.direction as 'forward' | 'backward' | 'any') ?? 'forward'); @@ -1997,8 +2037,8 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p text: JSON.stringify({ document: title, stats: loadResult.stats, - entitiesCreated: entities.length, - relationsCreated: relations.length, + entities: { created: entities.created.length, existing: entities.existing.length }, + relations: { created: relations.created.length, skippedDuplicates: relations.skippedDuplicates.length }, }, null, 2), }], }; diff --git a/src/store.ts b/src/store.ts index 4dd0076..b7bd15a 100644 --- a/src/store.ts +++ b/src/store.ts @@ -82,7 +82,7 @@ interface NativeStore { structuralSample(h: unknown, iterations: number, damping: number): number; computeMerwPsi(h: unknown, alpha: number, maxIter: number, tol: number): number; seedRng(h: unknown, seed: bigint): void; - randomWalk(h: unknown, start: bigint, depth: number, direction: number, merwMode: number, seed: bigint, avoidCycles: number): bigint[]; + randomWalk(h: unknown, start: bigint, depth: number, direction: number, merwMode: number, seed: bigint, avoidCycles: number): { path: bigint[]; uniformSteps: number }; validateObs(h: unknown): { offset: bigint; count: number; oversize: number }[]; validateDangling(h: unknown): { src: bigint; target: bigint }[]; setEntityFields(h: unknown, off: bigint, mtime: bigint, obsMtime: bigint, structuralVisits: bigint, walkerVisits: bigint, psi: number): void; @@ -171,7 +171,7 @@ export class Store { structuralSample(iterations: number, damping: number): number { return native.structuralSample(this.h, iterations, damping); } computeMerwPsi(alpha: number, maxIter: number, tol: number): number { return native.computeMerwPsi(this.h, alpha, maxIter, tol); } seedRng(seed: bigint): void { native.seedRng(this.h, seed); } - randomWalk(start: bigint, depth: number, direction: Direction, merwMode: boolean, seed: bigint, avoidCycles: boolean): bigint[] { + randomWalk(start: bigint, depth: number, direction: Direction, merwMode: boolean, seed: bigint, avoidCycles: boolean): { path: bigint[]; uniformSteps: number } { return native.randomWalk(this.h, start, depth, dirCode(direction), merwMode ? 1 : 0, seed, avoidCycles ? 1 : 0); } diff --git a/tests/memory-server.test.ts b/tests/memory-server.test.ts index 52ff6b5..8b59035 100644 --- a/tests/memory-server.test.ts +++ b/tests/memory-server.test.ts @@ -39,11 +39,12 @@ describe('MCP Memory Server E2E Tests', () => { { name: 'Alice', entityType: 'Person', observations: ['Likes coding'] }, { name: 'Bob', entityType: 'Person', observations: ['Likes music'] } ] - }) as Entity[]; + }) as { created: Entity[]; existing: string[] }; - expect(result).toHaveLength(2); - expect(result[0].name).toBe('Alice'); - expect(result[1].name).toBe('Bob'); + expect(result.created).toHaveLength(2); + expect(result.created[0].name).toBe('Alice'); + expect(result.created[1].name).toBe('Bob'); + expect(result.existing).toEqual([]); }); it('should not duplicate existing entities', async () => { @@ -51,17 +52,16 @@ describe('MCP Memory Server E2E Tests', () => { entities: [{ name: 'Alice', entityType: 'Person', observations: ['First'] }] }); - // Exact same entity should be silently skipped + // Exact same entity is reported in `existing`, not created again (R3). const result = await callTool(client, 'create_entities', { entities: [ { name: 'Alice', entityType: 'Person', observations: ['First'] }, { name: 'Bob', entityType: 'Person', observations: ['New'] } ] - }) as Entity[]; + }) as { created: Entity[]; existing: string[] }; - // Only Bob should be returned as new - expect(result).toHaveLength(1); - expect(result[0].name).toBe('Bob'); + expect(result.created.map(e => e.name)).toEqual(['Bob']); + expect(result.existing).toEqual(['Alice']); }); it('should error on duplicate name with different data (COLLISION, visible)', async () => { @@ -201,9 +201,10 @@ describe('MCP Memory Server E2E Tests', () => { { from: 'A', to: 'B', relationType: 'connects' }, { from: 'B', to: 'C', relationType: 'connects' } ] - }) as Relation[]; + }) as { created: Relation[]; skippedDuplicates: Relation[] }; - expect(result).toHaveLength(2); + expect(result.created).toHaveLength(2); + expect(result.skippedDuplicates).toEqual([]); }); it('should not duplicate relations', async () => { @@ -216,10 +217,12 @@ describe('MCP Memory Server E2E Tests', () => { { from: 'A', to: 'B', relationType: 'connects' }, { from: 'A', to: 'C', relationType: 'connects' } ] - }) as Relation[]; + }) as { created: Relation[]; skippedDuplicates: Relation[] }; - expect(result).toHaveLength(1); - expect(result[0].to).toBe('C'); + expect(result.created).toHaveLength(1); + expect(result.created[0].to).toBe('C'); + expect(result.skippedDuplicates).toHaveLength(1); + expect(result.skippedDuplicates[0].to).toBe('B'); }); it('should delete relations', async () => { @@ -2002,8 +2005,8 @@ describe('MCP Memory Server E2E Tests', () => { const result = await callTool(client, 'kb_load', { filePath: docFile }) as any; expect(result.document).toBe('test-doc'); - expect(result.entitiesCreated).toBeGreaterThan(0); - expect(result.relationsCreated).toBeGreaterThan(0); + expect(result.entities.created).toBeGreaterThan(0); + expect(result.relations.created).toBeGreaterThan(0); expect(result.stats.chunks).toBeGreaterThan(0); expect(result.stats.sentences).toBeGreaterThan(0); }); @@ -2111,7 +2114,7 @@ describe('MCP Memory Server E2E Tests', () => { await fs.writeFile(docFile, `${longWord} is a very long word that tests our splitting logic handles edge cases.`); const result = await callTool(client, 'kb_load', { filePath: docFile }) as any; - expect(result.entitiesCreated).toBeGreaterThan(0); + expect(result.entities.created).toBeGreaterThan(0); // All observations should be within limits const chunks = await callTool(client, 'get_entities_by_type', { entityType: 'TextChunk' }) as PaginatedResult; @@ -2133,7 +2136,7 @@ describe('MCP Memory Server E2E Tests', () => { filePath, title: `ext-test-${ext.slice(1)}`, }) as any; - expect(result.entitiesCreated).toBeGreaterThan(0); + expect(result.entities.created).toBeGreaterThan(0); } }); }); @@ -2248,4 +2251,146 @@ describe('MCP Memory Server E2E Tests', () => { expect(err.message).toContain('NoSuchEntity'); }); }); + + describe('API ledgers (R3)', () => { + it('create_entities reports exact duplicates in existing', async () => { + await callTool(client, 'create_entities', { + entities: [{ name: 'LedgerA', entityType: 'LedgerTag', observations: ['one'] }], + }); + + const result = await callTool(client, 'create_entities', { + entities: [ + { name: 'LedgerA', entityType: 'LedgerTag', observations: ['one'] }, // exact dupe + { name: 'LedgerB', entityType: 'LedgerTag', observations: ['two'] }, + ], + }) as { created: Entity[]; existing: string[] }; + + expect(result.created.map(e => e.name)).toEqual(['LedgerB']); + expect(result.existing).toEqual(['LedgerA']); + }); + + it('create_relations reports duplicates in skippedDuplicates', async () => { + await callTool(client, 'create_entities', { + entities: [ + { name: 'LedgerC', entityType: 'LedgerTag', observations: [] }, + { name: 'LedgerD', entityType: 'LedgerTag', observations: [] }, + ], + }); + await callTool(client, 'create_relations', { + relations: [{ from: 'LedgerC', to: 'LedgerD', relationType: 'LEDGES' }], + }); + + const result = await callTool(client, 'create_relations', { + relations: [ + { from: 'LedgerC', to: 'LedgerD', relationType: 'LEDGES' }, // dupe + { from: 'LedgerD', to: 'LedgerC', relationType: 'LEDGES' }, // new (reverse direction) + ], + }) as { created: Array<{ from: string; to: string }>; skippedDuplicates: Array<{ from: string; to: string }> }; + + expect(result.created).toHaveLength(1); + expect(result.created[0]).toMatchObject({ from: 'LedgerD', to: 'LedgerC' }); + expect(result.skippedDuplicates).toHaveLength(1); + expect(result.skippedDuplicates[0]).toMatchObject({ from: 'LedgerC', to: 'LedgerD' }); + }); + + it('add_observations reports alreadyPresent', async () => { + await callTool(client, 'create_entities', { + entities: [{ name: 'LedgerE', entityType: 'LedgerTag', observations: ['seed'] }], + }); + + const result = await callTool(client, 'add_observations', { + observations: [{ entityName: 'LedgerE', contents: ['seed', 'fresh'] }], + }) as Array<{ entityName: string; addedObservations: string[]; alreadyPresent: string[] }>; + + expect(result[0].addedObservations).toEqual(['fresh']); + expect(result[0].alreadyPresent).toEqual(['seed']); + }); + + it('delete_entities reports deleted + notFound', async () => { + await callTool(client, 'create_entities', { + entities: [{ name: 'LedgerF', entityType: 'LedgerTag', observations: [] }], + }); + + const result = await callTool(client, 'delete_entities', { + entityNames: ['LedgerF', 'LedgerGhost'], + }) as { deleted: string[]; notFound: string[] }; + + expect(result.deleted).toEqual(['LedgerF']); + expect(result.notFound).toEqual(['LedgerGhost']); + }); + + it('delete_relations distinguishes missing entity vs missing relation', async () => { + await callTool(client, 'create_entities', { + entities: [ + { name: 'LedgerG', entityType: 'LedgerTag', observations: [] }, + { name: 'LedgerH', entityType: 'LedgerTag', observations: [] }, + ], + }); + + const result = await callTool(client, 'delete_relations', { + relations: [ + { from: 'LedgerG', to: 'LedgerH', relationType: 'LEDGES' }, // entity exists, relation never created + { from: 'LedgerG', to: 'LedgerGhost', relationType: 'LEDGES' }, // entity missing + ], + }) as { deleted: unknown[]; notFound: Array<{ reason: string }> }; + + expect(result.deleted).toEqual([]); + expect(result.notFound.map(n => n.reason).sort()).toEqual(['entity', 'relation']); + }); + + it('delete_observations distinguishes entity vs observation', async () => { + await callTool(client, 'create_entities', { + entities: [{ name: 'LedgerI', entityType: 'LedgerTag', observations: ['keep'] }], + }); + + const result = await callTool(client, 'delete_observations', { + deletions: [ + { entityName: 'LedgerI', observations: ['keep', 'never-added'] }, + { entityName: 'LedgerGhost', observations: ['x'] }, + ], + }) as { deleted: Array<{ entityName: string; observations: string[] }>; notFound: Array<{ reason: string }> }; + + expect(result.deleted).toEqual([{ entityName: 'LedgerI', observations: ['keep'] }]); + expect(result.notFound.map(n => n.reason).sort()).toEqual(['entity', 'observation']); + }); + + it('open_nodes reports partial misses in missing[]', async () => { + await callTool(client, 'create_entities', { + entities: [{ name: 'LedgerJ', entityType: 'LedgerTag', observations: [] }], + }); + + const result = await callTool(client, 'open_nodes', { names: ['LedgerJ', 'LedgerGhost'] }) as PaginatedGraph; + + expect(result.entities.items.map(e => e.name)).toEqual(['LedgerJ']); + expect(result.missing).toEqual(['LedgerGhost']); + }); + + it('random_walk reports modeUsed and fallbackSteps', async () => { + await callTool(client, 'create_entities', { + entities: [ + { name: 'LedgerK', entityType: 'LedgerTag', observations: [] }, + { name: 'LedgerL', entityType: 'LedgerTag', observations: [] }, + ], + }); + await callTool(client, 'create_relations', { + relations: [{ from: 'LedgerK', to: 'LedgerL', relationType: 'LEDGES' }], + }); + + const merw = await callTool(client, 'random_walk', { start: 'LedgerK', depth: 1 }) as { modeUsed: string; fallbackSteps: number }; + expect(['merw', 'merw+fallback']).toContain(merw.modeUsed); + expect(merw.modeUsed === 'merw').toBe(merw.fallbackSteps === 0); + + const uni = await callTool(client, 'random_walk', { start: 'LedgerK', depth: 1, mode: 'uniform' }) as { modeUsed: string; fallbackSteps: number }; + expect(uni.modeUsed).toBe('uniform'); + expect(uni.fallbackSteps).toBe(0); + }); + + it('sequentialthinking returns linkedTo', async () => { + const first = await callTool(client, 'sequentialthinking', { observations: ['ledger thought 1'] }) as { ctxId: string; linkedTo: string | null }; + expect(first.linkedTo).toBeNull(); + + const second = await callTool(client, 'sequentialthinking', { observations: ['ledger thought 2'], previousCtxId: first.ctxId }) as { ctxId: string; linkedTo: string | null }; + expect(second.linkedTo).toBe(first.ctxId); + }); + }); }); diff --git a/tests/test-utils.ts b/tests/test-utils.ts index ea99ebe..745bd4d 100644 --- a/tests/test-utils.ts +++ b/tests/test-utils.ts @@ -14,6 +14,8 @@ export interface PaginatedResult { export interface PaginatedGraph { entities: PaginatedResult; relations: PaginatedResult; + /** open_nodes only: requested names that do not exist (partial misses). */ + missing?: string[]; } /** From 4a0cd0a76bb8cfab4eba2ef142d234acf7fbb591 Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Thu, 8 Oct 2026 03:03:07 -0400 Subject: [PATCH 7/8] =?UTF-8?q?feat(api):=20cursor=20fingerprinting=20?= =?UTF-8?q?=E2=80=94=20CURSOR=5FSTALE=20(policy=20R5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursors become ":" (computed over the serialized bytes pagination cuts on). A page request whose fingerprint no longer matches the current set errors with CURSOR_STALE instead of serving a shifted page; malformed cursors are rejected; bare-number cursors are accepted for legacy callers (unchecked). Prerequisite shipped in the same change: all sort tie-breaks were Math.random(), which made any multi-page drain of a default-sorted list nondeterministic. Tie-breaks are now deterministic (name), giving result sets a stable total order (cf. Determinacy). - paginateItems: cursor parse + fingerprint verify; nextCursor: string|null - schemas: cursor fields accept number|string, described as opaque - tests: 4 new R5 cases (stale after mutation, clean drain via string cursors, malformed rejected, legacy numbers) - full suite 196/196; fuzz clean Co-Authored-By: Claude Code --- docs/api-error-policy.md | 13 ++-- server.ts | 127 ++++++++++++++++++++++++++---------- tests/memory-server.test.ts | 78 ++++++++++++++++++++-- tests/test-utils.ts | 2 +- 4 files changed, 176 insertions(+), 44 deletions(-) diff --git a/docs/api-error-policy.md b/docs/api-error-policy.md index c4975d9..6250e75 100644 --- a/docs/api-error-policy.md +++ b/docs/api-error-policy.md @@ -31,8 +31,9 @@ Every failure the caller can act on is returned as a tool-level error: Stable codes: `ENTITY_NOT_FOUND`, `TYPE_NOT_FOUND`, `ENDPOINT_MISSING`, `COLLISION`, `LIMIT_EXCEEDED`, `INVALID_REGEX`, `INVALID_FILE`, -`NO_MATCHES`, `VALIDATION_FAILED`. JSON-RPC protocol errors are reserved for -genuine server faults and are never used for caller-actionable failures. +`NO_MATCHES`, `CURSOR_STALE`, `VALIDATION_FAILED`. JSON-RPC protocol errors +are reserved for genuine server faults and are never used for +caller-actionable failures. **R2 — Validate-first, atomic.** A mutation validates the entire request under the write lock before touching @@ -84,10 +85,14 @@ faults; Q1 = atomic-fail on missing endpoints; Q2/Q3 = error. (drops→errors: `ENDPOINT_MISSING`, all-missing `open_nodes`, `TYPE_NOT_FOUND`, unknown starts), `INVALID_FILE`, `INVALID_REGEX`, `NO_MATCHES` envelope. -- **Ledger PR:** R3 result ledgers + report fields +- **Ledger PR (#248, merged):** R3 result ledgers + report fields (`created`/`existing`/`skippedDuplicates`/`notFound`/`alreadyPresent`, `open_nodes.missing[]`, `random_walk.modeUsed`/`fallbackSteps`, `sequentialthinking.linkedTo`). -- **Next:** R5 cursor fingerprinting (`CURSOR_STALE`). +- **Cursor PR (this change):** R5 — cursors are `:`; + any change to the set between pages → visible `CURSOR_STALE`; malformed + cursors rejected; bare-number cursors accepted as legacy (unchecked). + Prerequisite shipped in the same change: all sort tie-breaks are + deterministic (name), so result sets have a stable total order. - **v4 line (`feat/libsegstore`):** inherits this policy at reconciliation; kbd4's batch=txn=commit model is the natural home for R2 atomicity. diff --git a/server.ts b/server.ts index 4e0e903..3d10871 100644 --- a/server.ts +++ b/server.ts @@ -42,6 +42,7 @@ export type ToolErrorCode = | 'INVALID_REGEX' | 'INVALID_FILE' | 'NO_MATCHES' + | 'CURSOR_STALE' | 'VALIDATION_FAILED'; /** @@ -157,7 +158,8 @@ export interface KnowledgeGraph { export interface PaginatedResult { items: T[]; - nextCursor: number | null; + /** Opaque ":" token, or null when the drain is complete. */ + nextCursor: string | null; totalCount: number; } @@ -212,7 +214,7 @@ function sortEntities( const bRank = rankMaps?.structural.get(b.name) ?? 0; const diff = aRank - bRank; if (diff !== 0) return mult * diff; - return Math.random() - 0.5; // random tiebreak + return a.name.localeCompare(b.name); // deterministic tiebreak (R5: total order keeps pagination stable) } if (sortBy === "llmrank") { // Primary: walker rank @@ -225,8 +227,8 @@ function sortEntities( const bStruct = rankMaps?.structural.get(b.name) ?? 0; const structDiff = aStruct - bStruct; if (structDiff !== 0) return mult * structDiff; - // Final: random tiebreak - return Math.random() - 0.5; + // Final: deterministic tiebreak — a total order keeps pagination stable (R5) + return a.name.localeCompare(b.name); } // For timestamps, treat undefined as 0 (oldest) const aVal = a[sortBy] ?? 0; @@ -257,7 +259,7 @@ function sortNeighbors( const bRank = rankMaps?.structural.get(b.name) ?? 0; const diff = aRank - bRank; if (diff !== 0) return mult * diff; - return Math.random() - 0.5; + return a.name.localeCompare(b.name); } if (sortBy === "llmrank") { const aWalker = rankMaps?.walker.get(a.name) ?? 0; @@ -268,7 +270,7 @@ function sortNeighbors( const bStruct = rankMaps?.structural.get(b.name) ?? 0; const structDiff = aStruct - bStruct; if (structDiff !== 0) return mult * structDiff; - return Math.random() - 0.5; + return a.name.localeCompare(b.name); } const aVal = a[sortBy] ?? 0; const bVal = b[sortBy] ?? 0; @@ -278,10 +280,43 @@ function sortNeighbors( export const MAX_CHARS = 4096; +/** + * FNV-1a (32-bit) over a string; `seed` lets callers stream multiple strings + * into one hash. Used for result-set fingerprints. + */ +function fnv1a32(s: string, seed = 0x811c9dc5): number { + let h = seed; + for (let i = 0; i < s.length; i++) { + h ^= s.charCodeAt(i); + h = Math.imul(h, 0x01000193); + } + return h >>> 0; +} + +/** + * Fingerprint of a result set: FNV-1a over every item's serialized form — the + * exact bytes pagination boundaries are computed over — so any change to the + * set's content or order changes the fingerprint. + */ +function resultSetFingerprint(items: T[]): string { + let h = 0x811c9dc5; + for (const item of items) { + h = fnv1a32(JSON.stringify(item), h); + h = fnv1a32('\u001f', h); + } + return h.toString(16).padStart(8, '0'); +} + /** * Paginate `items` starting at `cursor`, producing a page no larger than * `maxChars` (best-effort). * + * Policy R5 (docs/api-error-policy.md): a cursor is `":"` + * for the result set it came from. If the set has changed since the cursor was + * issued (content or order), the page boundaries have silently shifted, so the + * call errors with CURSOR_STALE rather than serving a wrong page. Bare numbers + * are accepted as legacy cursors and are NOT staleness-checked. + * * Forward-progress invariant: every call advances the cursor by at least one * item, even when that item's JSON exceeds `maxChars`. Without this guarantee, * an oversized item at position `cursor` would cause `nextCursor === cursor`, @@ -293,12 +328,37 @@ export const MAX_CHARS = 4096; * alternative (skipping the item, or returning an error) hides data from the * caller and makes downstream pagination inconsistent. */ -function paginateItems(items: T[], cursor: number = 0, maxChars: number = MAX_CHARS): PaginatedResult { +function paginateItems(items: T[], cursor: number | string = 0, maxChars: number = MAX_CHARS): PaginatedResult { + // Parse the cursor first; a malformed one is a stale/invalid cursor. + let i = 0; + let expected: string | null = null; + if (typeof cursor === 'string') { + const sep = cursor.indexOf(':'); + const idx = sep >= 0 ? Number(cursor.slice(0, sep)) : NaN; + if (!Number.isInteger(idx) || idx < 0) { + throw new ToolError('CURSOR_STALE', `Malformed cursor ${JSON.stringify(cursor)}; re-run the query and page from the start.`); + } + i = idx; + expected = cursor.slice(sep + 1); + } else if (typeof cursor === 'number' && Number.isFinite(cursor) && cursor >= 0) { + i = Math.floor(cursor); + } else { + throw new ToolError('CURSOR_STALE', 'Invalid cursor value; re-run the query and page from the start.'); + } + + const fingerprint = resultSetFingerprint(items); + if (expected !== null && expected !== fingerprint) { + throw new ToolError( + 'CURSOR_STALE', + `Result set changed since this cursor was issued (fingerprint ${expected} -> ${fingerprint}); re-run the query and page from the start.`, + { cursorIndex: i, totalCount: items.length }, + ); + } + const result: T[] = []; - let i = cursor; // Calculate overhead for wrapper: {"items":[],"nextCursor":null,"totalCount":123} - const wrapperTemplate = { items: [] as T[], nextCursor: null as number | null, totalCount: items.length }; + const wrapperTemplate = { items: [] as T[], nextCursor: null as string | null, totalCount: items.length }; const overhead = JSON.stringify(wrapperTemplate).length; let charCount = overhead; @@ -323,8 +383,7 @@ function paginateItems(items: T[], cursor: number = 0, maxChars: number = MAX i++; } - // Update nextCursor - recalculate if we stopped early (cursor digits may differ from null) - const nextCursor = i < items.length ? i : null; + const nextCursor = i < items.length ? `${i}:${fingerprint}` : null; return { items: result, @@ -333,7 +392,7 @@ function paginateItems(items: T[], cursor: number = 0, maxChars: number = MAX }; } -function paginateGraph(graph: KnowledgeGraph, entityCursor: number = 0, relationCursor: number = 0): { entities: PaginatedResult; relations: PaginatedResult } { +function paginateGraph(graph: KnowledgeGraph, entityCursor: number | string = 0, relationCursor: number | string = 0): { entities: PaginatedResult; relations: PaginatedResult } { // Entities and relations have independent cursors, so paginate them // independently — each gets the full budget. The caller already has // previously-returned pages and only needs the next page of whichever @@ -1612,8 +1671,8 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { direction: { type: "string", enum: ["forward", "backward", "any"], description: "Edge direction filter for returned relations. Default: forward" }, sortBy: { type: "string", enum: ["mtime", "obsMtime", "name", "pagerank", "llmrank"], description: "Sort field for entities. Omit for insertion order." }, sortDir: { type: "string", enum: ["asc", "desc"], description: "Sort direction. Default: desc for timestamps, asc for name." }, - entityCursor: { type: "number", description: "Cursor for entity pagination (from previous response's nextCursor)" }, - relationCursor: { type: "number", description: "Cursor for relation pagination" }, + entityCursor: { type: ["number", "string"], description: "Opaque entity page cursor; pass back exactly what nextCursor returned" }, + relationCursor: { type: ["number", "string"], description: "Opaque relation page cursor; pass back exactly what nextCursor returned" }, }, required: ["query"], }, @@ -1630,8 +1689,8 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { description: "An array of entity names to retrieve", }, direction: { type: "string", enum: ["forward", "backward", "any"], description: "Edge direction filter for returned relations. Default: forward" }, - entityCursor: { type: "number", description: "Cursor for entity pagination" }, - relationCursor: { type: "number", description: "Cursor for relation pagination" }, + entityCursor: { type: ["number", "string"], description: "Opaque entity page cursor; pass back exactly what nextCursor returned" }, + relationCursor: { type: ["number", "string"], description: "Opaque relation page cursor; pass back exactly what nextCursor returned" }, }, required: ["names"], }, @@ -1647,7 +1706,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { direction: { type: "string", enum: ["forward", "backward", "any"], description: "Edge direction to follow. Default: forward" }, sortBy: { type: "string", enum: ["mtime", "obsMtime", "name", "pagerank", "llmrank"], description: "Sort field for neighbors. Omit for arbitrary order." }, sortDir: { type: "string", enum: ["asc", "desc"], description: "Sort direction. Default: desc for timestamps, asc for name." }, - cursor: { type: "number", description: "Cursor for pagination" }, + cursor: { type: ["number", "string"], description: "Opaque page cursor; pass back exactly what nextCursor returned. Bare numbers are accepted for legacy callers (no staleness check)." }, }, required: ["entityName"], }, @@ -1662,7 +1721,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { toEntity: { type: "string", description: "The name of the target entity" }, maxDepth: { type: "number", description: "Maximum depth to search (default: 5)", default: 5 }, direction: { type: "string", enum: ["forward", "backward", "any"], description: "Edge direction to follow. Default: forward" }, - cursor: { type: "number", description: "Cursor for pagination" }, + cursor: { type: ["number", "string"], description: "Opaque page cursor; pass back exactly what nextCursor returned. Bare numbers are accepted for legacy callers (no staleness check)." }, }, required: ["fromEntity", "toEntity"], }, @@ -1676,7 +1735,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { entityType: { type: "string", description: "The type of entities to retrieve" }, sortBy: { type: "string", enum: ["mtime", "obsMtime", "name", "pagerank", "llmrank"], description: "Sort field for entities. Omit for insertion order." }, sortDir: { type: "string", enum: ["asc", "desc"], description: "Sort direction. Default: desc for timestamps, asc for name." }, - cursor: { type: "number", description: "Cursor for pagination" }, + cursor: { type: ["number", "string"], description: "Opaque page cursor; pass back exactly what nextCursor returned. Bare numbers are accepted for legacy callers (no staleness check)." }, }, required: ["entityType"], }, @@ -1687,7 +1746,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { inputSchema: { type: "object", properties: { - cursor: { type: "number", description: "Cursor for pagination" }, + cursor: { type: ["number", "string"], description: "Opaque page cursor; pass back exactly what nextCursor returned. Bare numbers are accepted for legacy callers (no staleness check)." }, }, }, }, @@ -1697,7 +1756,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { inputSchema: { type: "object", properties: { - cursor: { type: "number", description: "Cursor for pagination" }, + cursor: { type: ["number", "string"], description: "Opaque page cursor; pass back exactly what nextCursor returned. Bare numbers are accepted for legacy callers (no staleness check)." }, }, }, }, @@ -1718,7 +1777,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { strict: { type: "boolean", description: "If true, returns entities not connected to 'Self' (directly or indirectly). Default: false" }, sortBy: { type: "string", enum: ["mtime", "obsMtime", "name", "pagerank", "llmrank"], description: "Sort field for entities. Omit for insertion order." }, sortDir: { type: "string", enum: ["asc", "desc"], description: "Sort direction. Default: desc for timestamps, asc for name." }, - cursor: { type: "number", description: "Cursor for pagination" }, + cursor: { type: ["number", "string"], description: "Opaque page cursor; pass back exactly what nextCursor returned. Bare numbers are accepted for legacy callers (no staleness check)." }, }, }, }, @@ -1728,8 +1787,8 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { inputSchema: { type: "object", properties: { - entitiesCursor: { type: "number", description: "Cursor for the missingEntities list" }, - violationsCursor: { type: "number", description: "Cursor for the observationViolations list" }, + entitiesCursor: { type: ["number", "string"], description: "Opaque cursor for the missingEntities list; pass back exactly what nextCursor returned" }, + violationsCursor: { type: ["number", "string"], description: "Opaque cursor for the observationViolations list; pass back exactly what nextCursor returned" }, }, }, }, @@ -1892,26 +1951,26 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p // Record walker visits for entities that will be returned to the LLM knowledgeGraphManager.recordWalkerVisits(graph.entities.map(e => e.name)); - return { content: [{ type: "text", text: JSON.stringify(paginateGraph(graph, args.entityCursor as number ?? 0, args.relationCursor as number ?? 0)) }] }; + return { content: [{ type: "text", text: JSON.stringify(paginateGraph(graph, (args.entityCursor as number | string | undefined) ?? 0, (args.relationCursor as number | string | undefined) ?? 0)) }] }; } case "open_nodes": { const graph = await knowledgeGraphManager.openNodes(args.names as string[], (args.direction as 'forward' | 'backward' | 'any') ?? 'forward'); // Record walker visits for opened nodes knowledgeGraphManager.recordWalkerVisits(graph.entities.map(e => e.name)); // R3: report partial misses alongside the (paginated) graph. - const paginated = paginateGraph(graph, args.entityCursor as number ?? 0, args.relationCursor as number ?? 0); + const paginated = paginateGraph(graph, (args.entityCursor as number | string | undefined) ?? 0, (args.relationCursor as number | string | undefined) ?? 0); return { content: [{ type: "text", text: JSON.stringify({ ...paginated, missing: graph.missing }) }] }; } case "get_neighbors": { const neighbors = await knowledgeGraphManager.getNeighbors(args.entityName as string, args.depth as number ?? 0, args.sortBy as EntitySortField | undefined, args.sortDir as SortDirection | undefined, (args.direction as 'forward' | 'backward' | 'any') ?? 'forward'); // Record walker visits for returned neighbors knowledgeGraphManager.recordWalkerVisits(neighbors.map(n => n.name)); - return { content: [{ type: "text", text: JSON.stringify(paginateItems(neighbors, args.cursor as number ?? 0)) }] }; + return { content: [{ type: "text", text: JSON.stringify(paginateItems(neighbors, (args.cursor as number | string | undefined) ?? 0)) }] }; } case "find_path": { const toEntityName = args.toEntity as string; const result = await knowledgeGraphManager.findPath(args.fromEntity as string, toEntityName, args.maxDepth as number, (args.direction as 'forward' | 'backward' | 'any') ?? 'forward'); - const paginated = paginateItems(result.path, args.cursor as number ?? 0); + const paginated = paginateItems(result.path, (args.cursor as number | string | undefined) ?? 0); // β-contract response. Pagination of `path` is unchanged; the // result wrapper additionally carries `targetReached` (did we // actually reach `toEntity`?), `budgetExhausted` (did we stop @@ -1946,27 +2005,27 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p } case "get_entities_by_type": { const entities = await knowledgeGraphManager.getEntitiesByType(args.entityType as string, args.sortBy as EntitySortField | undefined, args.sortDir as SortDirection | undefined); - return { content: [{ type: "text", text: JSON.stringify(paginateItems(entities, args.cursor as number ?? 0)) }] }; + return { content: [{ type: "text", text: JSON.stringify(paginateItems(entities, (args.cursor as number | string | undefined) ?? 0)) }] }; } case "get_entity_types": { const types = await knowledgeGraphManager.getEntityTypes(); - return { content: [{ type: "text", text: JSON.stringify(paginateItems(types, args.cursor as number ?? 0)) }] }; + return { content: [{ type: "text", text: JSON.stringify(paginateItems(types, (args.cursor as number | string | undefined) ?? 0)) }] }; } case "get_relation_types": { const types = await knowledgeGraphManager.getRelationTypes(); - return { content: [{ type: "text", text: JSON.stringify(paginateItems(types, args.cursor as number ?? 0)) }] }; + return { content: [{ type: "text", text: JSON.stringify(paginateItems(types, (args.cursor as number | string | undefined) ?? 0)) }] }; } case "get_stats": return { content: [{ type: "text", text: JSON.stringify(await knowledgeGraphManager.getStats(), null, 2) }] }; case "get_orphaned_entities": { const entities = await knowledgeGraphManager.getOrphanedEntities(args.strict as boolean ?? false, args.sortBy as EntitySortField | undefined, args.sortDir as SortDirection | undefined); - return { content: [{ type: "text", text: JSON.stringify(paginateItems(entities, args.cursor as number ?? 0)) }] }; + return { content: [{ type: "text", text: JSON.stringify(paginateItems(entities, (args.cursor as number | string | undefined) ?? 0)) }] }; } case "validate_graph": { const report = await knowledgeGraphManager.validateGraph(); return { content: [{ type: "text", text: JSON.stringify({ - missingEntities: paginateItems(report.missingEntities, args.entitiesCursor as number ?? 0), - observationViolations: paginateItems(report.observationViolations, args.violationsCursor as number ?? 0), + missingEntities: paginateItems(report.missingEntities, (args.entitiesCursor as number | string | undefined) ?? 0), + observationViolations: paginateItems(report.observationViolations, (args.violationsCursor as number | string | undefined) ?? 0), }) }] }; } case "decode_timestamp": diff --git a/tests/memory-server.test.ts b/tests/memory-server.test.ts index 8b59035..83f44b2 100644 --- a/tests/memory-server.test.ts +++ b/tests/memory-server.test.ts @@ -259,7 +259,7 @@ describe('MCP Memory Server E2E Tests', () => { it('should search by regex pattern', async () => { // Accumulate all entities across pagination const allEntities: Entity[] = []; - let entityCursor: number | null = 0; + let entityCursor: string | number | null = 0; while (entityCursor !== null) { const result = await callTool(client, 'search_nodes', { @@ -279,7 +279,7 @@ describe('MCP Memory Server E2E Tests', () => { it('should search with alternation', async () => { // Accumulate all entities across pagination const allEntities: Entity[] = []; - let entityCursor: number | null = 0; + let entityCursor: string | number | null = 0; while (entityCursor !== null) { const result = await callTool(client, 'search_nodes', { @@ -345,7 +345,7 @@ describe('MCP Memory Server E2E Tests', () => { // result sets, not nondeterministic top-llmrank pages. async function drainAll(query: string): Promise> { const out = new Set(); - let cursor: number | null = 0; + let cursor: string | number | null = 0; while (cursor !== null) { const r = await callTool(client, 'search_nodes', { query, sortBy: 'name', sortDir: 'asc', entityCursor: cursor, @@ -452,7 +452,7 @@ describe('MCP Memory Server E2E Tests', () => { }); const allEntities: Entity[] = []; - let entityCursor: number | null = 0; + let entityCursor: string | number | null = 0; let iterations = 0; const ITERATION_CAP = 50; // any healthy graph fits in << 50 pages here @@ -1840,7 +1840,7 @@ describe('MCP Memory Server E2E Tests', () => { // Fetch all pages sorted by name descending const allEntities: Entity[] = []; - let entityCursor: number | null = 0; + let entityCursor: string | number | null = 0; while (entityCursor !== null) { const result = await callTool(client, 'search_nodes', { @@ -2393,4 +2393,72 @@ describe('MCP Memory Server E2E Tests', () => { expect(second.linkedTo).toBe(first.ctxId); }); }); + + describe('API cursor integrity (R5)', () => { + async function seedCursorTag(count: number) { + const entities = Array.from({ length: count }, (_, i) => ({ + name: `CursorTag${String(i).padStart(2, '0')}`, + entityType: 'CursorTag', + observations: ['x'.repeat(100)], + })); + await callTool(client, 'create_entities', { entities }); + } + + it('a stale cursor errors instead of serving a shifted page', async () => { + await seedCursorTag(30); + + const page1 = await callTool(client, 'get_entities_by_type', { + entityType: 'CursorTag', sortBy: 'name', sortDir: 'asc', + }) as PaginatedResult; + expect(page1.nextCursor).not.toBeNull(); + expect(typeof page1.nextCursor).toBe('string'); + + // Mutate the result set between pages. + await callTool(client, 'create_entities', { + entities: [{ name: 'CursorTagNew', entityType: 'CursorTag', observations: ['y'.repeat(100)] }], + }); + + const raw = await callToolRaw(client, 'get_entities_by_type', { + entityType: 'CursorTag', sortBy: 'name', sortDir: 'asc', cursor: page1.nextCursor, + }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('CURSOR_STALE'); + }); + + it('an unchanged set drains cleanly via string cursors (no dupes, no gaps)', async () => { + await seedCursorTag(30); + + const seen: string[] = []; + let cursor: string | number | null = 0; + let guard = 0; + while (cursor !== null && guard++ < 20) { + const page = await callTool(client, 'get_entities_by_type', { + entityType: 'CursorTag', sortBy: 'name', sortDir: 'asc', cursor, + }) as PaginatedResult; + seen.push(...page.items.map(e => e.name)); + cursor = page.nextCursor; + } + expect(seen).toHaveLength(30); + expect(new Set(seen).size).toBe(30); + }); + + it('malformed cursors are rejected with CURSOR_STALE', async () => { + await seedCursorTag(1); + const raw = await callToolRaw(client, 'get_entities_by_type', { + entityType: 'CursorTag', sortBy: 'name', sortDir: 'asc', cursor: 'garbage', + }); + expect(raw.isError).toBe(true); + expect(parseToolError(raw).code).toBe('CURSOR_STALE'); + }); + + it('legacy bare-number cursors still work (no staleness check)', async () => { + await seedCursorTag(30); + const page = await callTool(client, 'get_entities_by_type', { + entityType: 'CursorTag', sortBy: 'name', sortDir: 'asc', cursor: 1, + }) as PaginatedResult; + expect(page.items.length).toBeGreaterThan(0); + // Starts at index 1 (the second item in name order), not at the head. + expect(page.items[0].name).toBe('CursorTag01'); + }); + }); }); diff --git a/tests/test-utils.ts b/tests/test-utils.ts index 745bd4d..20c4ff0 100644 --- a/tests/test-utils.ts +++ b/tests/test-utils.ts @@ -7,7 +7,7 @@ export { MAX_CHARS }; export interface PaginatedResult { items: T[]; - nextCursor: number | null; + nextCursor: string | null; totalCount: number; } From 9cd7e2eae8c22ff00bd161b56b1c6a7afa554cda Mon Sep 17 00:00:00 2001 From: Levalicious <24482560+Levalicious@users.noreply.github.com> Date: Thu, 8 Oct 2026 03:16:56 -0400 Subject: [PATCH 8/8] Bump version Co-Authored-By: Claude Code --- package.json | 2 +- server.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 27f7ad7..49e289c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@levalicious/server-memory", - "version": "0.0.29", + "version": "0.0.30", "description": "MCP server for enabling memory for Claude through a knowledge graph", "license": "MIT", "author": "Levalicious", diff --git a/server.ts b/server.ts index 3d10871..11ea88c 100644 --- a/server.ts +++ b/server.ts @@ -1509,7 +1509,7 @@ export function createServer(memoryFilePath?: string): Server { sizes: ["any"] } ], - version: "0.0.29", + version: "0.0.30", }, { capabilities: { tools: {},