diff --git a/CHANGELOG.md b/CHANGELOG.md index 75020e8..56278d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ All notable changes to engram are documented here. The format follows ## [Unreleased] +### Fixed +- Tenant-scope isolation batch (#106, #107, #108, #109, #110, #111, #112). `memory_suppressions` is now keyed on `(content_hash, scope)` — new checkpointed migration `suppression_scope_v1`, additive for an older build — so one tenant's `forget` no longer suppresses dream re-extraction of the same sentence for every tenant, and only a matching-scope `remember` (or a global one) lifts it; `filterSuppressedFacts` takes the conversation's scope, `clearSuppression`/`restore` delete only their own scope's row, and a `global` suppression still applies everywhere. `resolveCallScoping` treats `ENGRAM_READ_SCOPES` / `ENGRAM_SCOPE` as a ceiling: a call's `read_scopes` is intersected with the env list (no overlap → error) and a call's `scope` must be one of the readable scopes, so an env-pinned stdio child can no longer read or write another tenant with one argument (an unpinned server — the shared HTTP daemon — is unchanged). `forget`'s `scope: "global"` override is honoured only on a server the env does not pin. Recall sessions keep the scopes they were created with, so a refine cannot merge another tenant's results into a session. `linkMemoryToEntities` stamps new `related_to` edges with the caller's scope and widens an existing edge seen from a second scope, instead of always writing `global`. `scopeInClause` coalesces a NULL scope column to `'global'` like `scopeVisible` does. `ftsIndexedRowids` reads `memories_fts_docsize` instead of an `fts5vocab('instance')` scan, so a forgotten memory whose text tokenizes to nothing is audited and repaired by `engram validate --fix`. + ## [0.4.0] - 2026-09-18 First release that ships the Claude Code plugin, the `engram mcp` daemon bridge, `engram mcp install`, `forget`, `engram setup`, `doctor --fix` and `update --rollback` (PRD issues #50 #53 #55 #58 #61 #63 #65) plus the Dependabot majors (#68–#73, #72). The 0.3.0 tarball on npm predates all of it, so the plugin's `npx -y @devinmlowe/engram@ mcp` pin only works from this version on. Windows + Node 22 note: see the better-sqlite3 13 entry below (#84). diff --git a/CLAUDE.md b/CLAUDE.md index d6072f5..6e6ac63 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,7 +22,7 @@ Shared infrastructure in `_core/` (config, db, types, embeddings, search, llm, c - **show** — Retrieve full conversation or memory context - **explore** — Fixed-depth graph traversal from an entity - **reflect** — Graph analysis — communities, bridges, temporal patterns (health reports `stale_nodes` — entities flagged by `forget`, pruned on the next dream run) -- **forget** — Remove a memory the user says is wrong or stale (#55). `memory_id` (the `id` attribute on every recalled ``) acts in one call; `query` runs semantic search and returns candidates with ids, acting only with `confirm: true` and exactly one match (a single result, or one whose content equals the query). Soft delete (#56): `is_active = 0` + `deleted_at`/`deleted_by`, vector + FTS rows removed immediately, a `memory_changes` row (`before` = content, actor = the MCP client's `clientInfo.name`), and a `memory_suppressions` content hash so dream extract does not re-extract it; the dream prune phase hard-deletes after `ENGRAM_FORGET_RETENTION_DAYS`. `hard: true` deletes outright. Graph (#57): decrements `mention_count` on evidenced entities and stamps `stale_since` at zero; never deletes graph rows (`pruneOrphanEntities` does, next run). `readOnlyHint: false`, `destructiveHint: true`; honours `read_scopes` unless `scope: "global"`. +- **forget** — Remove a memory the user says is wrong or stale (#55). `memory_id` (the `id` attribute on every recalled ``) acts in one call; `query` runs semantic search and returns candidates with ids, acting only with `confirm: true` and exactly one match (a single result, or one whose content equals the query). Soft delete (#56): `is_active = 0` + `deleted_at`/`deleted_by`, vector + FTS rows removed immediately, a `memory_changes` row (`before` = content, actor = the MCP client's `clientInfo.name`), and a `memory_suppressions` row keyed `(content_hash, scope)` so dream extract does not re-extract it in that scope (#106; a `global` suppression applies everywhere); the dream prune phase hard-deletes after `ENGRAM_FORGET_RETENTION_DAYS`. `hard: true` deletes outright. Graph (#57): decrements `mention_count` on evidenced entities and stamps `stale_since` at zero; never deletes graph rows (`pruneOrphanEntities` does, next run). `readOnlyHint: false`, `destructiveHint: true`; honours `read_scopes` unless `scope: "global"` — and that override is ignored on a server the env pins to a tenant (#109). ### Memory & Knowledge (Phase 6 RLM) @@ -46,7 +46,7 @@ Shared infrastructure in `_core/` (config, db, types, embeddings, search, llm, c - **ingest_turn** — Record one user/assistant turn of an external agent session (`session_id`, `turn_index`, `scope`, `user_text`, `assistant_text`); idempotent upsert, extracted memories inherit the scope. 16 tools total; `src/interfaces/mcp/tool-names.ts` is canonical. -Recall tools (`recall`, `recall_session`, `recall_drill`) reinforce returned memories (FSRS bookkeeping only) and keep `readOnlyHint: true`; `reinforce: false` opts out. `recall`/`recall_session`/`remember`/`remember_batch`/`explore`/`explore_selective`/`commitments` accept per-call `scope` / `read_scopes` over the `ENGRAM_SCOPE` / `ENGRAM_READ_SCOPES` defaults. Since #25 `scope` lives on memories, conversations, exchanges, entities, relationships and commitments (`src/_core/db/scope.ts`): episodic recall, graph search, explore and the commitments ledger filter by `read_scopes`; dream extraction stamps entities/relationships/commitments with the conversation's scope, and a graph row seen from a second scope widens to `global`. `reflect` stays global. +Recall tools (`recall`, `recall_session`, `recall_drill`) reinforce returned memories (FSRS bookkeeping only) and keep `readOnlyHint: true`; `reinforce: false` opts out. `recall`/`recall_session`/`remember`/`remember_batch`/`explore`/`explore_selective`/`commitments` accept per-call `scope` / `read_scopes` over the `ENGRAM_SCOPE` / `ENGRAM_READ_SCOPES` defaults. #108: when the env sets them it is a ceiling, not a default — a call's `read_scopes` is intersected with it (no overlap → error) and a call's `scope` must be one of the readable scopes; with no env scoping the params are the tenant identity (shared HTTP daemon). A recall session keeps the scopes it was created with (#110). Since #25 `scope` lives on memories, conversations, exchanges, entities, relationships and commitments (`src/_core/db/scope.ts`): episodic recall, graph search, explore and the commitments ledger filter by `read_scopes`; dream extraction stamps entities/relationships/commitments with the conversation's scope, and a graph row seen from a second scope widens to `global`. `reflect` stays global. ### Recommended Workflow diff --git a/src/_core/db/index.ts b/src/_core/db/index.ts index 303d4da..023b727 100644 --- a/src/_core/db/index.ts +++ b/src/_core/db/index.ts @@ -12,8 +12,10 @@ export { backfillEventTs, migrateCommitments, migrateForget, + migrateSuppressionScope, COMMITMENTS_MIGRATION, FORGET_MIGRATION, + SUPPRESSION_SCOPE_MIGRATION, MEMORY_CHANGE_OPS, EVENT_TS_SUBQUERY, SCHEMA_MIGRATIONS, diff --git a/src/_core/db/schema.ts b/src/_core/db/schema.ts index 5051b3e..09eccb5 100644 --- a/src/_core/db/schema.ts +++ b/src/_core/db/schema.ts @@ -398,6 +398,7 @@ function createSchema(db: Database.Database, config: EngramConfig): void { // the graph `stale_since` flags (#56/#57). Must run AFTER // migrateExpandedTypes (entities/relationships rebuild). migrateForget(db); + migrateSuppressionScope(db); // FTS5 virtual tables (created separately — can't use IF NOT EXISTS) createFtsIfNeeded(db, "exchanges_fts", ` @@ -785,10 +786,11 @@ export function migrateForget(db: Database.Database): boolean { CREATE INDEX IF NOT EXISTS idx_memory_changes_at ON memory_changes(at); CREATE TABLE IF NOT EXISTS memory_suppressions ( - content_hash TEXT PRIMARY KEY, + content_hash TEXT NOT NULL, memory_id TEXT, - scope TEXT DEFAULT 'global', - created_at TEXT NOT NULL + scope TEXT NOT NULL DEFAULT 'global', + created_at TEXT NOT NULL, + PRIMARY KEY (content_hash, scope) ); `); if (!hadChanges || !hadSuppressions) added = true; @@ -796,6 +798,49 @@ export function migrateForget(db: Database.Database): boolean { return added; } +/** Checkpoint name recorded when memory_suppressions becomes per-scope. */ +export const SUPPRESSION_SCOPE_MIGRATION = "suppression_scope_v1"; + +/** + * #106: `memory_suppressions` was keyed on `content_hash` alone, so one + * tenant's forget suppressed dream re-extraction of the same sentence for + * every tenant (and any tenant's remember lifted it). Rebuild the table with + * `PRIMARY KEY (content_hash, scope)`, carrying the existing rows over. + * Additive for an older build: the columns are unchanged. + */ +export function migrateSuppressionScope(db: Database.Database): boolean { + db.exec(` + CREATE TABLE IF NOT EXISTS schema_migrations ( + name TEXT PRIMARY KEY, + applied_at INTEGER DEFAULT (unixepoch()) + ) + `); + const ddl = db + .prepare("SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'memory_suppressions'") + .get() as { sql: string } | undefined; + let added = false; + if (ddl && !/PRIMARY KEY \(content_hash, scope\)/.test(ddl.sql)) { + db.transaction(() => { + db.exec(` + CREATE TABLE memory_suppressions_new ( + content_hash TEXT NOT NULL, + memory_id TEXT, + scope TEXT NOT NULL DEFAULT 'global', + created_at TEXT NOT NULL, + PRIMARY KEY (content_hash, scope) + ); + INSERT OR REPLACE INTO memory_suppressions_new (content_hash, memory_id, scope, created_at) + SELECT content_hash, memory_id, COALESCE(scope, 'global'), created_at FROM memory_suppressions; + DROP TABLE memory_suppressions; + ALTER TABLE memory_suppressions_new RENAME TO memory_suppressions; + `); + }).immediate(); + added = true; + } + db.prepare("INSERT OR IGNORE INTO schema_migrations (name) VALUES (?)").run(SUPPRESSION_SCOPE_MIGRATION); + return added; +} + // ─── schema version (#65) ──────────────────────────────────────────── /** @@ -810,9 +855,10 @@ export const SCHEMA_MIGRATIONS = [ CONVERSATIONS_SCOPE_MIGRATION, EXCHANGES_AUTHOR_MIGRATION, FORGET_MIGRATION, + SUPPRESSION_SCOPE_MIGRATION, ] as const; -/** Number of checkpointed migrations this build applies (5 as of `forget_v1`). */ +/** Number of checkpointed migrations this build applies (6 as of `suppression_scope_v1`). */ export const SCHEMA_VERSION: number = SCHEMA_MIGRATIONS.length; /** diff --git a/src/_core/db/scope.ts b/src/_core/db/scope.ts index a01dabe..ac46a2b 100644 --- a/src/_core/db/scope.ts +++ b/src/_core/db/scope.ts @@ -1,9 +1,9 @@ /** * Tenant-scope helpers shared by every scoped table (#25). * - * Rows carry `scope` ('global' or e.g. 'hermes:career'). A read with - * `scopes` set sees only rows whose scope is in the list; without it, every - * scope (single-tenant behaviour). Graph rows are shared knowledge: an + * Rows carry `scope` ('global' or e.g. 'hermes:career'; a NULL column reads + * as 'global'). A read with `scopes` set sees only rows whose scope is in the + * list; without it, every scope (single-tenant behaviour). Graph rows are shared knowledge: an * entity or edge first seen from one profile and later from another is * *widened* to 'global' rather than duplicated per tenant. */ @@ -16,10 +16,19 @@ export function scopeVisible(rowScope: string | null | undefined, scopes?: reado return scopes.includes(rowScope ?? GLOBAL_SCOPE); } -/** `col IN (?, ?)` fragment + params, or null when unfiltered. */ +/** + * `COALESCE(col, 'global') IN (?, ?)` fragment + params, or null when + * unfiltered. #107: the COALESCE keeps this in step with `scopeVisible`, + * which reads a NULL scope as 'global'; a bare `col IN (...)` is never true + * for NULL, so such a row would be visible to one reader and hidden from the + * other. + */ export function scopeInClause(col: string, scopes?: readonly string[]): { sql: string; params: string[] } | null { if (!scopes || scopes.length === 0) return null; - return { sql: `${col} IN (${scopes.map(() => "?").join(", ")})`, params: [...scopes] }; + return { + sql: `COALESCE(${col}, '${GLOBAL_SCOPE}') IN (${scopes.map(() => "?").join(", ")})`, + params: [...scopes], + }; } /** diff --git a/src/_core/search/session.ts b/src/_core/search/session.ts index 1ade745..284939c 100644 --- a/src/_core/search/session.ts +++ b/src/_core/search/session.ts @@ -50,6 +50,12 @@ export interface RecallSession { expandedIds: Set; totalBudgetUsed: number; maxBudget: number; + /** + * Tenant scopes the session was created with (#110). Every refine reuses + * them, so results from another tenant can never be merged into a session + * a scoped caller later drills into. Undefined = unscoped (single-tenant). + */ + scopes?: string[]; } export interface DrillResult { @@ -79,7 +85,7 @@ export class SessionStore { /** * Create a new recall session. */ - create(query: string, options?: { maxBudget?: number }): RecallSession { + create(query: string, options?: { maxBudget?: number; scopes?: string[] }): RecallSession { this.evictExpired(); // Evict oldest (LRU) if at capacity @@ -99,6 +105,7 @@ export class SessionStore { expandedIds: new Set(), totalBudgetUsed: 0, maxBudget: options?.maxBudget ?? 3000, + scopes: options?.scopes, }; this.sessions.set(id, session); diff --git a/src/dream/daemon.ts b/src/dream/daemon.ts index 90c053a..fe18c67 100644 --- a/src/dream/daemon.ts +++ b/src/dream/daemon.ts @@ -971,7 +971,8 @@ function getConversationScope(db: Database.Database, conversationId: string): st /** * #55: drop extracted facts whose content hash is in memory_suppressions — a * statement the user forgot must not come back from the same exchanges on - * the next run. Counted in the report as `suppressedFacts`. + * the next run. Counted in the report as `suppressedFacts`. #106: only this + * conversation's own scope (and 'global') suppresses. */ function dropSuppressed( db: Database.Database, @@ -980,7 +981,7 @@ function dropSuppressed( logPath: string, report: DreamReport, ): ExtractedFact[] { - const { kept, suppressed } = filterSuppressedFacts(db, facts); + const { kept, suppressed } = filterSuppressedFacts(db, facts, getConversationScope(db, conversationId)); if (suppressed.length > 0) { report.suppressedFacts = (report.suppressedFacts ?? 0) + suppressed.length; logEntry(logPath, "extract", `Suppressed ${suppressed.length} forgotten fact(s) re-extracted from ${conversationId}`, { diff --git a/src/interfaces/mcp/scoping.ts b/src/interfaces/mcp/scoping.ts index 16ff16d..6ab2b36 100644 --- a/src/interfaces/mcp/scoping.ts +++ b/src/interfaces/mcp/scoping.ts @@ -68,6 +68,12 @@ function requireScope(value: string, param: string): string { * - `scope` overrides ENGRAM_SCOPE. When ENGRAM_READ_SCOPES is unset it also * re-derives the read default to global + own, mirroring the env rule. * - `read_scopes` overrides ENGRAM_READ_SCOPES (and the derived default). + * + * #108: the env is a ceiling, never a default the client may raise. When the + * env restricts reads, `read_scopes` is intersected with it (empty → throw) + * and `scope` must be one of the readable scopes (write only where you may + * read). With no env restriction the params are the only tenant identity + * (shared HTTP daemon) and apply as given. */ export function resolveCallScoping( env: Record, @@ -77,12 +83,23 @@ export function resolveCallScoping( const writeScope = params.scope !== undefined ? requireScope(params.scope, "scope") : base.writeScope; - let readScopes = base.readScopes; + const allowed = base.readScopes; + if (allowed && writeScope !== undefined && params.scope !== undefined && !allowed.includes(writeScope)) { + throw new Error(`scope "${writeScope}" is outside this server's read scopes (${allowed.join(", ")})`); + } + + let readScopes = allowed; if (params.read_scopes !== undefined) { if (!Array.isArray(params.read_scopes) || params.read_scopes.length === 0) { throw new Error("read_scopes must contain at least one scope"); } readScopes = params.read_scopes.map((s) => requireScope(s, "read_scopes")); + if (allowed) { + readScopes = readScopes.filter((s) => allowed.includes(s)); + if (readScopes.length === 0) { + throw new Error(`read_scopes has no scope in common with this server's read scopes (${allowed.join(", ")})`); + } + } } else if (params.scope !== undefined && !env.ENGRAM_READ_SCOPES?.trim()) { readScopes = ["global", writeScope as string]; } diff --git a/src/interfaces/mcp/server.ts b/src/interfaces/mcp/server.ts index 14fef12..0f8987a 100644 --- a/src/interfaces/mcp/server.ts +++ b/src/interfaces/mcp/server.ts @@ -30,7 +30,7 @@ import { updateHealthField } from "../cli/update-check.js"; import { escapeXml } from "../../_core/search/index.js"; import { initEmbeddings } from "../../_core/embeddings/index.js"; import { rememberFact, storeMemoryBatch } from "../shared/remember.js"; -import { resolveCallScoping } from "./scoping.js"; +import { getTenantScoping, resolveCallScoping } from "./scoping.js"; import { ingestTurn, DEFAULT_TURN_SOURCE } from "../../episodic/ingest-turn.js"; import { forgetMemory, @@ -1292,7 +1292,8 @@ export const MCP_TOOL_DEFINITIONS: Tool[] = [ "out of every recall path immediately, is logged in the change log with " + "this client's name, and will not be re-extracted from the same " + "conversation. hard: true deletes it outright. Only memories within " + - "read_scopes can be forgotten unless scope is \"global\".", + "read_scopes can be forgotten; scope \"global\" reaches any scope, but " + + "only on a server whose env does not pin it to one tenant.", inputSchema: { type: "object", properties: { @@ -1321,7 +1322,8 @@ export const MCP_TOOL_DEFINITIONS: Tool[] = [ minLength: 1, description: "Tenant identity for this call (e.g. \"hermes:career\"); reads default to " + - "global + this scope. \"global\" acts on a memory in any scope.", + "global + this scope. \"global\" acts on a memory in any scope " + + "(unless the server env pins it to one tenant).", }, read_scopes: { type: "array", @@ -1894,6 +1896,11 @@ async function handleForget(params: ForgetParams, context?: ToolCallContext): Pr const actor = context?.clientName ?? UNKNOWN_MCP_ACTOR; const scoping = resolveCallScoping(process.env, params); const db = getDb(); + // #109: `scope: "global"` acts on a memory in any scope. Honour that only + // for a server the env does not pin to a tenant — otherwise an env-pinned + // child (Hermes stdio) could delete another tenant's memories with one arg. + const envPinned = getTenantScoping(process.env).readScopes !== undefined; + const callScope = envPinned ? undefined : params.scope; const act = (memoryId: string): ToolResult => { const result = forgetMemory(db, { @@ -1901,7 +1908,7 @@ async function handleForget(params: ForgetParams, context?: ToolCallContext): Pr actor, hard: params.hard, readScopes: scoping.readScopes, - scope: params.scope, + scope: callScope, }); return { content: [{ type: "text", text: formatForgottenXml(result, actor) }] }; }; @@ -1923,7 +1930,7 @@ async function handleForget(params: ForgetParams, context?: ToolCallContext): Pr mode: "hybrid", limit: FORGET_CANDIDATE_LIMIT, budget: 4000, - scopes: params.scope === "global" ? undefined : scoping.readScopes, + scopes: callScope === "global" ? undefined : scoping.readScopes, reinforce: false, }, config, diff --git a/src/interfaces/shared/remember.ts b/src/interfaces/shared/remember.ts index f4d45d9..44b0a49 100644 --- a/src/interfaces/shared/remember.ts +++ b/src/interfaces/shared/remember.ts @@ -18,6 +18,7 @@ import { } from "../../semantic/memory.js"; import { insertVector } from "../../_core/db/index.js"; import { clearSuppression } from "../../semantic/forget.js"; +import { GLOBAL_SCOPE, widenScope } from "../../_core/db/scope.js"; import { generateStructured } from "../../_core/llm/index.js"; import type { IntelligenceConfig } from "../../_core/llm/index.js"; @@ -158,7 +159,7 @@ export async function rememberFact( // statement the user once forgot lifts its extraction suppression (#55). const newId = crypto.randomUUID(); const now = Math.floor(Date.now() / 1000); - clearSuppression(db, params.content); + clearSuppression(db, params.content, scope); insertMemory( db, @@ -295,7 +296,7 @@ export async function storeMemoryBatch( if (!isDuplicate) { const newId = crypto.randomUUID(); const now = Math.floor(Date.now() / 1000); - clearSuppression(db, input.content); + clearSuppression(db, input.content, scope); insertMemory( db, @@ -350,7 +351,7 @@ export async function storeMemoryBatch( // The memory is already committed; a linking failure must be reported // on this item, not thrown past N persisted memories as a tool error try { - const linked = linkMemoryToEntities(db, detail.id, input.relates_to_entities); + const linked = linkMemoryToEntities(db, detail.id, input.relates_to_entities, input.scope ?? opts?.scope); result.entitiesLinked += linked; } catch (err) { result.errors++; @@ -371,7 +372,9 @@ export async function storeMemoryBatch( * 1. Find the entity by name (case-insensitive) * 2. Bump its mention_count and update last_seen * 3. For each pair of found entities, create or update a `related_to` - * relationship with the memory ID tracked in source_memories + * relationship with the memory ID tracked in source_memories, stamped with + * the caller's `scope` (#111); an existing edge seen from a second scope + * widens to 'global' like every other graph write * * Returns the number of entities found and linked. */ @@ -379,6 +382,7 @@ export function linkMemoryToEntities( db: Database.Database, memoryId: string, entityNames: string[], + scope?: string, ): number { // Find all matching entities (limit to 10) const entities: Array<{ id: string; name: string }> = []; @@ -418,13 +422,15 @@ export function linkMemoryToEntities( db.prepare( "UPDATE relationships SET source_memories = ?, weight = weight + 0.5, updated_at = unixepoch(), stale_since = NULL WHERE id = ?", ).run(JSON.stringify(memories), existing.id); + // #25: an edge seen from a second profile is shared knowledge + widenScope(db, "relationships", existing.id, scope); } else { - // Create new relationship + // Create new relationship in the caller's scope (#111) const relId = crypto.randomUUID(); db.prepare( - `INSERT INTO relationships (id, source_entity_id, target_entity_id, type, weight, source_memories, created_at) - VALUES (?, ?, ?, 'related_to', 1.0, ?, unixepoch())`, - ).run(relId, sourceId, targetId, JSON.stringify([memoryId])); + `INSERT INTO relationships (id, source_entity_id, target_entity_id, type, weight, source_memories, created_at, scope) + VALUES (?, ?, ?, 'related_to', 1.0, ?, unixepoch(), ?)`, + ).run(relId, sourceId, targetId, JSON.stringify([memoryId]), scope ?? GLOBAL_SCOPE); } } } diff --git a/src/interfaces/shared/search.ts b/src/interfaces/shared/search.ts index 87df043..8891076 100644 --- a/src/interfaces/shared/search.ts +++ b/src/interfaces/shared/search.ts @@ -210,6 +210,8 @@ export async function createOrRefineRecallSession( }; } + // #110: the session's own scopes, not this call's — a refine must not + // widen (or narrow) what the session already holds. const response = await searchMultiSource( db, { @@ -217,7 +219,7 @@ export async function createOrRefineRecallSession( sources, mode: "hybrid", budget: remainingBudget, - scopes: params.scopes, + scopes: session.scopes, }, config, ); @@ -236,7 +238,7 @@ export async function createOrRefineRecallSession( // Create new session const maxBudget = params.budget ?? 3000; - const session = store.create(params.query, { maxBudget }); + const session = store.create(params.query, { maxBudget, scopes: params.scopes }); const response = await searchMultiSource( db, diff --git a/src/semantic/forget.ts b/src/semantic/forget.ts index f631560..1d281ab 100644 --- a/src/semantic/forget.ts +++ b/src/semantic/forget.ts @@ -166,16 +166,29 @@ export function contentHash(content: string): string { return createHash("sha256").update(normalized).digest("hex"); } -export function isSuppressed(db: Database.Database, content: string): boolean { +/** + * #106: a suppression belongs to the scope the forgotten memory lived in. It + * applies to that scope, and a 'global' one applies everywhere (global + * knowledge); it never reaches across tenants. + */ +const SUPPRESSION_MATCH = "content_hash = ? AND scope IN (?, 'global')"; + +export function isSuppressed(db: Database.Database, content: string, scope?: string): boolean { const row = db - .prepare("SELECT 1 FROM memory_suppressions WHERE content_hash = ?") - .get(contentHash(content)); + .prepare(`SELECT 1 FROM memory_suppressions WHERE ${SUPPRESSION_MATCH}`) + .get(contentHash(content), scope ?? GLOBAL_SCOPE); return row !== undefined; } -/** Remove the suppression for this content (an explicit remember wins). Returns true when one existed. */ -export function clearSuppression(db: Database.Database, content: string): boolean { - const r = db.prepare("DELETE FROM memory_suppressions WHERE content_hash = ?").run(contentHash(content)); +/** + * Remove this scope's suppression for this content (an explicit remember + * wins). Returns true when one existed. A 'global' suppression is only + * cleared by a global remember — one tenant must not lift it for everyone. + */ +export function clearSuppression(db: Database.Database, content: string, scope?: string): boolean { + const r = db + .prepare("DELETE FROM memory_suppressions WHERE content_hash = ? AND scope = ?") + .run(contentHash(content), scope ?? GLOBAL_SCOPE); return r.changes > 0; } @@ -187,13 +200,15 @@ export function clearSuppression(db: Database.Database, content: string): boolea export function filterSuppressedFacts>( db: Database.Database, facts: readonly T[], + scope?: string, ): { kept: T[]; suppressed: T[] } { if (facts.length === 0) return { kept: [], suppressed: [] }; - const lookup = db.prepare("SELECT 1 FROM memory_suppressions WHERE content_hash = ?"); + const lookup = db.prepare(`SELECT 1 FROM memory_suppressions WHERE ${SUPPRESSION_MATCH}`); + const seenFrom = scope ?? GLOBAL_SCOPE; const kept: T[] = []; const suppressed: T[] = []; for (const fact of facts) { - if (lookup.get(contentHash(fact.content))) suppressed.push(fact); + if (lookup.get(contentHash(fact.content), seenFrom)) suppressed.push(fact); else kept.push(fact); } return { kept, suppressed }; @@ -451,7 +466,8 @@ export function restoreMemory(db: Database.Database, options: RestoreOptions): M "UPDATE memories SET is_active = 1, deleted_at = NULL, deleted_by = NULL, updated_at = unixepoch() WHERE id = ?", ).run(row.id); reindex(db, row, options.embedding); - db.prepare("DELETE FROM memory_suppressions WHERE content_hash = ?").run(contentHash(row.content)); + db.prepare("DELETE FROM memory_suppressions WHERE content_hash = ? AND scope = ?") + .run(contentHash(row.content), row.scope ?? GLOBAL_SCOPE); return logChange(db, { memoryId: row.id, op: "restore", before: null, after: row.content, actor: options.actor, at: now }); }); return run.immediate(); diff --git a/src/semantic/index-integrity.ts b/src/semantic/index-integrity.ts index 423e530..456dcc0 100644 --- a/src/semantic/index-integrity.ts +++ b/src/semantic/index-integrity.ts @@ -35,15 +35,15 @@ export interface IndexRepair { ftsRowsRemoved: number; } -/** Rowids currently present in memories_fts, via a temporary fts5vocab table. */ +/** + * Rowids currently present in memories_fts, read from its docsize shadow + * table. #112: an fts5vocab 'instance' table has one row per term occurrence, + * so a document that tokenizes to nothing (punctuation or emoji only) has no + * rows there and was never audited; docsize holds one row per indexed + * document regardless of its terms. + */ export function ftsIndexedRowids(db: Database.Database): number[] { - const name = `memories_fts_vocab_${process.pid}_${Math.floor(Math.random() * 1e6)}`; - db.exec(`CREATE VIRTUAL TABLE temp.${name} USING fts5vocab('main', 'memories_fts', 'instance')`); - try { - return (db.prepare(`SELECT DISTINCT doc FROM temp.${name}`).all() as Array<{ doc: number }>).map((r) => r.doc); - } finally { - db.exec(`DROP TABLE IF EXISTS temp.${name}`); - } + return (db.prepare("SELECT id FROM memories_fts_docsize").all() as Array<{ id: number }>).map((r) => r.id); } export function auditMemoryIndex(db: Database.Database): IndexAudit { diff --git a/tests/core/tenant-scoping.test.ts b/tests/core/tenant-scoping.test.ts index fadfa48..4824b0d 100644 --- a/tests/core/tenant-scoping.test.ts +++ b/tests/core/tenant-scoping.test.ts @@ -57,20 +57,39 @@ describe("resolveCallScoping (per-request override)", () => { expect(s.readScopes).toEqual(["global", "hermes:career"]); }); - it("scope param overrides ENGRAM_SCOPE but keeps an explicit ENGRAM_READ_SCOPES", () => { - const env = { ENGRAM_SCOPE: "hermes:pmp", ENGRAM_READ_SCOPES: "global,hermes:pmp" }; + it("scope param overrides ENGRAM_SCOPE within an explicit ENGRAM_READ_SCOPES", () => { + const env = { ENGRAM_SCOPE: "hermes:pmp", ENGRAM_READ_SCOPES: "global,hermes:pmp,hermes:career" }; const s = resolveCallScoping(env, { scope: "hermes:career" }); expect(s.writeScope).toBe("hermes:career"); - expect(s.readScopes).toEqual(["global", "hermes:pmp"]); + expect(s.readScopes).toEqual(["global", "hermes:pmp", "hermes:career"]); }); - it("read_scopes param overrides env read scopes and leaves writeScope alone", () => { - const env = { ENGRAM_SCOPE: "hermes:pmp" }; + it("read_scopes param narrows env read scopes and leaves writeScope alone", () => { + const env = { ENGRAM_SCOPE: "hermes:pmp", ENGRAM_READ_SCOPES: "global,hermes:pmp,hermes:career" }; const s = resolveCallScoping(env, { read_scopes: [" hermes:career "] }); expect(s.writeScope).toBe("hermes:pmp"); expect(s.readScopes).toEqual(["hermes:career"]); }); + // #108: an env-pinned child (Hermes stdio) must not be widened by tool args. + it("read_scopes outside the env ceiling are dropped; none left → throws", () => { + const env = { ENGRAM_SCOPE: "hermes:pmp" }; // reads default to global + own + expect(resolveCallScoping(env, { read_scopes: ["hermes:career", "global"] }).readScopes).toEqual(["global"]); + expect(() => resolveCallScoping(env, { read_scopes: ["hermes:career"] })).toThrow(/no scope in common/); + }); + + it("scope outside the env read scopes throws (write only where you may read)", () => { + const env = { ENGRAM_SCOPE: "hermes:pmp", ENGRAM_READ_SCOPES: "global,hermes:pmp" }; + expect(() => resolveCallScoping(env, { scope: "hermes:career" })).toThrow(/outside this server's read scopes/); + expect(resolveCallScoping(env, { scope: "global" }).writeScope).toBe("global"); + }); + + it("with no env restriction the params apply as given (shared HTTP daemon)", () => { + const s = resolveCallScoping({}, { scope: "hermes:career", read_scopes: ["hermes:pmp"] }); + expect(s.writeScope).toBe("hermes:career"); + expect(s.readScopes).toEqual(["hermes:pmp"]); + }); + it("rejects empty / whitespace scope strings and empty read_scopes", () => { expect(() => resolveCallScoping({}, { scope: "" })).toThrow(/scope/); expect(() => resolveCallScoping({}, { scope: " " })).toThrow(/scope/); diff --git a/tests/interfaces/cli/update.test.ts b/tests/interfaces/cli/update.test.ts index df46c62..c18f53a 100644 --- a/tests/interfaces/cli/update.test.ts +++ b/tests/interfaces/cli/update.test.ts @@ -968,7 +968,7 @@ describe("engram update --rollback (#65, decision #66)", () => { // the additive caveat when the update added checkpoints that are not breaking const additive = await rollbackUpdate(older, res.planFile!, d); expect(additive.ok, additive.lines.join("\n")).toBe(true); - expect(additive.lines.join("\n")).toContain("additive migration(s) applied by the update stay in place (forget_v1; version 4 -> 5)"); + expect(additive.lines.join("\n")).toContain(`additive migration(s) applied by the update stay in place (forget_v1; version 4 -> ${SCHEMA_VERSION})`); expect(readRollbackPlan(res.planFile!).rolledBack).toMatchObject({ ok: true, mode: "code-only" }); // with a backup, --restore-data is allowed across a breaking migration (the backup predates it) mkdirSync(join(root, "bk"), { recursive: true }); diff --git a/tests/interfaces/mcp/forget-tool.test.ts b/tests/interfaces/mcp/forget-tool.test.ts index 5e21897..05d0a72 100644 --- a/tests/interfaces/mcp/forget-tool.test.ts +++ b/tests/interfaces/mcp/forget-tool.test.ts @@ -268,6 +268,21 @@ describe("forget honours read_scopes (#25)", () => { expect(memoryRow(HOME)!.is_active).toBe(1); }); + // #109: the global override is for an unpinned server only — an env-pinned + // child must not reach another tenant's memory with scope: "global". + it('scope: "global" is ignored when the env pins the server to a tenant', async () => { + const id = memoryRow(HOME)!.id; + process.env.ENGRAM_SCOPE = "hermes:career"; + try { + const res = await handleToolCall("forget", { memory_id: id, scope: "global" }); + expect(res.isError).toBe(true); + expect(text(res)).toMatch(/outside read_scopes/); + expect(memoryRow(HOME)!.is_active).toBe(1); + } finally { + delete process.env.ENGRAM_SCOPE; + } + }); + it('scope: "global" acts across scopes', async () => { const id = memoryRow(HOME)!.id; const res = await handleToolCall("forget", { memory_id: id, read_scopes: ["global", "hermes:career"], scope: "global" }); diff --git a/tests/interfaces/mcp/per-request-scoping.test.ts b/tests/interfaces/mcp/per-request-scoping.test.ts index 7b3eb6e..1318c3d 100644 --- a/tests/interfaces/mcp/per-request-scoping.test.ts +++ b/tests/interfaces/mcp/per-request-scoping.test.ts @@ -113,6 +113,26 @@ describe("per-request scope / read_scopes params", () => { expect(text).not.toContain("Databricks"); }); + // #110: a session keeps the scopes it was created with; a refine that omits + // read_scopes (or passes wider ones) must not pull in another tenant. + it("recall_session pins its scopes for every refine", async () => { + const created = await handleToolCall("recall_session", { + query: "interview", + sources: ["semantic"], + read_scopes: ["global"], + }); + const sessionId = / { const res = await handleToolCall("remember_batch", { scope: "hermes:pmp", @@ -182,9 +202,10 @@ describe("per-request scope / read_scopes params", () => { // env read default = global + own, so the career memory is hidden… const hidden = await handleToolCall("recall", { query: "interview", sources: ["semantic"] }); expect(hidden.content[0].text as string).not.toContain("Databricks"); - // …unless the call overrides read_scopes - const shown = await handleToolCall("recall", { query: "interview", sources: ["semantic"], read_scopes: ["hermes:career"] }); - expect(shown.content[0].text as string).toContain("Databricks"); + // …and a call cannot widen past the env ceiling (#108) + const refused = await handleToolCall("recall", { query: "interview", sources: ["semantic"], read_scopes: ["hermes:career"] }); + expect(refused.isError).toBe(true); + expect(refused.content[0].text as string).toMatch(/no scope in common/); } finally { delete process.env.ENGRAM_SCOPE; } diff --git a/tests/migration/graph-scope-columns.test.ts b/tests/migration/graph-scope-columns.test.ts index bac80cb..c6f5bc8 100644 --- a/tests/migration/graph-scope-columns.test.ts +++ b/tests/migration/graph-scope-columns.test.ts @@ -45,8 +45,14 @@ describe("graph scope migration", () => { expect(scopeVisible("global", undefined)).toBe(true); expect(scopeVisible("hermes:a", ["global"])).toBe(false); expect(scopeVisible(null, ["global"])).toBe(true); - expect(scopeInClause("e.scope", ["a", "b"])).toEqual({ sql: "e.scope IN (?, ?)", params: ["a", "b"] }); + // #107: COALESCE keeps the IN clause in step with scopeVisible, which + // reads a NULL scope as 'global'. + expect(scopeInClause("e.scope", ["a", "b"])).toEqual({ sql: "COALESCE(e.scope, 'global') IN (?, ?)", params: ["a", "b"] }); expect(scopeInClause("e.scope", [])).toBeNull(); + t.db.prepare("INSERT INTO exchanges (id, conversation_id, project, timestamp, exchange_index, user_message, scope) VALUES ('xnull', 'c9', 'p', '2026-09-17T00:00:00Z', 0, 'null scope', NULL)").run(); + const clause = scopeInClause("scope", ["global"])!; + const seen = t.db.prepare(`SELECT id FROM exchanges WHERE ${clause.sql}`).all(...clause.params) as Array<{ id: string }>; + expect(seen.map((r) => r.id)).toContain("xnull"); entity("e1", "Engram", "hermes:a"); expect(widenScope(t.db, "entities", "e1", "hermes:a")).toBe(false); // same scope: unchanged expect(getEntity(t.db, "e1")!.scope).toBe("hermes:a"); diff --git a/tests/semantic/batch-remember.test.ts b/tests/semantic/batch-remember.test.ts index 8e1ab09..7d29c35 100644 --- a/tests/semantic/batch-remember.test.ts +++ b/tests/semantic/batch-remember.test.ts @@ -216,6 +216,34 @@ describe("storeMemoryBatch", () => { expect(t.db.prepare("SELECT COUNT(*) AS n FROM memories").get()).toEqual({ n: 2 }); }); + // #111: entity links from a tenant write must not land in the global graph. + it("stamps new related_to edges with the caller's scope and widens an existing one", async () => { + const { storeMemoryBatch } = await import( + "../../src/interfaces/shared/remember.js" + ); + const ins = t.db.prepare("INSERT INTO entities (id, name, type) VALUES (?, ?, ?)"); + ins.run("ent-c", "Gamma", "concept"); + ins.run("ent-d", "Delta", "concept"); + + await storeMemoryBatch( + t.db, + [{ content: "Gamma relates to Delta", type: "fact", relates_to_entities: ["Gamma", "Delta"] }], + { scope: "hermes:career" }, + ); + const edge = () => + t.db.prepare("SELECT scope FROM relationships WHERE source_entity_id = 'ent-c' AND target_entity_id = 'ent-d'") + .get() as { scope: string } | undefined; + expect(edge()?.scope).toBe("hermes:career"); + + // the same edge seen from a second tenant is shared knowledge + await storeMemoryBatch( + t.db, + [{ content: "Gamma also relates to Delta at home", type: "fact", relates_to_entities: ["Gamma", "Delta"] }], + { scope: "hermes:home" }, + ); + expect(edge()?.scope).toBe("global"); + }); + it("deduplicates within the batch", async () => { const { storeMemoryBatch } = await import( "../../src/interfaces/shared/remember.js" diff --git a/tests/semantic/forget.test.ts b/tests/semantic/forget.test.ts index 73ce3bf..9fb34a9 100644 --- a/tests/semantic/forget.test.ts +++ b/tests/semantic/forget.test.ts @@ -523,6 +523,42 @@ describe("extraction suppression", () => { expect(isSuppressed(t.db, "keep me after all")).toBe(false); expect(clearSuppression(t.db, "keep me after all")).toBe(false); }); + + // #106: a suppression belongs to the scope it was forgotten in. + it("one tenant's forget does not suppress the same sentence for another", () => { + seed("career", "Standups are at 9", { scope: "hermes:career" }); + forgetMemory(t.db, { memoryId: "career", actor: "hermes" }); + const facts = [{ type: "fact" as const, content: "standups are at 9", importance: 0.5, sourceExchangeIds: [] }]; + + expect(filterSuppressedFacts(t.db, facts, "hermes:career").suppressed).toHaveLength(1); + expect(filterSuppressedFacts(t.db, facts, "hermes:home").suppressed).toHaveLength(0); + expect(filterSuppressedFacts(t.db, facts).suppressed).toHaveLength(0); // 'global' + expect(isSuppressed(t.db, "Standups are at 9", "hermes:career")).toBe(true); + expect(isSuppressed(t.db, "Standups are at 9", "hermes:home")).toBe(false); + }); + + it("a global suppression applies to every scope, and only a global remember lifts it", () => { + seed("g1", "Deploy freeze in December"); + forgetMemory(t.db, { memoryId: "g1", actor: "cli" }); + expect(isSuppressed(t.db, "deploy freeze in december", "hermes:career")).toBe(true); + expect(clearSuppression(t.db, "deploy freeze in december", "hermes:career")).toBe(false); + expect(isSuppressed(t.db, "deploy freeze in december", "hermes:career")).toBe(true); + expect(clearSuppression(t.db, "deploy freeze in december")).toBe(true); + }); + + it("two tenants forgetting the same sentence keep separate suppressions", () => { + seed("a", "Same sentence", { scope: "hermes:career" }); + seed("b", "Same sentence", { scope: "hermes:home" }); + forgetMemory(t.db, { memoryId: "a", actor: "hermes" }); + forgetMemory(t.db, { memoryId: "b", actor: "hermes" }); + const rows = t.db + .prepare("SELECT memory_id, scope FROM memory_suppressions WHERE content_hash = ? ORDER BY scope") + .all(contentHash("Same sentence")) as Array<{ memory_id: string; scope: string }>; + expect(rows).toEqual([ + { memory_id: "a", scope: "hermes:career" }, + { memory_id: "b", scope: "hermes:home" }, + ]); + }); }); // ─── Inspection ───────────────────────────────────────────────── @@ -638,4 +674,16 @@ describe("memory index integrity (engram validate)", () => { expect(() => t.db.prepare("INSERT INTO memories_fts(memories_fts) VALUES('integrity-check')").run()).not.toThrow(); expect(repairMemoryIndex(t.db)).toEqual({ vectorsDeleted: 0, ftsRebuilt: false, ftsRowsRemoved: 0 }); }); + + // #112: an fts5vocab 'instance' scan misses a document with no terms. + it("audits a forgotten memory whose text tokenizes to nothing", async () => { + const { auditMemoryIndex, repairMemoryIndex } = await import("../../src/semantic/index-integrity.js"); + seed("silent", "..."); + forgetMemory(t.db, { memoryId: "silent", actor: "cli" }); + t.db.prepare("INSERT INTO memories_fts (rowid, content, context) VALUES (?, ?, ?)").run(rowidOf("silent"), "...", null); + + expect(auditMemoryIndex(t.db).orphanFtsForgotten).toEqual([rowidOf("silent")]); + expect(repairMemoryIndex(t.db).ftsRebuilt).toBe(true); + expect(auditMemoryIndex(t.db).orphanFtsForgotten).toEqual([]); + }); });