Skip to content

Commit f0aaf04

Browse files
committed
feat(mcp): document removal and warn on mcpServers in config.yaml
The default agent's system prompt now documents the full MCP lifecycle — add (stdio/http), remove, list, and test — and hard-steers off writing mcpServers into config.yaml/YAML, which Pythinker never parses for MCP (the entry is silently dropped and the server never appears in /mcp). As a backstop, MCP config loading now logs a warning when it finds an mcpServers block in a global or project config.yaml, so a misplaced entry is diagnosable in pythinker.log instead of failing silently. Adds regression tests for the prompt guidance and the loader detection.
1 parent 2db6915 commit f0aaf04

5 files changed

Lines changed: 160 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ GitHub Releases page; `0.8.0` is the new starting line.
2121
- **Agent phase-0 enhancements.** Adds a model-invocable cross-session Recall tool (search and read prior sessions on demand, sanitized and read-only for subagents), read-only MCP resources/prompts surfaced as tools, project-scoped `.pythinker/mcp.json` layering, subagent token/cost roll-up to the orchestrator, and truncated tool output that spills to disk with a recovery hint instead of being lost.
2222
- **No more spurious `coroutine … was never awaited` warnings.** Dropped Sentry's `AsyncioIntegration`, whose `create_task` monkeypatch wrapped every coroutine and — when a task was cancelled before its first step during turn/prompt teardown — orphaned the inner coroutine, printing `WireUISide.receive` and prompt_toolkit "never awaited" `RuntimeWarning`s to the console. The integration added no spans (tracing/profiling are off), and exception capture for async tasks is preserved by the existing asyncio exception handler.
2323
- **Read-only profile guard hardened against version-pinned interpreters.** Inline-code interpreter invocations that use a version-suffixed or absolute binary (`python3.14 -c …`, `/usr/bin/python3.12 -c …`, `node20 -e …`) are now classified as mutating/destructive just like the bare `python`/`node` forms, so they can no longer bypass a read-only subagent profile or skip destructive deliberation.
24-
- **The agent sets up MCP servers on request instead of refusing.** Asked to add or set up an MCP server, the default agent now knows it runs in Pythinker: it configures the server via `pythinker mcp add` or by editing `~/.pythinker/mcp.json` / `./.pythinker/mcp.json`, then tells you to restart to load it — rather than refusing or citing Claude Code/Desktop config paths (`~/.claude.json`) it cannot use.
24+
- **The agent sets up and removes MCP servers on request instead of refusing.** Asked to add, remove, or set up an MCP server, the default agent now knows it runs in Pythinker: it configures the server with the `pythinker mcp add`/`remove` CLI (or by editing `~/.pythinker/mcp.json` / `./.pythinker/mcp.json`), verifies with `pythinker mcp list`/`test`, and tells you to restart or `/reload` to load the change — rather than refusing or citing Claude Code/Desktop config paths (`~/.claude.json`) it cannot use. The prompt now also hard-steers the agent away from writing `mcpServers` into `~/.pythinker/config.yaml` (YAML is never parsed for MCP, so such an entry is silently dropped and the server never appears in `/mcp`). As a backstop, MCP config loading now logs a warning when it finds an `mcpServers` block in a `config.yaml` (global or project), so a human or agent that misplaces it gets a diagnosable trace instead of a silent drop.
2525

2626
## 0.38.0 (2026-06-08)
2727

src/pythinker_code/agents/default/system.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,16 @@ When handling the user's request, if it involves creating, modifying, or running
114114

115115
MCP (Model Context Protocol) servers expose their capabilities as ordinary tools that are already connected and present in your toolset (their descriptions name the originating server). When the user asks to use, test, or call an MCP server, just invoke its tools directly — never pip install the server, import it as a Python module, or search the repo for its configuration. If the user names an MCP server but you see no tools from it in your toolset, the server is not connected (still loading, failed, or unauthorized) rather than missing — do not try to install or build it. Tell the user to check `/mcp` for server status, and for an OAuth server reported as unauthorized, to run `pythinker mcp auth <server_name>`.
116116

117-
When the user asks you to **add, install, or set up a new MCP server** (as opposed to using one that is already connected), you can and should do it — you are running in **Pythinker**, whose MCP configuration is a JSON file you have the tools to edit. This is not Claude Code or Claude Desktop, so never reference `~/.claude.json`, `claude_desktop_config.json`, or any non-Pythinker config path. Server definitions live under the `mcpServers` map in `./.pythinker/mcp.json` (project-scoped, applies to this workspace) and `~/.pythinker/mcp.json` (global); the global file loads first and the project file layers on top. Configure a server either by running `pythinker mcp add <name> …` via `Shell` (e.g. `pythinker mcp add --transport stdio <name> -- npx some-mcp@latest`, or `pythinker mcp add --transport http <name> <url>`) or by writing the `mcpServers` entry directly into one of those JSON files. A newly added server is **not** available in the current session — its tools only connect the next time Pythinker starts — so after configuring it, do the actual edit, then tell the user to restart Pythinker and use `/mcp` to confirm it loaded. Never claim a server has been added without actually writing the config, and never refuse on the grounds that you "have no tool to edit it."
117+
When the user asks you to **add, remove, install, or set up an MCP server** (as opposed to using one that is already connected), you can and should do it — you are running in **Pythinker**, whose MCP configuration is a **JSON** file you have the tools to edit. This is not Claude Code or Claude Desktop, so never reference `~/.claude.json`, `claude_desktop_config.json`, or any non-Pythinker config path. Server definitions live under the `mcpServers` map in `./.pythinker/mcp.json` (project-scoped, applies to this workspace) and `~/.pythinker/mcp.json` (global); the global file loads first and the project file layers on top. **Only these `mcp.json` files are read for MCP.** Never put an `mcpServers` block in `~/.pythinker/config.yaml` or any YAML file — `config.yaml` holds unrelated user settings, is not parsed for MCP, and an `mcpServers` entry there is silently dropped, so the server never appears in `/mcp`.
118+
119+
Prefer the `pythinker mcp` CLI (run via `Shell`) over hand-editing JSON — it validates the entry and fails loudly instead of writing a broken config:
120+
121+
- Add a stdio server: `pythinker mcp add --transport stdio <name> -- npx some-mcp@latest`
122+
- Add an HTTP server: `pythinker mcp add --transport http <name> <url>` (append `--header "KEY: value"` for auth, or `--auth oauth` for an OAuth server)
123+
- Remove a server: `pythinker mcp remove <name>`
124+
- Verify: `pythinker mcp list` to confirm it is registered, and `pythinker mcp test <name>` to check it actually connects and list its tools
125+
126+
If you hand-edit instead, write the `mcpServers` entry only into one of the `mcp.json` files above — never YAML. A newly added or removed server does **not** take effect in the current session; the toolset connects servers only when Pythinker next starts or the user runs `/reload`. So after configuring it, do the actual edit, then tell the user to restart Pythinker (or run `/reload`) and use `/mcp` to confirm the change. Never claim a server has been added or removed without actually writing the config, and never refuse on the grounds that you "have no tool to edit it."
118127

119128
If the `Agent` tool is available, you can use it to delegate a focused subtask to a subagent instance. Treat subagents as focused roles, not just extra capacity: use `explore` for read-only mapping, `plan` for strategy, `coder` or `implementer` for scoped edits, `review` for severity-scored critique, `verifier` for validation gates, and `judge` for final quality checks before delivery. The tool can either start a new instance or resume an existing one by `agent_id`. Subagent instances are persistent session objects with their own context history. When delegating, provide a complete prompt with all necessary context because a newly created subagent instance does not automatically see your current context. If an existing subagent already has useful context or the task clearly continues its prior work, prefer resuming it instead of creating a new instance. Default to foreground subagents. Use `run_in_background=true` only when there is a clear benefit to letting the conversation continue before the subagent finishes, and you do not need the result immediately to decide your next step. Spawn multiple subagents in the same turn when they can investigate independent regions concurrently, but keep background launches within available background task slots.
120129

src/pythinker_code/cli/__init__.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import os
55
from importlib import import_module
66
from pathlib import Path
7-
from typing import TYPE_CHECKING, Annotated, Any, Literal
7+
from typing import TYPE_CHECKING, Annotated, Any, Literal, cast
88

99
import typer
1010

@@ -191,6 +191,51 @@ def _find_project_mcp_config_file() -> Path | None:
191191
return None
192192

193193

194+
def _yaml_files_with_misplaced_mcp_servers() -> list[Path]:
195+
"""Return ``config.yaml``/``.yml`` files that wrongly carry an ``mcpServers`` block.
196+
197+
Pythinker reads MCP servers only from ``mcp.json`` (see
198+
``_load_mcp_configs_from_cli_inputs``); a ``config.yaml`` is not part of that
199+
path. An ``mcpServers`` block written into a ``config.yaml`` is therefore never
200+
parsed for MCP — the server silently fails to load and never appears in
201+
``/mcp``. We surface that as a warning instead of dropping it silently.
202+
203+
Checks the global ``~/.pythinker`` directory and the nearest project
204+
``.pythinker`` directory (walking up to the repository root), mirroring the
205+
two locations from which ``mcp.json`` itself is read.
206+
"""
207+
import yaml
208+
209+
from .mcp import get_global_mcp_config_file
210+
211+
dirs: list[Path] = [get_global_mcp_config_file().parent]
212+
cwd = Path.cwd().resolve()
213+
for directory in (cwd, *cwd.parents):
214+
project_dir = directory / ".pythinker"
215+
if project_dir.is_dir():
216+
dirs.append(project_dir)
217+
if (directory / ".git").exists():
218+
break
219+
220+
offending: list[Path] = []
221+
seen: set[Path] = set()
222+
for directory in dirs:
223+
for name in ("config.yaml", "config.yml"):
224+
path = directory / name
225+
if path in seen:
226+
continue
227+
seen.add(path)
228+
if not path.is_file():
229+
continue
230+
try:
231+
data = yaml.safe_load(path.read_text(encoding="utf-8"))
232+
except (OSError, yaml.YAMLError):
233+
continue
234+
if isinstance(data, dict) and cast("dict[str, Any]", data).get("mcpServers"):
235+
offending.append(path)
236+
return offending
237+
238+
194239
def _load_mcp_configs_from_cli_inputs(
195240
mcp_config_file: list[Path] | None,
196241
mcp_config: list[str] | None,
@@ -239,6 +284,17 @@ def _load_mcp_configs_from_cli_inputs(
239284
except json.JSONDecodeError as e:
240285
raise typer.BadParameter(f"Invalid JSON: {e}", param_hint="--mcp-config") from e
241286

287+
for path in _yaml_files_with_misplaced_mcp_servers():
288+
from pythinker_code.utils.logging import logger
289+
290+
logger.warning(
291+
"Ignoring `mcpServers` in {path}: Pythinker reads MCP servers only from "
292+
"mcp.json, so this block has no effect and the server will not appear in "
293+
"/mcp. Move it into ~/.pythinker/mcp.json (or .pythinker/mcp.json), e.g. "
294+
"with `pythinker mcp add`.",
295+
path=path,
296+
)
297+
242298
return configs
243299

244300

tests/core/test_cli_reload.py

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55

66
import pytest
77

8-
from pythinker_code.cli import _load_mcp_configs_from_cli_inputs
8+
from pythinker_code.cli import (
9+
_load_mcp_configs_from_cli_inputs,
10+
_yaml_files_with_misplaced_mcp_servers,
11+
)
912

1013

1114
def test_load_mcp_configs_rechecks_default_file_between_reloads(
@@ -27,3 +30,68 @@ def test_load_mcp_configs_rechecks_default_file_between_reloads(
2730
default_mcp_file.write_text(json.dumps(expected), encoding="utf-8")
2831

2932
assert _load_mcp_configs_from_cli_inputs(None, None) == [expected]
33+
34+
35+
def test_detects_misplaced_mcp_servers_in_yaml_configs(
36+
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
37+
) -> None:
38+
"""`mcpServers` in a config.yaml (global or project) is flagged, since
39+
Pythinker reads MCP only from mcp.json and would otherwise drop it silently."""
40+
share = tmp_path / "share"
41+
share.mkdir()
42+
monkeypatch.setattr(
43+
"pythinker_code.cli.mcp.get_global_mcp_config_file", lambda: share / "mcp.json"
44+
)
45+
(share / "config.yaml").write_text("mcpServers:\n foo:\n command: npx\n", encoding="utf-8")
46+
47+
project = tmp_path / "proj"
48+
(project / ".pythinker").mkdir(parents=True)
49+
(project / ".git").mkdir()
50+
(project / ".pythinker" / "config.yaml").write_text(
51+
"mcpServers:\n bar:\n command: npx\n", encoding="utf-8"
52+
)
53+
monkeypatch.chdir(project)
54+
55+
found = {p.resolve() for p in _yaml_files_with_misplaced_mcp_servers()}
56+
assert (share / "config.yaml").resolve() in found
57+
assert (project / ".pythinker" / "config.yaml").resolve() in found
58+
59+
60+
def test_clean_yaml_config_is_not_flagged(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
61+
"""A config.yaml without an `mcpServers` block must not be flagged."""
62+
share = tmp_path / "share"
63+
share.mkdir()
64+
monkeypatch.setattr(
65+
"pythinker_code.cli.mcp.get_global_mcp_config_file", lambda: share / "mcp.json"
66+
)
67+
(share / "config.yaml").write_text(
68+
"onboarding:\n seen:\n busy_input_prompt: true\n", encoding="utf-8"
69+
)
70+
project = tmp_path / "proj"
71+
project.mkdir()
72+
(project / ".git").mkdir()
73+
monkeypatch.chdir(project)
74+
75+
assert _yaml_files_with_misplaced_mcp_servers() == []
76+
77+
78+
def test_load_mcp_configs_ignores_misplaced_yaml(
79+
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
80+
) -> None:
81+
"""A stray mcpServers-in-config.yaml must not break loading or leak into the
82+
returned configs — only the real mcp.json is loaded."""
83+
share = tmp_path / "share"
84+
share.mkdir()
85+
mcp_file = share / "mcp.json"
86+
monkeypatch.setattr("pythinker_code.cli.mcp.get_global_mcp_config_file", lambda: mcp_file)
87+
expected = {"mcpServers": {"ctx": {"url": "https://mcp.example.test", "transport": "http"}}}
88+
mcp_file.write_text(json.dumps(expected), encoding="utf-8")
89+
(share / "config.yaml").write_text(
90+
"mcpServers:\n ignored:\n command: npx\n", encoding="utf-8"
91+
)
92+
project = tmp_path / "proj"
93+
project.mkdir()
94+
(project / ".git").mkdir()
95+
monkeypatch.chdir(project)
96+
97+
assert _load_mcp_configs_from_cli_inputs(None, None) == [expected]

tests/core/test_load_agent.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,29 @@ def test_system_prompt_explains_adding_mcp_servers(builtin_args: BuiltinSystemPr
8181
assert "not Claude Code or Claude Desktop" in prompt
8282

8383

84+
def test_system_prompt_explains_removing_and_rejects_yaml_mcp_config(
85+
builtin_args: BuiltinSystemPromptArgs,
86+
):
87+
"""The agent must also know how to *remove* a server, and must be steered off
88+
the real-world failure of writing `mcpServers` into `config.yaml` (YAML),
89+
which Pythinker never parses for MCP — the entry is silently dropped and the
90+
server never shows in `/mcp`.
91+
"""
92+
from pythinker_code.agentspec import DEFAULT_AGENT_FILE
93+
94+
prompt = _load_system_prompt(
95+
DEFAULT_AGENT_FILE.parent / "system.md",
96+
{"ROLE_ADDITIONAL": ""},
97+
builtin_args,
98+
)
99+
100+
# Removal is documented, not just add.
101+
assert "pythinker mcp remove" in prompt
102+
# Hard steer away from the config.yaml / YAML misplacement seen in the wild.
103+
assert "config.yaml" in prompt
104+
assert "silently dropped" in prompt
105+
106+
84107
def test_system_prompt_treats_injected_date_as_authoritative(
85108
builtin_args: BuiltinSystemPromptArgs,
86109
):

0 commit comments

Comments
 (0)