From 9093e723522f98a82e35b2fa6c9fcd20132004a8 Mon Sep 17 00:00:00 2001 From: devinmlowe Date: Mon, 21 Sep 2026 01:31:51 -0500 Subject: [PATCH 1/2] =?UTF-8?q?test(mcp):=20regression=20tests=20for=20#87?= =?UTF-8?q?=20=E2=80=94=20env-scoped=20stdio=20start=20must=20not=20bridge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the fix before it lands: ENGRAM_SCOPE / ENGRAM_READ_SCOPES on a stdio start force inline without probing, ENGRAM_DB_PATH bridges only when the daemon's /health reports the same database, and an end-to-end child with ENGRAM_SCOPE set never opens a daemon session. Autoresearch baseline (iteration 0): these fail on main. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BoKzYWjzY9jngoKL35LWSA --- tests/interfaces/mcp/bridge.test.ts | 36 +++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) 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, From 685adbbfd70dd4ecda5aaad80f4a33cfe2619da7 Mon Sep 17 00:00:00 2001 From: devinmlowe Date: Mon, 21 Sep 2026 01:33:21 -0500 Subject: [PATCH 2/2] experiment: env-scoped stdio start runs inline instead of bridging (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: decideStdioMode probed /health first and bridged whenever the daemon answered, while runBridge forwards tools/call verbatim — so ENGRAM_SCOPE / ENGRAM_READ_SCOPES / ENGRAM_DB_PATH in the stdio process's env (every Hermes profile child sets the first two) were resolved under the daemon's env instead. Every profile wrote global and read every scope. Fix, at the one decision point all stdio starts route through: - ENGRAM_SCOPE / ENGRAM_READ_SCOPES set (non-blank) → inline, no probe, reason names the variable. - /health now carries dbPath; ENGRAM_DB_PATH set and different → inline. A daemon without dbPath (older build) keeps bridging. Docs: README, CLAUDE.md, integrate-your-agent, CHANGELOG (Fixed). Autoresearch iteration 1: bridge.test.ts failures 3 → 0, lint passes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BoKzYWjzY9jngoKL35LWSA --- CHANGELOG.md | 3 +++ CLAUDE.md | 5 ++++- README.md | 5 ++++- docs/integrate-your-agent.md | 5 +++++ src/interfaces/mcp/bridge.ts | 38 +++++++++++++++++++++++++++++++++--- src/interfaces/mcp/server.ts | 7 +++++-- 6 files changed, 56 insertions(+), 7 deletions(-) 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) }; }, });