fix: pre-release hardening for 0.9.22 — regex correctness + MCP env guard#588
Conversation
…uard 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* `</entity>` — 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`).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR fixes environment placeholder handling in MCP configuration and XML entity parsing in graph functions. It introduces ChangesBug Fixes and Hardening
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Pre-release hardening for 0.9.22. Three concerns surfaced auditing PRs merged since v0.9.21:
1. Graph parser regex correctness (real regression from #494)
#494added self-closing entity support with:The greedy
[^>]*eats the trailing/of a self-closing tag, so the alternation falls through to the explicit-close branch, and[\s\S]*?runs ahead to the next</entity>— merging two entity declarations into one match and silently dropping a node. The test that #494 itself added (graph-extract accepts self-closing entity tags) was failing on main but the failure was masked by being a single test in the suite.Direct repro:
Fix: switch greedy
[^>]*→ lazy[^>]*?so the self-closing branch is reached before the explicit-close branch greedily consumes the next entity. Both branches now match correctly; #494's test passes; allows attribute values containing/(e.g.path="a/b/c").2. MCP env-var literal-placeholder guard
#386ships${AGENTMEMORY_URL}/${AGENTMEMORY_SECRET}inplugin/.mcp.json. Claude Code and Cursor expand those at MCP launch; hosts that don't expand pass the literal string through. A truthy literal"${AGENTMEMORY_URL}"defeats the|| DEFAULT_URLfallback insrc/mcp/rest-proxy.ts:35— the shim would POST to${AGENTMEMORY_URL}/agentmemory/...(DNS failure).Fix: new exported
resolveEnvOrEmpty(name)inrest-proxy.tsthat returns""for any value matching/^\$\{.*\}$/.baseUrl()andauthHeader()route through it. Mirrored insrc/mcp/standalone.ts's mode-announce log line so error messages don't show${AGENTMEMORY_URL}literal.3. CHANGELOG for 0.9.22
Entries for all PRs landed since v0.9.21:
Diff
src/functions/graph.ts—+7/-1lazy quantifiersrc/mcp/rest-proxy.ts—+16/-2placeholder guard + helper exportsrc/mcp/standalone.ts—+12/-1displayAgentmemoryUrl()for log linetest/mcp-env-placeholder.test.ts—+578 unit tests on the guard contractCHANGELOG.md—+39/-0unreleased section131 insertions, 4 deletions across 5 files.
Validation
npm test→ 98 test files, 1091 tests pass (was failing on 1 test before this PR — fix: parse self-closing graph entities #494's self-closing test).npm pack+ fresh install in clean/tmp/: 142 files shipped, 180 deps resolved,iii-sdk@0.11.2pinned,plugin/{hooks,opencode,scripts,skills}/present.Release readiness
After this lands, 0.9.22 is shippable. Open contributor PRs still recommended for merge before tag:
codex plugin addcommand--with-hooksOr release 0.9.22 with what's already on main + this hardening, defer those to 0.9.23. Your call.
Summary by CodeRabbit
Bug Fixes
${...}patterns as empty values, preventing misuse as fallback valuesTests
Documentation