feat(agents): agents init --mcp wiring (PR 8)#135
Conversation
🦋 Changeset detectedLatest commit: 6c0377f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 18 minutes and 3 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
📝 WalkthroughWalkthroughAdds ChangesMCP Initialization Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 unit tests (beta)
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 |
MCP-only re-run on existing .agents/, interactive prompt fix, git-hook combo paths, force-replace warnings, expanded tests, and v1 docs/plan updates.
Validate mcpServers shape, clean CLI exit on MCP JSON errors, expand tests, architecture/docs sync, and add changeset for --mcp.
Project .cursor/mcp.json and Claude .mcp.json merge idempotently; Claude permissions allow mcp__codemap__*.
MCP-only re-run on existing .agents/, interactive prompt fix, git-hook combo paths, force-replace warnings, expanded tests, and v1 docs/plan updates.
Validate mcpServers shape, clean CLI exit on MCP JSON errors, expand tests, architecture/docs sync, and add changeset for --mcp.
e19e226 to
1128b26
Compare
Unblocks the Release workflow — changeset version failed because two files referenced "codemap" instead of "@stainless-code/codemap".
Clean wave-2 table after rebase: PR 6 merged, PR 8 open (#135).
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/main.ts (1)
108-113:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle
-iin the interactive/git-hooks conflict guard.Line 102 checks only
--interactive, so-ican bypass the intended conflict validation with--git-hooks/--no-git-hooks.💡 Suggested fix
- if (gitHooks !== undefined && rest.includes("--interactive")) { + if ( + gitHooks !== undefined && + (rest.includes("--interactive") || rest.includes("-i")) + ) { console.error( "codemap: --git-hooks / --no-git-hooks cannot be combined with --interactive.", ); process.exit(1); }🤖 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/main.ts` around lines 108 - 113, The conflict guard that validates mutually exclusive interactive/git-hooks options currently only checks for "--interactive" and can be bypassed by "-i"; update the validation logic (the code that determines the interactive variable passed into runAgentsInitCmd and the pre-check that guards gitHooks vs interactive) to treat "-i" the same as "--interactive" (e.g., use rest.includes("--interactive") || rest.includes("-i")) so both the conflict detection and the runAgentsInitCmd call use the same combined check for interactive when evaluating gitHooks and enforcing mutual-exclusion.
🧹 Nitpick comments (2)
src/agents-init-mcp.test.ts (1)
280-315: ⚡ Quick winAdd a regression test for malformed
permissions.allowshape in.claude/settings.json.Current tests cover invalid JSON, but not parseable invalid schema (e.g.,
permissions.allow: "bad"). A focused case should assert: throws without--force, replaces with--force.Also applies to: 317-370
🤖 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/agents-init-mcp.test.ts` around lines 280 - 315, Add a regression test alongside the existing invalid-JSON tests to cover a parseable but schema-invalid `.claude/settings.json` (e.g., permissions.allow set to a string). Create two specs using applyAgentsInitMcp and CODEMAP_MCP_PERMISSION_ALLOW: one that writes a settings.json with permissions.allow: "bad" and asserts applyAgentsInitMcp throws when called without force, and a second that calls applyAgentsInitMcp with force: true and asserts the file is replaced and the resulting JSON.permissions.allow contains CODEMAP_MCP_PERMISSION_ALLOW; reuse the same console.error capture and cleanup pattern and reference the '.claude/settings.json' path and applyAgentsInitMcp in the test text to locate the code.src/cli.test.ts (1)
164-166: ⚡ Quick winTighten the parse-error assertion to avoid false positives.
Line 165 currently passes on any
Codemap:error, even if parsing is not the failure.💡 Suggested refinement
- expect(err).toMatch(/could not parse|Codemap:/); + expect(err).toMatch(/Codemap:.*could not parse/i);🤖 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.test.ts` around lines 164 - 166, The assertion is too loose because matching "Codemap:" lets non-parse errors pass; update the test to assert the parser-specific message only by replacing expect(err).toMatch(/could not parse|Codemap:/) with a stricter match that requires the parse failure text (e.g., expect(err).toMatch(/could not parse/)) so the test only succeeds on actual parse errors while keeping the existing exitCode and the negative check for upsertMcpServersFile.
🤖 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.
Inline comments:
In `@docs/plans/agent-surface-delivery.md`:
- Around line 44-51: Remove the Git conflict markers (<<<<<<<, =======, >>>>>>>)
from docs/plans/agent-surface-delivery.md and resolve the table rows for entries
**6** and **7** by using the merged variant: set the row for **6** to status
"merged" (keep the MCP trace description and PR links) and update the row for
**7** to include PR 6 in its "planned" PR list, ensuring the resulting markdown
table has no conflict markers or duplicate rows.
In `@docs/plans/agents-init-mcp-wiring.md`:
- Line 3: Update the contradictory status fields: if PR `#135` is merged, change
the "**Status:** open (PR [`#135`]...)" text to "**Status:** shipped (PR
[`#135`]...)" so it matches the existing "Shipped: `#135`" note; if the PR is still
open, remove or rephrase the "Shipped: `#135`" line so it doesn't claim the PR
shipped. Ensure the document's "Status" and "Shipped" statements refer to the
same PR state and use the exact PR reference "`#135`" for clarity.
In `@src/agents-init-mcp.ts`:
- Around line 149-163: The mergeClaudeCodemapPermissions function assumes
existing.permissions.allow is an array; first validate that existing.permissions
is an object and Array.isArray(existing.permissions.allow) — if the shape is
malformed, do not mutate or append; instead throw or return a clear error
indicating the settings file is malformed and instruct the user to rerun the
command with --force to overwrite, and add the same validation/behavior to the
other merge routine referenced around lines 175-188 (the other merge/upsert
function) so both functions guard against non-array permissions.allow before
merging.
In `@src/cli/bootstrap.ts`:
- Around line 140-142: The help/error message for unknown agents subcommands in
the console.error call is missing the supported flags --git-hooks and
--no-git-hooks; update the error hint string (the console.error invocation that
prints `codemap: unknown agents command "${rest[1] ?? "(missing)"}". Expected:
codemap agents init [...]`) so the Expected text lists all valid flags for
agents init, including --git-hooks and --no-git-hooks (alongside --force,
--interactive|-i, and --mcp) to keep the hint aligned with actual CLI support.
In `@src/cli/cmd-agents.ts`:
- Around line 23-34: The interactive branch returns the promise from
runAgentsInitInteractive without awaiting it, so rejections bypass the
surrounding try/catch; change the return runAgentsInitInteractive(opts) call to
await the imported runAgentsInitInteractive inside the try block (i.e., await
runAgentsInitInteractive(opts)) so any rejection is caught by the existing catch
handling and errors are handled correctly.
---
Outside diff comments:
In `@src/cli/main.ts`:
- Around line 108-113: The conflict guard that validates mutually exclusive
interactive/git-hooks options currently only checks for "--interactive" and can
be bypassed by "-i"; update the validation logic (the code that determines the
interactive variable passed into runAgentsInitCmd and the pre-check that guards
gitHooks vs interactive) to treat "-i" the same as "--interactive" (e.g., use
rest.includes("--interactive") || rest.includes("-i")) so both the conflict
detection and the runAgentsInitCmd call use the same combined check for
interactive when evaluating gitHooks and enforcing mutual-exclusion.
---
Nitpick comments:
In `@src/agents-init-mcp.test.ts`:
- Around line 280-315: Add a regression test alongside the existing invalid-JSON
tests to cover a parseable but schema-invalid `.claude/settings.json` (e.g.,
permissions.allow set to a string). Create two specs using applyAgentsInitMcp
and CODEMAP_MCP_PERMISSION_ALLOW: one that writes a settings.json with
permissions.allow: "bad" and asserts applyAgentsInitMcp throws when called
without force, and a second that calls applyAgentsInitMcp with force: true and
asserts the file is replaced and the resulting JSON.permissions.allow contains
CODEMAP_MCP_PERMISSION_ALLOW; reuse the same console.error capture and cleanup
pattern and reference the '.claude/settings.json' path and applyAgentsInitMcp in
the test text to locate the code.
In `@src/cli.test.ts`:
- Around line 164-166: The assertion is too loose because matching "Codemap:"
lets non-parse errors pass; update the test to assert the parser-specific
message only by replacing expect(err).toMatch(/could not parse|Codemap:/) with a
stricter match that requires the parse failure text (e.g.,
expect(err).toMatch(/could not parse/)) so the test only succeeds on actual
parse errors while keeping the existing exitCode and the negative check for
upsertMcpServersFile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d183307-cc0a-4ef6-9e82-6fc48cf41159
📒 Files selected for processing (16)
.changeset/agents-init-mcp.mdREADME.mddocs/agents.mddocs/architecture.mddocs/plans/agent-surface-delivery.mddocs/plans/agents-init-mcp-wiring.mddocs/roadmap.mdsrc/agents-init-interactive.tssrc/agents-init-mcp.test.tssrc/agents-init-mcp.tssrc/agents-init.test.tssrc/agents-init.tssrc/cli.test.tssrc/cli/bootstrap.tssrc/cli/cmd-agents.tssrc/cli/main.ts
Preserve non-mcpServers keys on force shape repair; clearer warnings; CLI subprocess side-effect tests; git-hooks+mcp combo coverage.
Registry-driven MCP wiring with verify-after-write for all project-local integrations; address CodeRabbit review (await interactive, bootstrap hints, -i/git-hooks guard, permissions.allow validation).
Empty interactive integration selection skips MCP; slim registry exports; Claude settings Codemap error re-throw; changeset patch bump; docs/roadmap sync.
Per AWS docs, workspace MCP uses .amazonq/default.json (canonical) and .amazonq/mcp.json (legacy, default-on). Registry maps one integration to both writers; IDE entries include transportType stdio.
Amazon Q legacy shape + idempotency tests, filtered integration E2E, registry JSDoc fix, agents.md --mcp wording.
Summary
codemap agents init --mcp(and interactive confirm) writes registry-driven MCP config for 8 integrations: Cursor, Claude Code, VS Code/Copilot, Continue, Cline, Amazon Q, Gemini CLI, and Windsurf (user-global, only when that integration is selected).--mcpwrites 8 registry targets (project-local MCP JSON) plus.claude/settings.jsonpermissions — 9 project-local paths on disk. Windsurf is opt-in via integration selection..cursor/mcp.json,.mcp.json+.claude/settings.json,.vscode/mcp.json,.continue/mcpServers/codemap-mcp.json,.cline/mcp.json,.amazonq/default.json(IDE canonical) +.amazonq/mcp.json(legacy workspace),.gemini/settings.json,~/.codeium/windsurf/mcp_config.json(Windsurf only).codemap mcp --watch --root ${workspaceFolder}(Cursor); other hosts use stdio MCP entries fromAGENTS_INIT_MCP_REGISTRY. Amazon Q IDE entries includetransportType: stdio.permissions.allowwithmcp__codemap__*.--mcp/--git-hookswork when.agents/already exists without--force.-i: MCP writes only for selected integrations; empty selection skips MCP.Test plan
bun test src/agents-init-mcp*.ts src/agents-init.test.ts src/cli.test.tsbun run typecheckcodemap agents init --force --mcpin a test repo; spot-check Cursor +.amazonq/default.json+.amazonq/mcp.jsonDependencies
Independent of #134 (no file overlap). Can merge in either order.