From 300456855b55e4c14bff3ecd758aa1ad8197d462 Mon Sep 17 00:00:00 2001 From: Curry Date: Wed, 5 Aug 2026 16:44:17 +0800 Subject: [PATCH 1/2] =?UTF-8?q?feat(serve):=20agent=20=E5=8E=9F=E7=94=9F?= =?UTF-8?q?=E6=9F=A5=E8=AF=A2=E9=9D=A2=E2=80=94=E2=80=94=E7=AE=A1=E7=BA=BF?= =?UTF-8?q?=E5=86=99=E7=A0=81=E4=B8=AD=E9=80=94=E7=BB=88=E4=BA=8E=E8=83=BD?= =?UTF-8?q?=E8=A2=AB=E9=97=AE=E4=B8=80=E5=8F=A5=E8=AF=9D=20(#54=20#58)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `code-intel serve --mcp`：stdio MCP server，把已提交 run 投影成六个可调工具。 #58 的审计结论是，管线在写码时隐形是结构性的，不是采纳惰性：每个 agent 面 都是"读/审"形状或事后门，没有任何一个在写的过程中帮上忙。它的提案 1（SKILL 写路径触发词）与提案 2（`change impact --staleness advisory`）已先后落地， 剩下提案 3——agent 引过来之后，没有一句话能问的接口。本次补上这一块。 工具： - get_gate_verdict 权威 run 的门禁结论 + 第一条失败规则 + 最小重跑命令 - get_facts 按 artifact type / schema / 子串查已验证事实 - get_evidence 一条 finding 的证据链：产物、sha256、记录时的 snapshot - get_audit_status 各科室结论、评分、覆盖 - get_change_impact 改这些文件波及谁、先跑哪些测试（默认 stale-advisory） - plan_structural_edit ast-grep 结构改写预览 只读边界是机械的，不是口头的： - 门禁判定照旧只走 CLI 与 CI 路径。查询面被 prompt injection 说服也改不了结论。 - 唯一会执行东西的 plan_structural_edit，在跑之前拿注册表核对自己 capability 的 allowedEffects，一旦出现 repo_mutation 就拒绝执行——这是 ai-safety-002 教训的架构化，有测试为证。 - 路径参数复用 change_impact / evidence_query 的既有请求类型：JSON 进来的路径 和 --changed 打进来的走同一道越界闸，不新开一个更松的解析器。 - 参数集闭合校验先于任何 IO：schema 里的 additionalProperties:false 只是给 规矩客户端的提示，不是闸门。 诚实位： - 工具拒答走 isError 结果，不走 JSON-RPC error。"还没跑过 run"是答案，不是 传输故障；混淆二者会把能用的工具训成"看起来老是坏的"。 - 没有 audit 产物时报 unavailable 并说明原因，不静默当成审计通过。 - 证据链查不到时报 unbacked，并声明这只表示没有已提交产物提到它，不表示 该 finding 为假。 README 与 SKILL.md 改为主推查询面，全量 --mode normal 降为深检模式（#54 出口 条件 3）。仓库 .mcp.json 注册本 server，--repo 显式钉住：worktree 目录名不是 run commit 发布时用的仓名。 路由拆进 routes/serve_routes.rs，与 edit_routes / run_routes 同一约定—— routes/mod.rs 已逼近本仓自己的巨石阈值，新命令不该再往里堆。 验证：cargo test 全绿（56 个 suite）、cargo fmt --check 干净、clippy 对新模块 零告警、权威 self-scan 在本分支 completed/green（4657 anchors verified， 0 dropped，failing_rules 0）。真机 stdio 会话核实六个工具全部作答：脏工作树下 get_change_impact 返回 stale-advisory 并同时给出 recorded/current 两个 snapshot identity，requireCurrent:true 仍按原语义 fail-closed。 Refs #54 #58 --- .mcp.json | 10 + CHANGELOG.md | 2 + README.md | 27 + crates/code-intel-cli/src/change_impact.rs | 33 ++ .../src/cli/command_catalog/mod.rs | 6 +- .../src/cli/command_catalog/routes/mod.rs | 2 + .../command_catalog/routes/serve_routes.rs | 54 ++ .../src/cli/command_catalog/tests.rs | 2 +- crates/code-intel-cli/src/cli/legacy.rs | 1 + crates/code-intel-cli/src/evidence_query.rs | 32 ++ crates/code-intel-cli/src/main.rs | 1 + .../code-intel-cli/src/mcp_serve/handlers.rs | 476 ++++++++++++++++++ crates/code-intel-cli/src/mcp_serve/mod.rs | 316 ++++++++++++ crates/code-intel-cli/src/mcp_serve/tests.rs | 350 +++++++++++++ crates/code-intel-cli/src/mcp_serve/tools.rs | 175 +++++++ .../tests/fixtures/cli-head-parity.v2.json | 4 +- crates/code-intel-cli/tests/mcp_serve.rs | 207 ++++++++ skills/code-intel-pipeline/SKILL.md | 19 + 18 files changed, 1712 insertions(+), 5 deletions(-) create mode 100644 crates/code-intel-cli/src/cli/command_catalog/routes/serve_routes.rs create mode 100644 crates/code-intel-cli/src/mcp_serve/handlers.rs create mode 100644 crates/code-intel-cli/src/mcp_serve/mod.rs create mode 100644 crates/code-intel-cli/src/mcp_serve/tests.rs create mode 100644 crates/code-intel-cli/src/mcp_serve/tools.rs create mode 100644 crates/code-intel-cli/tests/mcp_serve.rs diff --git a/.mcp.json b/.mcp.json index 7a7f94ef..bfe33544 100644 --- a/.mcp.json +++ b/.mcp.json @@ -1,5 +1,15 @@ { "mcpServers": { + "code-intel": { + "command": "code-intel", + "args": [ + "serve", + "--mcp", + "--repo", + "code-intel-pipeline" + ], + "description": "code-intel: committed-run query surface — gate verdict, facts, evidence chain, audit status, mid-edit blast radius, structural-edit preview. Read-only; gates nowhere. --repo is pinned because a worktree directory name is not the name run commit publishes under; --repo-path defaults to the working directory." + }, "repowise": { "command": "repowise", "args": [ diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ee38a9d..1640ba32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`code-intel serve --mcp`：agent 原生查询面上线，管线在写码中途终于能被问一句话**（#54、#58 提案 3）。stdio MCP server，六个工具：`get_gate_verdict`（权威 run 的门禁结论 + 第一条失败规则 + 最小重跑命令）、`get_facts`（按 artifact type/schema/子串查已验证事实）、`get_evidence`（一条 finding 的证据链：产物、sha256、记录时的 snapshot；查不到明说 `unbacked`，不装真）、`get_audit_status`（各科室结论；没跑过 audit 明说 `unavailable`，不装绿）、`get_change_impact`（默认 stale-advisory——CLI 在这里 fail-closed 正是管线写码时隐形的原因，#58 定为 critical）、`plan_structural_edit`（ast-grep 预览，`repositoryMutation=false`）。**只读，不裁决**：门禁判定照旧只走 CLI 与 CI，查询面被 prompt injection 说服也改不了结论；唯一会执行东西的 `plan_structural_edit` 在跑之前拿注册表核对自己的 capability 声明，一旦出现 `repo_mutation` 直接拒绝（有测试为证）。路径参数复用 `change_impact` / `evidence_query` 的既有请求类型，JSON 进来的路径和 `--changed` 打进来的走同一道越界闸。工具拒答走 `isError` 结果而不是 JSON-RPC error——"还没跑过 run"是答案，不是传输故障。仓库 `.mcp.json` 已注册；README 与 SKILL.md 改为主推查询面，全量扫描降为深检模式。 + - **`code-intel edit apply` + `edit.span-apply` 能力：span 寻址补丁，终结"改一个字重写整行"**（#96 item 1、charter gate G4 #139）。`--span --expect-sha256 <该 span 当前字节的 sha256> --replacement `，行列 1-based、结束列开区间；同一文件可带多个互不重叠的 span，全部对改动前字节定位、写临时同级文件后 rename，整文件原子替换。写之前逐 span 比对 digest：不一致即拒绝（退出码 10、`applied:false`），产物给出 `expectedSha256` / `foundSha256` / 有界原文，且信封 `observedEffects` 不含 `repo_mutation`——"没写"是机器可校验的，不是自报的。写路径全程走既有 capability envelope（`edit.span-apply.compat`，`allowedEffects` 含 `repo_mutation`），能力自身在动手前先检查请求的 effectPolicy，把事后审计变成事前门。不含 ast-grep 接线（下一档）。 ### Fixed diff --git a/README.md b/README.md index 368f96e7..8876aa56 100644 --- a/README.md +++ b/README.md @@ -536,6 +536,33 @@ code-intel audit --operation scope --repo C:\path\to\repo --since ## Agent 工作流 +### 先接查询面，全量扫描是深检模式 + +Agent 平时问单点问题走 MCP，不必为一个问题跑一整轮 pipeline： + +```powershell +code-intel serve --mcp --repo +``` + +stdio MCP server，按需起、随 session 生灭，答案全部来自最近一次已提交的 run。注册进 `.mcp.json` 后 Claude Code / Codex 直连可调： + +| 工具 | 回答什么 | +|---|---| +| `get_gate_verdict` | 权威 run 的门禁结论、第一条失败规则、最小重跑命令 | +| `get_facts` | 按 artifact type / schema / 子串查已验证事实（热点、import 边、scorecard、符号） | +| `get_evidence` | 一条 finding 的证据链：哪些产物提到它、各自 sha256、记录时的 snapshot | +| `get_audit_status` | 各科室审计结论、评分、覆盖；没跑过 audit 会明说"不可用"而不是装绿 | +| `get_change_impact` | 改这些文件会波及谁、该先跑哪些测试（默认 stale-advisory，写码中途也能问） | +| `plan_structural_edit` | ast-grep 结构改写预览，只出匹配清单，不落盘 | + +**这个面只读，不裁决。** 门禁判定照旧只走 CLI 与 CI 路径——查询面被 prompt injection 说服也改不了结论。唯一会执行东西的工具是 `plan_structural_edit`，它在跑之前拿注册表核对自己的 capability 声明，一旦声明里出现 `repo_mutation` 就直接拒绝。 + +`--repo` 建议显式给：worktree 的目录名不是 `run commit` 发布时用的仓名，不给就会去查错仓的 run。`--repo-path` 默认取工作目录。 + +全量 `code-intel --mode normal` 留给深检和出证据，不是日常问答的入口。 + +### 结构门禁 + Agent 开始改代码前： ```powershell diff --git a/crates/code-intel-cli/src/change_impact.rs b/crates/code-intel-cli/src/change_impact.rs index 4594c8d5..85e8c502 100644 --- a/crates/code-intel-cli/src/change_impact.rs +++ b/crates/code-intel-cli/src/change_impact.rs @@ -55,6 +55,39 @@ pub(crate) struct ChangeImpactRequest { changed: Vec, } +impl ChangeImpactRequest { + /// Build a request from already-typed values instead of argv. + /// + /// `changed` goes through the same `normalize_relative` guard the + /// `--changed` flag uses, so a path arriving as a JSON string over the MCP + /// surface cannot escape the repository by a route the flag parser closes. + /// Reusing the guard is the point; re-stating it here would be the bug. + pub(crate) fn new( + artifact_root: PathBuf, + repo: String, + repo_path: PathBuf, + changed: Vec, + ) -> Result { + let mut changed = changed + .iter() + .map(|path| normalize_relative(path)) + .collect::, _>>()?; + changed.sort(); + changed.dedup(); + if changed.is_empty() { + return Err(ImpactError::Contract( + "at least one changed path is required".into(), + )); + } + Ok(Self { + artifact_root, + repo, + repo_path, + changed, + }) + } +} + impl ChangeImpactInvocation { pub(crate) fn parse(raw: &[String]) -> Result { if raw.first().map(String::as_str) != Some("impact") { diff --git a/crates/code-intel-cli/src/cli/command_catalog/mod.rs b/crates/code-intel-cli/src/cli/command_catalog/mod.rs index e619dc22..e8d1acdf 100644 --- a/crates/code-intel-cli/src/cli/command_catalog/mod.rs +++ b/crates/code-intel-cli/src/cli/command_catalog/mod.rs @@ -6,8 +6,8 @@ use serde_json::{json, Value}; use crate::{ admissibility, artifact_index, audit_report, change_agenda, change_impact, change_risk, compatibility_retirement_ticket, decision_port, decision_record, doctor_bootstrap, edit_apply, - edit_impact, evidence_query, model_channels, ponytail_gate, providers, repin, run_cli, - run_commit, session_evidence, snapshot, survival_scan, + edit_impact, evidence_query, mcp_serve, model_channels, ponytail_gate, providers, repin, + run_cli, run_commit, session_evidence, snapshot, survival_scan, }; use super::legacy::{ @@ -85,6 +85,7 @@ enum CompatibilityRoute { Decision, RunExecute, RunDagCoordinate, + Serve, Governance, } @@ -447,6 +448,7 @@ fn execute_compatibility(command: CompatibilityCommand) -> i32 { CompatibilityRoute::RunExecute | CompatibilityRoute::RunDagCoordinate => { run_cli::run_raw(raw) } + CompatibilityRoute::Serve => mcp_serve::run_raw(raw), CompatibilityRoute::Governance => ponytail_gate::run_raw(raw), } } diff --git a/crates/code-intel-cli/src/cli/command_catalog/routes/mod.rs b/crates/code-intel-cli/src/cli/command_catalog/routes/mod.rs index 04da470c..1b604ff6 100644 --- a/crates/code-intel-cli/src/cli/command_catalog/routes/mod.rs +++ b/crates/code-intel-cli/src/cli/command_catalog/routes/mod.rs @@ -7,6 +7,7 @@ use crate::cli::help_contract::{HELP_ALIASES, HELP_COMMAND}; mod edit_routes; mod run_routes; +mod serve_routes; mod types; pub(super) use types::{CommandRoute, LegacyRoute, RawRoute, VersionRoute}; @@ -540,6 +541,7 @@ pub(super) const COMMAND_ROUTES: &[CommandRoute] = &[ }, CommandRoute::Raw(run_routes::EXECUTE), CommandRoute::Raw(run_routes::DAG_COORDINATE), + CommandRoute::Raw(serve_routes::MCP), raw_route! { command: "governance", subcommand: None, diff --git a/crates/code-intel-cli/src/cli/command_catalog/routes/serve_routes.rs b/crates/code-intel-cli/src/cli/command_catalog/routes/serve_routes.rs new file mode 100644 index 00000000..377468e2 --- /dev/null +++ b/crates/code-intel-cli/src/cli/command_catalog/routes/serve_routes.rs @@ -0,0 +1,54 @@ +//! The agent-native query surface (#54, #58 proposal 3). +//! +//! Split out of `routes.rs` for the reason `edit_routes` and `run_routes` +//! were: the route table is the file every new command touches, and it already +//! sits just under this repository's own god-file threshold. The seam here is +//! the transport — this is the only route that speaks a protocol rather than +//! argv-in / stdout-out, and the only one whose process lifetime is a client +//! session rather than a single answer. + +/// `serve` takes no subcommand so the transport stays a flag: `--mcp` is the +/// only one today, and a future transport should be `serve --http`, not a +/// second route with a duplicated contract. The parser refuses when no +/// transport is named rather than defaulting to one, because "which protocol +/// is this process speaking on stdio" is not a question to guess at. +/// +/// `LocalWrite` and `ProcessSpawn` are declared for the single tool that +/// executes anything — `plan_structural_edit` stages an ast-grep preview into +/// a temporary directory. `RepoMutation` is absent and the handler refuses if +/// the registry ever declares it, so the effect set here is the enforced +/// boundary, not a description of intent. +pub(super) const MCP: super::RawRoute = super::RawRoute { + command: "serve", + subcommand: None, + argument_offset: 1, + id: super::CompatibilityRoute::Serve, + contract: super::CommandContract { + stability: super::CommandStability::Public, + controller: super::ControllerOwnership::AgentSession, + authority: super::CommandAuthority::Conditional( + super::AuthorityCondition::CommittedOrStaleAdvisory, + ), + effects: &[ + super::CommandEffect::RepoRead, + super::CommandEffect::LocalWrite, + super::CommandEffect::ProcessSpawn, + ], + output_contract: super::OutputContract::Stdout { + identities: &[ + "text-format:mcp-jsonrpc-stream.v1", + "code-intel-mcp-gate-verdict.v1", + "code-intel-mcp-evidence-chain.v1", + "code-intel-mcp-audit-status.v1", + "code-intel-mcp-structural-edit-plan.v1", + "code-intel-mcp-tool-error.v1", + "code-intel-evidence-query.v1", + "code-intel-change-impact.v1", + ], + }, + exit_contract: super::ExitContract::Exact(&[0, 64, 74]), + retirement_condition: + "retire only through a versioned agent query-surface replacement; the served payloads \ +are projections and may be retired individually with their source contracts", + }, +}; diff --git a/crates/code-intel-cli/src/cli/command_catalog/tests.rs b/crates/code-intel-cli/src/cli/command_catalog/tests.rs index d8940aad..61f521f4 100644 --- a/crates/code-intel-cli/src/cli/command_catalog/tests.rs +++ b/crates/code-intel-cli/src/cli/command_catalog/tests.rs @@ -208,7 +208,7 @@ fn unified_route_inventory_owns_version_primary_raw_and_legacy_dispatch() { .iter() .filter(|route| matches!(route, CommandRoute::Raw(_))) .count(), - 31 + 32 ); assert_eq!( COMMAND_ROUTES diff --git a/crates/code-intel-cli/src/cli/legacy.rs b/crates/code-intel-cli/src/cli/legacy.rs index 75432964..fbc40c36 100644 --- a/crates/code-intel-cli/src/cli/legacy.rs +++ b/crates/code-intel-cli/src/cli/legacy.rs @@ -1156,6 +1156,7 @@ Commands: run execute --repo --out --authority-root --final-name [--profile default|strict|offline] [--manifest ] [--max-concurrency ] [--session-evidence ] run dag-coordinate --repo --out [--manifest ] [--max-concurrency ] [--session-evidence ] run commit --source-root --authority-root --manifest-ref --final-name + serve --mcp [--repo-path ] [--repo ] [--artifact-root ] [--manifest ] (stdio MCP query surface over the committed run; read-only, gates nowhere) benchmark orientation --out [--repetitions <2..10>] benchmark tools --corpus --runs --artifact-root --out governance ponytail-gate --request diff --git a/crates/code-intel-cli/src/evidence_query.rs b/crates/code-intel-cli/src/evidence_query.rs index c3fb8de9..f5941ca0 100644 --- a/crates/code-intel-cli/src/evidence_query.rs +++ b/crates/code-intel-cli/src/evidence_query.rs @@ -43,6 +43,38 @@ pub(crate) struct EvidenceQueryRequest { } impl EvidenceQueryRequest { + /// Build a request from already-typed values instead of argv. + /// + /// The MCP query surface receives its filters as JSON, not as flags, but + /// must not therefore get a laxer request: the limit bound checked here is + /// the same `1..=MAX_LIMIT` the flag parser enforces, read from the same + /// constant. A second copy of that range is how the two surfaces would + /// drift. + pub(crate) fn new( + artifact_root: PathBuf, + repo: String, + repo_path: Option, + artifact_schema: Option, + artifact_type: Option, + contains: Option, + limit: usize, + ) -> Result { + if !(1..=MAX_LIMIT).contains(&limit) { + return Err(QueryError::Contract( + "limit must be an integer in 1..=100".into(), + )); + } + Ok(Self { + artifact_root, + repo, + repo_path, + artifact_schema, + artifact_type, + contains, + limit, + }) + } + pub(crate) fn parse(raw: &[String]) -> Result { if raw.first().map(String::as_str) != Some("query") { return Err(QueryError::Contract("usage: artifact query --artifact-root --repo [--repo-path ] [--artifact-schema ] [--type ] [--contains ] [--limit <1..100>]".into())); diff --git a/crates/code-intel-cli/src/main.rs b/crates/code-intel-cli/src/main.rs index c8689716..4514744c 100644 --- a/crates/code-intel-cli/src/main.rs +++ b/crates/code-intel-cli/src/main.rs @@ -37,6 +37,7 @@ mod hardened_git; mod hospital_score; mod impact_graph; mod language_pref; +mod mcp_serve; mod method_catalog; mod model_channels; mod orchestration; diff --git a/crates/code-intel-cli/src/mcp_serve/handlers.rs b/crates/code-intel-cli/src/mcp_serve/handlers.rs new file mode 100644 index 00000000..6f70e2c9 --- /dev/null +++ b/crates/code-intel-cli/src/mcp_serve/handlers.rs @@ -0,0 +1,476 @@ +//! The six served tools. +//! +//! Each one is a projection: it re-reads what `run commit` published, or it +//! re-runs a registered preview capability. None of them decides anything. The +//! request types come from the same modules the CLI parsers use +//! (`evidence_query`, `change_impact`), so a path arriving as a JSON string +//! over stdio meets the same traversal and inventory guards as one typed after +//! `--changed` — there is no second, looser parser on this side. + +use std::env; +use std::fs; +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; + +use serde_json::{json, Map, Value}; + +use super::ServeContext; +use crate::committed_evidence::{self, CommittedEvidence, EvidenceError}; +use crate::{ + capability, capability_inventory, change_impact, evidence_query, execution_policy, snapshot, +}; + +const EDIT_PLAN_CAPABILITY: &str = "edit.ast-grep-plan"; +const DEFAULT_LIMIT: usize = 20; + +pub(super) fn call(context: &ServeContext, name: &str, arguments: &Value) -> Result { + if !arguments.is_object() { + return Err("arguments must be a JSON object".into()); + } + match name { + "get_gate_verdict" => gate_verdict(context, arguments), + "get_facts" => facts(context, arguments), + "get_evidence" => evidence_chain(context, arguments), + "get_audit_status" => audit_status(context, arguments), + "get_change_impact" => blast_radius(context, arguments), + "plan_structural_edit" => structural_edit(context, arguments), + other => Err(format!("unknown tool: {other}")), + } +} + +fn gate_verdict(context: &ServeContext, arguments: &Value) -> Result { + expect_keys(arguments, &[])?; + let evidence = load(context)?; + let freshness = freshness(&evidence, context)?; + let mut result = json!({ + "schema": "code-intel-mcp-gate-verdict.v1", + "repo": context.repo, + "run": evidence.entry["run"], + "runIdentity": evidence.entry["runIdentity"], + "runOutcome": evidence.entry["outcome"], + "snapshotIdentity": evidence.snapshot_identity(), + "freshness": freshness, + "minimalRerunCommand": rerun_command(context), + // Said out loud on every verdict: reading a verdict here is not the + // same as the gate having run, and this surface cannot make it run. + "authority": {"status": "committed", "readOnly": true, "gatesHere": false}, + }); + match evidence.artifact("diagnosis.hospital") { + Some((artifact_ref, verified)) => { + let hospital: Value = serde_json::from_slice(verified.bytes()) + .map_err(|error| format!("committed hospital artifact is invalid JSON: {error}"))?; + let triage = &hospital["triage"]; + let failing = triage["failing_rules"] + .as_array() + .cloned() + .unwrap_or_default(); + result["verdict"] = json!({ + "status": triage["status"], + "domainVerdict": hospital["domainVerdict"], + "primaryDiagnosis": triage["primary_diagnosis"], + "disposition": triage["disposition"], + "risk": hospital["diagnosis"]["risk"], + }); + result["firstFailingRule"] = failing.first().cloned().unwrap_or(Value::Null); + result["failingRuleCount"] = json!(failing.len()); + result["evidenceRef"] = artifact_ref.clone(); + } + None => { + result["verdict"] = json!({ + "status": "unknown", + "reason": "the committed run publishes no diagnosis.hospital artifact", + }); + result["firstFailingRule"] = Value::Null; + result["failingRuleCount"] = Value::Null; + result["evidenceRef"] = Value::Null; + } + } + Ok(result) +} + +fn facts(context: &ServeContext, arguments: &Value) -> Result { + expect_keys(arguments, &["type", "artifactSchema", "contains", "limit"])?; + let request = evidence_query::EvidenceQueryRequest::new( + context.artifact_root.clone(), + context.repo.clone(), + Some(context.repo_path.clone()), + optional_text(arguments, "artifactSchema")?, + optional_text(arguments, "type")?, + optional_text(arguments, "contains")?, + optional_limit(arguments)?, + ) + .map_err(query_message)?; + let evidence = load(context)?; + evidence_query::execute(request, &evidence) + .map(|result| result.value().clone()) + .map_err(query_message) +} + +/// The provenance chain behind one identifier. +/// +/// "Which artifacts mention this" is a weaker claim than "this finding is +/// real", and the result says so: `status` reports whether committed evidence +/// backs the identifier at all, and an empty chain is reported as `unbacked` +/// rather than as an error. An agent asked to justify a finding needs to be +/// able to discover that nothing recorded supports it. +fn evidence_chain(context: &ServeContext, arguments: &Value) -> Result { + expect_keys(arguments, &["findingId", "limit"])?; + let finding = required_text(arguments, "findingId")?; + let limit = optional_limit(arguments)?; + let evidence = load(context)?; + let needle = finding.to_lowercase(); + let mut links = Vec::new(); + let mut scanned = 0usize; + let mut truncated = false; + for (artifact_ref, verified) in evidence.refs.iter().zip(evidence.verified.iter()) { + scanned += 1; + if !String::from_utf8_lossy(verified.bytes()) + .to_lowercase() + .contains(&needle) + { + continue; + } + if links.len() == limit { + truncated = true; + break; + } + links.push(json!({ + "artifactRef": artifact_ref, + "artifactType": artifact_ref["type"], + "artifactSchema": artifact_ref["artifactSchema"], + "sha256": artifact_ref["sha256"], + "consumedSnapshotIdentity": artifact_ref["consumedSnapshotIdentity"], + })); + } + Ok(json!({ + "schema": "code-intel-mcp-evidence-chain.v1", + "repo": context.repo, + "run": evidence.entry["run"], + "runIdentity": evidence.entry["runIdentity"], + "snapshotIdentity": evidence.snapshot_identity(), + "findingId": finding, + "status": if links.is_empty() { "unbacked" } else { "backed" }, + "links": links, + "linksTruncated": truncated, + "artifactsScanned": scanned, + "explanation": "Each link is an A07-committed artifact whose verified bytes mention the \ + requested identifier. A digest-verified mention is provenance, not proof that the finding is \ + correct; 'unbacked' means no committed artifact mentions it, not that it is false.", + })) +} + +fn audit_status(context: &ServeContext, arguments: &Value) -> Result { + expect_keys(arguments, &["department"])?; + let wanted = optional_text(arguments, "department")?; + let evidence = load(context)?; + let mut result = json!({ + "schema": "code-intel-mcp-audit-status.v1", + "repo": context.repo, + "run": evidence.entry["run"], + "snapshotIdentity": evidence.snapshot_identity(), + "department": wanted, + }); + let Some((artifact_ref, verified)) = evidence.artifact("diagnosis.audit") else { + // An absent audit is not a clean audit. Naming the reason keeps an + // agent from reading silence as a pass. + result["status"] = json!("unavailable"); + result["reason"] = json!( + "the committed run publishes no diagnosis.audit artifact; no audit department has run \ +against this snapshot" + ); + return Ok(result); + }; + let report: Value = serde_json::from_slice(verified.bytes()) + .map_err(|error| format!("committed audit artifact is invalid JSON: {error}"))?; + let matches = |row: &Value, key: &str| match &wanted { + Some(wanted) => row[key] == json!(wanted), + None => true, + }; + let filtered = |key: &str, id_key: &str| -> Vec { + report[key] + .as_array() + .into_iter() + .flatten() + .filter(|row| matches(row, id_key)) + .cloned() + .collect() + }; + let departments = filtered("departments", "id"); + if wanted.is_some() && departments.is_empty() { + result["status"] = json!("unknown_department"); + result["reason"] = json!("the committed audit report has no run for that department"); + result["knownDepartments"] = json!(report["departments"] + .as_array() + .into_iter() + .flatten() + .filter_map(|row| row["id"].as_str()) + .collect::>()); + return Ok(result); + } + result["status"] = json!("available"); + result["departments"] = json!(departments); + result["scores"] = json!(filtered("scores", "department")); + result["coverage"] = json!(filtered("coverage", "department")); + result["findings"] = json!(filtered("findings", "department")); + result["evidenceRef"] = artifact_ref.clone(); + Ok(result) +} + +/// The mid-edit question the CLI refuses by design. +/// +/// `change impact` fails closed when the committed snapshot is not current, +/// which is always true while an agent is typing — that refusal is what made +/// the pipeline invisible at write time (#58). Here the default is the +/// advisory answer, labelled with both snapshot identities, and `requireCurrent` +/// restores the strict behaviour for a caller that wants it. +fn blast_radius(context: &ServeContext, arguments: &Value) -> Result { + expect_keys(arguments, &["changed", "requireCurrent"])?; + let changed = required_string_array(arguments, "changed")?; + let require_current = optional_bool(arguments, "requireCurrent")?.unwrap_or(false); + // The request is built before the evidence is loaded so a rejected path + // costs an argument check rather than a full index rebuild — and so the + // traversal guard is reachable in a test that has no committed run. + let request = change_impact::ChangeImpactRequest::new( + context.artifact_root.clone(), + context.repo.clone(), + context.repo_path.clone(), + changed, + ) + .map_err(impact_message)?; + let evidence = load(context)?; + let result = if require_current { + change_impact::execute_committed(request, &evidence) + } else { + change_impact::execute_stale_advisory(request, &evidence) + } + .map_err(impact_message)?; + Ok(result.into_value()) +} + +fn structural_edit(context: &ServeContext, arguments: &Value) -> Result { + expect_keys(arguments, &["language", "pattern", "rewrite", "paths"])?; + let language = required_text(arguments, "language")?; + let pattern = required_text(arguments, "pattern")?; + let rewrite = optional_text(arguments, "rewrite")?; + let paths = match arguments.get("paths") { + None | Some(Value::Null) => vec![".".to_string()], + Some(_) => required_string_array(arguments, "paths")?, + }; + let declaration = + capability::declaration_for(EDIT_PLAN_CAPABILITY, context.manifest.as_deref()) + .map_err(|error| format!("capability registry: {error}"))?; + let policy = + execution_policy::ExecutionPolicy::for_profile(execution_policy::RunProfile::Default); + let allowed_effects = policy.allowed_effects(&declaration); + refuse_repository_mutation(&allowed_effects)?; + let snapshot = snapshot::build_for_dag(&context.repo_path, "explicit_overlay", &paths) + .map_err(|error| format!("snapshot identity for the requested paths: {error}"))?; + + // `rewrite` is inserted only when supplied: the adapter validates its + // options as a closed key set and rejects a null rewrite as an empty + // string, so an omitted argument must stay an omitted key. + let mut options = Map::new(); + options.insert("repoPath".into(), json!(context.repo_path)); + options.insert("language".into(), json!(language)); + options.insert("pattern".into(), json!(pattern)); + if let Some(rewrite) = &rewrite { + options.insert("rewrite".into(), json!(rewrite)); + } + options.insert("paths".into(), json!(paths)); + + let request = json!({ + "schema": "code-intel-capability-request.v1", + "capability": EDIT_PLAN_CAPABILITY, + "contractVersion": 1, + "implementation": declaration["implementation"], + "snapshot": snapshot["snapshot"], + "options": options, + "inputs": [], + "effectPolicy": {"allowedEffects": allowed_effects}, + }); + + let staging = staging_path()?; + let outcome = capability::exec_in_process( + EDIT_PLAN_CAPABILITY, + &request, + &staging, + context.manifest.as_deref(), + capability_inventory::execute, + ); + let plan = outcome + .result + .as_ref() + .and_then(|result| result["artifacts"][0]["path"].as_str()) + .and_then(|relative| fs::read(staging.join(relative)).ok()) + .and_then(|bytes| serde_json::from_slice::(&bytes).ok()); + let _ = fs::remove_dir_all(&staging); + match plan { + Some(plan) => Ok(json!({ + "schema": "code-intel-mcp-structural-edit-plan.v1", + "capability": EDIT_PLAN_CAPABILITY, + "repo": context.repo, + "snapshotIdentity": snapshot["snapshot"]["identity"], + "authority": {"mode": "preview_only", "repositoryMutation": false}, + "exitCode": outcome.exit_code, + "plan": plan, + })), + None => Err(format!( + "edit plan produced no artifact (exit {}): {}", + outcome.exit_code, + outcome + .diagnostic + .unwrap_or_else(|| "no diagnostic was emitted".into()) + )), + } +} + +/// The read-only boundary, enforced against the registry rather than asserted +/// in prose. +/// +/// `plan_structural_edit` is the one tool here that executes anything. If the +/// capability it fronts ever declares `repo_mutation`, that is a contract +/// change someone must review — not something this server should discover at +/// runtime and proceed through. Refusing here means no argument value, however +/// crafted, can reach a writer from the MCP surface. +pub(super) fn refuse_repository_mutation(allowed_effects: &Value) -> Result<(), String> { + let mutates = allowed_effects + .as_array() + .into_iter() + .flatten() + .any(|effect| effect == "repo_mutation"); + if mutates { + return Err(format!( + "refused: {EDIT_PLAN_CAPABILITY} declares repo_mutation; the MCP surface never \ +executes a repository writer" + )); + } + Ok(()) +} + +fn load(context: &ServeContext) -> Result { + committed_evidence::load(&context.artifact_root, &context.repo).map_err(|error| match error { + EvidenceError::Contract(message) | EvidenceError::HostIo(message) => message, + }) +} + +fn freshness(evidence: &CommittedEvidence, context: &ServeContext) -> Result { + evidence + .freshness(Some(&context.repo_path)) + .map_err(|error| match error { + EvidenceError::Contract(message) | EvidenceError::HostIo(message) => message, + }) +} + +/// The command is meant to be run, so it is spelled the way a shell accepts. +/// +/// `repo_path` is canonicalized, which on Windows yields the `\\?\` verbatim +/// prefix. Handing an agent a command it cannot paste is worse than handing it +/// none — it will invent one. +pub(super) fn rerun_command(context: &ServeContext) -> String { + let path = context.repo_path.display().to_string(); + let path = path.strip_prefix(r"\\?\").unwrap_or(&path); + if path.contains(' ') { + format!("code-intel \"{path}\" --mode normal") + } else { + format!("code-intel {path} --mode normal") + } +} + +fn staging_path() -> Result { + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|error| format!("resolve staging nonce: {error}"))? + .as_nanos(); + Ok(env::temp_dir().join(format!( + "code-intel-mcp-plan-{}-{nonce}", + std::process::id() + ))) +} + +/// A closed argument set, checked before anything is read. +/// +/// The tool schemas declare `additionalProperties: false`, but a schema is a +/// hint to a well-behaved client, not a guard. Rejecting unexpected keys here +/// means a caller cannot smuggle an argument that a future version of a +/// handler might start honouring. +fn expect_keys(arguments: &Value, allowed: &[&str]) -> Result<(), String> { + let object = arguments + .as_object() + .ok_or_else(|| "arguments must be a JSON object".to_string())?; + match object.keys().find(|key| !allowed.contains(&key.as_str())) { + Some(unexpected) => Err(format!( + "unexpected argument: {unexpected} (accepted: {})", + if allowed.is_empty() { + "none".to_string() + } else { + allowed.join(", ") + } + )), + None => Ok(()), + } +} + +fn required_text(arguments: &Value, key: &str) -> Result { + optional_text(arguments, key)?.ok_or_else(|| format!("{key} is required")) +} + +fn optional_text(arguments: &Value, key: &str) -> Result, String> { + match arguments.get(key) { + None | Some(Value::Null) => Ok(None), + Some(Value::String(value)) if !value.is_empty() => Ok(Some(value.clone())), + Some(Value::String(_)) => Err(format!("{key} must not be empty")), + Some(_) => Err(format!("{key} must be a string")), + } +} + +fn optional_bool(arguments: &Value, key: &str) -> Result, String> { + match arguments.get(key) { + None | Some(Value::Null) => Ok(None), + Some(Value::Bool(value)) => Ok(Some(*value)), + Some(_) => Err(format!("{key} must be a boolean")), + } +} + +fn optional_limit(arguments: &Value) -> Result { + match arguments.get("limit") { + None | Some(Value::Null) => Ok(DEFAULT_LIMIT), + Some(Value::Number(value)) => value + .as_u64() + .map(|value| value as usize) + .ok_or_else(|| "limit must be a positive integer".to_string()), + Some(_) => Err("limit must be an integer".into()), + } +} + +fn required_string_array(arguments: &Value, key: &str) -> Result, String> { + let items = arguments + .get(key) + .and_then(Value::as_array) + .ok_or_else(|| format!("{key} must be an array of strings"))?; + if items.is_empty() { + return Err(format!("{key} must contain at least one entry")); + } + items + .iter() + .map(|item| { + item.as_str() + .filter(|value| !value.is_empty()) + .map(str::to_string) + .ok_or_else(|| format!("{key} entries must be non-empty strings")) + }) + .collect() +} + +fn query_message(error: evidence_query::QueryError) -> String { + match error { + evidence_query::QueryError::Contract(message) + | evidence_query::QueryError::HostIo(message) => message, + } +} + +fn impact_message(error: change_impact::ImpactError) -> String { + match error { + change_impact::ImpactError::Contract(message) + | change_impact::ImpactError::HostIo(message) => message, + } +} diff --git a/crates/code-intel-cli/src/mcp_serve/mod.rs b/crates/code-intel-cli/src/mcp_serve/mod.rs new file mode 100644 index 00000000..c9b2d918 --- /dev/null +++ b/crates/code-intel-cli/src/mcp_serve/mod.rs @@ -0,0 +1,316 @@ +//! `code-intel serve --mcp` — the agent-native query surface (#54, and the +//! third proposal of the write-path audit in #58). +//! +//! Every other agent-facing surface in this repository answers *after* a full +//! authoritative run and *through* artifact files. That shape is correct for +//! auditing and useless while an agent is writing code: it cannot ask one +//! question and get one answer. This module is that missing plane — a stdio +//! MCP server projecting the already-committed evidence, plus the two +//! write-assist projections #58 named. +//! +//! The boundary is structural, not a convention: this server owns no +//! authority. It re-reads what `run commit` published, re-verifies the digests +//! through `committed_evidence`, and re-uses the same request types the CLI +//! parsers build — so a query that arrives over stdio traverses exactly the +//! guards a query typed at a shell does. The single capability it can execute +//! (`edit.ast-grep-plan`) is checked against its registry declaration before +//! it runs and refused if that declaration ever admits `repo_mutation`. Gate +//! verdicts stay where they were: in the CLI and CI paths. A prompt-injected +//! query string reaching this surface can therefore read, and cannot decide. + +use std::env; +use std::fs; +use std::io::{self, BufRead, Write}; +use std::path::PathBuf; + +use serde_json::{json, Value}; + +use crate::artifacts; + +mod handlers; +mod tools; + +#[cfg(test)] +mod tests; + +/// The newest MCP revision this server implements. Clients that ask for an +/// older supported revision get theirs echoed back; anything else is answered +/// with this one, which is what the specification prescribes for a version the +/// server does not speak. +const PROTOCOL_VERSION: &str = "2025-06-18"; +const SUPPORTED_PROTOCOL_VERSIONS: &[&str] = &["2025-06-18", "2025-03-26", "2024-11-05"]; + +const USAGE: &str = "usage: serve --mcp [--repo-path ] [--repo ] [--artifact-root ] [--manifest ]"; + +const INSTRUCTIONS: &str = + "Read-only projection of this repository's committed Code Intel evidence. \ +Ask get_gate_verdict before trusting a green tree, get_change_impact before editing files, and \ +plan_structural_edit before a mechanical multi-file rewrite. Every answer names the run and \ +snapshot identity it came from; treat a stale-advisory freshness as advice, never as a gate."; + +/// Where a served answer came from and which checkout it is being compared +/// against. Resolved once at startup so no per-call argument can redirect the +/// server at another repository — the client chooses tools, never targets. +#[derive(Debug)] +pub(super) struct ServeContext { + pub(super) repo_path: PathBuf, + pub(super) repo: String, + pub(super) artifact_root: PathBuf, + pub(super) manifest: Option, +} + +pub(crate) fn run_raw(raw: &[String]) -> i32 { + let context = match parse(raw) { + Ok(context) => context, + Err(message) => { + eprintln!("{message}"); + return 64; + } + }; + let stdin = io::stdin(); + let stdout = io::stdout(); + match serve(&context, &mut stdin.lock(), &mut stdout.lock()) { + Ok(()) => 0, + Err(message) => { + eprintln!("{message}"); + 74 + } + } +} + +fn parse(raw: &[String]) -> Result { + let mut transport = false; + let mut repo_path: Option = None; + let mut repo: Option = None; + let mut artifact_root: Option = None; + let mut manifest: Option = None; + let mut index = 0; + while index < raw.len() { + let flag = raw[index].as_str(); + if flag == "--mcp" { + if transport { + return Err("duplicate --mcp".into()); + } + transport = true; + index += 1; + continue; + } + if !matches!( + flag, + "--repo-path" | "--repo" | "--artifact-root" | "--manifest" + ) { + return Err(format!("unknown serve argument: {flag}\n{USAGE}")); + } + let value = raw + .get(index + 1) + .filter(|value| !value.is_empty() && !value.starts_with("--")) + .ok_or_else(|| format!("{flag} requires one value"))?; + match flag { + "--repo-path" => set_once(&mut repo_path, PathBuf::from(value), flag)?, + "--repo" => set_once(&mut repo, value.clone(), flag)?, + "--artifact-root" => set_once(&mut artifact_root, PathBuf::from(value), flag)?, + "--manifest" => set_once(&mut manifest, PathBuf::from(value), flag)?, + _ => unreachable!("serve flags are matched above"), + } + index += 2; + } + if !transport { + return Err(format!("serve requires a transport\n{USAGE}")); + } + let repo_path = match repo_path { + Some(path) => path, + None => { + env::current_dir().map_err(|error| format!("resolve working directory: {error}"))? + } + }; + if !repo_path.is_dir() { + return Err(format!( + "--repo-path is not a directory: {}", + repo_path.display() + )); + } + let repo_path = fs::canonicalize(&repo_path) + .map_err(|error| format!("resolve --repo-path {}: {error}", repo_path.display()))?; + // The artifact-index key defaults to the checkout's directory name because + // that is the name `run commit` publishes under. A worktree whose folder + // name differs from the published repository name must say so with + // `--repo`; guessing would silently answer from another repository's runs. + let repo = match repo { + Some(repo) => repo, + None => repo_path + .file_name() + .and_then(|name| name.to_str()) + .filter(|name| !name.is_empty()) + .ok_or("--repo-path has no usable directory name; pass --repo")? + .to_string(), + }; + let artifact_root = match artifact_root { + Some(root) => root, + None => artifacts::resolve_artifact_root(None) + .map_err(|error| format!("resolve artifact root: {error}"))?, + }; + Ok(ServeContext { + repo_path, + repo, + artifact_root, + manifest, + }) +} + +fn set_once(slot: &mut Option, value: T, flag: &str) -> Result<(), String> { + if slot.replace(value).is_some() { + Err(format!("duplicate {flag}")) + } else { + Ok(()) + } +} + +/// The stdio transport: newline-delimited JSON-RPC, one message per line. +/// +/// A malformed line is answered and the session continues. Only a broken pipe +/// or an unreadable stdin ends the loop with a host-IO exit, because a client +/// that disconnects mid-session is the normal way this process dies. +fn serve( + context: &ServeContext, + input: &mut impl BufRead, + output: &mut impl Write, +) -> Result<(), String> { + let mut line = String::new(); + loop { + line.clear(); + let read = input + .read_line(&mut line) + .map_err(|error| format!("read MCP stdin: {error}"))?; + if read == 0 { + return Ok(()); + } + let trimmed = line.trim(); + if trimmed.is_empty() { + continue; + } + let Some(response) = handle_line(context, trimmed) else { + continue; + }; + writeln!( + output, + "{}", + serde_json::to_string(&response).expect("MCP response serializes") + ) + .map_err(|error| format!("write MCP stdout: {error}"))?; + output + .flush() + .map_err(|error| format!("flush MCP stdout: {error}"))?; + } +} + +/// One request in, at most one response out. +/// +/// `None` means the message was a notification. JSON-RPC forbids answering +/// those, and a client that receives an unsolicited response for +/// `notifications/initialized` treats the session as broken. +pub(super) fn handle_line(context: &ServeContext, line: &str) -> Option { + let message: Value = match serde_json::from_str(line) { + Ok(message) => message, + Err(error) => { + return Some(failure( + Value::Null, + -32700, + &format!("parse error: {error}"), + )) + } + }; + let id = message.get("id").filter(|id| !id.is_null()).cloned()?; + let Some(method) = message["method"].as_str() else { + return Some(failure(id, -32600, "invalid request: no method")); + }; + Some(dispatch(context, id, method, &message["params"])) +} + +fn dispatch(context: &ServeContext, id: Value, method: &str, params: &Value) -> Value { + match method { + "initialize" => success(id, initialize_result(params)), + "ping" => success(id, json!({})), + "tools/list" => success(id, json!({ "tools": tools::descriptors() })), + "tools/call" => tools_call(context, id, params), + // Declared empty rather than unimplemented: a client that probes these + // during handshake should see "this server has none", not an error it + // may surface to the user as a failed connection. + "resources/list" => success(id, json!({ "resources": [] })), + "resources/templates/list" => success(id, json!({ "resourceTemplates": [] })), + "prompts/list" => success(id, json!({ "prompts": [] })), + other => failure(id, -32601, &format!("method not found: {other}")), + } +} + +fn initialize_result(params: &Value) -> Value { + let requested = params["protocolVersion"] + .as_str() + .unwrap_or(PROTOCOL_VERSION); + let negotiated = if SUPPORTED_PROTOCOL_VERSIONS.contains(&requested) { + requested + } else { + PROTOCOL_VERSION + }; + json!({ + "protocolVersion": negotiated, + "capabilities": {"tools": {"listChanged": false}}, + "serverInfo": {"name": "code-intel", "version": env!("CARGO_PKG_VERSION")}, + "instructions": INSTRUCTIONS, + }) +} + +/// A tool that refuses is a *result* with `isError`, not a JSON-RPC error. +/// +/// The distinction is load-bearing for an agent: a JSON-RPC error is a +/// transport fault it should retry or report, while `isError` is an answer it +/// should read — "no committed run exists yet", "that path escapes the +/// repository". Collapsing the two would train agents to treat a refusal as a +/// broken server. Only an unknown tool name is a protocol error. +fn tools_call(context: &ServeContext, id: Value, params: &Value) -> Value { + let Some(name) = params["name"].as_str() else { + return failure( + id, + -32602, + "invalid params: tools/call requires a tool name", + ); + }; + if !tools::is_registered(name) { + return failure(id, -32602, &format!("unknown tool: {name}")); + } + let arguments = params + .get("arguments") + .cloned() + .unwrap_or_else(|| json!({})); + match handlers::call(context, name, &arguments) { + Ok(payload) => success(id, tool_result(&payload, false)), + Err(message) => success( + id, + tool_result( + &json!({ + "schema": "code-intel-mcp-tool-error.v1", + "tool": name, + "error": message, + }), + true, + ), + ), + } +} + +fn tool_result(payload: &Value, is_error: bool) -> Value { + json!({ + "content": [{ + "type": "text", + "text": serde_json::to_string(payload).expect("tool payload serializes"), + }], + "isError": is_error, + }) +} + +fn success(id: Value, result: Value) -> Value { + json!({"jsonrpc": "2.0", "id": id, "result": result}) +} + +fn failure(id: Value, code: i64, message: &str) -> Value { + json!({"jsonrpc": "2.0", "id": id, "error": {"code": code, "message": message}}) +} diff --git a/crates/code-intel-cli/src/mcp_serve/tests.rs b/crates/code-intel-cli/src/mcp_serve/tests.rs new file mode 100644 index 00000000..dea114ad --- /dev/null +++ b/crates/code-intel-cli/src/mcp_serve/tests.rs @@ -0,0 +1,350 @@ +use std::env; +use std::fs; +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; + +use serde_json::{json, Value}; + +use super::{handlers, tools, ServeContext}; + +/// A context pointing at a real directory with no committed run. +/// +/// Most guard tests want exactly this: argument validation must refuse before +/// the evidence loader is ever consulted, so a context with nothing published +/// is the honest fixture. Tests that need the loader to be the thing that +/// fails read its message instead. +struct Fixture(PathBuf); + +impl Fixture { + fn create(label: &str) -> Self { + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock") + .as_nanos(); + let path = env::temp_dir().join(format!( + "code-intel-mcp-{label}-{}-{nonce}", + std::process::id() + )); + fs::create_dir_all(&path).expect("create MCP fixture directory"); + Self(path) + } + + fn context(&self) -> ServeContext { + ServeContext { + repo_path: self.0.clone(), + repo: "fixture-repo".into(), + artifact_root: self.0.join("artifacts"), + manifest: None, + } + } +} + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +fn request(id: i64, method: &str, params: Value) -> String { + json!({"jsonrpc": "2.0", "id": id, "method": method, "params": params}).to_string() +} + +fn call(context: &ServeContext, tool: &str, arguments: Value) -> Value { + super::handle_line( + context, + &request( + 1, + "tools/call", + json!({"name": tool, "arguments": arguments}), + ), + ) + .expect("tools/call is a request, not a notification") +} + +fn tool_payload(response: &Value) -> Value { + let text = response["result"]["content"][0]["text"] + .as_str() + .expect("tool result carries one text block"); + serde_json::from_str(text).expect("tool payload is JSON") +} + +#[test] +fn every_registered_tool_has_a_handler() { + let fixture = Fixture::create("registry"); + let context = fixture.context(); + for name in tools::NAMES { + let error = handlers::call(&context, name, &json!({})) + .err() + .unwrap_or_default(); + assert!( + !error.starts_with("unknown tool"), + "{name} is advertised but has no handler" + ); + } + assert_eq!(tools::descriptors().len(), tools::NAMES.len()); +} + +#[test] +fn every_descriptor_declares_a_closed_read_only_schema() { + for descriptor in tools::descriptors() { + let name = descriptor["name"].as_str().expect("descriptor name"); + assert!( + tools::is_registered(name), + "{name} is described but not registered" + ); + assert_eq!( + descriptor["inputSchema"]["additionalProperties"], + json!(false), + "{name} accepts unexpected arguments" + ); + assert_eq!( + descriptor["annotations"]["readOnlyHint"], + json!(true), + "{name} is not annotated read-only" + ); + assert!( + descriptor["description"] + .as_str() + .is_some_and(|text| text.len() > 80), + "{name} has no usable description" + ); + } +} + +#[test] +fn notifications_are_never_answered() { + let fixture = Fixture::create("notify"); + let context = fixture.context(); + for line in [ + json!({"jsonrpc": "2.0", "method": "notifications/initialized"}).to_string(), + json!({"jsonrpc": "2.0", "method": "notifications/cancelled", "id": Value::Null}) + .to_string(), + ] { + assert!( + super::handle_line(&context, &line).is_none(), + "a notification must not produce a response: {line}" + ); + } +} + +#[test] +fn initialize_echoes_a_supported_revision_and_substitutes_an_unknown_one() { + let fixture = Fixture::create("initialize"); + let context = fixture.context(); + let older = super::handle_line( + &context, + &request(1, "initialize", json!({"protocolVersion": "2024-11-05"})), + ) + .expect("initialize response"); + assert_eq!(older["result"]["protocolVersion"], json!("2024-11-05")); + + let unknown = super::handle_line( + &context, + &request(2, "initialize", json!({"protocolVersion": "1999-01-01"})), + ) + .expect("initialize response"); + assert_eq!( + unknown["result"]["protocolVersion"], + json!(super::PROTOCOL_VERSION) + ); + assert_eq!(unknown["result"]["serverInfo"]["name"], json!("code-intel")); + assert_eq!( + unknown["result"]["capabilities"]["tools"]["listChanged"], + json!(false) + ); +} + +#[test] +fn malformed_and_unroutable_messages_answer_as_protocol_errors() { + let fixture = Fixture::create("protocol"); + let context = fixture.context(); + + let parse_error = super::handle_line(&context, "{not json").expect("parse error response"); + assert_eq!(parse_error["error"]["code"], json!(-32700)); + assert_eq!(parse_error["id"], Value::Null); + + let no_method = super::handle_line(&context, &json!({"jsonrpc": "2.0", "id": 7}).to_string()) + .expect("invalid request response"); + assert_eq!(no_method["error"]["code"], json!(-32600)); + + let unknown_method = + super::handle_line(&context, &request(8, "tools/summon", json!({}))).expect("response"); + assert_eq!(unknown_method["error"]["code"], json!(-32601)); + + let unknown_tool = super::handle_line( + &context, + &request(9, "tools/call", json!({"name": "rm_rf", "arguments": {}})), + ) + .expect("response"); + assert_eq!(unknown_tool["error"]["code"], json!(-32602)); +} + +#[test] +fn tools_list_serves_the_whole_registry() { + let fixture = Fixture::create("list"); + let context = fixture.context(); + let response = + super::handle_line(&context, &request(1, "tools/list", json!({}))).expect("response"); + let served = response["result"]["tools"] + .as_array() + .expect("tools array") + .iter() + .filter_map(|tool| tool["name"].as_str()) + .collect::>(); + assert_eq!(served, tools::NAMES.to_vec()); +} + +/// A refusal is an answer, not a transport fault. +/// +/// An agent that sees a JSON-RPC error learns "this server is broken"; one +/// that sees `isError` with a readable payload learns "run the pipeline +/// first". Getting this backwards is how a working tool acquires a reputation +/// for being flaky. +#[test] +fn a_tool_that_cannot_answer_returns_an_error_result_not_a_transport_error() { + let fixture = Fixture::create("norun"); + let context = fixture.context(); + let response = call(&context, "get_gate_verdict", json!({})); + assert!( + response.get("error").is_none(), + "a missing run is not a protocol error: {response}" + ); + assert_eq!(response["result"]["isError"], json!(true)); + let payload = tool_payload(&response); + assert_eq!(payload["schema"], json!("code-intel-mcp-tool-error.v1")); + assert_eq!(payload["tool"], json!("get_gate_verdict")); + assert!( + payload["error"] + .as_str() + .is_some_and(|text| !text.is_empty()), + "a refusal must say why: {payload}" + ); +} + +/// Injection coverage: a crafted argument must not reach a path the flag +/// parsers close, and must not reach a handler at all when it is not a +/// declared argument. +#[test] +fn crafted_arguments_are_refused_before_any_evidence_is_read() { + let fixture = Fixture::create("injection"); + let context = fixture.context(); + + for escape in [ + "../../../etc/passwd", + "..\\..\\windows\\system32\\config\\sam", + "/etc/shadow", + "C:/Windows/System32/drivers/etc/hosts", + "src/../../outside.rs", + ] { + let response = call(&context, "get_change_impact", json!({"changed": [escape]})); + assert_eq!( + response["result"]["isError"], + json!(true), + "{escape} was not refused" + ); + let message = tool_payload(&response)["error"].to_string(); + assert!( + message.contains("portable repository-relative path"), + "{escape} was refused for the wrong reason: {message}" + ); + } + + let smuggled = call( + &context, + "get_facts", + json!({"type": "code_evidence.files", "repoPath": "/tmp/elsewhere"}), + ); + assert_eq!(smuggled["result"]["isError"], json!(true)); + assert!(tool_payload(&smuggled)["error"] + .as_str() + .expect("error text") + .contains("unexpected argument: repoPath")); + + let non_object = handlers::call(&context, "get_facts", &json!("--artifact-root /etc")); + assert_eq!( + non_object.err().as_deref(), + Some("arguments must be a JSON object") + ); +} + +#[test] +fn a_capability_that_declared_repository_mutation_would_be_refused() { + assert!(handlers::refuse_repository_mutation(&json!(["repo_read", "process_spawn"])).is_ok()); + let refused = + handlers::refuse_repository_mutation(&json!(["repo_read", "local_write", "repo_mutation"])); + assert!( + refused + .as_ref() + .err() + .is_some_and(|message| message.contains("never executes a repository writer")), + "a mutating declaration must be refused: {refused:?}" + ); +} + +/// The rerun command is advice an agent will act on, so it has to be +/// runnable: a canonicalized Windows path carries the `\\?\` verbatim prefix +/// that no shell accepts, and a path with a space needs quoting. +#[test] +fn the_rerun_command_is_shell_runnable() { + let verbatim = ServeContext { + repo_path: PathBuf::from(r"\\?\C:\repo\project"), + repo: "project".into(), + artifact_root: PathBuf::from(r"C:\artifacts"), + manifest: None, + }; + assert_eq!( + handlers::rerun_command(&verbatim), + r"code-intel C:\repo\project --mode normal" + ); + + let spaced = ServeContext { + repo_path: PathBuf::from(r"\\?\C:\my repo\project"), + repo: "project".into(), + artifact_root: PathBuf::from(r"C:\artifacts"), + manifest: None, + }; + assert_eq!( + handlers::rerun_command(&spaced), + "code-intel \"C:\\my repo\\project\" --mode normal" + ); +} + +#[test] +fn serve_requires_a_transport_and_rejects_unknown_flags() { + let argv = |args: &[&str]| args.iter().map(|arg| arg.to_string()).collect::>(); + + let no_transport = super::parse(&argv(&[])).expect_err("a transport is required"); + assert!(no_transport.contains("serve requires a transport")); + + let unknown = super::parse(&argv(&["--mcp", "--exec"])).expect_err("unknown flag"); + assert!(unknown.contains("unknown serve argument: --exec")); + + let duplicate = super::parse(&argv(&["--mcp", "--mcp"])).expect_err("duplicate transport"); + assert_eq!(duplicate, "duplicate --mcp"); + + let missing_value = + super::parse(&argv(&["--mcp", "--repo"])).expect_err("flag without a value"); + assert!(missing_value.contains("--repo requires one value")); +} + +#[test] +fn serve_defaults_the_repository_name_to_the_published_directory_name() { + let fixture = Fixture::create("defaults"); + let argv = [ + "--mcp".to_string(), + "--repo-path".to_string(), + fixture.0.display().to_string(), + "--artifact-root".to_string(), + fixture.0.join("artifacts").display().to_string(), + ]; + let context = super::parse(&argv).expect("serve parses"); + assert_eq!( + context.repo, + fixture + .0 + .file_name() + .and_then(|name| name.to_str()) + .expect("fixture directory name") + ); + assert!(context.manifest.is_none()); +} diff --git a/crates/code-intel-cli/src/mcp_serve/tools.rs b/crates/code-intel-cli/src/mcp_serve/tools.rs new file mode 100644 index 00000000..72dbd450 --- /dev/null +++ b/crates/code-intel-cli/src/mcp_serve/tools.rs @@ -0,0 +1,175 @@ +//! The served tool registry. +//! +//! Kept apart from `handlers` so the wire-facing contract (names, argument +//! schemas, the prose an agent reads when deciding what to call) can be +//! reviewed as one surface. `handlers::call` and `NAMES` are held together by +//! `every_registered_tool_has_a_handler` in `tests`, so a descriptor can never +//! advertise a tool that answers "unknown tool" at call time. + +use serde_json::{json, Value}; + +/// Every served tool, in the order an agent should reach for them: the four +/// read projections #54 specified, then the two write-assist projections #58 +/// added. `plan_structural_edit` is last deliberately — it is the only one +/// that spawns a child process, and an agent scanning this list top-down +/// should find the cheap answers first. +pub(super) const NAMES: &[&str] = &[ + "get_gate_verdict", + "get_facts", + "get_evidence", + "get_audit_status", + "get_change_impact", + "plan_structural_edit", +]; + +pub(super) fn is_registered(name: &str) -> bool { + NAMES.contains(&name) +} + +pub(super) fn descriptors() -> Vec { + vec![ + gate_verdict(), + facts(), + evidence(), + audit_status(), + change_impact(), + structural_edit(), + ] +} + +fn gate_verdict() -> Value { + json!({ + "name": "get_gate_verdict", + "title": "Gate verdict", + "description": "The committed run's gate conclusion for this repository: triage status, \ + the first failing rule, and the minimal command that reruns it. Call this before trusting that \ + the tree is green — a passing local build says nothing about the authoritative verdict. Answers \ + from the last committed run and reports whether that run is still current for this checkout.", + "inputSchema": { + "type": "object", + "properties": {}, + "additionalProperties": false + }, + "annotations": {"readOnlyHint": true, "destructiveHint": false, "openWorldHint": false}, + }) +} + +fn facts() -> Value { + json!({ + "name": "get_facts", + "title": "Query committed facts", + "description": "Search the committed run's verified artifacts — hotspots, import edges, \ + scorecards, coverage, symbols, doctor observations. Filter by artifact type, artifact schema, or \ + substring. Every hit carries the digest-verified artifact reference it came from. Use this \ + instead of reading artifact files off disk: the bytes here have already been checked against the \ + snapshot they were recorded under.", + "inputSchema": { + "type": "object", + "properties": { + "type": {"type": "string", "description": "Artifact type, e.g. code_evidence.files, diagnosis.hospital, code_evidence.scorecard."}, + "artifactSchema": {"type": "string", "description": "Artifact schema id, e.g. code-intel-hospital.v1."}, + "contains": {"type": "string", "description": "Case-insensitive substring the artifact bytes must contain."}, + "limit": {"type": "integer", "minimum": 1, "maximum": 100, "description": "Maximum matches to return (default 20)."} + }, + "additionalProperties": false + }, + "annotations": {"readOnlyHint": true, "destructiveHint": false, "openWorldHint": false}, + }) +} + +fn evidence() -> Value { + json!({ + "name": "get_evidence", + "title": "Evidence chain for a finding", + "description": "The provenance chain behind one finding, rule id, or file path: which \ + committed artifacts mention it, each one's sha256, the snapshot identity it was recorded under, \ + and the run that published it. Call this when a verdict or a finding needs to be justified to a \ + human, or when you need to know whether a claim is backed by recorded evidence at all.", + "inputSchema": { + "type": "object", + "properties": { + "findingId": {"type": "string", "description": "Finding id, rule id, symbol, or repository-relative path to trace."}, + "limit": {"type": "integer", "minimum": 1, "maximum": 100, "description": "Maximum evidence links to return (default 20)."} + }, + "required": ["findingId"], + "additionalProperties": false + }, + "annotations": {"readOnlyHint": true, "destructiveHint": false, "openWorldHint": false}, + }) +} + +fn audit_status() -> Value { + json!({ + "name": "get_audit_status", + "title": "Audit department status", + "description": "The latest audit report's per-department conclusions, scores, and \ + coverage. Optionally narrowed to one department. Reports explicitly when the committed run \ + carries no audit artifact, rather than implying a clean audit that never ran.", + "inputSchema": { + "type": "object", + "properties": { + "department": {"type": "string", "description": "Department id to narrow to; omit for every department."} + }, + "additionalProperties": false + }, + "annotations": {"readOnlyHint": true, "destructiveHint": false, "openWorldHint": false}, + }) +} + +fn change_impact() -> Value { + json!({ + "name": "get_change_impact", + "title": "Blast radius and test selection", + "description": "Given the files you are about to change (or just changed), the files \ + reachable from them through the committed reverse import graph, plus candidate tests to run. \ + This is the mid-edit question the CLI refuses to answer, because the snapshot is never current \ + while you are typing: here the answer is served as stale-advisory, labelled with the recorded \ + and current snapshot identities. Advisory only — never gate on it.", + "inputSchema": { + "type": "object", + "properties": { + "changed": { + "type": "array", + "items": {"type": "string"}, + "minItems": 1, + "description": "Repository-relative paths you are changing, e.g. src/auth/token.rs." + }, + "requireCurrent": { + "type": "boolean", + "description": "Refuse unless the committed snapshot still matches this checkout (default false)." + } + }, + "required": ["changed"], + "additionalProperties": false + }, + "annotations": {"readOnlyHint": true, "destructiveHint": false, "openWorldHint": false}, + }) +} + +fn structural_edit() -> Value { + json!({ + "name": "plan_structural_edit", + "title": "Preview a structural rewrite", + "description": "Run an ast-grep pattern across the checkout and return every match, with \ + the rewritten text when a rewrite is supplied. Preview only: nothing is written to the \ + repository. Call this before a mechanical multi-file rewrite so you edit from a match list \ + rather than from guesses, then apply the changes yourself.", + "inputSchema": { + "type": "object", + "properties": { + "language": {"type": "string", "description": "ast-grep language id, e.g. rust, ts, python."}, + "pattern": {"type": "string", "description": "ast-grep pattern to match."}, + "rewrite": {"type": "string", "description": "Optional ast-grep rewrite template; matches are previewed, never written."}, + "paths": { + "type": "array", + "items": {"type": "string"}, + "maxItems": 64, + "description": "Repository-relative paths to search (default the whole checkout)." + } + }, + "required": ["language", "pattern"], + "additionalProperties": false + }, + "annotations": {"readOnlyHint": true, "destructiveHint": false, "openWorldHint": false}, + }) +} diff --git a/crates/code-intel-cli/tests/fixtures/cli-head-parity.v2.json b/crates/code-intel-cli/tests/fixtures/cli-head-parity.v2.json index c1327eba..81df5abf 100644 --- a/crates/code-intel-cli/tests/fixtures/cli-head-parity.v2.json +++ b/crates/code-intel-cli/tests/fixtures/cli-head-parity.v2.json @@ -585,7 +585,7 @@ "--help", "--all" ], - "reason": "Phase 2 acceptance requires full help to expose every registered compatibility alias. The v1 bytes omitted aliases, so byte parity and alias completeness conflict; v2 resolves that conflict as one explicit versioned behavior change.", + "reason": "Phase 2 acceptance requires full help to expose every registered compatibility alias. The v1 bytes omitted aliases, so byte parity and alias completeness conflict; v2 resolves that conflict as one explicit versioned behavior change. Registering a route adds its line to these bytes (`serve --mcp`, #54): that is the same versioned behavior -- full help documents every registered route -- so the contract id pair stays v1 -> v2 rather than climbing a version per command.", "oldContractId": "text-format:help-full.v1", "newContractId": "text-format:help-full.v2", "old": { @@ -595,7 +595,7 @@ }, "new": { "exitCode": 0, - "stdoutUtf8": "code-intel [options]\n\nCommands:\n --version|-V [--json]\n help|--help|-h [--all]\n resume --repo [--artifact-root ] [--json]\n classify --report [--json]\n sentrux-normalize --steps [--out ]\n sentrux-debt-register --failures [--repo ] [--out ]\n doctor [--artifact-root ] [--json]\n doctor bootstrap [--repo ] [--repo-path ] [--config ] [--platform auto|windows|macos|linux] [--no-require-repowise] [--require-understand] [--json]\n graph|understand --repo [--language zh] [--full] [--write] [--json]\n provider|providers [--action List|Plan|Validate|Invoke] [--provider repowise|understand] [--operation ] [--repo ] [--language zh] [--write] [--json]\n provider repowise-adapt --request --artifact-root --evaluated-at --max-age-seconds \n provider graph-adapt --request --artifact-root --evaluated-at --max-age-seconds \n provider sentrux-adapt --request --artifact-root --evaluated-at --max-age-seconds \n provider session-adapt --repo --trace [--hotspots ] [--out ] [--working-tree-policy head_only|explicit_overlay]\n provider codenexus-adapt --request --artifact-root --evaluated-at --max-age-seconds \n provider file-boundary --request --out \n provider runtime-ci-evidence --artifact-root --request --out \n compatibility retirement-ticket lint --ticket --evaluated-at \n route|routes [--action List|Plan|Validate] [--provider repowise|understand] [--operation ] [--repo ] [--json]\n sentrux [--no-ratchet]\n (--no-ratchet: `check` only, skip the .sentrux/baseline.json ratchet)\n capability exec --request --out [--artifact-root ] [--manifest ]\n model inventory-validate --request [--out ]\n model route --request [--out ]\n snapshot identity --repo --working-tree-policy [--scope ]...\n repin [--repo ] [--write] [--json] [--exclude ]...\n evidence validate --request --artifact-root \n repository survival-scan --request --artifact-root \n audit --operation validate|render --repo --report [--format markdown|html]\n audit --operation scope --repo --since \n artifact index --artifact-root [--output ] [--operation rebuild|incremental] [--existing ]\n artifact query --artifact-root --repo [--repo-path ] [--artifact-schema ] [--type ] [--contains ] [--limit <1..100>]\n change impact --artifact-root --repo --repo-path --changed [--changed ]... [--staleness current|advisory]\n change risk [--repo ] [--sample ] [--format json|text] (git-only defect-risk score, no prior run, no index)\n change agenda [--repo ] [--min-cochange ] [--format json|text] (git-only review units clustered by co-change, ranked worst first)\n edit impact --repo-path --changed [--changed ]... [--scope ]... (working tree, no prior run, authority: none)\n edit apply --repo-path --file (--span --expect-sha256 --replacement |--replacement-file )... [--out ] [--manifest ] [--envelope] (span-addressed patch; refuses with evidence on digest drift, exit 10)\n decision request-response --request [--response |--cancel ] --now --branch ...\n decision record --resolution --store \n decision replay --query --store \n run execute --repo --out --authority-root --final-name [--profile default|strict|offline] [--manifest ] [--max-concurrency ] [--session-evidence ]\n run dag-coordinate --repo --out [--manifest ] [--max-concurrency ] [--session-evidence ]\n run commit --source-root --authority-root --manifest-ref --final-name \n benchmark orientation --out [--repetitions <2..10>]\n benchmark tools --corpus --runs --artifact-root --out \n governance ponytail-gate --request \n orchestrate|orchestration [--action Validate|List|Plan] [--repo ] [--mode lite|normal|full] [--capability ] [--manifest ] [--json]\n language set --language --repo [--json]\n", + "stdoutUtf8": "code-intel [options]\n\nCommands:\n --version|-V [--json]\n help|--help|-h [--all]\n resume --repo [--artifact-root ] [--json]\n classify --report [--json]\n sentrux-normalize --steps [--out ]\n sentrux-debt-register --failures [--repo ] [--out ]\n doctor [--artifact-root ] [--json]\n doctor bootstrap [--repo ] [--repo-path ] [--config ] [--platform auto|windows|macos|linux] [--no-require-repowise] [--require-understand] [--json]\n graph|understand --repo [--language zh] [--full] [--write] [--json]\n provider|providers [--action List|Plan|Validate|Invoke] [--provider repowise|understand] [--operation ] [--repo ] [--language zh] [--write] [--json]\n provider repowise-adapt --request --artifact-root --evaluated-at --max-age-seconds \n provider graph-adapt --request --artifact-root --evaluated-at --max-age-seconds \n provider sentrux-adapt --request --artifact-root --evaluated-at --max-age-seconds \n provider session-adapt --repo --trace [--hotspots ] [--out ] [--working-tree-policy head_only|explicit_overlay]\n provider codenexus-adapt --request --artifact-root --evaluated-at --max-age-seconds \n provider file-boundary --request --out \n provider runtime-ci-evidence --artifact-root --request --out \n compatibility retirement-ticket lint --ticket --evaluated-at \n route|routes [--action List|Plan|Validate] [--provider repowise|understand] [--operation ] [--repo ] [--json]\n sentrux [--no-ratchet]\n (--no-ratchet: `check` only, skip the .sentrux/baseline.json ratchet)\n capability exec --request --out [--artifact-root ] [--manifest ]\n model inventory-validate --request [--out ]\n model route --request [--out ]\n snapshot identity --repo --working-tree-policy [--scope ]...\n repin [--repo ] [--write] [--json] [--exclude ]...\n evidence validate --request --artifact-root \n repository survival-scan --request --artifact-root \n audit --operation validate|render --repo --report [--format markdown|html]\n audit --operation scope --repo --since \n artifact index --artifact-root [--output ] [--operation rebuild|incremental] [--existing ]\n artifact query --artifact-root --repo [--repo-path ] [--artifact-schema ] [--type ] [--contains ] [--limit <1..100>]\n change impact --artifact-root --repo --repo-path --changed [--changed ]... [--staleness current|advisory]\n change risk [--repo ] [--sample ] [--format json|text] (git-only defect-risk score, no prior run, no index)\n change agenda [--repo ] [--min-cochange ] [--format json|text] (git-only review units clustered by co-change, ranked worst first)\n edit impact --repo-path --changed [--changed ]... [--scope ]... (working tree, no prior run, authority: none)\n edit apply --repo-path --file (--span --expect-sha256 --replacement |--replacement-file )... [--out ] [--manifest ] [--envelope] (span-addressed patch; refuses with evidence on digest drift, exit 10)\n decision request-response --request [--response |--cancel ] --now --branch ...\n decision record --resolution --store \n decision replay --query --store \n run execute --repo --out --authority-root --final-name [--profile default|strict|offline] [--manifest ] [--max-concurrency ] [--session-evidence ]\n run dag-coordinate --repo --out [--manifest ] [--max-concurrency ] [--session-evidence ]\n run commit --source-root --authority-root --manifest-ref --final-name \n serve --mcp [--repo-path ] [--repo ] [--artifact-root ] [--manifest ] (stdio MCP query surface over the committed run; read-only, gates nowhere)\n benchmark orientation --out [--repetitions <2..10>]\n benchmark tools --corpus --runs --artifact-root --out \n governance ponytail-gate --request \n orchestrate|orchestration [--action Validate|List|Plan] [--repo ] [--mode lite|normal|full] [--capability ] [--manifest ] [--json]\n language set --language --repo [--json]\n", "stderrUtf8": "" } }, diff --git a/crates/code-intel-cli/tests/mcp_serve.rs b/crates/code-intel-cli/tests/mcp_serve.rs new file mode 100644 index 00000000..7baf36d2 --- /dev/null +++ b/crates/code-intel-cli/tests/mcp_serve.rs @@ -0,0 +1,207 @@ +//! Process-level contract for `code-intel serve --mcp`. +//! +//! The unit tests in `src/mcp_serve/tests.rs` cover dispatch and the argument +//! guards in-process. What they cannot cover is the wiring: that the route +//! table reaches the module, that stdio framing survives a real pipe, and that +//! a client's session ends cleanly when it closes stdin. Those only fail as a +//! spawned process, which is how every client will run this. + +use std::io::{BufRead, BufReader, Write}; +use std::path::PathBuf; +use std::process::{Command, Stdio}; + +use serde_json::{json, Value}; + +fn binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_code-intel")) +} + +/// A directory that exists but publishes nothing. +/// +/// The handshake and framing are what this file tests, and they must hold +/// before any run has been committed — that is precisely the state an agent +/// meets on the first day in a new repository. +struct Fixture(PathBuf); + +impl Fixture { + fn create(label: &str) -> Self { + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("clock") + .as_nanos(); + let path = std::env::temp_dir().join(format!( + "code-intel-serve-{label}-{}-{nonce}", + std::process::id() + )); + std::fs::create_dir_all(path.join("artifacts")).expect("create serve fixture"); + Self(path) + } +} + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +/// Feed the server a session and collect every response line. +/// +/// The environment is not inherited beyond what the OS requires: a host +/// `CODE_INTEL_ARTIFACT_ROOT` leaking in would silently point this at the +/// developer's real runs and make the test pass for the wrong reason. +fn session(fixture: &Fixture, requests: &[Value]) -> (Vec, i32, String) { + let mut child = Command::new(binary()) + .args(["serve", "--mcp", "--repo-path"]) + .arg(&fixture.0) + .args(["--repo", "serve-fixture", "--artifact-root"]) + .arg(fixture.0.join("artifacts")) + .env_remove("CODE_INTEL_ARTIFACT_ROOT") + .env_remove("CODE_INTEL_HOME") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn code-intel serve --mcp"); + + { + let stdin = child.stdin.as_mut().expect("serve stdin"); + for request in requests { + writeln!(stdin, "{request}").expect("write MCP request"); + } + } + // Dropping stdin is the session's end-of-input; the server must exit 0. + drop(child.stdin.take()); + + let output = child.wait_with_output().expect("await serve"); + let responses = BufReader::new(output.stdout.as_slice()) + .lines() + .map(|line| line.expect("read MCP response line")) + .filter(|line| !line.trim().is_empty()) + .map(|line| serde_json::from_str::(&line).expect("each response line is JSON")) + .collect(); + ( + responses, + output.status.code().expect("serve exit code"), + String::from_utf8_lossy(&output.stderr).to_string(), + ) +} + +#[test] +fn a_client_session_handshakes_lists_tools_and_closes_cleanly() { + let fixture = Fixture::create("session"); + let (responses, exit, stderr) = session( + &fixture, + &[ + json!({"jsonrpc":"2.0","id":1,"method":"initialize","params":{ + "protocolVersion":"2025-06-18","capabilities":{}, + "clientInfo":{"name":"contract-test","version":"1"}}}), + json!({"jsonrpc":"2.0","method":"notifications/initialized"}), + json!({"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}), + json!({"jsonrpc":"2.0","id":3,"method":"ping","params":{}}), + ], + ); + + assert_eq!(exit, 0, "closing stdin ends the session cleanly: {stderr}"); + assert!( + stderr.is_empty(), + "a clean session writes no stderr: {stderr}" + ); + assert_eq!( + responses.len(), + 3, + "four messages, one of them a notification: {responses:?}" + ); + + assert_eq!(responses[0]["id"], json!(1)); + assert_eq!(responses[0]["jsonrpc"], json!("2.0")); + assert_eq!( + responses[0]["result"]["serverInfo"]["name"], + json!("code-intel") + ); + assert_eq!( + responses[0]["result"]["protocolVersion"], + json!("2025-06-18") + ); + + let tools = responses[1]["result"]["tools"] + .as_array() + .expect("tools/list serves an array"); + let names = tools + .iter() + .filter_map(|tool| tool["name"].as_str()) + .collect::>(); + assert_eq!( + names, + vec![ + "get_gate_verdict", + "get_facts", + "get_evidence", + "get_audit_status", + "get_change_impact", + "plan_structural_edit", + ] + ); + for tool in tools { + assert_eq!( + tool["annotations"]["readOnlyHint"], + json!(true), + "{} is served without a read-only annotation", + tool["name"] + ); + } + + assert_eq!(responses[2]["result"], json!({}), "ping answers empty"); +} + +/// Before the first run there is nothing to serve, and the server has to say +/// so as an answer rather than as a crash — an agent that gets a dead process +/// on day one never calls the tool again. +#[test] +fn tools_refuse_readably_when_no_run_has_been_committed() { + let fixture = Fixture::create("norun"); + let (responses, exit, stderr) = session( + &fixture, + &[ + json!({"jsonrpc":"2.0","id":1,"method":"tools/call","params":{ + "name":"get_gate_verdict","arguments":{}}}), + ], + ); + + assert_eq!(exit, 0, "a refusal is not a process failure: {stderr}"); + assert_eq!(responses.len(), 1); + let result = &responses[0]["result"]; + assert!( + responses[0].get("error").is_none(), + "a missing run is not a transport error: {:?}", + responses[0] + ); + assert_eq!(result["isError"], json!(true)); + let payload: Value = serde_json::from_str( + result["content"][0]["text"] + .as_str() + .expect("one text block"), + ) + .expect("tool payload is JSON"); + assert_eq!(payload["schema"], json!("code-intel-mcp-tool-error.v1")); + assert!( + payload["error"] + .as_str() + .is_some_and(|text| text.contains("no committed authoritative run is indexed")), + "the refusal must name the missing precondition: {payload}" + ); +} + +#[test] +fn serve_without_a_transport_is_a_usage_error() { + let output = Command::new(binary()) + .arg("serve") + .output() + .expect("spawn serve without a transport"); + assert_eq!(output.status.code(), Some(64)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("serve requires a transport") && stderr.contains("--mcp"), + "usage must name the missing transport: {stderr}" + ); + assert!(output.stdout.is_empty(), "a usage error writes no stdout"); +} diff --git a/skills/code-intel-pipeline/SKILL.md b/skills/code-intel-pipeline/SKILL.md index 0b3302da..18932d2a 100644 --- a/skills/code-intel-pipeline/SKILL.md +++ b/skills/code-intel-pipeline/SKILL.md @@ -107,6 +107,25 @@ Use the Sentrux session wrapper for an Agent coding session: Keep `.sentrux/rules.toml` separate from `.sentrux/baseline.json`. Rules define architecture boundaries; baselines detect change. Never save a new baseline to hide a regression. +## Prefer the MCP query surface over the CLI for single questions + +If the host can register MCP servers, register this one and ask through it instead of shelling out +per question: + +```powershell +code-intel serve --mcp --repo +``` + +It is a stdio server over the last committed run, with `get_gate_verdict`, `get_facts`, +`get_evidence`, `get_audit_status`, `get_change_impact`, and `plan_structural_edit`. Pass `--repo` +explicitly: a worktree's directory name is not the name `run commit` published under. The surface is +read-only and gates nothing — a verdict read here is not a verdict earned, and the CLI and CI paths +remain the only places a gate runs. + +The CLI spellings below stay correct and are the fallback when no MCP host is available. A full +`code-intel --mode normal` run is the deep-inspection mode, not the way to answer one +question. + ## While writing code Run this loop whenever implementing, refactoring, or fixing code in an analyzed repository: From 6006cf256bc88a50f9d610392ec98b558190a69e Mon Sep 17 00:00:00 2001 From: Curry Date: Wed, 5 Aug 2026 17:08:40 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix(serve):=20=E5=A3=B0=E6=98=8E=E7=9A=84?= =?UTF-8?q?=E7=95=8C=E5=BF=85=E9=A1=BB=E4=B8=A4=E4=B8=AA=E8=B0=83=E7=94=A8?= =?UTF-8?q?=E6=96=B9=E9=83=BD=E5=AE=88=E2=80=94=E2=80=94limit=20=E8=B6=8A?= =?UTF-8?q?=E7=95=8C=E3=80=81=E6=89=B9=E9=87=8F=E9=9D=99=E9=BB=98=E4=B8=A2?= =?UTF-8?q?=E5=8C=85=E3=80=81=E5=8D=8F=E8=AE=AE=E7=89=88=E6=9C=AC=E8=99=9A?= =?UTF-8?q?=E6=8A=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit 在 #195 上报的四条,逐条核实全部成立。 1. `optional_limit` 不执行自己声明的 1..=100(真 bug) `get_facts` 的 limit 经 `EvidenceQueryRequest::new` 有界,`get_evidence` 直接 吃原值。`limit: 0` 让 `evidence_chain` 在第一条匹配上就 `links.len() == limit` 成立、break,于是返回 status "unbacked"——这是在断言"没有任何已提交产物提到 该标识符",而提到它的产物就摆在那里。诚实位被一个未校验的参数变成了谎报。 `as usize` 也未经检查:32 位目标上超 u32::MAX 会截断,被拒的值反而通过。 修法是把界放进 `optional_limit`,比较在 cast 之前对 u64 做;上界从 `evidence_query::MAX_LIMIT` 读,不再各写一份 100。只有一个调用方遵守的界不是界。 2. 只宣称本 transport 真能实现的 MCP 版本 原先 SUPPORTED_PROTOCOL_VERSIONS 含 2025-03-26 与 2024-11-05,这两版要求服务端 接受 JSON-RPC 批量(顶层数组)。本 transport 一行一条消息,没有批量派发——协商到 这些版本的客户端发批量后会永远等不到回应。2025-06-18 移除了批量,正是本框架能 诚实声明的唯一一版。 3. 批量请求显式拒绝,不再静默丢 数组没有自己的 `id`,原先会落进 notification 分支被丢掉——客户端挂死。现在回 -32600 并说明原因。即使版本已收窄,走错路的批量也该听见拒绝,而不是沉默。 4. 保留失败原因,不再三合一 `plan_structural_edit` 读回计划时,产物查找 / 文件读取 / JSON 解析三条失败路径 都经 `.ok()` 塌成同一个 None,于是"产物损坏"被报成"没产出产物"。拆成 `read_plan_artifact`,各自带各自的原因。 5. 数据来源逐工具写清 四个工具是已提交 run 的投影,但 `get_change_impact` 是"已提交 import 图 × 当前 --repo-path",`plan_structural_edit` 扫的是当前工作树。原先 .mcp.json / README / SKILL.md 一律写成"来自已提交 run",对后两个是错的。按工具分列。 验证:新增 3 个测试(版本面收窄、批量显式拒绝、两个 limit 调用方共享同一道界, 含 0/101 拒绝与 1/100/缺省放行)。cargo test 全绿(56 suite)、fmt 干净、clippy 对本模块零告警、权威 self-scan completed/green(4661 anchors verified、0 dropped)。 真机核实:limit 0 与 101 均被拒并给出 1..=100,limit 2 正常返回 2 条并正确标 truncated;批量收到 -32600;客户端请求 2025-03-26 被换成 2025-06-18。 Refs #54 #58 --- .mcp.json | 2 +- README.md | 20 ++--- crates/code-intel-cli/src/evidence_query.rs | 2 +- .../code-intel-cli/src/mcp_serve/handlers.rs | 58 +++++++++--- crates/code-intel-cli/src/mcp_serve/mod.rs | 30 +++++-- crates/code-intel-cli/src/mcp_serve/tests.rs | 89 +++++++++++++++++-- skills/code-intel-pipeline/SKILL.md | 19 ++-- 7 files changed, 176 insertions(+), 44 deletions(-) diff --git a/.mcp.json b/.mcp.json index bfe33544..ba9a72fc 100644 --- a/.mcp.json +++ b/.mcp.json @@ -8,7 +8,7 @@ "--repo", "code-intel-pipeline" ], - "description": "code-intel: committed-run query surface — gate verdict, facts, evidence chain, audit status, mid-edit blast radius, structural-edit preview. Read-only; gates nowhere. --repo is pinned because a worktree directory name is not the name run commit publishes under; --repo-path defaults to the working directory." + "description": "code-intel: agent query surface. Read-only; gates nowhere. Data source differs per tool — get_gate_verdict / get_facts / get_evidence / get_audit_status project the last committed run; get_change_impact reads the committed import graph but compares it against the live --repo-path and labels the result stale-advisory; plan_structural_edit scans the live checkout and writes nothing. --repo is pinned because a worktree directory name is not the name run commit publishes under; --repo-path defaults to the working directory." }, "repowise": { "command": "repowise", diff --git a/README.md b/README.md index 8876aa56..d5fdb08d 100644 --- a/README.md +++ b/README.md @@ -544,16 +544,16 @@ Agent 平时问单点问题走 MCP，不必为一个问题跑一整轮 pipeline code-intel serve --mcp --repo ``` -stdio MCP server，按需起、随 session 生灭，答案全部来自最近一次已提交的 run。注册进 `.mcp.json` 后 Claude Code / Codex 直连可调： - -| 工具 | 回答什么 | -|---|---| -| `get_gate_verdict` | 权威 run 的门禁结论、第一条失败规则、最小重跑命令 | -| `get_facts` | 按 artifact type / schema / 子串查已验证事实（热点、import 边、scorecard、符号） | -| `get_evidence` | 一条 finding 的证据链：哪些产物提到它、各自 sha256、记录时的 snapshot | -| `get_audit_status` | 各科室审计结论、评分、覆盖；没跑过 audit 会明说"不可用"而不是装绿 | -| `get_change_impact` | 改这些文件会波及谁、该先跑哪些测试（默认 stale-advisory，写码中途也能问） | -| `plan_structural_edit` | ast-grep 结构改写预览，只出匹配清单，不落盘 | +stdio MCP server，按需起、随 session 生灭。注册进 `.mcp.json` 后 Claude Code / Codex 直连可调。**数据来源逐个工具不同**，读答案时按这一列判它有多新： + +| 工具 | 回答什么 | 数据来源 | +|---|---|---| +| `get_gate_verdict` | 权威 run 的门禁结论、第一条失败规则、最小重跑命令 | 已提交 run；附 freshness | +| `get_facts` | 按 artifact type / schema / 子串查已验证事实（热点、import 边、scorecard、符号） | 已提交 run（digest 已校验） | +| `get_evidence` | 一条 finding 的证据链：哪些产物提到它、各自 sha256、记录时的 snapshot | 已提交 run | +| `get_audit_status` | 各科室审计结论、评分、覆盖；没跑过 audit 会明说"不可用"而不是装绿 | 已提交 run | +| `get_change_impact` | 改这些文件会波及谁、该先跑哪些测试 | **已提交 import 图 × 当前 `--repo-path`**；默认标 stale-advisory 并同时给出 recorded/current 两个 snapshot identity | +| `plan_structural_edit` | ast-grep 结构改写预览，只出匹配清单，不落盘 | **当前工作树**（不是已提交 run） | **这个面只读，不裁决。** 门禁判定照旧只走 CLI 与 CI 路径——查询面被 prompt injection 说服也改不了结论。唯一会执行东西的工具是 `plan_structural_edit`，它在跑之前拿注册表核对自己的 capability 声明，一旦声明里出现 `repo_mutation` 就直接拒绝。 diff --git a/crates/code-intel-cli/src/evidence_query.rs b/crates/code-intel-cli/src/evidence_query.rs index f5941ca0..01286410 100644 --- a/crates/code-intel-cli/src/evidence_query.rs +++ b/crates/code-intel-cli/src/evidence_query.rs @@ -6,7 +6,7 @@ use serde_json::{json, Value}; use crate::committed_evidence::{self, CommittedEvidence, EvidenceError}; const DEFAULT_LIMIT: usize = 20; -const MAX_LIMIT: usize = 100; +pub(crate) const MAX_LIMIT: usize = 100; const PREVIEW_CHARS: usize = 400; pub(crate) fn run_raw(raw: &[String]) -> i32 { diff --git a/crates/code-intel-cli/src/mcp_serve/handlers.rs b/crates/code-intel-cli/src/mcp_serve/handlers.rs index 6f70e2c9..ffb2bd90 100644 --- a/crates/code-intel-cli/src/mcp_serve/handlers.rs +++ b/crates/code-intel-cli/src/mcp_serve/handlers.rs @@ -9,7 +9,7 @@ use std::env; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; use serde_json::{json, Map, Value}; @@ -22,6 +22,10 @@ use crate::{ const EDIT_PLAN_CAPABILITY: &str = "edit.ast-grep-plan"; const DEFAULT_LIMIT: usize = 20; +/// Read from `evidence_query` rather than restated: the tool descriptors, the +/// `--limit` flag, and this surface must agree on the ceiling, and three copies +/// of `100` is how they stop agreeing. +use crate::evidence_query::MAX_LIMIT; pub(super) fn call(context: &ServeContext, name: &str, arguments: &Value) -> Result { if !arguments.is_object() { @@ -297,15 +301,14 @@ fn structural_edit(context: &ServeContext, arguments: &Value) -> Result(&bytes).ok()); + // Each step keeps its own failure reason. Collapsing all three through + // `.ok()` reported "produced no artifact" for a plan that was produced and + // then turned out to be unreadable or malformed — three very different + // things for whoever has to work out why. + let plan = read_plan_artifact(&outcome, &staging); let _ = fs::remove_dir_all(&staging); match plan { - Some(plan) => Ok(json!({ + Ok(plan) => Ok(json!({ "schema": "code-intel-mcp-structural-edit-plan.v1", "capability": EDIT_PLAN_CAPABILITY, "repo": context.repo, @@ -314,8 +317,8 @@ fn structural_edit(context: &ServeContext, arguments: &Value) -> Result Err(format!( - "edit plan produced no artifact (exit {}): {}", + Err(reason) => Err(format!( + "{reason} (exit {}): {}", outcome.exit_code, outcome .diagnostic @@ -324,6 +327,18 @@ fn structural_edit(context: &ServeContext, arguments: &Value) -> Result Result { + let relative = outcome + .result + .as_ref() + .and_then(|result| result["artifacts"][0]["path"].as_str()) + .ok_or("edit plan produced no artifact")?; + let bytes = fs::read(staging.join(relative)) + .map_err(|error| format!("edit plan artifact {relative} could not be read: {error}"))?; + serde_json::from_slice(&bytes) + .map_err(|error| format!("edit plan artifact {relative} is not valid JSON: {error}")) +} + /// The read-only boundary, enforced against the registry rather than asserted /// in prose. /// @@ -431,13 +446,28 @@ fn optional_bool(arguments: &Value, key: &str) -> Result, String> { } } +/// The declared `1..=100` bound, enforced here rather than at each call site. +/// +/// `get_facts` used to be the only bounded caller, because its limit passes +/// through `EvidenceQueryRequest::new`; `get_evidence` consumed the raw value. +/// A `limit` of 0 then made `evidence_chain` break on its first match and +/// report `status: "unbacked"` — a claim that no committed artifact mentions +/// the identifier — while artifacts that mention it were sitting right there. +/// A bound that only one of two callers honours is not a bound. +/// +/// The comparison happens on the `u64` before any cast: `as usize` truncates on +/// a 32-bit target, which would turn an out-of-range value into an accepted one. fn optional_limit(arguments: &Value) -> Result { + let out_of_range = || format!("limit must be an integer in 1..={MAX_LIMIT}"); match arguments.get("limit") { None | Some(Value::Null) => Ok(DEFAULT_LIMIT), - Some(Value::Number(value)) => value - .as_u64() - .map(|value| value as usize) - .ok_or_else(|| "limit must be a positive integer".to_string()), + Some(Value::Number(value)) => { + let value = value.as_u64().ok_or_else(out_of_range)?; + if !(1..=MAX_LIMIT as u64).contains(&value) { + return Err(out_of_range()); + } + Ok(value as usize) + } Some(_) => Err("limit must be an integer".into()), } } diff --git a/crates/code-intel-cli/src/mcp_serve/mod.rs b/crates/code-intel-cli/src/mcp_serve/mod.rs index c9b2d918..55c78621 100644 --- a/crates/code-intel-cli/src/mcp_serve/mod.rs +++ b/crates/code-intel-cli/src/mcp_serve/mod.rs @@ -33,12 +33,20 @@ mod tools; #[cfg(test)] mod tests; -/// The newest MCP revision this server implements. Clients that ask for an -/// older supported revision get theirs echoed back; anything else is answered -/// with this one, which is what the specification prescribes for a version the -/// server does not speak. +/// The MCP revisions this server implements — deliberately just one. +/// +/// `2025-03-26` and earlier require a server to accept JSON-RPC *batches*: a +/// top-level array of requests answered by an array of responses. This +/// transport answers one object per line and has no batch dispatch, so +/// advertising those revisions would promise something a client could hang +/// waiting for. `2025-06-18` removed batching, which is exactly why it is the +/// one revision this framing can honestly claim. +/// +/// A client asking for anything else is answered with this revision, which is +/// what the specification prescribes: the client then decides whether it can +/// proceed. That is a visible negotiation failure rather than a silent one. const PROTOCOL_VERSION: &str = "2025-06-18"; -const SUPPORTED_PROTOCOL_VERSIONS: &[&str] = &["2025-06-18", "2025-03-26", "2024-11-05"]; +const SUPPORTED_PROTOCOL_VERSIONS: &[&str] = &[PROTOCOL_VERSION]; const USAGE: &str = "usage: serve --mcp [--repo-path ] [--repo ] [--artifact-root ] [--manifest ]"; @@ -219,6 +227,18 @@ pub(super) fn handle_line(context: &ServeContext, line: &str) -> Option { )) } }; + // A batch is refused out loud. It has no `id` of its own, so falling + // through to the notification branch below would drop it silently and + // leave the client waiting for responses that are never coming — the exact + // hang that advertising a batching revision would have caused. + if message.is_array() { + return Some(failure( + Value::Null, + -32600, + "invalid request: JSON-RPC batches are not supported; this server speaks MCP \ + 2025-06-18, which sends messages individually", + )); + } let id = message.get("id").filter(|id| !id.is_null()).cloned()?; let Some(method) = message["method"].as_str() else { return Some(failure(id, -32600, "invalid request: no method")); diff --git a/crates/code-intel-cli/src/mcp_serve/tests.rs b/crates/code-intel-cli/src/mcp_serve/tests.rs index dea114ad..0005c7c1 100644 --- a/crates/code-intel-cli/src/mcp_serve/tests.rs +++ b/crates/code-intel-cli/src/mcp_serve/tests.rs @@ -127,17 +127,47 @@ fn notifications_are_never_answered() { } } +/// Only revisions this transport can actually honour are advertised. +/// +/// `2025-03-26` and earlier mandate JSON-RPC batch support, which this +/// line-per-message framing does not implement. Echoing one back would let a +/// client send a batch and wait forever. #[test] -fn initialize_echoes_a_supported_revision_and_substitutes_an_unknown_one() { - let fixture = Fixture::create("initialize"); +fn only_the_non_batching_revision_is_advertised() { + assert_eq!( + super::SUPPORTED_PROTOCOL_VERSIONS, + &[super::PROTOCOL_VERSION] + ); + for batching_revision in ["2025-03-26", "2024-11-05"] { + assert!( + !super::SUPPORTED_PROTOCOL_VERSIONS.contains(&batching_revision), + "{batching_revision} requires batch dispatch this transport does not have" + ); + } +} + +/// A batch has no `id`, so without an explicit refusal it would fall through +/// the notification branch and be dropped in silence — the client hangs. +#[test] +fn a_json_rpc_batch_is_refused_out_loud() { + let fixture = Fixture::create("batch"); let context = fixture.context(); - let older = super::handle_line( - &context, - &request(1, "initialize", json!({"protocolVersion": "2024-11-05"})), - ) - .expect("initialize response"); - assert_eq!(older["result"]["protocolVersion"], json!("2024-11-05")); + let batch = json!([ + {"jsonrpc": "2.0", "id": 1, "method": "tools/list", "params": {}}, + {"jsonrpc": "2.0", "id": 2, "method": "ping", "params": {}}, + ]) + .to_string(); + let response = super::handle_line(&context, &batch).expect("a batch must not be dropped"); + assert_eq!(response["error"]["code"], json!(-32600)); + assert!(response["error"]["message"] + .as_str() + .is_some_and(|message| message.contains("batches are not supported"))); +} +#[test] +fn initialize_substitutes_an_unsupported_revision() { + let fixture = Fixture::create("initialize"); + let context = fixture.context(); let unknown = super::handle_line( &context, &request(2, "initialize", json!({"protocolVersion": "1999-01-01"})), @@ -267,6 +297,49 @@ fn crafted_arguments_are_refused_before_any_evidence_is_read() { ); } +/// The declared `1..=100` bound must hold for every tool that takes a limit, +/// not just the one whose request type happened to check it. +/// +/// `limit: 0` used to make `get_evidence` break on its first match and answer +/// `status: "unbacked"` — asserting that no committed artifact mentions the +/// identifier while artifacts that mention it were right there. A bound one +/// caller honours is not a bound. +#[test] +fn every_limit_taking_tool_shares_the_declared_bound() { + let fixture = Fixture::create("limit"); + let context = fixture.context(); + for (tool, arguments) in [ + ("get_evidence", json!({"findingId": "anything", "limit": 0})), + ( + "get_evidence", + json!({"findingId": "anything", "limit": 101}), + ), + ("get_facts", json!({"limit": 0})), + ("get_facts", json!({"limit": 101})), + ] { + let error = handlers::call(&context, tool, &arguments) + .err() + .unwrap_or_else(|| panic!("{tool} accepted {arguments}")); + assert!( + error.contains("1..=100"), + "{tool} rejected {arguments} for the wrong reason: {error}" + ); + } + + // The bound is a range, not a rejection of everything: the endpoints and a + // missing limit must still get past argument validation and reach the + // (absent) committed run. + for arguments in [json!({"limit": 1}), json!({"limit": 100}), json!({})] { + let error = handlers::call(&context, "get_facts", &arguments) + .err() + .unwrap_or_default(); + assert!( + !error.contains("1..=100"), + "a valid limit was rejected: {arguments} -> {error}" + ); + } +} + #[test] fn a_capability_that_declared_repository_mutation_would_be_refused() { assert!(handlers::refuse_repository_mutation(&json!(["repo_read", "process_spawn"])).is_ok()); diff --git a/skills/code-intel-pipeline/SKILL.md b/skills/code-intel-pipeline/SKILL.md index 18932d2a..63e7fd13 100644 --- a/skills/code-intel-pipeline/SKILL.md +++ b/skills/code-intel-pipeline/SKILL.md @@ -116,11 +116,20 @@ per question: code-intel serve --mcp --repo ``` -It is a stdio server over the last committed run, with `get_gate_verdict`, `get_facts`, -`get_evidence`, `get_audit_status`, `get_change_impact`, and `plan_structural_edit`. Pass `--repo` -explicitly: a worktree's directory name is not the name `run commit` published under. The surface is -read-only and gates nothing — a verdict read here is not a verdict earned, and the CLI and CI paths -remain the only places a gate runs. +It is a stdio server with six tools whose data sources differ — check which one you are reading +before you trust how fresh it is: + +- `get_gate_verdict`, `get_facts`, `get_evidence`, `get_audit_status` project the **last committed + run**. Each answer carries the run, the snapshot identity, and a freshness field. +- `get_change_impact` reads the **committed import graph** but evaluates it against the **current + `--repo-path`**. It answers `stale-advisory` by default, naming both the recorded and the current + snapshot identity; pass `requireCurrent` to get the fail-closed behaviour instead. +- `plan_structural_edit` scans the **current working tree**, not the committed run, and writes + nothing. + +Pass `--repo` explicitly: a worktree's directory name is not the name `run commit` published under. +The surface is read-only and gates nothing — a verdict read here is not a verdict earned, and the +CLI and CI paths remain the only places a gate runs. The CLI spellings below stay correct and are the fallback when no MCP host is available. A full `code-intel --mode normal` run is the deep-inspection mode, not the way to answer one