From 7c389cbefc9c79ab22207caaca2508ea0ee6697b Mon Sep 17 00:00:00 2001 From: JG Tromp Date: Wed, 28 Oct 2026 12:51:59 +0100 Subject: [PATCH] fix(javascript): keep both-direction feature traces from crossing shared containers (#7743) * fix(javascript): keep both-direction feature traces from crossing shared containers trace_application_feature with direction "both" walked the application graph as if it were undirected. From a seed, one outgoing or incoming step into a shared container (an asset or the artifact root) followed by steps in the other direction reached every sibling of that container, so a trace seeded on one storage key returned the whole connected graph. "both" now returns the union of the outgoing and incoming closures. Each walk keeps its own predecessor chains, so a terminal path never changes direction midway. "outgoing" and "incoming" are unchanged. * test(javascript): check both-direction trace paths end to end The existing tests checked predecessor maps but no returned path. Add a trace whose seed reaches one storage terminal outgoing and another incoming, and assert each path's complete node_ids and edge_ids. A pathTo that reads only the first walk's map returns a one-node path for the incoming terminal and fails this test. * fix(javascript): confine only ascended containers in both-direction traces Making "both" the union of separate outgoing and incoming walks dropped modules that a feature reaches by crossing directions. In the Notes known-answer evaluation, the notes:export channel is pointed at by both the preload asset (invokes) and the handler (handles); its preload, renderer and csv modules are reached only through relationships that change direction, so the trace kept main.js alone and the feature_modules claim failed in tests/boundary/mcp/agentKnownAnswerMcp.test.ts. Walk both directions in one traversal again, and confine only the containment case the change was meant to stop: a node reached by stepping up a contains edge from one of its children follows only further contains steps up. A shared asset or artifact root then no longer leads into its other children, while relationships across the feature are kept. The Notes trace returns csv.js, main.js, preload.js and renderer.js again (30 nodes; the previous main returned the whole 32-node graph). outgoing and incoming traces are unchanged, and paths come from one predecessor map again, as before this branch. --- docs/javascript-application-workflows.md | 4 + .../javascriptFeatureTraversal.test.ts | 245 ++++++++++++++++++ .../javascript/javascriptFeatureTraversal.ts | 36 ++- 3 files changed, 277 insertions(+), 8 deletions(-) create mode 100644 src/domain/javascript/javascriptFeatureTraversal.test.ts diff --git a/docs/javascript-application-workflows.md b/docs/javascript-application-workflows.md index 4c746f5d3b..68280abf19 100644 --- a/docs/javascript-application-workflows.md +++ b/docs/javascript-application-workflows.md @@ -156,6 +156,10 @@ Select one literal seed kind: node ID, route, string, API, IPC channel, module, or native export. Matching is exact or literal substring matching; regular expressions and executable predicates are not accepted. The trace includes all matching seeds and the complete graph reachable in the selected direction. +`both` follows relationships in either direction, except that a container +reached through `contains` from one of its children only leads further up its +containment chain, so a shared asset or artifact root does not pull in its +other children. The result contains the matching basis, complete reachable subgraph, terminal paths, authority summaries, and native handoffs. A handoff binds diff --git a/src/domain/javascript/javascriptFeatureTraversal.test.ts b/src/domain/javascript/javascriptFeatureTraversal.test.ts new file mode 100644 index 0000000000..cb4282878d --- /dev/null +++ b/src/domain/javascript/javascriptFeatureTraversal.test.ts @@ -0,0 +1,245 @@ +import { describe, expect, it } from "vitest"; + +import { + createJavaScriptApplicationEdge, + createJavaScriptApplicationGraph, +} from "./javascriptApplicationGraph.js"; +import { + artifactEvidence, + completeCoverage, + fixtureNode, +} from "./javascriptApplicationGraph.fixture.js"; +import type { + ApplicationEdge, + ApplicationNode, +} from "./javascriptApplicationGraphSchemas.js"; +import { traceApplicationFeature } from "./javascriptFeatureTrace.js"; +import { traverseApplicationFeature } from "./javascriptFeatureTraversal.js"; + +const SHA = "a".repeat(64); + +const node = (kind: ApplicationNode["kind"], path: string): ApplicationNode => + fixtureNode( + kind, + { + strategy: "canonical-path", + stability: "artifact-version", + artifact_sha256: SHA, + path, + }, + path, + {}, + artifactEvidence(SHA, path), + ); + +const edge = ( + source: ApplicationNode, + target: ApplicationNode, + relation: ApplicationEdge["relation"], +): ApplicationEdge => + createJavaScriptApplicationEdge({ + source_node_id: source.node_id, + target_node_id: target.node_id, + relation, + properties: {}, + evidence: artifactEvidence(SHA, source.observations[0]?.label ?? "x"), + }); + +// One artifact holds two unrelated assets. Each module persists to its own +// storage key; the second module also calls an endpoint. +const root = node("artifact", "root"); +const firstAsset = node("javascript-asset", "first.js"); +const secondAsset = node("javascript-asset", "second.js"); +const firstModule = node("javascript-module", "first.js#module"); +const secondModule = node("javascript-module", "second.js#module"); +const firstKey = node("storage", "first-key"); +const secondKey = node("storage", "second-key"); +const endpoint = node("endpoint", "rpc-second"); +const graph = createJavaScriptApplicationGraph({ + schema: "JavaScriptApplicationGraph", + root_node_ids: [root.node_id], + nodes: [ + root, + firstAsset, + secondAsset, + firstModule, + secondModule, + firstKey, + secondKey, + endpoint, + ], + edges: [ + edge(root, firstAsset, "contains"), + edge(root, secondAsset, "contains"), + edge(firstAsset, firstModule, "exposes"), + edge(secondAsset, secondModule, "exposes"), + edge(firstModule, firstKey, "persists_to"), + edge(secondModule, secondKey, "persists_to"), + edge(secondModule, endpoint, "calls"), + ], + coverage: completeCoverage, + limitations: [], +}); + +const ids = (nodes: readonly ApplicationNode[]): string[] => + nodes.map(({ node_id: id }) => id).sort(); + +describe("traverseApplicationFeature", () => { + it("does not enter a shared container's other children for both", () => { + const both = traverseApplicationFeature(graph, [firstKey.node_id], "both"); + const incoming = traverseApplicationFeature( + graph, + [firstKey.node_id], + "incoming", + ); + const outgoing = traverseApplicationFeature( + graph, + [firstKey.node_id], + "outgoing", + ); + + expect(ids(both.nodes)).toEqual( + [...new Set([...ids(incoming.nodes), ...ids(outgoing.nodes)])].sort(), + ); + expect(ids(both.nodes)).toEqual( + ids([root, firstAsset, firstModule, firstKey]), + ); + expect(ids(both.nodes)).not.toContain(secondKey.node_id); + expect(ids(both.nodes)).not.toContain(endpoint.node_id); + }); + + it("keeps the shared container out of a sibling's trace paths", () => { + const trace = traceApplicationFeature({ + sourceEvidenceId: `ev_${"b".repeat(64)}`, + graph, + nativeEvidence: [], + seed: { + kind: "string", + value: "first-key", + match: "exact", + case_sensitive: true, + }, + direction: "both", + }); + + expect(trace.summary.traced_nodes).toBe(4); + expect(trace.paths.map(({ end_node_id: id }) => id)).toEqual([]); + }); + + it("follows a feature across relationships that point at it from both sides", () => { + // An IPC feature: the preload asset invokes the channel that the handler + // handles, and the main module loads the preload and requires a helper. + // Reaching the modules needs both edge directions; the shared root must + // not lead into the unrelated asset. + const preloadAsset = node("javascript-asset", "preload.js"); + const preloadModule = node("javascript-module", "preload.js#module"); + const mainAsset = node("javascript-asset", "main.js"); + const mainModule = node("javascript-module", "main.js#module"); + const helperModule = node("javascript-module", "csv.js#module"); + const channel = node("ipc-channel", "notes-export"); + const handler = node("ipc-handler", "notes-export-handler"); + const unrelatedAsset = node("javascript-asset", "unrelated.js"); + const unrelatedModule = node("javascript-module", "unrelated.js#module"); + const ipcGraph = createJavaScriptApplicationGraph({ + schema: "JavaScriptApplicationGraph", + root_node_ids: [root.node_id], + nodes: [ + root, + preloadAsset, + preloadModule, + mainAsset, + mainModule, + helperModule, + channel, + handler, + unrelatedAsset, + unrelatedModule, + ], + edges: [ + edge(root, preloadAsset, "contains"), + edge(root, mainAsset, "contains"), + edge(root, unrelatedAsset, "contains"), + edge(preloadAsset, preloadModule, "contains"), + edge(preloadAsset, channel, "invokes"), + edge(handler, channel, "handles"), + edge(mainAsset, handler, "contains"), + edge(mainAsset, mainModule, "contains"), + edge(mainModule, preloadAsset, "loads"), + edge(mainModule, helperModule, "imports"), + edge(unrelatedAsset, unrelatedModule, "contains"), + ], + coverage: completeCoverage, + limitations: [], + }); + + const traversal = traverseApplicationFeature( + ipcGraph, + [channel.node_id], + "both", + ); + + expect(ids(traversal.nodes)).toEqual( + ids([ + channel, + preloadAsset, + preloadModule, + handler, + mainAsset, + mainModule, + helperModule, + root, + ]), + ); + }); + + it("returns complete one-direction paths to terminals on both sides", () => { + // caller -> seed -> outgoing storage; incoming storage -> caller. + const seed = node("javascript-module", "seed.js#module"); + const caller = node("javascript-module", "caller.js#module"); + const outgoingKey = node("storage", "outgoing-key"); + const incomingKey = node("storage", "incoming-key"); + const persists = edge(seed, outgoingKey, "persists_to"); + const imports = edge(caller, seed, "imports"); + const calls = edge(incomingKey, caller, "calls"); + const trace = traceApplicationFeature({ + sourceEvidenceId: `ev_${"b".repeat(64)}`, + graph: createJavaScriptApplicationGraph({ + schema: "JavaScriptApplicationGraph", + root_node_ids: [caller.node_id], + nodes: [seed, caller, outgoingKey, incomingKey], + edges: [persists, imports, calls], + coverage: completeCoverage, + limitations: [], + }), + nativeEvidence: [], + seed: { + kind: "node-id", + value: seed.node_id, + match: "exact", + case_sensitive: true, + }, + direction: "both", + }); + + expect( + trace.paths + .map(({ end_node_id, node_ids, edge_ids }) => ({ + end_node_id, + node_ids, + edge_ids, + })) + .sort((left, right) => left.node_ids.length - right.node_ids.length), + ).toEqual([ + { + end_node_id: outgoingKey.node_id, + node_ids: [seed.node_id, outgoingKey.node_id], + edge_ids: [persists.edge_id], + }, + { + end_node_id: incomingKey.node_id, + node_ids: [seed.node_id, caller.node_id, incomingKey.node_id], + edge_ids: [imports.edge_id, calls.edge_id], + }, + ]); + }); +}); diff --git a/src/domain/javascript/javascriptFeatureTraversal.ts b/src/domain/javascript/javascriptFeatureTraversal.ts index 748314ba41..d6150e8487 100644 --- a/src/domain/javascript/javascriptFeatureTraversal.ts +++ b/src/domain/javascript/javascriptFeatureTraversal.ts @@ -8,6 +8,8 @@ import { compareUnicodeCodePoints } from "../unicodeCodePointOrder.js"; interface AdjacencyEntry { readonly edge: ApplicationEdge; readonly nextNodeId: string; + /** In "both", the step goes from a contained node up to its container. */ + readonly ascends: boolean; } interface Predecessor { @@ -33,20 +35,36 @@ export const traverseApplicationFeature = ( const visited = new Set(seedNodeIds); const retainedEdges = new Map(); const predecessors = new Map(); + // A feature crosses relationships in both directions: a renderer invokes a + // channel that its handler also points at. A container reached from one of + // its children only leads further up; expanding it would return every + // sibling in a shared asset or artifact root. + const ascendedOnly = new Set(); const queue = [...seedNodeIds]; for (let offset = 0; offset < queue.length; offset += 1) { const current = queue[offset]; if (current === undefined) continue; + const confined = ascendedOnly.has(current); for (const entry of adjacency.get(current) ?? []) { + if (confined && !entry.ascends) continue; retainedEdges.set(entry.edge.edge_id, entry.edge); - if (visited.has(entry.nextNodeId)) continue; - if (!nodeById.has(entry.nextNodeId)) continue; - visited.add(entry.nextNodeId); - predecessors.set(entry.nextNodeId, { - previousNodeId: current, - edgeId: entry.edge.edge_id, - }); - queue.push(entry.nextNodeId); + const next = entry.nextNodeId; + if (!nodeById.has(next)) continue; + const nextConfined = confined || entry.ascends; + // A node reached only from below is expanded again once it is reached + // another way. + if (visited.has(next) && (nextConfined || !ascendedOnly.has(next))) + continue; + if (!visited.has(next)) { + visited.add(next); + predecessors.set(next, { + previousNodeId: current, + edgeId: entry.edge.edge_id, + }); + } + if (nextConfined) ascendedOnly.add(next); + else ascendedOnly.delete(next); + queue.push(next); } } return { @@ -73,11 +91,13 @@ const buildAdjacency = ( addAdjacency(adjacency, edge.source_node_id, { edge, nextNodeId: edge.target_node_id, + ascends: false, }); if (direction !== "outgoing") addAdjacency(adjacency, edge.target_node_id, { edge, nextNodeId: edge.source_node_id, + ascends: direction === "both" && edge.relation === "contains", }); } for (const entries of adjacency.values())