From fceb12f516407a306226afa70e35f34deb090128 Mon Sep 17 00:00:00 2001 From: Rohit Ghumare Date: Thu, 21 May 2026 10:20:44 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20pre-release=20hardening=20for=200.9.22?= =?UTF-8?q?=20=E2=80=94=20regex=20correctness=20+=20MCP=20env=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three concerns surfaced when auditing PRs merged since v0.9.21: 1) Graph parser regex from #494 was correct for self-closing tags ONLY when they're the only entity in the document. With a self-closing entity followed by an explicit-close entity, the greedy `[^>]*` ate the trailing `/` of the self-close, the alternation fell through to the explicit-close branch, and `[\s\S]*?` ran ahead to the *next* `` — merging two entity declarations into one match and silently dropping a node. Switch to lazy `[^>]*?` so the self-closing alternation gets first chance. Test from #494 (which was failing on main but I didn't notice) now passes. 2) #386 shipped `${AGENTMEMORY_URL}` / `${AGENTMEMORY_SECRET}` placeholders in plugin/.mcp.json. Claude Code and Cursor expand those at MCP launch time; some hosts pass the literal string through. A truthy literal `"${AGENTMEMORY_URL}"` defeats the existing `|| DEFAULT_URL` fallback and would have the shim POST to `${AGENTMEMORY_URL}/agentmemory/...` (DNS failure). Add a defensive guard in src/mcp/rest-proxy.ts that strips any value of the form `${...}` so the fallback engages. Mirror in src/mcp/standalone.ts's mode-announce log line. 3) CHANGELOG entries for all PRs landed since v0.9.21 (#321, #325, #386, #454, #494, #526, #542, #560, #561, #562, #564, #567) plus the regex + env-guard hardening here. Validation: - npm test → 98 files, 1091 tests pass - npm pack → 142 files, fresh install clean, iii-sdk@0.11.2 pinned, plugin/ shipped with hooks/scripts/skills/opencode/ - New test file covers 8 placeholder cases (unset, empty, `${VAR}` literal, mismatched braces, real values with $, unclosed `${`, no-braces `$VAR`). --- CHANGELOG.md | 39 ++++++++++++++++++++++ src/functions/graph.ts | 8 ++++- src/mcp/rest-proxy.ts | 18 ++++++++-- src/mcp/standalone.ts | 13 +++++++- test/mcp-env-placeholder.test.ts | 57 ++++++++++++++++++++++++++++++++ 5 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 test/mcp-env-placeholder.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0188e05a..dbdd9e49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,45 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Fixed + +- **PostToolUse hook reads `tool_response`, falls back to `tool_output`** ([PR #561](https://github.com/rohitg00/agentmemory/pull/561) by [@faraz152](https://github.com/faraz152), closes [#539](https://github.com/rohitg00/agentmemory/issues/539)). Claude Code's PostToolUse payload sends the field as `tool_response`, not `tool_output`. The hook was reading the wrong field, so `cleanOutput` was always `undefined`, `mem::compress` consistently failed its XML schema validation (requires `narrative >= 10` chars), and observations from every real Claude Code tool call were dropped. Now reads `tool_response ?? tool_output` so older integrations that emit the legacy field name keep working. + +- **iii-sdk pinned to exact `0.11.2`** ([PR #567](https://github.com/rohitg00/agentmemory/pull/567), closes [#555](https://github.com/rohitg00/agentmemory/issues/555)). `iii-sdk@0.11.6` introduced a routing regression — all `/agentmemory/*` routes returned `404` from the iii-engine, even though `0.11.6` satisfied our previous `"^0.11.2"` range. `npm install -g @agentmemory/agentmemory` silently drifted everyone forward after `0.9.21` shipped. Pin removes the caret + also pins `iii-sdk@0.11.2` in `agentmemory setup`'s upgrade flow. + +- **OpenAI provider sends explicit `stream: false`** ([PR #526](https://github.com/rohitg00/agentmemory/pull/526) by [@Ptah-CT](https://github.com/Ptah-CT)). Some OpenAI-compatible proxies default to `text/event-stream` when `stream` is absent, which crashes `response.json()` and trips `ResilientProvider`'s circuit breaker, silently disabling LLM-backed compression / summarisation / reflection. + +- **Viewer search uses NFKC normalisation for CJK / fullwidth input** ([PR #542](https://github.com/rohitg00/agentmemory/pull/542) by [@kaushalrog](https://github.com/kaushalrog)). Fullwidth characters and ligatures pasted from PDFs were missed by `.includes()`. NFKC normalisation on both sides of the comparison makes search match. + +- **Viewer splash shows actual bound viewer port** ([PR #560](https://github.com/rohitg00/agentmemory/pull/560) by [@Tanmay-008](https://github.com/Tanmay-008), closes [#521](https://github.com/rohitg00/agentmemory/issues/521)). `/agentmemory/livez` now returns `viewerPort` + `viewerSkipped`; CLI polls until populated. New fields are additive — existing consumers of `/livez` are unaffected. + +- **Viewer tab bar height stable across tab switches** ([PR #325](https://github.com/rohitg00/agentmemory/pull/325) by [@hungtd119](https://github.com/hungtd119), closes [#324](https://github.com/rohitg00/agentmemory/issues/324)). Explicit `height: 48px` on `.tab-bar` prevents scrollbar-induced height shift on tab switch (complements #313's `flex: 0 0 auto`). + +- **Graph parser accepts self-closing `` tags** ([PR #494](https://github.com/rohitg00/agentmemory/pull/494) by [@Rex57](https://github.com/Rex57), follow-up regex fix in this release, closes [#492](https://github.com/rohitg00/agentmemory/issues/492)). Real LLM output uses self-closing entity tags for entities without properties; the parser regex required explicit `` closes and dropped them. Initial fix added an alternation; follow-up made the attribute matcher lazy (`[^>]*?`) so the self-closing branch is reached before the explicit-close branch greedily eats the next entity. + +- **Plugin MCP server inherits remote/auth env** ([PR #386](https://github.com/rohitg00/agentmemory/pull/386) by [@LaplaceYoung](https://github.com/LaplaceYoung), closes [#375](https://github.com/rohitg00/agentmemory/issues/375)). `plugin/.mcp.json` now passes `AGENTMEMORY_URL` + `AGENTMEMORY_SECRET` through to the bundled MCP server. Matches the `AGENTMEMORY_MCP_BLOCK` already used by the connect adapters. MCP hosts that expand `${VAR}` (Claude Code, Cursor) inherit shell env; hosts that don't expand (or pass the placeholder literally) still work — the shim now strips literal `${...}` strings and falls back to `http://localhost:3111` (hardening, this release). + +- **`@agentmemory/mcp` rejects literal `${VAR}` placeholders** (this release). Defensive guard in `src/mcp/rest-proxy.ts` and `src/mcp/standalone.ts`: any `AGENTMEMORY_URL` / `AGENTMEMORY_SECRET` value of the form `${...}` is treated as unset, so the shim falls back to `http://localhost:3111` instead of trying to fetch from a host literally named `${AGENTMEMORY_URL}`. Future-proofs against MCP hosts that don't expand env-var placeholders. + +### Added + +- **Pluggable benchmark harness with in-house coding-agent corpus** ([PR #562](https://github.com/rohitg00/agentmemory/pull/562)). New `eval/` directory: 15 fictional coding-agent sessions + 15 hand-graded queries (single-session, multi-session, preference, temporal) plus three adapters (grep, OpenAI embeddings + cosine, agentmemory hybrid). LongMemEval support via `eval/runner/longmemeval.ts`. Sandboxed agentmemory + iii-engine on alt ports via `eval/scripts/sandbox.sh`. No runtime impact on the package — `eval/` is dev-only. + +- **`agentmemory connect codex --with-hooks` opt-in flag** ([PR #564](https://github.com/rohitg00/agentmemory/pull/564), closes [#509](https://github.com/rohitg00/agentmemory/issues/509)). Workaround for [openai/codex#16430](https://github.com/openai/codex/issues/16430), which prevents plugin-local `hooks.json` from dispatching on Codex Desktop. Opt-in flag merges the bundled `hooks.codex.json` into `~/.codex/hooks.json` with absolute paths to the bundled scripts. Idempotent (re-install strips previous entries via the resolved script-path prefix; unrelated user hooks survive). MCP wiring is unchanged. + +- **Cross-platform CI matrix** ([PR #556](https://github.com/rohitg00/agentmemory/pull/556)). Runs tests on Node 20 + 22 with `paths-ignore` for doc-only changes and per-branch concurrency cancellation. + +### Docs + +- **README "Config File" section + Windows path + Claude subscription opt-in example** ([PR #321](https://github.com/rohitg00/agentmemory/pull/321) by [@aqilaziz](https://github.com/aqilaziz), closes [#293](https://github.com/rohitg00/agentmemory/issues/293)). + +- **README sudo install hint for `EACCES` on system Node** ([PR #454](https://github.com/rohitg00/agentmemory/pull/454) by [@kedar-1](https://github.com/kedar-1)). + +### Infrastructure + +- 98 test files, **1091 tests pass** on `fix/pre-release-hardening`. +- All MCP env-var consumers go through a single guarded `resolveEnvOrEmpty()` helper. + ## [0.9.21] — 2026-05-19 Quality + integration wave. Headline: native OpenCode plugin with full Claude Code hook parity ([#237](https://github.com/rohitg00/agentmemory/pull/237) by [@cl0ckt0wer](https://github.com/cl0ckt0wer)). Ten more PRs alongside: `memory_recall` returning the wrong shape, env-file `AGENTMEMORY_DROP_STALE_INDEX` silently ignored, hook scripts crashing on Windows usernames with spaces, viewer search inputs interrupting CJK IME composition, large sessions silently failing at the LLM context limit, lessons invisible to smart-search, Hermes plugin manifest missing hooks, cli onboarding crashing in non-TTY contexts, rebuildIndex blocking boot on large corpora, 25h embed-loop bottleneck during rebuild, and the v0.9.19 iii-console installer workaround can come out now that upstream is fixed. diff --git a/src/functions/graph.ts b/src/functions/graph.ts index 69fa788e..f24e52df 100644 --- a/src/functions/graph.ts +++ b/src/functions/graph.ts @@ -26,8 +26,14 @@ function parseGraphXml( const edges: GraphEdge[] = []; const now = new Date().toISOString(); + // Lazy `[^>]*?` so the self-closing alternation gets a chance before + // greedy attribute matching consumes the trailing `/` and the regex + // falls through to the explicit-close branch, which then runs ahead to + // the *next* entity's `` and silently drops a node (#494 + // follow-up: greedy `[^>]*` was eating the `/` and merging two entity + // declarations into one match). const entityRegex = - /]*(?:\/>|>([\s\S]*?)<\/entity>)/g; + /]*?(?:\/>|>([\s\S]*?)<\/entity>)/g; let match; while ((match = entityRegex.exec(xml)) !== null) { const type = match[1] as GraphNode["type"]; diff --git a/src/mcp/rest-proxy.ts b/src/mcp/rest-proxy.ts index ef03148a..1adbb46e 100644 --- a/src/mcp/rest-proxy.ts +++ b/src/mcp/rest-proxy.ts @@ -31,12 +31,26 @@ let cached: Handle | null = null; let cachedAt = 0; let probeInFlight: Promise | null = null; +// `${VAR}`-style placeholders ship in plugin/.mcp.json so MCP hosts that +// expand them (Claude Code, Cursor) substitute the user's shell value. +// Hosts that DON'T expand pass the literal string `"${AGENTMEMORY_URL}"` +// through to our subprocess — that string is truthy, defeats the `||` +// fallback, and would have us POST to `${AGENTMEMORY_URL}/agentmemory/...` +// (DNS failure). Strip any literal placeholder we see so the fallback +// engages instead. +export function resolveEnvOrEmpty(name: string): string { + const raw = process.env[name]; + if (!raw) return ""; + if (raw.startsWith("${") && raw.endsWith("}")) return ""; + return raw; +} + function baseUrl(): string { - return (process.env["AGENTMEMORY_URL"] || DEFAULT_URL).replace(/\/+$/, ""); + return (resolveEnvOrEmpty("AGENTMEMORY_URL") || DEFAULT_URL).replace(/\/+$/, ""); } function authHeader(): Record { - const secret = process.env["AGENTMEMORY_SECRET"]; + const secret = resolveEnvOrEmpty("AGENTMEMORY_SECRET"); return secret ? { authorization: `Bearer ${secret}` } : {}; } diff --git a/src/mcp/standalone.ts b/src/mcp/standalone.ts index 1413cbf8..f4683747 100644 --- a/src/mcp/standalone.ts +++ b/src/mcp/standalone.ts @@ -32,6 +32,17 @@ const SERVER_INFO = { const kv = new InMemoryKV(getStandalonePersistPath()); let modeAnnounced = false; +function displayAgentmemoryUrl(): string { + // Match the literal-placeholder guard in rest-proxy.ts so log lines + // don't show `${AGENTMEMORY_URL}` when an MCP host passed the + // placeholder through unexpanded. + const raw = process.env["AGENTMEMORY_URL"]; + if (!raw || (raw.startsWith("${") && raw.endsWith("}"))) { + return "http://localhost:3111"; + } + return raw; +} + function announceMode(handle: Handle): void { if (modeAnnounced) return; modeAnnounced = true; @@ -41,7 +52,7 @@ function announceMode(handle: Handle): void { ); } else { process.stderr.write( - `[@agentmemory/mcp] no server reachable at ${process.env["AGENTMEMORY_URL"] || "http://localhost:3111"}; falling back to local InMemoryKV\n`, + `[@agentmemory/mcp] no server reachable at ${displayAgentmemoryUrl()}; falling back to local InMemoryKV\n`, ); } } diff --git a/test/mcp-env-placeholder.test.ts b/test/mcp-env-placeholder.test.ts new file mode 100644 index 00000000..2481cf97 --- /dev/null +++ b/test/mcp-env-placeholder.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect, afterEach, beforeEach } from "vitest"; +import { resolveEnvOrEmpty } from "../src/mcp/rest-proxy.js"; + +const VAR = "AGENTMEMORY_TEST_URL"; + +describe("resolveEnvOrEmpty — guards against literal ${VAR} placeholders", () => { + let original: string | undefined; + + beforeEach(() => { + original = process.env[VAR]; + delete process.env[VAR]; + }); + + afterEach(() => { + if (original === undefined) delete process.env[VAR]; + else process.env[VAR] = original; + }); + + it("returns '' when env var is unset", () => { + expect(resolveEnvOrEmpty(VAR)).toBe(""); + }); + + it("returns '' when env var is empty string", () => { + process.env[VAR] = ""; + expect(resolveEnvOrEmpty(VAR)).toBe(""); + }); + + it("returns '' when env var is literal ${VAR} placeholder", () => { + process.env[VAR] = "${AGENTMEMORY_TEST_URL}"; + expect(resolveEnvOrEmpty(VAR)).toBe(""); + }); + + it("returns '' when env var is a different literal placeholder", () => { + process.env[VAR] = "${SOME_OTHER_VAR}"; + expect(resolveEnvOrEmpty(VAR)).toBe(""); + }); + + it("preserves a real URL value", () => { + process.env[VAR] = "https://memory.prod.example/api"; + expect(resolveEnvOrEmpty(VAR)).toBe("https://memory.prod.example/api"); + }); + + it("preserves a real secret value that happens to contain a $ char", () => { + process.env[VAR] = "secret-with-$dollar"; + expect(resolveEnvOrEmpty(VAR)).toBe("secret-with-$dollar"); + }); + + it("does not treat ${ at start without matching } as placeholder", () => { + process.env[VAR] = "${unclosed"; + expect(resolveEnvOrEmpty(VAR)).toBe("${unclosed"); + }); + + it("does not treat $VAR (no braces) as placeholder", () => { + process.env[VAR] = "$AGENTMEMORY_URL"; + expect(resolveEnvOrEmpty(VAR)).toBe("$AGENTMEMORY_URL"); + }); +});