Repository navigation
fix(mcp): preserve Muse-saved entries during reconciliation - #1345
Conversation
- Normalize Muse's inferred stdio type in a separate comparison map, leaving the document passed to atomic writes in the member's saved form. - Use the comparison view for ownership, adoption, removal and historical matching while applying only actual desired-set changes to stored entries; retain the original rollback snapshot. - Add regressions for writing pulls that add, update or remove team servers, preserving personal entries and unchanged managed entries; document the behavior in both guides and troubleshooting skill data. - Verify 33 Muse unit tests and all 7 built-CLI Muse E2E cases, including a writing pull followed by uninstall, plus TypeScript and lint. Refs: Tencent#1221
|
No findings in the specified PR diff. The comparison map separates Muse normalization from saved entries, while preserving the original rollback snapshot. The changes and regression tests are focused on the stated defect. The PR description satisfies the testing requirement: it records a build and representative real-CLI pull/uninstall verification. It also explicitly documents the two broader-suite failures reproduced on the baseline. Review was read-only; I did not run, build, or install PR code. |
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
@nateEc Thanks, approving. This closes the write-back half of the #1221 Muse item. #1346 covers the key-order half.
We built this head, its merge-base 83f6d1db and main 54622c3a. We ran a real pull that writes settings.json, adding a team server, while Muse-saved entries without type were present.
- base and main: the pull adds
"type": "stdio"to thepersonalandlocalentries. - this head: they stay byte-for-byte unchanged, and only the new
extraentry getstype: stdio, as a teamai write should.
Uninstall still removes the unchanged entries. The three new unit cases fail on base and pass here, muse.test.ts (33), the MCP unit set (536) and muse-support (7/7) pass, and tsc, lint and CI pass.
Suggestions:
[P2 non-blocking]#1346 conflicts with this PR in four files. Whichever of the two lands second has to keep the normalized view in #1346's legacy fallback:judge(name, comparable.servers[name], previous.hash)rather thandoc.servers[name]. We built the two together. Without that line, an unchanged legacy stdio server is kept on uninstall and amuse.test.tslegacy case fails, though the e2e tests pass. With it, everything passes. Landing this one first keeps the change on #1346's side.[P2 non-blocking]Thesharing.md(EN and zh-CN) and troubleshooting edits describe an internal comparison a user doesn't act on. AGENTS.md asks to leave docs alone for such a fix, so we'd drop them.[P3 nit]comparable.servers[name] ?? entryin the trailing stale-entry loop can't fall back, so?? entryis dead code.
CI review: it reported no findings and accepted the testing record. We agree on the code. The doc point and the interaction with #1346 are our additions.
After #1319,
applyJsonnormalized Muse's saved MCP map in place and then wrote it. Adding, updating or removing one team server could therefore addtype: stdioto a personal server or an unchanged managed server.Use a separate comparison view for normalization and ownership checks. Apply changes to the original document, retaining its saved entry shapes and original rollback snapshot.
Validation:
npx vitest run src/__tests__/muse.test.ts: 33 passed.npm run buildandnpx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/muse-support.test.ts: 7 passed, including a real built-CLI writing pull followed by uninstall that leaves the personal server unchanged.npx tsc --noEmit,npm run lint, andgit diff --check: passed.State review:
readJsonDocreads settings, ownership/adoption/history checks consume the comparison map, andwriteJsonDocwrites the original map with only actual desired-set changes.previousDataremains the original rollback snapshot. Installed-entry/doctor normalization stays read-only. Bilingual MCP guidance and troubleshooting skill data are updated.The saved settings rewrite is a deterministic fixture; no live Muse process was available locally.
Broader regression:
npx vitest run --maxWorkers=6in a frozen integration worktree containing the five changes finished with 500 files passed, 12,285 tests passed, 36 skipped, 2 failed. Both failures are the pre-existingsubmodule-checkout.test.tsempty legacy hook-file cases; a clean83f6d1dbworktree independently reproduced 24 passed / the same 2 failed. This is an integration-suite record, not a claim that the baseline is fully green.Refs #1221. One independently reviewable defect from the issue; the audit/issue remains open for its other items.