feat(cli): clarify MCP is opt-in — REST primary, MCP for MCP-only clients#409
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds a ChangesProtocol Notes Architecture
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/connect/types.ts (1)
10-15: ⚡ Quick winDrop the WHAT-style JSDoc on
protocolNoteand keep naming self-explanatory.The new comment explains behavior in detail and conflicts with the repo rule to avoid WHAT-comments in TS files. Prefer a concise field name and no explanatory block here.
As per coding guidelines,
src/**/*.ts: "Avoid code comments explaining WHAT — use clear naming instead".🤖 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 `@src/cli/connect/types.ts` around lines 10 - 15, Remove the WHAT-style JSDoc block above the protocolNote field and replace it with a concise, self-explanatory name (rename protocolNote → protocolSummary) so the field itself conveys purpose; keep no explanatory block comment, and update any usages of protocolNote to protocolSummary.
🤖 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 `@src/cli/connect/types.ts`:
- Around line 10-15: Remove the WHAT-style JSDoc block above the protocolNote
field and replace it with a concise, self-explanatory name (rename protocolNote
→ protocolSummary) so the field itself conveys purpose; keep no explanatory
block comment, and update any usages of protocolNote to protocolSummary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8838a1ca-ca9f-42c0-8d8f-9c59f58932b6
📒 Files selected for processing (13)
src/cli.tssrc/cli/connect/claude-code.tssrc/cli/connect/codex.tssrc/cli/connect/cursor.tssrc/cli/connect/gemini-cli.tssrc/cli/connect/hermes.tssrc/cli/connect/index.tssrc/cli/connect/json-mcp-adapter.tssrc/cli/connect/openclaw.tssrc/cli/connect/openhuman.tssrc/cli/connect/pi.tssrc/cli/connect/types.tssrc/index.ts
Patch bump per the established rule: additive surface only, no breaks to MemoryProvider trait, exported types, or default behaviour. New top-level subcommands (`--reset` already shipped 0.9.15, no new commands here) are opt-in. PRs included since v0.9.15: #408 — onboarding wires selected agents inline + memory-share callout #409 — clarify MCP is opt-in (REST primary) #410 — 5-port ready panel, iii console install, global-install prompt #411 — splash banner rerender + README install hoist + npx caveat #415 — agent-memory.dev refresh (FeaturedIn bar + Agents/Compare/ CommandCenter/Hero updates) Files bumped (9): package.json, packages/mcp/package.json, plugin/.claude-plugin/plugin.json, plugin/.codex-plugin/plugin.json, src/version.ts, src/types.ts, src/functions/export-import.ts, test/export-import.test.ts, CHANGELOG.md
Summary
Today's boot output foregrounds MCP equally with REST, and the
connectadapter logs say nothing about which protocol they're wiring. Users were reading this as MCP being required. It isn't — REST is the primary surface for hooks-supporting agents (Claude Code, Codex, OpenClaw, Hermes, pi, OpenHuman), and MCP is the bridge for MCP-only clients (Cursor, Gemini CLI, OpenCode, Cline, Goose, Kilo Code, Aider, Claude Desktop, Windsurf, Roo Code).Three rewording passes, same numbers everywhere, no behavioral change.
1. Boot endpoints summary (
src/index.ts)Before:
```
Endpoints: 107 REST + 51 MCP tools + 6 MCP resources + 3 MCP prompts
```
After:
```
REST API: 107 endpoints at http://localhost:3111/agentmemory/*
MCP surface (opt-in via `npx @agentmemory/mcp`): 51 tools · 6 resources · 3 prompts
```
2.
connectadapter pre-install linesAdded a
protocolNote: stringfield onConnectAdapter.runAdapterprints it once, right after the existing `Wiring …` step. Same field is plumbed throughcreateJsonMcpAdapterso cursor/gemini-cli/openclaw set it too.`→ Using MCP. Hooks are also available — see docs/.md.`
`→ Using native hooks (REST API at :3111). MCP not required.`
`→ Using MCP (the only protocol speaks). Memory bridge runs at :3111 underneath.`
3.
mcpsubcommand--helptext (src/cli.ts)Before: `mcp Start standalone MCP server (no engine required)`
After: `mcp Start standalone MCP shim — opt-in surface for MCP-only clients (Cursor, Gemini CLI, etc). REST always available at :3111.`
Why
Rohit pushed back directly: today's CLI surface reads as MCP-required. This is a copy fix that puts REST in the primary slot everywhere a user first encounters the protocol choice — boot output,
connect,--help.Test plan
npm run buildpassesorigin/mainbaseline)Summary by CodeRabbit