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..4a1e35e7 --- /dev/null +++ b/dokimi/changelog.d/20261020_090000_napetrov_repo-skill-arms.md @@ -0,0 +1,30 @@ +### 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`). + `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 + `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`. + +### 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 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. +- **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 (a file that is not UTF-8 text included), 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. diff --git a/dokimi/docs/benchmarking-and-comparison.md b/dokimi/docs/benchmarking-and-comparison.md index 64367f53..7e96d5f2 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_tokens` and +`cache_write_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/docs/preparing-a-repository.md b/dokimi/docs/preparing-a-repository.md index 977e99f3..25dbcc86 100644 --- a/dokimi/docs/preparing-a-repository.md +++ b/dokimi/docs/preparing-a-repository.md @@ -7,9 +7,11 @@ SPDX-License-Identifier: Apache-2.0 > **Status: recommendations.** §1–§7 apply today. The measurement loop in §8 runs > today for guidance arms (`guidance:none`, `guidance:raw`, `guidance:bridge`). -> `guidance:bridge` measures the `CLAUDE.md` import (§7). Arms that add a tool are -> planned; a changed guidance line can be tested today with two tasks that differ -> in a `setup_patches` entry (§8). +> `guidance:bridge` measures the `CLAUDE.md` import (§7). The `skill:` and +> `skill-catalog:` arms, which install skills, run today too +> ([skill arms](benchmarking-and-comparison.md#skill-arms-in-repository-mode)). Arms +> that add any other tool are planned; a changed guidance line can be tested today +> with two tasks that differ in a `setup_patches` entry (§8). This page is for repository owners who want coding agents (Claude Code, Codex, Copilot and others) to build, test, change and review their code reliably. It @@ -240,7 +242,7 @@ Treat each item above as a hypothesis for your repository. use. Today an arm can remove or keep the guidance (`guidance:none`, `guidance:raw`), or inject a file (`guidance:bridge`, which writes a `CLAUDE.md` containing `@AGENTS.md` next to each `AGENTS.md` for profiles with a bridge key). - Testing an added tool needs planned arms. Today, test one added guidance line + Testing an added tool, other than a skill (`skill:`, `skill-catalog:`), needs planned arms. Today, test one added guidance line with two task directories that differ only in a `setup_patches` entry (same prompt, same graders, distinct ids), or in their `base` commit (see [workspace preparation](repo-evaluation-contract.md#workspace-preparation-done-by-the-framework)). @@ -251,9 +253,12 @@ Treat each item above as a hypothesis for your repository. Wilson intervals in `repo report` do not overlap, or raise the cell count. A patch or a second `base` changes only the workspace tree: one image serves every task, so a tool or binary (a pinned formatter, a navigation server) has to be in the image already. With the - `claude-code` profile, which passes `--strict-mcp-config` with no - `--mcp-config`, an MCP server added by a patch is not loaded, so this route - cannot test an added MCP tool yet. + `claude-code` profile, which passes `--strict-mcp-config`, an MCP server is + loaded only from the `.mcp.json` at the workspace root, and only with + `--tools all` (the profile's `args_if_present` rule: the file must exist in the + workspace, and `read` and `none` never pass `--mcp-config`). A patch that adds + or edits that file therefore adds the server in `all` mode; `repo run --tools all` + refuses a task base whose `.mcp.json` is malformed before any cell starts. 3. Look at whether the agent used the change, not only at the score. A guidance line can move the score without the tool it names being called; tool use tells you whether the tool itself mattered. An arm in which the tool was never called diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 00aec556..a0fd2c26 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -260,6 +260,7 @@ belongs to the repository's grader and is passed through unread, except `grader` | `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 @@ -322,6 +323,18 @@ Both write files, as step 1 does: no object of the source's git dir is copied into the workspace, so its object store gains only the blobs of the files the arm wrote, and step 8 drops the guidance the arm replaced. +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`. The arm is named by the +directory names (`guidance:none+skill:x`, no `/`), so `repo compare` can select it. +The `name` in a skill's `SKILL.md` has to equal its directory name. 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. 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 step 7, the commit the agent starts from (§4). `base_sha` names the upstream @@ -332,7 +345,9 @@ grader diffs the agent's changes from `start_sha`. With the arm come files with no counterpart at ``); for `patch:` an `arm_patch` object (the file's absolute `path` and full `sha256`, `added`: the files the patch added that neither the guidance globs nor `.mcp.json` match, and `mcp_servers`: the servers -the patch added to the `.mcp.json` at the root, read before the agent runs). +the patch added to the `.mcp.json` at the root, read before the agent runs). `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 @@ -384,6 +399,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. + For a `patch:` arm with `added` files or `mcp_servers`, `result.json` has an `uptake` object: whether the agent used what the arm added. `files` and `mcp_servers` list what it added (sorted); `commands` counts @@ -634,6 +660,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 and the tool mode is `all` + .mcp.json: ["--mcp-config", ".mcp.json"] ``` - `invoke` and `version_cmd` are argv lists. Each element becomes exactly one @@ -650,6 +678,18 @@ bridge: {for: AGENTS.md, write: CLAUDE.md, content: "@AGENTS.md"} 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). So a `.mcp.json` that is + 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 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` diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index ef165f37..d59eb0a8 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -152,20 +152,26 @@ 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 + from dokimi.repos.workspace import load_tasks, parse_arm out = Path(args.out).resolve() 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([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 ''}", file=sys.stderr) results = run_check(ctx, tasks, out / "check", warm=args.warm, jobs=args.jobs, log=_progress) @@ -192,6 +198,8 @@ 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 ( @@ -211,6 +219,9 @@ 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, 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, tools=args.tools, timeout_s=args.timeout, max_turns=args.max_turns, @@ -228,6 +239,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, log=_progress) except _errors() as exc: @@ -242,6 +256,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.""" @@ -253,8 +289,13 @@ 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[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 + raise ValueError(f"arms {named.spec!r} and another spelling of {named!r} name different skill paths " + f"({named.skills} vs {first_skills[named]}) but would share one cell; " + f"rename a directory or spell the path alike") if named != named.spec: detail = f"{named.patch_path}, sha256 {named.patch_sha256}" if named.patch is not None else named.spec print(f"arm {named} = {detail}", file=sys.stderr) @@ -350,6 +391,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("--allow-unsupported", action="store_true", dest="allow_unsupported", help="Exit 0 when the only tasks that are not ok are unsupported because this host " "misses their task.json `host` requirements (still printed with the reason; " @@ -367,7 +411,10 @@ def register(repo_sub, positive_int, source_help: str, cache_help: str) -> None: "the guidance files of revision REV of the source; the arm is named by REV's full SHA, " "resolved once per batch) | patch:FILE (a unified diff on this host, applied with git " "apply on top of guidance:raw; named patch:@, path and " - "sha256 in meta.json); repeatable or comma-separated") + "sha256 in meta.json) | skill: | skill-catalog: (skills installed in the " + "workspace; a relative dir is in eval_dir; named by the directory, not the path). A " + "guidance part and skills join with + (guidance:none+skill:skills/x); " + "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..ee96d526 --- /dev/null +++ b/dokimi/dokimi/repos/arms.py @@ -0,0 +1,277 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Skills of the ``repo run`` arms: what step 5 of workspace preparation installs (contract §3). + +The arm grammar is :class:`dokimi.repos.workspace.Arm`; a guidance part may be joined with skill parts: + +- ``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 directory name of a skill is its name, and the name in its ``SKILL.md`` +frontmatter has to be the same. +""" + +from __future__ import annotations + +import hashlib +import os +import re +from dataclasses import dataclass +from pathlib import Path, PurePosixPath +from typing import Any + +import yaml + +from dokimi import process + +SKILL_KINDS = ("skill", "skill-catalog") +SKILLS_DIR = ".claude/skills" + + +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.""" + + +@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}" + + +def resolve_skills(arm: Any, content_dir: Path) -> list[Skill]: + """The skills ``arm`` (a :class:`.workspace.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 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}") + 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 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: + 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: + raise ArmError(f"arm {arm.spec!r}: more than one skill named {', '.join(dupes)}") + + +def _named(arm: Any, directory: Path, written: str) -> Skill: + """The skill in ``directory``; its ``SKILL.md`` ``name`` (when it has one) has to be the directory's name, + as it is the name the agent's ``Skill`` call carries and the one the install path and usage match on.""" + declared = _frontmatter_name(directory / "SKILL.md") + if declared is not None and declared != directory.name: + raise ArmError(f"arm {arm.spec!r}: the SKILL.md in {written} is named {declared!r}, but its directory " + f"is {directory.name!r}; rename one, as the agent calls the skill by its name") + return Skill(directory.name, directory) + + +def _frontmatter_name(skill_md: Path) -> str | None: + """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 + block.append(line) + try: + meta = yaml.safe_load("\n".join(block)) + 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 + + +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: + 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 + + +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 _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) + 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, + 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. + 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. 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.""" + 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: + 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) + 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 + if skills and git_dir is not None: + for task in tasks: + _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: + """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_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``. + + ``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] = {"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/bases.py b/dokimi/dokimi/repos/bases.py index be34ebb0..e7fe8c39 100644 --- a/dokimi/dokimi/repos/bases.py +++ b/dokimi/dokimi/repos/bases.py @@ -12,10 +12,57 @@ 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" + + +NOT_UTF8 = "not valid UTF-8 text" + + +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 UnicodeDecodeError: # the runner refuses the same bytes in the workspace + return NOT_UTF8 + except (SourceError, OSError): + 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 +76,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 +89,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.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()} 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/events.py b/dokimi/dokimi/repos/events.py index ea8e2eda..10bca016 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. @@ -188,6 +189,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/harness_profile.py b/dokimi/dokimi/repos/harness_profile.py index c4ac84fe..5b0054fb 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, 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 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 @@ -65,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, *, @@ -74,8 +82,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))})") @@ -90,6 +99,8 @@ def argv( for element in self.invoke: if element == TOOLS_ELEMENT: out.extend(self.tools[tools]) + for name in self.applied_args(tools, present): + out.extend(self.args_if_present[name]) else: out.append(_substitute(element, values)) return out @@ -190,6 +201,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/manifest.py b/dokimi/dokimi/repos/manifest.py index 28cfbc75..931a9ecc 100644 --- a/dokimi/dokimi/repos/manifest.py +++ b/dokimi/dokimi/repos/manifest.py @@ -309,22 +309,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) @@ -340,6 +325,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) + _check_mcp_at_tree(tree, report) else: from .bases import check_bases @@ -347,6 +333,39 @@ 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.""" + 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 {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, eval_dir): + report.errors.append( + f"explore_tasks {manifest.explore_tasks} is outside eval_dir {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() @@ -415,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" @@ -483,6 +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 = [] + 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): diff --git a/dokimi/dokimi/repos/profiles/claude-code.yaml b/dokimi/dokimi/repos/profiles/claude-code.yaml index 6748aedb..3674823a 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/report.py b/dokimi/dokimi/repos/report.py index 576bb28d..b1d40cc6 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -13,10 +13,18 @@ - **unsupported**: cells not run because the host did not meet their task's ``host`` requirements (``unsupported.json``, with the reason). They are neither failures nor missing. -- cost and turns are means over finished cells, as the harness reported them. +- cost, turns and tokens (``input_tokens``, ``output_tokens``, + ``cache_read_tokens``, ``cache_write_tokens``) are means over + finished cells, as the harness reported them. - **uptake**: for an arm that added a file or an MCP server, the cells whose agent used it (``result.json`` ``uptake``, see :mod:`.uptake`) over the cells with a trace to count from. +- 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; ``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). 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. @@ -34,6 +42,8 @@ from .runner import UNSUPPORTED_FILE, slug +_TOKENS = ("input_tokens", "output_tokens", "cache_read_tokens", "cache_write_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.""" @@ -91,6 +101,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}), "model_resolved": sorted({m for r in recs for m in resolved_models(r)})} uptake = uptake_counts(recs) @@ -102,6 +113,14 @@ 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") 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) + 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), @@ -131,6 +150,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[d]} {k.replace('_', ' ')}" + for k, d in (("skill_invoked", "skill_measured"), ("skill_selected_correct", "skill_expected")) + if k in r] if "uptake" in r: notes.append(f"arm's additions used in {r['uptake']['used']}/{r['uptake']['cells']} cell(s)" + unmeasured_note(r["uptake"])) diff --git a/dokimi/dokimi/repos/runner.py b/dokimi/dokimi/repos/runner.py index 1fb14685..0d6c4e28 100644 --- a/dokimi/dokimi/repos/runner.py +++ b/dokimi/dokimi/repos/runner.py @@ -59,6 +59,8 @@ Mount, docker_network, ) +from .arms import skill_unmeasured, skill_usage, slug +from .bases import MCP_CONFIG, NOT_UTF8, mcp_config_error from .events import convert from .uptake import arm_uptake from .grading import EvalContext, failed_outcome, outcome, run_entry @@ -87,10 +89,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}" @@ -147,6 +145,16 @@ def task_caps(task: Task, opts: AgentOptions) -> dict[str, float]: return caps +def _add_arm_use(record: dict[str, Any], meta: dict[str, Any], task: Task, skills: dict[str, Any] | None, + uptake_of: tuple[list[str], list[str]], events: list[Any] | None) -> None: + """Did the agent use what the arm installed (skills) or added (files, MCP servers)?""" + 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) + if any(uptake_of): # an arm that added something to use: did the agent use it (#41 D4) + record["uptake"] = arm_uptake(events, *uptake_of, WORKSPACE) + + def run_cell(ctx: EvalContext, task: Task, arm: str, rep: int, batch_dir: Path, opts: AgentOptions, force: bool = False, host: dict[str, Any] | None = None) -> dict[str, Any]: """Prepare, run the agent, grade. Returns the cell's ``result.json`` (``skipped`` when it existed). @@ -161,10 +169,21 @@ 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, 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 + try: + problem = mcp_config_error((rd / "workspace" / MCP_CONFIG).read_text(encoding="utf-8")) + except UnicodeDecodeError: + problem = NOT_UTF8 + 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 mounts, env = agent_layout(ctx, rd, task) bin_in = f"{HARNESS_DIR}/{opts.harness_bin.name}" @@ -179,7 +198,7 @@ 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=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", @@ -216,6 +235,8 @@ 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") 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) except Exception as exc: # noqa: BLE001 # the grade run failed to start, or the host failed around it @@ -228,8 +249,7 @@ 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), "score_max": task.score["max"] if task.score else None, "agent": agent, "grade": grade} - if any(uptake_of): # an arm that added something to use: did the agent use it (#41 D4) - record["uptake"] = arm_uptake(events, *uptake_of, WORKSPACE) + _add_arm_use(record, meta, task, skills, uptake_of, events) 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 704dc916..5812cc8b 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -12,7 +12,8 @@ 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`` | - ``guidance:rev:`` | ``patch:@``, see :class:`Arm`) + ``guidance:rev:`` | ``patch:@``, see :class:`Arm`), 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 @@ -48,16 +49,19 @@ from dokimi import process +from .arms import ArmError, Skill, resolve_skills, tree_sha256 from .errors import PrepError 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 from .uptake import MCP_CONFIG, base_servers, mcp_servers -ARMS = ("guidance:none", "guidance:raw", "guidance:bridge") +ARMS = ("guidance:none", "guidance:raw", "guidance:bridge") # the guidance parts REV_ARM, PATCH_ARM = "guidance:rev:", "patch:" #: Every form ``repo run --arm`` takes -ARM_FORMS = (*ARMS, f"{REV_ARM}", f"{PATCH_ARM}") +ARM_FORMS = (*ARMS, f"{REV_ARM}", f"{PATCH_ARM}", "skill:", "skill-catalog:") +SKILL_JOIN = "+" +_SKILL_PREFIXES = ("skill:", "skill-catalog:") SNAPSHOT_AUTHOR = ("dokimi", "dokimi@example.invalid") REVIEW_AUTHOR = ("Contributor", "contributor@example.invalid") FIXED_DATE = "2000-01-01T00:00:00+00:00" @@ -76,6 +80,12 @@ class Arm(str): :func:`pin_arm`, so a branch that moved gives a new arm rather than more cells of the old one. ``patch:`` is named ``patch:@``; the file is read once, so every cell applies the same bytes, and its path and full digest go to ``meta.json``. + + A guidance part may be joined with skills by ``+`` (``guidance:none+skill:lint``): ``skill:`` + installs one skill directory (it holds ``SKILL.md``) at ``.claude/skills//``, + ``skill-catalog:`` every subdirectory of ```` that holds one. The arm is named by the + directory names (``skill:lint``), so it holds no ``/`` whatever the path is; ``skills`` keeps the + paths as written, ``base`` is the guidance part alone (``guidance:raw`` when none is written). """ spec: str @@ -84,11 +94,15 @@ class Arm(str): patch: bytes | None patch_path: str | None patch_sha256: str | None + skills: tuple[tuple[str, str], ...] # (kind, path) as written + base: str def __new__(cls, name: str, *, spec: str | None = None, rev: str | None = None, rev_sha: str | None = None, - patch: bytes | None = None, patch_path: str | None = None) -> Arm: + patch: bytes | None = None, patch_path: str | None = None, + skills: tuple[tuple[str, str], ...] = (), base: str | None = None) -> Arm: self = super().__new__(cls, name) self.spec = spec or name + self.skills, self.base = skills, base or name self.rev, self.rev_sha = rev, rev_sha self.patch, self.patch_path = patch, patch_path self.patch_sha256 = hashlib.sha256(patch).hexdigest() if patch is not None else None @@ -96,10 +110,40 @@ def __new__(cls, name: str, *, spec: str | None = None, rev: str | None = None, def parse_arm(spec: str) -> Arm: - """``spec`` as ``repo run --arm`` takes it. A patch file is read here; a revision is resolved by - :func:`pin_arm`, which needs the source's git dir.""" + """``spec`` as ``repo run --arm`` takes it: a guidance part, skill parts, or both joined with ``+``. + A patch file is read here; a revision is resolved by :func:`pin_arm`, which needs the source's git dir.""" if isinstance(spec, Arm): return spec + parts: list[str] = [] + for piece in spec.split(SKILL_JOIN): + if parts and not _starts_part(piece): # a patch path may hold a "+" + parts[-1] += SKILL_JOIN + piece + else: + parts.append(piece) + skills = tuple((p.partition(":")[0], p.partition(":")[2]) for p in parts if p.startswith(_SKILL_PREFIXES)) + guidance = [p for p in parts if not p.startswith(_SKILL_PREFIXES)] + if not skills: + return _parse_guidance(spec) + if len(guidance) > 1: + raise PrepError(f"arm {spec!r}: more than one guidance part") + base = _parse_guidance(guidance[0]) if guidance else Arm("guidance:raw") + names = [] + for kind, path in skills: + leaf = PurePosixPath(path).name if path else "" + if leaf in ("", ".", ".."): + raise PrepError(f"arm {spec!r}: {kind}: needs the path of a directory (its name names the arm)") + names.append(f"{kind}:{leaf}") + name = SKILL_JOIN.join(([str(base)] if guidance else []) + names) + return Arm(name, spec=spec, rev=base.rev, patch=base.patch, patch_path=base.patch_path, skills=skills, + base=str(base)) + + +def _starts_part(text: str) -> bool: + return text in ARMS or text.startswith((REV_ARM, PATCH_ARM, *_SKILL_PREFIXES)) + + +def _parse_guidance(spec: str) -> Arm: + """A guidance part: ``guidance:none|raw|bridge``, ``guidance:rev:`` or ``patch:``.""" if spec in ARMS: return Arm(spec) if spec.startswith(REV_ARM): @@ -148,7 +192,9 @@ def pin_arm(arm: Arm, git_dir: Path, fetch: bool = False, worktree_dir: Path | N sha = proc.stdout.strip() if proc.ok else "" if not _FULL_SHA_RE.match(sha): raise PrepError(f"arm {arm.spec}: no commit {arm.rev!r} in {git_dir}") - return Arm(REV_ARM + sha, spec=arm.spec, rev=arm.rev, rev_sha=sha) + tail = str(arm)[len(arm.base):] if str(arm).startswith(arm.base) else "" # the skill parts + return Arm(REV_ARM + sha + tail, spec=arm.spec, rev=arm.rev, rev_sha=sha, skills=arm.skills, + base=REV_ARM + sha) @dataclass(frozen=True) @@ -181,6 +227,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") @@ -264,16 +314,50 @@ 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) -> dict[str, Any]: + 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. ``arm`` is an :class:`Arm` (``repo run`` parses and pins each once per batch) or an arm - string, parsed and pinned here. + string, parsed and pinned here. ``content_dir`` (the exported ``{eval_dir}``) is where + relative skill paths of the arm are found. """ arm = pin_arm(parse_arm(arm), git_dir) + try: + skills = resolve_skills(arm, content_dir or _content_dir(manifest, task)) if arm.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 arm.base == "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: @@ -292,40 +376,61 @@ 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: raise PrepError(f"task {task.id}: {exc}") from exc + 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] = { "task": task.id, "arm": str(arm), "base_rev": task.base, "base_sha": base_sha, "base_date": base_date, "patches": patches, "start_sha": start_sha, "arm_files": arm_files, **arm_meta, } + 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 _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.""" + 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 _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") + 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)}) + 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.""" @@ -340,13 +445,13 @@ def _apply_arm(ws: Path, env: dict[str, str], git_dir: Path, manifest: RepoManif bridge: dict[str, str] | None) -> tuple[list[str], dict[str, Any]]: """The files the arm removed (none), wrote (bridge, rev) or changed (patch), and its other ``meta.json`` fields: bridge targets left alone, the revision, the patch file.""" - if arm == "guidance:raw": + if arm.base == "guidance:raw": return [], {} if arm.patch is not None: return _apply_patch(ws, env, manifest, arm) tracked = [p for p in _git(ws, env, "ls-files", "-z").split("\0") if p] matched = sorted(p for p in tracked if any(match_glob(g, p) for g in manifest.guidance_globs)) - if arm == "guidance:none" or arm.rev_sha is not None: + if arm.base == "guidance:none" or arm.rev_sha is not None: for rel in matched: (ws / rel).unlink() if arm.rev_sha is None: diff --git a/dokimi/dokimi/schemas/harness_profile.v1.json b/dokimi/dokimi/schemas/harness_profile.v1.json index 20a8900d..60c5d8e2 100644 --- a/dokimi/dokimi/schemas/harness_profile.v1.json +++ b/dokimi/dokimi/schemas/harness_profile.v1.json @@ -44,6 +44,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 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": { "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/repo_eval_fixture.py b/dokimi/tests/repo_eval_fixture.py index e5186c3a..d2e9439b 100644 --- a/dokimi/tests/repo_eval_fixture.py +++ b/dokimi/tests/repo_eval_fixture.py @@ -71,7 +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 == "silent": # the agent dies before it writes a line +if action in ("silent", "startfail"): # silent: dies before a line; startfail: refuses its configuration + if action == "startfail": + sys.stderr.write("Error: Invalid MCP configuration\n") sys.exit(1) cwd = Path.cwd() # Guard against --model missing or being the last arg @@ -105,12 +107,18 @@ lines.append({"type": "assistant", "message": {"model": answered, "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}}]}}) if os.environ.get("FAKE_TOOL_USES"): # more tool calls, as a JSON list of [name, input] lines.append({"type": "assistant", "message": {"content": [ {"type": "tool_use", "id": f"u{i}", "name": n, "input": a} for i, (n, a) in enumerate(json.loads(os.environ["FAKE_TOOL_USES"]))]}}) result = {"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}} if os.environ.get("FAKE_USAGE"): # a subagent (Agent tool) and the token counts of the whole tree lines.append({"type": "assistant", "message": {"content": [ {"type": "tool_use", "id": "t2", "name": "Agent", "input": {"prompt": "look around"}}]}}) @@ -128,7 +136,7 @@ "--max-budget-usd", "{budget}", "{tools}"], "tools": {"none": ["--tools", ""], "read": ["--tools", "Read"], "all": []}, "config_dir_env": "FAKE_CONFIG_DIR", - "env_allow": ["FAKE_ACTION", "FAKE_ANSWER_MODEL", "FAKE_MARKER", "FAKE_USAGE", "FAKE_TOOL_USES", "PATH"], + "env_allow": ["FAKE_ACTION", "FAKE_ANSWER_MODEL", "FAKE_MARKER", "FAKE_SKILLS", "FAKE_USAGE", "FAKE_TOOL_USES", "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_arms.py b/dokimi/tests/test_repo_arms.py index 0aa73872..ce123f13 100644 --- a/dokimi/tests/test_repo_arms.py +++ b/dokimi/tests/test_repo_arms.py @@ -544,3 +544,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_bases.py b/dokimi/tests/test_repo_bases.py index 57ac0ed1..9c81332b 100644 --- a/dokimi/tests/test_repo_bases.py +++ b/dokimi/tests/test_repo_bases.py @@ -179,3 +179,96 @@ 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_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") + 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) + _run_cli(["repo", "validate", source, "--manifest", "dev/evals/repo-eval.yaml", "--json"]) # succeeds + out = json.loads(capsys.readouterr().out) + 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): + 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 + + +def _mcp_base(root: Path, content: bytes | None) -> str: + """Commit a base whose root `.mcp.json` holds ``content`` (none: no file); returns its sha.""" + if content is None: + _write(root, "README.md", "no mcp\n") + else: + (root / ".mcp.json").parent.mkdir(parents=True, exist_ok=True) + (root / ".mcp.json").write_bytes(content) + return _commit_all(root, "base") + + +def _claude_code_profile(): + from dokimi.repos.harness_profile import load_profile + return load_profile("claude-code") + + +def test_the_preflight_checks_every_task_base_not_only_the_first(tmp_path): + from types import SimpleNamespace + + from dokimi.repos.bases import check_mcp_configs + + root = tmp_path / "r" + good = _mcp_base(root, b'{"mcpServers": {}}') + bad = _mcp_base(root, b"{") + tasks = [SimpleNamespace(base=good), SimpleNamespace(base=bad)] + profile = _claude_code_profile() + with pytest.raises(ValueError, match=f"task base {bad}.*not valid JSON"): + check_mcp_configs(root / ".git", tasks, profile, "all") + check_mcp_configs(root / ".git", tasks, profile, "read") # the file is not passed to the agent + check_mcp_configs(root / ".git", tasks[:1], profile, "all") + + +def test_the_preflight_leaves_an_unresolvable_base_and_a_base_without_the_file_to_the_other_checks(tmp_path): + from types import SimpleNamespace + + from dokimi.repos.bases import check_mcp_configs + + root = tmp_path / "r" + none = _mcp_base(root, None) + tasks = [SimpleNamespace(base=none), SimpleNamespace(base="0" * 40), SimpleNamespace(base="no-such-ref")] + check_mcp_configs(root / ".git", tasks, _claude_code_profile(), "all") # no raise + + +def test_a_non_utf8_mcp_json_at_a_base_is_refused_as_the_runner_refuses_it(tmp_path): + from types import SimpleNamespace + + from dokimi.repos.bases import NOT_UTF8, check_mcp_configs, tree_mcp_error + from dokimi.repos.source import GitTree + + root = tmp_path / "r" + base = _mcp_base(root, b"\xff{}") + assert tree_mcp_error(GitTree(root / ".git", base)) == NOT_UTF8 + assert tree_mcp_error(WorkTree(root)) == NOT_UTF8 + with pytest.raises(ValueError, match="not valid UTF-8"): + check_mcp_configs(root / ".git", [SimpleNamespace(base=base)], _claude_code_profile(), "all") diff --git a/dokimi/tests/test_repo_host.py b/dokimi/tests/test_repo_host.py index 964c1f3c..4e37c4d8 100644 --- a/dokimi/tests/test_repo_host.py +++ b/dokimi/tests/test_repo_host.py @@ -34,7 +34,7 @@ from dokimi.repos.report import format_report, load_cells, load_unsupported, summarize from dokimi.repos.runner import UNSUPPORTED_FILE, _mark_unsupported, run_batch from dokimi.repos.workspace import PrepError, Task, prepare_workspace -from repo_eval_fixture import MANIFEST, FakeRunner, fake_harness, make_repo +from repo_eval_fixture import MANIFEST, FakeRunner, fake_harness, git, make_repo, write INTEL_CPUINFO = ("processor\t: 0\nvendor_id\t: GenuineIntel\nmodel name\t: Intel(R) Xeon(R) Platinum 8480+\n" "flags\t\t: fpu sse2 avx2 avx512f\n") @@ -697,3 +697,17 @@ def test_vendors_compare_in_any_case_and_unknown_names_are_refused(): assert unmet(_task({"cpu_vendors": ["genuineintel"]}), intel) == (None, None) with pytest.raises(HostSpecError, match="unknown vendor.*Intel"): requirements(_task({"cpu_vendors": ["Intel"]})) + + +def test_a_skill_arm_cell_of_an_unsupported_task_is_marked_not_run(tmp_path, monkeypatch): + root = _repo(tmp_path) + write(root, ".agent-eval/skills/lint/SKILL.md", "---\nname: lint\ndescription: lint\n---\nLint.\n") + git(root, "add", "-A") + git(root, "commit", "-qm", "skill") + out = tmp_path / "runs" + runner = HostRunner(_probe_out(affinity=2, cpuinfo=AMD_CPUINFO)) + _run(root, out, runner, monkeypatch, "--task", "fix_add", "--task", "fix_race", "--arm", "skill:skills/lint") + batch = out / "b1" + assert [p.name for p in (batch / "fix_race__skill-lint__m-1__r1").iterdir()] == ["unsupported.json"] + assert (batch / "fix_add__skill-lint__m-1__r1/result.json").is_file() + assert {u["cell"] for u in summarize(batch)["unsupported"]} >= {"fix_race__skill-lint__m-1__r1"} diff --git a/dokimi/tests/test_repo_manifest.py b/dokimi/tests/test_repo_manifest.py index da1eb1e8..e116ca87 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_reach.py b/dokimi/tests/test_repo_reach.py index 3d6f8940..6c4aebd8 100644 --- a/dokimi/tests/test_repo_reach.py +++ b/dokimi/tests/test_repo_reach.py @@ -193,6 +193,17 @@ 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 "--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): profile = load_profile("claude-code") env = profile.env(tmp_path / "cfg").resolve({"PATH": "/bin", "AWS_REGION": "us-east-1", diff --git a/dokimi/tests/test_repo_skill_arms.py b/dokimi/tests/test_repo_skill_arms.py new file mode 100644 index 00000000..2d341493 --- /dev/null +++ b/dokimi/tests/test_repo_skill_arms.py @@ -0,0 +1,742 @@ +# 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 dataclasses +import json +import os +import shutil +from pathlib import Path + +import pytest + +from dokimi.cli import build_parser +from dokimi.repos.arms import ArmError, Skill, check_arms, 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 PrepError, Task, _install_skills, parse_arm +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.base == "guidance:none" + assert arm.skills == (("skill", "skills/c++/x"), ("skill-catalog", "/abs/cat")) + assert str(arm) == "guidance:none+skill:x+skill-catalog:cat" and "/" not in arm # `repo compare` reads BATCH/ARM + assert arm.spec == "guidance:none+skill:skills/c++/x+skill-catalog:/abs/cat" + alone = parse_arm("skill:a") + assert alone.base == "guidance:raw" and str(alone) == "skill:a" + assert parse_arm("guidance:bridge").skills == () + for bad, why in (("main-raw", "unknown arm"), ("skill:", "needs the path"), ("skill:..", "needs the path"), + ("guidance:none+guidance:raw+skill:x", "more than one guidance")): + with pytest.raises(PrepError, 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([parse_arm("skill-catalog:skills")], tmp_path, [task]) + check_arms([parse_arm("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 + 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_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"}}]}}) + 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): + 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 _installed_dest(tmp_path: Path): + _skill(tmp_path / "eval", "skills/lint", "lint") + ws = tmp_path / "ws" + dest = ws / ".claude/skills/lint" + dest.parent.mkdir(parents=True) + return ws, dest, Skill("lint", tmp_path / "eval/skills/lint") + + +@pytest.mark.parametrize("make", ["file", "nested_file", "symlinked_dir", "dangling_link", "link_to_empty_dir"]) +def test_a_skill_directory_that_holds_anything_but_empty_directories_is_refused_and_kept(tmp_path, make): + """Only the emptied directories of a dropped skill are removed; real files, links and their targets stay.""" + ws, dest, skill = _installed_dest(tmp_path) + keep = tmp_path / "elsewhere" + keep.mkdir() + (keep / "precious.md").write_text("keep\n") + if make == "file": + dest.mkdir() + (dest / "SKILL.md").write_text("mine\n") + elif make == "nested_file": + (dest / "references/deep").mkdir(parents=True) + (dest / "references/deep/a.md").write_text("mine\n") + elif make == "symlinked_dir": + (dest / "references").mkdir(parents=True) + (dest / "references/linked").symlink_to(keep, target_is_directory=True) + elif make == "dangling_link": + dest.symlink_to(tmp_path / "nowhere") + else: + (keep / "empty").mkdir() + dest.symlink_to(keep / "empty", target_is_directory=True) + with pytest.raises(PrepError, match="already has"): + _install_skills(ws, Task("t", {}, tmp_path), [skill]) + assert (keep / "precious.md").read_text() == "keep\n" + assert os.path.lexists(dest) # nothing was removed to make room, and the skill was not installed over it + assert not (dest / "SKILL.md").exists() or (dest / "SKILL.md").read_text() == "mine\n" + if make == "link_to_empty_dir": + assert dest.is_symlink() and (keep / "empty").is_dir() + if make == "nested_file": + assert (dest / "references/deep/a.md").read_text() == "mine\n" + + +def test_a_skill_directory_of_only_empty_directories_is_replaced(tmp_path): + ws, dest, skill = _installed_dest(tmp_path) + (dest / "references/deep").mkdir(parents=True) + out = _install_skills(ws, Task("t", {}, tmp_path), [skill]) + assert out[0]["name"] == "lint" and (dest / "SKILL.md").is_file() and not (dest / "references").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", + "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_tokens, t.cache_write_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_tokens"] == 100 and result["agent"]["cache_write_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_tokens"] == 100.0 and cat["cache_write_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 + + +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) + (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_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") + _skill(tmp_path, "a_b", "a_b") + _skill(tmp_path, "s/one", "one") + _skill(tmp_path, "catalog-s", "catalog-s") + with pytest.raises(ArmError, match="share one result directory"): + check_arms([parse_arm("skill:a/b"), parse_arm("skill:x/b")], tmp_path, []) + with pytest.raises(ArmError, match="share one result directory"): + 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_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) + 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", "--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] + 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" + + +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"] + + +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"}) + + +@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"] + + +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"] + + +@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) + _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) + + +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") + 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("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 + + +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 + + +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"]