fix: avoid overwriting invalid MCP JSON configs

[Sushanth012]

Summary

• stop JSON MCP adapters from treating an unreadable existing config as an empty config
• leave invalid or JSONC-style settings files unchanged and return a skipped result instead
• add Zed regression tests for preserving existing settings/context servers and for the parse-failure path
• make the Zed test home override work on Windows by setting USERPROFILE as well as HOME

Verification

• npx vitest run test/connect-new-agents.test.ts -t "connect: Zed"

Notes:

• npx tsc --noEmit currently reports project-wide TypeScript errors outside this change.
• npm run build reaches tsdown bundling on Windows but the package script later fails on Unix shell commands: cp ... || true.

Closes #952

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of corrupted or invalid configuration files—now gracefully skipped with clear error reporting instead of being overwritten.
  • Enhanced preservation of existing settings when installing new agent connections.

[vercel]

@Sushanth012 is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cd99c5e9-7725-4759-8470-3d38b07674a1

📥 Commits

Reviewing files that changed from the base of the PR and between f6f9e3c and 3b13bb7.

📒 Files selected for processing (2)

• src/cli/connect/json-mcp-adapter.ts
• test/connect-new-agents.test.ts

---

📝 Walkthrough

Walkthrough

The createJsonMcpAdapter install method is updated to precompute config file existence and parse the existing file once. If the file exists but yields invalid JSON, install now logs an error and returns { kind: "skipped", reason: "invalid-json" } without modifying the file. The backup/mkdir branch is refactored to use the precomputed variable. New Zed tests cover both existing-settings merge preservation and the invalid-JSON skip path.

Changes

Invalid JSON guard and Zed settings merge

Layer / File(s)	Summary
Invalid JSON guard and configExists refactor in install src/cli/connect/json-mcp-adapter.ts	install precomputes configExists and calls readJsonSafe once; adds a guard returning { kind: "skipped", reason: "invalid-json" } when the file exists but parsing returns null; the backup/mkdir branch switches from an inline existsSync call to the precomputed configExists.
Zed tests: merge preservation and invalid-JSON skip test/connect-new-agents.test.ts	Updates node:fs imports to multiline style; adds USERPROFILE save/restore to Zed test setup; adds two tests verifying that existing Zed settings are preserved on merge and that an unparseable settings.json produces a skipped result with no backup directory created.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• rohitg00/agentmemory#677: Modifies the same createJsonMcpAdapter install/config read-write flow in src/cli/connect/json-mcp-adapter.ts, overlapping directly with the adapter core logic changed here.

Poem

🐇 Hoppity-hop through the config file lane,
Invalid JSON? We skip it — no pain!
Existing themes and servers, we'll keep,
No more clobbering settings in sleep.
A backup untouched, a file left in peace —
This bunny made config destruction cease! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing the adapter to avoid overwriting invalid JSON config files.
Linked Issues check	✅ Passed	The PR fully addresses all key coding objectives from issue #952: config parsing with invalid JSON detection, context_servers merging without overwriting existing entries, safe failure handling, and regression test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing config handling: JSON adapter improvements, test suite updates for Zed settings merge behavior, and Windows environment variable support are all within scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}