From b76d88f857b1b5a6ac6527e9c75e85b9a5c86435 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Mon, 19 Oct 2026 14:15:31 -0700 Subject: [PATCH 01/33] feat(repo): skill and skill-catalog arms for repo run, cache tokens in results `repo run` arms now compose with `+` like the Q&A --arms: a guidance part (none/raw/bridge, raw by default) plus `skill:` (one skill dir) and `skill-catalog:` (every skill subdir), installed into the workspace at .claude/skills// in prep step 5 and listed with digests in meta.json. Relative paths are under eval_dir; `repo run` and `repo tasks check --arm` refuse missing skills and a catalog without a task's `expected_skill`. A cell with skills records which ones the agent used (Skill tool calls or reads of their SKILL.md) as skills.skill_invoked / skill_selected_correct; `repo report` counts them. Each cell also records cache read/creation input tokens, and the report gives per-row token means. Signed-off-by: Nikolay Petrov --- dokimi/docs/benchmarking-and-comparison.md | 27 ++++ dokimi/dokimi/commands/repo_run.py | 24 ++- dokimi/dokimi/repos/arms.py | 171 +++++++++++++++++++++ dokimi/dokimi/repos/events.py | 7 + dokimi/dokimi/repos/manifest.py | 3 + dokimi/dokimi/repos/report.py | 17 +- dokimi/dokimi/repos/runner.py | 17 +- dokimi/dokimi/repos/workspace.py | 50 +++++- dokimi/tests/repo_eval_fixture.py | 10 +- dokimi/tests/test_repo_skill_arms.py | 136 ++++++++++++++++ 10 files changed, 441 insertions(+), 21 deletions(-) create mode 100644 dokimi/dokimi/repos/arms.py create mode 100644 dokimi/tests/test_repo_skill_arms.py diff --git a/dokimi/docs/benchmarking-and-comparison.md b/dokimi/docs/benchmarking-and-comparison.md index 7930d226..c00fab87 100644 --- a/dokimi/docs/benchmarking-and-comparison.md +++ b/dokimi/docs/benchmarking-and-comparison.md @@ -274,6 +274,33 @@ python cli.py arms run \ The `baseline` arm is the control. Every other arm is compared against it. Keep the answer model and judge fixed when comparing arms. +### Skill arms in repository mode + +`repo run` arms take the same `+` composition. Besides `guidance:none`, +`guidance:raw` and `guidance:bridge`, an arm can install Claude Code skills into +the task workspace at `.claude/skills//` before the agent starts: + +- `skill:`: one skill directory (holding `SKILL.md`); +- `skill-catalog:`: every subdirectory of `` with a `SKILL.md`, so the + agent has to pick the right one. + +A relative path is relative to the manifest's `eval_dir`. An arm without a +guidance part keeps the repository's guidance (`guidance:raw`). + +```bash +dokimi repo run @ --model --model --batch skills \ + --arm guidance:raw,guidance:raw+skill:skills/lint,guidance:none+skill-catalog:skills +dokimi repo tasks check @ --arm skill-catalog:skills # paths exist, catalog has expected_skill +``` + +When a cell installs skills, its `result.json` gets a `skills` object: +`skills_used` (the skills the agent called with the `Skill` tool, or whose +`SKILL.md` it read), `skill_invoked`, and `skill_selected_correct` (the task's +optional `expected_skill` was the first skill it used). `repo report` counts +both per row. Every cell also records `cache_read_input_tokens` and +`cache_creation_input_tokens` with the input and output tokens, and +`repo report --json` gives their means. + ### Reading the significance tables Arms are compared **paired per question** against the one shared `baseline` diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index 3f20bb87..3f2cb048 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -125,13 +125,15 @@ def _errors(): from dokimi.repos import ManifestError, SourceError from dokimi.repos.container import ContainerError from dokimi.repos.harness_profile import ProfileError + from dokimi.repos.arms import ArmError from dokimi.repos.workspace import PrepError - return (SourceError, ManifestError, ContainerError, PrepError, ProfileError, ValueError, OSError) + return (ArmError, SourceError, ManifestError, ContainerError, PrepError, ProfileError, ValueError, OSError) def cmd_repo_tasks_check(args: argparse.Namespace) -> None: """Self-check (no LLM): null fails the gate, oracle passes it, the agent cannot see the tasks.""" + from dokimi.repos.arms import check_arms from dokimi.repos.selfcheck import run_check from dokimi.repos.workspace import load_tasks @@ -139,6 +141,10 @@ def cmd_repo_tasks_check(args: argparse.Namespace) -> None: try: ctx, _ = _context(args, out) tasks = load_tasks(ctx.content_dir, ctx.manifest, args.task) + arms = [a for spec in args.arm or () for a in spec.split(",") if a] + check_arms(arms, ctx.content_dir, tasks) # skill paths exist, catalogs hold each expected_skill + if arms: + print(f"{len(arms)} arm(s) ok: {', '.join(arms)}", file=sys.stderr) print(f"{len(tasks)} task(s), image {ctx.image}{' (warm: with network)' if args.warm else ''}", file=sys.stderr) results = run_check(ctx, tasks, out / "check", warm=args.warm, jobs=args.jobs) @@ -165,17 +171,18 @@ def _run_locked(args: argparse.Namespace, arms: list[str], batch_dir: Path, lock from dokimi.repos.runner import ( AgentOptions, agent_network, batch_lock, harness_version, run_batch, task_caps) from dokimi.repos.selfcheck import ANCESTOR_NAMES, isolation_probe - from dokimi.repos.workspace import ARMS, load_tasks + from dokimi.repos.arms import check_arms, parse_arm + from dokimi.repos.workspace import load_tasks try: - bad_arms = [a for a in arms if a not in ARMS] - if bad_arms: - raise ValueError(f"unknown arm(s) {', '.join(bad_arms)} (known: {', '.join(ARMS)})") + for arm in arms: + parse_arm(arm) profile = load_profile(args.harness) harness_bin = _harness_bin(args.bin or profile.bin) lock.enter_context(batch_lock(batch_dir)) # before _context, which rewrites _eval/content ctx, _ = _context(args, batch_dir, pin=True) tasks = load_tasks(ctx.content_dir, ctx.manifest, args.task) + check_arms(arms, ctx.content_dir, tasks) options = [AgentOptions(profile=profile, model=m, harness_bin=harness_bin, agent_network=args.agent_network, tools=args.tools, timeout_s=args.timeout, max_turns=args.max_turns, budget_usd=args.budget) for m in args.model] @@ -287,6 +294,9 @@ def register(repo_sub, positive_int, source_help: str, cache_help: str) -> None: _common(check_p, source_help, cache_help, positive_int) check_p.add_argument("--warm", action="store_true", help="Run null and oracle with network to fill the caches before offline runs") + check_p.add_argument("--arm", action="append", default=None, + help="Also check this `repo run` arm: its skill paths exist and a skill-catalog " + "holds each task's expected_skill; repeatable or comma-separated") check_p.add_argument("--out", default="repo-check", help="Work directory (default: repo-check)") check_p.set_defaults(func=cmd_repo_tasks_check) @@ -295,7 +305,9 @@ def register(repo_sub, positive_int, source_help: str, cache_help: str) -> None: "result.json per cell, already finished cells skipped") _common(run_p, source_help, cache_help, positive_int) run_p.add_argument("--arm", action="append", default=None, required=True, - help="guidance:none | guidance:raw | guidance:bridge; repeatable or comma-separated") + help="guidance:none | guidance:raw | guidance:bridge | skill: | skill-catalog:, " + "parts joined with + (guidance:none+skill:skills/x); a relative path is in eval_dir. " + "Repeatable or comma-separated") run_p.add_argument("--model", action="append", required=True, help="Model string passed to the harness as it is; repeatable") run_p.add_argument("--reps", type=positive_int, default=1) diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py new file mode 100644 index 00000000..2ee0c1d3 --- /dev/null +++ b/dokimi/dokimi/repos/arms.py @@ -0,0 +1,171 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Arms of ``repo run``: what step 5 of workspace preparation does to the workspace (contract §3). + +An arm is one or more parts joined with ``+``, as in the Q&A ``--arms`` +(:mod:`dokimi.treatments.specs`): + +- ``guidance:none`` | ``guidance:raw`` | ``guidance:bridge``: the repository's + own guidance files, removed, kept or bridged. At most one; ``guidance:raw`` + when the arm names none; +- ``skill:``: one Claude Code skill directory (it holds ``SKILL.md``), + installed at ``.claude/skills//``; +- ``skill-catalog:``: every subdirectory of ```` with a ``SKILL.md``, + each installed the same way; the agent has to pick the one the task needs. + +A relative skill path is relative to the manifest's ``eval_dir`` (the exported +``{eval_dir}`` on the host) and must stay inside it; an absolute path is a host +path. The arm string is recorded as it was written: ``guidance:raw+skill:x`` and +``skill:x`` are two arms with the same workspace. +""" + +from __future__ import annotations + +import hashlib +from dataclasses import dataclass +from pathlib import Path, PurePosixPath +from typing import Any + +GUIDANCE = ("guidance:none", "guidance:raw", "guidance:bridge") +SKILL_KINDS = ("skill", "skill-catalog") +SKILLS_DIR = ".claude/skills" +COMBINE = "+" + + +class ArmError(ValueError): + """An arm that does not parse, or whose skills cannot be found.""" + + +@dataclass(frozen=True) +class Skill: + """One skill to install: its directory name (the name Claude Code knows it by) and host directory.""" + + name: str + dir: Path + + @property + def target(self) -> str: + return f"{SKILLS_DIR}/{self.name}" + + +@dataclass(frozen=True) +class RepoArm: + spec: str + guidance: str + skills: tuple[tuple[str, str], ...] = () # (kind, path) as written + + @property + def catalog(self) -> bool: + return any(kind == "skill-catalog" for kind, _ in self.skills) + + +def parse_arm(spec: str) -> RepoArm: + """``spec`` as a :class:`RepoArm`. A ``+`` starts a new part only before a known kind, so a path may hold one.""" + parts: list[str] = [] + for piece in spec.split(COMBINE): + if parts and not _starts_kind(piece): + parts[-1] += COMBINE + piece + else: + parts.append(piece) + guidance, skills = [], [] + for part in parts: + if part in GUIDANCE: + guidance.append(part) + continue + kind, sep, path = part.partition(":") + if kind not in SKILL_KINDS or not sep: + raise ArmError(f"unknown arm {spec!r} (known: {', '.join(GUIDANCE)}, skill:, " + f"skill-catalog:, joined with {COMBINE!r})") + if not path: + raise ArmError(f"arm {spec!r}: {kind}: needs a path") + skills.append((kind, path)) + if len(guidance) > 1: + raise ArmError(f"arm {spec!r}: more than one guidance part") + return RepoArm(spec, guidance[0] if guidance else "guidance:raw", tuple(skills)) + + +def _starts_kind(text: str) -> bool: + return text in GUIDANCE or any(text.startswith(k + ":") for k in SKILL_KINDS) + + +def resolve_skills(arm: RepoArm, content_dir: Path) -> list[Skill]: + """The skills ``arm`` installs, found on the host; two with one name are refused.""" + found: list[Skill] = [] + for kind, path in arm.skills: + root = _skill_path(path, content_dir) + if kind == "skill": + if not (root / "SKILL.md").is_file(): + raise ArmError(f"arm {arm.spec!r}: no SKILL.md in {path}") + found.append(Skill(root.name, root)) + continue + if not root.is_dir(): + raise ArmError(f"arm {arm.spec!r}: skill-catalog {path} is not a directory") + subs = [Skill(d.name, d) for d in sorted(root.iterdir()) if d.is_dir() and (d / "SKILL.md").is_file()] + if not subs: + raise ArmError(f"arm {arm.spec!r}: no subdirectory of {path} holds a SKILL.md") + found += subs + names = [s.name for s in found] + dupes = sorted({n for n in names if names.count(n) > 1}) + if dupes: + raise ArmError(f"arm {arm.spec!r}: more than one skill named {', '.join(dupes)}") + return found + + +def _skill_path(path: str, content_dir: Path) -> Path: + if Path(path).is_absolute(): + return Path(path).resolve() + root = content_dir.resolve() + full = (root / PurePosixPath(path)).resolve() + if not full.is_relative_to(root): + raise ArmError(f"skill path {path} leaves eval_dir") + return full + + +def check_arms(arms: list[str], content_dir: Path, tasks: list[Any]) -> list[RepoArm]: + """Parse every arm and find its skills before anything runs; a catalog must hold each task's + ``expected_skill``. Returns the parsed arms.""" + parsed = [] + for spec in arms: + arm = parse_arm(spec) + names = {s.name for s in resolve_skills(arm, content_dir)} + if arm.catalog: + for task in tasks: + expected = task.raw.get("expected_skill") + if expected and expected not in names: + raise ArmError(f"arm {spec!r}: task {task.id}'s expected_skill {expected!r} is not installed " + f"(installed: {', '.join(sorted(names))})") + parsed.append(arm) + return parsed + + +def tree_sha256(root: Path) -> str: + """sha256 over the relative paths and bytes of the files under ``root``.""" + h = hashlib.sha256() + for path in sorted(p for p in root.rglob("*") if p.is_file()): + h.update(path.relative_to(root).as_posix().encode() + b"\0") + h.update(hashlib.sha256(path.read_bytes()).digest()) + return h.hexdigest() + + +def skill_usage(events: list[dict[str, Any]], installed: list[str], expected: str | None) -> dict[str, Any]: + """Which installed skills the agent used: a ``Skill`` tool call naming it, or a read of its ``SKILL.md``. + + ``skill_invoked``: the expected skill was used (any installed one when the task names none); + ``skill_selected_correct``: the expected skill was the first one used, ``None`` without one. + """ + used: list[str] = [] + for e in events: + name = None + if e.get("kind") == "tool" and e.get("name") == "Skill": + name = str(e.get("skill") or "").split(":")[-1] # a plugin skill is plugin:name + elif e.get("kind") == "read": + parts = PurePosixPath(str(e.get("path") or "")).parts + if len(parts) == 4 and "/".join(parts[:2]) == SKILLS_DIR and parts[3] == "SKILL.md": + name = parts[2] + if name in installed and name not in used: + used.append(name) + out: dict[str, Any] = {"skills_installed": installed, "skills_used": used, "expected_skill": expected, + "skill_invoked": (expected in used) if expected else bool(used), + "skill_selected_correct": (used[:1] == [expected]) if expected else None} + return out diff --git a/dokimi/dokimi/repos/events.py b/dokimi/dokimi/repos/events.py index 66efe462..114d5a06 100644 --- a/dokimi/dokimi/repos/events.py +++ b/dokimi/dokimi/repos/events.py @@ -9,6 +9,7 @@ {"t": 3.1, "kind": "read", "path": "AGENTS.md"} {"t": 5.0, "kind": "edit", "path": "src/foo.cpp"} {"t": 20.2, "kind": "tool", "name": "WebFetch", "denied": true} + {"t": 2.0, "kind": "tool", "name": "Skill", "skill": "lint-fix"} ``t`` is seconds since the first timestamped trace line, or ``null`` when the format has no timestamps. Paths inside the workspace are made relative to it. @@ -46,6 +47,8 @@ class Transcript: num_turns: int | None = None input_tokens: int | None = None output_tokens: int | None = None + cache_read_input_tokens: int | None = None + cache_creation_input_tokens: int | None = None is_error: bool = False harness_version: str | None = None result_subtype: str | None = None @@ -137,6 +140,8 @@ def _apply(out: Transcript, rec: dict[str, Any], t: float | None, usage = rec.get("usage") or {} out.input_tokens = usage.get("input_tokens") out.output_tokens = usage.get("output_tokens") + out.cache_read_input_tokens = usage.get("cache_read_input_tokens") + out.cache_creation_input_tokens = usage.get("cache_creation_input_tokens") for denial in rec.get("permission_denials") or []: if isinstance(denial, dict) and denial.get("tool_use_id") in by_id: by_id[denial["tool_use_id"]]["denied"] = True @@ -154,6 +159,8 @@ def _tool_event(block: dict[str, Any], t: float | None, workdir: str | None) -> event.update(kind="read" if name in _READ_TOOLS else "edit", path=_relative(str(path), workdir)) else: event.update(kind="tool", name=name) + if name == "Skill": # Claude Code names the skill in `skill`; older versions in `command` + event["skill"] = str(args.get("skill") or args.get("command") or "") return event diff --git a/dokimi/dokimi/repos/manifest.py b/dokimi/dokimi/repos/manifest.py index 7edce9ab..a0b19e68 100644 --- a/dokimi/dokimi/repos/manifest.py +++ b/dokimi/dokimi/repos/manifest.py @@ -472,6 +472,9 @@ def _check_task(tree: RepoTree, task_dir: str, report: RepoCheckReport) -> None: if not (isinstance(egress, list) and all(isinstance(h, str) and _HOST_RE.match(h) for h in egress)): problems.append("egress must be a list of hosts (DNS labels, `*` for one whole label, the last two literal, optional :port)") egress = [] + expected = raw.get("expected_skill") + if expected is not None and not (isinstance(expected, str) and expected and "/" not in expected): + problems.append("expected_skill must be a skill name (its directory name)") for key in ("timeout_s", "max_turns", "budget_usd"): value = raw.get(key) if value is not None and not _positive(value): diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index 19fc454c..3f824ef0 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -10,7 +10,12 @@ - **missing**: cells planned or started that have no ``result.json``. They never finished, so they are left out of the rate rather than counted as failures; rerun the batch to fill them. -- cost and turns are means over finished cells, as the harness reported them. +- cost, turns and tokens (``input_tokens``, ``output_tokens``, + ``cache_read_input_tokens``, ``cache_creation_input_tokens``) are means over + finished cells, as the harness reported them. +- skill arms add ``skill_invoked`` (cells whose agent used the expected skill, + or any installed one when the task names none) and, when the task has an + ``expected_skill``, ``skill_selected_correct`` (cells that used it first). """ from __future__ import annotations @@ -24,6 +29,8 @@ from .runner import slug +_TOKENS = ("input_tokens", "output_tokens", "cache_read_input_tokens", "cache_creation_input_tokens") + def load_cells(batch_dir: Path) -> tuple[list[dict[str, Any]], list[str]]: """Finished cells (their ``result.json``) and the names of missing ones.""" @@ -68,6 +75,7 @@ def summarize(batch_dir: Path) -> dict[str, Any]: "cost_usd": _mean([r["agent"]["cost_usd"] for r in recs]), "total_cost_usd": round(sum(r["agent"]["cost_usd"] or 0 for r in recs), 6), "turns": _mean([r["agent"]["num_turns"] for r in recs]), + **{k: _mean([r["agent"].get(k) for r in recs]) for k in _TOKENS}, "networks": sorted({str(r["agent"].get("network")) for r in recs})} if score_only: scores = [0.0 if r["grade"]["status"] == "grader_failure" else r["grade"]["score"] for r in recs] @@ -75,6 +83,11 @@ def summarize(batch_dir: Path) -> dict[str, Any]: else: wins = sum(bool(r["grade"]["success"]) for r in recs) row.update(kind="gate", successes=wins, success_rate=round(wins / n, 4), ci95=wilson(wins, n)) + used = [r["skills"] for r in recs if r.get("skills")] + if used: + row["skill_invoked"] = sum(bool(u["skill_invoked"]) for u in used) + if any(u.get("expected_skill") for u in used): + row["skill_selected_correct"] = sum(bool(u["skill_selected_correct"]) for u in used) rows.append(row) return {"batch": str(batch_dir), "cells": len(cells), "missing": missing, "rows": rows, "total_cost_usd": round(sum(r["agent"]["cost_usd"] or 0 for r in cells), 6), @@ -102,6 +115,8 @@ def format_report(summary: dict[str, Any]) -> str: f"{result:<22} {r['grader_failures']:>11} {r['missing']:>7} {cost:>8} {turns:>6}") noted = ("agent_timeouts", "agent_errors", "over_budget", "cost_unknown") notes = [f"{r[k]} {k.replace('_', ' ')}" for k in noted if r[k]] + notes += [f"{r[k]}/{r['n']} {k.replace('_', ' ')}" for k in ("skill_invoked", "skill_selected_correct") + if k in r] if notes: lines.append(" " * (width + 2) + " " + ", ".join(notes)) if summary["missing"]: diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index e74d734b..1b6b0f2c 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -55,6 +55,7 @@ Mount, docker_network, ) +from .arms import skill_usage from .events import convert from .grading import EvalContext, failed_outcome, outcome, run_entry from .harness_profile import HarnessProfile @@ -153,10 +154,11 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, if rd.exists(): _rmtree(rd) rd.mkdir(parents=True) - meta = prepare_workspace(ctx.git_dir, ctx.manifest, task, arm, rd, opts.profile) + meta = prepare_workspace(ctx.git_dir, ctx.manifest, task, arm, rd, opts.profile, ctx.content_dir) meta.update(harness=opts.profile.name, harness_version=opts.harness_version, model=opts.model, rep=rep, batch=batch_dir.name, agent_network=network, egress_proxy=False) _write_json(rd / "meta.json", meta) + skills: dict[str, Any] | None = None mounts, env = agent_layout(ctx, rd, task) bin_in = f"{HARNESS_DIR}/{opts.harness_bin.name}" @@ -182,7 +184,8 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, "rc": result.returncode, "timed_out": result.timed_out, "wall_s": result.wall_s, "cost_usd": None, # A killed harness reports no cost; the batch budget counts the cell's cap instead of $0. "charged_usd": caps["budget_usd"], "num_turns": None, "is_error": None, "result_subtype": None, - "input_tokens": None, "output_tokens": None, "plugins": [], "over_budget": False, "caps": caps, + "input_tokens": None, "output_tokens": None, + "cache_read_input_tokens": None, "cache_creation_input_tokens": None, "plugins": [], "over_budget": False, "caps": caps, "network": network, "trace_errors": []} try: raw = (rd / "agent.stdout").read_text(encoding="utf-8", errors="replace") @@ -197,8 +200,11 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, cost_usd=cost, charged_usd=cost if cost is not None else caps["budget_usd"], num_turns=transcript.num_turns, is_error=transcript.is_error, result_subtype=transcript.result_subtype, input_tokens=transcript.input_tokens, - output_tokens=transcript.output_tokens, plugins=transcript.plugins, + output_tokens=transcript.output_tokens, cache_read_input_tokens=transcript.cache_read_input_tokens, + cache_creation_input_tokens=transcript.cache_creation_input_tokens, plugins=transcript.plugins, over_budget=cost is not None and cost > caps["budget_usd"], trace_errors=transcript.errors[:5]) + if meta.get("skills"): + skills = skill_usage(transcript.events, [s["name"] for s in meta["skills"]], task.expected_skill) graded = run_entry(ctx, "grade", task, rd, grade_network, container_name(rd.name, "grade")) grade = outcome(task, rd, graded) except Exception as exc: # noqa: BLE001 # the grade run failed to start, or the host failed around it @@ -206,9 +212,12 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, # Excluding the run would favour arms that break workspaces (contract §4.2): it fails the task. what = "grade run failed" if isinstance(exc, ContainerError) else f"host error after the agent ran ({type(exc).__name__})" grade = failed_outcome(f"{what}: {str(exc)[:300]}") - record = {"task": task.id, "family": task.family, "arm": arm, "model": opts.model, "rep": rep, + record: dict[str, Any] = {"task": task.id, "family": task.family, "arm": arm, "model": opts.model, "rep": rep, "harness": opts.profile.name, "harness_version": meta["harness_version"], "base_date": meta["base_date"], "score_task": bool(task.score), "agent": agent, "grade": grade} + if meta.get("skills"): # without a trace the agent used none of them + record["skills"] = skills if skills is not None else skill_usage( + [], [s["name"] for s in meta["skills"]], task.expected_skill) try: _own_dir(rd) # the grader had the run dir read-write, and may have taken write access away _write_json(rd / "result.json", record) diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index 67c7b647..e289cdd9 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -11,7 +11,8 @@ 2. delete ``eval_dir`` 3. ``git init`` and commit (no history, so an upstream fix cannot leak) 4. apply ``setup_patches`` -5. apply the arm (``guidance:none`` | ``guidance:raw`` | ``guidance:bridge``) +5. apply the arm: its guidance part (``guidance:none`` | ``guidance:raw`` | + ``guidance:bridge``), then its skills into ``.claude/skills/`` (:mod:`.arms`) 6. amend the commit 7. apply ``review_patch`` with ``git am`` under a neutral author 8. expire the reflog and prune unreachable objects, so the commits steps 6 @@ -40,11 +41,12 @@ from dokimi import process +from .arms import GUIDANCE, ArmError, Skill, parse_arm, resolve_skills, tree_sha256 from .harness_profile import HarnessProfile from .manifest import RepoManifest, _is_within, match_glob from .source import GitTree, RepoSource, RepoTree, SourceError, cache_path, default_cache_dir, resolve_rev -ARMS = ("guidance:none", "guidance:raw", "guidance:bridge") +ARMS = GUIDANCE # the guidance parts; an arm may add skills to one (:func:`.arms.parse_arm`) SNAPSHOT_AUTHOR = ("dokimi", "dokimi@example.invalid") REVIEW_AUTHOR = ("Contributor", "contributor@example.invalid") FIXED_DATE = "2000-01-01T00:00:00+00:00" @@ -85,6 +87,10 @@ def review_patch(self) -> str | None: def egress(self) -> tuple[str, ...]: return tuple(self.raw.get("egress") or ()) + @property + def expected_skill(self) -> str | None: + return self.raw.get("expected_skill") or None + @property def score(self) -> dict[str, Any] | None: return self.raw.get("score") @@ -169,12 +175,17 @@ def _export(tree: RepoTree, dest: Path, only: str | None, exclude: str | None) - def prepare_workspace(git_dir: Path, manifest: RepoManifest, task: Task, arm: str, run_dir: Path, - profile: HarnessProfile | None = None) -> dict[str, Any]: - """Steps 1-7 into ``run_dir/workspace``; returns the ``meta.json`` fields prep owns.""" - if arm not in ARMS: - raise PrepError(f"unknown arm {arm!r} (known: {', '.join(ARMS)})") + profile: HarnessProfile | None = None, content_dir: Path | None = None) -> dict[str, Any]: + """Steps 1-7 into ``run_dir/workspace``; returns the ``meta.json`` fields prep owns. + + ``content_dir`` (the exported ``{eval_dir}``) is where relative skill paths of ``arm`` are found.""" + try: + parsed = parse_arm(arm) + skills = resolve_skills(parsed, content_dir or _content_dir(manifest, task)) if parsed.skills else [] + except ArmError as exc: + raise PrepError(str(exc)) from exc bridge = profile.bridge if profile is not None else None - if arm == "guidance:bridge" and not bridge: + if parsed.guidance == "guidance:bridge" and not bridge: name = profile.name if profile is not None else "no harness" raise PrepError(f"guidance:bridge needs a harness profile with a `bridge` key; {name} has none") try: @@ -201,7 +212,8 @@ def prepare_workspace(git_dir: Path, manifest: RepoManifest, task: Task, arm: st except PrepError as exc: raise PrepError(f"task {task.id}: setup patch {rel} does not apply: {exc}") from exc patches.append({"path": rel, "sha256": _sha256(path)}) - arm_files, bridge_skipped = _apply_arm(ws, env, manifest, arm, bridge) # 5 + arm_files, bridge_skipped = _apply_arm(ws, env, manifest, parsed.guidance, bridge) # 5 + installed = _install_skills(ws, task, skills) _git(ws, env, "add", "-A") # 6 _git(ws, env, "commit", "-q", "--no-gpg-sign", "--no-verify", "--amend", "--allow-empty", "-m", "snapshot") if task.review_patch: # 7 @@ -223,9 +235,31 @@ def prepare_workspace(git_dir: Path, manifest: RepoManifest, task: Task, arm: st } if bridge_skipped: meta["bridge_skipped"] = bridge_skipped + if installed: + meta["skills"] = installed return meta +def _content_dir(manifest: RepoManifest, task: Task) -> Path: + """The exported eval dir a task directory of :func:`load_tasks` is in.""" + eval_dir, tasks = manifest.run_paths() + return task.dir.parents[len(PurePosixPath(tasks).relative_to(eval_dir).parts)] + + +def _install_skills(ws: Path, task: Task, skills: list[Skill]) -> list[dict[str, str]]: + """Copy each skill to ``.claude/skills//``; a skill the repository already has there is refused, + as replacing it would change what the guidance arm measures.""" + out = [] + for skill in skills: + dest = ws / skill.target + if dest.exists(): + raise PrepError(f"task {task.id}: the workspace already has {skill.target}") + dest.parent.mkdir(parents=True, exist_ok=True) + shutil.copytree(skill.dir, dest, symlinks=False) # a link out of the skill would not exist in the container + out.append({"name": skill.name, "path": skill.target, "sha256": tree_sha256(dest)}) + return out + + def _forget(ws: Path, env: dict[str, str]) -> None: """Drop what prep replaced: the pre-amend commits in the reflog (the unpatched code, the removed guidance, the review patch's upstream author) and every object only they reached.""" diff --git a/dokimi/tests/repo_eval_fixture.py b/dokimi/tests/repo_eval_fixture.py index dfe146bd..621bd11f 100644 --- a/dokimi/tests/repo_eval_fixture.py +++ b/dokimi/tests/repo_eval_fixture.py @@ -95,8 +95,14 @@ lines.append({"type": "assistant", "message": {"content": [ {"type": "tool_use", "id": "t1", "name": "Edit", "input": {"file_path": os.environ.get("FAKE_CONTAINER_CWD", str(cwd)) + "/src/calc.py"}}]}}) +for name in filter(None, os.environ.get("FAKE_SKILLS", "").split(",")): # use installed skills, in order + if (cwd / ".claude/skills" / name / "SKILL.md").is_file(): + lines.append({"type": "assistant", "message": {"content": [ + {"type": "tool_use", "id": "s-" + name, "name": "Skill", "input": {"skill": name}}]}}) lines.append({"type": "result", "subtype": "success", "result": "done: src/calc.py", "total_cost_usd": 0.01, - "num_turns": 3, "is_error": False}) + "num_turns": 3, "is_error": False, + "usage": {"input_tokens": 5, "output_tokens": 2, "cache_read_input_tokens": 100, + "cache_creation_input_tokens": 40}}) print("\n".join(json.dumps(x) for x in lines)) ''' @@ -106,7 +112,7 @@ "invoke": ["{bin}", "-p", "{prompt}", "--model", "{model}", "--max-turns", "{max_turns}", "--max-budget-usd", "{budget}", "{tools}"], "tools": {"none": ["--tools", ""], "read": ["--tools", "Read"], "all": []}, - "config_dir_env": "FAKE_CONFIG_DIR", "env_allow": ["FAKE_ACTION", "FAKE_MARKER", "PATH"], + "config_dir_env": "FAKE_CONFIG_DIR", "env_allow": ["FAKE_ACTION", "FAKE_MARKER", "FAKE_SKILLS", "PATH"], "egress": ["api.example.com"], "trace": "claude-stream-json", "prompt_suffix": "Stay in the foreground.", "config_files": {"settings.json": '{"hook": "python3 {support_dir}/no_background.py"}'}, diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py new file mode 100644 index 00000000..a40912ff --- /dev/null +++ b/dokimi/tests/test_repo_skill_arms.py @@ -0,0 +1,136 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""`repo run` skill arms (skill:, skill-catalog:, joined with guidance by +) and cache tokens. No docker, no LLM.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from dokimi.cli import build_parser +from dokimi.repos.arms import ArmError, check_arms, parse_arm, resolve_skills, skill_usage +from dokimi.repos.events import convert +from dokimi.repos.report import format_report, summarize +from dokimi.repos.workspace import Task +from repo_eval_fixture import FakeRunner, fake_harness, git, make_repo, write + + +def _skill(root: Path, rel: str, name: str) -> None: + write(root, f"{rel}/SKILL.md", f"---\nname: {name}\ndescription: {name} things\n---\nDo {name}.\n") + + +@pytest.fixture +def setup(tmp_path, monkeypatch): + root, _ = make_repo(tmp_path, {"fix_lint": {"family": "fix", "grader": "calc", "setup_patch": "setup.patch", + "expected_skill": "lint", "prompt": "fix it."}}) + for name in ("lint", "format"): + _skill(root, f".agent-eval/skills/{name}", name) + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + exe, profile = fake_harness(tmp_path) + runner = FakeRunner() + monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: runner) + monkeypatch.setenv("FAKE_ACTION", "fix") + return {"root": root, "exe": exe, "profile": profile, "out": tmp_path / "runs"} + + +def _run(s, *extra): + argv = ["repo", "run", f"{s['root']}@HEAD", "--harness", str(s["profile"]), "--bin", str(s["exe"]), + "--model", "m-1", "--batch", "b1", "--out", str(s["out"]), "--cache-root", str(s["out"] / "caches"), + "--agent-network", "host", *extra] + args = build_parser().parse_args(argv) + args.func(args) + + +def test_parse_arm_composes_guidance_and_skills(): + arm = parse_arm("guidance:none+skill:skills/c++/x+skill-catalog:/abs/cat") + assert arm.guidance == "guidance:none" + assert arm.skills == (("skill", "skills/c++/x"), ("skill-catalog", "/abs/cat")) and arm.catalog + assert parse_arm("skill:a").guidance == "guidance:raw" and parse_arm("guidance:bridge").skills == () + for bad, why in (("main-raw", "unknown arm"), ("skill:", "needs a path"), + ("guidance:none+guidance:raw", "more than one guidance")): + with pytest.raises(ArmError, match=why): + parse_arm(bad) + + +def test_resolve_skills_is_relative_to_eval_dir_and_stays_in_it(tmp_path): + _skill(tmp_path, "skills/lint", "lint") + _skill(tmp_path, "skills/format", "format") + (tmp_path / "skills/notes").mkdir() + assert [s.name for s in resolve_skills(parse_arm("skill-catalog:skills"), tmp_path)] == ["format", "lint"] + assert resolve_skills(parse_arm("skill:skills/lint"), tmp_path)[0].dir == (tmp_path / "skills/lint").resolve() + for spec, why in (("skill:skills/notes", "no SKILL.md"), ("skill:../x", "leaves eval_dir"), + ("skill-catalog:skills/notes", "no subdirectory"), + ("skill:skills/lint+skill-catalog:skills", "more than one skill named lint")): + with pytest.raises(ArmError, match=why): + resolve_skills(parse_arm(spec), tmp_path) + task = Task("t", {"expected_skill": "deploy"}, tmp_path) + with pytest.raises(ArmError, match="expected_skill 'deploy'"): + check_arms(["skill-catalog:skills"], tmp_path, [task]) + check_arms(["skill:skills/lint"], tmp_path, [task]) # one skill: no selection to check + + +def test_skill_usage_from_skill_calls_and_skill_md_reads(): + events = [{"kind": "read", "path": ".claude/skills/format/SKILL.md"}, + {"kind": "tool", "name": "Skill", "skill": "lint"}, {"kind": "tool", "name": "Skill", "skill": "nope"}] + used = skill_usage(events, ["format", "lint"], "lint") + assert used["skills_used"] == ["format", "lint"] + assert used["skill_invoked"] is True and used["skill_selected_correct"] is False + none = skill_usage([], ["lint"], None) + assert none["skill_invoked"] is False and none["skill_selected_correct"] is None + + +def test_trace_records_the_skill_and_cache_tokens(): + raw = "\n".join(json.dumps(x) for x in [ + {"type": "assistant", "message": {"content": [{"type": "tool_use", "id": "a", "name": "Skill", + "input": {"skill": "lint"}}]}}, + {"type": "result", "result": "ok", "usage": {"input_tokens": 1, "output_tokens": 2, + "cache_read_input_tokens": 30, "cache_creation_input_tokens": 4}}]) + t = convert("claude-stream-json", raw, "/w") + assert t.events == [{"t": None, "kind": "tool", "name": "Skill", "skill": "lint"}] + assert (t.cache_read_input_tokens, t.cache_creation_input_tokens) == (30, 4) + + +def test_catalog_arm_installs_every_skill_and_records_selection(setup, monkeypatch, capsys): + monkeypatch.setenv("FAKE_SKILLS", "format,lint") # the agent tries the wrong skill first + _run(setup, "--arm", "guidance:none+skill-catalog:skills,guidance:raw", "--task", "fix_lint") + batch = setup["out"] / "b1" + rd = next(batch.glob("fix_lint__guidance-none_skill-catalog-skills__*")) + meta = json.loads((rd / "meta.json").read_text()) + assert [s["path"] for s in meta["skills"]] == [".claude/skills/format", ".claude/skills/lint"] + assert (rd / "workspace/.claude/skills/lint/SKILL.md").is_file() + assert not (rd / "workspace/AGENTS.md").exists() # the guidance part still applies + result = json.loads((rd / "result.json").read_text()) + assert result["arm"] == "guidance:none+skill-catalog:skills" + assert result["skills"]["skills_used"] == ["format", "lint"] and result["skills"]["expected_skill"] == "lint" + assert result["skills"]["skill_invoked"] is True and result["skills"]["skill_selected_correct"] is False + assert result["agent"]["cache_read_input_tokens"] == 100 and result["agent"]["cache_creation_input_tokens"] == 40 + raw_rd = next(batch.glob("fix_lint__guidance-raw__*")) + assert "skills" not in json.loads((raw_rd / "result.json").read_text()) + + rows = {r["arm"]: r for r in summarize(batch)["rows"]} + cat = rows["guidance:none+skill-catalog:skills"] + assert cat["skill_invoked"] == 1 and cat["skill_selected_correct"] == 0 + assert cat["cache_read_input_tokens"] == 100.0 and cat["cache_creation_input_tokens"] == 40.0 + assert "skill_invoked" not in rows["guidance:raw"] + assert "0/1 skill selected correct" in format_report(summarize(batch)) + + +def test_bad_skill_arms_are_refused_before_any_cell(setup, capsys): + for arm in ("skill:skills/missing", "skill-catalog:skills/lint"): + with pytest.raises(SystemExit): + _run(setup, "--arm", arm, "--task", "fix_lint") + assert "✗" in capsys.readouterr().err + assert not list((setup["out"] / "b1").glob("*/meta.json")) + + +def test_tasks_check_validates_skill_arms(setup, capsys): + argv = ["repo", "tasks", "check", f"{setup['root']}@HEAD", "--out", str(setup["out"] / "chk"), + "--cache-root", str(setup["out"] / "caches"), "--arm", "skill:skills/nope"] + args = build_parser().parse_args(argv) + with pytest.raises(SystemExit): + args.func(args) + assert "no SKILL.md in skills/nope" in capsys.readouterr().err From 892d71ab360581c4daf8d9b4bab840146f05aabd Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Mon, 19 Oct 2026 21:58:19 -0700 Subject: [PATCH 02/33] feat(repo): args_if_present in harness profiles; claude-code starts a repo's .mcp.json servers The stock claude-code profile passes --strict-mcp-config with no --mcp-config, so a repo that ships .mcp.json ran with no MCP tools. Passing --mcp-config unconditionally breaks repos without the file (Claude Code exits on a missing config). args_if_present appends arguments only when the named file exists in the run's workspace. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/repos/harness_profile.py | 10 ++++++++-- dokimi/dokimi/repos/profiles/claude-code.yaml | 3 +++ dokimi/dokimi/repos/runner.py | 3 ++- dokimi/dokimi/schemas/harness_profile.v1.json | 5 +++++ dokimi/tests/test_repo_reach.py | 7 +++++++ 5 files changed, 25 insertions(+), 3 deletions(-) diff --git a/dokimi/dokimi/repos/harness_profile.py b/dokimi/dokimi/repos/harness_profile.py index 34d897cb..84eac7b8 100644 --- a/dokimi/dokimi/repos/harness_profile.py +++ b/dokimi/dokimi/repos/harness_profile.py @@ -6,7 +6,8 @@ A profile is data, not code. ``invoke`` is an argv template, and every element becomes exactly one argument after placeholder substitution, so a prompt with quotes or spaces never passes through a shell. The element ``"{tools}"`` is -replaced by the argument list of the selected tool mode. The child process +replaced by the argument list of the selected tool mode, followed by the +``args_if_present`` arguments of each named workspace file that exists. The child process gets a scrubbed environment: only ``env_allow`` names are inherited, then ``env_set`` is applied, then ``config_dir_env`` points at a per-run directory so the user's own settings and memory cannot leak into a run. @@ -52,6 +53,7 @@ class HarnessProfile: bridge: dict[str, str] | None = None prompt_suffix: str = "" config_files: dict[str, str] = field(default_factory=dict) + args_if_present: dict[str, tuple[str, ...]] = field(default_factory=dict) path: str = "" @property @@ -68,8 +70,9 @@ def argv( max_turns: int = 4, budget_usd: float = 0.25, bin_path: str | None = None, + present: frozenset[str] = frozenset(), ) -> list[str]: - """The command line for one run, without a shell.""" + """The command line for one run, without a shell; ``present`` names the workspace files that exist.""" if tools not in self.tools: raise ProfileError(f"{self.path}: tool mode {tools!r} is not defined " f"(have: {', '.join(sorted(self.tools))})") @@ -84,6 +87,8 @@ def argv( for element in self.invoke: if element == TOOLS_ELEMENT: out.extend(self.tools[tools]) + for name in sorted(present & self.args_if_present.keys()): + out.extend(self.args_if_present[name]) else: out.append(_substitute(element, values)) return out @@ -184,6 +189,7 @@ def parse_profile(raw: Any, path: str = "") -> HarnessProfile: bridge=raw.get("bridge"), prompt_suffix=raw.get("prompt_suffix") or "", config_files=dict(raw.get("config_files") or {}), + args_if_present={name: tuple(args) for name, args in (raw.get("args_if_present") or {}).items()}, path=path, ) diff --git a/dokimi/dokimi/repos/profiles/claude-code.yaml b/dokimi/dokimi/repos/profiles/claude-code.yaml index 739921e2..abc61542 100644 --- a/dokimi/dokimi/repos/profiles/claude-code.yaml +++ b/dokimi/dokimi/repos/profiles/claude-code.yaml @@ -29,6 +29,9 @@ tools: none: ["--tools", ""] read: ["--tools", "Read"] all: ["--disallowedTools", "WebFetch", "WebSearch"] +# --strict-mcp-config alone starts no MCP server; a repo that ships .mcp.json gets its servers. +args_if_present: + .mcp.json: ["--mcp-config", ".mcp.json"] config_dir_env: CLAUDE_CONFIG_DIR env_allow: - PATH diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index 1b6b0f2c..1bff2a23 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -173,7 +173,8 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, opts.profile.write_config(rd / "config", {"config_dir": CONFIG_DIR, "support_dir": SUPPORT_DIR}) argv = opts.profile.argv(prompt=opts.profile.prompt(str(task.raw["prompt"])), model=opts.model, tools=opts.tools, max_turns=int(caps["max_turns"]), budget_usd=caps["budget_usd"], - bin_path=bin_in) + bin_path=bin_in, present=frozenset( + n for n in opts.profile.args_if_present if (rd / "workspace" / n).exists())) spec = ContainerSpec(image=ctx.image, argv=argv, workdir=WORKSPACE, mounts=mounts, env=env, pass_env=passed, network=network, name=container_name(rd.name, "agent")) result = ctx.runner.run(spec, stdout=rd / "agent.stdout", stderr=rd / "agent.stderr", diff --git a/dokimi/dokimi/schemas/harness_profile.v1.json b/dokimi/dokimi/schemas/harness_profile.v1.json index 780ec6ea..05c2461a 100644 --- a/dokimi/dokimi/schemas/harness_profile.v1.json +++ b/dokimi/dokimi/schemas/harness_profile.v1.json @@ -43,6 +43,11 @@ } }, "prompt_suffix": {"type": "string", "description": "Appended to every task prompt after a blank line, for what the harness needs the agent to know about running headless (for example that background commands are refused)."}, + "args_if_present": { + "type": "object", + "description": "Workspace file name -> arguments appended after the tool-mode arguments when that file exists in the run's workspace (for example --mcp-config .mcp.json, so a repo's MCP servers start only when it ships them).", + "additionalProperties": {"type": "array", "items": {"type": "string"}} + }, "config_files": { "type": "object", "description": "Files written into the per-run config directory before the agent starts, name -> content. Placeholders: {config_dir} and {support_dir} (the framework's read-only helper scripts, such as the no-background hook), both as the agent's container sees them.", diff --git a/dokimi/tests/test_repo_reach.py b/dokimi/tests/test_repo_reach.py index c9896feb..bc8c8ad8 100644 --- a/dokimi/tests/test_repo_reach.py +++ b/dokimi/tests/test_repo_reach.py @@ -193,6 +193,13 @@ def test_builtin_claude_code_profile_builds_argv_without_a_shell(tmp_path): profile.argv(prompt="p", model="m", tools="everything") +def test_claude_code_profile_passes_mcp_config_only_when_the_workspace_has_one(): + profile = load_profile("claude-code") + assert "--mcp-config" not in profile.argv(prompt="p", model="m") + argv = profile.argv(prompt="p", model="m", present=frozenset({".mcp.json", "other"})) + assert argv[-4:] == ["--tools", "", "--mcp-config", ".mcp.json"] + + def test_profile_env_is_allow_listed_and_isolated(tmp_path): profile = load_profile("claude-code") env = profile.env(tmp_path / "cfg").resolve({"PATH": "/bin", "AWS_REGION": "us-east-1", From 5c17c6893608283341a2ab8ea411cf383e375b40 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Mon, 19 Oct 2026 22:09:41 -0700 Subject: [PATCH 03/33] docs(changelog): fragment for skill arms and MCP config Signed-off-by: Nikolay Petrov --- .../20261020_090000_napetrov_repo-skill-arms.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md new file mode 100644 index 00000000..675dd6cf --- /dev/null +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -0,0 +1,17 @@ +### Added + +- **`repo run` skill arms** — `--arm guidance:none+skill:` installs one + skill under `.claude/skills//`, and `skill-catalog:` installs + every skill in a directory. `result.json` records `skills` + (`skills_used`, `skill_invoked`, `skill_selected_correct`) and the agent's + cache read/creation token counts. `repo tasks check --arm` rejects a skill + path that does not exist. +- **`args_if_present` in harness profiles** — arguments appended only when a + file exists in the workspace. + +### Fixed + +- **claude-code starts a repo's MCP servers** — the stock profile passed + `--strict-mcp-config` without `--mcp-config`, so a repo that ships + `.mcp.json` ran with no MCP tools. It now passes `--mcp-config .mcp.json` + when that file is present. From a9b74b5883907a673603db934221e90f93375a12 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Tue, 20 Oct 2026 04:03:34 -0700 Subject: [PATCH 04/33] fix(repo): refuse skill links out of the skill; refuse arms that share a result dir A skill or catalog entry with a symlink to a host path was copied with its target's bytes (copytree dereferences), so the agent could read host files. Links that leave a skill, and catalog entries that leave the catalog, are now refused before any cell runs. Two arms whose strings slug to the same cell name (skill:a/b and skill:a_b) shared one result directory, so the second arm silently got no row. check_arms now refuses such arms up front. Regression tests cover both, and the runner's --mcp-config wiring. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/repos/arms.py | 30 +++++++++++++++- dokimi/dokimi/repos/workspace.py | 2 +- dokimi/tests/test_repo_skill_arms.py | 52 ++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 2ee0c1d3..995fcd1e 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -23,6 +23,7 @@ from __future__ import annotations import hashlib +import os from dataclasses import dataclass from pathlib import Path, PurePosixPath from typing import Any @@ -101,10 +102,20 @@ def resolve_skills(arm: RepoArm, content_dir: Path) -> list[Skill]: continue if not root.is_dir(): raise ArmError(f"arm {arm.spec!r}: skill-catalog {path} is not a directory") - subs = [Skill(d.name, d) for d in sorted(root.iterdir()) if d.is_dir() and (d / "SKILL.md").is_file()] + subs = [] + for d in sorted(root.iterdir()): + if not d.is_dir() or not (d / "SKILL.md").is_file(): + continue + if not d.resolve().is_relative_to(root.resolve()): + raise ArmError(f"arm {arm.spec!r}: {d.name} in {path} links outside it") + subs.append(Skill(d.name, d)) if not subs: raise ArmError(f"arm {arm.spec!r}: no subdirectory of {path} holds a SKILL.md") found += subs + for skill in found: + link = _escaping_link(skill.dir) + if link: + raise ArmError(f"arm {arm.spec!r}: {link.relative_to(skill.dir)} in skill {skill.name} links outside it") names = [s.name for s in found] dupes = sorted({n for n in names if names.count(n) > 1}) if dupes: @@ -112,6 +123,17 @@ def resolve_skills(arm: RepoArm, content_dir: Path) -> list[Skill]: return found +def _escaping_link(skill_dir: Path) -> Path | None: + """A link under ``skill_dir`` whose target is outside it: copying would take the host's bytes.""" + root = skill_dir.resolve() + for dirpath, dirnames, filenames in os.walk(skill_dir): + for name in dirnames + filenames: + link = Path(dirpath, name) + if link.is_symlink() and not link.resolve().is_relative_to(root): + return link + return None + + def _skill_path(path: str, content_dir: Path) -> Path: if Path(path).is_absolute(): return Path(path).resolve() @@ -125,9 +147,15 @@ def _skill_path(path: str, content_dir: Path) -> Path: def check_arms(arms: list[str], content_dir: Path, tasks: list[Any]) -> list[RepoArm]: """Parse every arm and find its skills before anything runs; a catalog must hold each task's ``expected_skill``. Returns the parsed arms.""" + from .runner import slug # runner imports this module + parsed = [] + cells: dict[str, str] = {} # cell-name slug of an arm -> the arm string that owns it for spec in arms: arm = parse_arm(spec) + cell = slug(spec.replace(":", "-")) + if cells.setdefault(cell, spec) != spec: + raise ArmError(f"arms {cells[cell]!r} and {spec!r} would share one result directory ({cell})") names = {s.name for s in resolve_skills(arm, content_dir)} if arm.catalog: for task in tasks: diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index e289cdd9..8e4187ec 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -255,7 +255,7 @@ def _install_skills(ws: Path, task: Task, skills: list[Skill]) -> list[dict[str, if dest.exists(): raise PrepError(f"task {task.id}: the workspace already has {skill.target}") dest.parent.mkdir(parents=True, exist_ok=True) - shutil.copytree(skill.dir, dest, symlinks=False) # a link out of the skill would not exist in the container + shutil.copytree(skill.dir, dest, symlinks=False) # resolve_skills refused any link out of the skill out.append({"name": skill.name, "path": skill.target, "sha256": tree_sha256(dest)}) return out diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index a40912ff..caf14472 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -134,3 +134,55 @@ def test_tasks_check_validates_skill_arms(setup, capsys): with pytest.raises(SystemExit): args.func(args) assert "no SKILL.md in skills/nope" in capsys.readouterr().err + + +def test_a_link_out_of_a_skill_or_catalog_is_refused(tmp_path): + host = tmp_path / "host" # outside eval_dir; a link to it must not copy its bytes into the workspace + host.mkdir() + (host / "SKILL.md").write_text("---\nname: evil\ndescription: x\n---\n") + (host / "token.txt").write_text("TOPSECRET") + evaldir = tmp_path / "eval" + _skill(evaldir, "skills/good", "good") + (evaldir / "skills/good/leak.txt").symlink_to(host / "token.txt") + with pytest.raises(ArmError, match="leak.txt in skill good links outside it"): + resolve_skills(parse_arm("skill:skills/good"), evaldir) + (evaldir / "skills/good/leak.txt").unlink() + (evaldir / "skills/evil").symlink_to(host) + with pytest.raises(ArmError, match="evil in skills links outside it"): + resolve_skills(parse_arm("skill-catalog:skills"), evaldir) + + +def test_arms_that_share_a_result_directory_are_refused(tmp_path): + _skill(tmp_path, "a/b", "x") + _skill(tmp_path, "a_b", "y") + _skill(tmp_path, "s/one", "one") + _skill(tmp_path, "catalog-s", "two") + with pytest.raises(ArmError, match="share one result directory"): + check_arms(["skill:a/b", "skill:a_b"], tmp_path, []) + with pytest.raises(ArmError, match="share one result directory"): + check_arms(["skill-catalog:s", "skill:catalog-s"], tmp_path, []) + + +def test_claude_code_gets_mcp_config_only_when_the_workspace_has_the_file(tmp_path, monkeypatch): + runner = FakeRunner() + monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: runner) + monkeypatch.setenv("FAKE_ACTION", "fix") + profile = Path(__file__).resolve().parents[1] / "dokimi/repos/profiles/claude-code.yaml" + exe, _ = fake_harness(tmp_path) + for with_mcp in (True, False): + runner.specs.clear() + base = tmp_path / ("with" if with_mcp else "without") + base.mkdir() + if with_mcp: + write(base / "toy", ".mcp.json", '{"mcpServers": {}}') # make_repo commits it into the task base + root, _ = make_repo(base) + argv = ["repo", "run", f"{root}@HEAD", "--harness", str(profile), "--bin", str(exe), "--model", "m-1", + "--batch", "b1", "--out", str(base / "runs"), "--cache-root", str(base / "caches"), + "--agent-network", "host", "--arm", "guidance:raw", "--task", "fix_add"] + args = build_parser().parse_args(argv) + args.func(args) + agent = [s.argv for s in runner.specs if "--strict-mcp-config" in s.argv] + assert len(agent) == 1 + assert ("--mcp-config" in agent[0]) is with_mcp + if with_mcp: + assert agent[0][agent[0].index("--mcp-config") + 1] == ".mcp.json" From f7280b0998745b6377e907d98163da5c9dd3d49a Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Tue, 20 Oct 2026 14:39:46 -0700 Subject: [PATCH 05/33] fix(repo): address review of #137 (skill arms) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - arms: move slug() out of runner so arms no longer imports runner (closes the import cycle test_import_cycles_do_not_grow catches) - arms: refuse skill links that dangle, loop, point at a directory, or are not regular files; they crashed every cell at workspace prep - workspace: refuse to install a skill through a .claude or .claude/skills that the repository ships as a link - runner/report: a cell whose trace could not be read records skill usage as null and is left out of the skill counts - docs: contract §3 covers skill arms; §6 shows args_if_present - tests: link cases, catalog with the expected skill first, guidance:none keeping a skill, tree hash sensitivity, unmeasured cells, write-through Signed-off-by: Nikolay Petrov --- dokimi/docs/repo-evaluation-contract.md | 10 +++ dokimi/dokimi/repos/arms.py | 45 +++++++++--- dokimi/dokimi/repos/report.py | 11 ++- dokimi/dokimi/repos/runner.py | 10 +-- dokimi/dokimi/repos/workspace.py | 6 ++ dokimi/tests/test_repo_skill_arms.py | 98 ++++++++++++++++++++++++- 6 files changed, 156 insertions(+), 24 deletions(-) diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 40069f9d..80a1dccd 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -261,6 +261,14 @@ harness profile; a repository supplies no arm content: only when no `CLAUDE.md` is in its start-up chain, so a repository-side bridge arm would mean different things on different versions. +A skill arm (`skill:`, `skill-catalog:`, joined to a guidance part +with `+`) installs skill directories into `.claude/skills//` after the +guidance part, so `guidance:none+skill:x` keeps `x`. A skill is refused before +any cell runs if it holds a link that copying cannot take (out of the skill, +dangling, looping, to a directory) or a file that is not regular. Preparation +also refuses to write through a `.claude` or `.claude/skills` the repository +ships as a link. + `meta.json` records the base (`base_rev`, `base_sha`, `base_date`), the arm, the SHA of every applied patch, and `start_sha`: the workspace `HEAD` after step 7, the commit the agent starts from (§4). `base_sha` names the upstream @@ -442,6 +450,8 @@ guidance: # DECLARED; verified by the reach probe, ne resolution: ancestors-of-cwd + lazy-on-subtree-access imports: "@path" bridge: {for: AGENTS.md, write: CLAUDE.md, content: "@AGENTS.md"} +args_if_present: # argv added after the "{tools}" element when the workspace file exists + .mcp.json: ["--mcp-config", ".mcp.json"] ``` - `invoke` and `version_cmd` are argv lists. Each element becomes exactly one diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 995fcd1e..f4baad2e 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -24,6 +24,7 @@ import hashlib import os +import re from dataclasses import dataclass from pathlib import Path, PurePosixPath from typing import Any @@ -34,6 +35,11 @@ COMBINE = "+" +def slug(text: str) -> str: + """``text`` as a file-system-safe name: the result directory of a cell, the cell name.""" + return re.sub(r"[^A-Za-z0-9._-]+", "_", text).strip("_") or "x" + + class ArmError(ValueError): """An arm that does not parse, or whose skills cannot be found.""" @@ -113,9 +119,10 @@ def resolve_skills(arm: RepoArm, content_dir: Path) -> list[Skill]: raise ArmError(f"arm {arm.spec!r}: no subdirectory of {path} holds a SKILL.md") found += subs for skill in found: - link = _escaping_link(skill.dir) - if link: - raise ArmError(f"arm {arm.spec!r}: {link.relative_to(skill.dir)} in skill {skill.name} links outside it") + bad = _uncopyable(skill.dir) + if bad: + path, why = bad + raise ArmError(f"arm {arm.spec!r}: {path.relative_to(skill.dir)} in skill {skill.name} {why}") names = [s.name for s in found] dupes = sorted({n for n in names if names.count(n) > 1}) if dupes: @@ -123,14 +130,25 @@ def resolve_skills(arm: RepoArm, content_dir: Path) -> list[Skill]: return found -def _escaping_link(skill_dir: Path) -> Path | None: - """A link under ``skill_dir`` whose target is outside it: copying would take the host's bytes.""" +def _uncopyable(skill_dir: Path) -> tuple[Path, str] | None: + """What under ``skill_dir`` the copy into the workspace cannot take, with the reason: a link out of the + skill (the copy would take the host's bytes), a dangling or looping link, a link to a directory (the copy + would recurse), or a file that is not regular (a FIFO would block the copy).""" root = skill_dir.resolve() for dirpath, dirnames, filenames in os.walk(skill_dir): for name in dirnames + filenames: - link = Path(dirpath, name) - if link.is_symlink() and not link.resolve().is_relative_to(root): - return link + path = Path(dirpath, name) + if path.is_symlink(): + try: + target = path.resolve(strict=True) + except (OSError, RuntimeError): + return path, "is a dangling or looping link" + if not target.is_relative_to(root): + return path, "links outside it" + if target.is_dir(): + return path, "links to a directory" + elif not path.is_file() and not path.is_dir(): + return path, "is not a regular file" return None @@ -147,8 +165,6 @@ def _skill_path(path: str, content_dir: Path) -> Path: def check_arms(arms: list[str], content_dir: Path, tasks: list[Any]) -> list[RepoArm]: """Parse every arm and find its skills before anything runs; a catalog must hold each task's ``expected_skill``. Returns the parsed arms.""" - from .runner import slug # runner imports this module - parsed = [] cells: dict[str, str] = {} # cell-name slug of an arm -> the arm string that owns it for spec in arms: @@ -176,6 +192,12 @@ def tree_sha256(root: Path) -> str: return h.hexdigest() +def skill_unmeasured(installed: list[str], expected: str | None) -> dict[str, Any]: + """The usage of a cell whose trace could not be read: unknown, not "did not use the skill" (``None``).""" + return {"measured": False, "skills_installed": installed, "skills_used": [], "expected_skill": expected, + "skill_invoked": None, "skill_selected_correct": None} + + def skill_usage(events: list[dict[str, Any]], installed: list[str], expected: str | None) -> dict[str, Any]: """Which installed skills the agent used: a ``Skill`` tool call naming it, or a read of its ``SKILL.md``. @@ -193,7 +215,8 @@ def skill_usage(events: list[dict[str, Any]], installed: list[str], expected: st name = parts[2] if name in installed and name not in used: used.append(name) - out: dict[str, Any] = {"skills_installed": installed, "skills_used": used, "expected_skill": expected, + out: dict[str, Any] = {"measured": True, "skills_installed": installed, "skills_used": used, + "expected_skill": expected, "skill_invoked": (expected in used) if expected else bool(used), "skill_selected_correct": (used[:1] == [expected]) if expected else None} return out diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index 3f824ef0..86a92a6a 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -16,6 +16,8 @@ - skill arms add ``skill_invoked`` (cells whose agent used the expected skill, or any installed one when the task names none) and, when the task has an ``expected_skill``, ``skill_selected_correct`` (cells that used it first). + Cells whose trace could not be read are left out of both (``skill_measured`` + counts the rest). """ from __future__ import annotations @@ -83,8 +85,9 @@ def summarize(batch_dir: Path) -> dict[str, Any]: else: wins = sum(bool(r["grade"]["success"]) for r in recs) row.update(kind="gate", successes=wins, success_rate=round(wins / n, 4), ci95=wilson(wins, n)) - used = [r["skills"] for r in recs if r.get("skills")] - if used: + used = [r["skills"] for r in recs if r.get("skills") and r["skills"].get("measured", True)] + if used: # cells whose trace was unreadable are not counted: no rate from a missing measurement + row["skill_measured"] = len(used) row["skill_invoked"] = sum(bool(u["skill_invoked"]) for u in used) if any(u.get("expected_skill") for u in used): row["skill_selected_correct"] = sum(bool(u["skill_selected_correct"]) for u in used) @@ -115,8 +118,8 @@ def format_report(summary: dict[str, Any]) -> str: f"{result:<22} {r['grader_failures']:>11} {r['missing']:>7} {cost:>8} {turns:>6}") noted = ("agent_timeouts", "agent_errors", "over_budget", "cost_unknown") notes = [f"{r[k]} {k.replace('_', ' ')}" for k in noted if r[k]] - notes += [f"{r[k]}/{r['n']} {k.replace('_', ' ')}" for k in ("skill_invoked", "skill_selected_correct") - if k in r] + notes += [f"{r[k]}/{r['skill_measured']} {k.replace('_', ' ')}" + for k in ("skill_invoked", "skill_selected_correct") if k in r] if notes: lines.append(" " * (width + 2) + " " + ", ".join(notes)) if summary["missing"]: diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index 1bff2a23..769e0168 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -55,7 +55,7 @@ Mount, docker_network, ) -from .arms import skill_usage +from .arms import skill_unmeasured, skill_usage, slug from .events import convert from .grading import EvalContext, failed_outcome, outcome, run_entry from .harness_profile import HarnessProfile @@ -82,10 +82,6 @@ class AgentOptions: budget_usd: float = 2.0 -def slug(text: str) -> str: - return re.sub(r"[^A-Za-z0-9._-]+", "_", text).strip("_") or "x" - - def cell_name(task: str, arm: str, model: str, rep: int) -> str: return f"{task}__{slug(arm.replace(':', '-'))}__{slug(model)}__r{rep}" @@ -217,8 +213,8 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, "harness": opts.profile.name, "harness_version": meta["harness_version"], "base_date": meta["base_date"], "score_task": bool(task.score), "agent": agent, "grade": grade} if meta.get("skills"): # without a trace the agent used none of them - record["skills"] = skills if skills is not None else skill_usage( - [], [s["name"] for s in meta["skills"]], task.expected_skill) + record["skills"] = skills if skills is not None else skill_unmeasured( + [s["name"] for s in meta["skills"]], task.expected_skill) try: _own_dir(rd) # the grader had the run dir read-write, and may have taken write access away _write_json(rd / "result.json", record) diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index 8e4187ec..9cc83c93 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -252,8 +252,14 @@ def _install_skills(ws: Path, task: Task, skills: list[Skill]) -> list[dict[str, out = [] for skill in skills: dest = ws / skill.target + for part in (ws / ".claude", ws / ".claude" / "skills"): # a repository link would write to the host + if part.is_symlink(): + raise PrepError(f"task {task.id}: {part.relative_to(ws)} is a symlink in the repository; " + f"refusing to install {skill.name} through it") if dest.exists(): raise PrepError(f"task {task.id}: the workspace already has {skill.target}") + if not dest.parent.resolve().is_relative_to(ws.resolve()): + raise PrepError(f"task {task.id}: {skill.target} resolves outside the workspace") dest.parent.mkdir(parents=True, exist_ok=True) shutil.copytree(skill.dir, dest, symlinks=False) # resolve_skills refused any link out of the skill out.append({"name": skill.name, "path": skill.target, "sha256": tree_sha256(dest)}) diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index caf14472..bad25e21 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -6,15 +6,17 @@ from __future__ import annotations import json +import os +import shutil from pathlib import Path import pytest from dokimi.cli import build_parser -from dokimi.repos.arms import ArmError, check_arms, parse_arm, resolve_skills, skill_usage +from dokimi.repos.arms import ArmError, Skill, check_arms, parse_arm, resolve_skills, skill_usage, tree_sha256 from dokimi.repos.events import convert from dokimi.repos.report import format_report, summarize -from dokimi.repos.workspace import Task +from dokimi.repos.workspace import PrepError, Task, _install_skills from repo_eval_fixture import FakeRunner, fake_harness, git, make_repo, write @@ -83,6 +85,51 @@ def test_skill_usage_from_skill_calls_and_skill_md_reads(): assert none["skill_invoked"] is False and none["skill_selected_correct"] is None +def test_catalog_arm_counts_the_expected_skill_picked_first(setup, monkeypatch): + monkeypatch.setenv("FAKE_SKILLS", "lint,format") # the agent picks the right skill first + _run(setup, "--arm", "guidance:none+skill-catalog:skills", "--task", "fix_lint") + rd = next((setup["out"] / "b1").glob("fix_lint__guidance-none_skill-catalog-skills__*")) + skills = json.loads((rd / "result.json").read_text())["skills"] + assert skills["skills_used"] == ["lint", "format"] and skills["skill_selected_correct"] is True + rows = {r["arm"]: r for r in summarize(setup["out"] / "b1")["rows"]} + assert rows["guidance:none+skill-catalog:skills"]["skill_selected_correct"] == 1 + + +def test_guidance_none_keeps_a_skill_when_the_repo_ships_claude_skills(tmp_path, monkeypatch): + write(tmp_path / "toy", ".claude/skills/other/SKILL.md", "---\nname: other\ndescription: x\n---\nOther.\n") + root, _ = make_repo(tmp_path, {"fix_lint": {"family": "fix", "grader": "calc", "setup_patch": "setup.patch", + "expected_skill": "lint", "prompt": "fix it."}}) + _skill(root, ".agent-eval/skills/lint", "lint") + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + exe, profile = fake_harness(tmp_path) + runner = FakeRunner() + monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: runner) + monkeypatch.setenv("FAKE_ACTION", "fix") + out = tmp_path / "runs" + args = build_parser().parse_args(["repo", "run", f"{root}@HEAD", "--harness", str(profile), "--bin", str(exe), + "--model", "m-1", "--batch", "b1", "--out", str(out), + "--cache-root", str(out / "caches"), "--agent-network", "host", + "--arm", "guidance:none+skill:skills/lint", "--task", "fix_lint"]) + args.func(args) + rd = next((out / "b1").glob("fix_lint__*")) + assert (rd / "workspace/.claude/skills/lint/SKILL.md").is_file() + + +@pytest.mark.parametrize("link", [".claude", ".claude/skills"]) +def test_a_skill_is_not_written_through_a_repository_link(tmp_path, link): + host = tmp_path / "host" # a host directory the repository's own link points at + host.mkdir() + ws = tmp_path / "ws" + (ws / ".claude").mkdir(parents=True) if link == ".claude/skills" else ws.mkdir() + (ws / link).symlink_to(host, target_is_directory=True) + _skill(tmp_path / "eval", "skills/lint", "lint") + skill = Skill("lint", tmp_path / "eval/skills/lint") + with pytest.raises(PrepError, match="is a symlink in the repository"): + _install_skills(ws, Task("t", {}, tmp_path), [skill]) + assert not (host / "skills").exists() + + def test_trace_records_the_skill_and_cache_tokens(): raw = "\n".join(json.dumps(x) for x in [ {"type": "assistant", "message": {"content": [{"type": "tool_use", "id": "a", "name": "Skill", @@ -150,6 +197,53 @@ def test_a_link_out_of_a_skill_or_catalog_is_refused(tmp_path): (evaldir / "skills/evil").symlink_to(host) with pytest.raises(ArmError, match="evil in skills links outside it"): resolve_skills(parse_arm("skill-catalog:skills"), evaldir) + (evaldir / "skills/evil").unlink() + # inside the skill, but the copy cannot take them: each is refused before any cell runs + good = evaldir / "skills/good" + cases = (("loop", lambda: (good / "loop").symlink_to("."), ["loop"], "links to a directory"), + ("up", lambda: ((good / "sub").mkdir(), (good / "sub/up").symlink_to("..")), ["sub"], + "links to a directory"), + ("dangling", lambda: (good / "d").symlink_to("nope"), ["d"], "dangling or looping link"), + ("ping", lambda: (good / "a").symlink_to("b") or (good / "b").symlink_to("a"), ["a", "b"], + "dangling or looping link"), + ("fifo", lambda: os.mkfifo(good / "pipe"), ["pipe"], "is not a regular file")) + for _name, make, names, why in cases: + make() + with pytest.raises(ArmError, match=why): + resolve_skills(parse_arm("skill:skills/good"), evaldir) + for leftover in names: + if (good / leftover).is_dir() and not (good / leftover).is_symlink(): + shutil.rmtree(good / leftover) + else: + (good / leftover).unlink() + + +def test_a_tree_hash_changes_with_the_skill_bytes(tmp_path): + for name, body in (("a", "Do lint."), ("b", "Do format.")): + write(tmp_path, f"{name}/SKILL.md", body) + assert tree_sha256(tmp_path / "a") != tree_sha256(tmp_path / "b") + + +def test_unmeasured_skill_usage_is_left_out_of_the_counts(tmp_path): + batch = tmp_path / "b" + + def cell(name, skills): + rd = batch / name + rd.mkdir(parents=True) + (rd / "meta.json").write_text("{}") + rec = {"task": "t", "arm": "skill:skills/lint", "model": "m", "score_task": False, "grade": { + "status": "ok", "success": True, "score_only": False, "score": None}, + "agent": {"timed_out": False, "is_error": False, "over_budget": False, "cost_usd": 0.1, + "num_turns": 3, "network": "none"}, "skills": skills} + (rd / "result.json").write_text(json.dumps(rec)) + + cell("c1", {"measured": True, "skills_installed": ["lint"], "skills_used": ["lint"], "expected_skill": "lint", + "skill_invoked": True, "skill_selected_correct": True}) + cell("c2", {"measured": False, "skills_installed": ["lint"], "skills_used": [], "expected_skill": "lint", + "skill_invoked": None, "skill_selected_correct": None}) + row = summarize(batch)["rows"][0] + assert (row["skill_measured"], row["skill_invoked"], row["skill_selected_correct"]) == (1, 1, 1) + assert "1/1 skill invoked" in format_report(summarize(batch)) def test_arms_that_share_a_result_directory_are_refused(tmp_path): From 4e4e4029b939f19f7cfb7119406bd97fbdb67e83 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Tue, 20 Oct 2026 23:25:30 -0700 Subject: [PATCH 06/33] fix(repo): pass args_if_present (MCP config) only in --tools all --tools none/read do not limit MCP tools, so a repo's .mcp.json started its servers in arms meant to give the agent no tools. Signed-off-by: Nikolay Petrov --- .../20261020_090000_napetrov_repo-skill-arms.md | 5 +++-- dokimi/docs/repo-evaluation-contract.md | 2 +- dokimi/dokimi/repos/harness_profile.py | 7 ++++--- dokimi/tests/test_repo_reach.py | 6 +++++- dokimi/tests/test_repo_skill_arms.py | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index 201f2129..3b74ae08 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -7,11 +7,12 @@ cache read/write token counts. `repo tasks check --arm` rejects a skill path that does not exist. - **`args_if_present` in harness profiles** — arguments appended only when a - file exists in the workspace. + file exists in the workspace and the tool mode is `all`. ### Fixed - **claude-code starts a repo's MCP servers** — the stock profile passed `--strict-mcp-config` without `--mcp-config`, so a repo that ships `.mcp.json` ran with no MCP tools. It now passes `--mcp-config .mcp.json` - when that file is present. + when that file is present and `--tools all` is used (`--tools none`/`read` do not + limit MCP tools, so they start no MCP server). diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index df48b2bd..fdbac5ea 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -526,7 +526,7 @@ guidance: # DECLARED; verified by the reach probe, ne resolution: ancestors-of-cwd + lazy-on-subtree-access imports: "@path" bridge: {for: AGENTS.md, write: CLAUDE.md, content: "@AGENTS.md"} -args_if_present: # argv added after the "{tools}" element when the workspace file exists +args_if_present: # argv added after the "{tools}" element when the workspace file exists and the tool mode is `all` .mcp.json: ["--mcp-config", ".mcp.json"] ``` diff --git a/dokimi/dokimi/repos/harness_profile.py b/dokimi/dokimi/repos/harness_profile.py index 6f3b2642..4f0da49f 100644 --- a/dokimi/dokimi/repos/harness_profile.py +++ b/dokimi/dokimi/repos/harness_profile.py @@ -6,7 +6,7 @@ A profile is data, not code. ``invoke`` is an argv template, and every element becomes exactly one argument after placeholder substitution, so a prompt with quotes or spaces never passes through a shell. The element ``"{tools}"`` is -replaced by the argument list of the selected tool mode, followed by the +replaced by the argument list of the selected tool mode, followed, in the ``all`` tool mode only, by the ``args_if_present`` arguments of each named workspace file that exists. The child process gets a scrubbed environment: only ``env_allow`` names are inherited, then ``env_set`` is applied, then ``config_dir_env`` points at a per-run directory @@ -93,8 +93,9 @@ def argv( for element in self.invoke: if element == TOOLS_ELEMENT: out.extend(self.tools[tools]) - for name in sorted(present & self.args_if_present.keys()): - out.extend(self.args_if_present[name]) + if tools == "all": # --tools does not limit MCP tools: only here is the repo's MCP config part of the arm + for name in sorted(present & self.args_if_present.keys()): + out.extend(self.args_if_present[name]) else: out.append(_substitute(element, values)) return out diff --git a/dokimi/tests/test_repo_reach.py b/dokimi/tests/test_repo_reach.py index ad08ee59..79a06ae4 100644 --- a/dokimi/tests/test_repo_reach.py +++ b/dokimi/tests/test_repo_reach.py @@ -197,7 +197,11 @@ def test_claude_code_profile_passes_mcp_config_only_when_the_workspace_has_one() profile = load_profile("claude-code") assert "--mcp-config" not in profile.argv(prompt="p", model="m") argv = profile.argv(prompt="p", model="m", present=frozenset({".mcp.json", "other"})) - assert argv[-4:] == ["--tools", "", "--mcp-config", ".mcp.json"] + assert "--mcp-config" not in argv # --tools "" does not stop MCP servers, so none is started + for tools in ("none", "read"): + assert "--mcp-config" not in profile.argv(prompt="p", model="m", tools=tools, present=frozenset({".mcp.json"})) + assert profile.argv(prompt="p", model="m", tools="all", present=frozenset({".mcp.json"}))[-2:] == [ + "--mcp-config", ".mcp.json"] def test_profile_env_is_allow_listed_and_isolated(tmp_path): diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 77ef6eea..760db383 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -277,7 +277,7 @@ def test_claude_code_gets_mcp_config_only_when_the_workspace_has_the_file(tmp_pa root, _ = make_repo(base) argv = ["repo", "run", f"{root}@HEAD", "--harness", str(profile), "--bin", str(exe), "--model", "m-1", "--batch", "b1", "--out", str(base / "runs"), "--cache-root", str(base / "caches"), - "--agent-network", "host", "--arm", "guidance:raw", "--task", "fix_add"] + "--agent-network", "host", "--arm", "guidance:raw", "--task", "fix_add", "--tools", "all"] args = build_parser().parse_args(argv) args.func(args) agent = [s.argv for s in runner.specs if "--strict-mcp-config" in s.argv] From 353ad50070e5142e823d56c276e0116464a5ecd2 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Tue, 20 Oct 2026 23:25:43 -0700 Subject: [PATCH 07/33] feat(repo): refuse a skill whose SKILL.md name differs from its directory The agent calls a skill by its frontmatter name, usage matches on the directory name; a mismatch scored skill_invoked False. The merge with main added the check; this adds its regression test and the changelog line. Signed-off-by: Nikolay Petrov --- .../20261020_090000_napetrov_repo-skill-arms.md | 4 +++- dokimi/tests/test_repo_skill_arms.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index 3b74ae08..1a8aa2c9 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -5,7 +5,9 @@ every skill in a directory. `result.json` records `skills` (`skills_used`, `skill_invoked`, `skill_selected_correct`) and the agent's cache read/write token counts. `repo tasks check --arm` rejects a skill - path that does not exist. + path that does not exist, and a skill whose `SKILL.md` `name` differs from its + directory name. The arm is named by the directory (`skill:lint`), so + `repo compare` can select it. - **`args_if_present` in harness profiles** — arguments appended only when a file exists in the workspace and the tool mode is `all`. diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 760db383..95d2ae9a 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -285,3 +285,14 @@ def test_claude_code_gets_mcp_config_only_when_the_workspace_has_the_file(tmp_pa assert ("--mcp-config" in agent[0]) is with_mcp if with_mcp: assert agent[0][agent[0].index("--mcp-config") + 1] == ".mcp.json" + + +def test_a_skill_whose_frontmatter_name_differs_from_its_directory_is_refused(tmp_path): + _skill(tmp_path, "skills/lint", "pretty-lint") # the agent would call it pretty-lint; usage matches on "lint" + _skill(tmp_path, "skills/format", "format") + with pytest.raises(ArmError, match="named 'pretty-lint', but its directory is 'lint'"): + resolve_skills(parse_arm("skill:skills/lint"), tmp_path) + with pytest.raises(ArmError, match="named 'pretty-lint'"): + resolve_skills(parse_arm("skill-catalog:skills"), tmp_path) + write(tmp_path, "skills/bare/SKILL.md", "No frontmatter.\n") # no name: the directory names it + assert [s.name for s in resolve_skills(parse_arm("skill:skills/bare"), tmp_path)] == ["bare"] From e1dd8204a38452c792d279842589114b365e0061 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Tue, 20 Oct 2026 23:26:02 -0700 Subject: [PATCH 08/33] test(repo): repo compare selects a skill arm Skill arms are named by directory (no '/'), so parse_selector needs no whole-string fallback; the test pins that. Signed-off-by: Nikolay Petrov --- dokimi/tests/test_repo_skill_arms.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 95d2ae9a..79bd9a71 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -296,3 +296,12 @@ def test_a_skill_whose_frontmatter_name_differs_from_its_directory_is_refused(tm resolve_skills(parse_arm("skill-catalog:skills"), tmp_path) write(tmp_path, "skills/bare/SKILL.md", "No frontmatter.\n") # no name: the directory names it assert [s.name for s in resolve_skills(parse_arm("skill:skills/bare"), tmp_path)] == ["bare"] + + +def test_repo_compare_selects_a_skill_arm_named_from_a_path_with_slashes(): + from dokimi.repos.compare import parse_selector, selector_matches + + arm = str(parse_arm("guidance:raw+skill:skills/lint")) + assert parse_selector(arm) == (None, arm) + assert parse_selector(f"b1/{arm}") == ("b1", arm) + assert selector_matches(parse_selector(f"b1/{arm}"), {"arm": arm, "batch": "b1"}) From 0413c5ecd367857cbf388d8469c7f28e39161863 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Wed, 21 Oct 2026 01:55:57 -0700 Subject: [PATCH 09/33] fix(repo): type the skill paths _pin_arms compares Arm.skills is a tuple of (kind, path) pairs; the annotation said tuple[str, ...] and mypy strict failed on it. Add a test for the guard, which no test ran: two paths with one directory name in one invocation are refused. Signed-off-by: Alexander Rybkin --- dokimi/dokimi/commands/repo_run.py | 2 +- dokimi/tests/test_repo_skill_arms.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index 5a8f9b40..4cf47aef 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -238,7 +238,7 @@ def _pin_arms(arms: list[Arm], git_dir: Path, source: str) -> list[str]: worktree = None if remote else Path(parsed.location) # HEAD of a linked worktree is not the main one's pinned: list[str] = [] # Arm objects: run_batch hands each to prepare_workspace first_path: dict[str, str | None] = {} - first_skills: dict[str, tuple[str, ...]] = {} + first_skills: dict[str, tuple[tuple[str, str], ...]] = {} for arm in arms: named = pin_arm(arm, git_dir, fetch=remote, worktree_dir=worktree) if first_skills.setdefault(named, named.skills) != named.skills: # skill arms are named by directory diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 79bd9a71..013948c1 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -262,6 +262,16 @@ def test_arms_that_share_a_result_directory_are_refused(tmp_path): check_arms([parse_arm("skill-catalog:s"), parse_arm("skill:catalog-s")], tmp_path, []) +def test_two_skill_paths_with_one_directory_name_in_one_run_are_refused(setup, capsys): + _skill(setup["root"], ".agent-eval/other/lint", "lint") # both arms are named skill:lint + git(setup["root"], "add", "-A") + git(setup["root"], "commit", "-qm", "other lint") + with pytest.raises(SystemExit): + _run(setup, "--arm", "skill:skills/lint,skill:other/lint", "--task", "fix_lint") + assert "name different skill paths" in capsys.readouterr().err + assert not list((setup["out"] / "b1").glob("fix_lint__*")) + + def test_claude_code_gets_mcp_config_only_when_the_workspace_has_the_file(tmp_path, monkeypatch): runner = FakeRunner() monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: runner) From 0c8a6e7b9cd0832333d87cc31b49a4da30a3093a Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Wed, 21 Oct 2026 01:56:54 -0700 Subject: [PATCH 10/33] fix(repo): refuse a resume that swaps the skills of an arm A skill arm is named by its directories, so resuming a batch with another path of that name, or after editing a host skill, pooled two skills in one row. The batch now records each skill arm's tree sha256 in _eval/skills.json and refuses a mismatch; a new arm is added, the same bytes resume. Signed-off-by: Alexander Rybkin --- ...0261020_090000_napetrov_repo-skill-arms.md | 3 ++- dokimi/docs/repo-evaluation-contract.md | 4 ++- dokimi/dokimi/commands/repo_run.py | 26 +++++++++++++++++++ dokimi/tests/test_repo_skill_arms.py | 26 +++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index 1a8aa2c9..4241ce46 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -7,7 +7,8 @@ cache read/write token counts. `repo tasks check --arm` rejects a skill path that does not exist, and a skill whose `SKILL.md` `name` differs from its directory name. The arm is named by the directory (`skill:lint`), so - `repo compare` can select it. + `repo compare` can select it. Resuming a batch with other skill content + under the same arm name is refused. - **`args_if_present` in harness profiles** — arguments appended only when a file exists in the workspace and the tool mode is `all`. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index fdbac5ea..646b72be 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -320,7 +320,9 @@ The `name` in a skill's `SKILL.md` has to equal its directory name. A skill is r any cell runs if it holds a link that copying cannot take (out of the skill, dangling, looping, to a directory) or a file that is not regular. Preparation also refuses to write through a `.claude` or `.claude/skills` the repository -ships as a link. +ships as a link. A batch records the content of each skill arm it ran +(`_eval/skills.json`), so resuming it with another path of the same +directory name, or with edited skill content, is refused. `meta.json` records the base (`base_rev`, `base_sha`, `base_date`), the arm, the SHA of every applied patch, and `start_sha`: the workspace `HEAD` after diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index 4cf47aef..2c3c661f 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -196,6 +196,7 @@ def _run_locked(args: argparse.Namespace, arms: list[str], batch_dir: Path, lock ctx, _ = _context(args, batch_dir, pin=True) tasks = load_tasks(ctx.content_dir, ctx.manifest, args.task) check_arms(arms, ctx.content_dir, tasks) # skill paths exist, catalogs hold each expected_skill + skill_pin = _skill_pin(batch_dir, arms, ctx.content_dir) # a resume may not swap an arm's skills validate_tasks(tasks) # refuse malformed host requirements before containers start options = [AgentOptions(profile=profile, model=m, harness_bin=harness_bin, agent_network=args.agent_network, tools=args.tools, timeout_s=args.timeout, max_turns=args.max_turns, @@ -213,6 +214,9 @@ def _run_locked(args: argparse.Namespace, arms: list[str], batch_dir: Path, lock raise ValueError("the agent's container is not isolated: " + "; ".join(problems)) print(f"{profile.name} {version} in {ctx.image}; {len(tasks)} task(s) x {len(arms)} arm(s) x " f"{len(options)} model(s) x {args.reps} rep(s) -> {batch_dir}", file=sys.stderr) + if skill_pin is not None: + (batch_dir / "_eval" / SKILL_PIN).write_text(json.dumps(skill_pin, sort_keys=True) + "\n", + encoding="utf-8") summary = run_batch(ctx, tasks, arms, options, args.reps, batch_dir, jobs=args.jobs, force=args.force, max_batch_usd=args.max_batch_usd) except _errors() as exc: @@ -227,6 +231,28 @@ def _run_locked(args: argparse.Namespace, arms: list[str], batch_dir: Path, lock sys.exit(1) +SKILL_PIN = "skills.json" + + +def _skill_pin(batch_dir: Path, arms: list[str], content_dir: Path) -> dict[str, list[list[str]]] | None: + """The batch's ``_eval/skills.json`` with this run's skill arms added (``None``: nothing to add): arm name -> + [skill name, tree sha256] of each skill. A skill arm is named by its directories, so a resume with another + path or edited content would add cells of other skills to the arm's row; that is refused.""" + from dokimi.repos.arms import resolve_skills, tree_sha256 + from dokimi.repos.workspace import parse_arm + + now = {str(a): [[s.name, tree_sha256(s.dir)] for s in resolve_skills(a, content_dir)] + for a in map(parse_arm, arms) if a.skills} + path = batch_dir / "_eval" / SKILL_PIN + old = json.loads(path.read_text(encoding="utf-8")) if path.is_file() else {} + for arm, skills in now.items(): + if old.get(arm, skills) != skills: + was, new = (", ".join(f"{name} {sha[:12]}" for name, sha in pin) for pin in (old[arm], skills)) + raise ValueError(f"batch {batch_dir.name} ran arm {arm} with skills {was}, not {new}; " + f"use another --batch") + return {**old, **now} if now.keys() - old.keys() else None + + def _pin_arms(arms: list[Arm], git_dir: Path, source: str) -> list[str]: """Each arm under the name its cells get, resolved once for the whole batch (a revision to its full SHA, so a branch that moves later names another arm); two spellings of one arm run once.""" diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 013948c1..5c00ed0b 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -272,6 +272,32 @@ def test_two_skill_paths_with_one_directory_name_in_one_run_are_refused(setup, c assert not list((setup["out"] / "b1").glob("fix_lint__*")) +def test_a_resumed_batch_refuses_other_skills_under_one_arm_name(setup, tmp_path, capsys): + _skill(setup["root"], ".agent-eval/other/lint", "lint") # same directory name, other bytes + write(setup["root"], ".agent-eval/other/lint/extra.md", "Other lint.\n") + git(setup["root"], "add", "-A") + git(setup["root"], "commit", "-qm", "other lint") + host = tmp_path / "host/lint" + _skill(tmp_path, "host/lint", "lint") + write(tmp_path, "host/lint/extra.md", "Host lint.\n") + batch = setup["out"] / "b1" + _run(setup, "--arm", "skill:skills/lint,skill-catalog:skills", "--task", "fix_lint") + _run(setup, "--arm", "skill:./skills/lint", "--task", "fix_lint", "--reps", "2") # same bytes: one arm + cells = sorted(p.name for p in batch.glob("fix_lint__*")) + capsys.readouterr() + for arm in ("skill:other/lint", f"skill:{host}"): + with pytest.raises(SystemExit): + _run(setup, "--arm", arm, "--task", "fix_lint", "--reps", "3") + assert "ran arm skill:lint with skills lint " in capsys.readouterr().err + assert sorted(p.name for p in batch.glob("fix_lint__*")) == cells + _run(setup, "--batch", "b2", "--arm", f"skill:{host}", "--task", "fix_lint") + (host / "SKILL.md").write_text("---\nname: lint\ndescription: lint things\n---\nEdited.\n") + with pytest.raises(SystemExit): + _run(setup, "--batch", "b2", "--arm", f"skill:{host}", "--task", "fix_lint", "--reps", "2") + assert "use another --batch" in capsys.readouterr().err + assert not list((setup["out"] / "b2").glob("*__r2")) + + def test_claude_code_gets_mcp_config_only_when_the_workspace_has_the_file(tmp_path, monkeypatch): runner = FakeRunner() monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: runner) From 77851553dcd805ac7a4be49373f108e6693897dc Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Wed, 21 Oct 2026 02:01:52 -0700 Subject: [PATCH 11/33] docs(repo): args_if_present applies in the all tool mode only 4e4e4029 limited the arguments to --tools all and updated the changelog and the contract, but the profile schema still said they apply whenever the file exists. Signed-off-by: Alexander Rybkin --- dokimi/dokimi/schemas/harness_profile.v1.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dokimi/dokimi/schemas/harness_profile.v1.json b/dokimi/dokimi/schemas/harness_profile.v1.json index 09d7f2f7..60c5d8e2 100644 --- a/dokimi/dokimi/schemas/harness_profile.v1.json +++ b/dokimi/dokimi/schemas/harness_profile.v1.json @@ -46,7 +46,7 @@ "prompt_suffix": {"type": "string", "description": "Appended to every task prompt after a blank line, for what the harness needs the agent to know about running headless (for example that background commands are refused)."}, "args_if_present": { "type": "object", - "description": "Workspace file name -> arguments appended after the tool-mode arguments when that file exists in the run's workspace (for example --mcp-config .mcp.json, so a repo's MCP servers start only when it ships them).", + "description": "Workspace file name -> arguments appended after the tool-mode arguments when that file exists in the run's workspace and the tool mode is `all` (--tools none and read do not limit MCP tools; for example --mcp-config .mcp.json, so a repo's MCP servers start only when it ships them).", "additionalProperties": {"type": "array", "items": {"type": "string"}} }, "config_files": { From ee9d0177fc9b6c5131d3fcd7f23731ee92bb7790 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Wed, 21 Oct 2026 02:02:10 -0700 Subject: [PATCH 12/33] fix(repo): a harness that never started leaves skill usage unmeasured An agent that exits before its first turn (for example on an MCP config it rejects) leaves an empty trace that reads without error, so the cell was recorded as measured and counted as not using the skill. A trace with no event and no result line is now unmeasured, as an unreadable one is. Signed-off-by: Alexander Rybkin --- dokimi/dokimi/repos/report.py | 4 ++-- dokimi/dokimi/repos/runner.py | 2 +- dokimi/tests/repo_eval_fixture.py | 3 +++ dokimi/tests/test_repo_skill_arms.py | 15 +++++++++++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index db5da575..dd1f8787 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -19,8 +19,8 @@ - skill arms add ``skill_invoked`` (cells whose agent used the expected skill, or any installed one when the task names none) and, when the task has an ``expected_skill``, ``skill_selected_correct`` (cells that used it first). - Cells whose trace could not be read are left out of both (``skill_measured`` - counts the rest). + Cells whose trace could not be read, or holds no event and no result (the + harness did not start), are left out of both (``skill_measured`` counts the rest). A warning is printed when the trace says a model other than the requested one answered a cell (``model_resolved``): a harness alias can pin an older model. diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index f2693102..64b6680e 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -209,7 +209,7 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, cache_write_tokens=transcript.cache_write_tokens, usage_scope=transcript.usage_scope, subagent_calls=transcript.subagent_calls, plugins=transcript.plugins, over_budget=cost is not None and cost > caps["budget_usd"], trace_errors=transcript.errors[:5]) - if meta.get("skills"): + if meta.get("skills") and (transcript.events or transcript.num_turns is not None): # else it never started skills = skill_usage(transcript.events, [s["name"] for s in meta["skills"]], task.expected_skill) graded = run_entry(ctx, "grade", task, rd, grade_network, container_name(rd.name, "grade")) grade = outcome(task, rd, graded) diff --git a/dokimi/tests/repo_eval_fixture.py b/dokimi/tests/repo_eval_fixture.py index 3c984503..9fba46e4 100644 --- a/dokimi/tests/repo_eval_fixture.py +++ b/dokimi/tests/repo_eval_fixture.py @@ -71,6 +71,9 @@ assert os.environ.get("FAKE_CONFIG_DIR"), "config dir not set" assert "SECRET_NOT_ALLOWED" not in os.environ, "environment not scrubbed" action = os.environ.get("FAKE_ACTION", "fix") +if action == "startfail": # the harness refuses its configuration before the first turn + sys.stderr.write("Error: Invalid MCP configuration\n") + sys.exit(1) cwd = Path.cwd() # Guard against --model missing or being the last arg try: diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 5c00ed0b..29cdbcdb 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -250,6 +250,21 @@ def cell(name, skills): assert "1/1 skill invoked" in format_report(summarize(batch)) +def test_a_harness_that_never_started_leaves_skill_usage_unmeasured(setup, monkeypatch): + monkeypatch.setenv("FAKE_ACTION", "startfail") # exit 1, no trace: not "the agent ignored the skill" + _run(setup, "--arm", "guidance:raw+skill:skills/lint", "--task", "fix_lint", "--reps", "2") + monkeypatch.setenv("FAKE_ACTION", "fix") # ran, used no skill: measured + _run(setup, "--batch", "b2", "--arm", "guidance:raw+skill:skills/lint", "--task", "fix_lint") + for batch, measured in (("b1", False), ("b2", True)): + cells = list((setup["out"] / batch).glob("fix_lint__*")) + assert cells + for rd in cells: + skills = json.loads((rd / "result.json").read_text())["skills"] + assert skills["measured"] is measured and skills["skill_invoked"] is (False if measured else None) + assert "skill invoked" not in format_report(summarize(setup["out"] / "b1")) + assert "0/1 skill invoked" in format_report(summarize(setup["out"] / "b2")) + + def test_arms_that_share_a_result_directory_are_refused(tmp_path): _skill(tmp_path, "a/b", "b") _skill(tmp_path, "x/b", "b") From c3527d951db5410df0e630d12e142802a04543a3 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Wed, 21 Oct 2026 02:02:44 -0700 Subject: [PATCH 13/33] test(repo): pin skill usage, the Skill command field and expected_skill These mutants passed the suite: skill_invoked as bool(used), the Skill event without the command fallback, the expected_skill name check disabled, the report's selection count for tasks without expected_skill, and the frontmatter quote strip removed. Each now fails a test. Signed-off-by: Alexander Rybkin --- dokimi/tests/test_repo_manifest.py | 2 ++ dokimi/tests/test_repo_skill_arms.py | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/dokimi/tests/test_repo_manifest.py b/dokimi/tests/test_repo_manifest.py index bdd81c26..d404462e 100644 --- a/dokimi/tests/test_repo_manifest.py +++ b/dokimi/tests/test_repo_manifest.py @@ -176,6 +176,8 @@ def test_missing_and_non_yaml_manifest(tmp_path): ({"egress": ["*.org"]}, "egress must be a list"), ({"egress": ["pypi.*"]}, "egress must be a list"), ({"egress": ["py*.org"]}, "egress must be a list"), + ({"expected_skill": "skills/lint"}, "expected_skill must be a skill name"), # a path never matches usage + ({"expected_skill": 3}, "expected_skill must be a skill name"), ], ) def test_bad_task_is_reported(tmp_path, task_extra, needle): diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 29cdbcdb..546a00d4 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -87,6 +87,31 @@ def test_skill_usage_from_skill_calls_and_skill_md_reads(): assert used["skill_invoked"] is True and used["skill_selected_correct"] is False none = skill_usage([], ["lint"], None) assert none["skill_invoked"] is False and none["skill_selected_correct"] is None + wrong = skill_usage([{"kind": "tool", "name": "Skill", "skill": "format"}], ["format", "lint"], "lint") + assert wrong["skills_used"] == ["format"] and wrong["skill_invoked"] is False # another skill is not it + plugin = skill_usage([{"kind": "tool", "name": "Skill", "skill": "plug:lint"}], ["lint"], "lint") + assert plugin["skill_invoked"] is True and plugin["skill_selected_correct"] is True + + +def test_an_older_claude_code_names_the_skill_in_command(): + raw = json.dumps({"type": "assistant", "message": {"content": [ + {"type": "tool_use", "id": "a", "name": "Skill", "input": {"command": "lint"}}]}}) + assert convert("claude-stream-json", raw, "/w").events == [{"t": None, "kind": "tool", "name": "Skill", + "skill": "lint"}] + + +def test_a_quoted_frontmatter_name_is_the_directory_name(tmp_path): + for quote in ('"', "'"): + write(tmp_path, "skills/lint/SKILL.md", f"---\nname: {quote}lint{quote}\ndescription: x\n---\nDo lint.\n") + assert [s.name for s in resolve_skills(parse_arm("skill:skills/lint"), tmp_path)] == ["lint"] + + +def test_a_task_without_expected_skill_reports_no_selection(setup, monkeypatch): + monkeypatch.setenv("FAKE_SKILLS", "lint") + _run(setup, "--arm", "skill:skills/lint", "--task", "fix_add") # fix_add names no expected_skill + row = summarize(setup["out"] / "b1")["rows"][0] + assert (row["skill_measured"], row["skill_invoked"]) == (1, 1) and "skill_selected_correct" not in row + assert "skill selected correct" not in format_report(summarize(setup["out"] / "b1")) def test_catalog_arm_counts_the_expected_skill_picked_first(setup, monkeypatch): From 1e3886714b7a41f43cd85bb138d6551e2bd3f00a Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Wed, 21 Oct 2026 13:16:56 +0000 Subject: [PATCH 14/33] docs(repo): cache token counts are only new as report means Signed-off-by: Anton Volkov --- .../changelog.d/20261020_090000_napetrov_repo-skill-arms.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index 4241ce46..eacd9ee0 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -3,8 +3,9 @@ - **`repo run` skill arms** — `--arm guidance:none+skill:` installs one skill under `.claude/skills//`, and `skill-catalog:` installs every skill in a directory. `result.json` records `skills` - (`skills_used`, `skill_invoked`, `skill_selected_correct`) and the agent's - cache read/write token counts. `repo tasks check --arm` rejects a skill + (`skills_used`, `skill_invoked`, `skill_selected_correct`). + `repo report --json` gives the mean input, output and cache token counts per row. + `repo tasks check --arm` rejects a skill path that does not exist, and a skill whose `SKILL.md` `name` differs from its directory name. The arm is named by the directory (`skill:lint`), so `repo compare` can select it. Resuming a batch with other skill content From 871109b208d12dffbe986e02b5ccbd7809e904c0 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Wed, 21 Oct 2026 13:16:56 +0000 Subject: [PATCH 15/33] docs(repo): document skills and expected_skill in the contract Signed-off-by: Anton Volkov --- dokimi/docs/repo-evaluation-contract.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 646b72be..58765ea1 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -250,6 +250,7 @@ belongs to the repository's grader and is passed through unread. | `timeout_s`, `max_turns`, `budget_usd` | no | framework | per-task caps, overriding run defaults; each a finite number > 0 | | `host` | no | framework | `{"min_cpus": , "cpu_vendors": [], "cpu_flags": []}`, every key optional: what the hidden test needs to show the bug (a thread race that needs 3+ CPUs, an ISA path). Vendor and flag names are `/proc/cpuinfo`'s (`GenuineIntel`, `avx512f`); both match in any case, and a vendor other than `GenuineIntel`, `AuthenticAMD`, `HygonGenuine`, `CentaurHauls`, `Shanghai` is refused (`repo validate` too). A probe that read neither vendor nor flags from `/proc/cpuinfo` is a probe failure, not a miss They are checked against a probe container started like the grading one: CPUs are its affinity mask capped by its cgroup CPU quota (`docker --cpus`). On a host that misses them the task is unsupported (§4.3), and the probed facts go to `meta.json` `host` of a cell that runs, or to its `unsupported.json` `host` when it does not | | `egress` | no | framework | extra hosts (DNS names, optional `:port`; `*` stands for one whole label and the last two labels are literal, so `*.pythonhosted.org` passes and `*` or `*.org` does not) this task's agent and entry points reach through the proxy (§2), for example `["pypi.org", "files.pythonhosted.org"]` | +| `expected_skill` | no | framework | the name (directory name) of the skill the agent should use; a `skill-catalog:` arm must install it, or the arm is refused before anything runs. `result.json` `skills` records whether the agent used it and whether it used it first | | anything else | — | repository | for example `hidden_files`, `targets`, `regression_targets`, `manifest` | `setup_patch` (a single string) is accepted as a synonym for a one-element @@ -384,6 +385,17 @@ Code's `modelUsage`), `main` when the harness reported the parent's only, and `null` when it reported none. `subagent_calls` counts the parent's and subagents' `Agent`/`Task` calls; `null` when the harness wrote no result record. +A cell of an arm that installs skills also has a `skills` object in +`result.json`: `measured`, `skills_installed` (the installed skill names), +`skills_used` (the installed skills the agent called with the `Skill` tool or +whose `SKILL.md` it read, in order of first use), `expected_skill` (the task's, +or `null`), `skill_invoked` (the expected skill was used; any installed skill +when the task names none) and `skill_selected_correct` (the expected skill was +the first one used; `null` when the task names none). When the trace could not +be read (the harness never started), `measured` is `false` and `skill_invoked` +and `skill_selected_correct` are `null`: the usage is unknown, not "did not use +the skill". `repo report` leaves such cells out of its skill counts. + ### 4.2 `grade.json` (written by `grade` to `{run_dir}/grade.json`) ```json From fee84eeeae7a9fbff822bd7c708e4c877331f147 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 11:54:22 -0700 Subject: [PATCH 16/33] fix(repo): read a skill's name from the top-level frontmatter key; pin skill-arm behaviour in tests Signed-off-by: Nikolay Petrov --- dokimi/dokimi/repos/arms.py | 18 ++++++++++----- dokimi/tests/test_repo_arms.py | 33 ++++++++++++++++++++++++++++ dokimi/tests/test_repo_skill_arms.py | 27 +++++++++++++++++++++++ 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 2483b95d..29dd71c5 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -25,6 +25,8 @@ from pathlib import Path, PurePosixPath from typing import Any +import yaml + SKILL_KINDS = ("skill", "skill-catalog") SKILLS_DIR = ".claude/skills" @@ -95,17 +97,21 @@ def _named(arm: Any, directory: Path, written: str) -> Skill: def _frontmatter_name(skill_md: Path) -> str | None: - """``name`` in the YAML frontmatter of ``skill_md``, ``None`` without one.""" - lines = skill_md.read_text(encoding="utf-8", errors="replace").splitlines() + """Top-level ``name`` in the YAML frontmatter of ``skill_md``, ``None`` without one (or unreadable YAML).""" + lines = skill_md.read_text(encoding="utf-8-sig", errors="replace").splitlines() if not lines or lines[0].strip() != "---": return None + block: list[str] = [] for line in lines[1:]: if line.strip() == "---": break - key, sep, value = line.partition(":") - if sep and key.strip() == "name": - return value.strip().strip("\"'") or None - return None + block.append(line) + try: + meta = yaml.safe_load("\n".join(block)) + except yaml.YAMLError: + return None + name = meta.get("name") if isinstance(meta, dict) else None + return str(name).strip() or None if name is not None else None def _uncopyable(skill_dir: Path) -> tuple[Path, str] | None: diff --git a/dokimi/tests/test_repo_arms.py b/dokimi/tests/test_repo_arms.py index dbc32d47..72f55a7d 100644 --- a/dokimi/tests/test_repo_arms.py +++ b/dokimi/tests/test_repo_arms.py @@ -543,3 +543,36 @@ def test_pin_arms_shares_one_cell_for_two_paths_to_the_same_patch(toy, capsys): assert err.count("share one cell") == 1 assert f"{same_bytes.resolve()} and {a.resolve()} are the same patch" in err assert f"meta.json names {a.resolve()}" in err + + +def _toy_skill(toy, name="lint"): + write(toy["tmp"] / "content", f"skills/{name}/SKILL.md", f"---\nname: {name}\ndescription: x\n---\nDo it.\n") + + +@pytest.mark.parametrize("arm", ["skill:skills/lint", "guidance:raw+skill:skills/lint"]) +def test_a_raw_arm_with_a_skill_leaves_the_repository_guidance_alone(toy, arm): + _toy_skill(toy) + meta = prepare_workspace(toy["git_dir"], toy["manifest"], toy["tasks"]["fix_add"], arm, toy["tmp"] / "rd", + content_dir=toy["tmp"] / "content") + ws = toy["tmp"] / "rd" / "workspace" + assert meta["arm_files"] == [] and [s["name"] for s in meta["skills"]] == ["lint"] + assert (ws / "AGENTS.md").read_text() == "# guidance\n" and (ws / "sub/AGENTS.md").is_file() + assert (ws / ".claude/skills/lint/SKILL.md").is_file() + + +def test_a_bridge_arm_with_a_skill_still_needs_a_bridge_key(toy): + _toy_skill(toy) + with pytest.raises(PrepError, match="needs a harness profile"): + prepare_workspace(toy["git_dir"], toy["manifest"], toy["tasks"]["fix_add"], + "guidance:bridge+skill:skills/lint", toy["tmp"] / "rd", content_dir=toy["tmp"] / "content") + + +def test_a_pinned_rev_arm_keeps_its_skill(toy): + _toy_skill(toy) + arm = pin_arm(parse_arm("guidance:rev:pr+skill:skills/lint"), toy["git_dir"]) + assert str(arm) == f"guidance:rev:{toy['pr']}+skill:lint" + assert arm.skills == (("skill", "skills/lint"),) and arm.base == f"guidance:rev:{toy['pr']}" + meta = prepare_workspace(toy["git_dir"], toy["manifest"], toy["tasks"]["fix_add"], arm, toy["tmp"] / "rd", + content_dir=toy["tmp"] / "content") + assert [s["name"] for s in meta["skills"]] == ["lint"] and meta["arm_rev"]["sha"] == toy["pr"] + assert (toy["tmp"] / "rd/workspace/AGENTS.md").read_text() == "# pr guidance\n" diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 546a00d4..109afa41 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -93,6 +93,16 @@ def test_skill_usage_from_skill_calls_and_skill_md_reads(): assert plugin["skill_invoked"] is True and plugin["skill_selected_correct"] is True +def test_only_a_read_of_skill_md_counts_as_using_a_skill(): + for path in (".claude/skills/lint/references/x.md", ".claude/skills/lint/notes.md"): + used = skill_usage([{"kind": "read", "path": path}], ["lint"], "lint") + assert used["skills_used"] == [] and used["skill_invoked"] is False + twice = skill_usage([{"kind": "tool", "name": "Skill", "skill": "lint"}, + {"kind": "tool", "name": "Skill", "skill": "lint"}, + {"kind": "read", "path": ".claude/skills/lint/SKILL.md"}], ["lint"], "lint") + assert twice["skills_used"] == ["lint"] + + def test_an_older_claude_code_names_the_skill_in_command(): raw = json.dumps({"type": "assistant", "message": {"content": [ {"type": "tool_use", "id": "a", "name": "Skill", "input": {"command": "lint"}}]}}) @@ -381,3 +391,20 @@ def test_repo_compare_selects_a_skill_arm_named_from_a_path_with_slashes(): assert parse_selector(arm) == (None, arm) assert parse_selector(f"b1/{arm}") == ("b1", arm) assert selector_matches(parse_selector(f"b1/{arm}"), {"arm": arm, "batch": "b1"}) + + +@pytest.mark.parametrize("front", [ + "name: lint # the linter\ndescription: x", + "name: >\n lint\ndescription: x", + "metadata:\n name: other\nname: lint\ndescription: x", + "description: |\n name: foo\nname: lint", + "name: lint\nmetadata:\n name: other", +]) +def test_a_valid_frontmatter_name_is_read_from_the_top_level_key_only(tmp_path, front): + write(tmp_path, "skills/lint/SKILL.md", f"---\n{front}\n---\nname: other\nBody.\n") + assert [s.name for s in resolve_skills(parse_arm("skill:skills/lint"), tmp_path)] == ["lint"] + + +def test_a_nested_name_alone_is_not_the_skill_name(tmp_path): + write(tmp_path, "skills/lint/SKILL.md", "---\nmetadata:\n name: other\n---\nBody.\n") + assert [s.name for s in resolve_skills(parse_arm("skill:skills/lint"), tmp_path)] == ["lint"] From 5c047415f2088decc64c4cdf62ed5c0fd54f382f Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 12:01:03 -0700 Subject: [PATCH 17/33] docs(changelog): MCP servers now start under --tools all and shift baselines Signed-off-by: Nikolay Petrov --- dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index eacd9ee0..9c7407c1 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -20,3 +20,6 @@ `.mcp.json` ran with no MCP tools. It now passes `--mcp-config .mcp.json` when that file is present and `--tools all` is used (`--tools none`/`read` do not limit MCP tools, so they start no MCP server). + This shifts baselines: a repo that ships `.mcp.json` now starts its MCP + servers under `--tools all`, so its results differ from earlier runs of the + same repo and arm. Compare only batches run after this change. From 0f73fcaff8be59e12a87124386557ed7ffd06e2d Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 12:16:04 -0700 Subject: [PATCH 18/33] fix(repo): refuse a skill the task base ships before any cell; record applied harness args check_arms now looks the skill's target up in each task's base, so repo tasks check --arm and repo run refuse a skill the repository already has instead of every cell crashing in prep. meta.json records the args_if_present names a cell ran with. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/commands/repo_run.py | 4 ++-- dokimi/dokimi/repos/arms.py | 16 ++++++++++--- dokimi/dokimi/repos/runner.py | 5 ++-- dokimi/tests/test_repo_skill_arms.py | 36 ++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index 2c3c661f..6192a361 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -146,7 +146,7 @@ def cmd_repo_tasks_check(args: argparse.Namespace) -> None: ctx, _ = _context(args, out) tasks = load_tasks(ctx.content_dir, ctx.manifest, args.task) arms = [a for spec in args.arm or () for a in spec.split(",") if a] - check_arms([parse_arm(a) for a in arms], ctx.content_dir, tasks) # skill paths exist, catalogs hold each expected_skill + check_arms([parse_arm(a) for a in arms], ctx.content_dir, tasks, ctx.git_dir) # skill paths exist, catalogs hold each expected_skill if arms: print(f"{len(arms)} arm(s) ok: {', '.join(arms)}", file=sys.stderr) print(f"{len(tasks)} task(s), image {ctx.image}{' (warm: with network)' if args.warm else ''}", @@ -195,7 +195,7 @@ def _run_locked(args: argparse.Namespace, arms: list[str], batch_dir: Path, lock lock.enter_context(batch_lock(batch_dir)) # before _context, which rewrites _eval/content ctx, _ = _context(args, batch_dir, pin=True) tasks = load_tasks(ctx.content_dir, ctx.manifest, args.task) - check_arms(arms, ctx.content_dir, tasks) # skill paths exist, catalogs hold each expected_skill + check_arms(arms, ctx.content_dir, tasks, ctx.git_dir) # skill paths exist, catalogs hold each expected_skill skill_pin = _skill_pin(batch_dir, arms, ctx.content_dir) # a resume may not swap an arm's skills validate_tasks(tasks) # refuse malformed host requirements before containers start options = [AgentOptions(profile=profile, model=m, harness_bin=harness_bin, agent_network=args.agent_network, diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 29dd71c5..45528fa7 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -27,6 +27,8 @@ import yaml +from dokimi import process + SKILL_KINDS = ("skill", "skill-catalog") SKILLS_DIR = ".claude/skills" @@ -146,15 +148,23 @@ def _skill_path(path: str, content_dir: Path) -> Path: return full -def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any]) -> None: +def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Path | None = None) -> None: """Find the skills of every parsed arm before anything runs; a catalog must hold each task's - ``expected_skill``. Two arms that would share a name or a result directory are refused.""" + ``expected_skill``. Two arms that would share a name or a result directory are refused, and so is a skill + the base of a task (in ``git_dir``) already ships, which preparing the workspace would refuse cell by cell.""" cells: dict[str, str] = {} # cell-name slug of an arm -> the arm spec that owns it for arm in arms: cell = slug(str(arm).replace(":", "-")) if cells.setdefault(cell, arm.spec) != arm.spec: raise ArmError(f"arms {cells[cell]!r} and {arm.spec!r} would share one result directory ({cell})") - names = {s.name for s in resolve_skills(arm, content_dir)} + skills = resolve_skills(arm, content_dir) + names = {s.name for s in skills} + for task in tasks if git_dir else (): + for skill in skills: + proc = process.run(["git", "-C", str(git_dir), "cat-file", "-e", f"{task.base}:{skill.target}"], + env=process.GIT_ENV, timeout=60, capture_limit=None) + if proc.returncode == 0: + raise ArmError(f"arm {arm.spec!r}: the base of task {task.id} already has {skill.target}") if any(kind == "skill-catalog" for kind, _ in arm.skills): for task in tasks: expected = task.raw.get("expected_skill") diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index 64b6680e..d0d59be9 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -174,10 +174,11 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, env.update({k: v for k, v in harness_env.items() if k not in _HOST_ONLY_ENV and k not in passed}) env["DOKIMI_HOOK_LOG"] = f"{CONFIG_DIR}/hook.log" opts.profile.write_config(rd / "config", {"config_dir": CONFIG_DIR, "support_dir": SUPPORT_DIR}) + present = frozenset(n for n in opts.profile.args_if_present if (rd / "workspace" / n).exists()) + meta["args_if_present"] = sorted(present) # which optional harness arguments this cell ran with argv = opts.profile.argv(prompt=opts.profile.prompt(str(task.raw["prompt"])), model=opts.model, tools=opts.tools, max_turns=int(caps["max_turns"]), budget_usd=caps["budget_usd"], - bin_path=bin_in, present=frozenset( - n for n in opts.profile.args_if_present if (rd / "workspace" / n).exists())) + bin_path=bin_in, present=present) spec = ContainerSpec(image=ctx.image, argv=argv, workdir=WORKSPACE, mounts=mounts, env=env, pass_env=passed, network=network, name=container_name(rd.name, "agent")) result = ctx.runner.run(spec, stdout=rd / "agent.stdout", stderr=rd / "agent.stderr", diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 109afa41..aa3949ce 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -408,3 +408,39 @@ def test_a_valid_frontmatter_name_is_read_from_the_top_level_key_only(tmp_path, def test_a_nested_name_alone_is_not_the_skill_name(tmp_path): write(tmp_path, "skills/lint/SKILL.md", "---\nmetadata:\n name: other\n---\nBody.\n") assert [s.name for s in resolve_skills(parse_arm("skill:skills/lint"), tmp_path)] == ["lint"] + + +def test_a_skill_the_task_base_already_ships_is_refused_before_any_cell(tmp_path, capsys): + _skill(tmp_path / "toy", ".claude/skills/lint", "lint") + root, base = make_repo(tmp_path) + _skill(root, ".agent-eval/skills/lint", "lint") + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + task = Task("t", {"base": base}, root) + with pytest.raises(ArmError, match="base of task t already has .claude/skills/lint"): + check_arms([parse_arm("skill:skills/lint")], root / ".agent-eval", [task], root / ".git") + with pytest.raises(SystemExit): # `repo tasks check --arm` goes through the same check + args = build_parser().parse_args(["repo", "tasks", "check", f"{root}@HEAD", "--out", str(tmp_path / "chk"), + "--cache-root", str(tmp_path / "caches"), "--arm", "skill:skills/lint"]) + args.func(args) + assert "already has .claude/skills/lint" in capsys.readouterr().err + + +def test_a_cell_records_the_optional_harness_arguments_it_ran_with(tmp_path, monkeypatch): + import yaml + from repo_eval_fixture import PROFILE + write(tmp_path / "toy", ".mcp.json", "{}") + root, _ = make_repo(tmp_path) + exe, profile = fake_harness(tmp_path) + profile.write_text(yaml.safe_dump({**PROFILE, "args_if_present": {".mcp.json": ["--mcp-config", ".mcp.json"]}}), + encoding="utf-8") + monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: FakeRunner()) + monkeypatch.setenv("FAKE_ACTION", "fix") + out = tmp_path / "runs" + args = build_parser().parse_args(["repo", "run", f"{root}@HEAD", "--harness", str(profile), "--bin", str(exe), + "--model", "m-1", "--batch", "b1", "--out", str(out), + "--cache-root", str(out / "caches"), "--agent-network", "host", + "--arm", "guidance:none", "--task", "fix_add"]) + args.func(args) + meta = json.loads(next((out / "b1").glob("fix_add__*")).joinpath("meta.json").read_text()) + assert meta["args_if_present"] == [".mcp.json"] From 256c179c0a598923af94ad9866073e110614f21d Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 14:45:49 -0700 Subject: [PATCH 19/33] fix(repo): skill pre-flight reads each base once and honours guidance removal; selection rate counts only expecting cells Signed-off-by: Nikolay Petrov --- ...0261020_090000_napetrov_repo-skill-arms.md | 3 +- dokimi/dokimi/commands/repo_run.py | 4 +- dokimi/dokimi/repos/arms.py | 28 ++++++++++--- dokimi/dokimi/repos/report.py | 14 ++++--- dokimi/tests/test_repo_skill_arms.py | 40 +++++++++++++++++++ 5 files changed, 75 insertions(+), 14 deletions(-) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index 9c7407c1..5b6d1c7c 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -4,7 +4,8 @@ skill under `.claude/skills//`, and `skill-catalog:` installs every skill in a directory. `result.json` records `skills` (`skills_used`, `skill_invoked`, `skill_selected_correct`). - `repo report --json` gives the mean input, output and cache token counts per row. + `repo report --json` gives, per row, the mean of the token fields `result.json` already has + (`input_tokens`, `output_tokens`, `cache_read_tokens`, `cache_write_tokens`). `repo tasks check --arm` rejects a skill path that does not exist, and a skill whose `SKILL.md` `name` differs from its directory name. The arm is named by the directory (`skill:lint`), so diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index 6192a361..07259306 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -146,7 +146,7 @@ def cmd_repo_tasks_check(args: argparse.Namespace) -> None: ctx, _ = _context(args, out) tasks = load_tasks(ctx.content_dir, ctx.manifest, args.task) arms = [a for spec in args.arm or () for a in spec.split(",") if a] - check_arms([parse_arm(a) for a in arms], ctx.content_dir, tasks, ctx.git_dir) # skill paths exist, catalogs hold each expected_skill + check_arms([parse_arm(a) for a in arms], ctx.content_dir, tasks, ctx.git_dir, ctx.manifest.guidance_globs) # skill paths exist, catalogs hold each expected_skill if arms: print(f"{len(arms)} arm(s) ok: {', '.join(arms)}", file=sys.stderr) print(f"{len(tasks)} task(s), image {ctx.image}{' (warm: with network)' if args.warm else ''}", @@ -195,7 +195,7 @@ def _run_locked(args: argparse.Namespace, arms: list[str], batch_dir: Path, lock lock.enter_context(batch_lock(batch_dir)) # before _context, which rewrites _eval/content ctx, _ = _context(args, batch_dir, pin=True) tasks = load_tasks(ctx.content_dir, ctx.manifest, args.task) - check_arms(arms, ctx.content_dir, tasks, ctx.git_dir) # skill paths exist, catalogs hold each expected_skill + check_arms(arms, ctx.content_dir, tasks, ctx.git_dir, ctx.manifest.guidance_globs) # skill paths exist, catalogs hold each expected_skill skill_pin = _skill_pin(batch_dir, arms, ctx.content_dir) # a resume may not swap an arm's skills validate_tasks(tasks) # refuse malformed host requirements before containers start options = [AgentOptions(profile=profile, model=m, harness_bin=harness_bin, agent_network=args.agent_network, diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 45528fa7..6f2075c5 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -148,22 +148,38 @@ def _skill_path(path: str, content_dir: Path) -> Path: return full -def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Path | None = None) -> None: +def _base_skill_files(git_dir: Path, base: str) -> list[str]: + """The files the commit ``base`` tracks under ``.claude/skills`` (one ``git ls-tree`` per base).""" + proc = process.run(["git", "-C", str(git_dir), "ls-tree", "-r", "-z", "--name-only", base, "--", ".claude/skills"], + env=process.GIT_ENV, timeout=60, capture_limit=None) + return [p for p in proc.stdout.split("\0") if p] if proc.returncode == 0 else [] + + +def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Path | None = None, + guidance_globs: tuple[str, ...] | list[str] = ()) -> None: """Find the skills of every parsed arm before anything runs; a catalog must hold each task's ``expected_skill``. Two arms that would share a name or a result directory are refused, and so is a skill - the base of a task (in ``git_dir``) already ships, which preparing the workspace would refuse cell by cell.""" + the base of a task (in ``git_dir``) already ships, which preparing the workspace would refuse cell by cell. + As preparation does, a ``guidance:none`` or pinned-revision arm first drops the base's files that match + ``guidance_globs``: only what is left under the skill's directory counts as shipped.""" + from dokimi.repos.manifest import match_glob + cells: dict[str, str] = {} # cell-name slug of an arm -> the arm spec that owns it + shipped: dict[str, list[str]] = {} # task base -> tracked files under .claude/skills for arm in arms: cell = slug(str(arm).replace(":", "-")) if cells.setdefault(cell, arm.spec) != arm.spec: raise ArmError(f"arms {cells[cell]!r} and {arm.spec!r} would share one result directory ({cell})") skills = resolve_skills(arm, content_dir) names = {s.name for s in skills} - for task in tasks if git_dir else (): + drops = arm.patch is None and (arm.base == "guidance:none" or arm.rev_sha is not None) + for task in tasks if git_dir and skills else (): + if task.base not in shipped: + shipped[task.base] = _base_skill_files(git_dir, task.base) + left = [p for p in shipped[task.base] + if not (drops and any(match_glob(g, p) for g in guidance_globs))] for skill in skills: - proc = process.run(["git", "-C", str(git_dir), "cat-file", "-e", f"{task.base}:{skill.target}"], - env=process.GIT_ENV, timeout=60, capture_limit=None) - if proc.returncode == 0: + if any(p.startswith(skill.target.rstrip("/") + "/") or p == skill.target for p in left): raise ArmError(f"arm {arm.spec!r}: the base of task {task.id} already has {skill.target}") if any(kind == "skill-catalog" for kind, _ in arm.skills): for task in tasks: diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index dd1f8787..1bf4af84 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -18,7 +18,8 @@ finished cells, as the harness reported them. - skill arms add ``skill_invoked`` (cells whose agent used the expected skill, or any installed one when the task names none) and, when the task has an - ``expected_skill``, ``skill_selected_correct`` (cells that used it first). + ``expected_skill``, ``skill_selected_correct`` (cells that used it first; ``skill_expected`` + counts the cells that have one). Cells whose trace could not be read, or holds no event and no result (the harness did not start), are left out of both (``skill_measured`` counts the rest). @@ -110,8 +111,10 @@ def summarize(batch_dir: Path) -> dict[str, Any]: if used: # cells whose trace was unreadable are not counted: no rate from a missing measurement row["skill_measured"] = len(used) row["skill_invoked"] = sum(bool(u["skill_invoked"]) for u in used) - if any(u.get("expected_skill") for u in used): - row["skill_selected_correct"] = sum(bool(u["skill_selected_correct"]) for u in used) + expecting = [u for u in used if u.get("expected_skill")] # a task naming no skill has nothing to select + if expecting: + row["skill_expected"] = len(expecting) + row["skill_selected_correct"] = sum(bool(u["skill_selected_correct"]) for u in expecting) rows.append(row) return {"batch": str(batch_dir), "cells": len(cells), "missing": missing, "rows": rows, "unsupported": load_unsupported(batch_dir), @@ -141,8 +144,9 @@ def format_report(summary: dict[str, Any]) -> str: f"{result:<22} {r['grader_failures']:>11} {r['missing']:>7} {cost:>8} {turns:>6}") noted = ("agent_timeouts", "agent_errors", "over_budget", "cost_unknown") notes = [f"{r[k]} {k.replace('_', ' ')}" for k in noted if r[k]] - notes += [f"{r[k]}/{r['skill_measured']} {k.replace('_', ' ')}" - for k in ("skill_invoked", "skill_selected_correct") if k in r] + notes += [f"{r[k]}/{r[d]} {k.replace('_', ' ')}" + for k, d in (("skill_invoked", "skill_measured"), ("skill_selected_correct", "skill_expected")) + if k in r] if notes: lines.append(" " * (width + 2) + " " + ", ".join(notes)) if summary["missing"]: diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index aa3949ce..3bc5616f 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -444,3 +444,43 @@ def test_a_cell_records_the_optional_harness_arguments_it_ran_with(tmp_path, mon args.func(args) meta = json.loads(next((out / "b1").glob("fix_add__*")).joinpath("meta.json").read_text()) assert meta["args_if_present"] == [".mcp.json"] + + +def test_the_base_check_ignores_a_skill_the_guidance_arm_removes_and_reads_each_base_once(tmp_path, monkeypatch): + _skill(tmp_path / "toy", ".claude/skills/lint", "lint") + root, base = make_repo(tmp_path) + _skill(root, ".agent-eval/skills/lint", "lint") + _skill(root, ".claude/skills/lint", "lint") + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + tasks = [Task("t1", {"base": base}, root), Task("t2", {"base": base}, root)] + content, globs = root / ".agent-eval", ["**/AGENTS.md", ".claude/skills/**"] + check_arms([parse_arm("guidance:none+skill:skills/lint")], content, tasks, root / ".git", globs) # removed first + for arm, given in (("guidance:raw+skill:skills/lint", globs), ("guidance:none+skill:skills/lint", ["**/AGENTS.md"])): + with pytest.raises(ArmError, match="already has .claude/skills/lint"): + check_arms([parse_arm(arm)], content, tasks, root / ".git", given) + from dokimi import process + calls = [] + real = process.run + monkeypatch.setattr(process, "run", lambda cmd, **kw: calls.append(cmd) or real(cmd, **kw)) + check_arms([parse_arm("guidance:none+skill:skills/lint")], content, tasks, root / ".git", globs) + assert len(calls) == 1 # two tasks on one base: one git process + + +def test_the_selected_correct_rate_counts_only_cells_whose_task_names_a_skill(tmp_path): + batch = tmp_path / "b" + for name, expected, correct in (("c1", "lint", True), ("c2", "lint", False), ("c3", None, None), ("c4", None, None)): + rd = batch / name + rd.mkdir(parents=True) + (rd / "meta.json").write_text("{}") + skills = {"measured": True, "skills_installed": ["lint"], "skills_used": ["lint"], "expected_skill": expected, + "skill_invoked": True, "skill_selected_correct": correct} + rec = {"task": "t", "arm": "skill:skills/lint", "model": "m", "score_task": False, "grade": { + "status": "ok", "success": True, "score_only": False, "score": None}, + "agent": {"timed_out": False, "is_error": False, "over_budget": False, "cost_usd": 0.1, + "num_turns": 3, "network": "none"}, "skills": skills} + (rd / "result.json").write_text(json.dumps(rec)) + summary = summarize(batch) + row = summary["rows"][0] + assert (row["skill_measured"], row["skill_expected"], row["skill_selected_correct"]) == (4, 2, 1) + assert "1/2 skill selected correct" in format_report(summary) and "4/4 skill invoked" in format_report(summary) From 22f98800a536ceac8435566f1c302d0a8b213ea5 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 15:33:40 -0700 Subject: [PATCH 20/33] fix(repo): narrow git_dir for mypy in the skill pre-flight Signed-off-by: Nikolay Petrov --- dokimi/dokimi/repos/arms.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 6f2075c5..f3995001 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -173,7 +173,9 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa skills = resolve_skills(arm, content_dir) names = {s.name for s in skills} drops = arm.patch is None and (arm.base == "guidance:none" or arm.rev_sha is not None) - for task in tasks if git_dir and skills else (): + for task in tasks if skills else (): + if git_dir is None: + break if task.base not in shipped: shipped[task.base] = _base_skill_files(git_dir, task.base) left = [p for p in shipped[task.base] From 48bc5ddb0c4b16b63e70412d93da09662689585b Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 20:42:33 -0700 Subject: [PATCH 21/33] fix(repo/arms): refuse a skill path whose leaf is not its directory; drop guidance for unpinned rev arms in check Signed-off-by: Nikolay Petrov --- dokimi/dokimi/repos/arms.py | 5 ++++- dokimi/tests/test_repo_skill_arms.py | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index f3995001..9c90bc43 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -59,6 +59,9 @@ def resolve_skills(arm: Any, content_dir: Path) -> list[Skill]: found: list[Skill] = [] for kind, path in arm.skills: root = _skill_path(path, content_dir) + if PurePosixPath(path).name != root.name: # the arm is named from the written leaf, the install from root + raise ArmError(f"arm {arm.spec!r}: {path} resolves to a directory named {root.name!r}, not " + f"{PurePosixPath(path).name!r}; point at the directory itself, as its name names the arm") if kind == "skill": if not (root / "SKILL.md").is_file(): raise ArmError(f"arm {arm.spec!r}: no SKILL.md in {path}") @@ -172,7 +175,7 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa raise ArmError(f"arms {cells[cell]!r} and {arm.spec!r} would share one result directory ({cell})") skills = resolve_skills(arm, content_dir) names = {s.name for s in skills} - drops = arm.patch is None and (arm.base == "guidance:none" or arm.rev_sha is not None) + drops = arm.patch is None and (arm.base == "guidance:none" or arm.rev is not None) for task in tasks if skills else (): if git_dir is None: break diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 3bc5616f..e9b217aa 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -484,3 +484,23 @@ def test_the_selected_correct_rate_counts_only_cells_whose_task_names_a_skill(tm row = summary["rows"][0] assert (row["skill_measured"], row["skill_expected"], row["skill_selected_correct"]) == (4, 2, 1) assert "1/2 skill selected correct" in format_report(summary) and "4/4 skill invoked" in format_report(summary) + + +def test_a_skill_path_whose_leaf_is_not_its_directory_name_is_refused(tmp_path): + _skill(tmp_path, "skills/lint-v2", "lint-v2") + (tmp_path / "skills" / "current").symlink_to("lint-v2") # the arm would be skill:current, the install lint-v2 + with pytest.raises(ArmError, match="resolves to a directory named 'lint-v2', not 'current'"): + resolve_skills(parse_arm("skill:skills/current"), tmp_path) + assert [s.name for s in resolve_skills(parse_arm("skill:skills/lint-v2"), tmp_path)] == ["lint-v2"] + + +def test_the_base_check_drops_guidance_for_an_unpinned_revision_arm(tmp_path): + _skill(tmp_path / "toy", ".claude/skills/lint", "lint") + root, base = make_repo(tmp_path) + _skill(root, ".agent-eval/skills/lint", "lint") + _skill(root, ".claude/skills/lint", "lint") + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + tasks = [Task("t1", {"base": base}, root)] + check_arms([parse_arm(f"guidance:rev:{base}+skill:skills/lint")], root / ".agent-eval", tasks, root / ".git", + [".claude/skills/**"]) # repo run pins the arm and removes the base's skill first From 939a42e2aa18af171cd8852f17bdfd6441448dd0 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Wed, 21 Oct 2026 21:09:50 -0700 Subject: [PATCH 22/33] fix(repo): count a revision arm's own skill files in the base check A guidance:rev arm drops the base's guidance files and then writes its revision's back. check_arms counted only the drop, so a revision that ships .claude/skills/ passed `repo tasks check --arm` and every cell of the arm then failed in prep with "the workspace already has". Signed-off-by: Alexander Rybkin --- dokimi/dokimi/repos/arms.py | 15 ++++++++++----- dokimi/tests/test_repo_skill_arms.py | 7 +++++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 9c90bc43..28613585 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -164,7 +164,8 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa ``expected_skill``. Two arms that would share a name or a result directory are refused, and so is a skill the base of a task (in ``git_dir``) already ships, which preparing the workspace would refuse cell by cell. As preparation does, a ``guidance:none`` or pinned-revision arm first drops the base's files that match - ``guidance_globs``: only what is left under the skill's directory counts as shipped.""" + ``guidance_globs``: only what is left under the skill's directory counts as shipped. A revision arm then + writes back its revision's files that match them, so those count as shipped too.""" from dokimi.repos.manifest import match_glob cells: dict[str, str] = {} # cell-name slug of an arm -> the arm spec that owns it @@ -176,16 +177,20 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa skills = resolve_skills(arm, content_dir) names = {s.name for s in skills} drops = arm.patch is None and (arm.base == "guidance:none" or arm.rev is not None) + rev = (arm.rev_sha or arm.rev) if arm.patch is None else None for task in tasks if skills else (): if git_dir is None: break - if task.base not in shipped: - shipped[task.base] = _base_skill_files(git_dir, task.base) + for commit in (task.base, rev): + if commit is not None and commit not in shipped: + shipped[commit] = _base_skill_files(git_dir, commit) left = [p for p in shipped[task.base] if not (drops and any(match_glob(g, p) for g in guidance_globs))] + written = [p for p in shipped[rev] if any(match_glob(g, p) for g in guidance_globs)] if rev else [] for skill in skills: - if any(p.startswith(skill.target.rstrip("/") + "/") or p == skill.target for p in left): - raise ArmError(f"arm {arm.spec!r}: the base of task {task.id} already has {skill.target}") + for where, files in ((f"the base of task {task.id}", left), (f"guidance revision {rev}", written)): + if any(p.startswith(skill.target.rstrip("/") + "/") or p == skill.target for p in files): + raise ArmError(f"arm {arm.spec!r}: {where} already has {skill.target}") if any(kind == "skill-catalog" for kind, _ in arm.skills): for task in tasks: expected = task.raw.get("expected_skill") diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index e9b217aa..d7a2bf9b 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -498,9 +498,12 @@ def test_the_base_check_drops_guidance_for_an_unpinned_revision_arm(tmp_path): _skill(tmp_path / "toy", ".claude/skills/lint", "lint") root, base = make_repo(tmp_path) _skill(root, ".agent-eval/skills/lint", "lint") - _skill(root, ".claude/skills/lint", "lint") + git(root, "rm", "-rq", ".claude/skills/lint") # HEAD's guidance has no lint skill, the task base has one git(root, "add", "-A") git(root, "commit", "-qm", "skills") tasks = [Task("t1", {"base": base}, root)] - check_arms([parse_arm(f"guidance:rev:{base}+skill:skills/lint")], root / ".agent-eval", tasks, root / ".git", + check_arms([parse_arm("guidance:rev:HEAD+skill:skills/lint")], root / ".agent-eval", tasks, root / ".git", [".claude/skills/**"]) # repo run pins the arm and removes the base's skill first + with pytest.raises(ArmError, match=f"guidance revision {base} already has .claude/skills/lint"): + check_arms([parse_arm(f"guidance:rev:{base}+skill:skills/lint")], root / ".agent-eval", tasks, + root / ".git", [".claude/skills/**"]) # ... and then writes the revision's own back From 135b273e3dc5316e1218afe43704e629696bd084 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Wed, 21 Oct 2026 21:10:04 -0700 Subject: [PATCH 23/33] fix(repo): record only the optional harness arguments a cell passed meta.json listed every args_if_present file in the workspace, so a --tools none or read cell said [".mcp.json"] though its argv had no --mcp-config. The stamp now comes from the rule argv uses, and is in the first meta.json write, so a cell whose trace fails to convert keeps it. Signed-off-by: Alexander Rybkin --- dokimi/dokimi/repos/harness_profile.py | 11 ++++++++--- dokimi/dokimi/repos/runner.py | 4 ++-- dokimi/tests/test_repo_skill_arms.py | 26 ++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/dokimi/dokimi/repos/harness_profile.py b/dokimi/dokimi/repos/harness_profile.py index 4f0da49f..5b0054fb 100644 --- a/dokimi/dokimi/repos/harness_profile.py +++ b/dokimi/dokimi/repos/harness_profile.py @@ -67,6 +67,12 @@ def import_max_depth(self) -> int | None: depth = self.guidance.get("import_max_depth") return int(depth) if depth is not None else None + def applied_args(self, tools: str, present: frozenset[str]) -> list[str]: + """The ``args_if_present`` names :meth:`argv` adds: those in ``present``, in the ``all`` tool mode only.""" + if tools != "all": # --tools does not limit MCP tools: only here is the repo's MCP config part of the arm + return [] + return sorted(present & self.args_if_present.keys()) + def argv( self, *, @@ -93,9 +99,8 @@ def argv( for element in self.invoke: if element == TOOLS_ELEMENT: out.extend(self.tools[tools]) - if tools == "all": # --tools does not limit MCP tools: only here is the repo's MCP config part of the arm - for name in sorted(present & self.args_if_present.keys()): - out.extend(self.args_if_present[name]) + for name in self.applied_args(tools, present): + out.extend(self.args_if_present[name]) else: out.append(_substitute(element, values)) return out diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index d0d59be9..d44d2953 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -160,6 +160,8 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, meta = prepare_workspace(ctx.git_dir, ctx.manifest, task, arm, rd, opts.profile, ctx.content_dir) meta.update(harness=opts.profile.name, harness_version=opts.harness_version, model=opts.model, rep=rep, batch=batch_dir.name, agent_network=network, egress_proxy=False, host=host) + present = frozenset(n for n in opts.profile.args_if_present if (rd / "workspace" / n).exists()) + meta["args_if_present"] = opts.profile.applied_args(opts.tools, present) # the ones the cell runs with _write_json(rd / "meta.json", meta) skills: dict[str, Any] | None = None @@ -174,8 +176,6 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, env.update({k: v for k, v in harness_env.items() if k not in _HOST_ONLY_ENV and k not in passed}) env["DOKIMI_HOOK_LOG"] = f"{CONFIG_DIR}/hook.log" opts.profile.write_config(rd / "config", {"config_dir": CONFIG_DIR, "support_dir": SUPPORT_DIR}) - present = frozenset(n for n in opts.profile.args_if_present if (rd / "workspace" / n).exists()) - meta["args_if_present"] = sorted(present) # which optional harness arguments this cell ran with argv = opts.profile.argv(prompt=opts.profile.prompt(str(task.raw["prompt"])), model=opts.model, tools=opts.tools, max_turns=int(caps["max_turns"]), budget_usd=caps["budget_usd"], bin_path=bin_in, present=present) diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index d7a2bf9b..857e0f9d 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -446,6 +446,32 @@ def test_a_cell_records_the_optional_harness_arguments_it_ran_with(tmp_path, mon assert meta["args_if_present"] == [".mcp.json"] +@pytest.mark.parametrize(("tools", "unreadable"), [("none", False), ("read", False), ("all", True)]) +def test_the_recorded_optional_arguments_are_the_ones_the_agent_got(tmp_path, monkeypatch, tools, unreadable): + import yaml + from repo_eval_fixture import PROFILE + write(tmp_path / "toy", ".mcp.json", "{}") + root, _ = make_repo(tmp_path) + exe, profile = fake_harness(tmp_path) + profile.write_text(yaml.safe_dump({**PROFILE, "args_if_present": {".mcp.json": ["--mcp-config", ".mcp.json"]}}), + encoding="utf-8") + runner = FakeRunner() + monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: runner) + monkeypatch.setenv("FAKE_ACTION", "fix") + if unreadable: # meta.json is rewritten only after the trace converts + monkeypatch.setattr("dokimi.repos.runner.convert", lambda *a, **k: (_ for _ in ()).throw(ValueError("bad"))) + out = tmp_path / "runs" + args = build_parser().parse_args(["repo", "run", f"{root}@HEAD", "--harness", str(profile), "--bin", str(exe), + "--model", "m-1", "--batch", "b1", "--out", str(out), + "--cache-root", str(out / "caches"), "--agent-network", "host", + "--arm", "guidance:raw", "--task", "fix_add", "--tools", tools]) + args.func(args) + meta = json.loads(next((out / "b1").glob("fix_add__*")).joinpath("meta.json").read_text()) + agent = [s.argv for s in runner.specs if "--max-budget-usd" in s.argv] + assert len(agent) == 1 and ("--mcp-config" in agent[0]) is (tools == "all") + assert meta["args_if_present"] == ([".mcp.json"] if tools == "all" else []) + + def test_the_base_check_ignores_a_skill_the_guidance_arm_removes_and_reads_each_base_once(tmp_path, monkeypatch): _skill(tmp_path / "toy", ".claude/skills/lint", "lint") root, base = make_repo(tmp_path) From e714cd956a61684dbf716639d220c595978ccbc2 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 22 Oct 2026 14:30:50 +0000 Subject: [PATCH 24/33] Fix the comment on unmeasured skill usage in run_cell Signed-off-by: Anton Volkov --- dokimi/dokimi/repos/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index d44d2953..3591c3ec 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -223,7 +223,7 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, "model_resolved": meta.get("model_resolved"), "rep": rep, "harness": opts.profile.name, "harness_version": meta["harness_version"], "base_date": meta["base_date"], "score_task": bool(task.score), "agent": agent, "grade": grade} - if meta.get("skills"): # without a trace the agent used none of them + if meta.get("skills"): # without a trace the usage is unmeasured, not zero record["skills"] = skills if skills is not None else skill_unmeasured( [s["name"] for s in meta["skills"]], task.expected_skill) try: From b3c89af16740771a022779b240e06518b9b2ab9e Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 22 Oct 2026 14:31:15 +0000 Subject: [PATCH 25/33] Say check_arms does not read patch files in its docstring Signed-off-by: Anton Volkov --- dokimi/dokimi/repos/arms.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 28613585..1238303c 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -165,7 +165,9 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa the base of a task (in ``git_dir``) already ships, which preparing the workspace would refuse cell by cell. As preparation does, a ``guidance:none`` or pinned-revision arm first drops the base's files that match ``guidance_globs``: only what is left under the skill's directory counts as shipped. A revision arm then - writes back its revision's files that match them, so those count as shipped too.""" + writes back its revision's files that match them, so those count as shipped too. The files of a ``patch:`` + arm's patch and of a task's ``setup_patches`` are not read: a skill directory they add is refused only in + preparation.""" from dokimi.repos.manifest import match_glob cells: dict[str, str] = {} # cell-name slug of an arm -> the arm spec that owns it From 03f0fb2a57fbdcd3a8f9e2edecd2904d47b8d79a Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 22 Oct 2026 14:31:47 +0000 Subject: [PATCH 26/33] Document args_if_present in the meta.json fields of the contract Signed-off-by: Anton Volkov --- dokimi/docs/repo-evaluation-contract.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 4c7a03d7..1ecb6971 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -333,7 +333,9 @@ grader diffs the agent's changes from `start_sha`. With the arm come `arm_files`, the files it deleted, wrote or changed; for `guidance:rev:` an `arm_rev` object (`rev` as given, its `sha`, and `removed`: the base's guidance files with no counterpart at ``); for `patch:` an `arm_patch` object (the -file's absolute `path` and full `sha256`). +file's absolute `path` and full `sha256`). `args_if_present` lists the profile's +`args_if_present` files the cell's command line carries (§6): `[]` when the +workspace has none of them, and always outside the `all` tool mode. Step 3 removes history only. A fix that landed upstream before a model's training cutoff may still be known to the model, so reports split results by From ecd26bfc2c2c6e4dcd8cb2d4fd37189f4fdc8941 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 08:21:28 -0700 Subject: [PATCH 27/33] test(repo): pin the skill base-check edges, frontmatter parsing and directory reads; document a malformed .mcp.json Signed-off-by: Nikolay Petrov --- dokimi/docs/repo-evaluation-contract.md | 6 +++ dokimi/tests/test_repo_skill_arms.py | 69 +++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 1ecb6971..d46e8855 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -560,6 +560,12 @@ args_if_present: # argv added after the "{tools}" element wh directory, with `{config_dir}` and `{support_dir}` (the framework's helper scripts) as the agent's container sees them. The built-in `claude-code` profile uses them for a hook that refuses backgrounded shell commands. +- `args_if_present` checks that the file exists, not that the harness accepts + it. The built-in `claude-code` profile passes `.mcp.json` as `--mcp-config` + in the `all` tool mode, and the CLI exits 1 before its first turn on a file + that is not valid MCP configuration (`{` for one). Every such cell is then + graded as a task failure, not reported as an agent error. Fix or remove the + file in the task base, or run the repository with `--tools read` or `none`. Profiles for other harnesses (Codex, Gemini CLI, Copilot CLI, Cursor agent, Aider, OpenCode, in-house agents) follow the same shape. Their `guidance` diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 857e0f9d..7ceeeb9f 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -533,3 +533,72 @@ def test_the_base_check_drops_guidance_for_an_unpinned_revision_arm(tmp_path): with pytest.raises(ArmError, match=f"guidance revision {base} already has .claude/skills/lint"): check_arms([parse_arm(f"guidance:rev:{base}+skill:skills/lint")], root / ".agent-eval", tasks, root / ".git", [".claude/skills/**"]) # ... and then writes the revision's own back + + +def test_the_base_check_matches_a_skill_directory_not_a_name_prefix(tmp_path): + _skill(tmp_path / "toy", ".claude/skills/linter", "linter") # the base ships linter, the arm installs lint + root, base = make_repo(tmp_path) + _skill(root, ".agent-eval/skills/lint", "lint") + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + check_arms([parse_arm("skill:skills/lint")], root / ".agent-eval", [Task("t", {"base": base}, root)], + root / ".git") + + +def test_the_base_check_counts_only_the_files_the_revision_writes_back(tmp_path): + root, base = make_repo(tmp_path) + _skill(root, ".agent-eval/skills/lint", "lint") + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + write(root, ".claude/skills/lint/helper.py", "x = 1\n") # tracked at HEAD, matched by no guidance glob + git(root, "add", "-A") + git(root, "commit", "-qm", "helper") + tasks = [Task("t", {"base": base}, root)] + arm = parse_arm("guidance:rev:HEAD+skill:skills/lint") + check_arms([arm], root / ".agent-eval", tasks, root / ".git", ["**/AGENTS.md"]) # preparation writes no helper + with pytest.raises(ArmError, match="guidance revision HEAD already has .claude/skills/lint"): + check_arms([arm], root / ".agent-eval", tasks, root / ".git", [".claude/skills/**"]) + + +def test_the_base_check_reads_the_pinned_commit_of_a_revision_arm_not_the_moved_branch(tmp_path): + from dokimi.repos.workspace import pin_arm + + root, base = make_repo(tmp_path) + _skill(root, ".agent-eval/skills/lint", "lint") + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + git(root, "branch", "guide") + pinned = pin_arm(parse_arm("guidance:rev:guide+skill:skills/lint"), root / ".git") + git(root, "checkout", "-q", "guide") + _skill(root, ".claude/skills/lint", "lint") # the branch moves to a commit that ships the skill + git(root, "add", "-A") + git(root, "commit", "-qm", "moved") + tasks = [Task("t", {"base": base}, root)] + check_arms([pinned], root / ".agent-eval", tasks, root / ".git", [".claude/skills/**"]) # checked at the pin + with pytest.raises(ArmError, match="already has .claude/skills/lint"): + check_arms([parse_arm("guidance:rev:guide+skill:skills/lint")], root / ".agent-eval", tasks, + root / ".git", [".claude/skills/**"]) + + +@pytest.mark.parametrize(("head", "name"), [ + ("---\nname: other\n---\nBody.\n", "a BOM"), + ("--- \nname: other\n---\nBody.\n", "trailing spaces after the fence"), +]) +def test_a_frontmatter_with_a_bom_or_a_padded_fence_is_still_read(tmp_path, head, name): + (tmp_path / "skills" / "lint").mkdir(parents=True) + (tmp_path / "skills" / "lint" / "SKILL.md").write_bytes(head.encode("utf-8")) + with pytest.raises(ArmError, match="named 'other', but its directory is 'lint'"): + resolve_skills(parse_arm("skill:skills/lint"), tmp_path) + + +def test_a_frontmatter_that_is_not_a_mapping_has_no_name(tmp_path): + for body in ("- a\n- b", "just text", ""): + write(tmp_path, "skills/lint/SKILL.md", f"---\n{body}\n---\nBody.\n") + assert [s.name for s in resolve_skills(parse_arm("skill:skills/lint"), tmp_path)] == ["lint"] + + +def test_a_read_of_a_skills_directory_is_not_a_use_of_a_skill(): + events = [{"kind": "read", "path": ".claude/skills/lint"}, {"kind": "read", "path": ".claude/skills"}, + {"kind": "read", "path": ""}] + used = skill_usage(events, ["lint"], "lint") + assert used["skills_used"] == [] and used["skill_invoked"] is False From 36a9448c29cf54e3b5dc698933efb91320342464 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 11:45:15 -0700 Subject: [PATCH 28/33] fix(repo): refuse a malformed .mcp.json up front; skill base check covers links and emptied dirs repo validate and the repo run pre-flight (and a cell whose arm wrote the file) refuse a .mcp.json the agent's CLI would exit on. check_arms refuses a base that tracks .claude or .claude/skills as a symlink, a guidance:none arm installs a skill whose directory the guidance drop emptied, and a SKILL.md name YAML reads as an impossible date is not a crash. Signed-off-by: Nikolay Petrov --- ...0261020_090000_napetrov_repo-skill-arms.md | 1 + dokimi/docs/repo-evaluation-contract.md | 11 ++- dokimi/dokimi/commands/repo_run.py | 2 + dokimi/dokimi/repos/arms.py | 29 ++++-- dokimi/dokimi/repos/bases.py | 53 ++++++++++- dokimi/dokimi/repos/manifest.py | 6 ++ dokimi/dokimi/repos/runner.py | 6 ++ dokimi/dokimi/repos/workspace.py | 11 ++- dokimi/tests/test_repo_bases.py | 38 ++++++++ dokimi/tests/test_repo_skill_arms.py | 91 +++++++++++++++++++ 10 files changed, 233 insertions(+), 15 deletions(-) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index 5b6d1c7c..d2e19615 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -24,3 +24,4 @@ This shifts baselines: a repo that ships `.mcp.json` now starts its MCP servers under `--tools all`, so its results differ from earlier runs of the same repo and arm. Compare only batches run after this change. +- **A malformed `.mcp.json` is refused up front** — `repo validate` reports one that is not a JSON object (or whose `mcpServers` is not) at a task base, and `repo run --tools all` exits before any cell, instead of every cell failing as a task failure when the CLI rejects the file; a `patch:` arm that writes one is a cell error. `check_arms` also refuses, before any cell, a base that tracks `.claude` or `.claude/skills` as a symlink; a `guidance:none` arm installs a skill the base ships under the guidance globs; a `SKILL.md` `name` that YAML reads as an impossible date is not a crash. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index d46e8855..a5466aee 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -563,9 +563,14 @@ args_if_present: # argv added after the "{tools}" element wh - `args_if_present` checks that the file exists, not that the harness accepts it. The built-in `claude-code` profile passes `.mcp.json` as `--mcp-config` in the `all` tool mode, and the CLI exits 1 before its first turn on a file - that is not valid MCP configuration (`{` for one). Every such cell is then - graded as a task failure, not reported as an agent error. Fix or remove the - file in the task base, or run the repository with `--tools read` or `none`. + that is not valid MCP configuration (`{` for one). So a `.mcp.json` that is + not a JSON object, or whose `mcpServers` is not an object, is refused up + front: `repo validate` reports an error for the one at each task base (at + the eval tree when the source has no git history), `repo run --tools all` + exits 1 before the image build and any cell, and a cell whose arm wrote + such a file (a `patch:` arm) is a cell error, not a graded run. Fix or remove + the file in the task base; `--tools read` or `none` do not pass it, so + `repo run` does not check it there. Profiles for other harnesses (Codex, Gemini CLI, Copilot CLI, Cursor agent, Aider, OpenCode, in-house agents) follow the same shape. Their `guidance` diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index 07259306..99dc2955 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -176,6 +176,7 @@ def cmd_repo_run(args: argparse.Namespace) -> None: def _run_locked(args: argparse.Namespace, arms: list[str], batch_dir: Path, lock: contextlib.ExitStack) -> None: from dokimi.repos import RepoSource, open_source from dokimi.repos.arms import check_arms + from dokimi.repos.bases import check_mcp_configs from dokimi.repos.container import docker_network from dokimi.repos.harness_profile import load_profile from dokimi.repos.runner import ( @@ -196,6 +197,7 @@ def _run_locked(args: argparse.Namespace, arms: list[str], batch_dir: Path, lock ctx, _ = _context(args, batch_dir, pin=True) tasks = load_tasks(ctx.content_dir, ctx.manifest, args.task) check_arms(arms, ctx.content_dir, tasks, ctx.git_dir, ctx.manifest.guidance_globs) # skill paths exist, catalogs hold each expected_skill + check_mcp_configs(ctx.git_dir, tasks, profile, args.tools) # a malformed .mcp.json fails here, not in every cell skill_pin = _skill_pin(batch_dir, arms, ctx.content_dir) # a resume may not swap an arm's skills validate_tasks(tasks) # refuse malformed host requirements before containers start options = [AgentOptions(profile=profile, model=m, harness_bin=harness_bin, agent_network=args.agent_network, diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 1238303c..c6685890 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -113,7 +113,7 @@ def _frontmatter_name(skill_md: Path) -> str | None: block.append(line) try: meta = yaml.safe_load("\n".join(block)) - except yaml.YAMLError: + except (yaml.YAMLError, ValueError): # ValueError: `name: 2024-13-45` resolves as a timestamp that does not exist return None name = meta.get("name") if isinstance(meta, dict) else None return str(name).strip() or None if name is not None else None @@ -151,11 +151,21 @@ def _skill_path(path: str, content_dir: Path) -> Path: return full -def _base_skill_files(git_dir: Path, base: str) -> list[str]: - """The files the commit ``base`` tracks under ``.claude/skills`` (one ``git ls-tree`` per base).""" - proc = process.run(["git", "-C", str(git_dir), "ls-tree", "-r", "-z", "--name-only", base, "--", ".claude/skills"], +def _base_skill_files(git_dir: Path, base: str) -> tuple[list[str], list[str]]: + """The files the commit ``base`` tracks under ``.claude/skills``, and which of ``.claude`` and + ``.claude/skills`` it tracks as a symlink (one ``git ls-tree`` per base).""" + proc = process.run(["git", "-C", str(git_dir), "ls-tree", "-r", "-z", base, "--", ".claude", ".claude/skills"], env=process.GIT_ENV, timeout=60, capture_limit=None) - return [p for p in proc.stdout.split("\0") if p] if proc.returncode == 0 else [] + if proc.returncode != 0: + return [], [] + files, links = [], [] + for entry in proc.stdout.split("\0"): # ` \t`; -r lists a link, never what is behind it + meta, _, path = entry.partition("\t") + if meta.startswith("120000 ") and path in (".claude", ".claude/skills"): + links.append(path) + elif path.startswith(".claude/skills/"): + files.append(path) + return files, links def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Path | None = None, @@ -171,7 +181,7 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa from dokimi.repos.manifest import match_glob cells: dict[str, str] = {} # cell-name slug of an arm -> the arm spec that owns it - shipped: dict[str, list[str]] = {} # task base -> tracked files under .claude/skills + shipped: dict[str, tuple[list[str], list[str]]] = {} # base -> (tracked files under .claude/skills, its links) for arm in arms: cell = slug(str(arm).replace(":", "-")) if cells.setdefault(cell, arm.spec) != arm.spec: @@ -186,9 +196,12 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa for commit in (task.base, rev): if commit is not None and commit not in shipped: shipped[commit] = _base_skill_files(git_dir, commit) - left = [p for p in shipped[task.base] + left = [p for p in shipped[task.base][0] if not (drops and any(match_glob(g, p) for g in guidance_globs))] - written = [p for p in shipped[rev] if any(match_glob(g, p) for g in guidance_globs)] if rev else [] + written = [p for p in shipped[rev][0] if any(match_glob(g, p) for g in guidance_globs)] if rev else [] + for link in shipped[task.base][1]: # preparation refuses to install through a link, cell by cell + raise ArmError(f"arm {arm.spec!r}: the base of task {task.id} tracks {link} as a symlink; " + f"refusing to install {skills[0].name} through it") for skill in skills: for where, files in ((f"the base of task {task.id}", left), (f"guidance revision {rev}", written)): if any(p.startswith(skill.target.rstrip("/") + "/") or p == skill.target for p in files): diff --git a/dokimi/dokimi/repos/bases.py b/dokimi/dokimi/repos/bases.py index be34ebb0..d7b8098f 100644 --- a/dokimi/dokimi/repos/bases.py +++ b/dokimi/dokimi/repos/bases.py @@ -12,10 +12,52 @@ from __future__ import annotations -from typing import Callable +import json +from typing import Any, Callable from .manifest import RepoCheckReport, match_glob -from .source import RepoTree, SourceError +from .source import GitTree, RepoTree, SourceError + +MCP_CONFIG = ".mcp.json" + + +def mcp_config_error(text: str) -> str | None: + """Why ``text`` is not a ``.mcp.json`` the agent's CLI starts with (a JSON object whose ``mcpServers``, when + there is one, is an object), or ``None``. The CLI exits before its first turn on anything else, which a + run would grade as a task failure.""" + try: + data = json.loads(text) + except ValueError as exc: + return f"not valid JSON ({exc})" + except RecursionError: + return "not valid JSON (nested too deeply)" + if not isinstance(data, dict): + return f"not a JSON object (got {type(data).__name__})" + if "mcpServers" in data and not isinstance(data["mcpServers"], dict): + return f"`mcpServers` is not an object (got {type(data['mcpServers']).__name__})" + return None + + +def tree_mcp_error(tree: RepoTree) -> str | None: + """``mcp_config_error`` of the ``.mcp.json`` at the root of ``tree``; ``None`` without one.""" + try: + if not tree.is_file(MCP_CONFIG): + return None + return mcp_config_error(tree.read_text(MCP_CONFIG)) + except (SourceError, OSError, UnicodeDecodeError): + return None + + +def check_mcp_configs(git_dir: Any, tasks: list[Any], profile: Any, tools: str) -> None: + """Before a run: refuse a task base whose ``.mcp.json`` the profile would pass to the agent (the ``all`` tool + mode) and the agent's CLI would not accept, instead of every cell of the task failing as a task failure.""" + if not profile.applied_args(tools, frozenset({MCP_CONFIG})): + return + for base in dict.fromkeys(t.base for t in tasks): + problem = tree_mcp_error(GitTree(git_dir, base)) + if problem: + raise ValueError(f"{MCP_CONFIG} at task base {base} is {problem}; `--tools all` passes it to the agent, " + "which exits on it before its first turn. Fix or remove the file, or use --tools read/none") def check_bases(report: RepoCheckReport, open_base: Callable[[str], RepoTree], @@ -29,7 +71,8 @@ def check_bases(report: RepoCheckReport, open_base: Callable[[str], RepoTree], if task.base in by_base or task.base in failed: continue try: - files = open_base(task.base).files() + base_tree = open_base(task.base) + files = base_tree.files() except SourceError as exc: report.errors.append(f"task {task.id}: base {task.base!r} does not resolve in {report.tree}: {exc}") failed.add(task.base) @@ -41,6 +84,10 @@ def check_bases(report: RepoCheckReport, open_base: Callable[[str], RepoTree], matched_somewhere.add(pattern) found.extend(m for m in matched if m not in found) by_base[task.base] = found + problem = tree_mcp_error(base_tree) + if problem: + report.errors.append(f"task {task.id}: {MCP_CONFIG} at base {task.base!r} is {problem}; `repo run --tools all` " + "would fail every cell of it as a task failure") report.guidance_by_base = {base: len(found) for base, found in by_base.items()} where = f"any of the {len(by_base)} task base(s) that resolved" if failed else "any task base" diff --git a/dokimi/dokimi/repos/manifest.py b/dokimi/dokimi/repos/manifest.py index aa0dad99..9eb7a14d 100644 --- a/dokimi/dokimi/repos/manifest.py +++ b/dokimi/dokimi/repos/manifest.py @@ -339,6 +339,12 @@ def check_repo(tree: RepoTree, manifest: RepoManifest, report.errors.append(f"tasks directory does not exist: {manifest.tasks}") if open_base is None: report.warnings.extend(f"guidance glob matches no file: {p}" for p in unmatched) + from .bases import MCP_CONFIG, tree_mcp_error + + problem = tree_mcp_error(tree) # no task base to read: the snapshot's own + if problem: + report.errors.append(f"{MCP_CONFIG} at the eval tree is {problem}; `repo run --tools all` would fail " + "every cell as a task failure") else: from .bases import check_bases diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index 3591c3ec..51ad9f5f 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -60,6 +60,7 @@ docker_network, ) from .arms import skill_unmeasured, skill_usage, slug +from .bases import MCP_CONFIG, mcp_config_error from .events import convert from .grading import EvalContext, failed_outcome, outcome, run_entry from .harness_profile import HarnessProfile @@ -162,6 +163,11 @@ def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, batch=batch_dir.name, agent_network=network, egress_proxy=False, host=host) present = frozenset(n for n in opts.profile.args_if_present if (rd / "workspace" / n).exists()) meta["args_if_present"] = opts.profile.applied_args(opts.tools, present) # the ones the cell runs with + if MCP_CONFIG in meta["args_if_present"]: # the arm may have written it: check what the agent is given + problem = mcp_config_error((rd / "workspace" / MCP_CONFIG).read_text(encoding="utf-8", errors="replace")) + if problem: + raise PrepError(f"task {task.id}: {MCP_CONFIG} in the workspace is {problem}; the agent's CLI would exit " + "on it before its first turn") _write_json(rd / "meta.json", meta) skills: dict[str, Any] | None = None diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index b33e7d5f..e81fe91f 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -389,6 +389,13 @@ def _content_dir(manifest: RepoManifest, task: Task) -> Path: return task.dir.parents[len(PurePosixPath(tasks).relative_to(eval_dir).parts)] +def _only_empty_dirs(path: Path) -> bool: + """``path`` is a real directory with nothing but real directories under it.""" + if path.is_symlink() or not path.is_dir(): + return False + return all(not f and not any(Path(d, n).is_symlink() for n in dn) for d, dn, f in os.walk(path)) + + def _install_skills(ws: Path, task: Task, skills: list[Skill]) -> list[dict[str, str]]: """Copy each skill to ``.claude/skills//``; a skill the repository already has there is refused, as replacing it would change what the guidance arm measures.""" @@ -399,7 +406,9 @@ def _install_skills(ws: Path, task: Task, skills: list[Skill]) -> list[dict[str, if part.is_symlink(): raise PrepError(f"task {task.id}: {part.relative_to(ws)} is a symlink in the repository; " f"refusing to install {skill.name} through it") - if dest.exists(): + if _only_empty_dirs(dest): + shutil.rmtree(dest) # what dropping the guidance of a skill the base ships left: its emptied directory + if dest.exists() or dest.is_symlink(): raise PrepError(f"task {task.id}: the workspace already has {skill.target}") if not dest.parent.resolve().is_relative_to(ws.resolve()): raise PrepError(f"task {task.id}: {skill.target} resolves outside the workspace") diff --git a/dokimi/tests/test_repo_bases.py b/dokimi/tests/test_repo_bases.py index 57ac0ed1..458a3eb1 100644 --- a/dokimi/tests/test_repo_bases.py +++ b/dokimi/tests/test_repo_bases.py @@ -179,3 +179,41 @@ def test_work_tree_lists_what_git_shows(tmp_path): plain = tmp_path / "plain" _write(plain, ".venv/AGENTS.md", "no git here, so every file counts\n") assert WorkTree(plain).files() == [".venv/AGENTS.md"] + + +@pytest.mark.parametrize(("text", "why"), [("{", "not valid JSON"), ("[]", "not a JSON object"), + ('{"mcpServers": []}', "`mcpServers` is not an object"), + pytest.param("[" * 100000, "not valid JSON", id="deeply-nested")]) +def test_validate_refuses_a_malformed_mcp_json_at_a_task_base_and_at_a_tree_without_history(tmp_path, capsys, + text, why): + root = tmp_path / "toy" + _write(root, ".mcp.json", text) + _write(root, "src/lib.c", "int f(void);\n") + base = _commit_all(root, "code") + _write(root, "dev/evals/Dockerfile", "FROM scratch\n") + _write(root, "dev/evals/repo-eval.yaml", yaml.safe_dump(MANIFEST)) + _task(root, "fix_one", base) + _commit_all(root, "evals") + for source in (f"{root}@HEAD", str(root)): # with git history (the task base), and a plain work tree + if source == str(root): + subprocess.run(["rm", "-rf", str(root / ".git")], check=True) + with pytest.raises(SystemExit): + _run_cli(["repo", "validate", source, "--manifest", "dev/evals/repo-eval.yaml", "--json"]) + out = json.loads(capsys.readouterr().out) + assert out["ok"] is False + assert any(".mcp.json" in e and why in e for e in out["errors"]), out["errors"] + + +def test_validate_accepts_a_valid_or_absent_mcp_json(tmp_path, capsys): + for name, text in (("valid", '{"mcpServers": {"a": {}}}'), ("no_servers", "{}"), ("absent", None)): + root = tmp_path / name + if text is not None: + _write(root, ".mcp.json", text) + _write(root, "src/lib.c", "int f(void);\n") + base = _commit_all(root, "code") + _write(root, "dev/evals/Dockerfile", "FROM scratch\n") + _write(root, "dev/evals/repo-eval.yaml", yaml.safe_dump(MANIFEST)) + _task(root, "fix_one", base) + _commit_all(root, "evals") + _run_cli(["repo", "validate", f"{root}@HEAD", "--manifest", "dev/evals/repo-eval.yaml", "--json"]) + assert json.loads(capsys.readouterr().out)["ok"] is True, name diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py index 7ceeeb9f..156b0f77 100644 --- a/dokimi/tests/test_repo_skill_arms.py +++ b/dokimi/tests/test_repo_skill_arms.py @@ -5,6 +5,7 @@ from __future__ import annotations +import dataclasses import json import os import shutil @@ -602,3 +603,93 @@ def test_a_read_of_a_skills_directory_is_not_a_use_of_a_skill(): {"kind": "read", "path": ""}] used = skill_usage(events, ["lint"], "lint") assert used["skills_used"] == [] and used["skill_invoked"] is False + + +def _claude_run(tmp_path, monkeypatch, mcp_json, *extra, patch=None): + """`repo run` with the built-in claude-code profile on a base that ships `mcp_json`; the agent specs.""" + runner = FakeRunner() + monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: runner) + monkeypatch.setenv("FAKE_ACTION", "fix") + profile = Path(__file__).resolve().parents[1] / "dokimi/repos/profiles/claude-code.yaml" + exe, _ = fake_harness(tmp_path) + if mcp_json is not None: + write(tmp_path / "toy", ".mcp.json", mcp_json) + root, _ = make_repo(tmp_path) + argv = ["repo", "run", f"{root}@HEAD", "--harness", str(profile), "--bin", str(exe), "--model", "m-1", + "--batch", "b1", "--out", str(tmp_path / "runs"), "--cache-root", str(tmp_path / "caches"), + "--agent-network", "host", "--task", "fix_add", *extra] + args = build_parser().parse_args(argv) + try: + args.func(args) + code = 0 + except SystemExit as exc: + code = exc.code + return code, [s for s in runner.specs if "--strict-mcp-config" in s.argv] + + +@pytest.mark.parametrize("text", ["{", "[]", '{"mcpServers": "x"}']) +def test_run_refuses_a_malformed_mcp_json_before_any_cell_when_the_agent_is_given_it(tmp_path, monkeypatch, capsys, + text): + code, agents = _claude_run(tmp_path, monkeypatch, text, "--arm", "guidance:raw", "--tools", "all") + err = capsys.readouterr().err + assert code == 1 and agents == [] + assert ".mcp.json" in err and "--tools read/none" in err + assert not list((tmp_path / "runs" / "b1").glob("fix_add__*")) # no cell was even prepared + + +def test_run_leaves_a_malformed_mcp_json_alone_when_the_agent_is_not_given_it(tmp_path, monkeypatch): + code, agents = _claude_run(tmp_path, monkeypatch, "{", "--arm", "guidance:raw", "--tools", "read") + assert code in (0, None) and len(agents) == 1 and "--mcp-config" not in agents[0].argv + + +def test_run_refuses_a_malformed_mcp_json_that_an_arms_patch_wrote(tmp_path, monkeypatch, capsys): + patch = tmp_path / "mcp.diff" + patch.write_text("diff --git a/.mcp.json b/.mcp.json\nnew file mode 100644\n--- /dev/null\n+++ b/.mcp.json\n" + "@@ -0,0 +1 @@\n+{\n") + code, agents = _claude_run(tmp_path, monkeypatch, None, "--arm", f"patch:{patch}", "--tools", "all") + assert code == 1 and agents == [] + out = capsys.readouterr() + assert ".mcp.json in the workspace is not valid JSON" in out.out + out.err + + +def test_a_guidance_none_arm_installs_a_skill_the_base_ships_under_the_guidance_globs(tmp_path): + _skill(tmp_path / "toy", ".claude/skills/lint", "lint") + root, base = make_repo(tmp_path) + _skill(root, ".agent-eval/skills/lint", "lint") + git(root, "add", "-A") + git(root, "commit", "-qm", "skills") + from dokimi.repos import RepoSource, load_manifest as load, open_source + from dokimi.repos.workspace import load_tasks, prepare_workspace + + source = RepoSource.parse(f"{root}@HEAD") + manifest = load(open_source(source), ".agent-eval/repo-eval.yaml") + manifest = dataclasses.replace(manifest, guidance_globs=(*manifest.guidance_globs, ".claude/skills/**")) + content = root / ".agent-eval" + task = load_tasks(content, manifest, ["fix_add"])[0] + arm = "guidance:none+skill:skills/lint" + check_arms([parse_arm(arm)], content, [task], root / ".git", manifest.guidance_globs) + rd = tmp_path / "rd" + rd.mkdir() + meta = prepare_workspace(root / ".git", manifest, task, arm, rd, None, content) + assert (rd / "workspace/.claude/skills/lint/SKILL.md").is_file() and meta["skills"][0]["name"] == "lint" + + +@pytest.mark.parametrize("link", [".claude", ".claude/skills"]) +def test_the_base_check_refuses_a_tracked_symlink_at_claude_or_its_skills(tmp_path, link): + root, base = make_repo(tmp_path) + _skill(root, ".agent-eval/skills/lint", "lint") + (root / "shared").mkdir() + if link == ".claude/skills": + (root / ".claude").mkdir() + (root / link).symlink_to("../shared" if "/" in link else "shared") + git(root, "add", "-A") + git(root, "commit", "-qm", "link") + task = Task("t", {"base": git(root, "rev-parse", "HEAD")}, root) + with pytest.raises(ArmError, match=rf"tracks {link} as a symlink"): + check_arms([parse_arm("skill:skills/lint")], root / ".agent-eval", [task], root / ".git") + check_arms([parse_arm("guidance:raw")], root / ".agent-eval", [task], root / ".git") # no skill: nothing to refuse + + +def test_a_frontmatter_name_that_yaml_reads_as_an_impossible_date_is_the_directory_name_not_a_crash(tmp_path): + write(tmp_path, "skills/lint/SKILL.md", "---\nname: 2024-13-45\n---\nBody.\n") + assert [s.name for s in resolve_skills(parse_arm("skill:skills/lint"), tmp_path)] == ["lint"] From 09f903e2873b26209822f687b1b32d3f40feed1f Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 14:01:55 -0700 Subject: [PATCH 29/33] refactor(repo): a malformed .mcp.json is a validate warning; extract helpers from the arm, manifest and workspace checks repo validate reports a malformed .mcp.json as a warning and succeeds; repo run --tools all and a patch arm that writes one still refuse it. check_arms, resolve_skills, check_repo and prepare_workspace are back under the complexity cap. Signed-off-by: Nikolay Petrov --- ...0261020_090000_napetrov_repo-skill-arms.md | 2 +- dokimi/docs/repo-evaluation-contract.md | 6 +- dokimi/dokimi/repos/arms.py | 96 ++++++++++++------- dokimi/dokimi/repos/bases.py | 2 +- dokimi/dokimi/repos/manifest.py | 54 ++++++----- dokimi/dokimi/repos/workspace.py | 50 ++++++---- dokimi/tests/test_repo_bases.py | 11 +-- 7 files changed, 132 insertions(+), 89 deletions(-) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index d2e19615..a6b7f668 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -24,4 +24,4 @@ This shifts baselines: a repo that ships `.mcp.json` now starts its MCP servers under `--tools all`, so its results differ from earlier runs of the same repo and arm. Compare only batches run after this change. -- **A malformed `.mcp.json` is refused up front** — `repo validate` reports one that is not a JSON object (or whose `mcpServers` is not) at a task base, and `repo run --tools all` exits before any cell, instead of every cell failing as a task failure when the CLI rejects the file; a `patch:` arm that writes one is a cell error. `check_arms` also refuses, before any cell, a base that tracks `.claude` or `.claude/skills` as a symlink; a `guidance:none` arm installs a skill the base ships under the guidance globs; a `SKILL.md` `name` that YAML reads as an impossible date is not a crash. +- **A malformed `.mcp.json` is caught up front** — `repo validate` warns (and still succeeds) about one that is not a JSON object (or whose `mcpServers` is not) at a task base, and `repo run --tools all` exits before any cell, instead of every cell failing as a task failure when the CLI rejects the file; a `patch:` arm that writes one is a cell error. `check_arms` also refuses, before any cell, a base that tracks `.claude` or `.claude/skills` as a symlink; a `guidance:none` arm installs a skill the base ships under the guidance globs; a `SKILL.md` `name` that YAML reads as an impossible date is not a crash. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index a5466aee..e34592fc 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -564,9 +564,9 @@ args_if_present: # argv added after the "{tools}" element wh it. The built-in `claude-code` profile passes `.mcp.json` as `--mcp-config` in the `all` tool mode, and the CLI exits 1 before its first turn on a file that is not valid MCP configuration (`{` for one). So a `.mcp.json` that is - not a JSON object, or whose `mcpServers` is not an object, is refused up - front: `repo validate` reports an error for the one at each task base (at - the eval tree when the source has no git history), `repo run --tools all` + not a JSON object, or whose `mcpServers` is not an object, is caught up + front: `repo validate` warns about the one at each task base (at + the eval tree when the source has no git history) and still succeeds, `repo run --tools all` exits 1 before the image build and any cell, and a cell whose arm wrote such a file (a `patch:` arm) is a cell error, not a graded run. Fix or remove the file in the task base; `--tools read` or `none` do not pass it, so diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index c6685890..3768199c 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -66,19 +66,30 @@ def resolve_skills(arm: Any, content_dir: Path) -> list[Skill]: if not (root / "SKILL.md").is_file(): raise ArmError(f"arm {arm.spec!r}: no SKILL.md in {path}") found.append(_named(arm, root, path)) + else: + found += _catalog_skills(arm, root, path) + _check_installable(arm, found) + return found + + +def _catalog_skills(arm: Any, root: Path, path: str) -> list[Skill]: + """The skills in the subdirectories of the catalog ``root`` (written as ``path``).""" + if not root.is_dir(): + raise ArmError(f"arm {arm.spec!r}: skill-catalog {path} is not a directory") + subs = [] + for d in sorted(root.iterdir()): + if not d.is_dir() or not (d / "SKILL.md").is_file(): continue - if not root.is_dir(): - raise ArmError(f"arm {arm.spec!r}: skill-catalog {path} is not a directory") - subs = [] - for d in sorted(root.iterdir()): - if not d.is_dir() or not (d / "SKILL.md").is_file(): - continue - if not d.resolve().is_relative_to(root.resolve()): - raise ArmError(f"arm {arm.spec!r}: {d.name} in {path} links outside it") - subs.append(_named(arm, d, path)) - if not subs: - raise ArmError(f"arm {arm.spec!r}: no subdirectory of {path} holds a SKILL.md") - found += subs + if not d.resolve().is_relative_to(root.resolve()): + raise ArmError(f"arm {arm.spec!r}: {d.name} in {path} links outside it") + subs.append(_named(arm, d, path)) + if not subs: + raise ArmError(f"arm {arm.spec!r}: no subdirectory of {path} holds a SKILL.md") + return subs + + +def _check_installable(arm: Any, found: list[Skill]) -> None: + """Refuse a skill the copy cannot take, and two skills of one name.""" for skill in found: bad = _uncopyable(skill.dir) if bad: @@ -88,7 +99,6 @@ def resolve_skills(arm: Any, content_dir: Path) -> list[Skill]: dupes = sorted({n for n in names if names.count(n) > 1}) if dupes: raise ArmError(f"arm {arm.spec!r}: more than one skill named {', '.join(dupes)}") - return found def _named(arm: Any, directory: Path, written: str) -> Skill: @@ -178,8 +188,6 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa writes back its revision's files that match them, so those count as shipped too. The files of a ``patch:`` arm's patch and of a task's ``setup_patches`` are not read: a skill directory they add is refused only in preparation.""" - from dokimi.repos.manifest import match_glob - cells: dict[str, str] = {} # cell-name slug of an arm -> the arm spec that owns it shipped: dict[str, tuple[list[str], list[str]]] = {} # base -> (tracked files under .claude/skills, its links) for arm in arms: @@ -187,31 +195,45 @@ def check_arms(arms: list[Any], content_dir: Path, tasks: list[Any], git_dir: Pa if cells.setdefault(cell, arm.spec) != arm.spec: raise ArmError(f"arms {cells[cell]!r} and {arm.spec!r} would share one result directory ({cell})") skills = resolve_skills(arm, content_dir) - names = {s.name for s in skills} drops = arm.patch is None and (arm.base == "guidance:none" or arm.rev is not None) rev = (arm.rev_sha or arm.rev) if arm.patch is None else None - for task in tasks if skills else (): - if git_dir is None: - break - for commit in (task.base, rev): - if commit is not None and commit not in shipped: - shipped[commit] = _base_skill_files(git_dir, commit) - left = [p for p in shipped[task.base][0] - if not (drops and any(match_glob(g, p) for g in guidance_globs))] - written = [p for p in shipped[rev][0] if any(match_glob(g, p) for g in guidance_globs)] if rev else [] - for link in shipped[task.base][1]: # preparation refuses to install through a link, cell by cell - raise ArmError(f"arm {arm.spec!r}: the base of task {task.id} tracks {link} as a symlink; " - f"refusing to install {skills[0].name} through it") - for skill in skills: - for where, files in ((f"the base of task {task.id}", left), (f"guidance revision {rev}", written)): - if any(p.startswith(skill.target.rstrip("/") + "/") or p == skill.target for p in files): - raise ArmError(f"arm {arm.spec!r}: {where} already has {skill.target}") - if any(kind == "skill-catalog" for kind, _ in arm.skills): + if skills and git_dir is not None: for task in tasks: - expected = task.raw.get("expected_skill") - if expected and expected not in names: - raise ArmError(f"arm {arm.spec!r}: task {task.id}'s expected_skill {expected!r} is not installed " - f"(installed: {', '.join(sorted(names))})") + _check_shipped(arm, skills, task, git_dir, shipped, (drops, rev, guidance_globs)) + if any(kind == "skill-catalog" for kind, _ in arm.skills): + _check_expected_skills(arm, tasks, {s.name for s in skills}) + + +def _check_shipped(arm: Any, skills: list[Skill], task: Any, git_dir: Path, + shipped: dict[str, tuple[list[str], list[str]]], + dropping: tuple[bool, str | None, tuple[str, ...] | list[str]]) -> None: + """Refuse ``skills`` that the base of ``task`` (or the arm's guidance revision) already ships, or a link that + preparation would refuse to install through; ``shipped`` caches ``_base_skill_files`` per commit.""" + from dokimi.repos.manifest import match_glob + + drops, rev, guidance_globs = dropping + for commit in (task.base, rev): + if commit is not None and commit not in shipped: + shipped[commit] = _base_skill_files(git_dir, commit) + left = [p for p in shipped[task.base][0] + if not (drops and any(match_glob(g, p) for g in guidance_globs))] + written = [p for p in shipped[rev][0] if any(match_glob(g, p) for g in guidance_globs)] if rev else [] + for link in shipped[task.base][1]: # preparation refuses to install through a link, cell by cell + raise ArmError(f"arm {arm.spec!r}: the base of task {task.id} tracks {link} as a symlink; " + f"refusing to install {skills[0].name} through it") + for skill in skills: + for where, files in ((f"the base of task {task.id}", left), (f"guidance revision {rev}", written)): + if any(p.startswith(skill.target.rstrip("/") + "/") or p == skill.target for p in files): + raise ArmError(f"arm {arm.spec!r}: {where} already has {skill.target}") + + +def _check_expected_skills(arm: Any, tasks: list[Any], names: set[str]) -> None: + """A catalog arm has to install the ``expected_skill`` of every task that names one.""" + for task in tasks: + expected = task.raw.get("expected_skill") + if expected and expected not in names: + raise ArmError(f"arm {arm.spec!r}: task {task.id}'s expected_skill {expected!r} is not installed " + f"(installed: {', '.join(sorted(names))})") def tree_sha256(root: Path) -> str: diff --git a/dokimi/dokimi/repos/bases.py b/dokimi/dokimi/repos/bases.py index d7b8098f..56f59195 100644 --- a/dokimi/dokimi/repos/bases.py +++ b/dokimi/dokimi/repos/bases.py @@ -86,7 +86,7 @@ def check_bases(report: RepoCheckReport, open_base: Callable[[str], RepoTree], by_base[task.base] = found problem = tree_mcp_error(base_tree) if problem: - report.errors.append(f"task {task.id}: {MCP_CONFIG} at base {task.base!r} is {problem}; `repo run --tools all` " + report.warnings.append(f"task {task.id}: {MCP_CONFIG} at base {task.base!r} is {problem}; `repo run --tools all` " "would fail every cell of it as a task failure") report.guidance_by_base = {base: len(found) for base, found in by_base.items()} diff --git a/dokimi/dokimi/repos/manifest.py b/dokimi/dokimi/repos/manifest.py index 9eb7a14d..222298c3 100644 --- a/dokimi/dokimi/repos/manifest.py +++ b/dokimi/dokimi/repos/manifest.py @@ -308,22 +308,7 @@ def check_repo(tree: RepoTree, manifest: RepoManifest, report.notes.append("guidance-only manifest: no eval_dir, tasks or entry_points, " "so `repo scan` and `repo reach` apply and `repo run` does not") return report - if not tree.is_dir(manifest.eval_dir): - report.errors.append(f"eval_dir does not exist: {manifest.eval_dir}") - elif not _is_within(manifest.path, manifest.eval_dir): - report.warnings.append( - f"manifest {manifest.path} is outside eval_dir {manifest.eval_dir}, " - "so it stays visible to the agent" - ) - if manifest.image is not None and not tree.is_file(manifest.image.dockerfile): - report.errors.append(f"image/dockerfile does not exist: {manifest.image.dockerfile}") - if manifest.explore_tasks and not tree.is_dir(manifest.explore_tasks): - report.errors.append(f"explore_tasks does not exist: {manifest.explore_tasks}") - elif manifest.explore_tasks and not _is_within(manifest.explore_tasks, manifest.eval_dir): - report.errors.append( - f"explore_tasks {manifest.explore_tasks} is outside eval_dir {manifest.eval_dir}; " - "its reference answers would be visible to the agent" - ) + _check_eval_paths(tree, manifest, report) files = tree.files() unmatched = _check_guidance(tree, manifest, report) @@ -339,12 +324,7 @@ def check_repo(tree: RepoTree, manifest: RepoManifest, report.errors.append(f"tasks directory does not exist: {manifest.tasks}") if open_base is None: report.warnings.extend(f"guidance glob matches no file: {p}" for p in unmatched) - from .bases import MCP_CONFIG, tree_mcp_error - - problem = tree_mcp_error(tree) # no task base to read: the snapshot's own - if problem: - report.errors.append(f"{MCP_CONFIG} at the eval tree is {problem}; `repo run --tools all` would fail " - "every cell as a task failure") + _check_mcp_at_tree(tree, report) else: from .bases import check_bases @@ -352,6 +332,36 @@ def check_repo(tree: RepoTree, manifest: RepoManifest, return report +def _check_mcp_at_tree(tree: RepoTree, report: RepoCheckReport) -> None: + """Warn about a malformed ``.mcp.json`` at the snapshot (no task base to read); ``repo run`` is what refuses it.""" + from .bases import MCP_CONFIG, tree_mcp_error + + problem = tree_mcp_error(tree) + if problem: + report.warnings.append(f"{MCP_CONFIG} at the eval tree is {problem}; `repo run --tools all` would fail " + "every cell as a task failure") + + +def _check_eval_paths(tree: RepoTree, manifest: RepoManifest, report: RepoCheckReport) -> None: + """The eval_dir, image Dockerfile and explore_tasks the manifest names exist, and stay inside eval_dir.""" + if not tree.is_dir(manifest.eval_dir): + report.errors.append(f"eval_dir does not exist: {manifest.eval_dir}") + elif not _is_within(manifest.path, manifest.eval_dir): + report.warnings.append( + f"manifest {manifest.path} is outside eval_dir {manifest.eval_dir}, " + "so it stays visible to the agent" + ) + if manifest.image is not None and not tree.is_file(manifest.image.dockerfile): + report.errors.append(f"image/dockerfile does not exist: {manifest.image.dockerfile}") + if manifest.explore_tasks and not tree.is_dir(manifest.explore_tasks): + report.errors.append(f"explore_tasks does not exist: {manifest.explore_tasks}") + elif manifest.explore_tasks and not _is_within(manifest.explore_tasks, manifest.eval_dir): + report.errors.append( + f"explore_tasks {manifest.explore_tasks} is outside eval_dir {manifest.eval_dir}; " + "its reference answers would be visible to the agent" + ) + + def require_runnable(manifest: RepoManifest) -> None: """Refuse a guidance-only manifest where tasks are about to be prepared or run.""" manifest.run_paths() diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index e81fe91f..87ac0f74 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -313,6 +313,35 @@ def _export(tree: RepoTree, dest: Path, only: str | None, exclude: str | None) - tmp.rename(dest) +def _apply_setup_patches(ws: Path, env: Any, task: Task) -> list[dict[str, str]]: + """Apply the task's ``setup_patches`` to ``ws`` (step 4); the record of each for ``meta.json``.""" + patches = [] + for rel in task.setup_patches: + path = _task_file(task, rel) + try: + _git(ws, env, "apply", "--whitespace=nowarn", str(path)) + except PrepError as exc: + raise PrepError(f"task {task.id}: setup patch {rel} does not apply: {exc}") from exc + patches.append({"path": rel, "sha256": _sha256(path)}) + return patches + + +def _apply_review_patch(ws: Path, task: Task, patches: list[dict[str, str]]) -> None: + """Apply the task's ``review_patch`` as a commit on ``ws`` (step 7) and append its record to ``patches``.""" + if not task.review_patch: + return + path = _task_file(task, task.review_patch) + review_env = _git_env(REVIEW_AUTHOR) + try: + _git(ws, review_env, "am", "-q", "--no-gpg-sign", str(path)) + except PrepError as exc: + _git(ws, review_env, "am", "--abort", check=False) + raise PrepError(f"task {task.id}: review patch {task.review_patch} does not apply: {exc}") from exc + # the patch's own author would name the upstream contributor + _git(ws, review_env, "commit", "-q", "--no-gpg-sign", "--no-verify", "--amend", "--no-edit", "--reset-author") + patches.append({"path": task.review_patch, "sha256": _sha256(path)}) + + def prepare_workspace(git_dir: Path, manifest: RepoManifest, task: Task, arm: str, run_dir: Path, profile: HarnessProfile | None = None, content_dir: Path | None = None) -> dict[str, Any]: """Steps 1-7 into ``run_dir/workspace``; returns the ``meta.json`` fields prep owns. @@ -346,14 +375,7 @@ def prepare_workspace(git_dir: Path, manifest: RepoManifest, task: Task, arm: st _git(ws, env, "init", "-q") # 3 _git(ws, env, "add", "-A") _git(ws, env, "commit", "-q", "--no-gpg-sign", "--allow-empty", "--no-verify", "-m", "snapshot") - patches = [] - for rel in task.setup_patches: # 4 - path = _task_file(task, rel) - try: - _git(ws, env, "apply", "--whitespace=nowarn", str(path)) - except PrepError as exc: - raise PrepError(f"task {task.id}: setup patch {rel} does not apply: {exc}") from exc - patches.append({"path": rel, "sha256": _sha256(path)}) + patches = _apply_setup_patches(ws, env, task) # 4 try: arm_files, arm_meta = _apply_arm(ws, env, git_dir, manifest, arm, bridge) # 5 except PrepError as exc: @@ -361,17 +383,7 @@ def prepare_workspace(git_dir: Path, manifest: RepoManifest, task: Task, arm: st installed = _install_skills(ws, task, skills) _git(ws, env, "add", "-A") # 6 _git(ws, env, "commit", "-q", "--no-gpg-sign", "--no-verify", "--amend", "--allow-empty", "-m", "snapshot") - if task.review_patch: # 7 - path = _task_file(task, task.review_patch) - review_env = _git_env(REVIEW_AUTHOR) - try: - _git(ws, review_env, "am", "-q", "--no-gpg-sign", str(path)) - except PrepError as exc: - _git(ws, review_env, "am", "--abort", check=False) - raise PrepError(f"task {task.id}: review patch {task.review_patch} does not apply: {exc}") from exc - # the patch's own author would name the upstream contributor - _git(ws, review_env, "commit", "-q", "--no-gpg-sign", "--no-verify", "--amend", "--no-edit", "--reset-author") - patches.append({"path": task.review_patch, "sha256": _sha256(path)}) + _apply_review_patch(ws, task, patches) # 7 _forget(ws, env) # 8 start_sha = _git(ws, env, "rev-parse", "HEAD").strip() meta: dict[str, Any] = { diff --git a/dokimi/tests/test_repo_bases.py b/dokimi/tests/test_repo_bases.py index 458a3eb1..db83ae09 100644 --- a/dokimi/tests/test_repo_bases.py +++ b/dokimi/tests/test_repo_bases.py @@ -184,8 +184,8 @@ def test_work_tree_lists_what_git_shows(tmp_path): @pytest.mark.parametrize(("text", "why"), [("{", "not valid JSON"), ("[]", "not a JSON object"), ('{"mcpServers": []}', "`mcpServers` is not an object"), pytest.param("[" * 100000, "not valid JSON", id="deeply-nested")]) -def test_validate_refuses_a_malformed_mcp_json_at_a_task_base_and_at_a_tree_without_history(tmp_path, capsys, - text, why): +def test_validate_warns_about_a_malformed_mcp_json_at_a_task_base_and_at_a_tree_without_history(tmp_path, capsys, + text, why): root = tmp_path / "toy" _write(root, ".mcp.json", text) _write(root, "src/lib.c", "int f(void);\n") @@ -197,11 +197,10 @@ def test_validate_refuses_a_malformed_mcp_json_at_a_task_base_and_at_a_tree_with for source in (f"{root}@HEAD", str(root)): # with git history (the task base), and a plain work tree if source == str(root): subprocess.run(["rm", "-rf", str(root / ".git")], check=True) - with pytest.raises(SystemExit): - _run_cli(["repo", "validate", source, "--manifest", "dev/evals/repo-eval.yaml", "--json"]) + _run_cli(["repo", "validate", source, "--manifest", "dev/evals/repo-eval.yaml", "--json"]) # succeeds out = json.loads(capsys.readouterr().out) - assert out["ok"] is False - assert any(".mcp.json" in e and why in e for e in out["errors"]), out["errors"] + assert out["ok"] is True, out["errors"] + assert any(".mcp.json" in w and why in w for w in out["warnings"]), out["warnings"] def test_validate_accepts_a_valid_or_absent_mcp_json(tmp_path, capsys): From 980e7d83bcd8dc58aa3e458c9cae658d41b33454 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 15:12:07 -0700 Subject: [PATCH 30/33] fix(repos): narrow eval_dir in _check_eval_paths; keep _check_task under its complexity baseline Signed-off-by: Nikolay Petrov --- dokimi/dokimi/repos/manifest.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/dokimi/dokimi/repos/manifest.py b/dokimi/dokimi/repos/manifest.py index 8d00fe57..931a9ecc 100644 --- a/dokimi/dokimi/repos/manifest.py +++ b/dokimi/dokimi/repos/manifest.py @@ -345,20 +345,23 @@ def _check_mcp_at_tree(tree: RepoTree, report: RepoCheckReport) -> None: def _check_eval_paths(tree: RepoTree, manifest: RepoManifest, report: RepoCheckReport) -> None: """The eval_dir, image Dockerfile and explore_tasks the manifest names exist, and stay inside eval_dir.""" - if not tree.is_dir(manifest.eval_dir): - report.errors.append(f"eval_dir does not exist: {manifest.eval_dir}") - elif not _is_within(manifest.path, manifest.eval_dir): + eval_dir = manifest.eval_dir + if eval_dir is None: + return + if not tree.is_dir(eval_dir): + report.errors.append(f"eval_dir does not exist: {eval_dir}") + elif not _is_within(manifest.path, eval_dir): report.warnings.append( - f"manifest {manifest.path} is outside eval_dir {manifest.eval_dir}, " + f"manifest {manifest.path} is outside eval_dir {eval_dir}, " "so it stays visible to the agent" ) if manifest.image is not None and not tree.is_file(manifest.image.dockerfile): report.errors.append(f"image/dockerfile does not exist: {manifest.image.dockerfile}") if manifest.explore_tasks and not tree.is_dir(manifest.explore_tasks): report.errors.append(f"explore_tasks does not exist: {manifest.explore_tasks}") - elif manifest.explore_tasks and not _is_within(manifest.explore_tasks, manifest.eval_dir): + elif manifest.explore_tasks and not _is_within(manifest.explore_tasks, eval_dir): report.errors.append( - f"explore_tasks {manifest.explore_tasks} is outside eval_dir {manifest.eval_dir}; " + f"explore_tasks {manifest.explore_tasks} is outside eval_dir {eval_dir}; " "its reference answers would be visible to the agent" ) @@ -431,6 +434,13 @@ def _glob_regex(pattern: str) -> re.Pattern[str]: return re.compile("".join(out)) +def _expected_skill_problems(raw: dict) -> list[str]: + expected = raw.get("expected_skill") + if expected is not None and not (isinstance(expected, str) and expected and "/" not in expected): + return ["expected_skill must be a skill name (its directory name)"] + return [] + + def _check_task(tree: RepoTree, task_dir: str, report: RepoCheckReport) -> None: name = PurePosixPath(task_dir).name where = f"{task_dir}/task.json" @@ -499,9 +509,7 @@ def _check_task(tree: RepoTree, task_dir: str, report: RepoCheckReport) -> None: if not (isinstance(egress, list) and all(isinstance(h, str) and _HOST_RE.match(h) for h in egress)): problems.append("egress must be a list of hosts (DNS labels, `*` for one whole label, the last two literal, optional :port)") egress = [] - expected = raw.get("expected_skill") - if expected is not None and not (isinstance(expected, str) and expected and "/" not in expected): - problems.append("expected_skill must be a skill name (its directory name)") + problems += _expected_skill_problems(raw) for key in ("timeout_s", "max_turns", "budget_usd"): value = raw.get(key) if value is not None and not _positive(value): From 904355ae0ad52ce5c75be802e554ffb82c8f34ed Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 23 Oct 2026 09:46:31 +0000 Subject: [PATCH 31/33] Catch RecursionError and oversized ints in _frontmatter_name Signed-off-by: Anton Volkov --- dokimi/dokimi/repos/arms.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dokimi/dokimi/repos/arms.py b/dokimi/dokimi/repos/arms.py index 3768199c..ee96d526 100644 --- a/dokimi/dokimi/repos/arms.py +++ b/dokimi/dokimi/repos/arms.py @@ -123,10 +123,12 @@ def _frontmatter_name(skill_md: Path) -> str | None: block.append(line) try: meta = yaml.safe_load("\n".join(block)) - except (yaml.YAMLError, ValueError): # ValueError: `name: 2024-13-45` resolves as a timestamp that does not exist + name = meta.get("name") if isinstance(meta, dict) else None + return str(name).strip() or None if name is not None else None + except (yaml.YAMLError, ValueError, RecursionError): + # ValueError: `name: 2024-13-45` resolves as a timestamp that does not exist, `name: 0xfff...` has too many digits + # RecursionError: deeply nested `[` return None - name = meta.get("name") if isinstance(meta, dict) else None - return str(name).strip() or None if name is not None else None def _uncopyable(skill_dir: Path) -> tuple[Path, str] | None: From 24e3532e16954186a9f58ceed80cf539e72a24d1 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 23 Oct 2026 09:46:56 +0000 Subject: [PATCH 32/33] Fix when repo run checks the base .mcp.json in the contract doc Signed-off-by: Anton Volkov --- dokimi/docs/repo-evaluation-contract.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 396e7b30..9ba9b712 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -665,10 +665,11 @@ args_if_present: # argv added after the "{tools}" element wh not a JSON object, or whose `mcpServers` is not an object, is caught up front: `repo validate` warns about the one at each task base (at the eval tree when the source has no git history) and still succeeds, `repo run --tools all` - exits 1 before the image build and any cell, and a cell whose arm wrote - such a file (a `patch:` arm) is a cell error, not a graded run. Fix or remove - the file in the task base; `--tools read` or `none` do not pass it, so - `repo run` does not check it there. + exits 1 before any cell, and a cell whose arm wrote + such a file (a `patch:` arm) is a cell error, not a graded run. The check + reads the task base, so a `patch:` arm that repairs the file does not help. + Fix or remove the file in the task base; `--tools read` or `none` do not + pass it, so `repo run` does not check it there. Profiles for other harnesses (Codex, Gemini CLI, Copilot CLI, Cursor agent, Aider, OpenCode, in-house agents) follow the same shape. Their `guidance` From 9e66dd3a96cbae6ed375745eb6e7af5c7de15d75 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 23 Oct 2026 09:47:13 +0000 Subject: [PATCH 33/33] Split the .mcp.json changelog bullet and reword the guidance:none one Signed-off-by: Anton Volkov --- .../changelog.d/20261020_090000_napetrov_repo-skill-arms.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md index a6b7f668..9cc7047f 100644 --- a/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -24,4 +24,7 @@ This shifts baselines: a repo that ships `.mcp.json` now starts its MCP servers under `--tools all`, so its results differ from earlier runs of the same repo and arm. Compare only batches run after this change. -- **A malformed `.mcp.json` is caught up front** — `repo validate` warns (and still succeeds) about one that is not a JSON object (or whose `mcpServers` is not) at a task base, and `repo run --tools all` exits before any cell, instead of every cell failing as a task failure when the CLI rejects the file; a `patch:` arm that writes one is a cell error. `check_arms` also refuses, before any cell, a base that tracks `.claude` or `.claude/skills` as a symlink; a `guidance:none` arm installs a skill the base ships under the guidance globs; a `SKILL.md` `name` that YAML reads as an impossible date is not a crash. +- **A malformed `.mcp.json` is caught up front** — `repo validate` warns (and still succeeds) about one that is not a JSON object (or whose `mcpServers` is not) at a task base, and `repo run --tools all` exits before any cell, instead of every cell failing as a task failure when the CLI rejects the file; a `patch:` arm that writes one is a cell error. +- `check_arms` refuses, before any cell, a base that tracks `.claude` or `.claude/skills` as a symlink. +- A `guidance:none` arm can install a skill that the base ships under the guidance globs, because the arm drops those files first. +- A `SKILL.md` `name` that YAML reads as an impossible date is not a crash.