diff --git a/packages/node-ui/src/ui/hooks/useSwmAttributions.ts b/packages/node-ui/src/ui/hooks/useSwmAttributions.ts index 688f294e9..511c92364 100644 --- a/packages/node-ui/src/ui/hooks/useSwmAttributions.ts +++ b/packages/node-ui/src/ui/hooks/useSwmAttributions.ts @@ -180,7 +180,20 @@ function subGraphFromMetaGraphUri(gUri: string, cgId: string): string | undefine } export function buildAttributionsQuery(cgId: string): string { - const cgUri = `did:dkg:context-graph:${cgId}`; + // Trailing slash makes this an EXACT CG prefix: every partition graph is + // `did:dkg:context-graph:/[/]_shared_memory_meta`, so "/" + // matches all of this CG's partitions while excluding sibling CGs whose id + // merely shares the prefix (cg-1 must not capture cg-10 / cg-1-foo). + // + // Known limitation: CG ids may themselves contain "/" (validateContextGraphId + // allows it; convention is /), so a hypothetical path-extending + // child CG `/` would still prefix-match. That URI is structurally + // identical to a real sub-graph `=`, so only the sub-graph registry + // can disambiguate — i.e. the deferred server-side sub-graph SWM routing + // (rc.17 A2/A4). The raw-prefix read is the deliberate client bridge and is + // preferred over a client-built registry allow-list, which would re-drop + // partitions whose `_meta` registration is missing — a condition seen live. + const cgPrefix = `did:dkg:context-graph:${cgId}/`; return `PREFIX dkg: PREFIX prov: SELECT ?op ?root ?agent ?publishedAt ?g WHERE { @@ -191,7 +204,7 @@ SELECT ?op ?root ?agent ?publishedAt ?g WHERE { prov:wasAttributedTo ?agent . } FILTER( - STRSTARTS(STR(?g), "${cgUri}") && + STRSTARTS(STR(?g), "${cgPrefix}") && CONTAINS(STR(?g), "_shared_memory_meta") ) } ORDER BY DESC(?publishedAt) LIMIT 5000`; @@ -251,8 +264,14 @@ export function useSwmAttributions(contextGraphId: string | undefined): SwmAttri headers: { 'Content-Type': 'application/json', ...authHeaders() }, signal: controller.signal, body: JSON.stringify({ + // Do NOT send contextGraphId here. The query already scopes to the + // CG via its exact STRSTARTS(?g, "/") filter and must read EVERY + // sub-graph's //_shared_memory_meta partition. Passing + // contextGraphId makes the engine constrain GRAPH ?g to CG-direct + // graphs only, dropping per-sub-graph attribution so the legend + // under-counts agents (B2). See dkg-query-engine.ts graph-variable + // allow-list. sparql: buildAttributionsQuery(contextGraphId), - contextGraphId, }), }); if (!res.ok) throw new Error(`SPARQL query failed: ${res.status}`); diff --git a/packages/node-ui/src/ui/hooks/useVerifiedMemoryAnchors.ts b/packages/node-ui/src/ui/hooks/useVerifiedMemoryAnchors.ts index 3aa066bdd..37f49e667 100644 --- a/packages/node-ui/src/ui/hooks/useVerifiedMemoryAnchors.ts +++ b/packages/node-ui/src/ui/hooks/useVerifiedMemoryAnchors.ts @@ -111,7 +111,15 @@ function agentLabel(agentId: string): string { /** Build the SPARQL that enumerates every publish-batch WorkspaceOperation * across every sub-graph's `_shared_memory_meta`. */ function buildAnchorsQuery(cgId: string): string { - const cgUri = `did:dkg:context-graph:${cgId}`; + // Trailing slash makes this an EXACT CG prefix: every partition graph is + // `did:dkg:context-graph:/[/]_shared_memory_meta`, so "/" + // matches all of this CG's partitions while excluding sibling CGs whose id + // merely shares the prefix (cg-1 must not capture cg-10 / cg-1-foo). Same + // known limitation as useSwmAttributions.buildAttributionsQuery: a + // path-extending child CG `/` (CG ids may contain "/") would still + // prefix-match; the authoritative fix is the deferred server-side sub-graph + // SWM routing (A2/A4). + const cgPrefix = `did:dkg:context-graph:${cgId}/`; return `PREFIX dkg: PREFIX prov: SELECT ?op ?root ?agent ?publishedAt ?g WHERE { @@ -122,7 +130,7 @@ SELECT ?op ?root ?agent ?publishedAt ?g WHERE { prov:wasAttributedTo ?agent . } FILTER( - STRSTARTS(STR(?g), "${cgUri}") && + STRSTARTS(STR(?g), "${cgPrefix}") && CONTAINS(STR(?g), "_shared_memory_meta") ) } ORDER BY ?publishedAt LIMIT 2000`; @@ -234,8 +242,13 @@ export function useVerifiedMemoryAnchors( method: 'POST', headers: { 'Content-Type': 'application/json', ...authHeaders() }, body: JSON.stringify({ + // Do NOT send contextGraphId here. buildAnchorsQuery already scopes to + // the CG via its exact STRSTARTS(?g, "/") and enumerates EVERY sub-graph's + // //_shared_memory_meta partition. Passing contextGraphId makes + // the engine constrain GRAPH ?g to CG-direct graphs only, dropping + // per-sub-graph anchors/attribution (same bug class as B2). See + // dkg-query-engine.ts graph-variable allow-list. sparql: buildAnchorsQuery(contextGraphId), - contextGraphId, }), }); if (!res.ok) throw new Error(`SPARQL query failed: ${res.status}`); diff --git a/packages/node-ui/test/use-swm-attributions.test.ts b/packages/node-ui/test/use-swm-attributions.test.ts index db68ebdfd..fd09ca717 100644 --- a/packages/node-ui/test/use-swm-attributions.test.ts +++ b/packages/node-ui/test/use-swm-attributions.test.ts @@ -25,6 +25,18 @@ describe('useSwmAttributions — SPARQL query shape', () => { // the new DESC and the old `ORDER BY ?publishedAt` both present. expect(q).not.toMatch(/ORDER BY \?publishedAt\s+LIMIT/); }); + + // Codex review (PR #1055) — with contextGraphId removed from the request + // (B2), scoping rests entirely on this STRSTARTS prefix, so it MUST end in + // "/" to be exact. A bare "did:dkg:context-graph:cg-1" prefix would also + // match a sibling CG like cg-10 / cg-1-foo and merge its _shared_memory_meta + // attribution rows into the legend. + it('scopes via an exact "/" STRSTARTS prefix so sibling CGs do not leak in', () => { + const q = buildAttributionsQuery('cg-1'); + expect(q).toContain('STRSTARTS(STR(?g), "did:dkg:context-graph:cg-1/")'); + // The slash-less prefix would over-match cg-10 / cg-1-foo. + expect(q).not.toMatch(/STRSTARTS\(STR\(\?g\), "did:dkg:context-graph:cg-1"\)/); + }); }); // Codex Code7 (PR #656) — the hook returns its previous-graph result @@ -49,7 +61,14 @@ describe('useSwmAttributions — stale-on-switch protection', () => { originalFetch = globalThis.fetch; globalThis.fetch = vi.fn(async (_url: any, init?: any) => { const body = init?.body ? JSON.parse(String(init.body)) : {}; - const cgId: string = body.contextGraphId; + // B2: the POST body no longer carries contextGraphId (it scoped the + // engine to CG-direct graphs and dropped per-sub-graph attribution). + // Recover the cgId from the SPARQL's exact STRSTARTS prefix + // ("did:dkg:context-graph:/") to route the deferred promise. + // Capture up to the trailing `/"` (non-greedy) rather than `[^"/]+`, so + // canonical slash-containing ids like `/project` route under the + // full key instead of being truncated at the first `/`. + const cgId: string = String(body.sparql).match(/context-graph:(.+?)\/"/)?.[1] ?? ''; const p = new Promise((resolve) => { pending.set(cgId, { resolve }); }); @@ -89,39 +108,85 @@ describe('useSwmAttributions — stale-on-switch protection', () => { return null; } - // Initial render for cg-A. + // Initial render for acme/alpha. await act(async () => { - root.render(React.createElement(Probe, { id: 'cg-A' })); + root.render(React.createElement(Probe, { id: 'acme/alpha' })); }); await flushMicrotasks(); expect(latest!.resultContextGraphId).toBeUndefined(); expect(latest!.events).toHaveLength(0); - // Resolve cg-A's fetch. - pending.get('cg-A')!.resolve(rowsFor('cg-A')); + // Resolve acme/alpha's fetch. + pending.get('acme/alpha')!.resolve(rowsFor('acme/alpha')); await flushMicrotasks(); - expect(latest!.resultContextGraphId).toBe('cg-A'); + expect(latest!.resultContextGraphId).toBe('acme/alpha'); expect(latest!.events).toHaveLength(1); - expect(latest!.events[0].rootUri).toBe('urn:e:cg-A'); + expect(latest!.events[0].rootUri).toBe('urn:e:acme/alpha'); - // Switch to cg-B. The hook still holds cg-A's events until the + // Switch to acme/beta. The hook still holds acme/alpha's events until the // new SPARQL lands — that's the pre-existing behaviour. The fix // is the discriminator: callers can detect the mismatch and // suppress downstream rendering until it clears. await act(async () => { - root.render(React.createElement(Probe, { id: 'cg-B' })); + root.render(React.createElement(Probe, { id: 'acme/beta' })); }); await flushMicrotasks(); - // Before cg-B's fetch resolves, the result still describes cg-A. + // Before acme/beta's fetch resolves, the result still describes acme/alpha. // A consumer that gates on `resultContextGraphId === currentId` // would now suppress these events (they're for the wrong graph). - expect(latest!.resultContextGraphId).toBe('cg-A'); + expect(latest!.resultContextGraphId).toBe('acme/alpha'); - // Resolve cg-B; the discriminator catches up. - pending.get('cg-B')!.resolve(rowsFor('cg-B')); + // Resolve acme/beta; the discriminator catches up. + pending.get('acme/beta')!.resolve(rowsFor('acme/beta')); await flushMicrotasks(); - expect(latest!.resultContextGraphId).toBe('cg-B'); + expect(latest!.resultContextGraphId).toBe('acme/beta'); expect(latest!.events).toHaveLength(1); - expect(latest!.events[0].rootUri).toBe('urn:e:cg-B'); + expect(latest!.events[0].rootUri).toBe('urn:e:acme/beta'); + }); +}); + +// B2 (DKG-NODE-ISSUES-FOR-RC17) — the attribution fetch MUST NOT send +// contextGraphId. The SPARQL scopes itself to the CG via STRSTARTS(?g, …) +// and must read every sub-graph's //_shared_memory_meta partition; +// sending contextGraphId makes the daemon constrain GRAPH ?g to CG-direct +// graphs only, so the legend under-counted agents (showed 1 of 5 agents live). +describe('useSwmAttributions — B2 POST body scoping', () => { + let root: Root; + let container: HTMLDivElement; + let originalFetch: typeof globalThis.fetch | undefined; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + originalFetch = globalThis.fetch; + globalThis.fetch = vi.fn(async () => ({ + ok: true, + status: 200, + json: async () => ({ result: { bindings: [] } }), + } as any)) as any; + }); + + afterEach(async () => { + await act(async () => { root.unmount(); }); + container.remove(); + if (originalFetch) globalThis.fetch = originalFetch; + }); + + it('omits contextGraphId from the /api/query POST body so all sub-graph partitions are reached', async () => { + function Probe({ id }: { id: string }) { + useSwmAttributions(id); + return null; + } + await act(async () => { + root.render(React.createElement(Probe, { id: 'cg-1' })); + }); + await act(async () => { await new Promise((r) => setTimeout(r, 0)); }); + + const calls = vi.mocked(fetch).mock.calls; + expect(calls.length).toBeGreaterThan(0); + const body = JSON.parse(String((calls[0][1] as any)?.body ?? '{}')); + expect(body.sparql).toContain('_shared_memory_meta'); + expect(body).not.toHaveProperty('contextGraphId'); }); }); diff --git a/packages/node-ui/test/use-verified-memory-anchors.test.ts b/packages/node-ui/test/use-verified-memory-anchors.test.ts new file mode 100644 index 000000000..f9a0c8663 --- /dev/null +++ b/packages/node-ui/test/use-verified-memory-anchors.test.ts @@ -0,0 +1,57 @@ +// @vitest-environment happy-dom + +import React, { act } from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createRoot, type Root } from 'react-dom/client'; +import { useVerifiedMemoryAnchors } from '../src/ui/hooks/useVerifiedMemoryAnchors.js'; + +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +// Same bug class as B2 (DKG-NODE-ISSUES-FOR-RC17): buildAnchorsQuery already +// scopes to the CG via STRSTARTS(?g, …) and enumerates EVERY sub-graph's +// //_shared_memory_meta partition, so the fetch MUST NOT also send +// contextGraphId — that makes the daemon constrain GRAPH ?g to CG-direct +// graphs only, dropping per-sub-graph anchors/attribution. +describe('useVerifiedMemoryAnchors — POST body scoping', () => { + let root: Root; + let container: HTMLDivElement; + let originalFetch: typeof globalThis.fetch | undefined; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + originalFetch = globalThis.fetch; + globalThis.fetch = vi.fn(async () => ({ + ok: true, + status: 200, + json: async () => ({ result: { bindings: [] } }), + } as any)) as any; + }); + + afterEach(async () => { + await act(async () => { root.unmount(); }); + container.remove(); + if (originalFetch) globalThis.fetch = originalFetch; + }); + + it('omits contextGraphId from the /api/query POST body so all sub-graph partitions are reached', async () => { + function Probe({ id }: { id: string }) { + useVerifiedMemoryAnchors(id); + return null; + } + await act(async () => { + root.render(React.createElement(Probe, { id: 'cg-1' })); + }); + await act(async () => { await new Promise((r) => setTimeout(r, 0)); }); + + const calls = vi.mocked(fetch).mock.calls; + expect(calls.length).toBeGreaterThan(0); + const body = JSON.parse(String((calls[0][1] as any)?.body ?? '{}')); + expect(body.sparql).toContain('_shared_memory_meta'); + expect(body).not.toHaveProperty('contextGraphId'); + // Codex review (PR #1055) — exact "/" prefix so a sibling CG + // (cg-10 / cg-1-foo) can't leak its anchors in once contextGraphId is gone. + expect(body.sparql).toContain('STRSTARTS(STR(?g), "did:dkg:context-graph:cg-1/")'); + }); +});