diff --git a/doc/architecture/SPEC/config.md b/doc/architecture/SPEC/config.md index 9410d203..e40a7411 100644 --- a/doc/architecture/SPEC/config.md +++ b/doc/architecture/SPEC/config.md @@ -1,6 +1,6 @@ # config.py —— 规范 - + `LOC ~658 · 配置 · refactor-status: oversized` @@ -79,3 +79,13 @@ provider 的缺省模型必须在**这一处**解析，否则 `ResolvedTarget.mo 如果再增长，请分组成嵌套 settings 模型（`LLMSettings` / `PushSettings` / `ReviewSettings` / `BackendSettings`），**而不是拆文件** —— 调用点依赖"一个 `Settings` 对象到达每个 `StepContext`"。 + +## rebase 后端抽象（2026-09-19） + +`rebase_backend`（默认 `api`）选择跑 rebase **module agent** 的 provider： +`api` 为进程内 Anthropic tool-use 循环，其余为 harness provider id +（`cursor` / `claude-code` / `codex`）。`rebase_backend_model` 是 harness +**内部**的模型 id（如 `cursor-grok-4.6-high-fast`）。 + +注意：tier 模型名指向 RAW-API 模型，对 harness CLI 无意义，**绝不**转发过去； +为空时回落到 `strict_backend_model`，再回落到 harness 自身默认值。 diff --git a/doc/architecture/SPEC/engine/steps/rebase_v3.md b/doc/architecture/SPEC/engine/steps/rebase_v3.md index b1164da0..c0b03d19 100644 --- a/doc/architecture/SPEC/engine/steps/rebase_v3.md +++ b/doc/architecture/SPEC/engine/steps/rebase_v3.md @@ -1,6 +1,6 @@ # engine/steps/rebase_v3.py —— 规范 - + `LOC ~2204 · step 库（v3 rebase 装配层） · refactor-status: oversized` @@ -90,3 +90,11 @@ import（见重构备注）。 违例；对 `.workspace._guard_clean_rebase` 的直接 import 是另一条。拆分时 substate + `state_updates` 双写、模块短路/串行锁的 crash-window 契约**必须** 原样保留（resume 完整性测试护住）。 + +## harness 后端派发（2026-09-19） + +module agent 可派发到 harness provider（`settings.rebase_backend`）。 +harness 会话通过 MCP tool bridge 拿到同一套工具面，plan gate 在派发处强制。 +`build_backends(repo=)` 收的是仓库 **ROOT 路径**（→ `TestRunner(repo_root=)`）， +不是仓库名：传名字会让 run_pytest/run_precommit/reproduce 以 +`FileNotFoundError` 静默失效，module 却报告完成。 diff --git a/doc/architecture/SPEC/providers/cursor.md b/doc/architecture/SPEC/providers/cursor.md index ce118559..ce5ed0fa 100644 --- a/doc/architecture/SPEC/providers/cursor.md +++ b/doc/architecture/SPEC/providers/cursor.md @@ -1,6 +1,6 @@ # providers/cursor.py —— 规范 - + `LOC ~207 · harness transport（Cursor 订阅） · refactor-status: ok` @@ -38,3 +38,10 @@ stdlib + `.base` + `.registry` + `..agent_loop.AgentOutcome` + `..llm` 的类型 ## 重构备注 这套调用形状是由 Composer 评测臂（`eval/dataset/run_cursor_arm.py`）验证出来的； **如果那个脚本和这个 transport 发生漂移，评测臂就不再是在测量产品了。** + +## 作为 rebase module-agent 后端（2026-09-19） + +`REBASE_BACKEND=cursor` 时，cursor-agent 承担 rebase module agent。 +registry 给 cursor 的能力是 `{mcp_tools, usage_reporting}`——**没有** +`builtin_tools_off`，所以 cursor 自带工具会绕过 bridge：bridged 调用受 +scope 约束，native 调用只被**记录**。 diff --git a/doc/architecture/SPEC/rebase_engine.md b/doc/architecture/SPEC/rebase_engine.md index c59965e4..9dda4e28 100644 --- a/doc/architecture/SPEC/rebase_engine.md +++ b/doc/architecture/SPEC/rebase_engine.md @@ -1,6 +1,6 @@ # rebase_engine/ —— 规范 - + `LOC ~7500（26 个模块） · repo-rebase-v3 的原生 rebase 引擎 · refactor-status: ok` @@ -132,3 +132,8 @@ worktree）、`test_assembly.py`（tools/loop/module/prompt/wheel）、 是 PR4d 执行后的退役候补（"无永久双 store 世界"）。docstring 里的 choke-point 编号（agent_loop/rebase_tools 写"C5"）落后于 `_CONSTRAINTS.md` 目录（工具 choke point = C3）—— 值得统一，改注释不改行为。 + +## module_rebase 的 harness 路径（2026-09-19） + +`module_rebase` 既可跑进程内 agent 循环，也可把一个 module 交给 harness +provider；两条路径共用同一 prompt、同一工具面与同一 plan gate。 diff --git a/doc/architecture/SPEC/tool_bridge.md b/doc/architecture/SPEC/tool_bridge.md index 62b25557..c0875e6f 100644 --- a/doc/architecture/SPEC/tool_bridge.md +++ b/doc/architecture/SPEC/tool_bridge.md @@ -1,6 +1,6 @@ # tool_bridge.py —— 规范 - + `LOC ~308 · 面向 harness 会话的 scoped 工具 MCP server · refactor-status: ok` @@ -48,3 +48,10 @@ knowledge/repo-map 工厂。 因为它捆绑的运行时不带 MCP client，拿到无法履约的 spec 时会记一条 `capability_gap`。 任何"这次 run 走了工具桥"的说法，都必须**对照那个标志核实**， 而不是从"后端是 harness"推定。 + +## 向 harness 会话提供 rebase 工具面（2026-09-19） + +bridge 把 run 的 20 工具面 + `doc_search`/`doc_read` 暴露给 harness 会话。 +生成的 MCP 工具函数必须是 **keyword-only**（schema 会交错 required/optional）， +零属性工具不能带裸 `*`，且必须**丢掉**未设置的 optional——传 None 会打断 +每一次 `read_file`。失败的 bridged 调用记录失败**原因**。 diff --git a/doc/features/provider-registry.md b/doc/features/provider-registry.md index 8aa3c671..11a75e8a 100644 --- a/doc/features/provider-registry.md +++ b/doc/features/provider-registry.md @@ -283,6 +283,38 @@ STRICT_BACKEND_TIMEOUT_S=1800 # 单会话墙钟上限 **M2 —— claude-code** · **M3 —— codex**：同样的验收形状（离线假 CLI 测试 + 各一次实网 冒烟）；M2 另外要证明在净化环境下**嵌套 CC**（Strict 宿主 = Claude Code）能工作。 +**M4 —— rebase 模块 agent 走 harness（2026-09-18）** + +rebase 模块 agent 此前**硬接**在 API transport 上（`rebase_v3._tier_client` → +`Anthropic`），provider 注册表只服务 `STRICT_BACKEND`/评审侧。M4 把它接到同一套 +抽象上：`ModuleRunConfig.backend`（默认 `api`，保持进程内循环逐字节不变）非 `api` +时，整个模块步骤交给该 harness 的 `run_session`。 + +两件进程内循环独有的东西必须跟着搬进桥里，否则 harness 后端等于没有它们： + +1. **20 个 adapter 工具**（不是共享注册表里的 6 个）。桥收到带 `rebase` 段的 spec 时， + 在**自己的进程内**用同一个 `build_rebase_tools` 重建工具包，经 `dispatch(..., extra=)` + 下发——scope 守卫、越界记录、结果上限与进程内一致。`build_backends`（原 + `_build_backends`）改为吃显式可序列化入参，正是为了能在桥进程里重建同一套生产 backends。 +2. **计划闸门**。`PlanGate` 在 **dispatch 处**拒绝 `edit_file`/`run_pytest`/ + `run_precommit`，并在闸门关闭时把 `write_file` 限制在计划目录内；开闸条件与 + `agent_loop` 完全一致：**成功**写出 `.decision.md`。这比"只是不广告工具"更强—— + harness 即使自己发出调用也会被拒。闸门开启会记 `plan_gate_opened`，父进程据此回读 + `plan_done`。 + +spec 只携带路径与模型标识：**不含 api_key，不含子进程 env**，凭证与 env 由桥进程自己的 +环境提供，`run_dir/bridge/` 下不落任何机密。 + +已知缺口（明示）：`_PATH_ARGS` 只认识共享工具，因此对带路径参数的 adapter 工具， +读包含性预检是 no-op（写仍由 `ToolScope` 在 dispatch 内守住）。另外 cursor/codex +**没有原生轮次上限**，`max_iters` 只在 claude-code（`--max-turns`）上生效，其余靠 +`harness_timeout_s` 与提示词里的预算纪律兜底。 + +**策略变更，需记录**：本仓库原先明确规定知识/记忆检索**不过桥**（"harness 会话可以读 +本仓库的知识，但永远不能写入"）。owner 于 2026-09-18 决定 rebase 走 harness 时要求 +**全量平价**，因此 `search_debug_memory`/`record_debug_memory`/`skill_manage`/ +`search_skills` 一并经桥重建——这是对既有决策的**反转**，不是顺带实现。 + ## 测试计划（离线优先，本仓库纪律） - `test_providers.py` —— 注册表解析；Strict 与 CLI 在空选择下的行为差异； diff --git a/src/infermatrix_copilot/config.py b/src/infermatrix_copilot/config.py index 8ad0cdff..4327f323 100644 --- a/src/infermatrix_copilot/config.py +++ b/src/infermatrix_copilot/config.py @@ -229,6 +229,17 @@ class Settings(BaseSettings): # v3 module-agent plan reviewer (rebase_engine/plan_review.py): empty = # review on the run's own tier model — never silently skipped rebase_reviewer_model: str = "" + # Which provider runs the rebase MODULE agents: "api" (default, the + # in-process Anthropic tool-use loop) or a harness provider id + # ("cursor", "claude-code", "codex"). Harness runs serve the same + # 20-tool surface through the MCP tool bridge; see + # doc/features/provider-registry.md. + rebase_backend: str = "api" + # Model id INSIDE the harness for rebase module agents (e.g. + # "cursor-grok-4.6-high-fast"). The tier model names a RAW-API model and + # is meaningless to a harness CLI, so it is never forwarded there; empty + # falls back to `strict_backend_model`, then the harness's own default. + rebase_backend_model: str = "" # v3 remote CI (rebase_engine/ci_loop.run_ci_rounds) — neutral knobs; # pipeline identities live in the adapter. Defaults are the parent's. rebase_ci_retries: int = 2 # push/rebuild rounds after the first diff --git a/src/infermatrix_copilot/engine/steps/rebase_v3.py b/src/infermatrix_copilot/engine/steps/rebase_v3.py index 9a486769..68da2a76 100644 --- a/src/infermatrix_copilot/engine/steps/rebase_v3.py +++ b/src/infermatrix_copilot/engine/steps/rebase_v3.py @@ -9,6 +9,8 @@ from __future__ import annotations +from dataclasses import asdict as _dc_asdict + import asyncio import shlex import weakref @@ -137,7 +139,7 @@ def _test_roots(manifest: dict) -> tuple: return tuple(tm.get("test_change_roots") or ("tests/",)) -def _target_test_env(ctx: StepContext, manifest: dict, +def _target_test_env(settings, manifest: dict, *, pythonpath_prepend: str | None = None) -> dict: """The env for TARGET-repo subprocesses (tests, precommit, wheel installs): inherit-plus-overlay with the target venv on PATH, the @@ -146,7 +148,7 @@ def _target_test_env(ctx: StepContext, manifest: dict, copilot's own virtualenv.""" import os from ...testing.env_plan import build_subprocess_env - venv = _target_venv(manifest, extra=ctx.settings.expansion_env()) + venv = _target_venv(manifest, extra=settings.expansion_env()) return build_subprocess_env( venv=Path(venv) if venv else None, cuda_visible_devices=os.environ.get("CUDA_VISIBLE_DEVICES"), @@ -197,7 +199,9 @@ def _agent_shell_env(ctx: StepContext, manifest: dict, repo_root: str, return env -def _build_backends(ctx: StepContext, manifest: dict, repo: str, target): +def build_backends(*, settings, state, run_dir: Path, trace, + manifest: dict, repo: str, model: str, + base_url: str = "", api_key: str = ""): """The PRODUCTION `RebaseBackends`: plan review on the run's resolved tier backend, pytest/reproduce/precommit through the PR1 runner in the TARGET env, and the knowledge tools on the copilot stores (agents may @@ -205,21 +209,26 @@ def _build_backends(ctx: StepContext, manifest: dict, repo: str, target): `_unwired` defaults for live module/debug agents; an unavailable collaborator still answers with an explicit error dict, never a silent success. All handlers are SYNC (tool dispatch is synchronous - inside the agent loop) — the reviewer uses its own sync client.""" + inside the agent loop) — the reviewer uses its own sync client. + + Takes EXPLICIT serializable inputs rather than a `StepContext` so the + harness tool bridge can rebuild the identical production backends in its + own process from a bridge spec (see `tool_bridge`). `_build_backends` + below is the thin in-process adapter.""" from ...memory import SkillStore from ...memory.debug_memory import DebugMemory from ...rebase_engine.plan_review import review_plan from ...rebase_engine.rebase_tools import RebaseBackends from ...testing.runner import TestJob, TestRunner - repo_name = (ctx.state.get("task_spec") or {}).get("repo", "") - reviewer_model = getattr(ctx.settings, "rebase_reviewer_model", "") \ - or target.model + repo_name = (state.get("task_spec") or {}).get("repo", "") + reviewer_model = getattr(settings, "rebase_reviewer_model", "") \ + or model def request_plan_review(**kw) -> dict: from anthropic import Anthropic - ckw: dict = {"api_key": target.api_key} - if target.base_url: - ckw["base_url"] = target.base_url + ckw: dict = {"api_key": api_key} + if base_url: + ckw["base_url"] = base_url return review_plan( Anthropic(**ckw), reviewer_model, plan_json_path=str(kw.get("plan_json_path", "")), @@ -239,13 +248,13 @@ def _run_tests(kw: Mapping, key: str) -> dict: if markers: cmd += f" -m '{markers}'" runner = TestRunner(repo_root=Path(repo), - tests_dir=ctx.run_dir / "tests", - gpu_lock_dir=ctx.run_dir / "gpu_lock") + tests_dir=run_dir / "tests", + gpu_lock_dir=run_dir / "gpu_lock") outcome = runner.run( TestJob(key=f"{key}_{abs(hash(cmd)) % 10 ** 8}", command=cmd, timeout_sec=float(kw.get("timeout") or 1800), min_gpus=0, gpu_lock=True), - _target_test_env(ctx, manifest)) + _target_test_env(settings, manifest)) tail = "" try: if outcome.log_file and Path(outcome.log_file).is_file(): @@ -282,20 +291,20 @@ def run_precommit(**kw) -> dict: command = _re.sub(r"\s(?:--all-files|-a)\b", "", command) command += " --files " + " ".join(str(f) for f in files) runner = TestRunner(repo_root=Path(repo), - tests_dir=ctx.run_dir / "tests", - gpu_lock_dir=ctx.run_dir / "gpu_lock") + tests_dir=run_dir / "tests", + gpu_lock_dir=run_dir / "gpu_lock") outcome = runner.run( TestJob(key="agent_precommit", command=command, timeout_sec=float(pc.get("timeout_sec") or 600), min_gpus=0, gpu_lock=False), - _target_test_env(ctx, manifest)) + _target_test_env(settings, manifest)) return {"exit_code": outcome.rc, "passed": outcome.rc == 0, "log_file": outcome.log_file} from ...memory.paths import KnowledgePaths kpaths = KnowledgePaths.resolve( - ctx.settings, repo_name, - adapter_root=Path(ctx.settings.adapters_dir) + settings, repo_name, + adapter_root=Path(settings.adapters_dir) / repo_name.replace("-", "_")) def _memory() -> DebugMemory: @@ -308,7 +317,7 @@ def _memory() -> DebugMemory: # kill a run the gate already provenance-stamped. from ...adapters.base import expand_path _knowledge_cfg = (manifest.get("rebase") or {}).get("knowledge") or {} - _kn_extra = ctx.settings.expansion_env() + _kn_extra = settings.expansion_env() parent_db_path = expand_path(str(_knowledge_cfg.get("parent_debug_db") or ""), extra=_kn_extra) parent_skills_path = expand_path( @@ -325,7 +334,7 @@ def _parent_memory_hits(query: str, k: int) -> list[dict]: upstream_column=str(_knowledge_cfg.get( "parent_upstream_column") or "")).search(query, k=k) except Exception as exc: # noqa: BLE001 — degrade open, traced - ctx.trace.record("capability_note", + trace.record("capability_note", capability="rebase.knowledge.parent_debug_db", detail=f"parent layer degraded mid-run: {exc}") return [] @@ -375,7 +384,7 @@ def search_debug_memory(**kw) -> dict: def record_debug_memory(**kw) -> dict: try: - run_id = str(ctx.state.get("run_id", "")) + run_id = str(state.get("run_id", "")) # additive v2 fields land in their OWN columns (round-4 F4 — # the old key-inside-verification packing lost every one of # them to curation and migration); v3_knowledge_prep @@ -393,7 +402,7 @@ def record_debug_memory(**kw) -> dict: tags=kw.get("tags", ""), watch_outs=str(kw.get("watch_outs", "") or ""), upstream_commit=str( - ctx.state.get("upstream_commit", "") or ""), + state.get("upstream_commit", "") or ""), last_seen_run=run_id, source="v3-agent") return {"ok": True, "id": entry_id} @@ -429,7 +438,7 @@ def search_skills(**kw) -> dict: found = store.find(query=query, module=module_q, k=len(store.load_all()) or 1) except Exception as exc: # noqa: BLE001 — degrade open, traced - ctx.trace.record( + trace.record( "capability_note", capability="rebase.knowledge.parent_skills_dir", detail=f"skill layer {store_dir} degraded: {exc}") @@ -466,6 +475,19 @@ def skill_manage(**kw) -> dict: run_precommit=run_precommit) + + +def _build_backends(ctx: StepContext, manifest: dict, repo: str, target): + """In-process adapter: the same production backends, built from a + `StepContext`. Kept so existing callers are untouched.""" + return build_backends( + settings=ctx.settings, state=ctx.state, run_dir=ctx.run_dir, + trace=ctx.trace, manifest=manifest, repo=repo, + model=target.model, + base_url=getattr(target, "base_url", "") or "", + api_key=getattr(target, "api_key", "") or "") + + def _module_scope(repo_root: str, module: str, manifest: dict, run_dir: Path | None = None) -> ToolScope: """C5 path governance for one module agent: the repo tree (plus the @@ -1306,7 +1328,7 @@ def run_fn(slug: str) -> tl.TestRunResult: # TARGET venv + CUDA + HF_HOME overlay — raw manifest commands # must resolve inside the target repo's runtime, never ours return _to_result(runner.run(_job(slug), - _target_test_env(ctx, manifest))) + _target_test_env(ctx.settings, manifest))) worktree_path = ctx.run_dir / "main_worktree" @@ -1326,7 +1348,7 @@ def baseline_fn(slug: str) -> tl.TestRunResult | None: # baseline PYTHONPATH override) — same TARGET env otherwise; infra # outcomes propagate (a baseline timeout must never read as "fails # on main too") - env = _target_test_env(ctx, manifest, pythonpath_prepend=str(wt)) + env = _target_test_env(ctx.settings, manifest, pythonpath_prepend=str(wt)) return _to_result(wt_runner.run(_job(slug), env, baseline=True)) async def debug_fn(slug: str, label: str, rc: int, @@ -1465,7 +1487,7 @@ def _precommit_baseline(ctx: StepContext, repo: str, pc: dict, job = TestJob(key="__precommit_baseline__", command=command, timeout_sec=float(pc.get("timeout_sec") or 600), min_gpus=0, gpu_lock=False) - outcome = runner.run(job, _target_test_env(ctx, manifest)) + outcome = runner.run(job, _target_test_env(ctx.settings, manifest)) if outcome.timed_out: # a timed-out probe proves nothing about the baseline - treat # it as unavailable so the original red stays STRUCTURAL @@ -1524,14 +1546,14 @@ async def _v3_precommit(ctx: StepContext) -> StepResult: job = TestJob(key="__precommit__", command=command, timeout_sec=float(pc.get("timeout_sec") or 600), min_gpus=0, gpu_lock=False) - outcome = runner.run(job, _target_test_env(ctx, manifest)) + outcome = runner.run(job, _target_test_env(ctx.settings, manifest)) attempt = 0 if outcome.rc != 0 and pc.get("retry_once", True): # parity: many hooks fix files in place; a second run then passes. # NO `git add -A` here (parent-documented: indiscriminate staging is # how stray artifacts ended up in rebase commits) attempt = 1 - outcome = runner.run(job, _target_test_env(ctx, manifest)) + outcome = runner.run(job, _target_test_env(ctx.settings, manifest)) passed = outcome.rc == 0 and not outcome.timed_out result = "passed" if passed else "failed" baseline_rc = None @@ -1735,7 +1757,18 @@ async def _v3_module_rebase(ctx: StepContext) -> StepResult: hf_home=os.environ.get("HF_HOME", "/model"), model_aliases=ctx.settings.model_aliases, model_mismatch_policy=ctx.settings.model_mismatch_policy, - baseline_ref=_baseline_ref(manifest)) + baseline_ref=_baseline_ref(manifest), + # harness delegation: "api" (default) keeps the in-process loop + backend=getattr(ctx.settings, "rebase_backend", "api") or "api", + backend_model=getattr(ctx.settings, "rebase_backend_model", "") or "", + settings=ctx.settings, + manifest_path=str(adapter_dir / "manifest.yaml"), + paths_spec={k: val for k, val in _dc_asdict(paths).items() + if k != "env"}, + repo=str((ctx.state.get("task_spec") or {}).get("repo") or ""), + state_slice={"task_spec": ctx.state.get("task_spec") or {}, + "run_id": ctx.state.get("run_id", ""), + "upstream_commit": ctx.state.get("upstream_commit", "")}) async with _serial_lock(ctx.run_dir): outcome = await rebase_module( module, client=client, config=config, @@ -2026,7 +2059,7 @@ def _verify_locally(slug: str) -> str: .get("artifact_globs") or []), cuda_visible_devices=os.environ.get("CUDA_VISIBLE_DEVICES", "")) outcome = runner.run(manifest_job_to_test_job(job), - _target_test_env(ctx, manifest)) + _target_test_env(ctx.settings, manifest)) if outcome.timed_out or outcome.watchdog_triggered: return "failed" if outcome.skipped: diff --git a/src/infermatrix_copilot/providers/cursor.py b/src/infermatrix_copilot/providers/cursor.py index 66fd7f48..402b302a 100644 --- a/src/infermatrix_copilot/providers/cursor.py +++ b/src/infermatrix_copilot/providers/cursor.py @@ -118,6 +118,19 @@ def _usage(events: list[dict]) -> SessionUsage: return usage # -- MCP bridge wiring --------------------------------------------------- + @staticmethod + def _is_our_stale_config(config: Path) -> bool: + """True when an existing mcp.json is one WE wrote (it launches our + tool bridge) and is therefore safe to replace.""" + try: + existing = json.loads(config.read_text(encoding="utf-8")) + entry = (existing.get("mcpServers") or {}).get( + "infermatrix-tools") or {} + except (OSError, ValueError, AttributeError): + return False + return "infermatrix_copilot.tool_bridge" in " ".join( + str(a) for a in (entry.get("args") or ())) + def _write_mcp_config(self, cwd: Path, spec_path: Path) -> list[Path]: """Project-scope `.cursor/mcp.json` in the session cwd pointing at the tool bridge. Returns the paths WE created (and only those) so the @@ -131,7 +144,13 @@ def _write_mcp_config(self, cwd: Path, spec_path: Path) -> list[Path]: cursor_dir.mkdir() created.append(cursor_dir) config = cursor_dir / "mcp.json" - if config.exists(): # never clobber a repo-committed config + if config.exists() and not self._is_our_stale_config(config): + # Never clobber a REPO-COMMITTED config. A config WE wrote and + # failed to clean up (a killed session skips the `finally`) is a + # different case and must be replaced: leaving it silently binds + # this session to a DEAD run's spec -- wrong ToolScope, wrong + # plan-gate prefix, and tool events appended to the old run's + # bridge trace. Observed 2026-09-18, 27 minutes of a module run. return created package_root = Path(__file__).resolve().parents[2] config.write_text(json.dumps({"mcpServers": {"infermatrix-tools": { diff --git a/src/infermatrix_copilot/rebase_engine/module_rebase.py b/src/infermatrix_copilot/rebase_engine/module_rebase.py index 4e23f2a1..e19508be 100644 --- a/src/infermatrix_copilot/rebase_engine/module_rebase.py +++ b/src/infermatrix_copilot/rebase_engine/module_rebase.py @@ -10,13 +10,13 @@ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from typing import Any, Mapping from ..run_trace import RunTrace from ..scopes import ToolScope -from .agent_loop import run_agent_loop +from .agent_loop import GATED_TOOL_NAMES, run_agent_loop from .hooks import RebaseHooks from .prompt_builder import (ModulePromptData, build_debug_prompt, build_module_prompt) @@ -37,6 +37,21 @@ class ModuleRunConfig: cuda_devices: str = "0,1" hf_home: str = "/model" max_turns: int = 150 + # Harness backend selection (doc/features/provider-registry.md). "api" + # keeps the in-process Anthropic tool-use loop; any other provider id + # delegates the whole module step to that harness, with the SAME 20-tool + # surface served through the MCP tool bridge. + backend: str = "api" + backend_model: str = "" # model INSIDE the harness; see config + settings: Any = None # Settings — transport construction + manifest_path: str = "" # adapter manifest, rebuilt inside the bridge + paths_spec: Mapping = field(default_factory=dict) # serialized RebasePaths + state_slice: Mapping = field(default_factory=dict) # run state the backends read + repo: str = "" # repo name recorded in the bridge spec + # Harness session bound. `max_iters` maps to a native turn cap only where + # the harness HAS one (claude --max-turns); cursor/codex have none, so the + # real bound there is this timeout plus the prompt's budget discipline. + harness_timeout_s: float = 7200.0 max_debug_retries: int = 3 plan_review_max_rounds: int = 2 model_aliases: Mapping[str, str] | None = None @@ -46,6 +61,168 @@ class ModuleRunConfig: baseline_ref: str = "origin/main" +def _plan_gate_opened(run_dir: Path) -> bool: + """Read `plan_done` back out of the bridge trace. + + The gate runs in the bridge process, so the parent cannot observe it + directly; `PlanGate` records `plan_gate_opened` when a decision file is + successfully written. Absent/unreadable trace ⇒ NOT opened (the same + fail-closed default the in-process loop starts from).""" + import json as _json + tp = Path(run_dir) / "bridge_trace.jsonl" + try: + for line in tp.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if not line: + continue + try: + if _json.loads(line).get("kind") == "plan_gate_opened": + return True + except ValueError: + continue + except OSError: + return False + return False + + +def _changed_files(root: str) -> dict[str, int]: + """{path: mtime_ns} for every file git reports as changed under `root`.""" + import subprocess + try: + proc = subprocess.run(["git", "-C", root, "status", "--porcelain"], + capture_output=True, text=True, timeout=120, + check=False) + except (OSError, subprocess.SubprocessError): + return {} + out: dict[str, int] = {} + for line in (proc.stdout or "").splitlines(): + rel = line[3:].strip().strip('"') + if " -> " in rel: # rename: take the destination + rel = rel.split(" -> ", 1)[1] + fp = Path(root) / rel + try: + out[str(fp)] = fp.stat().st_mtime_ns + except OSError: + continue + return out + + +def _native_writes(root: str, run_dir: Path, before: dict[str, int], + since_ts: float) -> tuple[list[str], list[str]]: + """Files the harness wrote WITHOUT going through the bridge. + + A harness that keeps its own built-in file tools (cursor has no + `builtin_tools_off`) can edit the checkout without touching the bridge, + which means those writes miss BOTH the scope guard's out-of-scope + recording and the plan gate. Sandboxing would prevent it, but needs user + namespaces; where that is unavailable, detection is the honest ceiling. + + Returns (native, pre_gate): every natively-written file, and the subset + that landed BEFORE the plan gate opened — the ones that broke the + contract rather than merely bypassing the bookkeeping. + """ + import json as _json + + bridged: set[str] = set() + gate_ts: float | None = None + tp = Path(run_dir) / "bridge_trace.jsonl" + try: + for line in tp.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if not line: + continue + try: + d = _json.loads(line) + except ValueError: + continue + if d.get("ts", 0) < since_ts: + continue + if d.get("kind") == "plan_gate_opened" and gate_ts is None: + gate_ts = float(d.get("ts") or 0) or None + if d.get("kind") == "tool_call" and d.get("path"): + bridged.add(str(d["path"])) + except OSError: + pass + + native, pre_gate = [], [] + for path, mtime_ns in _changed_files(root).items(): + if path in bridged or before.get(path) == mtime_ns: + continue + native.append(path) + if gate_ts is not None and mtime_ns / 1e9 < gate_ts: + pre_gate.append(path) + return sorted(native), sorted(pre_gate) + + +async def _harness_attempt(prompt: str, *, module: str, config, + scope: ToolScope, trace: RunTrace, + tool_defs: list[dict], plan_prefix: str, + require_plan_review: bool) -> dict: + """Delegate one module attempt to a harness backend. + + The harness runs its OWN loop, so the pieces `run_agent_loop` owns + in-process move into the bridge: the 20-tool surface (rebuilt there from + the spec) and the plan gate (enforced at dispatch, which a harness cannot + bypass). Returns the same dict shape the in-process attempt does so + `rebase_module`'s retry/debug logic is untouched. + """ + import asyncio + + from ..providers import AgentSessionRequest + from ..providers.registry import transport_for_id + from ..tool_bridge import write_bridge_spec + + run_dir = Path(config.log_dir) + transport = transport_for_id(config.settings, config.backend) + spec_path = write_bridge_spec( + run_dir=run_dir, step_name=f"rebase.module.{module}", scope=scope, + repo=config.repo, + rebase={ + "tool_schemas": str(Path(config.script_dir) / "tool_schemas.json"), + "manifest_path": config.manifest_path, + "model": config.model, + # prebuilt upstream: serializing the checkout paths HERE would + # add repo-specific vocabulary to a neutral core module + "paths": dict(config.paths_spec or {}), + "state": dict(config.state_slice or {}), + "plan_write_prefix": plan_prefix if require_plan_review else "", + "gated_tools": list(GATED_TOOL_NAMES), + }) + # NEVER forward the tier model: it names a raw-API model the harness + # does not have. Empty lets the transport fall back to its own setting. + req = AgentSessionRequest( + system=prompt, prompt="", scope=scope, + model=config.backend_model or "", + max_iters=config.max_turns, timeout_s=config.harness_timeout_s, + run_dir=run_dir, step_name=f"rebase.module.{module}", + bridge_spec_path=spec_path, trace=trace) + import time as _time + + before = _changed_files(scope.root) + started = _time.time() + outcome = await asyncio.to_thread(transport.run_session, req) + + native, pre_gate = _native_writes(scope.root, run_dir, before, started) + if native and trace is not None: + trace.record("harness_native_writes", step=f"rebase.module.{module}", + count=len(native), pre_gate=len(pre_gate), + files=[str(f) for f in native[:20]], + pre_gate_files=[str(f) for f in pre_gate[:20]]) + if pre_gate: + # product code changed before a decision existed: the plan gate's + # whole contract. Fail the module rather than let the wave gate + # accept work the contract never covered. + return {"done": False, "turns": 0, "plan_done": False, + "text": ("harness wrote product files BEFORE the plan-review " + "decision, bypassing the bridge: " + + ", ".join(pre_gate[:10]))} + return {"done": not getattr(outcome, "truncated", False), + "text": getattr(outcome, "text", "") or "", + "turns": int(getattr(outcome, "iterations", 0) or 0), + "plan_done": (not require_plan_review + or _plan_gate_opened(run_dir))} + + async def rebase_module( module: str, *, @@ -91,6 +268,11 @@ async def rebase_module( async def _attempt(p: str, *, require_plan_review: bool = True) -> dict: try: + if config.backend and config.backend != "api": + return await _harness_attempt( + p, module=module, config=config, scope=scope, trace=trace, + tool_defs=tool_defs, plan_prefix=plan_prefix, + require_plan_review=require_plan_review) return await run_agent_loop( client, p, model=config.model, tool_defs=tool_defs, extra_tools=extra_tools, scope=scope, trace=trace, diff --git a/src/infermatrix_copilot/tool_bridge.py b/src/infermatrix_copilot/tool_bridge.py index 54a8fffb..5f7b4229 100644 --- a/src/infermatrix_copilot/tool_bridge.py +++ b/src/infermatrix_copilot/tool_bridge.py @@ -41,12 +41,12 @@ from .scopes import PathScope, ToolScope from .tools import _PATH_ARGS, TOOLS, dispatch -_SPEC_VERSION = 1 +_SPEC_VERSION = 2 # -- spec serialization ------------------------------------------------------ def write_bridge_spec(*, run_dir: Path, step_name: str, scope: ToolScope, - repo: str) -> Path: + repo: str, rebase: dict | None = None) -> Path: """Serialize one step's tool surface under ``/bridge/``. The filename is sanitized from the step name (ensemble steps carry ``#``).""" bridge_dir = Path(run_dir) / "bridge" @@ -67,6 +67,11 @@ def write_bridge_spec(*, run_dir: Path, step_name: str, scope: ToolScope, "repo": repo, "run_dir": str(run_dir), "trace_path": str(Path(run_dir) / "bridge_trace.jsonl"), + # Optional rebase surface (see `_rebase_extra`). Carries PATHS and + # model identity only: no api_key and no child env, so nothing + # secret lands in this file — the bridge process reads credentials + # from its own environment. + **({"rebase": rebase} if rebase else {}), }, indent=2), encoding="utf-8") return path @@ -89,8 +94,171 @@ def load_bridge_spec(path: Path) -> tuple[ToolScope, dict]: return scope, data +# -- rebase surface ---------------------------------------------------------- +# The rebase module agent runs with 20 adapter-declared tools, not the 6 in the +# shared registry. When a spec carries a "rebase" section the bridge rebuilds +# that surface IN THIS PROCESS — the same `build_rebase_tools` the in-process +# loop uses, over production backends reconstructed from serializable inputs — +# and passes them to `dispatch` as `extra`, so scope guards, out-of-scope +# recording and result bounds stay identical. +# +# Containment caveat, disclosed: `_PATH_ARGS` only knows the shared tools, so +# the read-containment pre-check is a no-op for adapter tools that take paths. +# Writes remain guarded by `ToolScope` inside `dispatch`; reads through +# adapter tools are bounded by the handlers themselves. + + +class PlanGate: + """Bridge-side mirror of `agent_loop`'s plan gate. + + The in-process loop withholds gated tools until a decision file exists; + a harness runs its own loop, so without this the gate would simply not + exist for harness backends. Enforcing it at dispatch is STRONGER than + advertisement-based gating: the harness cannot call what it was never + offered, and cannot bypass by calling it anyway. + + Opens on the same event as the parent: a SUCCESSFUL `write_file` whose + path contains `.decision.md`. A refused or error-carrying write proves + nothing was decided and leaves the gate shut. + """ + + def __init__(self, plan_prefix: str, gated: tuple[str, ...], + trace: RunTrace | None = None): + self.plan_prefix = plan_prefix + self.gated = tuple(gated) + self.trace = trace + self.open = False + + def refusal(self, name: str, args: dict) -> str | None: + """The refusal text for a call that must not run yet, else None.""" + if self.open: + return None + from .rebase_engine.agent_loop import _under_plan_dir + locked_write = (name == "write_file" + and not _under_plan_dir( + str(args.get("file_path", "")), self.plan_prefix)) + if name in self.gated or locked_write: + what = ("write_file outside the plan directory" + if locked_write else name) + extra = (f" Write plan/decision files under {self.plan_prefix}" + if locked_write else "") + return (f"{what} is locked until the plan-review decision file " + f"(.decision.md) is written.{extra}") + return None + + def observe(self, name: str, args: dict, result: str) -> None: + """Open the gate on a successful decision-file write.""" + if self.open or name != "write_file": + return + if ".decision.md" not in str(args.get("file_path", "")): + return + try: + self.open = "error" not in json.loads(result) + except (TypeError, ValueError): + self.open = False + if self.open and self.trace is not None: + # the parent process reads this back to learn `plan_done` + self.trace.record("plan_gate_opened", + decision=str(args.get("file_path", ""))) + + +def _rebase_extra(spec: dict, trace: RunTrace, scope: ToolScope) -> dict: + """Rebuild the adapter's rebase tool pack in the bridge process. + + Credentials are NOT in the spec: `api_key` comes from this process's + settings, and the child env for shell/pytest handlers is this process's + own environment (inherited from the harness), so no secret is written to + `run_dir/bridge/`. + """ + from .config import Settings + from .engine.steps.rebase_v3 import build_backends + from .rebase_engine.rebase_tools import (RebasePaths, build_rebase_tools, + load_tool_schemas) + rb = spec["rebase"] + settings = Settings() + # generic reconstruction: the spec's payload IS the dataclass's fields, + # so naming them here would put repo vocabulary in a neutral module + payload = dict(rb.get("paths") or {}) + if "test_roots" in payload: + payload["test_roots"] = tuple(payload["test_roots"]) + paths = RebasePaths(env=dict(os.environ), **payload) + manifest: dict = {} + mpath = rb.get("manifest_path", "") + if mpath and Path(mpath).is_file(): + import yaml + manifest = yaml.safe_load(Path(mpath).read_text(encoding="utf-8")) or {} + # `repo` here is the repo ROOT PATH — build_backends feeds it to + # TestRunner(repo_root=...). The spec's "repo" is the repo NAME and + # belongs in `state.task_spec.repo`; passing the name here made every + # TestRunner-backed tool (run_pytest/run_precommit/reproduce) die with + # FileNotFoundError on a relative path. + state = dict(rb.get("state") or {}) + state.setdefault("task_spec", {"repo": spec.get("repo", "")}) + backends = build_backends( + settings=settings, state=state, + run_dir=Path(spec["run_dir"]), trace=trace, manifest=manifest, + # the module scope's root IS the repo root (see `_module_scope`); + # no repo-specific fallback, so a missing root fails loudly + repo=scope.root, + model=rb.get("model", ""), + base_url=rb.get("base_url", ""), + api_key=getattr(settings, "anthropic_api_key", "") or "", + ) + defs = load_tool_schemas(Path(rb["tool_schemas"])) + return build_rebase_tools(defs, paths, backends), defs + + +def _fn_from_schema(name: str, schema: dict, call): + """A real-signature function for FastMCP, generated from the tool's JSON + schema — `add_tool` derives its input schema by inspecting the signature, + so a `**kwargs` shim would advertise no parameters at all.""" + types = {"string": "str", "integer": "int", "number": "float", + "boolean": "bool", "array": "list", "object": "dict"} + props = (schema.get("properties") or {}) + required = set(schema.get("required") or ()) + params, names, omit_if_none = [], [], [] + for pname, pspec in props.items(): + if not pname.isidentifier(): + continue + ann = types.get((pspec or {}).get("type"), "str") + names.append(pname) + if pname in required: + params.append(f"{pname}: {ann}") + else: + default = (pspec or {}).get("default") + if default is not None: + params.append(f"{pname}: {ann} = {default!r}") + else: + # no schema default: sentinel None, and DROP it below rather + # than forwarding None into a handler that expects its own + # Python default (read_file's `offset` did `None + int`) + params.append(f"{pname}: {ann} | None = None") + omit_if_none.append(pname) + # KEYWORD-ONLY: a schema may interleave required and optional properties + # (record_debug_memory does), which as positionals is a SyntaxError — + # "parameter without a default follows parameter with a default". MCP + # calls tools by name, so keyword-only costs nothing and keeps the + # schema's own property order. + # a no-parameter tool (git_diff_tests_upstream) must not emit a bare "*" + sig = f"*, {', '.join(params)}" if params else "" + body_args = "{" + ", ".join(f"{n!r}: {n}" for n in names) + "}" + drop = "{" + ", ".join(repr(n) for n in omit_if_none) + "}" + src = (f"def _tool({sig}) -> str:\n" + f" _a = {body_args}\n" + f" _drop = {drop}\n" + " _a = {k: v for k, v in _a.items()" + " if not (k in _drop and v is None)}\n" + f" return _call({name!r}, _a)\n") + ns: dict = {"_call": call} + exec(compile(src, f"", "exec"), ns) # noqa: S102 + fn = ns["_tool"] + fn.__name__ = name + return fn + + # -- server ------------------------------------------------------------------ -def make_dispatcher(scope: ToolScope, roots: tuple[str, ...], trace: RunTrace): +def make_dispatcher(scope: ToolScope, roots: tuple[str, ...], trace: RunTrace, + extra: dict | None = None, gate: "PlanGate | None" = None): """The bridge's call path: read containment (stronger than ToolScope, which path-guards only writes — see module docstring) and then the same `tools.dispatch` the in-process loop uses. Errors raise so the MCP layer @@ -112,10 +280,24 @@ def _call(name: str, args: dict) -> str: raise RuntimeError( f"refused: {name} target is outside this session's " "worktree/run dir") - out = dispatch(name, args, scope=scope, trace=trace) + if gate is not None: + refusal = gate.refusal(name, args) + if refusal: + trace.record("tool_refused", tool=name, reason=refusal) + raise RuntimeError(f"refused: {refusal}") + out = dispatch(name, args, scope=scope, trace=trace, extra=extra) if not out["ok"]: - raise RuntimeError(str(out.get("error") or "tool error")) - return str(out["result"]) + # `dispatch` traces the call with ok=False but not WHY. Without + # the reason, "agent read a path that does not exist" and "this + # tool is broken" look identical in bridge_trace.jsonl and have + # to be reproduced by hand to tell apart. + err = str(out.get("error") or "tool error") + trace.record("tool_error", tool=name, error=err[:500]) + raise RuntimeError(err) + result = str(out["result"]) + if gate is not None: + gate.observe(name, args, result) + return result return _call @@ -128,10 +310,35 @@ def build_server(spec_path: Path): scope, spec = load_bridge_spec(spec_path) trace = RunTrace(Path(spec["trace_path"])) roots = tuple(r for r in (scope.root, spec.get("run_dir", "")) if r) - _call = make_dispatcher(scope, roots, trace) + + rebase_extra: dict = {} + rebase_defs: list[dict] = [] + gate = None + if spec.get("rebase"): + rb = spec["rebase"] + rebase_extra, rebase_defs = _rebase_extra(spec, trace, scope) + if rb.get("plan_write_prefix"): + gate = PlanGate(rb["plan_write_prefix"], + tuple(rb.get("gated_tools") + or ("edit_file", "run_pytest", + "run_precommit")), + trace=trace) + _call = make_dispatcher(scope, roots, trace, + extra=rebase_extra or None, gate=gate) mcp = FastMCP("infermatrix-tool-bridge") - allowed = scope.allowed_tools & set(TOOLS) + # `dispatch` resolves `extra` BEFORE the builtin registry, so a name the + # rebase pack provides must be served by ITS handler here too — register + # the adapter version and drop the builtin, never both. + allowed = (scope.allowed_tools & set(TOOLS)) - set(rebase_extra) + + for _d in rebase_defs: + _name = _d.get("name", "") + if not _name or _name not in rebase_extra: + continue + mcp.add_tool(_fn_from_schema(_name, _d.get("input_schema") or {}, + _call), + name=_name, description=_d.get("description", "")) if "read_file" in allowed: @mcp.tool(description=TOOLS["read_file"].description) diff --git a/test/test_rebase_harness_backend.py b/test/test_rebase_harness_backend.py new file mode 100644 index 00000000..c0ae1b3b --- /dev/null +++ b/test/test_rebase_harness_backend.py @@ -0,0 +1,376 @@ +"""Rebase module agents over a harness backend (provider-registry M1). + +The in-process loop owns two things a harness cannot inherit: the 20-tool +adapter surface and the plan gate. These tests pin that both survive the move +into the bridge process, and that the gate's semantics are IDENTICAL to +`agent_loop`'s — refuse gated tools, confine writes to the plan dir, and open +only on a successful decision-file write. +""" +from __future__ import annotations + +import inspect +import json +from pathlib import Path + +import pytest + +from infermatrix_copilot.scopes import PathScope, ToolScope +from infermatrix_copilot.tool_bridge import (PlanGate, _fn_from_schema, + load_bridge_spec, make_dispatcher, + write_bridge_spec) +from infermatrix_copilot.tools import ToolDef + +GATED = ("edit_file", "run_pytest", "run_precommit") + + +def _scope(root): + return ToolScope(name="module-worker_runner", + allowed_tools=frozenset({"read_file", "write_file", + "edit_file"}), + path_scope=PathScope(writable=(f"{root}/*",), + primary=(f"{root}/*",)), + read_only=False, root=str(root)) + + +def _rebase_section(plan_prefix): + return {"tool_schemas": "/adapter/rebase/tool_schemas.json", + "manifest_path": "/adapter/manifest.yaml", + "model": "some-model", + "paths": {"omni_path": "/omni", "vllm_path": "/vllm"}, + "plan_write_prefix": str(plan_prefix), + "gated_tools": list(GATED)} + + +def test_spec_carries_rebase_section_and_no_credentials(tmp_path): + run_dir = tmp_path / "run" + run_dir.mkdir() + path = write_bridge_spec(run_dir=run_dir, step_name="rebase.module.x", + scope=_scope(tmp_path), repo="vllm-omni", + rebase=_rebase_section(tmp_path / "plans")) + _, raw = load_bridge_spec(path) + assert raw["rebase"]["paths"]["omni_path"] == "/omni" + # credentials and the child env stay in the bridge process's environment + blob = json.dumps(raw).lower() + assert "api_key" not in blob and "sk-" not in blob + + +def test_spec_without_rebase_section_is_unchanged(tmp_path): + run_dir = tmp_path / "run" + run_dir.mkdir() + path = write_bridge_spec(run_dir=run_dir, step_name="agent.review", + scope=_scope(tmp_path), repo="vllm-omni") + _, raw = load_bridge_spec(path) + assert "rebase" not in raw + + +@pytest.mark.parametrize("name", GATED) +def test_gate_refuses_every_gated_tool_until_decision(name): + gate = PlanGate("/plans", GATED) + assert gate.refusal(name, {}) is not None + gate.observe("write_file", {"file_path": "/plans/p.decision.md"}, + json.dumps({"ok": True})) + assert gate.open is True + assert gate.refusal(name, {}) is None + + +def test_gate_confines_writes_to_plan_dir_while_closed(): + gate = PlanGate("/plans", GATED) + assert gate.refusal("write_file", {"file_path": "/repo/prod.py"}) is not None + assert gate.refusal("write_file", + {"file_path": "/plans/p.decision.md"}) is None + # traversal out of the plan dir must not open a back door + assert gate.refusal("write_file", + {"file_path": "/plans/../repo/prod.py"}) is not None + + +def test_failed_decision_write_leaves_gate_shut(): + gate = PlanGate("/plans", GATED) + gate.observe("write_file", {"file_path": "/plans/p.decision.md"}, + json.dumps({"error": "disk full"})) + assert gate.open is False + assert gate.refusal("edit_file", {}) is not None + + +def test_gate_records_opening_for_the_parent(tmp_path): + from infermatrix_copilot.rebase_engine.module_rebase import _plan_gate_opened + from infermatrix_copilot.run_trace import RunTrace + + run_dir = tmp_path / "run" + run_dir.mkdir() + assert _plan_gate_opened(run_dir) is False + trace = RunTrace(run_dir / "bridge_trace.jsonl") + gate = PlanGate("/plans", GATED, trace=trace) + gate.observe("write_file", {"file_path": "/plans/p.decision.md"}, + json.dumps({"ok": True})) + assert _plan_gate_opened(run_dir) is True + + +def test_dispatcher_serves_extra_tools_and_enforces_gate(tmp_path): + from infermatrix_copilot.run_trace import RunTrace + + calls = [] + extra = {"reproduce": ToolDef( + name="reproduce", description="d", input_schema={}, + handler=lambda **kw: (calls.append(kw) or json.dumps({"ok": True})))} + trace = RunTrace(tmp_path / "t.jsonl") + gate = PlanGate(str(tmp_path / "plans"), GATED) + call = make_dispatcher(_scope(tmp_path), (str(tmp_path),), trace, + extra=extra, gate=gate) + + # an adapter tool that is NOT gated runs even before the decision + assert call("reproduce", {"x": 1}) + assert calls == [{"x": 1}] + # a gated one is refused at dispatch, not merely unadvertised + with pytest.raises(RuntimeError, match="locked until the plan-review"): + call("run_pytest", {}) + + +def test_generated_signature_matches_schema(): + fn = _fn_from_schema( + "run_shell", + {"type": "object", + "properties": {"command": {"type": "string"}, + "timeout": {"type": "integer", "default": 60}, + "workdir": {"type": "string"}}, + "required": ["command"]}, + lambda name, args: json.dumps({"name": name, "args": args})) + sig = inspect.signature(fn) + assert sig.parameters["command"].default is inspect.Parameter.empty + assert sig.parameters["timeout"].default == 60 + assert sig.parameters["workdir"].default is None + # keyword-only by construction (see the interleaved-schema test below) + assert json.loads(fn(command="ls"))["args"]["command"] == "ls" + + +def test_schema_may_interleave_required_and_optional(): + """`record_debug_memory` declares an optional property BEFORE a required + one. Emitted as positionals that is a SyntaxError, so the generated + parameters are keyword-only (MCP calls by name regardless).""" + fn = _fn_from_schema( + "record_debug_memory", + {"type": "object", + "properties": {"module": {"type": "string"}, + "root_cause": {"type": "string"}, + "fix": {"type": "string"}}, + "required": ["module", "fix"]}, + lambda name, args: json.dumps(args)) + got = json.loads(fn(module="worker_runner", fix="patched")) + assert got["module"] == "worker_runner" and got["fix"] == "patched" + assert all(p.kind is inspect.Parameter.KEYWORD_ONLY + for p in inspect.signature(fn).parameters.values()) + + +def test_tool_with_no_parameters_generates(): + """A zero-property schema (git_diff_tests_upstream) must not emit a bare + `*` — "named arguments must follow bare *".""" + fn = _fn_from_schema("git_diff_tests_upstream", + {"type": "object", "properties": {}}, + lambda name, args: name) + assert inspect.signature(fn).parameters == {} + assert fn() == "git_diff_tests_upstream" + + +def test_unset_optionals_are_omitted_not_passed_as_none(): + """A schema optional with no default must be DROPPED when unset, not + forwarded as None: the handler has its own Python default, and + `read_file` did `offset + int` on the None (caught in live smoke).""" + seen = {} + fn = _fn_from_schema( + "read_file", + {"type": "object", + "properties": {"file_path": {"type": "string"}, + "offset": {"type": "integer"}, + "limit": {"type": "integer", "default": 200}}, + "required": ["file_path"]}, + lambda name, args: seen.update(args) or "ok") + fn(file_path="x") + assert "offset" not in seen # unset, no schema default -> dropped + assert seen["limit"] == 200 # schema default -> forwarded + seen.clear() + fn(file_path="x", offset=5) + assert seen["offset"] == 5 # explicit value -> forwarded + + +def test_tier_model_is_never_forwarded_to_a_harness(monkeypatch): + """The tier model names a RAW-API model (e.g. deepseek-flash) that a + harness CLI does not have; forwarding it would override the harness's + own model selection with an invalid id.""" + import asyncio + import types as _types + + from infermatrix_copilot import tool_bridge + from infermatrix_copilot.providers import registry + from infermatrix_copilot.rebase_engine.module_rebase import _harness_attempt + + seen = {} + + class _T: + def run_session(self, req): + seen["model"] = req.model + return _types.SimpleNamespace(truncated=False, text="ok", + iterations=0) + + monkeypatch.setattr(registry, "transport_for_id", lambda *a, **k: _T()) + monkeypatch.setattr(tool_bridge, "write_bridge_spec", lambda **k: None) + + cfg = _types.SimpleNamespace( + log_dir="/tmp", backend="cursor", backend_model="", settings=None, + model="deepseek-flash", script_dir="/tmp", manifest_path="", + repo="r", max_turns=10, harness_timeout_s=1.0, paths_spec={}, + state_slice={}, baseline_ref="origin/main") + scope = ToolScope(name="s", allowed_tools=frozenset(), path_scope=None, + read_only=True, root="/tmp") + asyncio.run(_harness_attempt( + "p", module="m", config=cfg, scope=scope, trace=None, tool_defs=[], + plan_prefix="/tmp/plans", require_plan_review=False)) + + assert seen["model"] == "", seen # empty, NOT "deepseek-flash" + + +def test_bridge_passes_repo_ROOT_not_repo_name(tmp_path, monkeypatch): + """`build_backends(repo=...)` is a filesystem path — it becomes + TestRunner(repo_root=Path(repo)). The spec's "repo" is the repo NAME and + belongs in state.task_spec.repo. Passing the name made run_pytest, + run_precommit and reproduce die with FileNotFoundError('vllm-omni').""" + from infermatrix_copilot import tool_bridge + from infermatrix_copilot.run_trace import RunTrace + + run_dir = tmp_path / "run" + run_dir.mkdir() + root = tmp_path / "checkout" + root.mkdir() + seen = {} + + def _fake_build_backends(**kw): + seen.update(kw) + return object() + + monkeypatch.setattr(tool_bridge, "_rebase_extra", + tool_bridge._rebase_extra) # keep real + monkeypatch.setattr("infermatrix_copilot.engine.steps.rebase_v3." + "build_backends", _fake_build_backends) + monkeypatch.setattr(tool_bridge, "build_rebase_tools", + lambda defs, paths, backends: {}, raising=False) + + scope = ToolScope(name="m", allowed_tools=frozenset(), path_scope=None, + read_only=False, root=str(root)) + spec = {"run_dir": str(run_dir), "repo": "vllm-omni", + "rebase": {"tool_schemas": "", "manifest_path": "", "model": "m", + "paths": {"omni_path": str(root), + "vllm_path": str(root)}}} + try: + tool_bridge._rebase_extra(spec, RunTrace(run_dir / "t.jsonl"), scope) + except Exception: + pass # load_tool_schemas("") fails after build_backends is called + + assert seen.get("repo") == str(root), seen.get("repo") + assert seen["state"]["task_spec"]["repo"] == "vllm-omni" + + +def test_failed_tool_records_the_reason(tmp_path): + """A failed bridge call must trace WHY: without it a missing path and a + broken tool are indistinguishable in bridge_trace.jsonl.""" + from infermatrix_copilot.run_trace import RunTrace + + trace = RunTrace(tmp_path / "bridge_trace.jsonl") + extra = {"reproduce": ToolDef( + name="reproduce", description="d", input_schema={}, + handler=lambda **kw: (_ for _ in ()).throw(RuntimeError("boom")))} + call = make_dispatcher(_scope(tmp_path), (str(tmp_path),), trace, + extra=extra) + with pytest.raises(RuntimeError): + call("reproduce", {}) + kinds = [json.loads(l) for l in + (tmp_path / "bridge_trace.jsonl").read_text().splitlines() if l.strip()] + errs = [d for d in kinds if d.get("kind") == "tool_error"] + assert errs and "boom" in errs[0]["error"] + + +def test_native_writes_detected_and_pre_gate_ones_are_fatal(tmp_path, + monkeypatch): + """A harness keeping its built-in file tools can edit the checkout + without touching the bridge, missing BOTH the scope guard and the plan + gate. Where sandboxing is unavailable, detection is the ceiling: report + every native write, and treat a PRE-GATE one as a contract breach.""" + import time + + from infermatrix_copilot.rebase_engine import module_rebase as mr + + root = tmp_path / "repo" + run_dir = tmp_path / "run" + root.mkdir() + run_dir.mkdir() + now = time.time() + gate_ts = now - 5 + bridged = root / "via_bridge.py" + after = root / "native_after.py" + before_gate = root / "native_before.py" + + (run_dir / "bridge_trace.jsonl").write_text("\n".join(json.dumps(d) for d in [ + {"ts": gate_ts, "kind": "plan_gate_opened", "decision": "d"}, + {"ts": gate_ts + 1, "kind": "tool_call", "tool": "edit_file", + "path": str(bridged)}, + ])) + + # mtimes: bridged + one native AFTER the gate, one native BEFORE it + monkeypatch.setattr(mr, "_changed_files", lambda r: { + str(bridged): int((gate_ts + 1) * 1e9), + str(after): int((gate_ts + 2) * 1e9), + str(before_gate): int((gate_ts - 2) * 1e9), + }) + + native, pre_gate = mr._native_writes(str(root), run_dir, {}, now - 60) + assert str(bridged) not in native # went through the bridge + assert str(after) in native # native, but post-gate + assert str(before_gate) in native + assert pre_gate == [str(before_gate)] # only this one breaks the gate + + +def test_unchanged_files_are_not_reported_as_native(tmp_path, monkeypatch): + """A file already dirty before the session (same mtime) is not a write + by this harness.""" + import time + + from infermatrix_copilot.rebase_engine import module_rebase as mr + + root = tmp_path / "repo" + run_dir = tmp_path / "run" + root.mkdir() + run_dir.mkdir() + stale = root / "already_dirty.py" + (run_dir / "bridge_trace.jsonl").write_text("") + monkeypatch.setattr(mr, "_changed_files", lambda r: {str(stale): 777}) + + native, pre_gate = mr._native_writes(str(root), run_dir, + {str(stale): 777}, time.time() - 60) + assert native == [] and pre_gate == [] + + +def test_stale_self_written_mcp_config_is_replaced(tmp_path): + """A killed session skips the transport's `finally`, leaving OUR + .cursor/mcp.json behind. Honouring it binds the next session to a DEAD + run's spec — wrong scope, wrong plan-gate prefix, traces appended to the + old run. A repo-committed config must still be left alone.""" + from infermatrix_copilot.config import Settings + from infermatrix_copilot.providers.cursor import CursorTransport + + t = CursorTransport(Settings()) + cwd = tmp_path / "checkout" + (cwd / ".cursor").mkdir(parents=True) + config = cwd / ".cursor" / "mcp.json" + + # ours, pointing at a dead run + config.write_text(json.dumps({"mcpServers": {"infermatrix-tools": { + "command": "python", + "args": ["-m", "infermatrix_copilot.tool_bridge", "--spec", + "/runs/DEAD/bridge/old.json"]}}})) + assert t._is_our_stale_config(config) is True + t._write_mcp_config(cwd, Path("/runs/LIVE/bridge/new.json")) + assert "/runs/LIVE/bridge/new.json" in config.read_text() + + # someone else's config: untouched + foreign = json.dumps({"mcpServers": {"their-tool": {"command": "x"}}}) + config.write_text(foreign) + assert t._is_our_stale_config(config) is False + t._write_mcp_config(cwd, Path("/runs/LIVE/bridge/new.json")) + assert config.read_text() == foreign