fix(mcp): full 51-tool surface by default + env defaults in plugin manifest - #650
Conversation
…nifest Two user-blocking MCP issues, single root surface. #510 Claude Code drops the agentmemory MCP server silently when AGENTMEMORY_URL is unset. Per Claude Code MCP docs: 'If a required environment variable is not set and has no default value, Claude Code will fail to parse the config' — and a parse failure removes the server from /reload-plugins output with no warning. plugin/.mcp.json now uses ${VAR:-default} form for AGENTMEMORY_URL, AGENTMEMORY_SECRET, and AGENTMEMORY_TOOLS so fresh installs Just Work without exporting envs. #553 OpenCode (and Claude Code, and Codex) saw 8 tools instead of the 51 every plugin manifest advertises. getVisibleTools() defaulted to AGENTMEMORY_TOOLS=core which capped at 8 ESSENTIAL_TOOLS. README has always said '51 MCP tools' — the default now matches the advertising. AGENTMEMORY_TOOLS=core still available for users who want the lean set. CLI help text and existing codex-plugin test updated to the new contract. New test/mcp-surface-default.test.ts adds 4 regression cases covering default, all, core, and the env-default form in the plugin manifest. 1141/1141 vitest pass.
|
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 (2)
📝 WalkthroughWalkthroughThis PR makes the MCP default tool surface "all" by adding ChangesDefault MCP Tool Surface Visibility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/mcp-surface-default.test.ts (2)
23-28: ⚡ Quick winConsider using exact equality for the full tool count.
Line 27 uses
toBeGreaterThanOrEqual(48)instead of checking for the exact expected count. The PR description advertises "51 tools" andgetAllTools()should return a stable count. Using>=48could silently pass if tools are removed, masking regressions.♻️ Proposed fix
it("default returns the full 51-tool surface, matching plugin advertising", () => { const visible = getVisibleTools(); const all = getAllTools(); expect(visible.length).toBe(all.length); - expect(visible.length).toBeGreaterThanOrEqual(48); + expect(visible.length).toBe(51); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/mcp-surface-default.test.ts` around lines 23 - 28, The test "default returns the full 51-tool surface, matching plugin advertising" uses a weak assertion; replace the leniency with an exact check by asserting the total tool count equals 51. Update the second expectation to assert either expect(visible.length).toBe(51) or expect(getAllTools().length).toBe(51) (referencing getVisibleTools and getAllTools) so the test fails if tools are removed or added unexpectedly.
35-51: ⚡ Quick winAvoid duplicating the ESSENTIAL_TOOLS definition.
Lines 39-50 hardcode the exact list of 8 tool names, duplicating the
ESSENTIAL_TOOLSset defined insrc/mcp/tools-registry.ts:920-929. IfESSENTIAL_TOOLSchanges, both locations must be updated, creating a maintenance burden and risk of inconsistency.♻️ Proposed refactor to import and reuse the canonical set
Update the import on line 3 to include
ESSENTIAL_TOOLS(note: you'll need to export it intools-registry.tsfirst):import { getAllTools, getVisibleTools, + ESSENTIAL_TOOLS, } from "../src/mcp/tools-registry.js";Then replace lines 37-50 with:
it("AGENTMEMORY_TOOLS=core returns the 8 essential tools", () => { process.env["AGENTMEMORY_TOOLS"] = "core"; const names = new Set(getVisibleTools().map((t) => t.name)); expect(names.size).toBe(8); - for (const t of [ - "memory_save", - "memory_recall", - "memory_consolidate", - "memory_smart_search", - "memory_sessions", - "memory_diagnose", - "memory_lesson_save", - "memory_reflect", - ]) { - expect(names.has(t)).toBe(true); - } + expect(names).toEqual(ESSENTIAL_TOOLS); });Note:
ESSENTIAL_TOOLSis currently not exported fromtools-registry.ts. If you prefer not to export internal constants, an alternative is to check thatnamesis a subset ofgetAllTools()tool names and has exactly 8 members, accepting the hardcoded list as documentation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/mcp-surface-default.test.ts` around lines 35 - 51, The test hardcodes the 8-tool list duplicating ESSENTIAL_TOOLS; either export ESSENTIAL_TOOLS from tools-registry.ts and import it into the test and replace the hardcoded array with ESSENTIAL_TOOLS, or (if you prefer not to export the constant) assert that getVisibleTools() returns exactly 8 names and that those names are a subset of getAllTools().map(t => t.name); update the test to reference ESSENTIAL_TOOLS or use getAllTools()/getVisibleTools() comparisons instead of the inline array to avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/mcp-surface-default.test.ts`:
- Around line 23-28: The test "default returns the full 51-tool surface,
matching plugin advertising" uses a weak assertion; replace the leniency with an
exact check by asserting the total tool count equals 51. Update the second
expectation to assert either expect(visible.length).toBe(51) or
expect(getAllTools().length).toBe(51) (referencing getVisibleTools and
getAllTools) so the test fails if tools are removed or added unexpectedly.
- Around line 35-51: The test hardcodes the 8-tool list duplicating
ESSENTIAL_TOOLS; either export ESSENTIAL_TOOLS from tools-registry.ts and import
it into the test and replace the hardcoded array with ESSENTIAL_TOOLS, or (if
you prefer not to export the constant) assert that getVisibleTools() returns
exactly 8 names and that those names are a subset of getAllTools().map(t =>
t.name); update the test to reference ESSENTIAL_TOOLS or use
getAllTools()/getVisibleTools() comparisons instead of the inline array to avoid
duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3621c373-c5fd-429c-8f27-40c1b9bb8433
📒 Files selected for processing (5)
plugin/.mcp.jsonsrc/cli.tssrc/mcp/tools-registry.tstest/codex-plugin.test.tstest/mcp-surface-default.test.ts
PR #650 fixed plugin/.mcp.json but not src/cli/connect/util.ts — `npx @agentmemory/agentmemory connect` (Cursor, Gemini CLI, Windsurf, all JSON-MCP agents) was still writing ${VAR} form into user configs, keeping #510 unfixed on every non-Claude-Code-plugin path. util.ts now ships the same ${VAR:-default} form with AGENTMEMORY_TOOLS=all. Codex TOML adapter already used a literal URL — unaffected. Hermes connect just prints YAML guidance — unaffected. test/cli-connect.test.ts: updated existing assertion to the new contract.
…651) Three new connect adapters and a Port Mapping section in README. #647 Qwen Code (~/.qwen/settings.json) — standard mcpServers shape. Qwen's 16-event hook system reuses Claude Code's payload field names (session_id, cwd, tool_name, tool_input, tool_response), so the existing compiled hook scripts run against Qwen without modification. Plugin extension support (qwen-extension.json) is a separate follow-up. #614 Antigravity (mcp_config.json under platform-specific User dir) — standard mcpServers shape. Gemini CLI is scheduled to stop serving Pro/Ultra/Code-Assist requests 2026-06-18 per Google's announcement; Antigravity is the GA replacement. #618 Kiro (~/.kiro/settings/mcp.json) — standard mcpServers shape. Workspace-level overrides live in .kiro/settings/mcp.json next to the code; we wire user-level by default. All three use the shared json-mcp-adapter, so they inherit the ${VAR:-default} env block from PR #650. #629 Port docs — README now has a Ports table under Configuration: 3111 REST, 3112 streams, 3113 viewer, 49134 engine WS. Env overrides called out. Stale-process cleanup recipe for macOS/Linux/Windows. Test coverage: - test/connect-new-agents.test.ts: 6 cases (detect + install + env-default form for each agent, plus a registry-membership check) - test/cli-connect.test.ts: bumped agent count 8 → 11 - 1156/1156 vitest pass Docs fetched first via context7: - Qwen Code docs (qwenlm.github.io/qwen-code-docs) - Kiro CLI docs (kiro.dev/docs/cli/mcp) - Antigravity docs (settings file layout)
…nifest (rohitg00#650) * fix(mcp): full 51-tool surface by default + env defaults in plugin manifest Two user-blocking MCP issues, single root surface. rohitg00#510 Claude Code drops the agentmemory MCP server silently when AGENTMEMORY_URL is unset. Per Claude Code MCP docs: 'If a required environment variable is not set and has no default value, Claude Code will fail to parse the config' — and a parse failure removes the server from /reload-plugins output with no warning. plugin/.mcp.json now uses ${VAR:-default} form for AGENTMEMORY_URL, AGENTMEMORY_SECRET, and AGENTMEMORY_TOOLS so fresh installs Just Work without exporting envs. rohitg00#553 OpenCode (and Claude Code, and Codex) saw 8 tools instead of the 51 every plugin manifest advertises. getVisibleTools() defaulted to AGENTMEMORY_TOOLS=core which capped at 8 ESSENTIAL_TOOLS. README has always said '51 MCP tools' — the default now matches the advertising. AGENTMEMORY_TOOLS=core still available for users who want the lean set. CLI help text and existing codex-plugin test updated to the new contract. New test/mcp-surface-default.test.ts adds 4 regression cases covering default, all, core, and the env-default form in the plugin manifest. 1141/1141 vitest pass. * fix(connect): apply same env defaults to wired-in MCP block PR rohitg00#650 fixed plugin/.mcp.json but not src/cli/connect/util.ts — `npx @agentmemory/agentmemory connect` (Cursor, Gemini CLI, Windsurf, all JSON-MCP agents) was still writing ${VAR} form into user configs, keeping rohitg00#510 unfixed on every non-Claude-Code-plugin path. util.ts now ships the same ${VAR:-default} form with AGENTMEMORY_TOOLS=all. Codex TOML adapter already used a literal URL — unaffected. Hermes connect just prints YAML guidance — unaffected. test/cli-connect.test.ts: updated existing assertion to the new contract.
…ohitg00#651) Three new connect adapters and a Port Mapping section in README. rohitg00#647 Qwen Code (~/.qwen/settings.json) — standard mcpServers shape. Qwen's 16-event hook system reuses Claude Code's payload field names (session_id, cwd, tool_name, tool_input, tool_response), so the existing compiled hook scripts run against Qwen without modification. Plugin extension support (qwen-extension.json) is a separate follow-up. rohitg00#614 Antigravity (mcp_config.json under platform-specific User dir) — standard mcpServers shape. Gemini CLI is scheduled to stop serving Pro/Ultra/Code-Assist requests 2026-06-18 per Google's announcement; Antigravity is the GA replacement. rohitg00#618 Kiro (~/.kiro/settings/mcp.json) — standard mcpServers shape. Workspace-level overrides live in .kiro/settings/mcp.json next to the code; we wire user-level by default. All three use the shared json-mcp-adapter, so they inherit the ${VAR:-default} env block from PR rohitg00#650. rohitg00#629 Port docs — README now has a Ports table under Configuration: 3111 REST, 3112 streams, 3113 viewer, 49134 engine WS. Env overrides called out. Stale-process cleanup recipe for macOS/Linux/Windows. Test coverage: - test/connect-new-agents.test.ts: 6 cases (detect + install + env-default form for each agent, plus a registry-membership check) - test/cli-connect.test.ts: bumped agent count 8 → 11 - 1156/1156 vitest pass Docs fetched first via context7: - Qwen Code docs (qwenlm.github.io/qwen-code-docs) - Kiro CLI docs (kiro.dev/docs/cli/mcp) - Antigravity docs (settings file layout)
Summary
Two MCP plugin issues, single source.
AGENTMEMORY_URLis unsetplugin/.mcp.jsonuses${VAR:-default}form for all three env entriesgetVisibleTools()default flipped fromcore(8) toall(51)#510 — Claude Code drops the server silently
From the Claude Code MCP docs:
Old
plugin/.mcp.json:Claude Code then drops the server from
/reload-pluginswith no surfaced error. Issue #510 reporter saw1 plugin MCP serverin/reload-pluginsoutput but zero tools available, confirming silent parse failure.New:
Defaults match the documented server runtime (localhost:3111, no auth, all tools). Users who export the vars still override.
#553 — 8 tools instead of 51
getVisibleTools()defaulted toAGENTMEMORY_TOOLS=corewhich returned 8ESSENTIAL_TOOLS. Plugin manifests, README, and the npm description all advertise 51. Default flipped:AGENTMEMORY_TOOLS=corestill gives the lean 8-tool set for users who want it.Tested
npm test— 1141/1141 pass (105 files), +4 new regression cases intest/mcp-surface-default.test.tstest/codex-plugin.test.tsassertion to the new env-default formnpm run build— 21 files, 2448 KBDocs verified against
${VAR:-default}requirement and the silent-parse-fail behavior)Summary by CodeRabbit
Configuration
Documentation
Tests