feat: v0.6.1 — citations, cascading updates, expanded mesh#79
Merged
Conversation
Add JIT verification with citation chains, self-organizing memory via cascading staleness propagation, and expand mesh sync to 7 scopes (semantic, procedural, relations, graph nodes/edges). - mem::verify traces memory/observation provenance back to source - mem::cascade-update flags stale graph items and sibling memories - Mesh default scopes expanded from 2 to 7 with project-based filtering - Memory.sourceObservationIds, CompressedObservation.confidence fields - GraphNode.stale, GraphEdge.stale for staleness tracking - MeshPeer.syncFilter for project-scoped action sync - 573 tests passing across 49 files
📝 WalkthroughWalkthroughAdds mem::cascade-update and mem::verify functions, propagates sourceObservationIds through memories, marks graph nodes/edges stale on supersession, adds confidence to compressed observations, expands mesh sync to include semantic/procedural/graph scopes with LWW merges, updates types/version to 0.6.1, and adds tests and API/mcp hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SDK
participant KV
participant GraphStore as Graph (nodes/edges)
Client->>SDK: trigger "mem::cascade-update" { supersededMemoryId }
SDK->>KV: get memory by id
alt memory not found
KV-->>SDK: null
SDK-->>Client: error (not found)
else memory found
KV-->>SDK: supersededMemory
SDK->>KV: list graphNodes
KV-->>SDK: nodes[]
SDK->>GraphStore: mark nodes stale where sourceObservationIds intersect
SDK->>KV: update node timestamps
SDK->>KV: list graphEdges
KV-->>SDK: edges[]
SDK->>GraphStore: mark edges stale where sourceObservationIds intersect
SDK->>KV: update edges
SDK->>KV: list memories (latest)
KV-->>SDK: memories[]
SDK->>KV: update siblingMemories counts for shared concepts
SDK-->>Client: result { success, counts, total }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/triggers/api.ts (1)
1474-1483:⚠️ Potential issue | 🟠 MajorPull exports still ignore
syncFilter.project.This endpoint only accepts
since; there is no way formesh-syncto request a project-scoped export. A peer registered with{ project: ... }will therefore receive the remote instance's full pull dataset.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/triggers/api.ts` around lines 1474 - 1483, The endpoint currently only honors the "since" query and then computes sinceTime and df (the filter function) but ignores any syncFilter.project; update request handling to accept/parse the peer's project filter (e.g., from req.query_params or the incoming syncFilter payload) and apply it before/alongside the since filter: read the project identifier, compute sinceTime as now, and modify the df function (or the filtering pipeline that uses df) to also require that (item as Record<string, unknown>).project === syncFilter.project (or matches the provided project param) so returned pulls are scoped to the requested project as well as the since time. Ensure you reference the existing symbols since, sinceTime, df and req.query_params when making the change.
🧹 Nitpick comments (5)
test/mcp-standalone.test.ts (1)
25-27: Also assert thatmemory_verifyis exposed.The exact total catches catalog-size drift, but not whether the new tool is actually present. A name-level assertion will fail with a much better signal.
Suggested tweak
it("getAllTools returns 38 tools", () => { const tools = getAllTools(); expect(tools.length).toBe(38); + expect(tools.map((tool) => tool.name)).toContain("memory_verify"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mcp-standalone.test.ts` around lines 25 - 27, The test currently only asserts the tools array length; update the "getAllTools returns 38 tools" test to also assert that the returned array from getAllTools contains a tool named "memory_verify" (e.g., use tools.some(t => t.name === 'memory_verify') or an equivalent name-level assertion) so the test fails if the specific tool is not exposed even if the total count matches; modify the test body that calls getAllTools() and add the additional expect check for the "memory_verify" name.test/mesh.test.ts (1)
105-114: Please cover actualsyncFilterbehavior.This only proves the field is persisted. A push/pull regression that ignores the filter would still pass, so a mixed-project sync test would be valuable here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mesh.test.ts` around lines 105 - 114, Add an integration test (or extend the existing "stores syncFilter when provided" scenario) that verifies syncFilter actually gates push/pull: use sdk.trigger("mem::mesh-register") to register two peers (one with syncFilter: { project: "/my/project" } and one without or with a different project), create/insert test documents in both projects, invoke the mesh push/pull/sync operations used elsewhere in tests (the same APIs your suite uses for pushing and pulling), and assert that the peer with syncFilter only receives documents matching { project: "/my/project" } while documents from the other project are not transferred; reference the existing MeshPeer.syncFilter field and the sdk.trigger registration flow to locate where to add these assertions.test/cascade.test.ts (2)
12-47: Consider extracting shared test utilities to reduce duplication.The
mockKV()andmockSdk()helper functions are identical to those inverify.test.ts. Extracting these to a shared test utility file (e.g.,test/helpers/mocks.ts) would reduce duplication and make maintenance easier.Additionally, using the loose
Functiontype on line 34 loses type safety. Consider using a more specific function signature.♻️ Suggested approach
Create a shared test helper:
// test/helpers/mocks.ts export function mockKV() { const store = new Map<string, Map<string, unknown>>(); return { get: async <T>(scope: string, key: string): Promise<T | null> => { return (store.get(scope)?.get(key) as T) ?? null; }, set: async <T>(scope: string, key: string, data: T): Promise<T> => { if (!store.has(scope)) store.set(scope, new Map()); store.get(scope)!.set(key, data); return data; }, delete: async (scope: string, key: string): Promise<void> => { store.get(scope)?.delete(key); }, list: async <T>(scope: string): Promise<T[]> => { const entries = store.get(scope); return entries ? (Array.from(entries.values()) as T[]) : []; }, }; } export function mockSdk() { const functions = new Map<string, (data: unknown) => Promise<unknown>>(); return { registerFunction: (opts: { id: string }, handler: (data: unknown) => Promise<unknown>) => { functions.set(opts.id, handler); }, registerTrigger: () => {}, trigger: async (id: string, data: unknown) => { const fn = functions.get(id); if (!fn) throw new Error(`No function: ${id}`); return fn(data); }, triggerVoid: () => {}, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/cascade.test.ts` around lines 12 - 47, Extract the duplicated mock helpers into a shared module (e.g., export mockKV and mockSdk from test/helpers/mocks.ts) and replace the local implementations in cascade.test.ts and verify.test.ts with imports from that helper; for mockSdk, tighten the loose Function type to a specific signature like (data: unknown) => Promise<unknown> (use the same signature for registerFunction's handler and the Map value) so type safety is preserved, and update the tests to import { mockKV, mockSdk } from the new helper.
57-57: Theas nevercast suppresses type checking.Using
as neverto bypass type mismatches between the mock and the actual SDK/KV interfaces may hide real incompatibilities. Consider defining a minimal interface type for the mock that captures only the methods used byregisterCascadeFunction, which would provide better type safety while still allowing the mock.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/cascade.test.ts` at line 57, The test is using `as never` to silence type errors when calling registerCascadeFunction(sdk as never, kv as never); replace the unsafe casts by declaring minimal mock interfaces that include only the methods/properties used by registerCascadeFunction (e.g., create a local type like MinimalSdk { methodA(...): ReturnType; ... } and MinimalKv { get(...): Promise<T>; put(...): Promise<void>; }), type the mock objects (`sdk` and `kv`) with those interfaces, and then call registerCascadeFunction(sdk as unknown as MinimalSdk, kv as unknown as MinimalKv) or simply cast to the minimal types directly so TypeScript enforces the required shape without suppressing checks for unrelated members.test/verify.test.ts (1)
33-46: Minor inconsistency with mockSdk in cascade.test.ts.This
mockSdk()implementation lacks thetriggerVoidmethod that exists incascade.test.ts. While not a bug (since verify tests don't use it), this inconsistency reinforces the benefit of extracting these helpers to a shared module with a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/verify.test.ts` around lines 33 - 46, The mockSdk implementation in verify.test.ts is missing the triggerVoid helper present in cascade.test.ts; update mockSdk (the mockSdk function and its returned object) to include a triggerVoid method with the same semantics as cascade.test.ts (accepting id and data, invoking the registered function without returning a value) or extract mockSdk into a shared test helper module and import it into both tests so both test files use the same implementation; reference the mockSdk function and triggerVoid when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/functions/cascade.ts`:
- Around line 25-46: The cascade marks GraphNode/GraphEdge items with stale =
true but graph-retrieval.ts still expands from all graph items; update the
retrieval/traversal code (the function that lists/expands GraphNode and
GraphEdge around lines 117-127) to ignore items where stale === true (e.g.,
after kv.list<GraphNode>(KV.graphNodes) and kv.list<GraphEdge>(KV.graphEdges)
apply .filter(node => !node.stale) / .filter(edge => !edge.stale)) so retired
facts are not included in traversal/expansion.
In `@src/functions/consolidate.ts`:
- Line 161: The current mapping const obsIds = top.map((o) => o.id) can produce
duplicate IDs when a concept repeats; before passing IDs to persistence or
mem::verify, deduplicate them (e.g. replace the direct map with a unique array
like Array.from(new Set(top.map(o => o.id)))) so sourceObservationIds contains
only unique IDs; apply the same deduplication to the other similar mapping(s)
around the 177-189 region so that mem::verify and any citationCount logic
receive de-duplicated IDs.
In `@src/functions/mesh.ts`:
- Around line 277-278: The graph sync currently uses createdAt as both the LWW
clock and delta cursor (seen in calls to lwwMergeList with
KV.graphNodes/KV.graphEdges and tags "mem:gnode"/"mem:gedge"), which prevents
later updates from being exported/overwritten; change the merge/delta key to use
a true modification clock (e.g., updatedAt/modifiedAt or a separate
lastModified/mtime field) instead of createdAt in those lwwMergeList calls and
any other graph sync calls (the other graph merge sites referenced in the
review), and ensure upstream writers update that modified timestamp when
nodes/edges are changed so LWW merging and delta export work correctly.
- Around line 321-351: The syncFilter.project is only applied to the "actions"
branch leaving other scopes (semantic, procedural, relations, graph:nodes,
graph:edges) unfiltered; update the code paths that build result.* (references:
syncFilter.project, scopes, KV.semantic, KV.procedural, KV.relations,
KV.graphNodes, KV.graphEdges, deltaFilter, GraphNode, GraphEdge, MemoryRelation)
so that either (A) you derive/attach project ownership for records that can
carry it and filter those lists by record.project when syncFilter.project is set
before calling deltaFilter, or (B) when syncFilter.project is present and a
scope does not have a project field (e.g., GraphNode/GraphEdge), reject that
scope early (throw or return a clear error) instead of returning unscoped data;
implement the chosen approach consistently for semantic, procedural, relations,
graph:nodes and graph:edges branches.
In `@src/functions/remember.ts`:
- Line 17: Validate and normalize the new public field sourceObservationIds
before persisting it: ensure in the remember flow (where sourceObservationIds is
read/assigned alongside files and concepts in src/functions/remember.ts) that
the value is either undefined or an Array of strings—reject or coerce other
types (e.g., if it's a single string, wrap it; if mixed/non-string entries,
filter to strings) and return a 4xx error or strip and log when invalid; this
prevents downstream failures in mem::verify and mem::cascade-update which expect
an array of string IDs.
In `@src/triggers/api.ts`:
- Around line 1481-1500: The current delta filter function df(items, field) uses
"createdAt" for graphNodes and graphEdges so later in-place mutations (like
stale flags) won't be returned; change the returned object to call
df(graphNodes, "updatedAt") and df(graphEdges, "updatedAt") instead of
"createdAt" and ensure the code paths that mutate nodes/edges update their
updatedAt timestamps; refer to the df helper and the graphNodes/graphEdges
return entries in this function to locate where to update the field usage and to
verify mutation handlers set updatedAt.
---
Outside diff comments:
In `@src/triggers/api.ts`:
- Around line 1474-1483: The endpoint currently only honors the "since" query
and then computes sinceTime and df (the filter function) but ignores any
syncFilter.project; update request handling to accept/parse the peer's project
filter (e.g., from req.query_params or the incoming syncFilter payload) and
apply it before/alongside the since filter: read the project identifier, compute
sinceTime as now, and modify the df function (or the filtering pipeline that
uses df) to also require that (item as Record<string, unknown>).project ===
syncFilter.project (or matches the provided project param) so returned pulls are
scoped to the requested project as well as the since time. Ensure you reference
the existing symbols since, sinceTime, df and req.query_params when making the
change.
---
Nitpick comments:
In `@test/cascade.test.ts`:
- Around line 12-47: Extract the duplicated mock helpers into a shared module
(e.g., export mockKV and mockSdk from test/helpers/mocks.ts) and replace the
local implementations in cascade.test.ts and verify.test.ts with imports from
that helper; for mockSdk, tighten the loose Function type to a specific
signature like (data: unknown) => Promise<unknown> (use the same signature for
registerFunction's handler and the Map value) so type safety is preserved, and
update the tests to import { mockKV, mockSdk } from the new helper.
- Line 57: The test is using `as never` to silence type errors when calling
registerCascadeFunction(sdk as never, kv as never); replace the unsafe casts by
declaring minimal mock interfaces that include only the methods/properties used
by registerCascadeFunction (e.g., create a local type like MinimalSdk {
methodA(...): ReturnType; ... } and MinimalKv { get(...): Promise<T>; put(...):
Promise<void>; }), type the mock objects (`sdk` and `kv`) with those interfaces,
and then call registerCascadeFunction(sdk as unknown as MinimalSdk, kv as
unknown as MinimalKv) or simply cast to the minimal types directly so TypeScript
enforces the required shape without suppressing checks for unrelated members.
In `@test/mcp-standalone.test.ts`:
- Around line 25-27: The test currently only asserts the tools array length;
update the "getAllTools returns 38 tools" test to also assert that the returned
array from getAllTools contains a tool named "memory_verify" (e.g., use
tools.some(t => t.name === 'memory_verify') or an equivalent name-level
assertion) so the test fails if the specific tool is not exposed even if the
total count matches; modify the test body that calls getAllTools() and add the
additional expect check for the "memory_verify" name.
In `@test/mesh.test.ts`:
- Around line 105-114: Add an integration test (or extend the existing "stores
syncFilter when provided" scenario) that verifies syncFilter actually gates
push/pull: use sdk.trigger("mem::mesh-register") to register two peers (one with
syncFilter: { project: "/my/project" } and one without or with a different
project), create/insert test documents in both projects, invoke the mesh
push/pull/sync operations used elsewhere in tests (the same APIs your suite uses
for pushing and pulling), and assert that the peer with syncFilter only receives
documents matching { project: "/my/project" } while documents from the other
project are not transferred; reference the existing MeshPeer.syncFilter field
and the sdk.trigger registration flow to locate where to add these assertions.
In `@test/verify.test.ts`:
- Around line 33-46: The mockSdk implementation in verify.test.ts is missing the
triggerVoid helper present in cascade.test.ts; update mockSdk (the mockSdk
function and its returned object) to include a triggerVoid method with the same
semantics as cascade.test.ts (accepting id and data, invoking the registered
function without returning a value) or extract mockSdk into a shared test helper
module and import it into both tests so both test files use the same
implementation; reference the mockSdk function and triggerVoid when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54789dbb-205f-4d41-a20d-26517d46a8a9
📒 Files selected for processing (18)
src/functions/cascade.tssrc/functions/compress.tssrc/functions/consolidate.tssrc/functions/export-import.tssrc/functions/mesh.tssrc/functions/remember.tssrc/functions/verify.tssrc/index.tssrc/mcp/server.tssrc/mcp/tools-registry.tssrc/triggers/api.tssrc/types.tssrc/version.tstest/cascade.test.tstest/export-import.test.tstest/mcp-standalone.test.tstest/mesh.test.tstest/verify.test.ts
Comment on lines
+1481
to
+1500
| const sinceTime = since ? new Date(since).getTime() : 0; | ||
| const df = <T>(items: T[], field: "updatedAt" | "createdAt") => | ||
| items.filter((i) => new Date((i as Record<string, unknown>)[field] as string).getTime() > sinceTime); | ||
| const memories = await kv.list<import("../types.js").Memory>(KV.memories); | ||
| const actions = await kv.list<import("../types.js").Action>(KV.actions); | ||
| const sinceTime = since ? new Date(since).getTime() : 0; | ||
| const semantic = await kv.list<import("../types.js").SemanticMemory>(KV.semantic); | ||
| const procedural = await kv.list<import("../types.js").ProceduralMemory>(KV.procedural); | ||
| const relations = await kv.list<import("../types.js").MemoryRelation>(KV.relations); | ||
| const graphNodes = await kv.list<import("../types.js").GraphNode>(KV.graphNodes); | ||
| const graphEdges = await kv.list<import("../types.js").GraphEdge>(KV.graphEdges); | ||
| return { | ||
| status_code: 200, | ||
| body: { | ||
| memories: memories.filter( | ||
| (m) => new Date(m.updatedAt).getTime() > sinceTime, | ||
| ), | ||
| actions: actions.filter( | ||
| (a) => new Date(a.updatedAt).getTime() > sinceTime, | ||
| ), | ||
| memories: df(memories, "updatedAt"), | ||
| actions: df(actions, "updatedAt"), | ||
| semantic: df(semantic, "updatedAt"), | ||
| procedural: df(procedural, "updatedAt"), | ||
| relations: df(relations, "createdAt"), | ||
| graphNodes: df(graphNodes, "createdAt"), | ||
| graphEdges: df(graphEdges, "createdAt"), |
There was a problem hiding this comment.
REST graph deltas should use last mutation, not createdAt.
graphNodes/graphEdges are filtered by createdAt, but this PR also introduces mutable stale flags and other in-place graph updates. Once a node/edge has been created, later mutations will never satisfy the since filter and will never be pulled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/triggers/api.ts` around lines 1481 - 1500, The current delta filter
function df(items, field) uses "createdAt" for graphNodes and graphEdges so
later in-place mutations (like stale flags) won't be returned; change the
returned object to call df(graphNodes, "updatedAt") and df(graphEdges,
"updatedAt") instead of "createdAt" and ensure the code paths that mutate
nodes/edges update their updatedAt timestamps; refer to the df helper and the
graphNodes/graphEdges return entries in this function to locate where to update
the field usage and to verify mutation handlers set updatedAt.
…oping - Filter stale nodes/edges in graph-retrieval.ts and graph.ts traversals - Use updatedAt (not createdAt) as LWW clock for graph node mesh sync - Set updatedAt in cascade.ts when flagging nodes as stale - Skip non-project scopes (semantic, procedural, relations, graph) when syncFilter.project is set to prevent leaking unscoped data - Validate sourceObservationIds as array of strings in remember.ts - Deduplicate observation IDs in consolidate.ts sourceObservationIds - mesh-export endpoint uses updatedAt for graph nodes, accepts project param - Extract shared test helpers into test/helpers/mocks.ts - Assert memory_verify tool name in mcp-standalone.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/triggers/api.ts (1)
1503-1506:⚠️ Potential issue | 🟡 MinorGraphEdge stale mutations may not propagate in delta exports.
Line 1503-1505 correctly handles
GraphNodewithupdatedAt || createdAtfallback. However, line 1506 usescreatedAtforGraphEdge, and the cascade function (lines 44-45 incascade.ts) setsstale = trueon edges without updating any timestamp. This means edges marked stale after creation won't appear in delta exports using thesinceparameter.Consider adding
updatedAttoGraphEdgeincascade.tswhen marking it stale, similar to how it's done forGraphNodeat line 33.🔧 Proposed fix in cascade.ts
if (overlap) { edge.stale = true; + edge.updatedAt = now; await kv.set(KV.graphEdges, edge.id, edge); flaggedEdges++; }Then update the delta filter in api.ts:
- body.graphEdges = df(graphEdges, "createdAt"); + body.graphEdges = graphEdges.filter( + (e) => new Date(e.updatedAt || e.createdAt).getTime() > sinceTime, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/triggers/api.ts` around lines 1503 - 1506, Delta exports miss edges marked stale because cascade.ts sets GraphEdge.stale=true but doesn't update GraphEdge.updatedAt; update cascade.ts so when you set stale=true on a GraphEdge (the same place GraphNode.updatedAt is set) also set updatedAt to the current timestamp, and then change the delta filter in api.ts from df(graphEdges, "createdAt") to use the updated timestamp (e.g., df(graphEdges, "updatedAt")) so graphEdges with updatedAt > sinceTime are included; refer to the cascade function where stale is assigned and to the df() call and body.graphEdges in api.ts.src/functions/mesh.ts (1)
308-310:⚠️ Potential issue | 🟠 MajorGraph edge sync still keyed by
createdAt, so post-create updates won’t replicate.Line 309, Line 386, and Line 430 use
createdAtfor graph edges. Any later edge mutation (for example stale/metadata updates written with newer timestamps) can be dropped, causing peer divergence.🔧 Suggested fix
+function graphEdgeTs(edge: GraphEdge): string { + return edge.updatedAt || edge.createdAt; +} + +async function lwwMergeGraphEdges( + kv: StateKV, + items: GraphEdge[] | undefined, +): Promise<number> { + if (!items || !Array.isArray(items)) return 0; + let count = 0; + for (const item of items) { + if (!item.id || typeof item.id !== "string") continue; + const ts = graphEdgeTs(item); + if (!ts || Number.isNaN(new Date(ts).getTime())) continue; + const wrote = await withKeyedLock(`mem:gedge:${item.id}`, async () => { + const existing = await kv.get<GraphEdge>(KV.graphEdges, item.id); + if (!existing || new Date(ts) > new Date(graphEdgeTs(existing))) { + await kv.set(KV.graphEdges, item.id, item); + return true; + } + return false; + }); + if (wrote) count++; + } + return count; +} @@ -accepted += await lwwMergeList(kv, KV.graphEdges, data.graphEdges, "mem:gedge", "createdAt"); +accepted += await lwwMergeGraphEdges(kv, data.graphEdges); @@ -result.graphEdges = deltaFilter(all, sinceTime, "createdAt"); +result.graphEdges = all.filter((e) => new Date(graphEdgeTs(e)).getTime() > sinceTime); @@ -applied += await lwwMergeList(kv, KV.graphEdges, data.graphEdges, "mem:gedge", "createdAt"); +applied += await lwwMergeGraphEdges(kv, data.graphEdges);Also applies to: 384-387, 429-431
🧹 Nitpick comments (3)
src/functions/graph.ts (1)
254-274: Consider adding stale counts to graph stats.The
mem::graph-statsfunction includes all nodes/edges regardless of stale status, which differs frommem::graph-querythat filters them out. This is likely intentional for monitoring, but consider addingstaleNodesandstaleEdgescounts to provide visibility into how many entities have been marked stale.💡 Optional enhancement
sdk.registerFunction({ id: "mem::graph-stats" }, async () => { const nodes = await kv.list<GraphNode>(KV.graphNodes); const edges = await kv.list<GraphEdge>(KV.graphEdges); const nodesByType: Record<string, number> = {}; for (const n of nodes) { nodesByType[n.type] = (nodesByType[n.type] || 0) + 1; } const edgesByType: Record<string, number> = {}; for (const e of edges) { edgesByType[e.type] = (edgesByType[e.type] || 0) + 1; } return { totalNodes: nodes.length, totalEdges: edges.length, + staleNodes: nodes.filter((n) => n.stale).length, + staleEdges: edges.filter((e) => e.stale).length, nodesByType, edgesByType, }; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/graph.ts` around lines 254 - 274, The mem::graph-stats function currently counts all GraphNode and GraphEdge entries from kv.list without distinguishing stale items; update the function (the sdk.registerFunction handler for "mem::graph-stats") to also compute staleNodes and staleEdges by iterating nodes and edges (the same loops that build nodesByType and edgesByType) and incrementing counters when item.stale (or the entity's stale flag) is truthy; return these two new fields alongside totalNodes, totalEdges, nodesByType and edgesByType so callers can see how many entities are marked stale.src/functions/remember.ts (1)
33-35: Validation partially addresses prior concern.The array check is good, but the past review also suggested validating that each element is a string. The defensive filter at lines 84-86 handles this, but rejecting malformed input early (with a clear error) would be cleaner than silently filtering.
💡 Stricter validation
- if (data.sourceObservationIds && !Array.isArray(data.sourceObservationIds)) { - return { success: false, error: "sourceObservationIds must be an array" }; - } + if ( + data.sourceObservationIds && + (!Array.isArray(data.sourceObservationIds) || + data.sourceObservationIds.some((id) => typeof id !== "string")) + ) { + return { success: false, error: "sourceObservationIds must be an array of strings" }; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/remember.ts` around lines 33 - 35, The current validation only checks that data.sourceObservationIds is an array but lets the later defensive filter silently drop non-string elements; change the initial validation (the block that checks data.sourceObservationIds) to also verify every element is a string (e.g., Array.isArray(data.sourceObservationIds) && data.sourceObservationIds.every(id => typeof id === 'string')) and if not return { success: false, error: "sourceObservationIds must be an array of strings" } so malformed input is rejected early instead of being filtered later.src/functions/mesh.ts (1)
166-167: Defensively copy scope arrays when storing peer config.Line 166 assigns
DEFAULT_SHARED_SCOPESby reference. Cloning here avoids accidental shared in-memory mutation between peer objects/tests.💡 Small hardening tweak
-sharedScopes: data.sharedScopes || DEFAULT_SHARED_SCOPES, +sharedScopes: data.sharedScopes ? [...data.sharedScopes] : [...DEFAULT_SHARED_SCOPES],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/mesh.ts` around lines 166 - 167, When assigning sharedScopes in the peer config (the sharedScopes: data.sharedScopes || DEFAULT_SHARED_SCOPES line), defensively clone the array instead of storing it by reference; if data.sharedScopes exists use a shallow copy (e.g. via slice/Array.from/spread) and if falling back to DEFAULT_SHARED_SCOPES return a cloned copy of that constant as well, so neither peer objects nor tests can mutate the original DEFAULT_SHARED_SCOPES in-place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/functions/cascade.ts`:
- Around line 44-45: When marking edges stale in cascade.ts (the block that sets
edge.stale = true and calls await kv.set(KV.graphEdges, edge.id, edge)), also
set a consistent updatedAt timestamp on the edge (e.g., edge.updatedAt =
Date.now() or the same timestamp used for nodes) before calling kv.set so edges
are updated the same way nodes are when marked stale.
In `@src/functions/mesh.ts`:
- Line 229: The pull/export request is missing the peer.syncFilter.project,
allowing project-scoped peers to pull unscoped data; update the code that calls
the remote export endpoint (the logic that posts to "/agentmemory/mesh/export")
to include peer.syncFilter.project in the request (e.g., append as a query param
or include in the request body), mirroring how collectSyncData uses
peer.syncFilter for push; ensure the same symbol peer.syncFilter.project is
passed when constructing the pull request so project-scoped peers only receive
project-scoped data.
---
Duplicate comments:
In `@src/triggers/api.ts`:
- Around line 1503-1506: Delta exports miss edges marked stale because
cascade.ts sets GraphEdge.stale=true but doesn't update GraphEdge.updatedAt;
update cascade.ts so when you set stale=true on a GraphEdge (the same place
GraphNode.updatedAt is set) also set updatedAt to the current timestamp, and
then change the delta filter in api.ts from df(graphEdges, "createdAt") to use
the updated timestamp (e.g., df(graphEdges, "updatedAt")) so graphEdges with
updatedAt > sinceTime are included; refer to the cascade function where stale is
assigned and to the df() call and body.graphEdges in api.ts.
---
Nitpick comments:
In `@src/functions/graph.ts`:
- Around line 254-274: The mem::graph-stats function currently counts all
GraphNode and GraphEdge entries from kv.list without distinguishing stale items;
update the function (the sdk.registerFunction handler for "mem::graph-stats") to
also compute staleNodes and staleEdges by iterating nodes and edges (the same
loops that build nodesByType and edgesByType) and incrementing counters when
item.stale (or the entity's stale flag) is truthy; return these two new fields
alongside totalNodes, totalEdges, nodesByType and edgesByType so callers can see
how many entities are marked stale.
In `@src/functions/mesh.ts`:
- Around line 166-167: When assigning sharedScopes in the peer config (the
sharedScopes: data.sharedScopes || DEFAULT_SHARED_SCOPES line), defensively
clone the array instead of storing it by reference; if data.sharedScopes exists
use a shallow copy (e.g. via slice/Array.from/spread) and if falling back to
DEFAULT_SHARED_SCOPES return a cloned copy of that constant as well, so neither
peer objects nor tests can mutate the original DEFAULT_SHARED_SCOPES in-place.
In `@src/functions/remember.ts`:
- Around line 33-35: The current validation only checks that
data.sourceObservationIds is an array but lets the later defensive filter
silently drop non-string elements; change the initial validation (the block that
checks data.sourceObservationIds) to also verify every element is a string
(e.g., Array.isArray(data.sourceObservationIds) &&
data.sourceObservationIds.every(id => typeof id === 'string')) and if not return
{ success: false, error: "sourceObservationIds must be an array of strings" } so
malformed input is rejected early instead of being filtered later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e672e89a-d141-4ca1-94b4-2f665da33997
📒 Files selected for processing (11)
src/functions/cascade.tssrc/functions/consolidate.tssrc/functions/graph-retrieval.tssrc/functions/graph.tssrc/functions/mesh.tssrc/functions/remember.tssrc/triggers/api.tstest/cascade.test.tstest/helpers/mocks.tstest/mcp-standalone.test.tstest/verify.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/verify.test.ts
Comment on lines
+44
to
+45
| edge.stale = true; | ||
| await kv.set(KV.graphEdges, edge.id, edge); |
There was a problem hiding this comment.
Set updatedAt on edges for consistency with nodes.
Graph nodes get updatedAt set when marked stale (line 33), but edges don't. This creates an inconsistency and affects delta exports.
🔧 Proposed fix
if (overlap) {
edge.stale = true;
+ edge.updatedAt = now;
await kv.set(KV.graphEdges, edge.id, edge);
flaggedEdges++;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| edge.stale = true; | |
| await kv.set(KV.graphEdges, edge.id, edge); | |
| edge.stale = true; | |
| edge.updatedAt = now; | |
| await kv.set(KV.graphEdges, edge.id, edge); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/cascade.ts` around lines 44 - 45, When marking edges stale in
cascade.ts (the block that sets edge.stale = true and calls await
kv.set(KV.graphEdges, edge.id, edge)), also set a consistent updatedAt timestamp
on the edge (e.g., edge.updatedAt = Date.now() or the same timestamp used for
nodes) before calling kv.set so edges are updated the same way nodes are when
marked stale.
|
|
||
| if (direction === "push" || direction === "both") { | ||
| const pushData = await collectSyncData(kv, scopes, peer.lastSyncAt); | ||
| const pushData = await collectSyncData(kv, scopes, peer.lastSyncAt, peer.syncFilter); |
There was a problem hiding this comment.
syncFilter.project is applied on push but not on pull export requests.
Line 229 uses peer.syncFilter for outbound collection, but Line 252 does not append project to /agentmemory/mesh/export. A project-scoped peer can still pull unscoped remote data.
🔧 Suggested fix
-const response = await fetch(
- `${peer.url}/agentmemory/mesh/export?since=${peer.lastSyncAt || ""}`,
- { signal: AbortSignal.timeout(30000), redirect: "error" },
-);
+const params = new URLSearchParams({ since: peer.lastSyncAt || "" });
+if (peer.syncFilter?.project) {
+ params.set("project", peer.syncFilter.project);
+}
+const response = await fetch(
+ `${peer.url}/agentmemory/mesh/export?${params.toString()}`,
+ { signal: AbortSignal.timeout(30000), redirect: "error" },
+);Also applies to: 249-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/mesh.ts` at line 229, The pull/export request is missing the
peer.syncFilter.project, allowing project-scoped peers to pull unscoped data;
update the code that calls the remote export endpoint (the logic that posts to
"/agentmemory/mesh/export") to include peer.syncFilter.project in the request
(e.g., append as a query param or include in the request body), mirroring how
collectSyncData uses peer.syncFilter for push; ensure the same symbol
peer.syncFilter.project is passed when constructing the pull request so
project-scoped peers only receive project-scoped data.
This was referenced Mar 27, 2026
This was referenced Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mem::verifyfunction traces a memory or observation back to its source observations and session context, returning provenance with confidence scores. Accessible via MCP toolmemory_verifyand RESTPOST /agentmemory/verify.mem::cascade-updatefunction automatically flags related graph nodes/edges as stale when a memory is superseded, and counts sibling memories sharing 2+ concepts. Fires automatically frommem::rememberafter supersede.memories,actions) to 7 (addingsemantic,procedural,relations,graph:nodes,graph:edges). AddedsyncFilterfor project-based action filtering. Refactored with genericlwwMergeListhelper anddeltaFilterfor all scopes.Type Changes
Memory: addedsourceObservationIds?: string[]CompressedObservation: addedconfidence?: number(0-1)GraphNode,GraphEdge: addedstale?: booleanMeshPeer: addedsyncFilter?: { project?: string }New Files
src/functions/verify.ts(~110 lines)src/functions/cascade.ts(~80 lines)test/verify.test.ts(6 tests)test/cascade.test.ts(8 tests)Modified Files
src/functions/compress.ts: setsconfidence: qualityScore / 100src/functions/remember.ts: populatessourceObservationIds, fires cascade on supersedesrc/functions/consolidate.ts: populatessourceObservationIdsfrom observation groupsrc/functions/mesh.ts: 7 scopes, syncFilter, generic LWW mergesrc/mcp/tools-registry.ts:memory_verifytool (38 total)src/mcp/server.ts: dispatch case for verifysrc/triggers/api.ts:POST /verify,POST /cascade-update, expanded mesh-exportsrc/index.ts: register new functionsTest plan
npx vitest run test/verify.test.ts— 6 tests passnpx vitest run test/cascade.test.ts— 8 tests passnpx vitest run test/mesh.test.ts— 30 tests pass (12 new)npx vitest run --exclude test/integration.test.ts— 573 tests pass across 49 filesnpx tsc --noEmit— no new type errorsSummary by CodeRabbit
New Features
Chores