chore(skills): move KG relationship tools from contacts to memory (#1502)

CHANGELOG.md

@@ -23,6 +23,7 @@ bus event types) are noted explicitly even in the `0.x` range.
 - **Tools vs skills vocabulary** — atoms are **tools** (`tool.json`, `ToolRegistry`; audit readers dual-match legacy `skill.*`); collections are **skills**. (#1485, #1489; ADR-031)
 - **Polymorphic pins** — a pin resolves a skill, a single tool, or an MCP-projected skill (per-tool `action_risk` preserved); `enable_task_management` retired for pinning `tasks` + `documents`; `tasks` gains `plan`/`checkpoint`, `memory` gains `decay-warnings-list`. (#1489, #1494; ADR-032)
 - **`approve-grant-recommendation`** — `action_risk` raised low→critical, matching direct permission grants. (#1499)
+- **KG relationship tools** — `query-relationships`/`delete-relationship` moved from the `contacts` skill to `memory`. (#1502)
 
 ### Fixed
 

agents/contacts.yaml

@@ -1,5 +1,5 @@
 name: contacts
-version: "0.11.0"
+version: "0.12.0"
 role: specialist
 description: >
   Contact domain specialist — briefings, CRUD, deduplication, relationship management,
@@ -311,6 +311,11 @@ pinned_skills:
   # Memory — query/store only (not memory-confirm / decay-warnings-list)
   - memory-query
   - memory-store
+  # KG relationship tools — moved to the `memory` skill (#1502); contacts still
+  # manages interpersonal edges (spouse, reports_to), so pin them individually here
+  # rather than pulling the whole memory bundle.
+  - query-relationships
+  - delete-relationship
   # Calendar — read-only, used for meeting history enrichment in briefings only.
   # Calendar specialist owns the domain; contacts keeps this read-only pin for briefing enrichment.
   - calendar-list-events

agents/coordinator.yaml

@@ -674,6 +674,8 @@ pinned_skills:
   - executive-profile
   - email
   - context-bridge
+  # memory bundle now also carries the KG relationship tools (query/delete edges,
+  # #1502); the coordinator intentionally gains them as the KG-central agent.
   - memory
   - scheduler
   - autonomy

skills/contacts/SKILL.md

@@ -1,8 +1,8 @@
 ---
 name: contacts
 description: >
-  Contact identity, lifecycle, grant recommendations, and relationship graph tools. Mixed action_risk (lookup=none … grant/revoke=critical) preserved per tool.
-version: "0.1.0"
+  Contact identity, lifecycle, and grant recommendations. Mixed action_risk (lookup=none … grant/revoke=critical) preserved per tool.
+version: "0.2.0"
 tools:
   - contact-create
   - contact-lookup
@@ -23,6 +23,4 @@ tools:
   - scan-grant-recommendations
   - approve-grant-recommendation
   - decline-grant-recommendation
-  - query-relationships
-  - delete-relationship
 ---

skills/memory/SKILL.md

@@ -1,11 +1,13 @@
 ---
 name: memory
 description: >
-  Shared knowledge-graph memory — query/store/confirm plus decay-warnings-list.
-version: "0.1.0"
+  Shared knowledge-graph memory — query/store/confirm facts, manage entity-to-entity relationships (query/delete edges), plus decay-warnings-list.
+version: "0.2.0"
 tools:
   - memory-query
   - memory-store
   - memory-confirm
   - decay-warnings-list
+  - query-relationships
+  - delete-relationship
 ---

skills/memory/tools/_shared/test-helpers.ts

@@ -0,0 +1,35 @@
+// Shared test scaffolding for the relationship-tool handler tests
+// (query-relationships, delete-relationship). Both build an in-memory EntityMemory
+// and a minimal ToolContext the same way; keep that in one place.
+//
+// NOTE: this directory has no tool.json, so skill discovery (discoverNestedToolNames)
+// skips it — it is not a tool.
+import pino from 'pino';
+import { KnowledgeGraphStore } from '../../../../src/memory/knowledge-graph.js';
+import { EmbeddingService } from '../../../../src/memory/embedding.js';
+import { EntityMemory } from '../../../../src/memory/entity-memory.js';
+import { MemoryValidator } from '../../../../src/memory/validation.js';
+import { createSilentLogger } from '../../../../src/logger.js';
+import type { ToolContext } from '../../../../src/skills/types.js';
+
+// Returns both mem and store for tests that need direct store access — e.g. to
+// simulate pre-migration duplicate data by bypassing upsert logic.
+export function makeEntityMemoryWithStore() {
+  const embeddingService = EmbeddingService.createForTesting();
+  const store = KnowledgeGraphStore.createInMemory(embeddingService);
+  const validator = new MemoryValidator(store, embeddingService);
+  return { mem: new EntityMemory(store, validator, embeddingService, createSilentLogger()), store };
+}
+
+export function makeEntityMemory(): EntityMemory {
+  return makeEntityMemoryWithStore().mem;
+}
+
+export function makeCtx(entityMemory: EntityMemory, input: Record<string, unknown>): ToolContext {
+  return {
+    input,
+    secret: () => 'test-key',
+    log: pino({ level: 'silent' }),
+    entityMemory,
+  } as unknown as ToolContext;
+}

skills/contacts/tools/delete-relationship/handler.test.ts → skills/memory/tools/delete-relationship/handler.test.ts

@@ -1,34 +1,6 @@
 import { describe, it, expect } from 'vitest';
-import pino from 'pino';
-import { KnowledgeGraphStore } from '../../../../src/memory/knowledge-graph.js';
-import { EmbeddingService } from '../../../../src/memory/embedding.js';
-import { EntityMemory } from '../../../../src/memory/entity-memory.js';
-import { MemoryValidator } from '../../../../src/memory/validation.js';
-import { createSilentLogger } from '../../../../src/logger.js';
 import { DeleteRelationshipHandler } from './handler.js';
-import type { ToolContext } from '../../../../src/skills/types.js';
-
-function makeEntityMemory() {
-  return makeEntityMemoryWithStore().mem;
-}
-
-// Returns both mem and store for tests that need to bypass upsert
-// (e.g. to simulate pre-migration duplicate data by inserting directly)
-function makeEntityMemoryWithStore() {
-  const embeddingService = EmbeddingService.createForTesting();
-  const store = KnowledgeGraphStore.createInMemory(embeddingService);
-  const validator = new MemoryValidator(store, embeddingService);
-  return { mem: new EntityMemory(store, validator, embeddingService, createSilentLogger()), store };
-}
-
-function makeCtx(entityMemory: EntityMemory, input: Record<string, unknown>): ToolContext {
-  return {
-    input,
-    secret: () => 'test-key',
-    log: pino({ level: 'silent' }),
-    entityMemory,
-  } as unknown as ToolContext;
-}
+import { makeEntityMemory, makeEntityMemoryWithStore, makeCtx } from '../_shared/test-helpers.js';
 
 describe('DeleteRelationshipHandler', () => {
   it('returns error for unknown predicate', async () => {

skills/contacts/tools/query-relationships/handler.test.ts → skills/memory/tools/query-relationships/handler.test.ts

@@ -1,34 +1,6 @@
 import { describe, it, expect } from 'vitest';
-import pino from 'pino';
-import { KnowledgeGraphStore } from '../../../../src/memory/knowledge-graph.js';
-import { EmbeddingService } from '../../../../src/memory/embedding.js';
-import { EntityMemory } from '../../../../src/memory/entity-memory.js';
-import { MemoryValidator } from '../../../../src/memory/validation.js';
-import { createSilentLogger } from '../../../../src/logger.js';
 import { QueryRelationshipsHandler } from './handler.js';
-import type { ToolContext } from '../../../../src/skills/types.js';
-
-// makeEntityMemoryWithStore returns both for tests that need direct store access
-// (e.g. to simulate pre-migration duplicates by bypassing upsert logic)
-function makeEntityMemoryWithStore() {
-  const embeddingService = EmbeddingService.createForTesting();
-  const store = KnowledgeGraphStore.createInMemory(embeddingService);
-  const validator = new MemoryValidator(store, embeddingService);
-  return { mem: new EntityMemory(store, validator, embeddingService, createSilentLogger()), store };
-}
-
-function makeEntityMemory() {
-  return makeEntityMemoryWithStore().mem;
-}
-
-function makeCtx(entityMemory: EntityMemory, input: Record<string, unknown>): ToolContext {
-  return {
-    input,
-    secret: () => 'test-key',
-    log: pino({ level: 'silent' }),
-    entityMemory,
-  } as unknown as ToolContext;
-}
+import { makeEntityMemory, makeEntityMemoryWithStore, makeCtx } from '../_shared/test-helpers.js';
 
 describe('QueryRelationshipsHandler', () => {
   it('returns empty relationships when entity is not found', async () => {
@@ -154,4 +126,21 @@ describe('QueryRelationshipsHandler', () => {
     expect(xiaopuData.relationships[0]!.subject).toBe('Jane Doe');
     expect(xiaopuData.relationships[0]!.object).toBe('John Smith');
   });
+
+  it('uses the resolved canonical label for the queried side, not the raw input casing', async () => {
+    const mem = makeEntityMemory();
+    const { entity: jane } = await mem.createEntity({ type: 'person', label: 'Jane Doe', properties: {}, source: 'test' });
+    const { entity: acme } = await mem.createEntity({ type: 'organization', label: 'Acme Corp', properties: {}, source: 'test' });
+    await mem.upsertEdge(jane.id, acme.id, 'member_of', {}, 'test', 0.8);
+
+    const handler = new QueryRelationshipsHandler();
+    // Query with a differently-cased label; resolution is case-insensitive, so this
+    // resolves to the 'Jane Doe' node whose canonical label must be what we return.
+    const ctx = makeCtx(mem, { entity: 'jane doe' });
+    const result = await handler.execute(ctx);
+
+    const data = (result as { success: true; data: { relationships: Array<{ subject: string; object: string }> } }).data;
+    expect(data.relationships[0]!.subject).toBe('Jane Doe'); // canonical, not 'jane doe'
+    expect(data.relationships[0]!.object).toBe('Acme Corp');
+  });
 });

skills/contacts/tools/query-relationships/handler.ts → skills/memory/tools/query-relationships/handler.ts

@@ -60,9 +60,12 @@ export class QueryRelationshipsHandler implements ToolHandler {
 
       const relationships = edges.map(({ edge, node, direction }) => ({
         edge_id: edge.id,
-        subject: direction === 'outbound' ? entity : node.label,
+        // Use the resolved node's canonical label for the queried side (not the raw
+        // `entity` input), so both sides read consistently — e.g. querying "jane doe"
+        // returns "Jane Doe" to match the KG, not the caller's casing.
+        subject: direction === 'outbound' ? entityNode.label : node.label,
         predicate: edge.type,
-        object: direction === 'outbound' ? node.label : entity,
+        object: direction === 'outbound' ? node.label : entityNode.label,
         direction,
         confidence: edge.temporal.confidence,
         last_confirmed_at: edge.temporal.lastConfirmedAt.toISOString(),

skills/contacts/tools/query-relationships/tool.json → skills/memory/tools/query-relationships/tool.json

@@ -1,7 +1,7 @@
 {
   "name": "query-relationships",
   "description": "Query entity-to-entity relationships from the knowledge graph. Resolves the entity by name. Returns all stored relationships, optionally filtered by edge type. When multiple nodes share the same name, returns an ambiguous response with candidates so you can ask the user to clarify.",
-  "version": "1.0.0",
+  "version": "1.0.1",
   "sensitivity": "normal",
   "action_risk": "none",
   "inputs": {

tests/unit/agents/resolved-pins-regression.test.ts

@@ -164,4 +164,38 @@ describe('resolved pin sets after #1494 bundling', () => {
     expect(config.pinned_skills).toContain('contact-update');
     expect(config.pinned_skills).not.toContain('contact-lookup');
   });
+
+  // #1502: the KG relationship tools moved from the contacts bundle to the memory
+  // bundle. They must NOT come along with the contacts bundle anymore, and the
+  // contacts agent keeps them only via its explicit individual pins (resolved set
+  // unchanged). The coordinator gains them because it pins the whole memory bundle.
+  it('query/delete-relationship live in the memory bundle, not contacts', () => {
+    const skills = new SkillRegistry();
+    loadOnDiskSkills(skills);
+    const memory = skills.get('memory');
+    const contacts = skills.get('contacts');
+    expect(memory?.manifest.tools).toEqual(
+      expect.arrayContaining(['query-relationships', 'delete-relationship']),
+    );
+    expect(contacts?.manifest.tools).not.toContain('query-relationships');
+    expect(contacts?.manifest.tools).not.toContain('delete-relationship');
+  });
+
+  it('contacts still resolves the relationship tools via explicit individual pins', () => {
+    const config = loadAgentConfig(resolve(agentsDir, 'contacts.yaml'));
+    // contacts pins the contacts bundle (which no longer carries them) AND the two
+    // tools individually — so removing them from the bundle is behavior-preserving.
+    expect(config.pinned_skills).toContain('query-relationships');
+    expect(config.pinned_skills).toContain('delete-relationship');
+    const tools = resolveAgent('contacts.yaml');
+    expect(tools).toContain('query-relationships');
+    expect(tools).toContain('delete-relationship');
+  });
+
+  it('coordinator gains the relationship tools via the memory bundle (#1502)', () => {
+    const tools = resolveAgent('coordinator.yaml');
+    expect(tools).toEqual(
+      expect.arrayContaining(['query-relationships', 'delete-relationship']),
+    );
+  });
 });