Skip to content

chore: add deepwiki MCP server config - #1612

Merged
flora131 merged 2 commits into
mainfrom
chore/add-deepwiki-mcp-config
Jul 3, 2026
Merged

chore: add deepwiki MCP server config#1612
flora131 merged 2 commits into
mainfrom
chore/add-deepwiki-mcp-config

Conversation

@flora131

@flora131 flora131 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a project-level .mcp.json registering the DeepWiki MCP server so agent sessions in this repo can query DeepWiki (https://mcp.deepwiki.com/mcp) out of the box, without any per-developer local setup. Also hardens the binary smoke tests so this new repo-local config can't interfere with CI.

Changes

  • Add .mcp.json with a deepwiki entry under mcpServers
  • Run the release and test-workflow binary smoke tests (atomic --no-session) from a clean temp working directory on Linux/macOS and Windows, so repo-local config like .mcp.json can't leak into the smoke run and hang startup

Notes

  • This file was previously removed in 75c7e92 ("chore: remove local MCP config"); this PR intentionally re-adds it as shared project config rather than local-only config.
  • The smoke-test cwd change (.github/workflows/publish.yml, .github/workflows/test.yml) was added after discovering the new .mcp.json caused smoke tests to pick up repo-local MCP config during startup.

Assistant-model: Claude Fable 5
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review

Overall this looks good and safe to merge — it's a small, well-scoped config change, and I verified it against how the repo's MCP extension actually loads config. A couple of optional suggestions below.

✅ Correctness

  • Location is right. Project-root .mcp.json is a recognized shared-project config path (PROJECT_CONFIG_NAME = ".mcp.json" in packages/mcp/config.ts:12), so agent sessions will discover it as intended.
  • URL-only entry works. An entry with just url (no type) is supported — the server manager auto-negotiates streamable HTTP and falls back to SSE (packages/mcp/server-manager.ts:205-230).
  • History checks out. The prior removal in 75c7e92 dropped an obsolete config left over from the browser-use migration branch (it pointed at the GitHub Copilot MCP server, not DeepWiki), so re-adding a shared DeepWiki config is not reverting that decision — the PR description explains this well.

💡 Suggestions (optional)

  1. Add an explicit "type": "http". The previous .mcp.json and the example in research/docs/2026-02-06-mcp-tool-calling-opentui.md:109-111 both declare the transport explicitly. It skips the transport-probe fallback on connect and makes intent obvious to readers:
    {
      "mcpServers": {
        "deepwiki": {
          "type": "http",
          "url": "https://mcp.deepwiki.com/mcp"
        }
      }
    }
  2. Indentation nit: the file uses 4-space indent while the repo's JSON files (e.g. root package.json) use 2 spaces.

🔒 Security

  • No secrets in the file (the old removed config interpolated ${GH_TOKEN} into a header; this one is clean), so committing it as shared config is fine.
  • Registering a third-party remote MCP server repo-wide does mean every agent session auto-discovers an external endpoint, and remote tool descriptions are a prompt-injection surface. This is acceptably mitigated by the extension's consent gating (ConsentManager, default once-per-server — users approve the server before its tools run), and it's consistent with the DeepWiki badge already in the README. Just noting it for the record.

🧪 Tests

None needed — config-only change with no runtime code.

The release-archive smoke steps invoked the built atomic binary with
--no-session from the repository checkout, so repo-local config (the
newly added root .mcp.json) was discovered at startup and the binary
hung connecting to the remote DeepWiki MCP server, stalling CI.

Run the --no-session smoke invocation from a clean RUNNER_TEMP working
directory in both test.yml and publish.yml (Linux and Windows) so
repo-local config can never leak into smoke runs.

Assistant-model: Claude Fable 5
@flora131
flora131 merged commit 8053939 into main Jul 3, 2026
11 checks passed
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Code Review

Overview: This PR re-adds a repo-root .mcp.json registering the DeepWiki remote MCP server, and updates the four CI smoke-test blocks (linux/windows in both test.yml and publish.yml) to run atomic --no-session from a clean temp cwd so the new config can't hang the smoke run. The CI mechanics are correct (smoke_dir/$atomic are absolute, $LASTEXITCODE is captured before Pop-Location), but the workflow comments themselves admit the underlying problem — ".mcp.json … can hang startup" — and the PR ships that hazard to users while removing CI's ability to see it. Findings, most severe first:

1. The startup hang is shipped to every fresh environment; only CI is shielded — .mcp.json:4

On any machine without the global MCP metadata cache (~/.atomic/agent/mcp-cache.json — i.e. every new dev machine, container, or CI runner HOME), packages/mcp/init.ts:80-82 sets bootstrapAll = true and lines 108-119 then connect all configured servers at startup, ignoring the default lazy lifecycle (and an explicit "lifecycle": "lazy" would not opt out either). The connect path in packages/mcp/server-manager.ts has no timeout/AbortSignal of its own: the StreamableHTTP probe is bounded only by the MCP SDK's 60s initialize default, and on non-401 failure it falls back to SSEClientTransport (server-manager.ts:230) whose start() has no timeout at all — a blackholed endpoint hangs indefinitely, and the pending socket keeps short-lived --no-session/-p runs from exiting. That is exactly the hang this PR's own comments describe, worked around in four CI blocks instead of fixed in the product. Consider pairing this PR with (or landing first) a product-side fix: a connect timeout in server-manager.ts, and/or having bootstrapAll skip (or time-box) lazy servers.

2. No trust/consent gate on project-scoped MCP servers — .mcp.json:3

loadMcpConfig (packages/mcp/config.ts:160-207) merges the shared-project .mcp.json unconditionally; the only consent mechanism in the package (ConsentManager) gates MCP-Apps UI tools, never server connection. So committing this file makes every contributor's session silently connect to a third-party server — including non-interactive runs where no prompt could be shown. More importantly, it establishes the precedent that committed .mcp.json entries are fine: a stdio entry ({"command": ...}) goes through StdioClientTransport (server-manager.ts:78-106) and would execute an arbitrary command on checkout + first startup with zero prompts. Note this file was deliberately removed in 75c7e92; before re-adding shared remote-server config, it's worth deciding whether project-scoped servers should require a first-use trust prompt (as e.g. Claude Code does for .mcp.json).

3. Smoke tests lose their only coverage of "binary starts with project config present", and have no time bound — .github/workflows/test.yml:104

After this change, all four --no-session smoke runs execute from an empty temp dir; the only repo-cwd invocations left are --version, which the workflow comment itself notes exits before extensions load. So the exact regression class just observed (project config breaks/hangs startup) is now permanently unexercised — a future release that hangs in any repo containing a .mcp.json would pass CI and publish. Also, neither workflow has any timeout-minutes or timeout wrapper, so a residual hang stalls the job until GitHub's 360-minute default with no diagnostic. Suggestion: keep the isolated run, but add a second bounded run (timeout 120 … / step-level timeout-minutes) from a temp cwd seeded with the repo's .mcp.json so the config-loading startup path stays covered without risking a wedge.

4. .mcp.json discovery is exact-cwd only, so the server is silently absent from subdirectory sessions — .mcp.json:2

getProjectConfigPath (packages/mcp/config.ts:86-88) is resolve(cwd, ".mcp.json") with no upward walk (findProjectRoot exists but is only used for RepoPrompt write-targeting), and the .atomic/ project override is likewise join(cwd, …). A session started from packages/coding-agent/ (a common place to work in this monorepo) never sees DeepWiki, undermining the PR's "out of the box for agent sessions in this repo" goal — while root-cwd sessions still pay the first-run connect cost from finding #1.

5. The smoke logic is now copy-pasted 4× and has already drifted — .github/workflows/publish.yml:91

This PR grows four near-identical blocks (with three different temp-dir naming schemes and a duplicated 4-line comment). The drift is already real: publish.yml's required-path lists are missing node_modules/@bufbuild/protobuf/package.json in both the linux and windows variants, which test.yml checks — so the release-gating smoke is weaker than the PR-gating one today. The repo already uses scripts/ for shared CI shell (scripts/build-binaries.sh is called from both workflows); extracting scripts/smoke-test-binary.sh/.ps1 would remove the drift surface. At minimum, please add the missing protobuf entry to publish.yml.

6. docs/ci.md local-repro instructions are now stale and reproduce the hang — docs/ci.md:313

The Release Checklist's copy-paste repro (lines ~297-326) runs output=$(printf '' | "$tmpdir/atomic/atomic" --no-session 2>&1) from the repo root — which, after this PR, contains the very .mcp.json that hangs startup. CLAUDE.md's Docs section requires keeping docs current ("ALWAYS keep the user-facing docs … up-to-date… Prefer to keep other docs up-to-date as well") and points to docs/ci.md as the CI overview. Please update the doc to match the new temp-cwd procedure.


Minor: the pwsh Push-Location/Pop-Location pair could be a plain Set-Location $workDir (each step is a fresh shell and nothing after depends on the restored cwd), but that's cosmetic.

Overall: the CI scripting itself is sound and the isolation intent is reasonable, but as-is the PR trades away the signal that caught this bug while shipping the bug's trigger to every fresh clone. Findings #1#3 are the ones I'd want addressed (or explicitly accepted) before merge.

Review performed by Claude (automated PR review); findings were adversarially verified against packages/mcp and the workflow files before posting.

@flora131
flora131 deleted the chore/add-deepwiki-mcp-config branch August 14, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant