From ad8c3cda58640888f1cfccf6815c3f2db09a790d Mon Sep 17 00:00:00 2001 From: tzhouam Date: Tue, 25 Aug 2026 13:25:55 +0800 Subject: [PATCH] thin mcp: declare ToolAnnotations so approval-gating hosts can pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-caused from #86's triage: codex raises a per-call approval elicitation for MCP tools without annotations (mcp_tool_call_approval_exec), cancels them outright in headless runs ("user cancelled MCP tool call"), and its interactive approval dialog is where the reported crash lives — the server itself returns review(mode=strict)'s run_id in 161 ms. Every tool now declares truthful hints: `review` is the only tool that mutates state (reserves a Strict run) or reaches the network (readOnlyHint=false, destructiveHint=false, openWorldHint=true); the other six are readOnlyHint=true. The mcp extra floor moves 1.2 -> 1.8, the first version verified to carry ToolAnnotations and the decorator kwarg. The fake FastMCP in the tests records tool kwargs and fakes mcp.types; a new guardrail test pins the read-only surface. codex host doc gains a "known behavior" section for the approval dialog and the headless auto-cancel; SPEC page re-verified with the new invariant. Refs #86. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0165QsXAeLVPrKfj8Yk5zns3 --- doc/architecture/SPEC/thin_mcp_server.md | 7 +++- doc/guide/hosts/codex.md | 16 ++++++++++ pyproject.toml | 2 +- src/infermatrix_copilot/thin_mcp_server.py | 25 +++++++++++---- test/test_thin_mcp_server.py | 37 +++++++++++++++++++++- 5 files changed, 77 insertions(+), 10 deletions(-) diff --git a/doc/architecture/SPEC/thin_mcp_server.md b/doc/architecture/SPEC/thin_mcp_server.md index 483d75ba..a5679bf5 100644 --- a/doc/architecture/SPEC/thin_mcp_server.md +++ b/doc/architecture/SPEC/thin_mcp_server.md @@ -1,6 +1,6 @@ # thin_mcp_server.py —— 规范 - + `LOC ~1286 · 默认 MCP：Direct 路由 + Strict 入口 · refactor-status: oversized` @@ -44,6 +44,11 @@ - **Strict 绝不启动注定失败的 run**：Strict 分支先查 `strict_readiness`， 改为返回缺失项。 - `update_knowledge` 只返回知识贡献入口 —— 它**不是** `imupdate` 的发版审计器。 +- **每个工具都声明 `ToolAnnotations`，且提示必须真实。** 审批门控的宿主（codex 对 + 无注解工具逐次弹批准框，headless 下自动取消，见 #86）靠这些提示放行只读面： + `review` 是唯一保留状态变更（预留 Strict run）与触网（Strict 子进程）的工具， + 其余六个全部 `readOnlyHint=true`。把一个会写的工具标成只读，比不标更糟。 + （要求 `mcp>=1.8`，注解类型自该版本起可用。） ## 边界 —— 不属于这里 Direct 路径里不调模型；不含 Strict 后台机器（`mcp_server.py`）；不定义策略 diff --git a/doc/guide/hosts/codex.md b/doc/guide/hosts/codex.md index bccb1c27..6dad2c88 100644 --- a/doc/guide/hosts/codex.md +++ b/doc/guide/hosts/codex.md @@ -173,6 +173,22 @@ Direct 模式**不跑第二个模型、不改知识、不发评论、不推代 描述里选出有界的知识 owner；范围校验和被引用代码证据的真伪，仍然由 Codex 自己负责。 完成校验器检查的是**评审结构**。 +## 已知行为：MCP 工具审批 + +Codex 会对 MCP 工具调用弹出**逐次批准**对话框（elicitation）。两个后果： + +- **交互式（TUI）**：第一次调用 `review` 时会先看到批准框，批准后才执行。若你的 + Codex 版本在这个批准框上崩溃，请升级 Codex —— server 侧此时**尚未开始**任何工作 + （run 目录都不会创建），崩溃发生在 Codex 自己的审批 UI 里。 +- **headless（`codex exec`，approval=never）**：需要审批的调用会被自动取消， + 报 `user cancelled MCP tool call` —— 这不是 server 错误，而是 Codex 把 + "从不询问"解释为"取消一切需要询问的调用"。 + +server 已为每个工具声明 MCP `ToolAnnotations`：除 `review`（预留 Strict run、 +子进程触网）以外，全部 `readOnlyHint=true`，让按注解放行只读面的 Codex 版本可以 +自动批准。另注意首次启动经由 `uvx` 拉包较慢——安装器已把该 server 的 +`startup_timeout_sec` 设为 120。 + ## 可选：autonomous BYOK 工作流 autonomous 工作流有独立的配置和文档： diff --git a/pyproject.toml b/pyproject.toml index 93b7a9f8..9ecea339 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ dependencies = [ dev = ["pytest>=8.0"] # MCP server (Claude Code / Codex integration) — kept optional so a standalone # CLI install stays dependency-free. -mcp = ["mcp>=1.2,<2"] +mcp = ["mcp>=1.8,<2"] [project.scripts] infermatrix-copilot = "infermatrix_copilot.cli:main" diff --git a/src/infermatrix_copilot/thin_mcp_server.py b/src/infermatrix_copilot/thin_mcp_server.py index 829138d8..900ac086 100644 --- a/src/infermatrix_copilot/thin_mcp_server.py +++ b/src/infermatrix_copilot/thin_mcp_server.py @@ -945,8 +945,16 @@ def build_mcp( core: CopilotMCP | None = None, ): from mcp.server.fastmcp import FastMCP + from mcp.types import ToolAnnotations core = core or CopilotMCP(settings) + + # Approval-gating hosts (codex asks per-call approval for tools without + # hints, and cancels them outright in headless runs) read these + # annotations, so they must stay truthful: `review` reserves run state + # and its Strict child reaches the network; everything else only reads. + read_only = ToolAnnotations( + readOnlyHint=True, idempotentHint=True, openWorldHint=False) mcp = FastMCP( "infermatrix-copilot", instructions=( @@ -984,7 +992,10 @@ def build_mcp( ), ) - @mcp.tool() + @mcp.tool(annotations=ToolAnnotations( + title="Start a Direct or Strict review", + readOnlyHint=False, destructiveHint=False, idempotentHint=False, + openWorldHint=True)) def review( target: str, repo: str = "vllm-omni", @@ -1140,7 +1151,7 @@ def run() -> dict: return _guard(run) - @mcp.tool() + @mcp.tool(annotations=read_only) def validate_direct_review( subtraction_signal: str = "", subtraction: list[dict[str, str]] | None = None, @@ -1182,7 +1193,7 @@ def validate_direct_review( } return result - @mcp.tool() + @mcp.tool(annotations=read_only) def get_review_result(run_id: str, offset: int = 0) -> dict: """Poll a Strict run and page its final report with ``next_offset``.""" def run() -> dict: @@ -1197,7 +1208,7 @@ def run() -> dict: return _guard(run) - @mcp.tool() + @mcp.tool(annotations=read_only) def get_review_status(run_id: str) -> dict: """Return a Strict run's durable status and step progress.""" def run() -> dict: @@ -1212,12 +1223,12 @@ def run() -> dict: return _guard(run) - @mcp.tool() + @mcp.tool(annotations=read_only) def update_knowledge(repo: str = "vllm-omni") -> dict: """Return the knowledge contribution entrypoint for the host to follow.""" return _guard(lambda: {"knowledge_entry": _contributing_entry()}) - @mcp.tool() + @mcp.tool(annotations=read_only) def doc_search( query: str, repo: str = "vllm-omni", @@ -1248,7 +1259,7 @@ def run() -> dict: return _guard(run) - @mcp.tool() + @mcp.tool(annotations=read_only) def doc_read( path: str, repo: str = "vllm-omni", diff --git a/test/test_thin_mcp_server.py b/test/test_thin_mcp_server.py index 96e0dbe3..d2ed5831 100644 --- a/test/test_thin_mcp_server.py +++ b/test/test_thin_mcp_server.py @@ -21,24 +21,34 @@ def _fake_mcp(monkeypatch): class FakeMCP: def __init__(self, *_args, **_kwargs): self.tools = {} + self.tool_annotations = {} - def tool(self): + def tool(self, **kwargs): def register(fn): self.tools[fn.__name__] = fn + self.tool_annotations[fn.__name__] = kwargs.get("annotations") return fn return register + class FakeToolAnnotations: + def __init__(self, **kwargs): + self.__dict__.update(kwargs) + fastmcp_module.FastMCP = FakeMCP + types_module = ModuleType("mcp.types") + types_module.ToolAnnotations = FakeToolAnnotations mcp_module = ModuleType("mcp") mcp_module.__path__ = [] server_module = ModuleType("mcp.server") server_module.__path__ = [] mcp_module.server = server_module + mcp_module.types = types_module server_module.fastmcp = fastmcp_module monkeypatch.setitem(sys.modules, "mcp", mcp_module) monkeypatch.setitem(sys.modules, "mcp.server", server_module) monkeypatch.setitem(sys.modules, "mcp.server.fastmcp", fastmcp_module) + monkeypatch.setitem(sys.modules, "mcp.types", types_module) class FakeCore: def __init__(self): @@ -67,6 +77,31 @@ def get_status(self, run_id): return build_mcp(core=core), core +def test_tool_annotations_mark_the_read_only_surface(monkeypatch): + """Approval-gating hosts (codex) read these hints; keep them truthful. + + Unannotated tools trigger a per-call approval dialog in codex and are + auto-cancelled in its headless runs (#86), so every tool declares its + hints: `review` is the only tool that mutates state (reserves a Strict + run) or reaches the network; everything else only reads. + """ + mcp, _core = _fake_mcp(monkeypatch) + + assert set(mcp.tool_annotations) == set(mcp.tools) + review = mcp.tool_annotations["review"] + assert review is not None + assert review.readOnlyHint is False + assert review.destructiveHint is False + assert review.openWorldHint is True + for name in ("validate_direct_review", "get_review_result", + "get_review_status", "update_knowledge", "doc_search", + "doc_read"): + hints = mcp.tool_annotations[name] + assert hints is not None, name + assert hints.readOnlyHint is True, name + assert hints.openWorldHint is False, name + + def test_direct_entrypoints_do_not_resolve_repo(monkeypatch): mcp, core = _fake_mcp(monkeypatch) assert set(mcp.tools) == {