diff --git a/CHANGELOG.md b/CHANGELOG.md index 73a2713..60b6992 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ All notable changes to engram are documented here. The format follows ## [Unreleased] +### Fixed +- A stdio start no longer bridges its per-process scoping away (#87, P1). `engram mcp` / `dist/interfaces/mcp/server.js` with `ENGRAM_SCOPE` or `ENGRAM_READ_SCOPES` in the environment — every Hermes profile child, or any host following `docs/integrate-your-agent.md` — used to bridge to a healthy daemon, which ran each forwarded call under *its* env: every profile wrote `global` and read every scope, silently. Such a start now runs inline without probing, with the reason on stderr (`running inline (ENGRAM_SCOPE=hermes:career is per-process; the daemon would ignore it)`). The daemon's `/health` gained `dbPath`; with `ENGRAM_DB_PATH` set the bridge is used only when it matches the daemon's, otherwise inline (`ENGRAM_DB_PATH=… but the daemon serves …`). A daemon that does not report `dbPath` (older build) keeps bridging as before. + ### Changed - Test scaffolding shared instead of copied (#126, part of the #114 over-engineering burn-down). No behaviour change and no test lost: `tests/mocks/embeddings.ts` replaces eleven copies of the hash-based embeddings `vi.mock` factory, `tests/mocks/llm-fetch.ts` the four `stubFetch` / `makeAnthropicMock` copies in the `*-factory` suites, `tests/mocks/fake-worker.ts` the two `FakeWorker` classes; `tests/helpers.ts` gains the raw row inserts (`insertEntity`, `insertRelationship`, `insertCluster`, `insertBridgeScore`, `insertConversation`, `insertExchange`, `insertMemory`), `createTestMemory`, `makeExchanges` and the built-CLI guard (`builtCli`, `itBuilt`), and loses its `cosineSimilarity` copy (tests import `src/_core/search/vector.ts`) and the one-caller `createTestFixture` / `createSyntheticToolCall`. `vitest.config.ts` sets `test.unstubEnvs`, so suites isolate env with `vi.stubEnv(key, undefined)` instead of hand-rolled save/restore loops. - MCP server trims (#119, part of the #114 over-engineering burn-down). Every tool's `inputSchema` is now generated from its zod schema (`z.toJSONSchema`, io `input`), replacing about 700 lines of hand-written JSON Schema; `tests/interfaces/mcp/tool-schemas.test.ts` snapshots the full ListTools surface of all 16 tools. Names, descriptions and annotations are unchanged. Visible differences: unbounded integer fields (`show.startLine`/`endLine`, `recall_drill.result_index`, `fetch_snippets` ranges, `ingest_turn.turn_index`) advertise `"integer"` instead of `"number"` (the validator always rejected fractions), `ingest_turn` `tool_calls[]` items no longer claim `additionalProperties: false` (never enforced), and the defaults the handlers already applied are now on the schema (`recall` budget 1500 / dateBasis `filed` / depth `shallow` / sources / reinforce, `recall_session` budget 3000, `commitments` status / limit / budget, `ingest_turn` source). `src/graph/format.ts` renders explore and reflect results once with an `xml` (MCP) / `text` (CLI) switch — output is byte-identical. `recall_session` returns `sessionId` on its result for worker affinity instead of the dispatcher scraping it out of the XML. The worker pool keeps the per-call timeout, the kill-and-respawn at 2× the timeout and respawn-on-exit, and loses the never-set `hangMultiplier` / `readyTimeoutMs` options, the generation counters and its three Error subclasses (same messages, plain `Error`). diff --git a/CLAUDE.md b/CLAUDE.md index 963d7e5..6d70259 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -121,7 +121,10 @@ proxy (`src/interfaces/mcp/bridge.ts`): an MCP SDK client over Streamable HTTP t daemon's `/mcp` plus an MCP server on stdio forwarding `tools/list`, `tools/call`, `ping`. It opens no database and loads no model — the CLI decides before `server.ts` is imported. Otherwise it runs inline. `engram mcp --standalone` / `ENGRAM_MCP_STANDALONE=1` force -inline. One stderr line names the mode: `Engram MCP: bridging stdio to +inline, as do `ENGRAM_SCOPE` / `ENGRAM_READ_SCOPES` in the process env (#87: per-process +scoping the daemon would ignore — every Hermes profile child); `ENGRAM_DB_PATH` bridges only +when `/health` reports the same `dbPath` (`Engram MCP: running inline (ENGRAM_SCOPE=hermes:x +is per-process; the daemon would ignore it)`). One stderr line names the mode: `Engram MCP: bridging stdio to http://127.0.0.1:9907/mcp (daemon healthy)` / `Engram MCP: running inline (no daemon on :9907 (ECONNREFUSED))`. The bridge forwards `Authorization: Bearer $ENGRAM_MCP_TOKEN` from its own env, re-opens its daemon session under the host's `clientInfo` after `initialize` diff --git a/README.md b/README.md index 0a9405c..2ac764c 100644 --- a/README.md +++ b/README.md @@ -519,7 +519,10 @@ Engram MCP: running inline (no daemon on :9907 (ECONNREFUSED)) ``` `engram mcp --standalone` (or `ENGRAM_MCP_STANDALONE=1` in the server's env) -forces inline. The bridge forwards `Authorization: Bearer $ENGRAM_MCP_TOKEN` +forces inline, and so do `ENGRAM_SCOPE` / `ENGRAM_READ_SCOPES` in that env (they +scope *this* process; the daemon would ignore them, #87). With `ENGRAM_DB_PATH` +set, the bridge is used only when the daemon's `/health` reports the same +`dbPath`. The bridge forwards `Authorization: Bearer $ENGRAM_MCP_TOKEN` from its own environment when the daemon requires a token, re-opens its daemon session under the host's `clientInfo` (so `forget` still records the real actor) and survives a daemon restart (`engram update`) with one reconnect. diff --git a/docs/integrate-your-agent.md b/docs/integrate-your-agent.md index a7f0ee2..cd45413 100644 --- a/docs/integrate-your-agent.md +++ b/docs/integrate-your-agent.md @@ -184,6 +184,11 @@ Scoping has a per-process default and a per-call override. | `ENGRAM_SCOPE` | Default write scope stamped on every `remember` / `remember_batch`. | | `ENGRAM_READ_SCOPES` | Comma-separated scopes `recall` may return by default. When unset but `ENGRAM_SCOPE` is set, defaults to `global` plus the write scope. | +Either variable in a stdio server's env makes that start run inline even when +the HTTP daemon is up (#87): the bridge forwards calls verbatim and the daemon +would resolve them under its own env. A shared daemon is scoped per call with +the `scope` / `read_scopes` parameters instead. + Per call, `recall` and `recall_session` accept `scope` (reads then default to `global` + that scope) and `read_scopes` (explicit list); `remember` and `remember_batch` accept `scope`; `ingest_turn` requires `scope`. A parameter diff --git a/src/interfaces/mcp/bridge.ts b/src/interfaces/mcp/bridge.ts index 400e3c3..92de55b 100644 --- a/src/interfaces/mcp/bridge.ts +++ b/src/interfaces/mcp/bridge.ts @@ -32,6 +32,7 @@ import { PingRequestSchema, type Implementation, } from "@modelcontextprotocol/sdk/types.js"; +import { resolve as resolvePath } from "node:path"; import { ENGRAM_VERSION } from "../../_core/version/index.js"; import { resolveMcpToken } from "./auth.js"; import { DEFAULT_MCP_PORT, isEngramDaemonHealth, parseMcpPort, probeMcpHealth, type McpHealthProbe } from "./port.js"; @@ -86,10 +87,30 @@ function errShort(err: unknown): string { return typeof code === "string" && code ? code : errMsg(err); } +/** + * Per-process scoping env (ADR-010, `scoping.ts`). The bridge forwards calls + * verbatim and the daemon resolves them under its *own* env, so a stdio start + * carrying these must run inline or the values are silently dropped (#87 — + * every Hermes profile child sets them). + */ +const PER_PROCESS_SCOPE_ENV = ["ENGRAM_SCOPE", "ENGRAM_READ_SCOPES"] as const; + +/** The database a `/health` body says the daemon serves, when it says (#87). */ +function daemonDbPath(body: string): string | undefined { + try { + const dbPath = (JSON.parse(body) as { dbPath?: unknown }).dbPath; + return typeof dbPath === "string" && dbPath ? dbPath : undefined; + } catch { + return undefined; + } +} + /** * Choose bridge or inline for a stdio start. Pure apart from the probe, so - * tests inject one. `--standalone` and `ENGRAM_MCP_STANDALONE` win without - * probing; otherwise one `GET /health` with a short timeout decides. + * tests inject one. `--standalone`, `ENGRAM_MCP_STANDALONE` and per-process + * scoping env win without probing; otherwise one `GET /health` with a short + * timeout decides, and a daemon serving a different `ENGRAM_DB_PATH` counts + * as no daemon. */ export async function decideStdioMode(options: DecideStdioModeOptions): Promise { const { args, env, probe = probeMcpHealth, timeoutMs = DEFAULT_PROBE_TIMEOUT_MS } = options; @@ -98,10 +119,21 @@ export async function decideStdioMode(options: DecideStdioModeOptions): Promise< if (args.includes(STANDALONE_FLAG)) return { mode: "inline", port, url, reason: STANDALONE_FLAG }; if (isTruthy(env[STANDALONE_ENV])) return { mode: "inline", port, url, reason: `${STANDALONE_ENV}=${env[STANDALONE_ENV]?.trim()}` }; + for (const name of PER_PROCESS_SCOPE_ENV) { + const value = env[name]?.trim(); + if (value) return { mode: "inline", port, url, reason: `${name}=${value} is per-process; the daemon would ignore it` }; + } try { const health = await probe(port, timeoutMs); - if (isEngramDaemonHealth(health)) return { mode: "bridge", port, url, reason: "daemon healthy" }; + if (isEngramDaemonHealth(health)) { + const mine = env.ENGRAM_DB_PATH?.trim(); + const theirs = daemonDbPath(health.body); + if (mine && theirs && resolvePath(mine) !== resolvePath(theirs)) { + return { mode: "inline", port, url, reason: `ENGRAM_DB_PATH=${mine} but the daemon serves ${theirs}` }; + } + return { mode: "bridge", port, url, reason: "daemon healthy" }; + } return { mode: "inline", port, url, reason: `something answers on :${port} but not like the engram daemon (HTTP ${health.status})`, diff --git a/src/interfaces/mcp/server.ts b/src/interfaces/mcp/server.ts index cf39eb5..8641c97 100644 --- a/src/interfaces/mcp/server.ts +++ b/src/interfaces/mcp/server.ts @@ -1380,8 +1380,11 @@ export async function startMcpServer(args: string[] = process.argv.slice(2)): Pr registerHandlers: (srv) => registerToolHandlers(srv, dispatcher.call), health: () => { const stats = dispatcher.stats(); - // #65: the daily version check the CLI caches (never the network from here) - return { workers: stats ?? { size: 0 }, update: updateHealthField((config ??= loadConfig()).dataDir) }; + config ??= loadConfig(); + // #65: the daily version check the CLI caches (never the network from here). + // #87: the database this daemon serves, so a scoped stdio start with another + // ENGRAM_DB_PATH runs inline instead of bridging into the wrong database. + return { workers: stats ?? { size: 0 }, dbPath: config.dbPath, update: updateHealthField(config.dataDir) }; }, }); diff --git a/tests/interfaces/mcp/bridge.test.ts b/tests/interfaces/mcp/bridge.test.ts index 28115f4..b052bec 100644 --- a/tests/interfaces/mcp/bridge.test.ts +++ b/tests/interfaces/mcp/bridge.test.ts @@ -194,6 +194,30 @@ describe("decideStdioMode", () => { const d = await decideStdioMode({ args: ["--port", "9912"], env: { ENGRAM_MCP_PORT: "9911" }, probe: ok() }); expect(d.port).toBe(9912); }); + + // #87: a stdio start scoped through its env (the Hermes plugin's child, or + // any host that sets ENGRAM_SCOPE in the server's env block) must never + // bridge — the daemon would run every call under its own env instead. + it("ENGRAM_SCOPE / ENGRAM_READ_SCOPES force inline without probing, naming the variable", async () => { + const probe = vi.fn(ok()); + const scoped = await decideStdioMode({ args: [], env: { ENGRAM_SCOPE: "hermes:career" }, probe }); + expect(scoped).toMatchObject({ mode: "inline", reason: "ENGRAM_SCOPE=hermes:career is per-process; the daemon would ignore it" }); + const read = await decideStdioMode({ args: [], env: { ENGRAM_READ_SCOPES: "global,hermes:career" }, probe }); + expect(read).toMatchObject({ mode: "inline", reason: "ENGRAM_READ_SCOPES=global,hermes:career is per-process; the daemon would ignore it" }); + expect(probe).not.toHaveBeenCalled(); + // Blank values do not count. + expect((await decideStdioMode({ args: [], env: { ENGRAM_SCOPE: " " }, probe })).mode).toBe("bridge"); + }); + + it("ENGRAM_DB_PATH bridges only when the daemon reports the same database", async () => { + const daemon = (dbPath: string) => ok(200, JSON.stringify({ status: "ok", dbPath })); + const same = await decideStdioMode({ args: [], env: { ENGRAM_DB_PATH: "/data/a/./engram.db" }, probe: daemon("/data/a/engram.db") }); + expect(same.mode).toBe("bridge"); + const other = await decideStdioMode({ args: [], env: { ENGRAM_DB_PATH: "/data/b/engram.db" }, probe: daemon("/data/a/engram.db") }); + expect(other).toMatchObject({ mode: "inline", reason: "ENGRAM_DB_PATH=/data/b/engram.db but the daemon serves /data/a/engram.db" }); + // A daemon that does not report its database (pre-#87) keeps bridging. + expect((await decideStdioMode({ args: [], env: { ENGRAM_DB_PATH: "/data/b/engram.db" }, probe: ok() })).mode).toBe("bridge"); + }); }); describe("bridge module stays light", () => { @@ -339,6 +363,18 @@ describe("stdio entry end to end (real daemon, real child process)", () => { expect((await child.client.listTools()).tools).toHaveLength(16); }, 60_000); + it("ENGRAM_SCOPE in the child's env runs inline even though the daemon is healthy (#87: Hermes profile isolation)", async () => { + const before = http.sessions.size; + const child = await spawnCli([], { ENGRAM_MCP_PORT: String(daemonPort), ENGRAM_SCOPE: "hermes:career" }); + children.push(child); + + expect(child.stderr()).toContain("Engram MCP: running inline (ENGRAM_SCOPE=hermes:career is per-process; the daemon would ignore it)"); + expect(child.stderr()).not.toContain("bridging"); + expect((await child.client.listTools()).tools).toHaveLength(16); + expect(child.trace()).toContain("/src/interfaces/mcp/server.ts"); + expect(http.sessions.size).toBe(before); + }, 60_000); + it("exits 0 when the host closes stdin", async () => { const child = spawn(process.execPath, ["--import", "tsx", CLI_SRC, "mcp"], { cwd: REPO_ROOT,