diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index 5a68438..7fbd946 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -44,6 +44,77 @@ jobs: - name: Run the suite run: python -m unittest discover -s tests -t . --verbose + sandbox: + name: Sandbox machinery (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + # Both sandboxes a run can get. Ubuntu has Docker, which is the + # default and what CI uses; Windows has none, which is the other + # supported shape. The Windows leg is the only place the cross-platform + # sandbox helpers are exercised at all -- inspect's own assume a POSIX + # guest, and a scorer that cannot list the guest cannot grade it. + os: [ubuntu-latest, windows-latest] + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + # inspect-ai pulls in the order of eighty packages, so the download + # is most of this step. Keyed on pyproject, which is where the extra + # is declared and the only thing that changes what gets installed. + cache: pip + cache-dependency-path: pyproject.toml + + - name: Install with the inspect extra + run: python -m pip install --upgrade pip && python -m pip install ".[inspect]" + + # The case seeds a file and expects that same file. Nothing an agent + # does can satisfy it and nothing an agent does can break it, so what + # passes or fails is the layer underneath: the sandbox started, the + # fixture was staged into it, the guest was listed, and the listing was + # matched. That is where every cross-platform bug so far has been. + - name: Build a repo to test + shell: bash + run: | + set -euo pipefail + mkdir -p fixture/demo-skill/evals fixture/demo-skill/fixtures + cat > fixture/demo-skill/SKILL.md <<'EOF' + --- + name: demo-skill + description: Does demonstrable things, for a test that needs a skill. + --- + EOF + echo "seeded by the case, not produced by the agent" \ + > fixture/demo-skill/fixtures/seeded.txt + cat > fixture/demo-skill/evals/evals.json <<'EOF' + { + "evaluations": [ + {"id": "demo-a", "skill_should_trigger": true, "prompt": "do the demo thing", + "workspace": "fixtures", "files_exist": ["seeded.txt"]}, + {"id": "demo-b", "skill_should_trigger": true, "prompt": "another way to ask"}, + {"id": "demo-c", "skill_should_trigger": true, "prompt": "a third phrasing"}, + {"id": "demo-d", "skill_should_trigger": false, "prompt": "something adjacent"}, + {"id": "demo-e", "skill_should_trigger": false, "prompt": "something else entirely"} + ] + } + EOF + + # No key, no model, no agent: a stub solver stands in for the agent so + # only the machinery beneath it can pass the case. That is what makes + # this runnable on a pull request from a fork, which is exactly where a + # cross-platform sandbox bug shows up. + - name: Exercise the sandbox, the staging and the listing + # The expected provider is supplied per platform rather than read back + # from the harness. Ubuntu must get a real container; a leg that + # quietly fell back to `local` would pass every check below against the + # host filesystem and stay green under the name "Sandbox machinery". + run: > + python tools/sandbox_smoketest.py fixture --skill demo-skill + --expect-sandbox ${{ matrix.os == 'windows-latest' && 'local' || 'docker' }} + action: name: Action against a throwaway repo runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index f998f82..29e0160 100644 --- a/.gitignore +++ b/.gitignore @@ -11,5 +11,10 @@ dist/ # Reports and kept transcripts, written into the repo under test .skillscope/ +# inspect's own run transcripts, wherever a tool was invoked from. Committing +# one is easy to do by accident and they are large, opaque and reproducible. +*.eval +tools/logs/ + # Nested checkout of this repo, used by the reusable workflows in CI .skillscope-action/ diff --git a/docs/authoring-evals.md b/docs/authoring-evals.md index a5c4743..77580ba 100644 --- a/docs/authoring-evals.md +++ b/docs/authoring-evals.md @@ -152,14 +152,31 @@ Setup a dataset cannot express: cloning a repo, tearing down a container, running an external scoring script. Every function is optional: ```python -def setup_session(cache_dir): ... # once per skill; returns {name: value} for {placeholders} in prompts -def setup(workspace, case, ctx): ... # before each case; may return more placeholders -def teardown(workspace, case, ctx): ... -def check(run, case, ctx): ... # after each case; raise AssertionError to fail it +def setup(workspace, case, ctx): ... # before each case +def teardown(workspace, case, ctx): ... # after it, even if the agent blew up ``` -`teardown` runs even when the agent itself blew up, and a `check` that raises -fails its case without killing the run. +`teardown` runs even when the agent itself blew up: it is wired to inspect's +`Task.cleanup`, which runs inside a `finally` under a shielded cancel scope, so +an exception or a cancelled sample does not skip it. + +**Two entry points are no longer supported**, and a skill that defines either +is refused rather than having it silently skipped: + +```python +def setup_session(cache_dir): ... # NOT supported +def check(run, case, ctx): ... # NOT supported +``` + +`check` was handed the retired engine's `Run` object; the engines here have +inspect's `TaskState`, which is a different thing with different attributes. +`setup_session` returned `{name: value}` pairs substituted into prompts as +`{placeholders}`, and prompts are built before any hook runs -- so honouring it +would mean constructing every case after the hook rather than before. A `setup` +that *returns* placeholders is refused at runtime for the same reason. + +If you were relying on either, move the work into `setup`/`teardown`, or put +the value in the dataset. Keep prompts and expectations in the dataset even when you use hooks, so what is being asserted stays readable without opening Python. @@ -170,9 +187,9 @@ for it rather than cloning: ```python from skillscope import sources -def setup_session(cache_dir): - source = sources.resolve("my-skill", cache_dir) - return {"repo": source.path} +def setup(workspace, case, ctx): + source = sources.resolve("my-skill", workspace) + # Use source.path here; returning it as a {placeholder} is not supported. ``` `resolve` answers with the checkout that matches the skill under test: an diff --git a/docs/usage.md b/docs/usage.md index 17cd0f9..4cbb79f 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -208,6 +208,99 @@ Legs with a scoped environment run as a separate job, because a job's credentials are fixed before its matrix expands. A repo that declares no scoped environment gets one matrix, labels and all. +## Which engine grades a run + +`--engine` chooses what actually runs the cases. The dataset, the CLI and the +reports are identical whichever you pick; only the thing driving the agent +changes. + +| `--engine` | What runs | Runs in | Needs | +| --- | --- | --- | --- | +| `claude-code-no-sandbox` (default) | The real CLI, under `inspect_ai` | the host | the CLI on `PATH` | +| `claude-code` | The real CLI, via `inspect_swe` | a sandbox | `skillscope[verify]`, Linux only | + +Both drive the agent a skill is written for, so what differs between them +is **where the agent runs**, not what it is. That is the axis worth choosing +along: the host measures the machine as it is, with whatever else is installed +on it, and the sandbox measures the skill alone. When the two disagree, the +disagreement is usually a fact about one of those environments rather than +about the skill -- which is a thing one engine on its own cannot tell you. + +`claude-code` is a reporting leg, never a gate. Harness runs are +nondeterministic and the harness is not what is being graded, so a divergence +there is a question about the skill rather than a build failure. + +**Routing runs on both**, and the choice matters more there than it does +for behavioral. A stray user-level skill on the runner does not spoil one +case's grade -- it is offered for every prompt, so it changes every decision at +once while the run still reports a clean accuracy. `claude-code` is the only +leg immune by construction: its guest has no `~/.claude` to contribute. + +The host leg cannot read the CLI's session-init event, so it has no way to +notice such contamination or report it -- and therefore refuses to run a +routing leg at all unless `ANTHROPIC_API_KEY` is set, which is what lets it +redirect the CLI's config dir away from the runner's own. Refusing beats being +quietly wrong about every case. + +Both legs stop at the decision, by different routes. `claude-code`'s calls +cross inspect's bridge, so the one that reveals the decision is declined +*before* it runs, and `--max-tool-calls` / `--max-inspection-calls` ride the +same path. `claude-code-no-sandbox` drives the CLI as a subprocess with nothing +to intercept its calls, so it reads the `stream-json` the CLI prints as it goes +and kills the process group the moment a skill fires -- one call of overshoot, +and the same rule. The report records which caps actually applied rather than +which were asked for. + +### Where a sandboxed run is sandboxed + +Two separate decisions, made by different people. + +**Which provider** is a property of the runner, chosen with +`SKILLSCOPE_SANDBOX`. Docker by default; `podman` on a host that has that +instead; `local` to skip the container. `local` is for working locally rather +than for CI, because a graded run that quietly dropped its sandbox would report +the same numbers with none of the isolation. + +Podman needs three things, and each was discovered by the next one failing: + +* `pip install 'skillscope[podman]'`. The provider is registered by a separate + package through an entry point, so the podman binary alone is not enough. +* `podman-compose`, and `INSPECT_PODMAN_COMPOSE=podman-compose`. Bare + `podman compose` is a shim that delegates to whichever compose provider it + finds, which on a host that also has Docker is Docker's -- and that then + talks to a daemon podman was chosen to avoid. +* A search registry, because podman will not guess one. Docker assumes Docker + Hub for an image name with no registry; podman refuses, and the default + sandbox image is named without one. `unqualified-search-registries = + ["docker.io"]` in `/etc/containers/registries.conf`. + +Podman is worth the setup where the runner's user cannot reach the Docker +socket, since it is daemonless and rootless and needs neither that nor group +membership. + +**What the sandbox must provide** is a property of the skill, declared as +`sandbox: compose.yaml` in its `evals/machine.yml`, resolved beside it. Skills get a container with +no network by default; one that installs a server or pulls a model cannot run +that way and says so. Selecting a provider does not discard what a skill asked +for -- the compose file rides along. + +[`examples/skill-with-a-device/evals/`](../examples/skill-with-a-device/evals) +is the pair, worked through: a `machine.yml` that asks for GPU runners and +names a compose file, and the compose file that binds the devices in and +grants egress. The two are not substitutes. `labels:` decides which machine the +job lands on; `sandbox:` decides whether the container on it can see the +hardware that machine has. A skill that sets only the first gets the right +runner and a container that cannot reach its device. + +Windows is the exception to both: inspect's sandbox layer and every tool built +on it assume a POSIX guest, so those legs run unsandboxed and trade isolation +for running on the platform they are meant to test. + +To see what changing engine would do to your own datasets before changing it, +[`tools/benchmark_engines.py`](../tools/benchmark_engines.py) runs the same +cases through two engines and reports per-case agreement, measured against how +much one engine already disagrees with itself. + ## In CI: one job [`reusable.yml`](../.github/workflows/reusable.yml) grades a repo's skills with diff --git a/examples/skill-with-a-device/evals/compose.yaml b/examples/skill-with-a-device/evals/compose.yaml new file mode 100644 index 0000000..f91d4aa --- /dev/null +++ b/examples/skill-with-a-device/evals/compose.yaml @@ -0,0 +1,49 @@ +############################################################################### +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT +############################################################################### + +# Example: the sandbox for a skill that needs a GPU and network egress. +# +# Named by `sandbox: compose.yaml` in the machine.yml beside this file. The +# default sandbox is this minus `devices`, `group_add`, `security_opt`, `ipc` +# and with `network_mode: none` -- so everything below is the opt-in, and a +# skill that needs none of it should not ship a compose file at all. + +services: + # inspect runs the cases in the service named `default`. One service with a + # different name and `x-default: true` works too; several services without + # either is the error you get instead of a container. + default: + # The default image. Whatever you put here needs inspect's tool support to + # be reachable inside the container, so start from this one and add to it + # via `build:` rather than replacing it with a bare vendor image. + image: "aisiuk/inspect-tool-support" + + # inspect execs into a container that is already up, so the container has + # to stay up on its own. Without these it exits immediately and every case + # fails on a sandbox that is not there. + command: "tail -f /dev/null" + init: true + stop_grace_period: 1s + + # NETWORK. The default sandbox sets `network_mode: none`; omitting that key + # is what grants egress. Only do this for a skill that genuinely needs it + # -- pulling a model, reaching a package index -- because it is also the + # thing that lets a case reach the internet and stop being reproducible. + # + # `network_mode: none` here would restore the default. + + # DEVICE. The GPU nodes, bound in from the host. The job has to already be + # on a machine that has them; that is what `labels:` in machine.yml is for. + devices: + - "/dev/kfd" + - "/dev/dri" + + # What the userspace stack needs on top of the device nodes themselves. + group_add: + - video + security_opt: + - seccomp=unconfined + ipc: host diff --git a/examples/skill-with-a-device/evals/machine.yml b/examples/skill-with-a-device/evals/machine.yml new file mode 100644 index 0000000..0aeff60 --- /dev/null +++ b/examples/skill-with-a-device/evals/machine.yml @@ -0,0 +1,30 @@ +############################################################################### +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT +############################################################################### + +# Example: a skill whose behavioral cases need real hardware. +# +# This file lives at /evals/machine.yml. It answers two questions that +# are asked by different people, and it is worth keeping them apart: +# +# * `os` / `labels` say which CI runner the job lands on -- a machine with +# the device physically in it. That is the repo's scheduling problem. +# * `sandbox` says what the container on that machine has to provide. That is +# the skill's problem, and without it the job lands on the right hardware +# and then runs in a container that cannot see it. +# +# Both are needed. Neither implies the other. + +os: + - Linux + +# The hardware the cases need, named as a runner label rather than a pool. +labels: + - mi300x + +# Resolved beside this file, in evals/. Absent means the default container: +# no network, no devices, which is what a skill that only reads and writes +# files should want. +sandbox: compose.yaml diff --git a/pyproject.toml b/pyproject.toml index d05674d..3ab8e4e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,16 +15,54 @@ requires-python = ">=3.10" license = { text = "MIT" } authors = [{ name = "Advanced Micro Devices, Inc." }] -# The runner itself is standard library only, so a run needs no wheels beyond -# this package. PyYAML is the one exception: it reads the optional -# evals/machine.yml, which only CI planning touches. -dependencies = ["pyyaml>=6.0"] +# Every engine runs under inspect_ai, so it is required rather than optional. +# +# It was an extra while `legacy` existed: that engine was standard library only, +# so a repo that had not migrated installed nothing new. `legacy` is gone, and +# keeping the extra would ship a tool whose two graded commands both fail on a +# plain `pip install` -- which is what CI found, by running the suite without +# extras and getting `ModuleNotFoundError: inspect_ai` from the default engine. +# +# The static commands -- `structural`, `references`, `select`, `list-skills` -- +# still import none of it; they are just no longer the only thing that works +# out of the box. +# +# `anthropic` is listed explicitly: inspect-ai treats every model provider as +# optional, so installing it alone gets you a harness that cannot reach a model. +# PyYAML reads the optional evals/machine.yml, which only CI planning touches. +dependencies = [ + "pyyaml>=6.0", + "inspect-ai>=0.3.263", + "anthropic>=0.40", +] + +[project.optional-dependencies] +# Kept as a name so `skillscope[inspect]` in a downstream workflow or a pinned +# requirements file keeps resolving. It adds nothing now. +inspect = [] + +# For a runner that has podman rather than docker. The provider registers +# itself through an `inspect_ai` entry point, so installing it is the whole +# setup; `SKILLSCOPE_SANDBOX=podman` then selects it. +podman = ["skillscope[inspect]", "inspect-podman"] + +# The Claude Code verification leg (`--engine claude-code`). Kept out of the +# `inspect` extra because it is a reporting-only cross-check, not something a +# graded run needs -- and because it only works on a POSIX guest. +# +# 0.2.71 for `claude_code(effort=)`. Not cosmetic: without it this leg runs at +# the model's default reasoning effort while every other leg runs at the one it +# was given, and the gap between them reads as a fact about isolation. 0.2.70 +# raises `TypeError: Unexpected keyword argument(s): effort`, and a runner that +# already had it satisfied the old floor and never upgraded -- so the trial +# installed a version that could not run the leg. +verify = ["skillscope[inspect]", "inspect-swe>=0.2.71"] [project.scripts] skillscope = "skillscope.cli:main" [tool.setuptools] -packages = ["skillscope"] +packages = ["skillscope", "skillscope.engine"] [tool.setuptools.package-data] skillscope = ["data/*.json", "schema/*.json"] diff --git a/skillscope/agent.py b/skillscope/agent.py index 53bce6d..d5ebca5 100644 --- a/skillscope/agent.py +++ b/skillscope/agent.py @@ -41,7 +41,7 @@ from dataclasses import dataclass from pathlib import Path -from . import datasets, deadline +from . import datasets, deadline, usage DEFAULT_MODEL = os.environ.get("SKILLSCOPE_MODEL", "opus") DEFAULT_EFFORT = os.environ.get("SKILLSCOPE_EFFORT", "high") @@ -70,10 +70,18 @@ def is_automated_env() -> bool: ) +# Model providers that reach no cloud service. The CI pin exists to keep paid +# runs comparable between runs; one of these grades nothing and costs nothing, +# so pinning it only turns a free wiring check into a run that needs a key. +NO_PROVIDER_PREFIXES = ("mockllm",) + + def enforce_model_policy(model: str | None) -> str | None: """Coerce non-opus models to opus in CI; pass through otherwise.""" if model is None or not is_automated_env() or "opus" in model.lower(): return model + if model.lower().startswith(NO_PROVIDER_PREFIXES): + return model _safe_print( f"[skillscope] automated run: coercing model '{model}' -> " f"'{AUTOMATED_MODEL}' to pin the CI model." @@ -387,6 +395,9 @@ def __init__(self, *, workspace: Path, events: list[dict], judge_model: str | No result_text = "" for ev in events: + # Recording what the run spent is what lets it be compared against + # the same cases on the other engine. + usage.record_stream_event(ev) if ev.get("type") == "result" and isinstance(ev.get("result"), str): result_text = ev["result"] diff --git a/skillscope/behavior.py b/skillscope/behavior.py index 154f48c..2c86bc6 100644 --- a/skillscope/behavior.py +++ b/skillscope/behavior.py @@ -25,7 +25,6 @@ from types import ModuleType from . import datasets, deadline -from .agent import Check, claude from .datasets import Case @@ -40,6 +39,11 @@ class BehaviorOutcome: elapsed_s: float checks: list[dict] = field(default_factory=list) error: str | None = None + # Set when the failure was the model provider's rather than the skill's. + # A behavioral case that failed on a gateway 504 is a skill nobody + # measured, and reporting it beside one that genuinely failed its + # expectations invites exactly the wrong conclusion. + degraded: bool = False # -------------------------------------------------------------------------- @@ -47,30 +51,6 @@ class BehaviorOutcome: # -------------------------------------------------------------------------- -def load_hooks(skill: str) -> ModuleType | None: - """Import ``/evals/hooks.py`` if it exists. - - A hook module holds environment plumbing only -- cloning a repo, tearing - down a container, running an external scoring script. Prompts and - expectations stay in the dataset, so the thing being asserted is always - readable without opening Python. - - Recognized (all optional):: - - setup_session(cache_dir) -> dict of template vars, once per skill - setup(workspace, case, ctx) -> dict of extra template vars, per case - teardown(workspace, case, ctx) - check(run, case, ctx) -> raise AssertionError to fail the case - """ - path = datasets.hooks_path(skill) - if not path.is_file(): - return None - spec = importlib.util.spec_from_file_location(f"evalhooks_{skill.replace('-', '_')}", path) - if spec is None or spec.loader is None: - raise SystemExit(f"error: could not import {path}") - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module def expand(text: str, ctx: dict) -> str: @@ -90,101 +70,9 @@ def expand(text: str, ctx: dict) -> str: # -------------------------------------------------------------------------- -def run_case( - case: Case, ctx: dict, hooks: ModuleType | None, model: str, effort: str -) -> BehaviorOutcome: - """Stage one skill, run the prompt to completion, grade what happened.""" - assert case.skill is not None - bound = deadline.active() - if bound is not None and bound.expired(): - print(f" [FAIL] {case.id}: {bound.message()}", flush=True) - return BehaviorOutcome( - id=case.id, - skill=case.skill, - prompt=case.prompt, - passed=False, - elapsed_s=0.0, - error=bound.message(), - ) - - seed = (datasets.skill_path(case.skill) / case.workspace) if case.workspace else None - started = time.perf_counter() - case_ctx = dict(ctx) - checks: list[Check] = [] - error: str | None = None - try: - with claude(model, skill=case.skill, effort=effort, seed=seed) as session: - workspace = session.workspace - assert workspace is not None - if hooks is not None and hasattr(hooks, "setup"): - case_ctx.update(hooks.setup(workspace, case, case_ctx) or {}) - try: - run = session.prompt(expand(case.prompt, case_ctx)) - checks = run.evaluate( - logs_contain=[expand(t, case_ctx) for t in case.logs_contain], - files_exist=[expand(p, case_ctx) for p in case.files_exist], - expected_behavior=case.expected_behavior, - unexpected_behavior=case.unexpected_behavior, - ) - if hooks is not None and hasattr(hooks, "check"): - try: - hooks.check(run, case, case_ctx) - checks.append(Check("hook", "evals/hooks.py check()", True)) - except AssertionError as exc: - checks.append(Check("hook", "evals/hooks.py check()", False, str(exc)[:400])) - finally: - if hooks is not None and hasattr(hooks, "teardown"): - hooks.teardown(workspace, case, case_ctx) - except Exception as exc: # noqa: BLE001 -- an infra failure is a result too - error = f"{type(exc).__name__}: {exc}" - traceback.print_exc() - - elapsed = round(time.perf_counter() - started, 2) - passed = error is None and all(c.passed for c in checks) and bool(checks) - if error is None and not checks: - error = "case has no behavioral assertions to grade" - print( - f" [{'PASS' if passed else 'FAIL'}] {case.id}: " - f"{sum(1 for c in checks if c.passed)}/{len(checks)} checks in {elapsed}s" - + (f" -- {error}" if error else ""), - flush=True, - ) - return BehaviorOutcome( - id=case.id, - skill=case.skill, - prompt=case.prompt, - passed=passed, - elapsed_s=elapsed, - checks=[asdict(c) for c in checks], - error=error, - ) -def run(skills: list[str], cases: list[Case], model: str, effort: str) -> list[BehaviorOutcome]: - """Run every behavioral case, grouped by skill so session setup happens once.""" - outcomes: list[BehaviorOutcome] = [] - for skill in skills: - skill_cases = [c for c in cases if c.skill == skill and c.has_behavior] - if not skill_cases: - continue - - hooks = load_hooks(skill) - ctx: dict = {} - cache_dir: Path | None = None - if hooks is not None and hasattr(hooks, "setup_session"): - cache_dir = Path(tempfile.mkdtemp(prefix=f"evalcache-{skill}-")) - print(f"[behavioral] {skill}: running evals/hooks.py setup_session()", flush=True) - ctx.update(hooks.setup_session(cache_dir) or {}) - try: - print(f"[behavioral] {skill}: {len(skill_cases)} case(s)", flush=True) - for case in skill_cases: - outcomes.append(run_case(case, ctx, hooks, model, effort)) - finally: - if cache_dir is not None: - shutil.rmtree(cache_dir, ignore_errors=True) - return outcomes - def summarize(outcomes: list[BehaviorOutcome], meta: dict) -> dict: per_skill: dict[str, dict] = {} @@ -204,12 +92,36 @@ def summarize(outcomes: list[BehaviorOutcome], meta: dict) -> dict: "checks": sum(len(o.checks) for o in outcomes), "checks_passed": sum(1 for o in outcomes for c in o.checks if c["passed"]), "errors": sum(1 for o in outcomes if o.error), + # Of those errors, the ones the provider caused. A behavioral run + # with these in it has not measured the skills it names. + "degraded": sum(1 for o in outcomes if o.degraded), }, "per_skill": per_skill, "cases": [asdict(o) for o in outcomes], } + + +def _isolation_note(meta: dict) -> str: + """One line saying whether the agent was contained while it worked. + + Behavioral runs the agent to completion with permissions bypassed, so + whether it was isolated changes what the numbers cost to obtain. A report + that omits it reads as though it were, and the answer differs per platform: + the Windows legs have no sandbox available at all. + """ + where = meta.get("sandbox") + if where is None: + return "" + if meta.get("sandbox_isolated"): + return f"Cases ran isolated, in `{where}`." + return ( + f"**Cases ran unsandboxed** (`{where}`): the agent worked directly in " + "the harness's own filesystem, with permissions bypassed." + ) + + def render_markdown(summary: dict) -> str: totals = summary["totals"] meta = summary["meta"] @@ -220,6 +132,8 @@ def render_markdown(summary: dict) -> str: f"({totals['checks_passed']}/{totals['checks']} individual expectations) " f"on `{meta['model']}` (effort `{meta['effort']}`).", "", + _isolation_note(meta), + "", "| Skill | Cases | Passed | Expectations | Met |", "| --- | --- | --- | --- | --- |", ] diff --git a/skillscope/cli.py b/skillscope/cli.py index f9d93b4..3089aa5 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -72,10 +72,50 @@ from concurrent.futures import ThreadPoolExecutor from pathlib import Path -from . import behavior, config, datasets, deadline, references, routing, structure +from . import ( + behavior, + config, + datasets, + deadline, + engine, + references, + routing, + structure, + usage, +) from . import selection as select_module from .agent import check_api_reachable, enforce_model_policy +# The engines a graded run can be driven by. +# claude-code -- the real CLI in a sandbox, via inspect_swe (Linux only) +# claude-code-no-sandbox -- the real CLI on the host, under inspect_ai (any platform) +# +# Both drive the agent a skill is actually written for; what differs is where +# it runs. Two others have been removed. `inspect` drove a harness-independent +# agent, and carried a smaller tool set than the real harness, so a skill +# referring to a tool it did not have failed for a reason that was the agent's +# rather than the skill's. `legacy` drove the CLI as a bare subprocess, and +# `claude-code-no-sandbox` replaced it: measured per case against the run's own +# noise floor, the two agreed on every routing case outside that floor, across +# two runs, and on 12 of 13 behavioral cases. +ENGINES = ["claude-code", "claude-code-no-sandbox"] + +# Every engine runs under inspect_ai now. Kept as its own name because the code +# that branches on it is about what an inspect-based engine needs -- a preflight +# of its own, a report that names the engine -- rather than about which engines +# happen to exist today. +INSPECT_ENGINES = tuple(ENGINES) + +# The engines routing has a leg for: all of them. Derived rather than listed, +# because the last time two engine lists were kept by hand one fell behind and +# the runs silently went somewhere else. +ROUTING_ENGINES = tuple(ENGINES) + +# What a run uses when nobody says. The host leg rather than the sandboxed one: +# it runs on every platform, where `claude-code` needs a POSIX guest and a +# container runtime, and a default that refuses on Windows is not a default. +DEFAULT_ENGINE = "claude-code-no-sandbox" + # Where JSON reports land inside the repo under test. One gitignored directory # rather than a path per repo, so a report is always in the same place. RUNS_DIRNAME = Path(".skillscope") / "runs" @@ -334,6 +374,32 @@ def _prepare_graded_run( selected = _selected_skills(args.skill) _structural_or_exit(selected if scope is None else sorted(set(scope))) args.model = enforce_model_policy(args.model) or args.model + if getattr(args, "engine", DEFAULT_ENGINE) in INSPECT_ENGINES: + # The CLI-based reachability probe tests something these engines do not + # use, but they still need one of their own: a graded run starts + # containers and installs skills before it first reaches a provider, so + # without this a bad key surfaces as a task that failed after all that. + engine.require(args.engine) + # Before the model probe, because it costs nothing and a run that + # cannot honour a skill's setup should not first spend a round trip + # finding out the credentials are fine. + _require_hook_support( + args.engine, selected, getattr(args, "command", "") + ) + if not args.skip_preflight: + from .engine import models as engine_models + + ok, detail = engine_models.check_reachable(engine_models.resolve(args.model)) + if not ok: + raise SystemExit(f"error: model not reachable -- {detail}") + if args.engine == "claude-code-no-sandbox": + # This engine reaches the provider for the judge but drives the + # real CLI as the agent, so both credentials are load-bearing + # and probing one proves nothing about the other. + ok, detail = check_api_reachable(args.model) + if not ok: + raise SystemExit(f"error: claude API not reachable -- {detail}") + return selected if not args.skip_preflight: ok, detail = check_api_reachable(args.model) if not ok: @@ -341,6 +407,124 @@ def _prepare_graded_run( return selected +def _require_hook_support(engine: str, skills: list[str], command: str) -> None: + """Refuse a run whose hooks use entry points this engine cannot honour. + + `evals/hooks.py` is environment plumbing -- clearing stale containers, + tearing down a service. `setup` and `teardown` run here: inspect's + `Task.setup` and `Task.cleanup` are the same two shapes, and `cleanup` + runs inside a `finally` under a shielded cancel scope, so teardown still + happens when the agent raises. + + `check` and `setup_session` do not, and this refuses rather than skipping + them. Skipping is the dangerous half: a hook that did not run leaves no + trace in the report, the case is graded as though it had, and the failure + surfaces later as a skill that mysteriously does not work on this runner. + + Refused by entry point, not by the file existing. An earlier version + grounded any skill that shipped a hook at all -- which took the behavioral + leg away from the one skill in the catalogue whose hook these engines can + run perfectly well. + + Routing is exempt on every engine: it installs the skills, asks which one + fires, and executes nothing. + """ + if command != "behavioral": + return + + from .engine import hooks as engine_hooks + + blocked: dict[str, list[str]] = {} + for skill in skills: + names = engine_hooks.unsupported_entry_points(engine_hooks.load(skill)) + if names: + blocked[skill] = names + if not blocked: + return + + listed = "\n".join( + f" {skill}: {datasets.hooks_path(skill)} defines " + f"{', '.join(names)}" + for skill, names in blocked.items() + ) + raise SystemExit( + f"error: --engine {engine} cannot run every entry point these " + f"skills' hooks define:\n{listed}\n" + " They would be skipped silently, and the cases graded as though " + "they had run.\n" + " `setup` and `teardown` are supported. `check` needs the legacy " + "engine's Run object, and `setup_session` returns template variables " + "that are substituted before any hook runs -- neither has an " + "equivalent here.\n" + " Move what they do into the dataset, or into setup/teardown." + ) + + +def _sandbox_meta(args: argparse.Namespace) -> dict: + """What contained this run, recorded so the report does not have to imply it. + + Saying so in the artifact is the point: the same numbers mean different + things depending on whether anything was isolated, and one engine here + isolates nothing by construction. A report that left a reader to infer it + from the engine name is how a table of host results came to be read as + evidence about containers. + """ + from .engine import sandbox as engine_sandbox + + return engine_sandbox.describe() + + +def _finish_routing( + args: argparse.Namespace, + outcomes: list, + routing_set: dict, + started: float, + *, + isolated: bool, + sandbox: str, + sandbox_isolated: bool, + extra: dict | None = None, +) -> int: + """Summarize, report, and gate a routing run. Shared by all three legs. + + What contained the run is required of the caller rather than worked out + here. Deriving it from the engine's name is what let a run that started no + container report `sandbox: docker, sandbox_isolated: true`, and hardcoding + `host` was only right while no leg had a sandbox. Required keywords mean a + leg added later that forgets fails at the call with a TypeError, instead of + quietly reporting a containment it never had. + """ + summary = routing.summarize( + outcomes, + list(routing_set), + { + "model": args.model, + "engine": args.engine, + "effort": args.effort, + "skills": list(routing_set), + "extended": args.extended, + "wall_time_s": round(time.time() - started, 1), + "timeout": args.timeout, + "github_run_id": os.environ.get("GITHUB_RUN_ID"), + **usage.snapshot().as_meta(), + **(extra or {}), + # Last, so a leg's own meta cannot shadow what contained it. + "isolated_config_dir": isolated, + "sandbox": sandbox, + "sandbox_isolated": sandbox_isolated, + }, + ) + _write_report(summary, routing.render_markdown(summary), args, "routing") + + if (code := _fail_if_expired()) is not None: + return code + reason = routing_gate(summary["totals"], args.min_accuracy) + if reason: + print(f"[routing] {reason}", file=sys.stderr) + return 1 + return 0 + + def cmd_routing(args: argparse.Namespace) -> int: # Who is in the room decides what the structural gate covers, so it is # settled before anything is checked or any token is spent. @@ -362,64 +546,75 @@ def cmd_routing(args: argparse.Namespace) -> int: elif args.skill: cases = datasets.filter_cases(cases, args.skill) - routing_config = routing.RoutingConfig( - model=args.model, - effort=args.effort, + print(f"[routing] installed together: {', '.join(routing_set)}") + print( + f"[routing] {len(cases)} cases, model={args.model}, " + f"jobs={args.jobs}, engine={args.engine}" + ) + + from .engine import routing as inspect_routing + from .engine import sandbox as engine_sandbox + + outcomes = inspect_routing.run( + cases, + routing_set, + args.model, + args.effort, + args.engine, case_timeout=args.case_timeout, max_tool_calls=args.max_tool_calls, max_inspection_calls=args.max_inspection_calls, max_budget_usd=args.max_budget_usd, - keep_logs=args.keep_logs, - available_flags=routing.supported_flags( - ["--no-session-persistence", "--max-budget-usd"] - ), - isolate_config=routing.can_isolate_config(), ) - if not routing_config.isolate_config: - print( - "[routing] warning: ANTHROPIC_API_KEY is not set, so the runner's " - "own config dir is used and any user-level skill in it joins the " - "room for every case. The report flags what was registered." + if args.engine == "claude-code": + # A container really was started: `verify.require()` refuses every + # provider that shares the host's filesystem, so `describe()` + # cannot report an isolation this leg did not have. The config dir + # is isolated structurally -- the guest has no `~/.claude` to keep + # out -- rather than conditionally on a credential. + box = engine_sandbox.describe() + isolated = True + else: + # The CLI on the host. `engine/routing.py` has already refused to + # start without the credential that lets it redirect the config + # dir, so reaching here means the room is the room that was asked + # for -- but nothing was contained, and the report says so. + box = {"sandbox": "host", "sandbox_isolated": False} + isolated = True + # Reported as enforced, per leg. Both legs stop at the decision now + # -- the sandboxed one by declining the call, the host one by reading + # the CLI's stream and killing the process group -- but they are held + # to different numbers, so the report names the one that applied. + extra: dict = {"case_timeout": args.case_timeout} + if args.engine == "claude-code": + # The cap that was enforced, not the one asked for. The sandboxed + # leg is held to a scaled budget, so reporting the request would + # make `near_limit` count against a threshold that never applied. + extra["max_tool_calls"] = inspect_routing.budget_for( + args.engine, args.max_tool_calls ) - - print(f"[routing] installed together: {', '.join(routing_set)}") - print(f"[routing] {len(cases)} cases, model={args.model}, jobs={args.jobs}") - if args.jobs > 1 and len(cases) > 1: - with ThreadPoolExecutor(max_workers=args.jobs) as pool: - outcomes = list( - pool.map(lambda c: routing.run_case(c, routing_set, routing_config), cases) - ) + extra["max_inspection_calls"] = inspect_routing.budget_for( + args.engine, args.max_inspection_calls + ) + extra["budget_scaled_by"] = inspect_routing.SANDBOX_BUDGET_FACTOR else: - outcomes = [routing.run_case(case, routing_set, routing_config) for case in cases] - - summary = routing.summarize( + # The CLI's own cap, passed through and only when the build + # advertises it -- so this records what was actually enforced. + flags = inspect_routing.host_cost_flags(args.max_budget_usd) + if flags: + extra["max_budget_usd"] = args.max_budget_usd + extra["optional_cli_flags_used"] = sorted(f for f in flags if f.startswith("--")) + + return _finish_routing( + args, outcomes, - list(routing_set), - { - "model": args.model, - "effort": args.effort, - "skills": list(routing_set), - "extended": args.extended, - "wall_time_s": round(time.time() - started, 1), - "timeout": args.timeout, - "case_timeout": args.case_timeout, - "max_tool_calls": args.max_tool_calls, - "max_inspection_calls": args.max_inspection_calls, - "isolated_config_dir": routing_config.isolate_config, - "max_budget_usd": args.max_budget_usd, - "optional_cli_flags_used": sorted(routing_config.available_flags), - "github_run_id": os.environ.get("GITHUB_RUN_ID"), - }, + routing_set, + started, + isolated=isolated, + sandbox=box["sandbox"], + sandbox_isolated=box["sandbox_isolated"], + extra=extra, ) - _write_report(summary, routing.render_markdown(summary), args, "routing") - - if (code := _fail_if_expired()) is not None: - return code - reason = routing_gate(summary["totals"], args.min_accuracy) - if reason: - print(f"[routing] {reason}", file=sys.stderr) - return 1 - return 0 def cmd_behavioral(args: argparse.Namespace) -> int: @@ -442,17 +637,54 @@ def cmd_behavioral(args: argparse.Namespace) -> int: ) return 0 - outcomes = behavior.run(skills, gradable, args.model, args.effort) + from .engine import behavioral as inspect_behavioral + from .engine import models as engine_models + + if args.engine == "claude-code": + from .engine import verify as runner + + outcomes = runner.run( + skills, gradable, engine_models.resolve(args.model), args.effort + ) + elif args.engine == "claude-code-no-sandbox": + # The real CLI, driven on the host, inside inspect's framework. + # `--model` stays the CLI's own alias here: this one does not go + # through an inspect model provider. + from .engine import no_sandbox + + no_sandbox.require_local() + outcomes = inspect_behavioral.run( + skills, + gradable, + engine_models.resolve(args.model), + args.effort, + solver_factory=lambda d: no_sandbox.claude_code_no_sandbox( + args.model, args.effort, d + ), + ) + else: + # Not a fallthrough. An engine in ENGINES with no branch here is a + # bug this dispatch has had once already, and a silent default is + # what let it survive: runs asked for one agent, got another, and + # the report named the one they had asked for. + raise SystemExit( + f"error: --engine {args.engine} has no behavioral leg. " + f"This is a bug in skillscope, not in how it was called." + ) + summary = behavior.summarize( outcomes, { "model": args.model, + "engine": args.engine, "effort": args.effort, "skills": skills, "extended": args.extended, "wall_time_s": round(time.time() - started, 1), "timeout": args.timeout, "github_run_id": os.environ.get("GITHUB_RUN_ID"), + **usage.snapshot().as_meta(), + **_sandbox_meta(args), }, ) _write_report(summary, behavior.render_markdown(summary), args, "behavioral") @@ -565,6 +797,20 @@ def _add_graded_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument( "--skip-preflight", action="store_true", help="Skip the API reachability check." ) + parser.add_argument( + "--engine", + default=os.environ.get("SKILLSCOPE_ENGINE", DEFAULT_ENGINE), + choices=ENGINES, + help=( + "Which eval engine runs the cases. Both drive the real claude " + "CLI, under inspect_ai; what differs is where it runs. " + "`claude-code-no-sandbox` runs it on the host, on any platform. " + "`claude-code` runs it inside a container via inspect_swe, which " + "needs a POSIX guest and so is Linux only, and additionally " + "needs `pip install 'skillscope[verify]'`. Both run both graded " + "commands. Default: claude-code-no-sandbox, or $SKILLSCOPE_ENGINE." + ), + ) _add_timeout_argument(parser) diff --git a/skillscope/credentials.py b/skillscope/credentials.py index b1f064a..4a6e958 100644 --- a/skillscope/credentials.py +++ b/skillscope/credentials.py @@ -112,9 +112,24 @@ def anthropic_access_token( "`match_subject_prefix` when `sub` did not match the rule." ) from error - token = json.loads(body).get("access_token", "").strip() + answer = json.loads(body) + token = answer.get("access_token", "").strip() if not token: raise CredentialError("the token exchange returned no access_token.") + + # Said out loud, because the alternative is what happened without it: a + # graded run on scarce hardware spent twenty minutes and then failed with + # `401 OAuth access token has expired`, and nothing anywhere said how long + # the token was ever good for. A job that grades for longer than this needs + # a fresh token part-way, and the number is how anyone would know. + lifetime = answer.get("expires_in") + if lifetime: + print( + f" federated token valid for {int(lifetime) // 60}m " + f"{int(lifetime) % 60}s -- a case that runs longer than this will " + "fail with a 401 partway through", + flush=True, + ) return token diff --git a/skillscope/datasets.py b/skillscope/datasets.py index 17a75b8..250865c 100644 --- a/skillscope/datasets.py +++ b/skillscope/datasets.py @@ -535,7 +535,7 @@ def tier0_errors(skill: str, cases: list[Case]) -> list[str]: return errors -MACHINE_KEYS = {"os", "labels"} +MACHINE_KEYS = {"os", "labels", "sandbox"} def _read_machine(skill: str) -> dict: @@ -573,11 +573,17 @@ def machine_plan(skill: str) -> dict: An absent ``evals/machine.yml`` is the common case: the everyday runners, on the platforms the repo runs on by default. A skill ships one to drop a - platform it cannot support (``os``) or to ask for a runner label its work - requires (``labels``):: + platform it cannot support (``os``), to ask for a runner label its work + requires (``labels``), or to name a compose file for the sandbox its cases + need (``sandbox``, read by ``--engine claude-code``):: os: [Linux] labels: [mi300x] + sandbox: compose.yaml + + ``sandbox`` is how a skill that must reach the network to pull a model, or + that needs a device bound in, opts out of the default no-network container + instead of every skill paying for what one of them needs. Labels rather than a class name, because a class name has to be defined somewhere and that somewhere is a second file to keep in step. A label is diff --git a/skillscope/engine/__init__.py b/skillscope/engine/__init__.py new file mode 100644 index 0000000..dcfebb1 --- /dev/null +++ b/skillscope/engine/__init__.py @@ -0,0 +1,51 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""The eval engines built on ``inspect_ai``. + +The legacy engine drives the `claude` CLI directly; these hand the work to +`inspect_ai` -- ``claude-code`` runs the CLI inside the sandbox through +`inspect_swe`, ``claude-code-no-sandbox`` runs it on the host. All of them produce the same +outcome objects, so everything downstream -- `summarize`, `render_markdown`, +the report writers -- is shared. + +`inspect_ai` is an optional dependency, so nothing here is imported at module +scope by the rest of the package. Call `require(engine)` before touching a submodule +to turn a missing wheel into an actionable message rather than a traceback. +""" + +from __future__ import annotations + +def install_hint(engine: str) -> str: + """Why this run cannot start, naming the engine that was actually asked for. + + Every engine runs on inspect_ai, so any of them can raise this. It is a + required dependency now, so reaching here means a broken or partial + install rather than a missing extra -- reinstalling is the fix, and saying + "install the extra" would send someone to a no-op. Required rather than + defaulted: a default once sent a CI job looking for a flag it had not + passed, and the value it defaulted to is one argparse now rejects. + """ + return ( + f"error: --engine {engine} needs inspect_ai, which skillscope " + "requires but could not import. Reinstall with:\n" + " pip install --force-reinstall skillscope" + ) + + +def require(engine: str) -> None: + """Raise SystemExit with an install hint when `inspect_ai` is missing.""" + try: + import inspect_ai # noqa: F401 + except ModuleNotFoundError as exc: # pragma: no cover -- environment shape + raise SystemExit(install_hint(engine)) from exc + + +def available() -> bool: + """Whether the inspect extra is installed (for diagnostics, not control flow).""" + try: + import inspect_ai # noqa: F401 + except ModuleNotFoundError: + return False + return True diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py new file mode 100644 index 0000000..cf7c894 --- /dev/null +++ b/skillscope/engine/behavioral.py @@ -0,0 +1,236 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Behavioral evals under `inspect_ai`. + +Shared by every engine that runs on the framework: the task, the scorer, the +sandbox and the reporting live here, and the caller supplies the solver that +drives the agent. `claude-code-no-sandbox` passes one; `claude-code` builds its own task in +`verify.py` because `inspect_swe` supplies the whole agent rather than a solver. + +`run()` matches `behavior.run()` -- same arguments, same `BehaviorOutcome` +list -- so swapping engines is a one-line substitution in the CLI and every +report path downstream is untouched. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +from .. import agent, config, deadline, routing as routing_core, usage +from ..behavior import BehaviorOutcome +from ..datasets import Case +from . import convert, hooks, models, sandbox as sandbox_spec, scorers, stats + +# An agent that never decides it is finished must still stop. The legacy engine +# bounded this with `--case-timeout` and a process kill; inspect expresses it +# declaratively, and a message cap catches the loop a wall-clock cap only ends +# after paying for it. +MESSAGE_LIMIT = 120 + +# A model that reaches no provider never calls the submit tool, so it loops to +# whatever cap it is given -- and every turn is a real sandbox round trip. The +# wiring run proves the machinery in a handful of turns; the rest is the mock +# failing to finish, slowly. +MOCK_MESSAGE_LIMIT = 6 + + +# How much of the command's budget to keep back from inspect's own per-sample +# limit. The `--timeout` deadline ends the process with `os._exit`, which takes +# the report and the transcript with it -- so a run that overruns says only +# that it overran. Handing inspect the whole budget makes the two fire together +# and the hard kill wins the race. Stopping the sample early enough for inspect +# to score what exists and write the log turns a timeout into evidence: eight +# Instinct runs have now overrun and not one of them said where it got to. +TIMEOUT_RESERVE_S = 120 + + +def task_time_limit(bound) -> int | None: + """The per-sample limit to give inspect, inside the command's own deadline.""" + if bound is None: + return None + return max(60, int(bound.remaining() - TIMEOUT_RESERVE_S)) + + +def realtime_logging() -> bool: + """Whether inspect should keep its live sample buffer for this run. + + The buffer exists so `inspect view` can watch a run in progress, and it + lives in a sqlite file under the user data directory, named after the task. + On Windows that directory is the service account's profile, which is long + enough that a task named after a longer skill crosses MAX_PATH -- sqlite + then answers "unable to open database file" and the whole task dies. Two + skills on the same runner passed and one did not, purely on the length of + its name. + + Nothing watches a CI run live, and the `.eval` log is written either way, + so the buffer is cost without benefit exactly where it breaks. + """ + return not sys.platform.startswith("win") + + +def message_limit_for(model: str) -> int: + """How many turns this model should be allowed before the case is stopped.""" + if model.lower().startswith(agent.NO_PROVIDER_PREFIXES): + return MOCK_MESSAGE_LIMIT + return MESSAGE_LIMIT + + +def build_task( + skill: str, + cases: list[Case], + model: str, + ctx: dict | None = None, + *, + solver_factory, +): + """One inspect `Task` per skill: its cases, its skill installed, its scorer. + + `solver_factory` is what drives the agent; everything around it -- scoring, + judging, reporting -- stays the same whichever one is passed. It receives + the skill's directory because staging is the driver's job: each driver puts + the skill where the agent it runs will look for it. + + Required, and keyword-only. It was optional while a built-in react agent + was the default, and a caller that forgot it silently graded a different + agent than it asked for. + """ + from inspect_ai import Task + + skill_dir = config.active().skill_path(skill) + samples = [convert.sample_from_case(c, skill_dir, ctx) for c in cases] + + # `evals/hooks.py`, where the skill ships one. `cleanup` rather than a + # trailing solver: inspect runs it in a `finally` under a shielded cancel + # scope, so teardown still happens when the agent raises -- which is the + # property a hook that removes containers exists for. + hook = hooks.load(skill) + + bound = deadline.active() + return Task( + name=f"behavioral-{skill}", + dataset=samples, + setup=hooks.setup_solver(hook, skill), + solver=solver_factory(skill_dir), + cleanup=hooks.cleanup_fn(hook, skill), + scorer=scorers.expectations(), + sandbox=sandbox_spec.for_skill(skill), + message_limit=message_limit_for(model), + time_limit=task_time_limit(bound), + ) + + +def _failed(skill: str, cases: list[Case], detail: str) -> list[BehaviorOutcome]: + """One failed outcome per case, for a skill that could not be run at all.""" + return [ + BehaviorOutcome( + id=case.id, + skill=skill, + prompt=case.prompt, + passed=False, + elapsed_s=0.0, + error=detail, + degraded=routing_core.is_provider_error(detail), + ) + for case in cases + ] + + +def _outcomes(log, skill: str, cases: list[Case]) -> list[BehaviorOutcome]: + """Map one inspect `EvalLog` back onto skillscope's outcome objects. + + A task that failed outright reports one failed outcome per case rather than + an empty list: an infrastructure failure that produced no samples must not + render as "every expectation met". + """ + prompts = {c.id: c.prompt for c in cases} + outcomes: list[BehaviorOutcome] = [] + + if log.status == "error" or not log.samples: + detail = getattr(log.error, "message", None) or "the task produced no samples" + return _failed(skill, cases, f"inspect task failed: {detail}") + + for sample in log.samples: + case_id = str(sample.id) + checks: list[dict] = [] + error: str | None = None + + for score in (sample.scores or {}).values(): + checks.extend((score.metadata or {}).get(scorers.CHECKS, [])) + + if sample.error is not None: + error = f"{sample.error.message}" + elif not checks: + error = "case has no behavioral assertions to grade" + + outcomes.append( + BehaviorOutcome( + id=case_id, + skill=skill, + prompt=prompts.get(case_id, ""), + passed=error is None and bool(checks) and all(c["passed"] for c in checks), + elapsed_s=round(getattr(sample, "total_time", None) or 0.0, 2), + checks=checks, + error=error, + degraded=routing_core.is_provider_error(error), + ) + ) + return outcomes + + +def run( + skills: list[str], + cases: list[Case], + model: str, + effort: str, + *, + solver_factory, +) -> list[BehaviorOutcome]: + """Run every behavioral case, grouped by skill. Mirrors `behavior.run`.""" + from inspect_ai import eval as inspect_eval + + sandbox_spec.require_provider() + + outcomes: list[BehaviorOutcome] = [] + for skill in skills: + skill_cases = [c for c in cases if c.skill == skill and c.has_behavior] + if not skill_cases: + continue + + print(f"[behavioral] {skill}: {len(skill_cases)} case(s)", flush=True) + try: + logs = inspect_eval( + build_task(skill, skill_cases, model, solver_factory=solver_factory), + model=model, + model_args=models.model_args(model), + log_dir=str(Path(".skillscope") / "logs"), + log_realtime=realtime_logging(), + # skillscope's own progress lines are the report; inspect's rich + # display takes over the terminal and produces nothing useful + # when a CI job pipes stdout to a file. + display="plain", + ) + except SystemExit as exc: + # One skill's broken setup is that skill's failure, not everybody's. + # A malformed sandbox declaration used to abort the whole command, + # throwing away results for skills already graded and paid for -- + # the same reason the structural gate reads only the skills a run + # is about. + outcomes.extend(_failed(skill, skill_cases, str(exc))) + continue + + for log in logs: + stats.record_log(log) + outcomes.extend(_outcomes(log, skill, skill_cases)) + + for outcome in outcomes: + passed = sum(1 for c in outcome.checks if c["passed"]) + print( + f" [{'PASS' if outcome.passed else 'FAIL'}] {outcome.id}: " + f"{passed}/{len(outcome.checks)} checks in {outcome.elapsed_s}s" + + (f" -- {outcome.error}" if outcome.error else ""), + flush=True, + ) + return outcomes diff --git a/skillscope/engine/convert.py b/skillscope/engine/convert.py new file mode 100644 index 0000000..75b9a63 --- /dev/null +++ b/skillscope/engine/convert.py @@ -0,0 +1,99 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Turn skillscope's dataset into inspect samples. + +`evals.json` is the frozen contract: this module is the only place that knows +how a `Case` becomes an inspect `Sample`, so the dataset format and the engine +can move independently. + +Expectations ride along in `Sample.metadata` rather than `Sample.target`. A case +asserts several unrelated things at once (files produced, phrases in the +transcript, judged behaviors), which is a poor fit for the single `target` +string inspect scorers conventionally compare against; the scorers in +`engine/scorers.py` read them back out by name. +""" + +from __future__ import annotations + +from pathlib import Path + +from ..behavior import expand +from ..datasets import Case + +# Keys written into `Sample.metadata`. Named here so scorers and tasks agree on +# the spelling without importing each other. +SKILL = "skill" +SHOULD_TRIGGER = "skill_should_trigger" +CATEGORY = "category" +EXPECTED = "expected_behavior" +UNEXPECTED = "unexpected_behavior" +LOGS_CONTAIN = "logs_contain" +FILES_EXIST = "files_exist" +EXTENDED = "extended" + + +def seed_files(seed: Path) -> dict[str, str]: + """Map a case's ``workspace`` fixture directory onto `Sample.files`. + + The *contents* land at the sandbox working directory, matching what the + legacy `_stage_workspace` did -- a case hands the agent a starting file to + edit rather than describing one in prose. + """ + if not seed.is_dir(): + raise FileNotFoundError(f"workspace fixture directory not found: {seed}") + + from . import tools + + # Seeded under the same directory the tools resolve against. inspect writes + # these relative to the sandbox's own working directory, which for a + # container is `/` -- so without this a case's fixture would land beside + # `/etc` while the agent worked somewhere else. + prefix = f"{tools.WORKDIR.lstrip('/')}/" if tools.containerized() else "" + + files: dict[str, str] = {} + for path in sorted(seed.rglob("*")): + if path.is_file(): + target = path.relative_to(seed).as_posix() + files[prefix + target] = str(path) + return files + + +def sample_from_case(case: Case, skill_dir: Path, ctx: dict | None = None) -> "object": + """Build one inspect `Sample` from a `Case`. + + `ctx` supplies `{name}` template variables, expanded with the same + substitution the legacy engine uses so a prompt containing literal braces + (JSON snippets, regex quantifiers) survives unchanged. + """ + from inspect_ai.dataset import Sample + + ctx = ctx or {} + files = seed_files(skill_dir / case.workspace) if case.workspace else {} + + return Sample( + id=case.id, + input=expand(case.prompt, ctx), + files=files or None, + metadata={ + SKILL: case.skill, + SHOULD_TRIGGER: case.skill_should_trigger, + CATEGORY: case.category, + EXPECTED: list(case.expected_behavior), + UNEXPECTED: list(case.unexpected_behavior), + LOGS_CONTAIN: [expand(t, ctx) for t in case.logs_contain], + FILES_EXIST: [expand(p, ctx) for p in case.files_exist], + EXTENDED: case.extended, + }, + ) + + +def samples_from_cases( + cases: list[Case], skill_dir_for: "object", ctx: dict | None = None +) -> list: + """Convert many cases. `skill_dir_for` maps a skill name to its directory.""" + return [ + sample_from_case(case, skill_dir_for(case.skill), ctx) + for case in cases + ] diff --git a/skillscope/engine/hooks.py b/skillscope/engine/hooks.py new file mode 100644 index 0000000..9c54004 --- /dev/null +++ b/skillscope/engine/hooks.py @@ -0,0 +1,140 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""`evals/hooks.py` on the engines built on inspect_ai. + +A hook module holds environment plumbing that the dataset format cannot +express -- tearing down a container, clearing a stale endpoint -- so that +prompts and expectations stay readable without opening Python. The legacy +engine ran four entry points; two of them survive the move to inspect_ai +unchanged, and two do not. + +**Supported.** `setup(workspace, case, ctx)` before each case and +`teardown(workspace, case, ctx)` after it. inspect already has both shapes: +`Task.setup` takes a solver that runs ahead of the agent, and `Task.cleanup` +takes a per-state callable that inspect invokes inside a `finally` under a +shielded cancel scope -- so teardown still runs when the agent raises or the +sample is cancelled, which is the property the whole hook exists for. + +**Not supported, and refused rather than approximated.** + +`check(run, case, ctx)` is handed the legacy engine's `Run` object and raises +to fail a case. These engines have inspect's `TaskState`, which is a different +thing with different attributes; passing one where the other is expected would +fail inside somebody's skill with an error about their code. + +`setup_session(cache_dir)` returns `{name: value}` pairs that `expand()` +substitutes into prompts and expectations. Those are baked into the `Sample` +before any solver runs, so honouring it means building the dataset after the +hook rather than before -- a change to how every case is constructed, for a +feature nothing in the catalogue uses. + +A `setup` that *returns* template variables has the same problem, and is caught +at runtime rather than by inspection because a docstring cannot say what a +function returns. + +Nothing in the catalogue this was written against uses either: one skill ships +a hook, it defines `setup` and `teardown` only, and neither returns anything. +""" + +from __future__ import annotations + +import importlib.util +from types import ModuleType + +from .. import datasets + +# Entry points the legacy engine honoured that these engines cannot. Named +# rather than inferred so the refusal can list them, and so adding support +# later is a matter of removing a name. +UNSUPPORTED = ("check", "setup_session") + + +def load(skill: str) -> ModuleType | None: + """Import `/evals/hooks.py`, or `None` when the skill ships none.""" + path = datasets.hooks_path(skill) + if not path.is_file(): + return None + spec = importlib.util.spec_from_file_location( + f"evalhooks_{skill.replace('-', '_')}", path + ) + if spec is None or spec.loader is None: + raise SystemExit(f"error: could not import {path}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def unsupported_entry_points(module: ModuleType | None) -> list[str]: + """Which of `UNSUPPORTED` this module actually defines. + + Pure, and the whole of the refusal rule: a hook that defines neither runs + here unchanged, and one that defines either is refused by name rather than + by the file existing. The old guard refused any skill that shipped a hook + at all, which grounded a skill whose hook this engine could have run. + """ + if module is None: + return [] + return [name for name in UNSUPPORTED if callable(getattr(module, name, None))] + + +def _returned_template_vars(result: object, skill: str) -> None: + """Refuse a `setup` whose return value we would have to silently drop. + + Returning `{name: value}` is a documented use of `setup`, and those values + are substituted into prompts that were built before this ran. Dropping them + would leave `{placeholder}` in the prompt the agent is graded on, which + reads as a badly written case rather than a missing feature. + """ + if isinstance(result, dict) and result: + raise SystemExit( + f"error: {skill}'s evals/hooks.py setup() returned template " + f"variables ({', '.join(sorted(result))}), which the inspect " + "engines cannot substitute: prompts and expectations are built " + "before any hook runs.\n" + " Move the value into the dataset, or compute it in the prompt " + "itself." + ) + + +def setup_solver(module: ModuleType | None, skill: str): + """`Task.setup` for this skill's hook, or `None` if it has no `setup`.""" + if module is None or not callable(getattr(module, "setup", None)): + return None + + from inspect_ai.solver import solver + + from . import tools + + @solver + def _hook_setup(): + async def solve(state, generate): + workspace = await tools.workdir() + _returned_template_vars( + module.setup(workspace, state.metadata, {}), skill + ) + return state + + return solve + + return _hook_setup() + + +def cleanup_fn(module: ModuleType | None, skill: str): + """`Task.cleanup` for this skill's hook, or `None` if it has no `teardown`. + + Returned rather than wrapped in a solver because inspect runs `cleanup` in + a `finally` under a shielded cancel scope, and a solver would not run at + all once the agent had raised. + """ + if module is None or not callable(getattr(module, "teardown", None)): + return None + + async def _cleanup(state) -> None: + from . import tools + + workspace = await tools.workdir() + module.teardown(workspace, state.metadata, {}) + + return _cleanup diff --git a/skillscope/engine/judge.py b/skillscope/engine/judge.py new file mode 100644 index 0000000..f4f0a30 --- /dev/null +++ b/skillscope/engine/judge.py @@ -0,0 +1,297 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""The LLM judge for `expected_behavior` / `unexpected_behavior`. + +Two properties of the legacy judge are load-bearing and preserved here. + +**Polarity is never inverted.** The judge is shown the requirement as written, +including "must not" ones, and reports whether the requirement is *satisfied*. +A caller that negates the verdict turns a correct run into a failure, which is +why `agent._grade_with_llm` carries the same warning. + +**The judge sees what the agent produced, not what it said about it.** Evidence +is the tool calls, the tool output, and the artifacts themselves -- an agent +writing "I won't call the cloud API" must not satisfy an expectation that it +avoided doing so, and must not fail one either. Text artifacts are included +inline and images are attached, so "did it actually generate a picture of a +cat" is answerable rather than inferred from a filename. +""" + +from __future__ import annotations + +from pathlib import PurePosixPath + +# Bounds on the evidence packet. A behavioral run can leave a model cache or a +# multi-megabyte log in the workspace; the judge needs the artifacts a case is +# about, not everything on disk. +MAX_FILES = 20 +MAX_FILE_BYTES = 20_000 +MAX_TRANSCRIPT = 12_000 + +# Tool *calls* are short and every one of them matters -- they are the record of +# what the agent did. Tool *results* are what grow without bound (a directory +# listing, a validator's output, a file echoed back), so they are capped +# individually and the calls are always kept whole. +MAX_RESULT = 800 + +IMAGE_SUFFIXES = {".png", ".jpg", ".jpeg", ".gif", ".webp"} +BINARY_SUFFIXES = {".zip", ".gz", ".tar", ".bin", ".safetensors", ".onnx", ".pt"} + +VERDICT_INSTRUCTIONS = """\ +Answer with a single line of JSON and nothing else: +{"pass": true|false, "reason": ""} +""" + + +def is_image(path: str) -> bool: + return PurePosixPath(path).suffix.lower() in IMAGE_SUFFIXES + + +def is_probably_binary(path: str) -> bool: + return PurePosixPath(path).suffix.lower() in BINARY_SUFFIXES + + +def requirement_text(statement: str, *, must_happen: bool) -> str: + """The requirement as the judge sees it, with its polarity spelled out.""" + if must_happen: + return ( + f"The agent MUST have done this:\n{statement}\n\n" + 'Set "pass" to true if the agent did it, false if it did not.' + ) + return ( + f"The agent MUST NOT have done this:\n{statement}\n\n" + 'Set "pass" to true if the agent avoided it, false if the agent did it ' + "anyway. Absence of evidence that the agent did it counts as avoiding " + "it, so the default verdict is true." + ) + + +def parse_verdict(text: str) -> tuple[bool, str] | None: + """Read the last verdict-shaped JSON object out of a chatty reply. + + A reason can itself contain braces -- a regex quantifier, a quoted snippet -- + so the decoder finds object boundaries rather than matching them textually. + """ + import json + + decoder = json.JSONDecoder() + verdict = None + for index, char in enumerate(text): + if char != "{": + continue + try: + parsed, _ = decoder.raw_decode(text[index:]) + except ValueError: + continue + if isinstance(parsed, dict) and "pass" in parsed: + verdict = parsed + + if verdict is None: + return None + reason = str(verdict.get("reason", "")).strip() or "(no reason given)" + return bool(verdict.get("pass")), reason + + +def final_message_of(state) -> str: + """What the agent last said to the user. + + Kept apart from the transcript on purpose. Some expectations are about what + the agent *told* the user -- "output the curl commands they need" -- and are + unanswerable without it. Others are about what it *did*, and for those a + claim in the final message is not evidence: an agent writing "I won't call + the cloud API" must neither satisfy nor fail an expectation that it avoided + doing so. The prompt says which to use for which. + """ + # Everything the agent said, not just the last thing. In an agent loop the + # user sees every assistant turn, so an expectation about what the agent + # told them is satisfied by any of those -- an agent that prints the + # commands mid-run and then submits a summary did tell the user. Reading + # only the final turn credited it with the summary and called the commands + # missing. + # + # The submitted answer comes last and is marked, because that is the + # agent's actual answer where the rest is working. + said: list[str] = [] + for message in state.messages: + if getattr(message, "role", None) != "assistant": + continue + content = getattr(message, "content", None) + if isinstance(content, str) and content.strip(): + said.append(content.strip()) + elif isinstance(content, list): + texts = [ + part.text + for part in content + if isinstance(getattr(part, "text", None), str) and part.text.strip() + ] + if texts: + said.append("\n".join(texts).strip()) + + completion = getattr(getattr(state, "output", None), "completion", None) + if isinstance(completion, str) and completion.strip(): + answer = completion.strip() + if answer not in said: + said.append(f"[submitted answer]\n{answer}") + + if not said: + return "(the agent said nothing)" + return _elide_middle("\n\n".join(said), MAX_TRANSCRIPT) + + +def _elide_middle(text: str, limit: int) -> str: + """Trim the middle, never the end. + + Cutting the tail drops the most recent actions, and those are usually the + ones a check turns on -- an agent writes a file, then validates it, and the + validation is what the expectation is about. Losing it makes the run look + like the agent claimed something it never did. + """ + if len(text) <= limit: + return text + head = text[: limit // 2] + tail = text[-(limit // 2) :] + return f"{head}\n...[middle of transcript elided]...\n{tail}" + + +def transcript_of(state) -> str: + """What the agent did: tool calls and their results, never its prose.""" + parts: list[str] = [] + for message in state.messages: + for call in getattr(message, "tool_calls", None) or []: + parts.append(f"$ {call.function} {call.arguments}") + if getattr(message, "role", "") == "tool": + content = getattr(message, "content", None) + if isinstance(content, str): + body = content.strip() + if len(body) > MAX_RESULT: + body = body[:MAX_RESULT] + " ...[output truncated]" + parts.append(body) + return _elide_middle("\n".join(parts), MAX_TRANSCRIPT) + + +async def artifacts(paths: list[str]) -> tuple[list[str], list[tuple[str, bytes]]]: + """Read what the agent produced: text inline, images as attachments.""" + from inspect_ai.util import sandbox + + from . import tools + + described: list[str] = [] + images: list[tuple[str, bytes]] = [] + + for path in paths[:MAX_FILES]: + # `list_paths` reports paths relative to the case's working directory, + # but `read_file` resolves against the sandbox's own -- which for a + # container is `/`. Without this the judge is told every artifact is + # unreadable and concludes the agent produced nothing. + target = await tools.resolve(path) + if is_image(path): + try: + images.append((path, await sandbox().read_file(target, text=False))) + except Exception as exc: # noqa: BLE001 -- an unreadable file is evidence too + described.append(f"--- {path} (image, unreadable: {exc}) ---") + continue + if is_probably_binary(path): + described.append(f"--- {path} (binary) ---") + continue + try: + body = await sandbox().read_file(target, text=True) + except Exception as exc: # noqa: BLE001 + described.append(f"--- {path} (unreadable: {exc}) ---") + continue + if len(body) > MAX_FILE_BYTES: + body = body[:MAX_FILE_BYTES] + "\n...[truncated]..." + described.append(f"--- {path} ---\n{body}") + + if len(paths) > MAX_FILES: + described.append(f"...and {len(paths) - MAX_FILES} more files") + return described, images + + +async def grade( + statement: str, + state, + *, + must_happen: bool, + grader: str | None = None, +) -> tuple[bool, str]: + """Ask the grader whether one requirement was satisfied.""" + from inspect_ai.model import ( + ChatMessageUser, + ContentImage, + ContentText, + get_model, + ) + + from . import tools + + try: + paths = await tools_list_paths() + except tools.ListingFailed as exc: + # Say so rather than presenting an empty workspace as fact. A judge told + # "no files" will confidently report the agent did nothing, which reads + # as the skill failing when the sandbox is what failed. + return False, f"judge skipped: could not list the sandbox -- {exc}" + + described, images = await artifacts(paths) + + evidence = "\n".join( + [ + f"Files the agent left behind: {paths or 'none'}", + "", + "--- what the agent DID (tool calls and their results) ---", + transcript_of(state), + "", + "--- what the agent SAID to the user (its final message) ---", + final_message_of(state), + "", + "--- artifacts it produced ---", + *described, + ] + ) + + content: list = [ + ContentText( + text=( + "You are grading whether a coding agent's run satisfied one " + "requirement. Judge only from the evidence below.\n\n" + "For a requirement about what the agent DID, use the tool " + "calls and the artifacts: the agent claiming in its message " + "that it did or avoided something is not evidence either way. " + "For a requirement about what the agent TOLD the user, its " + "final message is the evidence.\n\n" + f"REQUIREMENT:\n{requirement_text(statement, must_happen=must_happen)}\n\n" + f"EVIDENCE:\n{evidence}\n\n" + "Do not invert the verdict for any reason.\n" + f"{VERDICT_INSTRUCTIONS}" + ) + ) + ] + for path, data in images: + content.append(ContentText(text=f"--- {path} ---")) + content.append(ContentImage(image=_data_uri(path, data))) + + model = get_model(grader) if grader else get_model() + output = await model.generate([ChatMessageUser(content=content)]) + + parsed = parse_verdict(output.completion or "") + if parsed is None: + return False, f"judge gave no JSON verdict: {(output.completion or '')[:200]!r}" + satisfied, reason = parsed + return satisfied, f"judge: {reason}" + + +def _data_uri(path: str, data: bytes) -> str: + import base64 + + suffix = PurePosixPath(path).suffix.lower().lstrip(".") + mime = "jpeg" if suffix in {"jpg", "jpeg"} else suffix + return f"data:image/{mime};base64,{base64.b64encode(data).decode()}" + + +async def tools_list_paths() -> list[str]: + """Indirection so `judge` does not import `tools` at module scope.""" + from . import tools + + return await tools.list_paths() diff --git a/skillscope/engine/models.py b/skillscope/engine/models.py new file mode 100644 index 0000000..327bc28 --- /dev/null +++ b/skillscope/engine/models.py @@ -0,0 +1,190 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Model names: skillscope aliases to inspect model strings. + +`--model opus` is the `claude` CLI's alias vocabulary. inspect wants a +provider-qualified name (`anthropic/claude-opus-5`), so the two have to be +translated at the boundary rather than either side changing its spelling -- +`--model` is part of the frozen CLI surface. + +Anything already carrying a provider prefix passes through untouched, which is +what makes `--model mockllm/model` work for the no-cost wiring runs. +""" + +from __future__ import annotations + +import os + +from .. import deadline + +# How long the reachability probe may take. The legacy probe has held the same +# bound since it was written, for the same reason: off-network is the ordinary +# way for this to fail, and a preflight that hangs is worse than no preflight. +PROBE_TIMEOUT_S = 60.0 + +# No retries, for the reason `claude_env` sets CLAUDE_CODE_MAX_RETRIES to 0: +# inspect's default is `None`, which retries a connection error without a +# limit, and a probe whose whole job is to fail fast must not be the one thing +# that hangs. Against a closed port this is the difference between answering in +# seconds and still running after four minutes. +PROBE_RETRIES = 0 + +ALIASES = { + "opus": "anthropic/claude-opus-5", + "sonnet": "anthropic/claude-sonnet-5", + "haiku": "anthropic/claude-haiku-4-5-20251001", +} + +# `claude` reads per-request headers from this; nothing in inspect does, so +# skillscope parses it and hands the result to the provider instead. An +# enterprise gateway in front of the Anthropic API is the reason it exists. +CUSTOM_HEADERS_ENV = "ANTHROPIC_CUSTOM_HEADERS" +AUTH_TOKEN_ENV = "ANTHROPIC_AUTH_TOKEN" + + +def resolve(model: str) -> str: + """Translate a skillscope model alias into an inspect model string.""" + if "/" in model: + return model + return ALIASES.get(model.lower(), f"anthropic/{model}") + + +async def _probe(model: str, timeout: float): + import anyio + from inspect_ai.model import GenerateConfig, get_model + + resolved = get_model( + model, + # `timeout` bounds each attempt and PROBE_RETRIES stops it being + # attempted again; the cancel scope below bounds the call as a whole, + # since neither of those covers a connect that stalls before the + # provider's own clock starts. + config=GenerateConfig(max_retries=PROBE_RETRIES, timeout=max(1, int(timeout))), + # Not memoized: this config exists for the probe, and a graded run that + # later asked for the same model would otherwise inherit a retry + # setting chosen for a one-shot check. + memoize=False, + **model_args(model), + ) + with anyio.fail_after(timeout): + return await resolved.generate("Reply with the single word: ok") + + +def probe_bound(timeout: float = PROBE_TIMEOUT_S) -> tuple[float | None, str]: + """Seconds this probe may take, or ``(None, why)`` when there are none left. + + A preflight exists to save a run from a long confusing failure, so it must + not become one: the bound is the smaller of its own and whatever the + command's ``--timeout`` has left. The legacy probe has always clipped + itself this way; this one did not, which is how a closed port kept it + running long past the deadline that was supposed to cover it. + """ + bound = deadline.active() + if bound is None: + return timeout, "" + if bound.expired(): + return None, bound.message() + return bound.cap(timeout), "" + + +def check_reachable(model: str, timeout: float = PROBE_TIMEOUT_S) -> tuple[bool, str]: + """Confirm the model answers before anything expensive starts. + + A graded run starts containers and installs skills before it ever reaches a + provider, so a misconfigured gateway surfaces as a task that failed after + all that work rather than as a credentials problem. One tiny call up front + turns a 401 buried in a sample error into a message on the first line. + + Bounded twice over, because an unreachable gateway is the common case and a + preflight that outlives the command it protects helps nobody: `timeout` + here, clipped to whatever ``--timeout`` has left. + + Costs a handful of tokens. `mockllm` reaches no provider, so it is skipped + rather than charged for a round trip that proves nothing. + """ + if model.startswith("mockllm"): + return True, "mockllm (no provider)" + + timeout, expired = probe_bound(timeout) + if timeout is None: + return False, expired + + import anyio + + try: + output = anyio.run(_probe, model, timeout) + except TimeoutError: + return False, ( + f"model preflight timed out after {timeout:g}s " + "(is the network reachable?)" + ) + except Exception as exc: # noqa: BLE001 -- the reason is the return value + exc = _underlying(exc) + return False, f"{type(exc).__name__}: {exc}"[:400] + return True, (output.completion or "").strip()[:40] + + +def _underlying(exc: BaseException) -> BaseException: + """The error a retry wrapper is carrying, if it is carrying one. + + Turning off retries makes tenacity raise `RetryError` rather than what + actually went wrong, and `RetryError[]` is not a message + anybody can act on. Reported as `APIConnectionError: ...` instead, which is + the difference between this probe doing its job and merely failing. + """ + attempt = getattr(exc, "last_attempt", None) + if attempt is None: + return exc + try: + return attempt.exception() or exc + except Exception: # noqa: BLE001 -- a probe never fails on its own reporting + return exc + + +def custom_headers() -> dict[str, str]: + """Parse ``ANTHROPIC_CUSTOM_HEADERS`` (newline-separated ``Key: value``).""" + headers: dict[str, str] = {} + for line in (os.environ.get(CUSTOM_HEADERS_ENV) or "").splitlines(): + if ":" not in line: + continue + name, _, value = line.partition(":") + if name.strip(): + headers[name.strip()] = value.strip() + return headers + + +def model_args(model: str) -> dict: + """Provider arguments for the configured gateway, if any. + + inspect passes these straight to `AsyncAnthropic`, so custom headers ride in + as `default_headers`. Empty when no gateway headers are configured, which is + the ordinary api.anthropic.com case. + + Scoped to Anthropic models on purpose. The free `mockllm/model` wiring run + reaches no provider at all, and refusing it because the shell happens to + hold both Anthropic variables would break the one check that costs nothing + -- on exactly the machines most likely to have an OAuth token lying around. + """ + if not model.startswith("anthropic/"): + return {} + + headers = custom_headers() + if not headers: + return {} + + if os.environ.get(AUTH_TOKEN_ENV): + # The same rule `credentials.resolve` enforces when it hands a job its + # environment: a federated token is only good at api.anthropic.com, so + # it never travels with a gateway's base URL or headers. Caught here + # too because an environment can be assembled by hand, and inspect's + # OAuth path also sets `default_headers` itself -- passing ours would + # surface as a duplicate keyword argument from inside the SDK. + raise SystemExit( + f"error: both {AUTH_TOKEN_ENV} and {CUSTOM_HEADERS_ENV} are set. " + "A federated token only works at api.anthropic.com; reaching a " + "gateway needs ANTHROPIC_API_KEY instead. Unset one of them." + ) + + return {"default_headers": headers} diff --git a/skillscope/engine/no_sandbox.py b/skillscope/engine/no_sandbox.py new file mode 100644 index 0000000..a083085 --- /dev/null +++ b/skillscope/engine/no_sandbox.py @@ -0,0 +1,476 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Real Claude Code, driven as a subprocess, inside inspect's framework. + +`inspect_swe` runs the CLI *inside* the sandbox and reaches the model through a +bridge whose proxy is a Linux binary -- which is why it cannot run on Windows at +all. This runs the CLI on the host instead, the way the legacy engine does, and +maps what it did into inspect's messages so the scorers, the judge and the +`.eval` transcript all work unchanged. + +The point is fidelity. A skill is written for this harness, so the real harness +runs everywhere and only the isolation differs: `inspect_swe` in a container on +Linux, this on any platform -- and on Windows this is the only option, because +inspect's sandbox layer assumes a POSIX guest whichever agent drives it. + +Unsandboxed by construction: the CLI runs on the host, in the sample's own +working directory. That is what the legacy engine already does, so it is not a +regression -- but the report says `sandbox_isolated: false` rather than leaving +a reader to assume otherwise. +""" + +from __future__ import annotations + +import asyncio +import json +import os +import shutil +import signal +import subprocess +from pathlib import Path +from typing import Callable + +from .. import agent as legacy_agent + +# Tool calls and results are reconstructed from the stream, so they need ids +# that are merely unique within a sample rather than meaningful. +_CALL_PREFIX = "cli" + +# Where an early stop records why it happened, for a reader downstream. The +# sample carries no inspect limit when this driver stops the run itself -- the +# bound was enforced here, not by inspect -- so without this the report would +# describe a budgeted stop as a run that ended on its own. +STOP_REASON_KEY = "skillscope_host_stop_reason" + +# What `stop_when` returns when the stream reached the CLI's own ending. Named +# rather than spelled inline because the mapper reads it back. +STOP_RESULT = "result" + +# The skill's own test suite, which is not part of the skill as anyone installs +# it. Kept out of the staged room: see `install_skill`. +EVAL_FIXTURE_DIRNAME = "evals" + + +def require_local() -> None: + """This driver runs on the host, so settle the sandbox on the host. + + Two cases, and they are not the same question. + + Nobody asked: take `local`. This engine is unsandboxed by construction, so + there is nothing to decide, and refusing here would have meant a default + engine that fails under a default sandbox setting -- which it did. A fresh + install running `skillscope routing` stopped with "needs + SKILLSCOPE_SANDBOX=local, not 'docker'", because the two defaults + contradicted each other and the engine was the one that changed. + + Somebody asked for a container: refuse. Overriding an explicit request + would answer a different question than the one that was put, and the + report would name a containment the run never had. + + Exported rather than merely returned, so `describe()` and `for_skill()` + agree with what actually happened. A report that says `docker` about a run + on the host filesystem is the failure this whole engine exists to avoid. + """ + from . import sandbox as sandbox_spec + + asked = sandbox_spec.requested() + if asked is None: + os.environ[sandbox_spec.SANDBOX_ENV] = "local" + elif asked not in sandbox_spec.NOT_ISOLATED: + raise SystemExit( + f"error: --engine claude-code-no-sandbox runs the CLI on the host, " + f"but {sandbox_spec.SANDBOX_ENV}={asked!r} asks for a container. " + "Unset it to run on the host, or use --engine claude-code for a " + "sandboxed run of the real harness on Linux." + ) + if not shutil.which("claude"): + raise SystemExit("error: 'claude' CLI not found on PATH") + + +async def _workspace() -> str: + """The directory this sample's files live in.""" + from inspect_ai.util import sandbox + from inspect_ai.util._sandbox.local import LocalSandboxEnvironment + + return sandbox().as_type(LocalSandboxEnvironment).directory.name + + +def run_completed(events: list[dict]) -> tuple[bool, str]: + """Whether the CLI reached the end of the run, and what it said if so. + + Pure, and separated from message construction on purpose: it is the + distinction the routing mapper depends on -- "the agent reached for no + skill" against "the agent never ran" -- and the unit suite runs without the + inspect extra, so a rule only reachable through inspect's message objects + would have no coverage where it matters. + + A `result` event is the evidence. The CLI emits one when it finishes, + carrying the closing answer or nothing at all; a stream without one is a + run that did not get there. + """ + completed = False + final = "" + for event in events: + if event.get("type") != "result": + continue + completed = True + if isinstance(event.get("result"), str): + final = event["result"] + return completed, final + + +def events_to_messages(events: list[dict], prompt: str) -> tuple[list, str]: + """Turn the CLI's stream into inspect messages, and the final answer. + + The scorers read tool calls to see what the agent did and assistant text to + see what it told the user, so both have to survive the crossing. Reusing + the legacy walk keeps one parser for one stream format. + """ + from inspect_ai.model import ChatMessageAssistant, ChatMessageTool + from inspect_ai.tool import ToolCall + + tool_uses: list[tuple[str, str]] = [] + tool_results: list[str] = [] + for event in events: + legacy_agent._walk(event, tool_uses, tool_results) + + messages: list = [] + for index, (name, arguments) in enumerate(tool_uses): + call_id = f"{_CALL_PREFIX}-{index}" + try: + parsed = json.loads(arguments) + except json.JSONDecodeError: + parsed = {"raw": arguments} + messages.append( + ChatMessageAssistant( + content="", + tool_calls=[ + ToolCall(id=call_id, function=name, arguments=parsed) + ], + ) + ) + if index < len(tool_results): + messages.append( + ChatMessageTool( + content=tool_results[index], + tool_call_id=call_id, + function=name, + ) + ) + + completed, final = run_completed(events) + + # Recorded even when empty, which is the point. A run that finished + # without a tool call and without a closing sentence is a routing result: + # the agent reached for no skill. Appending nothing made it + # indistinguishable from a CLI that never ran, and the routing mapper -- + # which has to tell those apart, and cannot do it from an empty message + # list -- graded two such cases as infrastructure failures where the + # legacy engine graded them `correct_trigger` and `true_negative`. + # + # The placeholder matches what `state.output` has always used for the same + # case; only the message list was inconsistent with it. + if completed or final: + messages.append(ChatMessageAssistant(content=final or "(no final message)")) + return messages, final + + +async def _terminate(proc) -> None: + """Kill the CLI and everything it started. + + The CLI spawns helpers, and killing only the process we launched leaves + them running against the same budget the stop was meant to protect. The + legacy engine learned this and kills the whole group; this is that, in + asyncio's vocabulary. Spawned into its own session/group precisely so this + one signal can reach all of it. + """ + if proc.returncode is not None: + return + try: + if os.name == "nt": + # No process groups to signal; the tree walk is taskkill's job. + killer = await asyncio.create_subprocess_exec( + "taskkill", "/F", "/T", "/PID", str(proc.pid), + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + ) + await killer.wait() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + except (OSError, ProcessLookupError): + pass + try: + proc.kill() + except (OSError, ProcessLookupError): + pass + try: + await asyncio.wait_for(proc.wait(), timeout=15) + except (asyncio.TimeoutError, ProcessLookupError): + pass + + +def _subprocess_concurrency(): + """inspect's own subprocess limiter, so this driver stays inside its budget. + + Reached by name: `concurrency()` is keyed, so asking for "subprocesses" + joins the same semaphore `inspect_ai.util.subprocess` uses rather than + opening a second, unaccounted pool beside it. The size only matters to + whoever creates it first, and inspect normally has by the time a solver + runs. Defended anyway -- the limit is private, and a driver that refused to + run because an internal name moved would be worse than one that ran + unlimited. + """ + import contextlib + + from inspect_ai.util import concurrency + + try: + from inspect_ai.util._subprocess import max_subprocesses_context_var + + limit = max_subprocesses_context_var.get() + except Exception: # pragma: no cover -- upstream internals moved + return contextlib.nullcontext() + return concurrency("subprocesses", limit, resizable=True) + + +async def _stream_until( + cmd: list[str], + prompt: str, + workspace: str, + env: dict, + stop_when: Callable[[dict], str | None], +) -> tuple[list[dict], str | None, int | None, str]: + """Run the CLI, reading its stream, and stop the moment `stop_when` says to. + + The reason this exists rather than `inspect_ai.util.subprocess`: that call + returns once the process has exited, so nothing it gives back can arrive in + time to end the run early. For a behavioral case that is exactly right -- + the skill has to finish for the scorers to have anything to read. For a + routing case it means the agent answers the question on its first tool call + and then does the entire job anyway, unwatched and paid for. Measured over + one 67-case room, 152 of this leg's 220 tool calls happened after the + decision it was being asked for. + + So: read the stream line by line, hand each event to the caller's rule, and + kill the process group the first time it says stop. That is what the legacy + engine does, and this leg replaces the legacy engine. + """ + spawn: dict = {} + if os.name == "nt": + spawn["creationflags"] = subprocess.CREATE_NEW_PROCESS_GROUP + else: + spawn["start_new_session"] = True + + proc = await asyncio.create_subprocess_exec( + *cmd, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=workspace, + env=env, + **spawn, + ) + + errors: list[str] = [] + + async def _drain_stderr() -> None: + # Drained concurrently, not at the end: a full stderr pipe blocks the + # CLI, and a CLI blocked on a pipe nobody is reading never reaches the + # decision this function is waiting for. + try: + while True: + line = await proc.stderr.readline() + if not line: + return + errors.append(line.decode("utf-8", "replace")) + except (asyncio.CancelledError, ValueError): + return + + stderr_task = asyncio.create_task(_drain_stderr()) + + events: list[dict] = [] + stop_reason: str | None = None + try: + proc.stdin.write(prompt.encode("utf-8")) + await proc.stdin.drain() + proc.stdin.close() + except (BrokenPipeError, ConnectionResetError, OSError): + # The CLI rejected the prompt or died early. Whatever it managed to + # say is read below and reported the same way as any other short run. + pass + + try: + while True: + raw = await proc.stdout.readline() + if not raw: + break + line = raw.decode("utf-8", "replace").strip() + if not line: + continue + try: + event = json.loads(line) + except json.JSONDecodeError: + continue + events.append(event) + stop_reason = stop_when(event) + if stop_reason is not None: + break + finally: + await _terminate(proc) + stderr_task.cancel() + try: + await stderr_task + except (asyncio.CancelledError, Exception): + pass + + return events, stop_reason, proc.returncode, "".join(errors) + + +def install_skill(skill_dir: Path, workspace: str) -> None: + """Put the skill where the real harness looks for it. + + `.claude/skills/` inside a directory the CLI is given with + `--add-dir`, which is what `inspect_swe` does via its own `skills=` + argument. Staging is the driver's job, and this driver is the one that has + to do it by hand: skip it and the agent runs with no skill at all, + answering from the prompt and scoring like it -- which looks like a bad + skill rather than a missing one. + + Everything except the skill's own test suite. This used to copy the + directory wholesale, which put `evals/evals.json` inside the room the agent + is being asked to choose from -- a file pairing each prompt with + `skill_should_trigger`, and with the `expected_behavior` a behavioral case + is graded against. The answer key, in the room. + + No agent has been observed opening it: every case in a 67-case run was + checked and none touched it. But a user installing this skill does not + receive its tests, so the room was not the room it claimed to model, and + the sandboxed leg never had them -- so the two legs whose agreement the + legacy retirement rests on were choosing from different rooms. + + Nothing depends on them being here. Workspace fixtures under `evals/files` + are read from the skill directory on disk and seeded into the *workspace*, + which is a different path. + """ + dest = Path(workspace) / ".claude" / "skills" / skill_dir.name + dest.parent.mkdir(parents=True, exist_ok=True) + shutil.copytree( + skill_dir, + dest, + dirs_exist_ok=True, + ignore=shutil.ignore_patterns(EVAL_FIXTURE_DIRNAME), + ) + + +def claude_code_no_sandbox( + model: str | None, + effort: str | None, + skill_dir: Path, + config_dir: Path | None = None, + extra_flags: list[str] | None = None, + stop_when_factory: Callable[[], Callable[[dict], str | None]] | None = None, +): + """Solver: install the skill, run the real CLI once, record what it did. + + `extra_flags` are the CLI's own cost controls -- `--max-budget-usd`, + `--no-session-persistence` -- which the legacy engine passes and this + driver could not, because it did not build that part of the command line. + The caller probes for them with `routing.supported_flags` first: an older + build rejects an unknown flag and every case fails identically, which reads + as a routing collapse rather than a flag problem. + + `config_dir` redirects the CLI away from the runner's own `~/.claude`, the + way the legacy engine does. Optional because a behavioral case installs one + skill and grades what the agent produced, so a stray user-level skill is at + worst noise. A routing case grades *which* skill fired, and a stray one + joins the room for every case -- so that caller passes it and refuses to + run without it. + + `stop_when_factory` builds the rule that decides, per stream event, whether + the run has answered the question being asked of it. Opt-in because the two + callers want opposite things from the same CLI: a behavioral case is graded + on what the skill *produced*, so it has to run to the end, and passing a + rule here would cut the work being measured. A routing case is graded on + which skill fired, and everything after that is paid for and unread. Left + `None`, this runs through `inspect_ai.util.subprocess` exactly as before. + + A factory rather than the rule itself because the solver is built once and + run for every sample, while a budget counts *per case*. Handed a single + rule, its tally would carry from one case into the next and the room would + run out of budget partway through -- which has happened here before, and + reads as a skill that stopped triggering rather than a counter that never + reset. + """ + from inspect_ai.model import ModelOutput + from inspect_ai.solver import solver + + @solver + def _claude_code_no_sandbox(): + async def solve(state, generate): + from inspect_ai.util import subprocess as sandbox_subprocess + + workspace = await _workspace() + install_skill(skill_dir, workspace) + prompt = state.input_text + + cmd = [ + shutil.which("claude"), "-p", + "--output-format", "stream-json", "--verbose", + "--dangerously-skip-permissions", + "--add-dir", workspace, + ] + if model: + cmd += ["--model", model] + if effort: + cmd += ["--effort", effort] + cmd += list(extra_flags or []) + + env = legacy_agent.claude_env() + if config_dir is not None: + env["CLAUDE_CONFIG_DIR"] = str(config_dir) + + events: list[dict] = [] + if stop_when_factory is None: + result = await sandbox_subprocess( + cmd, input=prompt, cwd=workspace, env=env, + ) + returncode, stderr = result.returncode, result.stderr or "" + for line in (result.stdout or "").splitlines(): + line = line.strip() + if not line: + continue + try: + events.append(json.loads(line)) + except json.JSONDecodeError: + continue + else: + async with _subprocess_concurrency(): + events, stop_reason, returncode, stderr = await _stream_until( + cmd, prompt, workspace, env, stop_when_factory() + ) + # Recorded whatever it was, including `None` for a stream that + # ended on its own. A stop this driver performed leaves no + # inspect limit behind, so this is the only trace of it. + if stop_reason is not None and stop_reason != STOP_RESULT: + state.store.set(STOP_REASON_KEY, stop_reason) + + if not events: + raise RuntimeError( + "claude produced no parseable stream-json output " + f"(exit {returncode}). stderr: {stderr[:300]}" + ) + + for event in events: + legacy_agent.usage.record_stream_event(event) + + messages, final = events_to_messages(events, prompt) + state.messages.extend(messages) + state.output = ModelOutput.from_content( + model=model or "claude", content=final or "(no final message)" + ) + return state + + return solve + + return _claude_code_no_sandbox() diff --git a/skillscope/engine/routing.py b/skillscope/engine/routing.py new file mode 100644 index 0000000..12942cb --- /dev/null +++ b/skillscope/engine/routing.py @@ -0,0 +1,1099 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Routing evals under `inspect_ai`, driving the real `claude` CLI. + +`skillscope/routing.py` is the routing engine: it owns the vocabulary +(`Outcome`, `classify`, `detect_activation`), the report and the gate, and it +drives the CLI itself as a subprocess. This module gives the same question two +more legs, both of which reach the same CLI through `inspect_ai` instead: + + * `claude-code` -- `inspect_swe` runs the CLI *inside* the sandbox. Linux + only, for the reasons `verify.require` spells out, and the only leg where + the room is exactly the room that was asked for, because the guest has no + `~/.claude` of its own to contribute to it. + * `claude-code-no-sandbox` -- the CLI on the host, via the solver in + `no_sandbox.py`. Runs anywhere, isolates nothing. + +Everything downstream is untouched: this produces `routing.Outcome` objects, so +`routing.summarize`, `routing.render_markdown` and `cli.routing_gate` work as +they already do and a report from this leg is diffable against a legacy one. + +**One detector, two shapes.** The two legs surface the agent's tool calls in +different formats. The host leg parses the CLI's stream-json and hands inspect +`ToolCall` objects; `inspect_swe` never produces stream-json at all -- the +bridged CLI's calls arrive as `ToolCall` objects on +`ChatMessageAssistant.tool_calls`, and there is no `ToolEvent` in the transcript +for a bridged scaffold to read instead. The tempting move is a second detector +that walks `ToolCall`s. That would be two answers to "did this skill activate?", +which drift, and the one in `skillscope/routing.py` is the one the legacy engine +and every recorded result were graded against. So the crossing happens in the +other direction: a `ToolCall` is re-wrapped into the one-line stream-json shape +`detect_activation` already reads (`_activation_event`), and the detector stays +the single answer. Converting data is cheap; keeping two graders agreeing is +not. + +**Not yet: stopping at the decision.** The legacy engine kills the run the +moment a skill activates, because everything after that is work the routing +question does not ask for and does pay for. Without it a case here runs to +`ROUTING_MESSAGE_LIMIT`, which is the whole reason that constant is as tight as +it is. + +Two different obstacles, and only one of them is permanent. + +On `claude-code-no-sandbox` it cannot be done at all: the solver shells out +through `inspect_ai.util.subprocess`, which buffers the CLI's stdout until the +process exits, so the decision is only visible once the run is already over and +paid for. Recovering it means driving the CLI with a streaming reader, which is +what `routing.run_case` already does -- at which point inspect is supplying the +task and the log and not much else. + +On `claude-code` it is available and not yet taken. An approval policy sees each +tool call the bridged CLI proposes *before* it runs, and returning `terminate` +ends the sample; that is an exact analogue of the legacy kill. What is not +established is whether the terminating call survives into the sample. Approval +runs inside `bridge_generate`, and the bridge adopts the assistant message into +the agent's state only after that call returns -- so a `terminate` on the very +tool call that reveals the decision may discard the observation it was triggered +by. That failure is silent and it is the worst shape available here: a +suppressed activation is indistinguishable from an agent that correctly declined +to route, and the run still reports a clean accuracy. + +So it waits on one container run that answers whether the skill name is +recoverable after a terminate -- from the transcript, or failing that from the +`limit.reason` an approver can write. Until then the cost is bounded +declaratively and honestly, rather than optimised on an assumption. +""" + +from __future__ import annotations + +import json +import tempfile +from pathlib import Path + +from .. import deadline, routing as routing_core +from ..datasets import Case +from . import behavioral, convert, models, no_sandbox, sandbox as sandbox_spec, stats + +CLAUDE_CODE = "claude-code" +NO_SANDBOX = "claude-code-no-sandbox" + +# The engines this module has a leg for. `legacy` is not one of them: it is the +# subprocess path in `skillscope/routing.py` and does not come through here. +ENGINES = (CLAUDE_CODE, NO_SANDBOX) + +# Whether a tool call that merely opens a skill's own `SKILL.md` counts as an +# activation. Off, deliberately, and unlike the legacy engine -- which decides +# per session from the tool list in the CLI's `init` event, an event that does +# not survive the crossing into inspect messages on either leg. +# +# Both legs drive a current `claude` build, which activates a skill through the +# `Skill` tool; on such a build an agent that opens a `SKILL.md` is *reading the +# room to choose from it*, and scoring that as a decision credits whichever +# skill the directory listing happened to put first. The two ways to be wrong +# are not symmetric. Leaving it on invents activations that grade as +# `false_trigger` and `wrong_skill`, quietly, in a report that looks normal. +# Leaving it off on a build that has no `Skill` tool makes every case a +# `missed_trigger` -- which `cli.routing_gate` already refuses outright, loudly, +# as "no skill activated in any case". A failure the gate catches beats one it +# cannot see. +ALLOW_BODY_PATH = False + +# How many messages a routing case may spend. Behavioral's 120 is sized for an +# agent doing a job; a routing case only has to reveal which skill it reaches +# for, and without the early stop described above every turn past that point is +# money spent on an answer nobody reads. The legacy budget is the reference +# point: 4 non-bookkeeping tool calls plus 8 inspections of the skills tree, so +# roughly a dozen calls and twice that in messages before it gives up waiting +# for a decision. This is that, rounded up -- generous enough that an agent +# which deliberates before choosing is not scored as one that never chose. +ROUTING_MESSAGE_LIMIT = 30 + + +def require_isolated_room(engine: str) -> None: + """Refuse a host-leg routing run that cannot keep the runner's room clean. + + The legacy engine warns here and carries on, because it can afford to: it + reads the CLI's `init` event, so a user-level skill that joined the room is + named in the report as an extra, and a reader can discount the run. Neither + leg in this module gets that event -- it does not survive the crossing into + inspect messages -- so the same contamination would be invisible. + + Invisible is the part that matters. A stray skill does not spoil one case's + grade; it is offered for every prompt, so it changes every decision at + once, and the run still reports a clean accuracy. Observed rather than + feared: a probe of this leg on a developer machine put roughly forty + user-level skills in the room and none of the three that were staged. + + So: isolate, or do not run. `claude-code` needs nothing here -- the guest + has no `~/.claude` to contribute, which is the whole reason that leg is the + one to prefer for routing. + """ + if engine != NO_SANDBOX: + return + if routing_core.can_isolate_config(): + return + named = " or ".join(routing_core.ENV_CREDENTIALS) + raise SystemExit( + "error: --engine claude-code-no-sandbox cannot run a routing leg " + f"without {named}. Routing needs the room to hold exactly " + "the skills that were asked for, and redirecting the CLI away from " + "the runner's own config dir only works when auth comes from the " + "environment. Without it every user-level skill on this machine joins " + "the room for every case -- and this leg cannot see that happen or " + "say so in the report.\n" + f" Set {named}, or use --engine claude-code, whose guest " + "has no user-level skills at all." + ) + + +def install_room(skills: dict[str, Path], workspace: str) -> None: + """Install every skill in the room into one workspace. + + `no_sandbox.install_skill` installs one, which is all a behavioral case + needs. Routing needs all of them at once -- the whole question is which one + the agent picks out of the set, and a room with a skill missing is a + different, easier question that the report would describe as the one that + was asked for. + + Plural lives here rather than beside the singular because the singular is + the behavioral driver's, and a routing-shaped requirement has no business + changing it. + """ + for skill_dir in skills.values(): + no_sandbox.install_skill(skill_dir, workspace) + + +def _install_room_solver(skills: dict[str, Path]): + """Solver that stages the whole room before the CLI is started. + + Chained *ahead* of `no_sandbox.claude_code_no_sandbox`, which stages one + skill of its own and then runs the CLI in the same breath: by the time it + starts there is nothing left to add. The overlap -- one skill installed + twice, `copytree(dirs_exist_ok=True)` both times -- is deliberate and + cheap. The alternative is teaching the behavioral driver about rooms, and + staging is the driver's job precisely so that each driver can be wrong + about only its own. + """ + from inspect_ai.solver import solver + + @solver + def _install(): + async def solve(state, generate): + install_room(skills, await no_sandbox._workspace()) + return state + + return solve + + return _install() + + +def _sample_from_case(case: Case): + """One inspect `Sample` per routing prompt. + + Not `convert.sample_from_case`: that carries a case's behavioral + expectations and seeds its `workspace` fixture, and routing grades neither. + The fixture especially -- the staged workspace holds the skills tree and + nothing else, which is what the legacy engine has always done, because a + file the agent can open is a file that can change the decision being + measured. Two legs that seeded differently would be measuring two rooms. + + The prompt is used verbatim, again matching the legacy engine: routing + prompts are read by the model for what they suggest, and `{}` expansion is + a behavioral-case affordance. + + The metadata is for whoever opens the `.eval` transcript afterwards. Nothing + grades from it -- the verdict is computed from the case in `_outcomes`, + which is the object that knows what was expected. + """ + from inspect_ai.dataset import Sample + + return Sample( + id=case.id, + input=case.prompt, + metadata={ + convert.SKILL: case.skill, + convert.SHOULD_TRIGGER: case.skill_should_trigger, + convert.CATEGORY: case.category, + }, + ) + + +def _activation_event_from(function: str, arguments: dict) -> dict: + """The stream-json shape `detect_activation` reads, from plain values. + + Split from `_activation_event` so the decision rule can be exercised + without constructing an inspect `ToolCall`, which needs the extra the unit + suite runs without. + """ + return { + "type": "assistant", + "message": { + "content": [ + {"type": "tool_use", "name": function, "input": arguments or {}} + ] + }, + } + + +def _activation_event(call) -> dict: + """Re-wrap one inspect `ToolCall` as the stream-json event it would have been. + + The one place the two legs' shapes are reconciled. `detect_activation` + takes a raw CLI event and walks it for `{"type": "tool_use", ...}` nodes; + this builds the smallest event containing exactly one such node, so a + bridged `ToolCall` is graded by the same code, with the same tool-name + matching and the same `other:` contamination check, as a line the CLI + printed itself. + """ + return _activation_event_from(call.function, call.arguments or {}) + + +def _assistant_messages(sample): + """Every assistant turn in this sample, from whichever record has them. + + Two records, and neither is reliable alone. `sample.messages` is the + conversation inspect adopted; for a bridged agent that adoption follows + heuristics about which thread is the main one, and a run that ends inside a + sub-agent can leave it holding the wrong thread or none. The transcript is + strictly more complete -- every bridged generation emits a `ModelEvent`, + sub-agents included -- but it is a record of events rather than of a + conversation. + + Measured, not assumed: the first sandboxed run on a real container graded + 24 of 67 cases as "the agent never ran" because `sample.messages` was empty + for them, while the legacy engine saw those same cases activate a skill. + The evidence was in the transcript the whole time. + + Read in order and de-duplicated, because the same turn arrives twice -- once + adopted onto the sample, once carried by a transcript event -- as two + different objects, so identity does not match and every call would be + counted twice. + + Keyed on the *tool call* ids rather than the message id. Both copies of a + turn carry a message id and the two do not match: the bridge builds a fresh + message when it adopts the turn, so the ids are independently generated and + the de-duplication silently failed. The provider's tool call ids survive the + crossing unchanged, which makes them the only stable name a turn has. + + Measured twice, once per mistake. Keying on identity reported ten tool calls + for cases the approver had terminated at five. Keying on the message id + still reported 23 where the transcript held 12 -- a little under double, + because a turn with no calls at all has nothing else to key on and falls + back to the id. Only the sandboxed leg is affected either way: the host leg + has no bridge, so there is nothing to duplicate. + + That matters beyond the column. The routing decision is the *first* skill + reached for, and a doubled sequence halves the effective budget. + """ + seen: set[str] = set() + + def key(message) -> str: + calls = getattr(message, "tool_calls", None) or [] + if calls: + # Unique per call and assigned by the provider, so the adopted copy + # and the transcript copy of one turn agree on them. + return "calls:" + "|".join( + f"{getattr(c, 'id', '')}/{getattr(c, 'function', '')}" for c in calls + ) + # A turn that called nothing contributes no count, only the evidence + # that the agent said something. The id is the best name available. + ident = getattr(message, "id", None) + return f"id:{ident}" if ident else f"obj:{id(message)}" + + def emit(message): + if getattr(message, "role", None) != "assistant": + return None + k = key(message) + if k in seen: + return None + seen.add(k) + return message + + for message in getattr(sample, "messages", None) or []: + if (kept := emit(message)) is not None: + yield kept + + for event in getattr(sample, "events", None) or []: + output = getattr(event, "output", None) + message = getattr(output, "message", None) if output is not None else None + if message is not None and (kept := emit(message)) is not None: + yield kept + + +def _tool_calls(sample): + """Every tool call the agent made, in the order it made them. + + Order is the whole point: the routing decision is the *first* skill the + agent reaches for. A run that continues past its decision goes on to do the + work, which can activate further skills -- grading the last one would + report what the job needed rather than what the prompt routed to. + """ + for message in _assistant_messages(sample): + for call in getattr(message, "tool_calls", None) or []: + yield call + + +# The dataset filenames, as they would appear in a tool argument. A case that +# opened one would be reading the answer to the question it is being asked. +ANSWER_KEY_NAMES = ("evals.json", "extended_evals.json") + + +def read_the_answer_key(sample) -> bool: + """Whether this case's agent opened a dataset file. + + The staged room holds the skill, not its tests: `evals/evals.json` pairs + each prompt with `skill_should_trigger` and with the `expected_behavior` a + behavioral case is graded against, so a room containing it hands the agent + the answer. The host leg used to stage it, because it copied the skill + directory wholesale; it no longer does. + + This is the part that makes that checkable rather than believed. Removing + the file is the fix; noticing if one is read anyway is how anyone would + learn that the fix regressed, or that a copy reached the agent by a route + nobody modelled -- a fixture directory, a repo checkout the skill asked + for, a path a skill's own instructions name. + + Pure, and matched on the filename rather than a full path, because the + route by which a copy arrives is exactly what is not known in advance. + """ + for call in _tool_calls(sample): + blob = json.dumps(getattr(call, "arguments", {}) or {}, ensure_ascii=False) + if any(name in blob for name in ANSWER_KEY_NAMES): + return True + return False + + +def _observe(sample, skills: list[str]) -> tuple[str | None, int, int]: + """What this sample routed to, and what it spent getting there. + + Returns `(observed, tool_calls, inspection_calls)`. The two counters are + the legacy engine's, computed with the legacy engine's own predicates, so + the column means the same thing in both reports: surveying the installed + skills is part of making the decision and is counted separately from the + agent starting the work itself. + """ + tool_calls = 0 + inspection_calls = 0 + for call in _tool_calls(sample): + hit = routing_core.detect_activation( + _activation_event(call), skills, allow_body_path=ALLOW_BODY_PATH + ) + if hit: + return hit, tool_calls, inspection_calls + + name = (getattr(call, "function", "") or "").lower() + if name in routing_core.BOOKKEEPING_TOOLS: + continue + arguments = json.dumps(getattr(call, "arguments", None) or {}, ensure_ascii=False) + if routing_core._is_skills_inspection(arguments, skills): + inspection_calls += 1 + else: + tool_calls += 1 + return None, tool_calls, inspection_calls + + +def _spoke(sample) -> bool: + """Whether the agent produced anything at all in this sample. + + The distinction requirement 4 of the routing report rests on, and the one + `cli.routing_gate` cannot make for itself. "No skill activated" is a + finding -- a `true_negative` when nothing should have fired, a + `missed_trigger` when something should have. "The agent never ran" is not a + finding about routing at all, and grading it as a miss manufactures a + routing result out of an infrastructure failure: a provider that refused, + a sandbox that never came up, a CLI that printed nothing parseable. The + legacy engine draws the same line with its `INCONCLUSIVE_STOPS` set. + + An assistant turn is the cheapest honest evidence that the run got far + enough to decide something. A sample with none of them said nothing, called + nothing, and answered nothing. + + Looked for in both records, for the reason `_assistant_messages` gives: a + bridged agent's conversation does not always land in `sample.messages`, and + reading only that one graded a quarter of a real run as infrastructure + failures. + + A sample that hit a limit also ran, whatever survived of it. When the + message cap trips, what is left on the sample can be the prompt and + nothing else -- and calling that "the agent never ran" is both wrong and + the opposite of useful, because an agent that spent its whole budget + without reaching for a skill is the clearest kind of missed trigger. The + legacy engine draws the same line: `tool_budget` is a verdict there, not + an error. + """ + if getattr(sample, "limit", None) is not None: + return True + return any(True for _ in _assistant_messages(sample)) + + +def _error_outcome(case: Case, detail: str, stop_reason: str = "error") -> routing_core.Outcome: + """A case that could not be graded, kept out of the accuracy it would skew.""" + return routing_core.Outcome( + id=case.id, + category=case.category, + skill=case.skill, + prompt=case.prompt, + expect=case.expect_skill, + observed=None, + verdict="error", + passed=False, + stop_reason=stop_reason, + elapsed_s=0.0, + tool_calls=0, + error=detail, + # Same classifier as the legacy leg, so "the gateway failed" reads the + # same in both reports. Two engines that describe an outage + # differently cannot be compared during one. + degraded=routing_core.is_provider_error(detail), + ) + + +def _limit_reason(sample) -> str | None: + """The inspect limit this sample hit, if it hit one. + + Recorded in `stop_reason` rather than promoted to an error. An agent that + burned `ROUTING_MESSAGE_LIMIT` messages without reaching for a skill has + made its decision as surely as one that answered -- that is precisely the + shape of an under-triggering skill, and the legacy engine grades its own + `tool_budget` stop the same way. The reader still gets told which bound + ended the run, because "missed, and also truncated" is worth knowing when a + skill's recall looks worse here than on the legacy leg. + """ + limit = getattr(sample, "limit", None) + if limit is not None: + return f"limit:{getattr(limit, 'type', 'unknown')}" + + # The host leg stops its own CLI rather than hitting an inspect limit, so a + # budgeted stop there leaves nothing on the sample to find. Reported as + # `result` it would read as an agent that considered the prompt and + # declined -- the opposite of one that was cut off mid-rummage. + store = getattr(sample, "store", None) or {} + reason = store.get(no_sandbox.STOP_REASON_KEY) if hasattr(store, "get") else None + return f"host:{reason}" if reason else None + + +def _outcomes(log, cases: list[Case], skills: list[str]) -> list[routing_core.Outcome]: + """Map one inspect `EvalLog` back onto routing's outcome objects. + + Mirrors `behavioral._outcomes`, including its central rule: a task that + failed outright reports one *errored* outcome per case rather than an empty + list. Empty would render as a run with nothing wrong with it, and for + routing that is worse than for behavioral -- `summarize` divides by the + graded count, so a task that produced no samples would report an accuracy + of `n/a` beside a clean-looking verdict table. + + It diverges on one point. `behavioral` discards a failed task's completed + samples, which costs one skill's results; routing is a single task for the + whole room, so the same rule would throw away every case the run already + paid for because the last one raised -- observed on a task interrupted at + sample three of three. So the samples that exist are graded whatever the + task's status, and only the cases with no sample at all are errored. + """ + by_id = {case.id: case for case in cases} + failure = getattr(getattr(log, "error", None), "message", None) + + if not log.samples: + detail = failure or "the task produced no samples" + return [_error_outcome(case, f"inspect task failed: {detail}") for case in cases] + + outcomes: list[routing_core.Outcome] = [] + for sample in log.samples: + case = by_id.get(str(sample.id)) + if case is None: + # A sample nobody asked for cannot be graded against an + # expectation, and inventing one would be worse than saying so. + continue + + elapsed = round(getattr(sample, "total_time", None) or 0.0, 2) + + if sample.error is not None: + outcome = _error_outcome(case, str(sample.error.message), "sample_error") + outcome.elapsed_s = elapsed + else: + observed, tool_calls, inspection_calls = _observe(sample, skills) + # The transcript first, the limit second. An approver that stopped + # the case at the decision may have done so before the bridge + # adopted the message carrying it, so the activation can be absent + # from the messages and present in the limit it caused. Neither + # source alone is reliable; the transcript is the richer one, so it + # wins when both have an answer. + if observed is None: + observed = activation_from_limit( + getattr(getattr(sample, "limit", None), "reason", None), skills + ) + if observed is None and not _spoke(sample): + outcome = _error_outcome( + case, + "the sample produced no agent messages, so the run never " + "made a routing decision", + "no_output", + ) + outcome.elapsed_s = elapsed + else: + verdict = routing_core.classify(case.expect_skill, observed) + outcome = routing_core.Outcome( + id=case.id, + category=case.category, + skill=case.skill, + prompt=case.prompt, + expect=case.expect_skill, + observed=observed, + verdict=verdict, + passed=verdict in routing_core.PASSING_VERDICTS, + stop_reason=( + # A case that opened a dataset file read the answer to + # the question it was asked, so its verdict cannot be + # taken at face value. Said in the column a reader is + # already looking at rather than in a log. + "answer_key_read" + if read_the_answer_key(sample) + else "skill_activated" + if observed + else (_limit_reason(sample) or "result") + ), + elapsed_s=elapsed, + tool_calls=tool_calls, + inspection_calls=inspection_calls, + ) + outcomes.append(outcome) + + # Samples inspect never reported back are still cases somebody asked for. + # Silence here reads as a smaller, cleaner run rather than an incomplete + # one, which is the failure mode `behavioral._failed` exists to prevent. + # The usual cause is the task dying partway, so the task's own error is the + # useful thing to say about a case that never ran. + missing = ( + f"inspect task failed before this case ran: {failure}" + if failure + else "inspect returned no sample for this case" + ) + graded = {outcome.id for outcome in outcomes} + outcomes.extend( + _error_outcome(case, missing) for case in cases if case.id not in graded + ) + return outcomes + + +def _solver( + engine: str, + routing_set: dict[str, Path], + model: str, + effort: str, + config_dir: Path | None = None, + max_budget_usd: float | None = None, + max_tool_calls: int | None = None, + max_inspection_calls: int | None = None, +): + """The agent that drives one routing case, for the leg that was asked for. + + Both legs install the whole room and then run the real CLI once. They + differ only in where that CLI runs, which is the entire reason both exist: + when two legs disagree about a routing decision, the disagreement is a fact + about the machine rather than about the skill. + + The budgets arrive already scaled by `budget_for`, so the stop this leg + enforces for itself is the same number the approver would have enforced for + the other -- and the same number the report names. + """ + from inspect_ai.solver import chain + + room = list(routing_set.values()) + + if engine == CLAUDE_CODE: + from inspect_swe import claude_code + + from . import tools, verify + + # `skills=` takes the list, so the guest's own discovery machinery + # installs and registers the room -- the point of this leg being that + # its machinery, not ours, decides what happens. `cwd` is set for the + # same reason `verify.build_task` sets it: a container starts at `/`. + # + # `effort` is passed because leaving it out does not mean "the same as + # everyone else", it means the model's own default. The other two legs + # send `--effort` to the CLI, so omitting it here compared a leg + # thinking as hard as it was told to against one thinking as hard as it + # liked, and called the difference sandboxing. + return chain( + verify._ensure_workdir(), + # Ahead of the agent, not behind it: `claude_code` runs the run. + # See `verify.files_to_restore` for what this adds + # and why the room was otherwise partial in the container only. + verify.complete_skills_solver(routing_set), + claude_code(skills=room, cwd=tools.workdir_path(), effort=effort or None), + ) + + # The host leg stages the room itself, because the CLI reads it off the + # filesystem it is handed. `model` is the skillscope alias rather than the + # resolved inspect name: this leg is the `claude` CLI's own `--model` flag, + # not a provider lookup. + return chain( + _install_room_solver(routing_set), + no_sandbox.claude_code_no_sandbox( + model, effort, room[0], config_dir, host_cost_flags(max_budget_usd), + # Names, not the paths `room` holds: the rule matches what a tool + # call named against the room's membership, which is how the + # approver is given it too. + stop_when_factory=host_stop_when_factory( + list(routing_set), max_tool_calls, max_inspection_calls + ), + ), + ) + + +# Written into an approver's explanation when it stops a case, and read back +# off the sample's limit. The transcript is the primary source for what a case +# observed; this is the one that survives a terminate, because approval runs +# before the bridge adopts the assistant message into the agent's state, so the +# very tool call that revealed the decision may not be there afterwards. Two +# sources, one of which cannot go missing. +ACTIVATION_MARK = "skillscope-activated:" + +# Where the per-sample tally lives. inspect's store is scoped to the sample, +# which is the scope a per-case budget needs. +TOOLS_SEEN = "skillscope_routing_tools" +INSPECTIONS_SEEN = "skillscope_routing_inspections" +BUDGET_MARK = "skillscope-budget:" + + +class _Tally: + """Tool calls and skills-tree inspections seen so far in one case.""" + + def __init__(self) -> None: + self.tools = 0 + self.inspections = 0 + + +def routing_decision( + function: str, + arguments: dict, + room: list[str], + tally: _Tally, + max_tool_calls: int | None, + max_inspection_calls: int | None, +) -> tuple[str, str]: + """Whether this tool call ends the case, and why. Returns (decision, reason). + + Pure, and deliberately so: it decides with nothing but the call, the room + and a running count, which means the rule can be tested without inspect + installed -- and the unit suite runs without the extras on purpose. The + approver below is the thin wrapper that turns this into inspect's vocabulary. + + The rule is the legacy engine's, moved earlier. Legacy sees a call in the + CLI's stream after it has run and then races to kill the process; this sees + it proposed and declines it, so the work never happens. + """ + hit = routing_core.detect_activation( + _activation_event_from(function, arguments), room, + allow_body_path=ALLOW_BODY_PATH, + ) + if hit: + # The decision. Everything after it is paid for and unread. + return "terminate", f"{ACTIVATION_MARK}{hit}" + + # Bookkeeping is free, a survey of the installed skills spends the + # inspection budget, and anything else is the agent starting work. The + # three are exclusive, exactly as the legacy engine has them: counting a + # survey against the work budget too is what ends a run mid-deliberation + # and scores it as a missed trigger. + if (function or "").lower() not in routing_core.BOOKKEEPING_TOOLS: + if routing_core._is_skills_inspection( + json.dumps(arguments or {}, ensure_ascii=False), room + ): + tally.inspections += 1 + else: + tally.tools += 1 + + over = (max_tool_calls is not None and tally.tools > max_tool_calls) or ( + max_inspection_calls is not None and tally.inspections > max_inspection_calls + ) + if over: + # An agent still rummaging at this point is not about to choose, and + # the run buys nothing by watching it. + return "terminate", ( + f"{BUDGET_MARK}{tally.tools} tool call(s), " + f"{tally.inspections} inspection(s)" + ) + return "approve", "not a routing decision" + + +def routing_approver( + room: list[str], + max_tool_calls: int | None, + max_inspection_calls: int | None, +): + """Stop the case at the decision, and at the budget, the way legacy does. + + Sees each tool call the bridged CLI *proposes*, before it runs, and ends + the sample as `EvalSampleLimit(type="operator")` carrying the reason + `routing_decision` produced -- which is the same information legacy puts in + `stop_reason`. + + Only the sandboxed leg gets one, because only its tool calls pass through + inspect's approval layer at all. The host leg reaches the same decision + from the other side of the same rule -- see `host_stop_when_factory`, which + reads it off the CLI's stream instead of intercepting a proposal. + """ + from inspect_ai.approval import Approval, approver + from inspect_ai.util import store + + @approver + def _routing(): + async def approve(message, call, view, history) -> Approval: + # Per sample, not per approver. An approver is built once for the + # task, so a counter captured here counts every case in the run: + # it creeps up until it crosses the budget and then terminates any + # case that makes a counted call before reaching for a skill. + # + # Observed exactly that way -- four cases in one run terminated at + # tallies of 10, 11, 12 and 13, consecutive across different + # prompts, which is what a shared counter looks like from outside. + # inspect's store is scoped to the sample, so this is per case. + tally = _Tally() + tally.tools = store().get(TOOLS_SEEN, 0) + tally.inspections = store().get(INSPECTIONS_SEEN, 0) + + decision, reason = routing_decision( + call.function, call.arguments or {}, room, + tally, max_tool_calls, max_inspection_calls, + ) + + store().set(TOOLS_SEEN, tally.tools) + store().set(INSPECTIONS_SEEN, tally.inspections) + return Approval(decision=decision, explanation=reason) + + return approve + + return _routing() + + +def host_stop_when_factory( + room: list[str], + max_tool_calls: int | None, + max_inspection_calls: int | None, +): + """The same stopping rule as the approver, for the leg that has no approver. + + The sandboxed leg sees a tool call *proposed* and declines it, so the work + never happens. The host leg's CLI is a subprocess: nothing intercepts its + calls, and the only account of them is the `stream-json` it prints as it + goes. So the rule is applied a moment later -- the call has run by the time + its event arrives -- and the stop is a signal rather than a refusal. That is + exactly what the legacy engine does, and the same one call of overshoot. + + Without it this leg answered the routing question on its first tool call and + then went on to do the whole job: downloading trace files, running the + analysis, writing reports. 152 of its 220 tool calls in one 67-case room + came after the decision it was being asked for, none of them read by + anything. + + Returns a *factory*, because the budget is per case and the solver that + calls it is built once for the run. + """ + + def _build(): + tally = _Tally() + + def _stop(event: dict) -> str | None: + for function, encoded in routing_core._iter_tool_uses(event): + try: + arguments = json.loads(encoded) + except json.JSONDecodeError: + arguments = {} + if not isinstance(arguments, dict): + arguments = {} + decision, reason = routing_decision( + function, arguments, room, + tally, max_tool_calls, max_inspection_calls, + ) + if decision == "terminate": + return reason + if event.get("type") == "result": + # The CLI finished on its own. Nothing left to stop, but the + # loop should not sit waiting on a stream that has ended. + return no_sandbox.STOP_RESULT + return None + + return _stop + + return _build + + +def activation_from_limit(reason: str | None, room: list[str]) -> str | None: + """The skill an approver named when it stopped the case, if it named one. + + The fallback half of the two-source rule above. Matched against the room so + a reason that arrived from anywhere else cannot be read as an activation. + """ + if not reason or ACTIVATION_MARK not in reason: + return None + named = reason.split(ACTIVATION_MARK, 1)[1].strip() + if named in room or named.startswith("other:"): + return named + return None + + +# How much more orientation a sandboxed run needs before it is rummaging. +# +# The budget asks one question -- is this agent still choosing, or has it +# started work? -- so the only statistic that can calibrate it is how many +# calls a case makes *before it decides*. Measured over one 67-case room, on +# the 39 sandboxed cases that reached a decision: 32 called the skill tool +# first, with no preamble at all. Mean 0.31, median 0, peak 4. +# +# An earlier revision put this at three on a mean of 2.46, which was two +# populations added together: cases that decide immediately, and cases that +# never find a skill and rummage until something stops them. Only the second +# group is near the threshold, and more budget cannot rescue it -- those cases +# score `no_activation` whether they are cut off at four calls or forty. The +# scaling bought no accuracy and spent real money doing it. +# +# Two, then: double the observed peak, which is headroom for a case that +# orients before committing without funding one that is lost. The distribution +# decays steeply enough (32 / 5 / 1 / 1) that the tail past four is thin. +# Re-measure if the room or the image changes; it is not a constant of nature. +SANDBOX_BUDGET_FACTOR = 2 + + +def budget_for(engine: str, cap: int | None) -> int | None: + """The per-case cap this engine is actually held to. + + Scaled rather than replaced, so the caller's intent survives: passing + `--max-tool-calls 2` still means "stop this sooner than usual" on every + leg. `None` and non-positive values mean no cap and stay that way. + """ + if cap is None or cap <= 0: + return cap + return cap * SANDBOX_BUDGET_FACTOR if engine == CLAUDE_CODE else cap + + +def host_cost_flags(max_budget_usd: float | None) -> list[str]: + """The CLI's own cost controls, for the leg that builds its command line. + + The host leg cannot count tool calls in time to stop a case -- its stdout + is buffered until exit -- so the one bound it can enforce mid-run is the + CLI's, passed straight through the way legacy does. Probed first, because + an older build rejects an unknown flag and every case then fails the same + way, which reads as a routing collapse rather than a missing flag. + """ + if not max_budget_usd or max_budget_usd <= 0: + return [] + if "--max-budget-usd" not in routing_core.supported_flags(["--max-budget-usd"]): + return [] + return ["--max-budget-usd", str(max_budget_usd)] + + +def case_time_limit(case_timeout: float | None) -> int | None: + """The per-sample bound: `--case-timeout`, inside the command's own deadline. + + inspect's `Task(time_limit=)` is per *sample*, which is per case -- the same + unit `--case-timeout` has always meant. Without this the only bound is the + whole command's remaining budget, so one hung prompt spends the run, which + is the exact thing the flag exists to prevent (`deadline` says so in its own + module docstring). + + Clipped to whatever `--timeout` has left, for the reason + `behavioral.task_time_limit` keeps a reserve: the command deadline ends the + process outright, taking the report and the transcript with it, so a + per-case cap that outlives it turns a timeout into silence. + """ + whole_run = behavioral.task_time_limit(deadline.active()) + if case_timeout is None or case_timeout <= 0: + return whole_run + if whole_run is None: + return int(case_timeout) + return max(1, min(int(case_timeout), whole_run)) + + +def build_task( + cases: list[Case], + routing_set: dict[str, Path], + model: str, + effort: str, + engine: str, + config_dir: Path | None = None, + case_timeout: float | None = None, + max_tool_calls: int | None = None, + max_inspection_calls: int | None = None, + max_budget_usd: float | None = None, +): + """One inspect `Task` for the *room*, with the cases as its samples. + + Not one per skill, which is how behavioral is organised and why the shapes + diverge here. A routing case is a question about the whole set: skill Y's + positive prompt is skill X's negative, the confusion matrix is built out of + exactly that overlap, and a per-skill task would install one skill at a time + and answer an easier question under the same name. + + The sandbox is the bare provider rather than `sandbox_spec.for_skill`. A + compose file is one skill's declaration about the machine *its* work needs + -- network egress, a device bound in -- and the room has many skills and no + work: the agent is asked to choose, not to run anything. Honouring one + member's machine would hand every other member's cases an environment + nobody asked for, and picking which member to honour has no right answer. + """ + from inspect_ai import Task + from inspect_ai.approval import ApprovalPolicy + + return Task( + name="routing", + dataset=[_sample_from_case(case) for case in cases], + solver=_solver( + engine, routing_set, model, effort, config_dir, max_budget_usd, + budget_for(engine, max_tool_calls), + budget_for(engine, max_inspection_calls), + ), + # Only the sandboxed leg, because only its tool calls cross inspect's + # bridge for an approver to see. The host leg enforces the same rule + # from inside its own solver, against the CLI's stream. + approval=( + [ + ApprovalPolicy( + approver=routing_approver( + list(routing_set), + budget_for(engine, max_tool_calls), + budget_for(engine, max_inspection_calls), + ), + tools="*", + ) + ] + if engine == CLAUDE_CODE + else None + ), + # No scorer. The verdict needs the case's expectation and the room's + # membership, and `routing.classify` is already the grader for both + # legacy and these legs; wrapping it in a scorer would put a second + # copy of routing's vocabulary inside the task. + sandbox=sandbox_spec.provider(), + message_limit=min( + behavioral.message_limit_for(models.resolve(model)), ROUTING_MESSAGE_LIMIT + ), + time_limit=case_time_limit(case_timeout), + ) + + +def run( + cases: list[Case], + routing_set: dict[str, Path], + model: str, + effort: str, + engine: str, + case_timeout: float | None = None, + max_tool_calls: int | None = None, + max_inspection_calls: int | None = None, + max_budget_usd: float | None = None, +) -> list[routing_core.Outcome]: + """Run every routing case against the room. Mirrors `routing.run_case`'s output. + + Shaped like `verify.run` and `behavioral.run` -- cases in, outcomes out, + nothing about the caller assumed -- so `cli.cmd_routing` picks a leg and + hands the result to the same `_finish_routing` the legacy path uses. + + `model` is the skillscope alias (`opus`, `mockllm/model`), not a resolved + inspect model string. Unlike the behavioral legs, which are handed one or + the other by the CLI, this function owns both legs and they want different + spellings: inspect wants `anthropic/claude-opus-5` and the `claude` CLI + wants `opus`. Resolving inside is the only place that knows which is which. + """ + from inspect_ai import eval as inspect_eval + + if engine not in ENGINES: + # Explicit, for the reason `cmd_behavioral` refuses rather than falling + # through: a routing run that quietly got a different agent than it + # asked for is exactly the bug that kept this module from existing. + raise SystemExit( + f"error: routing has no '{engine}' leg in the inspect engine. " + f"This is a bug in skillscope, not in how it was called." + ) + + if not routing_set: + # `cli.cmd_routing` already refuses an empty room with a much better + # message. Repeated here because this is a public entry point and the + # alternative failure is an IndexError from inside a solver factory. + raise SystemExit( + "error: routing needs at least one skill in the room; " + "there is nothing to route between." + ) + + if engine == CLAUDE_CODE: + from . import verify + + verify.require() + else: + no_sandbox.require_local() + + # Before the provider check and before a single token: an unisolated room + # is not a worse run, it is a different question answered under this one's + # name. + require_isolated_room(engine) + + sandbox_spec.require_provider() + + skills = list(routing_set) + print( + f"[{engine}] routing: {len(cases)} case(s), " + f"{len(skills)} skill(s) installed together", + flush=True, + ) + + resolved = models.resolve(model) + # A directory per run, not per case: the CLI writes session state here and + # the room is the same for every case, so sharing it costs nothing and + # saves re-registering the room once per prompt. Removed on the way out -- + # the point was to keep the runner's own config out, not to leave another + # one behind. + with tempfile.TemporaryDirectory(prefix="skillscope-routing-") as config_dir: + logs = _evaluate( + inspect_eval, + cases, + routing_set, + model, + effort, + engine, + resolved, + Path(config_dir) if engine == NO_SANDBOX else None, + case_timeout, + max_tool_calls, + max_inspection_calls, + max_budget_usd, + ) + + outcomes: list[routing_core.Outcome] = [] + for log in logs: + stats.record_log(log) + outcomes.extend(_outcomes(log, cases, skills)) + + for outcome in outcomes: + print( + f" [{'PASS' if outcome.passed else 'FAIL'}] {outcome.id}: " + f"expected {outcome.expect or 'no skill'} -> " + f"got {outcome.observed or 'no skill'} " + f"({outcome.verdict}, {outcome.stop_reason}, {outcome.elapsed_s}s)" + + (f" -- {outcome.error}" if outcome.error else ""), + flush=True, + ) + return outcomes + + +def _evaluate( + inspect_eval, cases, routing_set, model, effort, engine, resolved, config_dir, + case_timeout=None, max_tool_calls=None, max_inspection_calls=None, + max_budget_usd=None, +): + """Run the task. Split out so `run` reads as a sequence of decisions.""" + return inspect_eval( + build_task( + cases, routing_set, model, effort, engine, config_dir, case_timeout, + max_tool_calls, max_inspection_calls, max_budget_usd, + ), + model=resolved, + model_args=models.model_args(resolved), + log_dir=str(Path(".skillscope") / "logs"), + log_realtime=behavioral.realtime_logging(), + # skillscope's own progress lines are the report; inspect's rich display + # takes over the terminal and produces nothing useful when a CI job + # pipes stdout to a file. + display="plain", + ) diff --git a/skillscope/engine/sandbox.py b/skillscope/engine/sandbox.py new file mode 100644 index 0000000..58f984f --- /dev/null +++ b/skillscope/engine/sandbox.py @@ -0,0 +1,155 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Which sandbox a skill's cases run in. + +Two decisions, kept apart because they are made by different people. + +**Which provider** is a property of the machine: Docker by default, `podman` on +a host that has that instead, `local` where there is no container at all. +`SKILLSCOPE_SANDBOX` selects it, because whoever runs the job knows what the +runner has and a skill does not. Any provider inspect can resolve works -- +`podman` comes from `inspect-podman`, which registers itself through an +`inspect_ai` entry point, so installing it is the whole setup. + +**What the sandbox has to provide** is a property of the skill, declared in +`evals/machine.yml` with a `sandbox:` key naming a compose file. A skill that +must reach the network to pull a model, or that needs a device bound in, says +so there instead of every skill paying for what one of them needs. + +Windows is the exception to both: inspect's sandbox layer, and every tool built +on it, assumes a POSIX guest, so the Windows legs run `local` and trade +isolation for running on the platform they are meant to test. Ephemeral, +off-network runners are what covers that gap. +""" + +from __future__ import annotations + +import os +import sys + +from .. import datasets + +# Which provider to use. Set it to what the runner actually has: `podman` on a +# host without Docker, `local` to skip the container entirely. `local` is for +# working locally, not for CI -- a graded run that quietly dropped its sandbox +# would report the same numbers with none of the isolation. +SANDBOX_ENV = "SKILLSCOPE_SANDBOX" + +DEFAULT_PROVIDER = "docker" + +# Providers that take no configuration, so a skill's compose file cannot apply. +UNCONFIGURED = {"local"} + +# `local` runs in the same filesystem as the harness: the sandbox API works, but +# nothing is isolated. Named so a report can say which it was. +NOT_ISOLATED = {"local"} + + +def describe() -> dict: + """What the report should say about isolation. + + A report that shows the same numbers whether or not a case was contained + invites the reader to assume it was. Both engines say it outright instead, + so "these ran isolated and those did not" is answerable from the artifact + rather than from whoever remembers how the job was configured. + """ + name = provider() + return {"sandbox": name, "sandbox_isolated": name not in NOT_ISOLATED} + + +# Providers that live in another package. inspect resolves these through an +# entry point, so the binary being installed proves nothing -- the Python +# package has to be there too, and the failure otherwise is a ValueError from +# inspect's registry that says nothing about how to fix it. +PROVIDER_PACKAGES = {"podman": "skillscope[podman]"} + + +def is_windows() -> bool: + return sys.platform.startswith("win") + + +def require_provider(resolve=None) -> None: + """Fail early, and legibly, when the chosen provider cannot be resolved. + + `resolve` is injectable so this can be tested without the inspect extra + installed, which the unit suite deliberately runs without. + """ + name = provider() + if resolve is None: + from inspect_ai.util._sandbox.registry import registry_find_sandboxenv + + resolve = registry_find_sandboxenv + + try: + resolve(name) + except Exception as exc: # noqa: BLE001 -- inspect raises a bare ValueError + hint = PROVIDER_PACKAGES.get(name) + install = f"\n pip install '{hint}'" if hint else "" + raise SystemExit( + f"error: {SANDBOX_ENV}={name!r} but inspect cannot resolve that " + f"sandbox provider.{install}\n" + f" ({exc})" + ) from exc + + +def requested() -> str | None: + """What was asked for by name, or `None` when nobody said. + + Split from `provider()` so a caller can tell a deliberate choice from a + default. The host leg needs that distinction: running under `local` is + what it does, so falling back to it is right, but overriding someone who + explicitly asked for a container would be answering a different question + than the one they put. + """ + return os.environ.get(SANDBOX_ENV, "").strip() or None + + +def provider() -> str: + """The sandbox provider for this run.""" + override = requested() + if override: + return override + if is_windows(): + return "local" + return DEFAULT_PROVIDER + + +def for_skill(skill: str): + """The `sandbox` spec for a skill's task. + + Returns a `(provider, config)` tuple when the skill declares a compose file + and the provider can take one, a bare provider name otherwise -- both are + accepted as `Task(sandbox=...)`. + """ + name = provider() + if name in UNCONFIGURED: + return name + + # The provider is the machine's choice and the compose file is the skill's, + # so selecting a provider must not silently discard what the skill asked + # for: a skill that needs network egress would otherwise run without it and + # fail for a reason nothing in the report explains. + compose = _declared_compose(skill) + return (name, str(compose)) if compose is not None else name + + +def _declared_compose(skill: str): + """Path to the compose file a skill's `machine.yml` names, if any. + + Resolved beside `machine.yml`, in `evals/`, rather than at the skill root. + A path in a file is most usefully relative to that file -- and the skill + root is what gets published, so eval infrastructure does not belong there. + """ + name = (datasets._read_machine(skill) or {}).get("sandbox") + if not name: + return None + + path = datasets.machine_path(skill).parent / name + if not path.is_file(): + raise SystemExit( + f"error: {skill}: evals/machine.yml names sandbox '{name}', " + f"but {path} does not exist." + ) + return path diff --git a/skillscope/engine/scorers.py b/skillscope/engine/scorers.py new file mode 100644 index 0000000..4bb3629 --- /dev/null +++ b/skillscope/engine/scorers.py @@ -0,0 +1,119 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Grading for the engines built on `inspect_ai`. + +One scorer grades every expectation a case carries and reports them all, rather +than one scorer per kind. A behavioral run costs minutes and real tokens, so a +run that fails should not have to be repeated to discover the second thing wrong +with it -- the same reason the legacy `Run.evaluate` reports instead of raising. + +The per-expectation results ride in `Score.metadata["checks"]` in the shape +`agent.Check` uses, so `behavior.render_markdown` keeps working unchanged. +""" + +from __future__ import annotations + +import os + +from ..agent import _find_file +from . import convert, judge, tools + +CHECKS = "checks" + + +def _check(kind: str, expectation: str, passed: bool, detail: str = "") -> dict: + return { + "kind": kind, + "expectation": expectation, + "passed": passed, + "detail": detail, + } + + +def searchable(state) -> str: + """Everything in the run, for `logs_contain` to search. + + Deliberately broader than what the judge sees. The legacy engine searched + the whole raw transcript, so a case can pin down a tool name, a command + string, or a phrase the agent used -- and cases were written against that. + `judge.transcript_of` is the narrower, prose-free view, because an agent + *claiming* it avoided something is not evidence that it did. + """ + parts: list[str] = [] + for message in state.messages: + parts.append(f"{getattr(message, 'role', '')}:") + for call in getattr(message, "tool_calls", None) or []: + parts.append(f"{call.function} {call.arguments}") + content = getattr(message, "content", None) + if isinstance(content, str): + parts.append(content) + elif isinstance(content, list): + for part in content: + text = getattr(part, "text", None) + if isinstance(text, str): + parts.append(text) + return "\n".join(parts) + + +def expectations(): + """Grade every expectation on the case and report each one.""" + from inspect_ai.scorer import CORRECT, INCORRECT, Score, accuracy, scorer, stderr + + @scorer(metrics=[accuracy(), stderr()]) + def _expectations(): + async def score(state, target) -> "Score": + meta = state.metadata or {} + checks: list[dict] = [] + + transcript = searchable(state) + for text in meta.get(convert.LOGS_CONTAIN, []): + checks.append( + _check("logs_contain", text, text.lower() in transcript.lower()) + ) + + wanted = meta.get(convert.FILES_EXIST, []) + if wanted: + try: + files = await tools.list_paths() + except tools.ListingFailed as exc: + # Report the sandbox, not the skill. "Nothing was produced" + # would blame the agent for the harness's failure. + for path in wanted: + checks.append( + _check("files_exist", path, False, f"could not list the sandbox: {exc}") + ) + files = None + else: + for path in wanted: + found = _find_file(files, path) + detail = "" + if found is None: + detail = f"sandbox holds: {files or 'nothing'}" + elif found != path: + detail = f"found at {found}" + checks.append( + _check("files_exist", path, found is not None, detail) + ) + + # Judged expectations last: the deterministic results are on screen + # before the grader calls, which take a few seconds each, begin. + for statement in meta.get(convert.EXPECTED, []): + ok, reason = await judge.grade(statement, state, must_happen=True) + checks.append(_check("expected_behavior", statement, ok, reason)) + + for statement in meta.get(convert.UNEXPECTED, []): + ok, reason = await judge.grade(statement, state, must_happen=False) + checks.append(_check("unexpected_behavior", statement, ok, reason)) + + passed = bool(checks) and all(c["passed"] for c in checks) + return Score( + value=CORRECT if passed else INCORRECT, + answer=f"{sum(c['passed'] for c in checks)}/{len(checks)} checks", + metadata={CHECKS: checks}, + ) + + return score + + return _expectations() diff --git a/skillscope/engine/stats.py b/skillscope/engine/stats.py new file mode 100644 index 0000000..c28160c --- /dev/null +++ b/skillscope/engine/stats.py @@ -0,0 +1,41 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Read what an inspect run spent out of its `EvalLog`. + +inspect already counts this per model in `log.stats.model_usage`; skillscope +just has to move it somewhere the report can see. Kept apart from the engine +modules so `usage` stays the only shared vocabulary between the two engines. +""" + +from __future__ import annotations + +from .. import usage + + +def record_log(log) -> None: + """Add one `EvalLog`'s token and cost totals to the run.""" + stats = getattr(log, "stats", None) + for model_usage in (getattr(stats, "model_usage", None) or {}).values(): + usage.record( + input_tokens=getattr(model_usage, "input_tokens", 0) or 0, + output_tokens=getattr(model_usage, "output_tokens", 0) or 0, + # Populated only when the provider supplies pricing; a gateway + # generally does not, so this stays None and the report omits it. + cost_usd=getattr(model_usage, "total_cost", None), + calls=0, + ) + + # Count assistant messages, not samples. The legacy engine records one call + # per assistant event in its stream, so counting per sample here would be + # the same number only for routing -- where each case is a single turn -- + # and a large undercount for behavioral, where the agent loops. The two + # columns sit side by side in the benchmark, so they have to mean the same + # thing. + responses = 0 + for sample in getattr(log, "samples", None) or []: + for message in getattr(sample, "messages", None) or []: + if getattr(message, "role", None) == "assistant": + responses += 1 + usage.record(calls=responses) diff --git a/skillscope/engine/tools.py b/skillscope/engine/tools.py new file mode 100644 index 0000000..cea62f8 --- /dev/null +++ b/skillscope/engine/tools.py @@ -0,0 +1,171 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Reading a sandbox, on a guest that may not be POSIX. + +Not tools an agent calls -- those were removed with the harness-independent +engine. What is left is how *skillscope itself* inspects a sandbox after the +agent has finished: where the work was supposed to land (`workdir`), what is +actually there (`list_paths`), and how to resolve a path a case named. The +scorers, the judge and the claude-code leg all read a sandbox this way. + +inspect's own helpers assume a POSIX guest: `bash()` execs +`["bash", "--login", "-c", ...]` and `list_files()`/`grep()` shell out to +`find`/`grep`. On a Windows host with the `local` sandbox that is a scorer +which cannot see the files it is grading. + +Everything here is built on `SandboxEnvironment.exec` / `read_file` / +`write_file`, which are provider-level and platform-neutral. Only the shell +invocation differs, and that is probed once per sample rather than assumed: +the same Docker sandbox is POSIX whichever host started it, so the host's own +platform is not the answer. +""" + +from __future__ import annotations + +SHELL_KEY = "skillscope_shell" +WORKDIR_KEY = "skillscope_workdir" + +# A container sandbox starts at `/`, so a relative path lands beside `/proc` and +# `/etc` and a recursive listing walks the whole image. Everything the case does +# happens here instead: fixtures are seeded into it, tools resolve against it, +# and it is what gets listed. inspect_swe resolves the same problem the same way +# -- its agent cwd falls back to the home directory when the sandbox default is +# `/`. +WORKDIR = "/workspace" + +POSIX_SHELL = ["bash", "-lc"] +WINDOWS_SHELL = ["powershell", "-NoProfile", "-Command"] + +# Listing is the one thing `SandboxEnvironment` has no method for, so it stays a +# shell command -- but only one, defined here, used by both the tools and the +# scorers. +POSIX_LIST = "find . -type f" +WINDOWS_LIST = "Get-ChildItem -Recurse -File | Resolve-Path -Relative" + + +async def shell_prefix() -> list[str]: + """The argv prefix that runs a shell command in this sample's sandbox. + + Probed once and remembered: a probe per tool call would double the round + trips on the slowest part of a run. + """ + from inspect_ai.util import sandbox, store + + cached = store().get(SHELL_KEY) + if cached: + return list(cached) + + # A guest without bash does not answer "that failed" -- there is nothing + # to run, so the exec raises before any result exists. On a Windows host + # under the `local` sandbox that surfaced as WinError 2 and took the whole + # task down, which reads as the harness being broken rather than the probe + # learning what it asked. + try: + probe = await sandbox().exec(["bash", "-lc", "exit 0"], concurrency=False) + posix = probe.success + except (FileNotFoundError, OSError): + posix = False + prefix = POSIX_SHELL if posix else WINDOWS_SHELL + store().set(SHELL_KEY, prefix) + return list(prefix) + + +def containerized() -> bool: + """Whether this run has a sandbox of its own to work in.""" + from . import sandbox as sandbox_spec + + return sandbox_spec.provider() not in sandbox_spec.NOT_ISOLATED + + +def workdir_path() -> str | None: + """The same answer as `workdir()`, without creating anything. + + A task is built before any sandbox exists, so a solver that needs to be + *told* the working directory at construction time cannot await the version + that makes it. + """ + return WORKDIR if containerized() else None + + +async def workdir() -> str | None: + """The directory a case works in, or None to use the sandbox's own. + + `local` needs none: the harness's working directory is already a sensible + place and creating `/workspace` on someone's machine would not be. + """ + if not containerized(): + return None + + from inspect_ai.util import sandbox, store + + cached = store().get(WORKDIR_KEY) + if cached: + return cached + + prefix = await shell_prefix() + await sandbox().exec(prefix + [f"mkdir -p {WORKDIR}"], concurrency=False) + store().set(WORKDIR_KEY, WORKDIR) + return WORKDIR + + +async def resolve(path: str) -> str: + """A case-relative path, as the sandbox should see it.""" + base = await workdir() + if base is None or path.startswith("/"): + return path + return f"{base}/{path.lstrip('./')}" + + +async def run(command: str, timeout: int | None = None): + """Run `command` through whichever shell the sandbox has, in the workdir.""" + from inspect_ai.util import sandbox + + prefix = await shell_prefix() + return await sandbox().exec( + prefix + [command], cwd=await workdir(), timeout=timeout + ) + + +def normalize_listing(stdout: str) -> list[str]: + """Turn a directory listing into relative POSIX-style paths. + + `find` and `Get-ChildItem` disagree about separators and prefixes, so this + normalises both: backslashes become slashes and a leading `./` or `.\\` is + dropped. The harness's own furniture is filtered out -- an installed skill + is not something the case produced, and `files_exist` must not be satisfied + by one. + """ + paths: list[str] = [] + for line in stdout.splitlines(): + rel = line.strip().replace("\\", "/") + while rel.startswith("./"): + rel = rel[2:] + if not rel or rel.startswith(".claude/") or rel.startswith("skills/"): + continue + paths.append(rel) + return sorted(paths) + + +class ListingFailed(RuntimeError): + """The sandbox could not be listed, which is not the same as it being empty. + + Returning an empty list here would make a broken sandbox look exactly like + an idle agent: `files_exist` fails, and the judge -- which builds its + evidence from the same listing -- reports that nothing was produced. Both + read as the skill's fault. Raising keeps the two apart. + """ + + +async def list_paths() -> list[str]: + """Files in the sandbox working directory, as relative POSIX-style paths.""" + prefix = await shell_prefix() + listing = WINDOWS_LIST if prefix == WINDOWS_SHELL else POSIX_LIST + result = await run(listing) + if not result.success: + raise ListingFailed( + f"`{listing}` failed in the sandbox (exit {result.returncode}). " + f"stderr: {result.stderr.strip()[:200] or '(none)'}" + ) + return normalize_listing(result.stdout) diff --git a/skillscope/engine/verify.py b/skillscope/engine/verify.py new file mode 100644 index 0000000..aa48748 --- /dev/null +++ b/skillscope/engine/verify.py @@ -0,0 +1,308 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""The Claude Code verification leg (`--engine claude-code`). + +The only leg that runs the real agent *and* isolates it. `legacy` and +`claude-code-no-sandbox` both drive the same CLI on the host, so they measure +the machine as it is, with whatever else is installed on it. This one measures +the skill alone -- and when the two disagree, the disagreement is a fact about +one of those environments rather than about the skill. + +It runs actual Claude Code inside the sandbox, via `inspect_swe`, and produces +the same outcome objects as the other two engines, so the benchmark tool can +diff its report against theirs with nothing new. + +**Reporting only.** It is not a gate. Harness runs are nondeterministic and the +harness is not what we are grading; a divergence here is a question about the +skill, not a build failure. + +**Linux only.** `inspect_swe` shells `bash -c` merely to locate the CLI, and the +model proxy it starts in the guest is a Linux binary, so a Windows guest cannot +run this leg at all. That is the whole reason the primary engine does not depend +on it. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +from .. import config, deadline +from ..behavior import BehaviorOutcome +from ..datasets import Case +from . import ( + behavioral, + convert, + hooks, + models, + sandbox as sandbox_spec, + scorers, + stats, + tools, +) + +INSTALL_HINT = ( + "error: --engine claude-code needs the verify extra. Install it with:\n" + " pip install 'skillscope[verify]'" +) + + +def require() -> None: + """Fail early and legibly rather than at the first sandbox call.""" + if sys.platform.startswith("win"): + raise SystemExit( + "error: --engine claude-code cannot run on Windows. inspect_swe " + "requires a POSIX guest, both to locate the CLI and to run the " + "model proxy it installs in the sandbox." + ) + + # Not a preference. `inspect_swe` prepares the guest for the CLI by writing + # $HOME/.claude/settings.json outright, discarding whatever was there. In a + # container that file belongs to nobody and the write is the setup working + # as intended. Under the `local` provider $HOME is the developer's own, and + # the same write destroys their real configuration -- permissions, model, + # gateway environment -- with no backup and no warning. Observed, not + # theorised: it cost one settings.json before this guard existed. + provider = sandbox_spec.provider() + if provider in sandbox_spec.NOT_ISOLATED: + raise SystemExit( + f"error: --engine claude-code needs a real sandbox, but " + f"{sandbox_spec.SANDBOX_ENV}={provider!r} selects one that shares " + "the host's filesystem. inspect_swe would overwrite your own " + "~/.claude/settings.json to set up the agent. Unset " + f"{sandbox_spec.SANDBOX_ENV} for a container, or use " + "--engine claude-code-no-sandbox to run the CLI on the host " + "without it touching your configuration." + ) + + try: + import inspect_swe # noqa: F401 + except ModuleNotFoundError as exc: # pragma: no cover -- environment shape + raise SystemExit(INSTALL_HINT) from exc + + # Checked here rather than left to the call, because the call is inside a + # task inspect has already started: the failure arrives as a TypeError + # underneath a traceback, one leg of a three-leg comparison quietly + # produces no report, and the run carries on. Observed exactly that way on + # a runner whose inspect-swe predated the argument and so satisfied the + # old floor without upgrading. + import inspect as _inspect + + from inspect_swe import claude_code + + if "effort" not in _inspect.signature(claude_code).parameters: + raise SystemExit( + "error: --engine claude-code needs inspect-swe >= 0.2.71 for " + "claude_code(effort=). Without it this leg runs at the model's " + "default reasoning effort while the others run at the effort they " + "were given, so the two are not comparable. Upgrade with:\n" + " pip install --upgrade 'inspect-swe>=0.2.71'" + ) + + +# What `inspect_ai.tool`'s installer puts into a sandbox for a skill: `SKILL.md`, +# plus anything under these three subdirectories. Named because the gap between +# it and what a skill actually ships is what the staging below repairs. +SANDBOX_SKILL_SUBDIRS = ("scripts", "references", "assets") + + +# A skill's own test suite. Not part of the skill as anyone installs it, and +# `evals/evals.json` holds each case's prompt *and the skill it expects* -- so +# staging it would put the answers in the room the agent is being graded on +# choosing from. +EVAL_FIXTURE_DIR = "evals" + + +def files_to_restore(skill_dir: Path) -> list[Path]: + """The files a sandboxed skill is missing, minus the ones it must not have. + + `read_skills` collects `SKILL.md` and the contents of `scripts/`, + `references/` and `assets/`. Everything else is discarded, silently -- and + a skill's supporting material is not obliged to live in those three places. + Across the catalogue these legs run against, *no* skill has a `references/` + directory at all: they ship `reference.md`, `examples.md`, `skill-card.md`, + `templates/` and `agents/` at the top level, none of which the sandbox ever + saw. + + So the room was whole on the host and partial in the container, and the + reports compared the two as though they were one room. The clearest case: + the agent opened the right skill's `SKILL.md`, followed it to + `reference.md`, found nothing, and stopped -- graded as a missed trigger, + which reads as a skill failing to attract the prompt it was written for. + + `evals/` is excluded rather than restored, which makes this deliberately + *not* a faithful copy of what the host legs stage. They `copytree` the + whole directory and so carry the expectations into the room with it. No + agent has been observed opening them, but matching that would be copying a + mistake for the sake of symmetry. + + Pure, and returning paths rather than copying, so the rule is testable + without a sandbox and the caller can say what it staged. + """ + restore: list[Path] = [] + for path in sorted(skill_dir.rglob("*")): + if not path.is_file(): + continue + rel = path.relative_to(skill_dir) + if rel.as_posix() == "SKILL.md": + continue + if rel.parts and rel.parts[0] in SANDBOX_SKILL_SUBDIRS: + continue + if rel.parts and rel.parts[0] == EVAL_FIXTURE_DIR: + continue + restore.append(path) + return restore + + +def complete_skills_solver(skills: dict[str, Path]): + """Add the files inspect's installer drops, so the guest has a whole skill. + + Chained *ahead* of `claude_code`, which installs its own half when the + agent starts. Safe in that order: the installer writes named files and + never clears the directory, so the two halves land side by side. Ordering + it after would be worse than useless -- `claude_code` runs the agent, so + anything staged behind it arrives once the run is over. + + Additive on purpose. Discovery stays inspect's, which is the point of this + leg; the only difference is that a skill the agent chooses to open is all + there. The host legs need none of it -- they `copytree` the directory and + have always had the whole thing. + """ + from inspect_ai.solver import solver + + @solver + def _complete(): + async def solve(state, generate): + from inspect_ai.util import sandbox + + root = tools.workdir_path() or "." + for name, skill_dir in skills.items(): + for path in files_to_restore(skill_dir): + rel = path.relative_to(skill_dir).as_posix() + await sandbox().write_file( + f"{root}/.claude/skills/{name}/{rel}", path.read_bytes() + ) + return state + + return solve + + return _complete() + + +def _ensure_workdir(): + """Create the directory the scorers read, before the agent runs in it. + + The other engines reach it lazily, the first time one of our own tools is + used. This leg's agent brings its own tools and never calls ours, so + nothing would create it -- and `cwd` below has to name a directory that + exists. + """ + from inspect_ai.solver import solver + + @solver + def _ensure(): + async def solve(state, generate): + await tools.workdir() + return state + + return solve + + return _ensure() + + +def build_task( + skill: str, + cases: list[Case], + model: str, + ctx: dict | None = None, + effort: str | None = None, +): + """One task per skill, solved by real Claude Code rather than our agent.""" + from inspect_ai import Task + from inspect_ai.solver import chain + from inspect_swe import claude_code + + skill_dir = config.active().skill_path(skill) + samples = [convert.sample_from_case(c, skill_dir, ctx) for c in cases] + + # Where the agent works has to be where the scorers look. A container + # starts at `/`, and left to itself the agent scattered its output there: + # every `files_exist` check in the first trial run reported an empty + # sandbox, which reads as "the agent did nothing" rather than "the agent + # worked somewhere else". `inspect_swe` takes a working directory rather + # than instructions, so this is set rather than asked for. + # See `behavioral.build_task`: the sandboxed leg runs the same hooks, and + # a container the agent started on a shared runner outlives the sample + # whichever engine started it. + hook = hooks.load(skill) + + bound = deadline.active() + return Task( + name=f"claude-code-{skill}", + setup=hooks.setup_solver(hook, skill), + cleanup=hooks.cleanup_fn(hook, skill), + dataset=samples, + # `skills=` installs into .claude/skills inside the sandbox, which is + # where the real harness looks -- the point of this leg is that its + # discovery machinery, not ours, decides what happens. + # `effort` reaches the agent rather than being dropped here: unset is + # not neutral, it is the model's own default, and the legs this one is + # compared against all pass the value they were given. + # A behavioral case has more riding on this than a routing one: it runs + # the skill to completion, so a `reference.md` the installer dropped is + # a step the agent cannot take, and the scorer records it as work the + # skill failed to do. + solver=chain( + _ensure_workdir(), + complete_skills_solver({skill_dir.name: skill_dir}), + claude_code( + skills=[skill_dir], cwd=tools.workdir_path(), effort=effort or None + ), + ), + scorer=scorers.expectations(), + sandbox=sandbox_spec.for_skill(skill), + message_limit=behavioral.message_limit_for(model), + time_limit=behavioral.task_time_limit(bound), + ) + + +def run( + skills: list[str], cases: list[Case], model: str, effort: str +) -> list[BehaviorOutcome]: + """Mirrors `behavior.run`, so the CLI and the benchmark treat it the same.""" + from inspect_ai import eval as inspect_eval + + require() + + sandbox_spec.require_provider() + + outcomes: list[BehaviorOutcome] = [] + for skill in skills: + skill_cases = [c for c in cases if c.skill == skill and c.has_behavior] + if not skill_cases: + continue + + print(f"[claude-code] {skill}: {len(skill_cases)} case(s)", flush=True) + logs = inspect_eval( + build_task(skill, skill_cases, model, effort=effort), + model=model, + model_args=models.model_args(model), + log_dir=str(Path(".skillscope") / "logs"), + log_realtime=behavioral.realtime_logging(), + display="plain", + ) + for log in logs: + stats.record_log(log) + outcomes.extend(behavioral._outcomes(log, skill, skill_cases)) + + for outcome in outcomes: + passed = sum(1 for c in outcome.checks if c["passed"]) + print( + f" [{'PASS' if outcome.passed else 'FAIL'}] {outcome.id}: " + f"{passed}/{len(outcome.checks)} checks in {outcome.elapsed_s}s" + + (f" -- {outcome.error}" if outcome.error else ""), + flush=True, + ) + return outcomes diff --git a/skillscope/routing.py b/skillscope/routing.py index fbaac65..0e69cda 100644 --- a/skillscope/routing.py +++ b/skillscope/routing.py @@ -49,7 +49,7 @@ from dataclasses import asdict, dataclass, field from pathlib import Path -from . import deadline +from . import deadline, usage from .agent import claude_env from .datasets import Case @@ -65,6 +65,68 @@ # Where the staged skills live, as they appear in a tool argument. STAGED_SKILLS_DIR = ".claude/skills" +# Signatures of a failure that belongs to the model provider rather than to the +# skill. Matched case-insensitively against whatever the run reported. +# +# Worth naming rather than leaving as prose: a gateway that answers 504 lands +# in a report as a lower score with nothing saying why, and a reader cannot +# tell it from the skill failing. At the rate these have been observed -- a +# third of runs on one catalogue -- an unmarked provider error is the single +# biggest reason two runs of the same engine disagree, which makes it the +# first thing to rule out before a difference between engines means anything. +PROVIDER_ERROR_SIGNS = ( + "api error", + "overloaded", + "rate limit", + "429", + "500", + "502", + "503", + "504", + "gateway", + "upstream", + "server-side", + "connection error", + "apiconnectionerror", + "timed out", + "timeout", +) + + +def _result_error(event: dict) -> str: + """Why a `result` event says the run failed, keeping the reason it gave. + + The CLI puts the reason in `result` sometimes and in `subtype` other times + -- `error_during_execution`, `error_max_turns` -- and collapsing both into + "result event reported an error" discards the one thing that makes the + failure readable. It did exactly that to a gateway 504 on this catalogue: + the report showed a case that failed for no stated reason, and nothing + downstream could tell it from a skill that simply did not fire. + + Both are kept when both exist, because the subtype says what class of + failure it was and the body says what happened. + """ + detail = str(event.get("result") or "").strip() + subtype = str(event.get("subtype") or "").strip() + if detail and subtype: + return f"{detail} (subtype: {subtype})"[:400] + return (detail or subtype or "result event reported an error")[:400] + + +def is_provider_error(message: str | None) -> bool: + """Whether this failure came from the provider rather than from the skill. + + Deliberately generous. A false positive marks a real skill failure as + degraded, which makes a reader look twice at a run that was fine. A false + negative lets a gateway outage score as a routing miss, which makes a + reader trust a number that measured nothing. The costs are not symmetric. + """ + if not message: + return False + lowered = str(message).lower() + return any(sign in lowered for sign in PROVIDER_ERROR_SIGNS) + + VERDICTS = ("correct_trigger", "true_negative", "missed_trigger", "wrong_skill", "false_trigger", "error") PASSING_VERDICTS = {"correct_trigger", "true_negative"} @@ -104,23 +166,18 @@ class Outcome: visible_skills: list[str] = field(default_factory=list) extra_skills: list[str] = field(default_factory=list) error: str | None = None + # Set when the failure was the provider's. Kept beside `error` rather than + # folded into `verdict` so the verdict vocabulary stays about routing, and + # so a reader can subtract these without re-parsing error strings. + degraded: bool = False -def stage_workspace(skills: dict[str, Path]) -> Path: - """Install every skill in the routing set into a fresh temp workspace. - Claude Code loads ``.claude/skills/`` from a directory passed with - ``--add-dir``, which registers each skill's name and description in the - system prompt without injecting its body -- exactly the state a routing - decision is made from. One workspace per case keeps cases isolated (and - lets them run concurrently). - """ - workspace = Path(tempfile.mkdtemp(prefix="routing-")) - dest_root = workspace / ".claude" / "skills" - dest_root.mkdir(parents=True, exist_ok=True) - for name, source in skills.items(): - shutil.copytree(source, dest_root / name) - return workspace + +def _capped_timeout(seconds: float) -> float: + """``seconds``, or whatever the command's ``--timeout`` has left.""" + bound = deadline.active() + return seconds if bound is None else bound.cap(seconds) def supported_flags(flags: list[str]) -> set[str]: @@ -148,6 +205,12 @@ def supported_flags(flags: list[str]) -> set[str]: return {flag for flag in flags if flag in text} +# The credentials that live in the environment rather than in the CLI's own +# config dir. Either can be carried into a throwaway config dir; a login stored +# in the real one cannot, which is the whole distinction this turns on. +ENV_CREDENTIALS = ("ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN") + + def can_isolate_config() -> bool: """Whether the runner's own ``~/.claude`` can be kept out of the session. @@ -156,8 +219,15 @@ def can_isolate_config() -> bool: Pointing the CLI at a throwaway config dir achieves that, but only when auth comes from the environment -- if the login lives in the real config dir, hiding it means no case even starts. + + `ANTHROPIC_AUTH_TOKEN` counts for the same reason `ANTHROPIC_API_KEY` + does: it is in the environment, so it survives the redirect. Testing only + for the key refused every runner that authenticates by workload identity + federation -- which holds no key at all, by design, and is what the + reusable workflow offers downstream repos through `federation_rule_id`. + Found by running the default engine the way a product repo would. """ - return bool(os.environ.get("ANTHROPIC_API_KEY", "").strip()) + return any(os.environ.get(name, "").strip() for name in ENV_CREDENTIALS) def _iter_tool_uses(obj) -> list[tuple[str, str]]: @@ -287,130 +357,18 @@ def detect_activation(event: dict, skills: list[str], allow_body_path: bool = Tr return None -def _init_skills(event: dict, skills: list[str]) -> list[str] | None: - """Skill names the CLI reported at session init, if this is that event. - Used to prove the agent really saw the whole routing set (and nothing extra): - a stray user-level skill on the runner would change every routing decision. - """ - if event.get("type") != "system" or event.get("subtype") != "init": - return None - seen: list[str] = [] - for key in ("skills", "slash_commands", "slashCommands", "commands"): - entries = event.get(key) - if not isinstance(entries, list): - continue - for entry in entries: - text = entry if isinstance(entry, str) else json.dumps(entry, ensure_ascii=False) - hit = _match_skill(text, skills) - if hit and hit not in seen: - seen.append(hit) - return seen - - -def _init_tools(event: dict) -> set[str] | None: - """Tool names the CLI reported at session init, if this is that event. - - Used to decide whether the SKILL.md-path fallback in ``detect_activation`` - applies to this build. An init event without a tool list leaves the - fallback on, which is how older builds behaved. - """ - if event.get("type") != "system" or event.get("subtype") != "init": - return None - tools = event.get("tools") - if not isinstance(tools, list): - return set() - return {str(tool).lower() for tool in tools} -def _init_extra_skills(event: dict, skills: list[str]) -> list[str] | None: - """Skills the CLI reported at init that this eval did not install. - A user-level skill on the runner is registered alongside the staged ones - and competes for every prompt, so the routing numbers describe a room - nobody asked for. The ``other:`` check only notices such a skill when it - actually fires; this notices it being installed at all. - """ - if event.get("type") != "system" or event.get("subtype") != "init": - return None - entries = event.get("skills") - if not isinstance(entries, list): - return [] - known = {skill.lower() for skill in skills} - extra: list[str] = [] - for entry in entries: - if isinstance(entry, str): - name = entry - elif isinstance(entry, dict): - name = str(entry.get("name") or "") - else: - continue - name = name.strip().lstrip("/") - if name and name.lower() not in known and name not in extra: - extra.append(name) - return extra - - -def _pump(stream, sink: queue.Queue) -> None: - try: - for line in stream: - sink.put(line) - finally: - sink.put(None) -def _terminate(proc: subprocess.Popen) -> None: - """Kill the CLI and its children. - The `claude` process spawns helpers, so killing only the parent can leave - an orphan holding the API call open -- which is the cost this eval exists - to avoid. Kill the whole group/tree. - """ - if proc.poll() is not None: - return - try: - if os.name == "nt": - subprocess.run( - ["taskkill", "/F", "/T", "/PID", str(proc.pid)], - capture_output=True, - check=False, - ) - else: - os.killpg(os.getpgid(proc.pid), signal.SIGKILL) - except (OSError, subprocess.SubprocessError): - pass - try: - proc.kill() - except OSError: - pass - try: - proc.wait(timeout=15) - except subprocess.TimeoutExpired: - pass -def _capped_timeout(seconds: float) -> float: - """``seconds``, or whatever the command's ``--timeout`` has left.""" - bound = deadline.active() - return seconds if bound is None else bound.cap(seconds) -def _command_timeout_outcome(case: Case, bound: deadline.Deadline) -> Outcome: - print(f" [FAIL] {case.id}: {bound.message()}", flush=True) - return Outcome( - id=case.id, - category=case.category, - skill=case.skill, - prompt=case.prompt, - expect=case.expect_skill, - observed=None, - verdict="error", - passed=False, - stop_reason="timeout", - elapsed_s=0.0, - tool_calls=0, - error=bound.message(), - ) + def classify(expect: str | None, observed: str | None) -> str: @@ -421,203 +379,28 @@ def classify(expect: str | None, observed: str | None) -> str: return "correct_trigger" if observed == expect else "wrong_skill" -def run_case(case: Case, routing_set: dict[str, Path], config: RoutingConfig) -> Outcome: - """Run one prompt, stopping as soon as the routing decision is known.""" - bound = deadline.active() - if bound is not None and bound.expired(): - return _command_timeout_outcome(case, bound) - claude_bin = shutil.which("claude") - if not claude_bin: - raise SystemExit("error: 'claude' CLI not found on PATH") - - skills = list(routing_set) - workspace = stage_workspace(routing_set) - # Outside the workspace: the agent can list its own cwd, and a config dir - # sitting in there would be one more thing for it to find. - config_dir = ( - Path(tempfile.mkdtemp(prefix="routing-config-")) if config.isolate_config else None - ) - cmd = [ - claude_bin, - "-p", - "--output-format", - "stream-json", - "--verbose", - "--dangerously-skip-permissions", - "--add-dir", - str(workspace), - "--model", - config.model, - ] - if config.effort: - cmd += ["--effort", config.effort] - # Dozens of throwaway sessions per run; don't leave them on disk. - if "--no-session-persistence" in config.available_flags: - cmd += ["--no-session-persistence"] - if config.max_budget_usd > 0 and "--max-budget-usd" in config.available_flags: - cmd += ["--max-budget-usd", str(config.max_budget_usd)] - - spawn: dict = {} - if os.name == "nt": - spawn["creationflags"] = subprocess.CREATE_NEW_PROCESS_GROUP - else: - spawn["start_new_session"] = True - - env = claude_env() - if config_dir is not None: - env["CLAUDE_CONFIG_DIR"] = str(config_dir) - - events: list[dict] = [] - observed: str | None = None - visible: list[str] = [] - extra: list[str] = [] - stop_reason = "completed" - tool_calls = 0 - inspection_calls = 0 - allow_body_path = True - error: str | None = None - stderr_lines: list[str] = [] - - start = time.perf_counter() - proc = subprocess.Popen( - cmd, - cwd=str(workspace), - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - encoding="utf-8", - errors="replace", - bufsize=1, - env=env, - **spawn, - ) - try: - assert proc.stdin is not None - proc.stdin.write(case.prompt) - proc.stdin.close() - - stdout_q: queue.Queue = queue.Queue() - threading.Thread(target=_pump, args=(proc.stdout, stdout_q), daemon=True).start() - threading.Thread( - target=lambda: stderr_lines.extend(proc.stderr.readlines()), daemon=True - ).start() - - case_deadline = time.perf_counter() + _capped_timeout(config.case_timeout) - while True: - remaining = case_deadline - time.perf_counter() - if remaining <= 0: - stop_reason = "timeout" - break - try: - line = stdout_q.get(timeout=min(1.0, remaining)) - except queue.Empty: - continue - if line is None: - break - line = line.strip() - if not line: - continue - try: - event = json.loads(line) - except json.JSONDecodeError: - continue - events.append(event) - - reported = _init_skills(event, skills) - if reported is not None: - visible = reported - uninstalled = _init_extra_skills(event, skills) - if uninstalled is not None: - extra = uninstalled - tools = _init_tools(event) - if tools is not None: - allow_body_path = not (tools & SKILL_TOOLS) - - hit = detect_activation(event, skills, allow_body_path=allow_body_path) - if hit: - observed = hit - stop_reason = "skill_activated" - break - - if event.get("type") == "result": - stop_reason = "result" - if event.get("is_error"): - error = str(event.get("result") or "result event reported an error")[:400] - break - - for name, tool_input in _iter_tool_uses(event): - if name.lower() in BOOKKEEPING_TOOLS: - continue - if _is_skills_inspection(tool_input, skills): - inspection_calls += 1 - else: - tool_calls += 1 - # Inspection is exempt from the tool budget but not unbounded: an - # agent that has read every installed skill and still called none - # has made its decision, and the run should not idle to timeout. - if tool_calls >= config.max_tool_calls or inspection_calls >= config.max_inspection_calls: - stop_reason = "tool_budget" - break - finally: - _terminate(proc) - elapsed = time.perf_counter() - start - if config.keep_logs: - logs_dir = Path(config.keep_logs) - logs_dir.mkdir(parents=True, exist_ok=True) - (logs_dir / f"{case.id}.jsonl").write_text( - "\n".join(json.dumps(e, ensure_ascii=False) for e in events), - encoding="utf-8", - ) - shutil.rmtree(workspace, ignore_errors=True) - if config_dir is not None: - shutil.rmtree(config_dir, ignore_errors=True) - - if not events: - error = ("".join(stderr_lines).strip() or "claude produced no stream-json output")[:400] - - # "no skill activated" is only a real finding when the run got far enough to - # show a decision: the agent answered (`result`) or started doing the work - # itself (`tool_budget`). A stream that just ends, or a hang, means the run - # never made a routing decision -- grading that as a missed trigger would - # invent a result out of an infrastructure failure. - if observed is None and stop_reason in INCONCLUSIVE_STOPS: - verdict = "error" - detail = "".join(stderr_lines).strip() - error = error or ( - f"run ended without a routing decision (stopped after: {stop_reason})" - + (f"; stderr: {detail[:300]}" if detail else "") - ) - elif error and observed is None: - verdict = "error" - else: - verdict = classify(case.expect_skill, observed) - - outcome = Outcome( - id=case.id, - category=case.category, - skill=case.skill, - prompt=case.prompt, - expect=case.expect_skill, - observed=observed, - verdict=verdict, - passed=verdict in PASSING_VERDICTS, - stop_reason=stop_reason, - elapsed_s=round(elapsed, 2), - tool_calls=tool_calls, - inspection_calls=inspection_calls, - visible_skills=visible, - extra_skills=extra, - error=error, - ) - print( - f" [{'PASS' if outcome.passed else 'FAIL'}] {case.id}: " - f"expected {case.expect_skill or 'no skill'} -> got {observed or 'no skill'} " - f"({verdict}, {stop_reason}, {outcome.elapsed_s}s)", - flush=True, - ) - return outcome + +def near_a_limit(outcome: "Outcome", meta: dict) -> bool: + """Whether this case stopped close enough to a cap to be decided by one. + + A case that used one call of a budget of four is measuring the agent. A + case that used four is measuring the budget: ordinary run-to-run variation + moves it across the line, and the verdict flips with it. Those two look + identical in a report, and the difference is the whole of whether a flip + between two runs means anything. + + Within one, because that is the resolution a single extra call has. Caps + the run did not set are not caps: a leg that could not enforce a budget + reports none, and nothing here should invent a threshold for it. + """ + for used, cap in ( + (outcome.tool_calls, meta.get("max_tool_calls")), + (outcome.inspection_calls, meta.get("max_inspection_calls")), + ): + if isinstance(cap, int) and cap > 0 and used >= cap - 1: + return True + return False def summarize(outcomes: list[Outcome], skills: list[str], meta: dict) -> dict: @@ -686,6 +469,17 @@ def summarize(outcomes: list[Outcome], skills: list[str], meta: dict) -> dict: # way the numbers are an artifact, not a result. "activations": sum(1 for o in graded if o.observed), "activations_expected": sum(1 for o in graded if o.expect), + # Cases the provider failed rather than the skill. Reported beside + # the score because it is the number that decides whether the + # score can be read at all: a run with a third of its cases + # degraded has measured the gateway, and comparing it against + # another run attributes an outage to whatever changed in between. + "degraded": sum(1 for o in outcomes if o.degraded), + # Cases that stopped within one call of a cap. Not failures -- + # a flag on how much of this run measured the agent and how much + # measured the budget. A flip on one of these between two runs is + # a threshold artefact before it is anything else. + "near_limit": sum(1 for o in outcomes if near_a_limit(o, meta)), }, "verdicts": {name: verdicts.get(name, 0) for name in VERDICTS}, "by_category": by_category, @@ -716,6 +510,34 @@ def render_markdown(summary: dict) -> str: "is only as meaningful as the room is realistic.", "", ] + + # Before the table, not after it. A reader who has already taken in the + # score has formed a view, and a note underneath it does not undo that -- + # whereas a run with a tenth of its cases degraded is one whose score + # should be read differently from the first glance. + near = totals.get("near_limit", 0) + if near: + lines += [ + f"> **{near} of {totals['cases']} cases stopped within one call of " + "a budget.** Those measured the budget as much as the agent: one " + "more call either way moves them across the line and the verdict " + "with them. Compare two runs on these last, and expect them to " + "flip without meaning anything.", + "", + ] + + degraded = totals.get("degraded", 0) + if degraded: + share = degraded / totals["cases"] if totals["cases"] else 0 + lines += [ + f"> **{degraded} of {totals['cases']} cases failed at the model " + f"provider, not in the skill** ({share:.0%}). A gateway error " + "lands as a lower score with nothing in the verdict saying why, " + "so treat this run as degraded rather than as a measurement: the " + "difference between it and another run may be the provider's " + "rather than the skill's or the engine's.", + "", + ] lines += [ "| Verdict | Count | Meaning |", "| --- | --- | --- |", diff --git a/skillscope/schema/machine.schema.json b/skillscope/schema/machine.schema.json index 8f5997e..734f93a 100644 --- a/skillscope/schema/machine.schema.json +++ b/skillscope/schema/machine.schema.json @@ -19,6 +19,12 @@ "items": { "type": "string", "minLength": 1 }, "description": "Extra `runs-on` labels the behavioral cases need, added to the base labels the workflow supplies. Name the hardware, not the pool: `mi300x` says what the skill requires and lands it on any runner registered with that label. A leg that asks for labels is treated as scoped, so it is also what the repo may hold behind a gate label and pay for from a separate environment. Keep the list as short as the runners allow -- every label is a condition a pool has to satisfy, and a label no runner carries is a job that queues forever rather than an error.", "examples": [["mi300x"]] + }, + "sandbox": { + "type": "string", + "minLength": 1, + "description": "Compose file, resolved beside this machine.yml in evals/, describing the sandbox the behavioral cases need under `--engine claude-code`. Absent means the default container with no network, which is what a skill that only reads and writes files should want. Name one to opt into network egress -- a skill that installs a server or pulls a model cannot run without it -- or to bind a device in. Ignored on Windows, where inspect's sandbox layer assumes a POSIX guest and cases run unsandboxed on the host instead.", + "examples": ["compose.yaml"] } } } diff --git a/skillscope/usage.py b/skillscope/usage.py new file mode 100644 index 0000000..2160a1c --- /dev/null +++ b/skillscope/usage.py @@ -0,0 +1,97 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""What a graded run spent, recorded by whichever engine ran it. + +Both engines know their own cost and neither reported it: the legacy engine +discards the `total_cost_usd` the CLI hands back with every result, and inspect +keeps usage in its own `.eval` log where skillscope's report never looks. That +was fine while there was one engine and nothing to compare it against. + +Accumulated in module state rather than threaded through return values, because +the engines' entry points return outcome lists and that signature is what lets +the CLI swap one for the other in a single line. A run is a process, so the +scope is right even if the shape is blunt; `reset()` exists for tests. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + + +@dataclass +class Usage: + """Totals for one graded run. Fields are None when the engine cannot say.""" + + input_tokens: int = 0 + output_tokens: int = 0 + cost_usd: float | None = None + calls: int = 0 + + @property + def total_tokens(self) -> int: + return self.input_tokens + self.output_tokens + + def as_meta(self) -> dict: + """The shape that goes into a report's `meta`, omitting what is unknown.""" + meta: dict = { + "model_calls": self.calls, + "input_tokens": self.input_tokens, + "output_tokens": self.output_tokens, + "total_tokens": self.total_tokens, + } + if self.cost_usd is not None: + meta["cost_usd"] = round(self.cost_usd, 4) + return meta + + +_current: Usage = Usage() + + +def reset() -> None: + global _current + _current = Usage() + + +def snapshot() -> Usage: + return _current + + +def record( + *, + input_tokens: int = 0, + output_tokens: int = 0, + cost_usd: float | None = None, + calls: int = 1, +) -> None: + """Add one model interaction's cost to the run.""" + _current.input_tokens += int(input_tokens or 0) + _current.output_tokens += int(output_tokens or 0) + _current.calls += int(calls or 0) + if cost_usd is not None: + _current.cost_usd = (_current.cost_usd or 0.0) + float(cost_usd) + + +def record_stream_event(event: dict) -> None: + """Record what one `claude` stream-json event says the run has spent. + + Shared by both legacy commands, because they read the same stream and a + column that means "responses" in one and "cases" in the other is worse than + no column at all. + + Tokens and responses come from assistant events, one per model reply. Cost + comes only from the result event, where it is a run total -- and a routing + case is normally killed before that event arrives, so it reports responses + with no cost. That is what the legacy engine can actually observe. + """ + kind = event.get("type") + if kind == "assistant": + message = event.get("message") + counts = (message or {}).get("usage") if isinstance(message, dict) else None + record( + input_tokens=(counts or {}).get("input_tokens", 0), + output_tokens=(counts or {}).get("output_tokens", 0), + ) + elif kind == "result": + record(cost_usd=event.get("total_cost_usd"), calls=0) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 0bf0fb1..47cf954 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -19,15 +19,20 @@ from __future__ import annotations import argparse +import sys +import asyncio +import inspect import contextlib import io import json import os import re import runpy +import shutil import subprocess import tempfile import time +import types import unittest import urllib.error from pathlib import Path @@ -42,12 +47,25 @@ credentials, datasets, deadline, + engine as engine_module, references, routing, structure, ) from skillscope import selection as select_module from skillscope.datasets import EVALUATIONS_KEY, TRIGGER_KEY +from skillscope.engine import behavioral as engine_behavioral +from skillscope.engine import hooks as engine_hooks +from skillscope.engine import no_sandbox as engine_no_sandbox +from skillscope.engine import behavioral as engine_behavioral +from skillscope.engine import judge as engine_judge +from skillscope.engine import models as engine_models +from skillscope.engine import sandbox as engine_sandbox +from skillscope.engine import verify as engine_verify +from skillscope.engine import routing as engine_routing +from skillscope.engine import no_sandbox as engine_no_sandbox +from skillscope.engine import behavioral as engine_behavioral +from skillscope.engine import tools as engine_tools REPO_ROOT = datasets.PACKAGE_DIR.parent SCHEMA_DIR = datasets.PACKAGE_DIR / "schema" @@ -331,13 +349,16 @@ def setUp(self) -> None: def test_documented_keys_match_the_parser(self) -> None: self.assertEqual(set(self.schema["properties"]), datasets.MACHINE_KEYS) - def test_neither_key_is_enumerated_in_the_schema(self) -> None: - # Neither can be: a label means whatever a repo registered its runners - # with, so the schema documents what the key is for and the workflow - # supplies the labels around it. - for key in datasets.MACHINE_KEYS: + def test_no_list_key_is_enumerated_in_the_schema(self) -> None: + # Neither `os` nor `labels` can be: a label means whatever a repo + # registered its runners with, so the schema documents what the key is + # for and the workflow supplies the labels around it. Scoped to the + # list-valued keys, since `sandbox` names a file rather than a set. + for key, spec in self.schema["properties"].items(): + if spec.get("type") != "array": + continue with self.subTest(key=key): - self.assertNotIn("enum", self.schema["properties"][key]["items"]) + self.assertNotIn("enum", spec["items"]) def test_every_machine_yml_in_the_repo_resolves(self) -> None: for skill in datasets.declared_skills(): @@ -764,32 +785,7 @@ def test_an_elapsed_bound_is_expired(self) -> None: self.assertEqual(bound.cap(240), 0.0) self.assertIn("--timeout of 1s", bound.message()) - def test_an_expired_deadline_does_not_start_a_routing_case(self) -> None: - cases, errors = parse(triggers(id="hung", prompt="go")) - self.assertEqual(errors, []) - bound = deadline.Deadline(1, command="routing", start=time.perf_counter() - 2) - previous = deadline.use(bound) - try: - outcome = routing.run_case( - cases[0], {"demo-skill": Path(".")}, routing.RoutingConfig() - ) - finally: - deadline.use(previous) - self.assertEqual(outcome.verdict, "error") - self.assertEqual(outcome.stop_reason, "timeout") - self.assertIn("routing exceeded --timeout", outcome.error) - - def test_an_expired_deadline_does_not_start_a_behavioral_case(self) -> None: - cases, errors = parse(triggers(id="hung", prompt="go", logs_contain=["x"])) - self.assertEqual(errors, []) - bound = deadline.Deadline(1, command="behavioral", start=time.perf_counter() - 2) - previous = deadline.use(bound) - try: - outcome = behavior.run_case(cases[0], {}, None, "opus", "high") - finally: - deadline.use(previous) - self.assertFalse(outcome.passed) - self.assertIn("behavioral exceeded --timeout", outcome.error) + class TestCredentialResolution(unittest.TestCase): @@ -1489,19 +1485,6 @@ def test_every_listed_skill_brings_prompts_to_the_routing_run(self) -> None: covered = {case.expect_skill for case in cases if case.expect_skill} self.assertEqual(sorted(covered), sorted(listed)) - def test_hooks_are_importable_and_expose_known_entry_points(self) -> None: - known = {"setup_session", "setup", "teardown", "check"} - for skill in datasets.skills_with_datasets(): - if not datasets.hooks_path(skill).is_file(): - continue - with self.subTest(skill=skill): - module = behavior.load_hooks(skill) - exported = { - name - for name in dir(module) - if not name.startswith("_") and callable(getattr(module, name)) - } - self.assertTrue(exported & known, f"{skill} hooks export nothing usable") def test_the_shipped_negatives_pool_parses(self) -> None: shared = datasets.load_shared_negatives() @@ -2173,24 +2156,6 @@ def test_inspecting_the_installed_skills_is_recognized(self) -> None: self.assertFalse(routing._is_skills_inspection('{"path": "src/main.py"}', self.SKILLS)) -class TestRoutingStaging(unittest.TestCase): - def test_the_routing_set_lands_in_the_workspace_and_nothing_else(self) -> None: - repo = Repo(self) - repo.skill("one", dataset=tier0_dataset("one")) - repo.skill("two", dataset=tier0_dataset("two")) - repo.skill("unlisted", dataset=tier0_dataset("unlisted")) - cfg = repo.activate(routing_room="one,two") - workspace = routing.stage_workspace(cfg.routing_set) - try: - staged = sorted(p.name for p in (workspace / ".claude" / "skills").iterdir()) - self.assertEqual(staged, ["one", "two"]) - self.assertTrue( - (workspace / ".claude" / "skills" / "one" / "SKILL.md").is_file() - ) - finally: - import shutil - - shutil.rmtree(workspace, ignore_errors=True) class TestPromptTemplating(unittest.TestCase): @@ -2327,155 +2292,8 @@ def prompt(self, text: str): return agent.Run(workspace=self.workspace, events=self.events, judge_model=None) -class TestBehaviorCaseFlow(unittest.TestCase): - """The hook contract and prompt templating, without spending tokens.""" - - def setUp(self) -> None: - self.repo = Repo(self) - self.repo.skill( - "demo-skill", - dataset=tier0_dataset("demo"), - workspace={"evals/files/stub/main.py": "print('hi')\n"}, - ) - self.repo.activate() - - def run_case(self, case_payload: dict, hooks=None, events=None, skill="demo-skill"): - cases, errors = parse(triggers(**case_payload), skill=skill) - self.assertEqual(errors, []) - made: list[FakeAgent] = [] - - def fake_claude(model, *, skill, effort, seed=None): - made.append(FakeAgent(events or stream(), seed)) - return made[-1] - - original = behavior.claude - behavior.claude = fake_claude - try: - outcome = behavior.run_case(cases[0], {}, hooks, "opus", "high") - finally: - behavior.claude = original - return outcome, made[0] - - def test_a_passing_case(self) -> None: - outcome, session = self.run_case( - {"id": "a", "prompt": "run it", "logs_contain": ["detect.py"]}, - events=stream(("Bash", {"command": "detect.py"})), - ) - self.assertTrue(outcome.passed) - self.assertEqual(session.prompts, ["run it"]) - - def test_a_failing_expectation_fails_the_case(self) -> None: - outcome, _ = self.run_case({"id": "a", "prompt": "run it", "logs_contain": ["nope"]}) - self.assertFalse(outcome.passed) - - def test_hooks_run_in_order_and_can_template_the_prompt(self) -> None: - calls: list[str] = [] - - class Hooks: - @staticmethod - def setup(workspace, case, ctx): - calls.append("setup") - return {"output_dir": workspace / "out"} - - @staticmethod - def check(run, case, ctx): - calls.append("check") - - @staticmethod - def teardown(workspace, case, ctx): - calls.append("teardown") - - outcome, session = self.run_case( - {"id": "a", "prompt": "write to {output_dir}", "logs_contain": ["detect"]}, - hooks=Hooks, - events=stream(("Bash", {"command": "detect"})), - ) - self.assertEqual(calls, ["setup", "check", "teardown"]) - self.assertNotIn("{output_dir}", session.prompts[0]) - self.assertTrue(outcome.passed) - - def test_a_raising_hook_check_fails_the_case_without_killing_the_run(self) -> None: - class Hooks: - @staticmethod - def check(run, case, ctx): - raise AssertionError("scorer reported 3 failures") - - outcome, _ = self.run_case({"id": "a", "prompt": "p", "logs_contain": []}, hooks=Hooks) - self.assertFalse(outcome.passed) - self.assertTrue(any("scorer reported" in c["detail"] for c in outcome.checks)) - - def test_teardown_runs_even_when_the_agent_raises(self) -> None: - calls: list[str] = [] - - class Hooks: - @staticmethod - def teardown(workspace, case, ctx): - calls.append("teardown") - - class Exploding(FakeAgent): - def prompt(self, text): - raise RuntimeError("claude produced no output") - - cases, _ = parse(triggers(id="a", prompt="p", unexpected_behavior=["x"])) - original = behavior.claude - behavior.claude = lambda model, *, skill, effort, seed=None: Exploding(stream(), seed) - try: - outcome = behavior.run_case(cases[0], {}, Hooks, "opus", "high") - finally: - behavior.claude = original - self.assertEqual(calls, ["teardown"]) - self.assertFalse(outcome.passed) - self.assertIn("claude produced no output", outcome.error) - - def test_workspace_fixtures_are_staged(self) -> None: - outcome, _ = self.run_case( - { - "id": "a", - "prompt": "edit it", - "workspace": "evals/files/stub", - "files_exist": ["main.py"], - } - ) - self.assertTrue(outcome.passed, outcome.checks) -class TestBehaviorReporting(unittest.TestCase): - def test_summary_counts_cases_and_expectations(self) -> None: - outcomes = [ - behavior.BehaviorOutcome( - id="a", - skill="s", - prompt="p", - passed=True, - elapsed_s=1.0, - checks=[ - {"kind": "logs_contain", "expectation": "x", "passed": True, "detail": ""} - ], - ), - behavior.BehaviorOutcome( - id="b", - skill="s", - prompt="p", - passed=False, - elapsed_s=1.0, - checks=[ - { - "kind": "expected_behavior", - "expectation": "y", - "passed": False, - "detail": "no", - } - ], - ), - ] - summary = behavior.summarize(outcomes, {"model": "opus", "effort": "high"}) - self.assertEqual( - summary["totals"], - {"cases": 2, "passed": 1, "checks": 2, "checks_passed": 1, "errors": 0}, - ) - report = behavior.render_markdown(summary) - self.assertIn("1/2 cases passed", report) - self.assertIn("`b`", report) class TestCaseFiltering(unittest.TestCase): @@ -2536,5 +2354,2440 @@ def test_an_empty_room_leaves_only_the_shared_pool(self) -> None: self.assertTrue(all(case.skill is None for case in cases)) +class TestCiModelPin(unittest.TestCase): + """The pin keeps paid runs comparable; a mock is neither paid nor graded.""" + + def test_a_real_model_is_pinned_under_ci(self) -> None: + with mock.patch.dict(os.environ, {"CI": "true"}): + self.assertEqual(agent.enforce_model_policy("sonnet"), "opus") + + def test_a_mock_is_left_alone_under_ci(self) -> None: + # Otherwise the free wiring run becomes a run that needs a key, in the + # one place where not needing a key is the whole point. + with mock.patch.dict(os.environ, {"CI": "true"}): + self.assertEqual( + agent.enforce_model_policy("mockllm/model"), "mockllm/model" + ) + + def test_nothing_is_pinned_outside_ci(self) -> None: + with mock.patch.dict(os.environ, {"CI": "", "GITHUB_ACTIONS": ""}): + self.assertEqual(agent.enforce_model_policy("sonnet"), "sonnet") + + +class TestEngineMessageLimit(unittest.TestCase): + """A model that cannot finish should not be given a hundred turns to prove it.""" + + def test_a_real_model_gets_the_full_budget(self) -> None: + self.assertEqual( + engine_behavioral.message_limit_for("anthropic/claude-opus-5"), + engine_behavioral.MESSAGE_LIMIT, + ) + + def test_a_mock_gets_a_short_one(self) -> None: + # It never calls submit, so it loops to whatever cap it is given, and + # every turn is a real sandbox round trip. + self.assertEqual( + engine_behavioral.message_limit_for("mockllm/model"), + engine_behavioral.MOCK_MESSAGE_LIMIT, + ) + + +class TestEngineSkillFailureIsContained(unittest.TestCase): + """One skill's broken setup is that skill's failure, not everybody's.""" + + def test_a_skill_that_cannot_run_becomes_failed_outcomes(self) -> None: + cases = [ + datasets.Case(id="a", prompt="p", skill="broken", skill_should_trigger=True), + datasets.Case(id="b", prompt="q", skill="broken", skill_should_trigger=True), + ] + outcomes = engine_behavioral._failed(cases[0].skill, cases, "no compose file") + self.assertEqual([o.id for o in outcomes], ["a", "b"]) + self.assertTrue(all(not o.passed for o in outcomes)) + self.assertTrue(all("no compose file" in (o.error or "") for o in outcomes)) + # Not silence: an unreported skill would let a run that graded nothing + # call itself green. + self.assertTrue(all(o.checks == [] for o in outcomes)) + + +class TestBehavioralEngineDispatch(unittest.TestCase): + """Every engine has a behavioral branch, and reaches its own runner. + + `claude-code-no-sandbox` was once added to the dispatch chain but not to + the guard around it, so it fell through to the engine that no longer + exists: runs asked for one agent, silently got another, and the reports + said `engine: claude-code-no-sandbox` throughout. Nothing in the suite + noticed, because nothing asserted which runner a flag actually reaches. + """ + + def test_every_engine_runs_under_inspect_now(self) -> None: + self.assertEqual(set(cli.INSPECT_ENGINES), set(cli.ENGINES)) + + def test_the_dispatch_reads_that_set_rather_than_a_literal(self) -> None: + # The bug was a branch list that fell behind the choices list, so + # the source is what to assert: every engine named, and an `else` + # that raises rather than quietly picking one. + source = inspect.getsource(cli.cmd_behavioral) + for engine in cli.ENGINES: + with self.subTest(engine=engine): + self.assertIn(f'args.engine == "{engine}"', source) + self.assertIn("has no behavioral leg", source) + + def test_the_preflight_uses_the_same_set(self) -> None: + # These once disagreed: the preflight demanded the inspect extra for + # claude-code-no-sandbox while the dispatch sent it somewhere that + # never used it, which is how a Windows job failed on a flag it had + # not passed. One set now, and the preflight reads it. + self.assertIn("INSPECT_ENGINES", inspect.getsource(cli._prepare_graded_run)) + + +class TestEngineInstallHint(unittest.TestCase): + """The hint has to name the engine the user actually asked for.""" + + def test_it_names_the_requested_engine(self) -> None: + # Naming `inspect` regardless sent a Windows CI job looking for a flag + # it had never passed -- it had asked for claude-code-no-sandbox. + self.assertIn("--engine claude-code-no-sandbox", engine_module.install_hint("claude-code-no-sandbox")) + + def test_it_points_at_a_command_that_would_actually_help(self) -> None: + # It used to say `pip install 'skillscope[inspect]'`. inspect_ai is a + # required dependency now, so that extra is empty and following the + # advice would change nothing -- reaching this error means a broken or + # partial install, and reinstalling is the fix. + hint = engine_module.install_hint("claude-code") + self.assertNotIn("skillscope[inspect]", hint) + self.assertIn("reinstall", hint.lower()) + + +class TestEngineWorkdirPath(unittest.TestCase): + """The working directory has to be knowable before a sandbox exists.""" + + def setUp(self) -> None: + self.addCleanup(os.environ.pop, engine_sandbox.SANDBOX_ENV, None) + + def test_a_container_run_names_the_workdir_without_creating_it(self) -> None: + os.environ[engine_sandbox.SANDBOX_ENV] = "podman" + self.assertEqual(engine_tools.workdir_path(), engine_tools.WORKDIR) + + def test_a_local_run_has_none_so_the_agent_keeps_its_own(self) -> None: + # Creating /workspace on somebody's laptop is not ours to do, and the + # harness's own directory is already where the scorers look. + os.environ[engine_sandbox.SANDBOX_ENV] = "local" + self.assertIsNone(engine_tools.workdir_path()) + + +class TestEngineNoSandboxInstallsSkill(unittest.TestCase): + """A driver that replaces the react agent must stage the skill itself. + + It did not, so the CLI ran with no skill and answered from the prompt + alone -- scoring 4/21 where every other engine scored 21/21, while + finishing faster, which was the only visible sign. + """ + + def test_the_skill_lands_where_the_harness_looks(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + src = Path(tmp) / "my-skill" + (src / "scripts").mkdir(parents=True) + (src / "SKILL.md").write_text("# my-skill", encoding="utf-8") + (src / "scripts" / "validate.py").write_text("x = 1", encoding="utf-8") + workspace = Path(tmp) / "ws" + workspace.mkdir() + + engine_no_sandbox.install_skill(src, str(workspace)) + + staged = workspace / ".claude" / "skills" / "my-skill" + self.assertTrue((staged / "SKILL.md").is_file()) + # The whole tree, not just the manifest: skills ship validators and + # references the agent is expected to run. + self.assertTrue((staged / "scripts" / "validate.py").is_file()) + + +class TestTaskTimeLimit(unittest.TestCase): + """A run that overruns should still say where it got to.""" + + def test_inspect_stops_before_the_hard_deadline_does(self) -> None: + bound = deadline.Deadline(1800, command="behavioral") + limit = engine_behavioral.task_time_limit(bound) + self.assertLess(limit, bound.remaining()) + # Enough room for inspect to score what exists and write the log. + self.assertGreaterEqual(bound.remaining() - limit, 60) + + def test_a_tiny_budget_still_gets_a_usable_limit(self) -> None: + # Never negative, never zero: a nonsense limit would fail the sample + # instantly and look like the agent doing nothing. + self.assertGreaterEqual(engine_behavioral.task_time_limit(deadline.Deadline(5)), 60) + + def test_no_deadline_means_no_limit(self) -> None: + self.assertIsNone(engine_behavioral.task_time_limit(None)) + + +class TestRealtimeLogging(unittest.TestCase): + """The live sample buffer is what MAX_PATH kills on Windows.""" + + def test_windows_runs_without_the_buffer(self) -> None: + with mock.patch.object(engine_behavioral.sys, "platform", "win32"): + self.assertFalse(engine_behavioral.realtime_logging()) + + def test_posix_keeps_it(self) -> None: + with mock.patch.object(engine_behavioral.sys, "platform", "linux"): + self.assertTrue(engine_behavioral.realtime_logging()) + + +class TestShellPrefixProbe(unittest.TestCase): + """A guest without bash raises rather than answering.""" + + def _probe(self, exc: Exception | None): + from skillscope.engine import tools as t + + class _Result: + success = True + + class _Sandbox: + async def exec(self, *a, **k): + if exc is not None: + raise exc + return _Result() + + store: dict = {} + + class _Store: + def get(self, k, default=None): + return store.get(k, default) + + def set(self, k, v): + store[k] = v + + with mock.patch.dict( + sys.modules, + {"inspect_ai.util": mock.MagicMock(sandbox=lambda: _Sandbox(), store=_Store)}, + ): + return asyncio.run(t.shell_prefix()) + + def test_a_missing_bash_selects_powershell_rather_than_failing(self) -> None: + # WinError 2 here took the whole task down and reported 0/0 + # expectations, which reads as the harness being broken. + self.assertEqual( + self._probe(FileNotFoundError(2, "The system cannot find the file specified")), + engine_tools.WINDOWS_SHELL, + ) + + def test_a_working_bash_still_selects_posix(self) -> None: + self.assertEqual(self._probe(None), engine_tools.POSIX_SHELL) + + +class TestClaudeCodeRefusesTheHostsFilesystem(unittest.TestCase): + """`claude-code` needs a container, and not for isolation's sake. + + `inspect_swe` prepares the guest by writing `$HOME/.claude/settings.json` + outright. In a container that file belongs to nobody. Under the `local` + provider `$HOME` is the developer's own, and the same write silently + destroys their real configuration -- permissions, model, gateway + environment -- with no backup. This cost one settings.json before the + guard existed, which is why the guard is a refusal rather than a warning. + """ + + def setUp(self) -> None: + self.addCleanup(os.environ.pop, engine_sandbox.SANDBOX_ENV, None) + os.environ.pop(engine_sandbox.SANDBOX_ENV, None) + # Two different levers, and this class is about the second one. + # `provider()` asks `is_windows()`; `require()` reads `sys.platform` + # itself and reads it first. Patching only the former left these + # tests measuring the platform guard on a Windows runner and the + # sandbox guard on a Linux one, under the same names. + for target, attr, value in ( + (engine_sandbox, "is_windows", lambda: False), + (engine_verify.sys, "platform", "linux"), + ): + patch = mock.patch.object(target, attr, value) + patch.start() + self.addCleanup(patch.stop) + + def test_a_host_sharing_provider_is_refused(self) -> None: + os.environ[engine_sandbox.SANDBOX_ENV] = "local" + with self.assertRaises(SystemExit) as caught: + engine_verify.require() + message = str(caught.exception) + self.assertIn("settings.json", message) + self.assertIn("--engine claude-code-no-sandbox", message) + + def test_every_unisolated_provider_is_refused(self) -> None: + # Keyed off the same set `describe()` reports from, so a provider that + # is added as unisolated cannot quietly stay allowed here. + for provider in engine_sandbox.NOT_ISOLATED: + with self.subTest(provider=provider): + os.environ[engine_sandbox.SANDBOX_ENV] = provider + with self.assertRaises(SystemExit): + engine_verify.require() + + def test_a_container_provider_is_not_refused_for_being_a_container(self) -> None: + # Asserted as "not this refusal" rather than "no refusal at all": the + # unit suite runs without the inspect extra on purpose, so `require` + # may still stop on the missing wheel. That is a different answer to a + # different question, and conflating them made this pass locally and + # fail in CI. + os.environ[engine_sandbox.SANDBOX_ENV] = "docker" + try: + engine_verify.require() + except SystemExit as exc: + # Whatever it stopped on, it was not the sandbox. + self.assertNotIn("settings.json", str(exc)) + self.assertIn("pip install", str(exc)) + + +class TestEngineNoSandboxGuard(unittest.TestCase): + """The CLI runs on the host, so the sandbox has to be the host.""" + + def setUp(self) -> None: + self.addCleanup(os.environ.pop, engine_sandbox.SANDBOX_ENV, None) + + def test_a_container_provider_is_refused_with_the_alternative(self) -> None: + # Otherwise the CLI would work in the host's filesystem while the + # scorers read a container, and every expectation would fail for a + # reason nothing in the report explains. + os.environ[engine_sandbox.SANDBOX_ENV] = "podman" + with self.assertRaises(SystemExit) as caught: + engine_no_sandbox.require_local() + message = str(caught.exception) + self.assertIn("podman", message) + self.assertIn("--engine claude-code", message) + + +class TestEngineModelNames(unittest.TestCase): + """`--model` speaks the claude CLI's aliases; inspect wants provider names.""" + + def test_an_alias_becomes_a_provider_qualified_name(self) -> None: + self.assertEqual(engine_models.resolve("opus"), "anthropic/claude-opus-5") + + def test_an_alias_is_case_insensitive(self) -> None: + self.assertEqual(engine_models.resolve("Opus"), "anthropic/claude-opus-5") + + def test_a_qualified_name_passes_through(self) -> None: + # What makes `--model mockllm/model` work for the no-cost wiring runs. + self.assertEqual(engine_models.resolve("mockllm/model"), "mockllm/model") + + def test_an_unknown_bare_name_is_assumed_to_be_anthropic(self) -> None: + self.assertEqual(engine_models.resolve("claude-x"), "anthropic/claude-x") + + +class TestEngineGatewayHeaders(unittest.TestCase): + """`ANTHROPIC_CUSTOM_HEADERS` is a claude CLI variable; inspect ignores it.""" + + def setUp(self) -> None: + for var in (engine_models.CUSTOM_HEADERS_ENV, engine_models.AUTH_TOKEN_ENV): + self.addCleanup(os.environ.pop, var, None) + os.environ.pop(var, None) + + def test_no_headers_configured_means_no_provider_arguments(self) -> None: + self.assertEqual(engine_models.model_args("anthropic/claude-opus-5"), {}) + + def test_headers_are_parsed_into_default_headers(self) -> None: + os.environ[engine_models.CUSTOM_HEADERS_ENV] = ( + "X-Subscription-Key: secret\nuser: ci-runner\n" + ) + self.assertEqual( + engine_models.model_args("anthropic/claude-opus-5"), + { + "default_headers": { + "X-Subscription-Key": "secret", + "user": "ci-runner", + } + }, + ) + + def test_a_value_containing_a_colon_survives(self) -> None: + os.environ[engine_models.CUSTOM_HEADERS_ENV] = "Referer: https://example.com/x" + self.assertEqual( + engine_models.custom_headers(), {"Referer": "https://example.com/x"} + ) + + def test_blank_and_malformed_lines_are_skipped(self) -> None: + os.environ[engine_models.CUSTOM_HEADERS_ENV] = "\nnot-a-header\n\nk: v\n" + self.assertEqual(engine_models.custom_headers(), {"k": "v"}) + + def test_a_non_anthropic_model_needs_no_gateway_arguments(self) -> None: + # The free wiring run reaches no provider, so a shell that happens to + # hold both Anthropic variables must not break the one check that costs + # nothing -- and those are exactly the machines that have an OAuth token. + os.environ[engine_models.CUSTOM_HEADERS_ENV] = "k: v" + os.environ[engine_models.AUTH_TOKEN_ENV] = "token" + self.assertEqual(engine_models.model_args("mockllm/model"), {}) + + def test_oauth_and_gateway_headers_together_are_refused(self) -> None: + # inspect's OAuth path sets `default_headers` itself, so ours would be a + # duplicate keyword argument deep inside the SDK. Fail with the reason. + os.environ[engine_models.CUSTOM_HEADERS_ENV] = "k: v" + os.environ[engine_models.AUTH_TOKEN_ENV] = "token" + with self.assertRaises(SystemExit) as caught: + engine_models.model_args("anthropic/claude-opus-5") + self.assertIn(engine_models.AUTH_TOKEN_ENV, str(caught.exception)) + + +class TestEngineListingNormalisation(unittest.TestCase): + """`find` and `Get-ChildItem` disagree about separators and prefixes.""" + + def test_posix_output(self) -> None: + listing = "./out.png\n./docs/plan.md\n" + self.assertEqual( + engine_tools.normalize_listing(listing), ["docs/plan.md", "out.png"] + ) + + def test_windows_output(self) -> None: + listing = ".\\out.png\r\n.\\docs\\plan.md\r\n" + self.assertEqual( + engine_tools.normalize_listing(listing), ["docs/plan.md", "out.png"] + ) + + def test_the_installed_skill_does_not_satisfy_files_exist(self) -> None: + # The harness put it there, so a case asserting SKILL.md was produced + # would otherwise pass without the agent doing anything. + listing = "./skills/demo/SKILL.md\n./.claude/settings.json\n./out.png\n" + self.assertEqual(engine_tools.normalize_listing(listing), ["out.png"]) + + def test_blank_lines_are_dropped(self) -> None: + self.assertEqual(engine_tools.normalize_listing("\n\n \n"), []) + + +class TestEngineJudgeVerdicts(unittest.TestCase): + """A grader is chatty and its reasons contain punctuation.""" + + def test_a_bare_verdict(self) -> None: + self.assertEqual( + engine_judge.parse_verdict('{"pass": true, "reason": "it did"}'), + (True, "it did"), + ) + + def test_a_verdict_wrapped_in_prose(self) -> None: + text = 'Looking at the evidence...\n{"pass": false, "reason": "no file"}\nDone.' + self.assertEqual(engine_judge.parse_verdict(text), (False, "no file")) + + def test_a_reason_containing_braces(self) -> None: + # A regex quantifier or a quoted snippet in the reason must not confuse + # the scan, which is why boundaries are decoded rather than matched. + text = '{"pass": true, "reason": "matched a{2,3} in the output"}' + self.assertEqual( + engine_judge.parse_verdict(text), (True, "matched a{2,3} in the output") + ) + + def test_the_last_verdict_wins(self) -> None: + text = '{"pass": true, "reason": "first"}\n{"pass": false, "reason": "second"}' + self.assertEqual(engine_judge.parse_verdict(text), (False, "second")) + + def test_no_verdict_at_all(self) -> None: + self.assertIsNone(engine_judge.parse_verdict("I could not decide.")) + + def test_a_missing_reason_still_yields_a_verdict(self) -> None: + self.assertEqual( + engine_judge.parse_verdict('{"pass": true}'), (True, "(no reason given)") + ) + + +class TestEngineJudgePolarity(unittest.TestCase): + """The judge grades the requirement; callers must never negate the verdict.""" + + def test_a_must_requirement_asks_whether_it_happened(self) -> None: + text = engine_judge.requirement_text("generate an image", must_happen=True) + self.assertIn("MUST have done this", text) + self.assertIn("true if the agent did it", text) + + def test_a_must_not_requirement_asks_whether_it_was_avoided(self) -> None: + # Read as a pass when the agent avoided it: negating this verdict is + # what turns a correct run into a failure. + text = engine_judge.requirement_text("call a cloud API", must_happen=False) + self.assertIn("MUST NOT have done this", text) + self.assertIn("true if the agent avoided it", text) + self.assertIn("default verdict is true", text) + + +class TestEngineJudgeTruncation(unittest.TestCase): + """What settles a check is usually the last thing the agent did.""" + + def test_short_transcripts_are_untouched(self) -> None: + self.assertEqual(engine_judge._elide_middle("abc", 100), "abc") + + def test_the_end_survives(self) -> None: + # Cutting the tail would drop the validator run that a "did it verify + # its work" expectation turns on, making the agent look like it lied. + text = "START" + ("x" * 5000) + "VALIDATED" + trimmed = engine_judge._elide_middle(text, 400) + self.assertTrue(trimmed.startswith("START")) + self.assertTrue(trimmed.endswith("VALIDATED")) + self.assertIn("elided", trimmed) + self.assertLess(len(trimmed), 600) + + +class _State: + def __init__(self, messages, output=None) -> None: + self.messages = messages + self.output = output + + +class _Output: + def __init__(self, completion: str) -> None: + self.completion = completion + + +class _Assistant: + role = "assistant" + + def __init__(self, content: str) -> None: + self.content = content + + +class TestEngineJudgeFinalMessage(unittest.TestCase): + """A `react` agent answers through submit, not through a chat message.""" + + def test_the_submitted_answer_is_included_and_marked(self) -> None: + state = _State( + [_Assistant("Here are the commands you need:")], + _Output("curl -X POST /api/v1/pull -d '{...}'"), + ) + said = engine_judge.final_message_of(state) + self.assertIn("curl -X POST", said) + self.assertIn("[submitted answer]", said) + + def test_an_earlier_turn_still_counts_as_having_told_the_user(self) -> None: + # The user sees every assistant turn, so an agent that prints the + # commands mid-run and then submits a summary did tell them. Reading + # only the last turn credited the summary and called the commands + # missing. + state = _State( + [ + _Assistant("Run: curl -X POST /api/v1/pull"), + _Assistant("Done -- commands delivered above."), + ], + _Output("Done -- commands delivered above."), + ) + self.assertIn("curl -X POST", engine_judge.final_message_of(state)) + + def test_it_works_without_a_submit_tool(self) -> None: + state = _State([_Assistant("no submit tool in this agent")], None) + self.assertEqual( + engine_judge.final_message_of(state), "no submit tool in this agent" + ) + + def test_silence_is_reported_rather_than_guessed_at(self) -> None: + self.assertEqual( + engine_judge.final_message_of(_State([], None)), "(the agent said nothing)" + ) + + +class TestEngineJudgeArtifacts(unittest.TestCase): + def test_images_are_recognised_by_suffix(self) -> None: + self.assertTrue(engine_judge.is_image("out.PNG")) + self.assertTrue(engine_judge.is_image("art/cat.jpeg")) + self.assertFalse(engine_judge.is_image("notes.md")) + + def test_known_binaries_are_not_read_as_text(self) -> None: + self.assertTrue(engine_judge.is_probably_binary("model.safetensors")) + self.assertFalse(engine_judge.is_probably_binary("report.md")) + + +class TestEngineSandboxSelection(unittest.TestCase): + """The provider is the machine's choice; the compose file is the skill's.""" + + def setUp(self) -> None: + self.addCleanup(os.environ.pop, engine_sandbox.SANDBOX_ENV, None) + os.environ.pop(engine_sandbox.SANDBOX_ENV, None) + self.repo = Repo(self) + # Pinned, because the answer depends on the platform and the suite runs + # on both. Without this these assertions quietly mean something + # different on a Windows runner than on a Linux one. + self._posix_host() + + def _posix_host(self) -> None: + patch = mock.patch.object(engine_sandbox, "is_windows", lambda: False) + patch.start() + self.addCleanup(patch.stop) + + def _windows_host(self) -> None: + patch = mock.patch.object(engine_sandbox, "is_windows", lambda: True) + patch.start() + self.addCleanup(patch.stop) + + def _skill(self, machine: str | None = None, compose: bool = False) -> None: + folder = self.repo.skill( + "boxed", dataset=tier0_dataset("boxed"), machine=machine + ) + if compose: + # Beside machine.yml, not at the skill root: the skill root is what + # gets published, and eval infrastructure does not belong there. + (folder / "evals" / "compose.yaml").write_text( + "services: {}\n", encoding="utf-8" + ) + self.repo.activate() + + def test_docker_by_default(self) -> None: + self._skill() + self.assertEqual(engine_sandbox.for_skill("boxed"), "docker") + + def test_the_env_var_selects_the_provider(self) -> None: + self._skill() + os.environ[engine_sandbox.SANDBOX_ENV] = "podman" + self.assertEqual(engine_sandbox.for_skill("boxed"), "podman") + + def test_a_declared_compose_file_rides_along(self) -> None: + self._skill(machine="sandbox: compose.yaml\n", compose=True) + provider, config = engine_sandbox.for_skill("boxed") + self.assertEqual(provider, "docker") + self.assertTrue(config.endswith("compose.yaml")) + + def test_selecting_a_provider_keeps_the_skill_s_compose_file(self) -> None: + # The skill asked for network egress; choosing podman must not drop it, + # or the case runs without what it needs and fails unexplainably. + self._skill(machine="sandbox: compose.yaml\n", compose=True) + os.environ[engine_sandbox.SANDBOX_ENV] = "podman" + provider, config = engine_sandbox.for_skill("boxed") + self.assertEqual(provider, "podman") + self.assertTrue(config.endswith("compose.yaml")) + + def test_local_takes_no_configuration(self) -> None: + self._skill(machine="sandbox: compose.yaml\n", compose=True) + os.environ[engine_sandbox.SANDBOX_ENV] = "local" + self.assertEqual(engine_sandbox.for_skill("boxed"), "local") + + def test_windows_has_no_sandbox_available(self) -> None: + # inspect's sandbox layer assumes a POSIX guest, so those legs run + # unsandboxed -- and a compose file the skill declared cannot apply, + # because there is no container to apply it to. + self._windows_host() + self._skill(machine="sandbox: compose.yaml\n", compose=True) + self.assertEqual(engine_sandbox.for_skill("boxed"), "local") + + def test_an_unresolvable_provider_says_what_to_install(self) -> None: + # The binary being present proves nothing: inspect resolves a + # third-party provider through an entry point, so the Python package + # has to be installed too. Its own error names neither the variable + # nor the package. + os.environ[engine_sandbox.SANDBOX_ENV] = "podman" + + def unresolvable(name: str): + raise ValueError(f"SandboxEnvironment type {name!r} not recognized.") + + with self.assertRaises(SystemExit) as caught: + engine_sandbox.require_provider(resolve=unresolvable) + message = str(caught.exception) + self.assertIn(engine_sandbox.SANDBOX_ENV, message) + self.assertIn("skillscope[podman]", message) + + def test_a_named_compose_file_that_is_missing_is_an_error(self) -> None: + self._skill(machine="sandbox: nope.yaml\n") + with self.assertRaises(SystemExit) as caught: + engine_sandbox.for_skill("boxed") + self.assertIn("nope.yaml", str(caught.exception)) + + +class TestRoutingRunsOnEveryEngine(unittest.TestCase): + """Routing reaches the leg it was asked for, for every engine there is. + + It used to have one leg, and `claude-code` / `claude-code-no-sandbox` were + refused because routing had no path that drove the real CLI. Now both do, + and `legacy` -- the leg they were refused in favour of -- is gone. The + assertion that matters is that asking for a leg reaches that leg: the + defect this replaces was a run asking for one engine, silently getting + another, and the report naming the one it had asked for. + """ + + def setUp(self) -> None: + self.repo = Repo(self) + self.repo.skill("alpha", dataset=tier0_dataset("alpha")) + self.repo.activate(routing_room="alpha") + self.reached: list[str] = [] + + def record_inspect(cases, routing_set, model, effort, engine, **kwargs): + # The cap is recorded, not merely tolerated: a leg that never + # receives it bounds a case by the whole command's budget while the + # report claims otherwise. + self.reached.append(f"inspect:{engine}") + self.passed_kwargs = kwargs + return [] + + patch = mock.patch("skillscope.engine.routing.run", record_inspect) + patch.start() + self.addCleanup(patch.stop) + + for name in ("_write_report", "_prepare_graded_run"): + patch = mock.patch.object(cli, name, lambda *a, **k: None) + patch.start() + self.addCleanup(patch.stop) + + def run_routing(self, engine: str) -> None: + args = cli.build_parser().parse_args( + ["routing", "--engine", engine, "--skip-preflight", "--model", "mockllm/model"] + ) + cli.cmd_routing(args) + + def test_every_engine_now_has_a_routing_leg(self) -> None: + self.assertEqual(set(cli.ROUTING_ENGINES), set(cli.ENGINES)) + + def test_asking_for_the_sandboxed_leg_reaches_it(self) -> None: + self.run_routing("claude-code") + self.assertEqual(self.reached, ["inspect:claude-code"]) + + def test_asking_for_the_host_leg_reaches_it(self) -> None: + # The case that used to fall through: `claude-code-no-sandbox` was in + # the choices list but not in the dispatch guard, so it silently ran + # somewhere else while the report named what had been asked for. + self.run_routing("claude-code-no-sandbox") + self.assertEqual(self.reached, ["inspect:claude-code-no-sandbox"]) + + def test_the_leg_is_given_the_per_case_cap(self) -> None: + self.run_routing("claude-code") + self.assertIn("case_timeout", self.passed_kwargs) + + def test_the_dispatch_distinguishes_the_legs(self) -> None: + # Routing branches on the sandboxed leg and lets the host leg fall to + # the else, so there is no per-engine name to assert here as there is + # in `cmd_behavioral`. What must hold is that it branches at all: a + # dispatch that treated both alike would report a containment one of + # them never had. + source = inspect.getsource(cli.cmd_routing) + self.assertIn('args.engine == "claude-code"', source) + self.assertIn("sandbox_isolated", source) + + def test_the_removed_engines_are_refused_by_the_parser(self) -> None: + for engine in ("inspect", "legacy"): + with self.subTest(engine=engine): + self.assertNotIn(engine, cli.ENGINES) + with self.assertRaises(SystemExit), contextlib.redirect_stderr( + io.StringIO() + ): + cli.build_parser().parse_args(["routing", "--engine", engine]) + + + + +class TestTheTranscriptIsReadTooNotJustTheMessages(unittest.TestCase): + """A bridged agent's work does not always land in `sample.messages`. + + inspect adopts one conversation onto the sample, and for a bridged scaffold + that adoption follows heuristics about which thread is the main one -- a + run ending inside a sub-agent can leave it holding the wrong thread or + none. The transcript is strictly more complete: every bridged generation + emits a `ModelEvent`, sub-agents included. + + Measured rather than reasoned about. The first sandboxed routing run on a + real container graded 24 of 67 cases as "the agent never ran", while the + legacy engine saw those same cases activate a skill. Reading only + `sample.messages` was the whole of the bug. + """ + + class Call: + def __init__(self, id, function, arguments): + self.id, self.function, self.arguments = id, function, arguments + + class Msg: + def __init__(self, role="assistant", tool_calls=None, id=None): + self.role, self.tool_calls, self.id = role, tool_calls, id + + class Event: + def __init__(self, message): + self.output = type("O", (), {"message": message})() + + class Sample: + def __init__(self, messages=None, events=None): + self.messages, self.events = messages, events + + ROOM = ["alpha", "beta"] + + def skill_call(self): + return self.Call("t0", "Skill", {"command": "alpha"}) + + def test_an_activation_only_in_the_messages_is_seen(self) -> None: + sample = self.Sample(messages=[self.Msg(tool_calls=[self.skill_call()])]) + self.assertEqual(engine_routing._observe(sample, self.ROOM)[0], "alpha") + + def test_an_activation_only_in_the_transcript_is_seen(self) -> None: + # The case that was being missed. + sample = self.Sample(events=[self.Event(self.Msg(tool_calls=[self.skill_call()]))]) + self.assertEqual(engine_routing._observe(sample, self.ROOM)[0], "alpha") + + def test_the_same_turn_as_two_objects_is_counted_once(self) -> None: + # What the bridge actually produces: the turn adopted onto the sample + # and the turn carried by the transcript event are different objects + # with the same id. De-duplicating by identity missed that and counted + # every call twice -- a sandboxed run reported ten tool calls for + # cases the approver had terminated at five, halving the effective + # budget. + calls = [self.Call("t1", "Bash", {"command": "ls"})] + adopted = self.Msg(tool_calls=calls) + adopted.id = "m1" + in_event = self.Msg(tool_calls=calls) + in_event.id = "m1" + sample = self.Sample(messages=[adopted], events=[self.Event(in_event)]) + self.assertEqual(engine_routing._observe(sample, self.ROOM)[1], 1) + + def test_two_genuinely_different_turns_are_both_counted(self) -> None: + first = self.Msg(tool_calls=[self.Call("t1", "Bash", {"command": "ls"})]) + first.id = "m1" + second = self.Msg(tool_calls=[self.Call("t2", "Bash", {"command": "pwd"})]) + second.id = "m2" + sample = self.Sample(messages=[first, second]) + self.assertEqual(engine_routing._observe(sample, self.ROOM)[1], 2) + + def test_a_run_that_chose_nothing_is_still_a_run(self) -> None: + sample = self.Sample(messages=[self.Msg(tool_calls=[])]) + self.assertIsNone(engine_routing._observe(sample, self.ROOM)[0]) + self.assertTrue(engine_routing._spoke(sample)) + + class Limit: + def __init__(self, type): + self.type = type + + def test_a_sample_that_hit_a_limit_counts_as_having_run(self) -> None: + # When the message cap trips, what survives on the sample can be the + # prompt and nothing else. Calling that "never ran" is wrong and the + # opposite of useful: an agent that spent its whole budget without + # reaching for a skill is the clearest kind of missed trigger, which + # is how legacy grades its own `tool_budget` stop. + sample = self.Sample(messages=[self.Msg(role="user")]) + sample.limit = self.Limit("message") + self.assertTrue(engine_routing._spoke(sample)) + + def test_a_prompt_with_no_limit_and_no_reply_still_never_ran(self) -> None: + sample = self.Sample(messages=[self.Msg(role="user")]) + self.assertFalse(engine_routing._spoke(sample)) + + def test_a_sample_with_neither_record_never_ran(self) -> None: + # Preserved: this is the infrastructure failure the guard exists for, + # and grading it as a miss would invent a routing result. + self.assertFalse(engine_routing._spoke(self.Sample())) + + def test_the_transcript_alone_counts_as_having_run(self) -> None: + sample = self.Sample(events=[self.Event(self.Msg(tool_calls=[]))]) + self.assertTrue(engine_routing._spoke(sample)) + + +class TestAnEmptyAnswerIsStillAnAnswer(unittest.TestCase): + """A run that finished saying nothing is a routing result, not a failure. + + The routing mapper has to tell "the agent reached for no skill" from "the + agent never ran", and it draws that line at whether the sample produced any + messages. The host driver appended a closing message only when the CLI had + something to say, so a run that finished quietly left none -- and two cases + on the real runner were graded as infrastructure failures where the legacy + engine graded them `correct_trigger` and `true_negative`. + + The distinction is still drawn, just in the right place: a stream carrying + no result event at all is a CLI that never finished. + + Exercised through `run_completed`, which is pure. The unit suite runs + without the inspect extra, and a rule reachable only through inspect's + message objects would go untested in CI -- which is where it matters. + """ + + def test_a_run_that_finished_with_an_answer(self) -> None: + done, final = engine_no_sandbox.run_completed( + [{"type": "result", "result": "no skill needed"}] + ) + self.assertTrue(done) + self.assertEqual(final, "no skill needed") + + def test_a_run_that_finished_saying_nothing_still_counts_as_finished(self) -> None: + # The case this fixes. An empty answer is the agent declining to route. + done, final = engine_no_sandbox.run_completed([{"type": "result", "result": ""}]) + self.assertTrue(done) + self.assertEqual(final, "") + + def test_a_result_event_with_no_text_at_all_still_counts(self) -> None: + self.assertTrue(engine_no_sandbox.run_completed([{"type": "result"}])[0]) + + def test_a_stream_that_never_finished_does_not_count(self) -> None: + # Preserved deliberately: no result event means the CLI did not reach + # the end, which is an infrastructure failure and must stay one. + self.assertFalse( + engine_no_sandbox.run_completed( + [{"type": "assistant", "message": {"content": []}}] + )[0] + ) + + def test_the_last_result_event_wins(self) -> None: + _, final = engine_no_sandbox.run_completed( + [{"type": "result", "result": "first"}, {"type": "result", "result": "last"}] + ) + self.assertEqual(final, "last") + + def test_the_placeholder_matches_what_the_output_already_used(self) -> None: + # The message list was the inconsistent half; `state.output` has always + # substituted this for an empty answer. + source = inspect.getsource(engine_no_sandbox) + self.assertEqual(source.count('"(no final message)"'), 2) + + +class TestTheRoomHoldsTheSkillNotItsTests(unittest.TestCase): + """`evals/` is the answer key, and it was in the room on one leg. + + `evals/evals.json` pairs each prompt with `skill_should_trigger` -- the + routing answer -- and with the `expected_behavior` a behavioral case is + graded against. The host leg copied the skill directory wholesale, so that + file sat inside the room the agent was being asked to choose from. The + sandboxed leg never had it, which meant the two legs whose agreement the + legacy retirement rests on were choosing from different rooms. + + No agent was ever observed opening it -- every case in a 67-case run was + checked. That is why this is a removal plus a detector rather than a + removal alone: "nobody reads it" is a belief, and the detector is what + would notice if it stopped being true. + """ + + def staged(self) -> Path: + root = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, root, ignore_errors=True) + skill = root / "src" / "demo" + (skill / "evals" / "files").mkdir(parents=True) + (skill / "scripts").mkdir() + (skill / "SKILL.md").write_text("---\nname: demo\n---\n") + (skill / "reference.md").write_text("details") + (skill / "scripts" / "run.py").write_text("print()") + (skill / "evals" / "evals.json").write_text('{"evaluations": []}') + (skill / "evals" / "files" / "seed.txt").write_text("fixture") + workspace = root / "ws" + workspace.mkdir() + engine_no_sandbox.install_skill(skill, str(workspace)) + return workspace / ".claude" / "skills" / "demo" + + def test_the_answer_key_is_not_staged(self) -> None: + self.assertFalse((self.staged() / "evals").exists()) + + def test_the_skill_itself_still_is(self) -> None: + room = self.staged() + for rel in ("SKILL.md", "reference.md", "scripts/run.py"): + with self.subTest(rel=rel): + self.assertTrue((room / rel).is_file(), rel) + + def test_both_legs_now_exclude_the_same_directory(self) -> None: + # The asymmetry was the point: one leg had it, the other never did. + self.assertEqual( + engine_no_sandbox.EVAL_FIXTURE_DIRNAME, engine_verify.EVAL_FIXTURE_DIR + ) + + +class TestReadingTheAnswerKeyIsReported(unittest.TestCase): + """Removing the file is the fix; noticing a read is how we would know.""" + + def sample(self, *arguments: dict): + calls = [_FakeCall(f"t{i}") for i, _ in enumerate(arguments)] + for call, args in zip(calls, arguments): + call.arguments = args + return _FakeSample([_FakeMessage("m", calls)], []) + + def test_a_case_that_opened_the_dataset_is_flagged(self) -> None: + sample = self.sample({"file_path": ".claude/skills/x/evals/evals.json"}) + self.assertTrue(engine_routing.read_the_answer_key(sample)) + + def test_the_extended_dataset_counts_too(self) -> None: + sample = self.sample({"command": "cat evals/extended_evals.json"}) + self.assertTrue(engine_routing.read_the_answer_key(sample)) + + def test_ordinary_work_is_not_flagged(self) -> None: + sample = self.sample( + {"command": "ls /workspace"}, {"file_path": "reference.md"} + ) + self.assertFalse(engine_routing.read_the_answer_key(sample)) + + def test_it_matches_wherever_the_copy_came_from(self) -> None: + # Matched on the filename, because the route by which a copy reaches + # the agent is exactly what is not known in advance. + sample = self.sample({"command": "cat /some/other/checkout/evals.json"}) + self.assertTrue(engine_routing.read_the_answer_key(sample)) + + +class TestTheDefaultsAgreeWithEachOther(unittest.TestCase): + """A default engine that fails under a default sandbox is not a default. + + `claude-code-no-sandbox` became the default when `legacy` was retired, and + it required `SKILLSCOPE_SANDBOX=local` while the default provider is + `docker`. So a fresh install running `skillscope routing` stopped with + "needs SKILLSCOPE_SANDBOX=local, not 'docker'" -- the two defaults + contradicted each other, and every downstream repo that upgraded would + have met it, because the reusable workflow names no engine and takes + whatever the default is. + + Found by running the default engine the way a product repo would, which is + the only way it could have been found: every test set the variable. + """ + + def setUp(self) -> None: + for name in ("SKILLSCOPE_SANDBOX",): + self.addCleanup(os.environ.pop, name, None) + os.environ.pop("SKILLSCOPE_SANDBOX", None) + patch = mock.patch("shutil.which", lambda _: "/usr/bin/claude") + patch.start(); self.addCleanup(patch.stop) + + def test_nobody_asked_so_the_host_leg_settles_on_local(self) -> None: + engine_no_sandbox.require_local() + self.assertEqual(os.environ["SKILLSCOPE_SANDBOX"], "local") + + def test_the_report_then_says_host_not_the_default_provider(self) -> None: + # The failure this avoids is worse than refusing: a report naming + # `docker` for a run that happened on the host filesystem. + engine_no_sandbox.require_local() + self.assertEqual( + engine_sandbox.describe(), + {"sandbox": "local", "sandbox_isolated": False}, + ) + + def test_an_explicit_container_request_is_still_refused(self) -> None: + # Overriding it would answer a different question than the one put. + os.environ["SKILLSCOPE_SANDBOX"] = "docker" + with self.assertRaises(SystemExit) as caught: + engine_no_sandbox.require_local() + self.assertIn("docker", str(caught.exception)) + + def test_an_explicit_local_request_is_honoured(self) -> None: + os.environ["SKILLSCOPE_SANDBOX"] = "local" + engine_no_sandbox.require_local() + self.assertEqual(os.environ["SKILLSCOPE_SANDBOX"], "local") + + def test_requested_tells_a_choice_from_a_default(self) -> None: + self.assertIsNone(engine_sandbox.requested()) + os.environ["SKILLSCOPE_SANDBOX"] = "podman" + self.assertEqual(engine_sandbox.requested(), "podman") + + +class TestAFederatedTokenCountsAsACredential(unittest.TestCase): + """Workload identity federation holds no API key, by design. + + The host routing leg refuses when it cannot redirect the CLI away from the + runner's own config dir, and that turns on whether auth lives in the + environment. It tested for `ANTHROPIC_API_KEY` alone -- so every runner + authenticating by federation lost its routing leg, including the one the + reusable workflow offers downstream repos through `federation_rule_id`, + where `scoped_api_key_secret` is empty on purpose. + """ + + def setUp(self) -> None: + for name in ("ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN"): + self.addCleanup(os.environ.pop, name, None) + os.environ.pop(name, None) + + def test_an_api_key_still_counts(self) -> None: + os.environ["ANTHROPIC_API_KEY"] = "sk-x" + self.assertTrue(routing.can_isolate_config()) + + def test_a_federated_token_counts_too(self) -> None: + os.environ["ANTHROPIC_AUTH_TOKEN"] = "oat-x" + self.assertTrue(routing.can_isolate_config()) + + def test_neither_is_still_a_refusal(self) -> None: + self.assertFalse(routing.can_isolate_config()) + with self.assertRaises(SystemExit): + engine_routing.require_isolated_room("claude-code-no-sandbox") + + def test_blank_does_not_count_as_set(self) -> None: + os.environ["ANTHROPIC_AUTH_TOKEN"] = " " + self.assertFalse(routing.can_isolate_config()) + + def test_the_refusal_names_both_credentials(self) -> None: + # Naming one sends a federated runner looking for a key it will never + # have. + with self.assertRaises(SystemExit) as caught: + engine_routing.require_isolated_room("claude-code-no-sandbox") + message = str(caught.exception) + for name in routing.ENV_CREDENTIALS: + self.assertIn(name, message) + + +class TestTheFederatedTokenSaysHowLongItLasts(unittest.TestCase): + """The exchange knows the lifetime. It used to throw it away. + + A graded run on an MI300X spent twenty minutes and failed with `401 OAuth + access token has expired`, forty-eight times, and nothing in the job said + the token had ever had a deadline. Measured afterwards from the transcript: + minted 14:01:41, first 401 at 14:14:20 -- about twelve and a half minutes, + a number that was in the exchange response all along. + """ + + def exchange(self, payload: dict) -> str: + import json as _json + + captured: list[str] = [] + with mock.patch("builtins.print", lambda *a, **k: captured.append(" ".join(map(str, a)))): + credentials.anthropic_access_token( + "assertion", + rule_id="fdrl_x", + organization_id="org", + service_account_id="svc", + fetch=lambda *a, **k: _json.dumps(payload).encode(), + ) + return "\n".join(captured) + + def test_it_reports_the_lifetime_it_was_given(self) -> None: + out = self.exchange({"access_token": "t", "expires_in": 750}) + self.assertIn("12m 30s", out) + self.assertIn("401", out) # says what failure to expect + + def test_it_stays_quiet_when_the_exchange_does_not_say(self) -> None: + self.assertEqual(self.exchange({"access_token": "t"}), "") + + def test_a_missing_token_is_still_the_louder_failure(self) -> None: + import json as _json + + with self.assertRaises(credentials.CredentialError): + credentials.anthropic_access_token( + "assertion", rule_id="r", organization_id="o", + service_account_id="s", + fetch=lambda *a, **k: _json.dumps({"expires_in": 750}).encode(), + ) + + +class TestWhichHookEntryPointsSurvive(unittest.TestCase): + """The rule that decides refusal, tested without a task or a sandbox.""" + + def module(self, body: str): + import importlib.util + + root = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, root, ignore_errors=True) + path = root / "hooks.py" + path.write_text(body) + spec = importlib.util.spec_from_file_location("h_under_test", path) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + def test_setup_and_teardown_are_not_blockers(self) -> None: + mod = self.module( + "def setup(w, c, x): pass\ndef teardown(w, c, x): pass\n" + ) + self.assertEqual(engine_hooks.unsupported_entry_points(mod), []) + + def test_check_and_setup_session_are(self) -> None: + mod = self.module( + "def check(r, c, x): pass\ndef setup_session(d): return {}\n" + ) + self.assertEqual( + sorted(engine_hooks.unsupported_entry_points(mod)), + ["check", "setup_session"], + ) + + def test_a_skill_with_no_hook_blocks_nothing(self) -> None: + self.assertEqual(engine_hooks.unsupported_entry_points(None), []) + + def test_a_non_callable_of_the_same_name_is_not_an_entry_point(self) -> None: + # `check = True` is a module constant, not a hook. + mod = self.module("check = True\nsetup_session = 3\n") + self.assertEqual(engine_hooks.unsupported_entry_points(mod), []) + + def test_a_setup_that_returns_template_vars_is_refused(self) -> None: + # Dropping them would leave `{placeholder}` in the prompt the agent is + # graded on, which reads as a badly written case. + with self.assertRaises(SystemExit) as caught: + engine_hooks._returned_template_vars({"endpoint": "x"}, "skl") + message = str(caught.exception) + self.assertIn("endpoint", message) + self.assertIn("skl", message) + + def test_a_setup_that_returns_nothing_is_fine(self) -> None: + for value in (None, {}, "", 0): + with self.subTest(value=value): + self.assertIsNone( + engine_hooks._returned_template_vars(value, "skl") + ) + + +class TestHooksRunOrTheRunStops(unittest.TestCase): + """A skill's setup either runs, or the run stops. Never skipped quietly. + + `setup` and `teardown` run on these engines: inspect's `Task.setup` and + `Task.cleanup` are the same two shapes, and `cleanup` runs inside a + `finally` under a shielded cancel scope, so teardown still happens when + the agent raises. + + `check` and `setup_session` do not. `check` is handed the legacy engine's + `Run` object and these engines have inspect's `TaskState`; `setup_session` + returns template variables that are substituted into prompts before any + hook runs. Both are refused rather than skipped, because skipping leaves + no trace: the case is graded as though the hook had run, and the failure + surfaces later as a skill that mysteriously does not work on this runner. + + Refused by entry point, not by the file existing. An earlier version + grounded any skill that shipped a hook at all -- which took the behavioral + leg away from the one skill in the catalogue whose hook these engines run + perfectly well: `setup` clears stale vLLM containers, `teardown` removes + them, and neither touches `ctx`. + """ + + def setUp(self) -> None: + self.repo = Repo(self) + self.repo.skill("plain", dataset=tier0_dataset("plain")) + self.repo.skill( + "hooked", + dataset=tier0_dataset("hooked"), + hooks="def setup(workspace, case, ctx):\n pass\n", + ) + self.repo.activate() + + def refuse(self, engine: str, skills: list[str], command: str = "behavioral"): + return cli._require_hook_support(engine, skills, command) + + def test_setup_and_teardown_are_supported_so_the_run_proceeds(self) -> None: + # The regression this replaces: the skill that ships exactly this hook + # lost its behavioral leg on every engine. + self.assertIsNone(self.refuse("claude-code-no-sandbox", ["hooked"])) + + def test_a_check_hook_is_refused_by_name(self) -> None: + self.repo.skill( + "checked", + dataset=tier0_dataset("checked"), + hooks="def check(run, case, ctx):\n pass\n", + ) + with self.assertRaises(SystemExit) as caught: + self.refuse("claude-code-no-sandbox", ["checked"]) + message = str(caught.exception) + self.assertIn("checked", message) + self.assertIn("check", message) + self.assertIn("setup", message) # says what IS supported + + def test_a_setup_session_hook_is_refused_by_name(self) -> None: + self.repo.skill( + "sessioned", + dataset=tier0_dataset("sessioned"), + hooks="def setup_session(cache_dir):\n return {}\n", + ) + with self.assertRaises(SystemExit) as caught: + self.refuse("claude-code", ["sessioned"]) + self.assertIn("setup_session", str(caught.exception)) + + def test_the_refusal_names_every_skill_it_blocks(self) -> None: + # Naming one of three sends someone round the loop twice. + for name in ("checked", "also-checked"): + self.repo.skill( + name, + dataset=tier0_dataset(name), + hooks="def check(run, case, ctx):\n pass\n", + ) + with self.assertRaises(SystemExit) as caught: + self.refuse("claude-code", ["checked", "also-checked", "plain"]) + message = str(caught.exception) + self.assertIn("checked", message) + self.assertIn("also-checked", message) + + def test_a_skill_without_a_hook_is_not_refused(self) -> None: + self.assertIsNone(self.refuse("claude-code", ["plain"])) + + def test_routing_is_exempt_because_it_never_reads_hooks(self) -> None: + # A routing run installs the skills and asks which one fires. It + # executes nothing, so there is no setup to skip. + self.assertIsNone(self.refuse("claude-code", ["hooked"], command="routing")) + + def test_every_engine_refuses_what_it_cannot_honour(self) -> None: + self.repo.skill( + "checked", + dataset=tier0_dataset("checked"), + hooks="def check(run, case, ctx):\n pass\n", + ) + for engine in cli.ENGINES: + with self.subTest(engine=engine): + with self.assertRaises(SystemExit): + self.refuse(engine, ["checked"]) + + def test_both_engines_wire_the_hook_into_their_task(self) -> None: + # The other half: refusing the unsupported ones is only correct if the + # supported ones actually run. + import skillscope.engine.behavioral as eb + import skillscope.engine.verify as ev + + for module in (eb, ev): + with self.subTest(module=module.__name__): + source = inspect.getsource(module.build_task) + self.assertIn("setup=hooks.setup_solver", source) + self.assertIn("cleanup=hooks.cleanup_fn", source) + + + + +class TestTheBudgetFitsTheEngineItJudges(unittest.TestCase): + """The cap exists to catch an agent that started working, not one thinking. + + So it is calibrated on calls made *before a decision*, which is the only + span it can meaningfully cut short. Measured over one 67-case room, on the + 39 sandboxed cases that decided: 32 called the skill tool first with no + preamble, mean 0.31, peak 4. + + An earlier revision read a mean of 2.46 off the same run and scaled by + three. That mean summed two populations -- cases that decide at once, and + cases that never find a skill and rummage until stopped. Only the second + sits near the threshold, and budget cannot rescue it: those score + `no_activation` at four calls or at forty. Headroom over the decision peak + is the thing worth buying; headroom over the rummaging is just spend. + """ + + def test_a_host_leg_is_held_to_the_cap_as_given(self) -> None: + for engine in ("claude-code-no-sandbox",): + with self.subTest(engine=engine): + self.assertEqual(engine_routing.budget_for(engine, 4), 4) + + def test_the_sandboxed_leg_gets_room_to_orient(self) -> None: + scaled = engine_routing.budget_for("claude-code", 4) + self.assertEqual(scaled, 4 * engine_routing.SANDBOX_BUDGET_FACTOR) + + def test_the_scaled_cap_clears_the_observed_decision_peak(self) -> None: + # The slowest sandboxed case to decide took 4 calls. A cap that cannot + # clear that discards real activations, which is the bug being fixed. + self.assertGreater(engine_routing.budget_for("claude-code", 4), 4) + + def test_the_headroom_is_proportionate_to_what_was_measured(self) -> None: + # Guards the other direction, which is the mistake that was made: a cap + # far above the decision peak only funds cases that will not activate. + observed_peak = 4 + self.assertLessEqual( + engine_routing.budget_for("claude-code", 4), observed_peak * 2 + ) + + def test_the_callers_intent_survives_the_scaling(self) -> None: + # Asking for a tighter budget still means tighter, on every leg. + self.assertLess( + engine_routing.budget_for("claude-code", 2), + engine_routing.budget_for("claude-code", 4), + ) + + def test_no_cap_stays_no_cap(self) -> None: + for cap in (None, 0): + with self.subTest(cap=cap): + self.assertEqual(engine_routing.budget_for("claude-code", cap), cap) + + def test_the_report_states_the_cap_that_was_enforced(self) -> None: + # Otherwise `near_limit` counts against a threshold that never applied. + source = inspect.getsource(cli.cmd_routing) + self.assertIn("budget_for(", source) + + +def _activation_event(skill: str) -> dict: + """One `stream-json` assistant event in which the agent fires a skill.""" + return { + "type": "assistant", + "message": { + "content": [ + {"type": "tool_use", "name": "Skill", "input": {"skill": skill}} + ] + }, + } + + +def _work_event(n: int) -> dict: + """An event that is the agent doing work rather than choosing.""" + return { + "type": "assistant", + "message": { + "content": [ + { + "type": "tool_use", + "name": "Bash", + "input": {"command": f"echo {n}", "description": f"step {n}"}, + } + ] + }, + } + + +class TestTheBehavioralReportActuallyRenders(unittest.TestCase): + """Both report writers get called, on a summary shaped like a real one. + + Nothing exercised `behavior.render_markdown`, and a function it calls was + deleted with the legacy engine. Every unit test passed. The break surfaced + on a self-hosted MI300X, after two cases had been graded 4/4 and the agent + time paid for -- `NameError: _isolation_note` while writing the report -- + and `continue-on-error` turned it into a green job. + + The cheapest check there is: render it, and look at what came out. + """ + + def summary(self, **meta): + base = { + "model": "opus", + "engine": "claude-code", + "effort": "high", + "skills": ["alpha"], + } + return behavior.summarize([], {**base, **meta}) + + def test_it_renders_for_a_sandboxed_run(self) -> None: + out = behavior.render_markdown( + self.summary(sandbox="podman", sandbox_isolated=True) + ) + self.assertIn("podman", out) + self.assertIn("isolated", out) + + def test_it_renders_for_an_unsandboxed_run(self) -> None: + # The line that matters most, because its absence reads as isolation. + out = behavior.render_markdown( + self.summary(sandbox="host", sandbox_isolated=False) + ) + self.assertIn("unsandboxed", out) + + def test_it_renders_when_nothing_said_what_contained_it(self) -> None: + out = behavior.render_markdown(self.summary()) + self.assertIsInstance(out, str) + self.assertTrue(out.strip()) + + def test_the_routing_report_renders_too(self) -> None: + # Same class of gap, same cheap guard. + out = routing.render_markdown( + routing.summarize( + [], + ["alpha"], + { + "model": "opus", + "engine": "claude-code", + "effort": "high", + "skills": ["alpha"], + }, + ) + ) + self.assertIsInstance(out, str) + self.assertTrue(out.strip()) + + +class TestTheSandboxedRoomHoldsWholeSkills(unittest.TestCase): + """inspect's skill installer carries less than a skill ships. + + `read_skills` collects `SKILL.md` and the contents of `scripts/`, + `references/` and `assets/`; everything else is dropped without a word. The + catalogue these legs run against puts its supporting material at the top + level -- `reference.md`, `examples.md`, `skill-card.md`, `templates/`, + `agents/` -- and no skill in it has a `references/` directory at all. + + So the container held `SKILL.md` and little else while the host legs held + the whole directory, and the two were compared as one room. Measured: the + agent opened the right skill's `SKILL.md`, followed it to `reference.md`, + found nothing, and stopped -- scored as a missed trigger. + """ + + def _skill(self, layout: dict[str, str]) -> Path: + root = Path(tempfile.mkdtemp()) + for rel, body in layout.items(): + target = root / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(body) + self.addCleanup(shutil.rmtree, root, ignore_errors=True) + return root + + def test_top_level_supporting_files_are_restored(self) -> None: + skill = self._skill( + { + "SKILL.md": "---\nname: x\n---\n", + "reference.md": "the details", + "examples.md": "worked examples", + "skill-card.md": "card", + } + ) + restored = { + p.relative_to(skill).as_posix() + for p in engine_verify.files_to_restore(skill) + } + self.assertEqual(restored, {"reference.md", "examples.md", "skill-card.md"}) + + def test_nested_directories_the_installer_ignores_are_restored(self) -> None: + skill = self._skill( + { + "SKILL.md": "---\nname: x\n---\n", + "templates/rule.md": "t", + "agents/analyzer.md": "a", + } + ) + restored = { + p.relative_to(skill).as_posix() + for p in engine_verify.files_to_restore(skill) + } + self.assertEqual(restored, {"templates/rule.md", "agents/analyzer.md"}) + + def test_what_the_installer_already_carries_is_not_duplicated(self) -> None: + skill = self._skill( + { + "SKILL.md": "---\nname: x\n---\n", + "scripts/detect.py": "print()", + "references/api.md": "r", + "assets/logo.png": "bytes", + } + ) + self.assertEqual(engine_verify.files_to_restore(skill), []) + + def test_the_skills_own_tests_are_never_staged(self) -> None: + # `evals/evals.json` pairs each prompt with the skill it expects. In + # the room, that is the answer sheet. The host legs copytree it in + # today; this deliberately does not match them. + skill = self._skill( + { + "SKILL.md": "---\nname: x\n---\n", + "evals/evals.json": '{"evaluations": [{"expect": "x"}]}', + "evals/files/input.hip": "kernel", + "reference.md": "keep me", + } + ) + restored = { + p.relative_to(skill).as_posix() + for p in engine_verify.files_to_restore(skill) + } + self.assertEqual(restored, {"reference.md"}) + + def test_the_staging_runs_before_the_agent_not_after(self) -> None: + # Chained behind `claude_code` it would land after the run finished, + # which looks identical in the source and does nothing at all. + source = inspect.getsource(engine_routing._solver) + staged = source.index("complete_skills_solver") + agent = source.index("claude_code(skills=room") + self.assertLess( + staged, agent, "the room is completed after the agent has already run" + ) + + def test_the_behavioral_leg_completes_its_skill_too(self) -> None: + # It matters more there: a behavioral case runs the skill to the end, + # so a missing reference.md is a step the agent cannot take. + self.assertIn( + "complete_skills_solver", inspect.getsource(engine_verify.build_task) + ) + + +class TestEveryLegThinksAsHardAsItWasTold(unittest.TestCase): + """`--effort` has to reach all three legs or the comparison is not one. + + `legacy` and `claude-code-no-sandbox` build the CLI's command line and pass + `--effort` into it. The sandboxed leg goes through `inspect_swe`, whose + `effort` argument defaults to `None` -- documented as leaving the model's + own default in place, which is emphatically not the same value. + + So the trial was comparing a leg thinking at the effort it was given with + one thinking at whatever it liked, and attributing the difference to + isolation. Asserted against the source because the alternative is a live + sandboxed run, and a silent revert here looks like a finding about skills. + """ + + def _claude_code_call(self, func) -> str: + """The text of the `claude_code(...)` invocation in `func`. + + Read by balancing parentheses rather than by regex: the arguments + contain calls of their own, so a non-greedy match ends at the wrong + bracket and the assertion fails on correct code. + """ + source = inspect.getsource(func) + start = source.index("claude_code(") + len("claude_code(") + depth, end = 1, start + while depth: + depth += {"(": 1, ")": -1}.get(source[end], 0) + end += 1 + return source[start : end - 1] + + def test_the_sandboxed_routing_leg_is_given_the_effort(self) -> None: + self.assertIn( + "effort=", + self._claude_code_call(engine_routing._solver), + "the sandboxed routing leg drops --effort", + ) + + def test_the_sandboxed_behavioral_leg_is_given_the_effort(self) -> None: + self.assertIn( + "effort=", + self._claude_code_call(engine_verify.build_task), + "the sandboxed behavioral leg drops --effort", + ) + + def test_an_inspect_swe_without_effort_is_refused_up_front(self) -> None: + # The failure it replaces: a TypeError raised inside a task inspect had + # already started, leaving one leg of a three-leg comparison with no + # report while the run reported success. + def _no_effort(skills=None, cwd=None): # pragma: no cover -- a stub + return None + + fake = types.ModuleType("inspect_swe") + fake.claude_code = _no_effort + with mock.patch.dict(sys.modules, {"inspect_swe": fake}), mock.patch.object( + engine_verify.sandbox_spec, "provider", lambda: "podman" + ), mock.patch.object(engine_verify.sys, "platform", "linux"): + with self.assertRaises(SystemExit) as caught: + engine_verify.require() + self.assertIn("0.2.71", str(caught.exception)) + + def test_a_current_inspect_swe_is_accepted(self) -> None: + # The other direction, so the guard cannot pass by always raising. + def _with_effort(skills=None, cwd=None, effort=None): # pragma: no cover + return None + + fake = types.ModuleType("inspect_swe") + fake.claude_code = _with_effort + with mock.patch.dict(sys.modules, {"inspect_swe": fake}), mock.patch.object( + engine_verify.sandbox_spec, "provider", lambda: "podman" + ), mock.patch.object(engine_verify.sys, "platform", "linux"): + engine_verify.require() + + def test_the_pin_matches_what_the_guard_demands(self) -> None: + # A guard naming a version the package does not require would send + # people to an upgrade their install then undoes. + pin = (Path(__file__).resolve().parents[1] / "pyproject.toml").read_text() + self.assertIn("inspect-swe>=0.2.71", pin) + + def test_the_behavioral_runner_hands_the_effort_down(self) -> None: + # build_task grew the parameter once already without the caller + # filling it, which reads exactly like the bug it was meant to fix. + self.assertIn("effort", inspect.signature(engine_verify.build_task).parameters) + self.assertIn( + "effort=", + inspect.getsource(engine_verify.run).split("build_task(", 1)[1][:120], + "verify.run accepts an effort it never passes on", + ) + + +class _FakeCall: + def __init__(self, call_id: str, function: str = "Bash") -> None: + self.id = call_id + self.function = function + self.arguments: dict = {} + + +class _FakeMessage: + def __init__(self, message_id: str, calls: list[_FakeCall]) -> None: + self.id = message_id + self.role = "assistant" + self.tool_calls = calls + + +class _FakeEvent: + def __init__(self, message) -> None: + self.output = type("O", (), {"message": message})() + + +class _FakeSample: + def __init__(self, messages, events) -> None: + self.messages = messages + self.events = events + + +class TestOneTurnIsCountedOnce(unittest.TestCase): + """The same turn reaches this code twice and must be counted once. + + A bridged agent's turn is both adopted onto `sample.messages` and carried by + a transcript `ModelEvent`, as two separate objects. De-duplicating them has + been wrong twice: first by object identity, which never matches, and then by + message id -- which looks right and is not, because the bridge *builds* the + adopted message rather than moving it, so the two copies carry + independently generated ids. + + Measured on a real run: 23 reported against 12 actually made. Verdicts were + never affected -- an activation is detected by presence, not by count -- but + every tool-call column, the budget's headroom and `near_limit` all were. + """ + + def _sample(self, same_call_ids: bool): + first = [_FakeCall("toolu_1"), _FakeCall("toolu_2")] + second = ( + [_FakeCall("toolu_1"), _FakeCall("toolu_2")] + if same_call_ids + else [_FakeCall("toolu_3"), _FakeCall("toolu_4")] + ) + # Different message ids on purpose: that is what the bridge produces. + return _FakeSample( + messages=[_FakeMessage("adopted", first)], + events=[_FakeEvent(_FakeMessage("transcript", second))], + ) + + def test_the_same_turn_from_both_records_counts_once(self) -> None: + calls = list(engine_routing._tool_calls(self._sample(same_call_ids=True))) + self.assertEqual( + [c.id for c in calls], + ["toolu_1", "toolu_2"], + "a turn present in both records was counted twice", + ) + + def test_genuinely_different_turns_are_both_kept(self) -> None: + # The other direction: the transcript is the more complete record, and + # collapsing distinct turns would hide calls rather than duplicate them. + calls = list(engine_routing._tool_calls(self._sample(same_call_ids=False))) + self.assertEqual( + [c.id for c in calls], ["toolu_1", "toolu_2", "toolu_3", "toolu_4"] + ) + + def test_a_differing_message_id_does_not_defeat_the_dedup(self) -> None: + # The regression itself, stated directly. + sample = self._sample(same_call_ids=True) + self.assertNotEqual( + sample.messages[0].id, + sample.events[0].output.message.id, + "the fixture no longer reproduces the bridge's behaviour", + ) + self.assertEqual(len(list(engine_routing._tool_calls(sample))), 2) + + def test_turns_that_called_nothing_still_count_as_speech(self) -> None: + # `_spoke` rests on these, and they have no call ids to key on. + quiet = _FakeMessage("only-text", []) + sample = _FakeSample(messages=[quiet], events=[]) + self.assertEqual(len(list(engine_routing._assistant_messages(sample))), 1) + + +class TestTheHostLegStopsAtTheDecision(unittest.TestCase): + """The host leg used to answer the question and then do the whole job. + + It has no approver -- its CLI is a subprocess, so nothing intercepts a tool + call before it runs -- and it awaited that subprocess to completion. So a + case activated the right skill on its first call and then went on to + download trace files, run the analysis and write reports, none of which any + scorer read. Measured over one 67-case room: 152 of its 220 tool calls came + after the decision. + + The rule here is the legacy engine's, applied to the CLI's stream as it + arrives, which is the only place this leg can see a decision in time. + """ + + def _rule(self, room=("alpha", "beta"), tools=4, inspections=4): + return engine_routing.host_stop_when_factory(list(room), tools, inspections)() + + def test_an_activation_stops_the_run(self) -> None: + reason = self._rule()(_activation_event("alpha")) + self.assertIsNotNone(reason) + self.assertIn("alpha", reason) + + def test_the_named_skill_survives_into_the_reason(self) -> None: + # The mapper reads the activation back off this string, so a reason + # that stops the run without naming what fired loses the verdict. + reason = self._rule()(_activation_event("beta")) + self.assertEqual( + engine_routing.activation_from_limit(reason, ["alpha", "beta"]), "beta" + ) + + def test_ordinary_work_does_not_stop_the_run(self) -> None: + self.assertIsNone(self._rule()(_work_event(1))) + + def test_the_budget_stops_an_agent_that_never_chooses(self) -> None: + rule = self._rule(tools=2) + reasons = [rule(_work_event(n)) for n in range(6)] + self.assertTrue( + any(r and engine_routing.BUDGET_MARK in r for r in reasons), + f"budget never tripped: {reasons}", + ) + + def test_the_cli_finishing_ends_the_read(self) -> None: + # Not a stop we imposed, but the loop must not wait on a dead stream. + self.assertEqual( + self._rule()({"type": "result", "result": "done"}), + engine_no_sandbox.STOP_RESULT, + ) + + def test_each_case_gets_its_own_budget(self) -> None: + # The bug this guards against has been shipped here once already, in + # the approver: a counter built with the solver rather than per sample + # creeps up across the room until it terminates every later case + # mid-deliberation. Two independent rules from one factory must not + # share a tally. + factory = engine_routing.host_stop_when_factory(["alpha"], 2, 2) + first = factory() + for n in range(6): + first(_work_event(n)) + second = factory() + self.assertIsNone( + second(_work_event(99)), + "a fresh case inherited the previous case's spent budget", + ) + + +class TestTheHostLegActuallyKillsTheProcess(unittest.IsolatedAsyncioTestCase): + """Deciding to stop is not stopping; the CLI has to actually die. + + Worth an end-to-end check rather than a unit test of the rule, because the + failure mode is silent: a stop that breaks the read loop but leaves the + process running still pays for every call it goes on to make, and the + events simply stop being recorded. The run looks cheaper and is not. + """ + + async def _run(self, script: str, stop_after: int): + seen = {"n": 0} + + def stop_when(event: dict) -> str | None: + seen["n"] += 1 + return "stop" if seen["n"] >= stop_after else None + + return await engine_no_sandbox._stream_until( + [sys.executable, "-u", "-c", script], + "prompt", + tempfile.gettempdir(), + dict(os.environ), + stop_when, + ) + + async def test_reading_stops_where_the_rule_says(self) -> None: + script = ( + "import json,sys\n" + "for i in range(20):\n" + " print(json.dumps({'type':'assistant','i':i}), flush=True)\n" + ) + events, reason, _, _ = await self._run(script, stop_after=3) + self.assertEqual(reason, "stop") + self.assertEqual(len(events), 3) + + async def test_the_work_after_the_decision_never_happens(self) -> None: + # The direct evidence: the child tries to leave a mark behind after the + # point we stop it. If the process outlived the stop, the mark is there. + # + # The wait afterwards is the whole test. Without it this passes even + # with the kill removed, because asyncio reaps surviving children when + # the loop closes -- which in a real run does not happen until the eval + # is over, long after the orphan has spent the budget. So the check has + # to happen while the loop is still up, exactly as it is mid-eval. + with tempfile.TemporaryDirectory() as tmp: + mark = Path(tmp) / "kept-working" + script = ( + "import json,sys,time\n" + "print(json.dumps({'type':'assistant','i':0}), flush=True)\n" + "time.sleep(1.5)\n" + f"open({str(mark)!r},'w').write('x')\n" + ) + started = time.perf_counter() + await self._run(script, stop_after=1) + elapsed = time.perf_counter() - started + + self.assertLess( + elapsed, 1.4, "the stop waited for the process instead of killing it" + ) + await asyncio.sleep(3.0) + self.assertFalse( + mark.exists(), "the CLI outlived the stop and kept working" + ) + + async def test_a_stream_that_ends_on_its_own_is_not_an_error(self) -> None: + script = "import json\nprint(json.dumps({'type':'result','result':'ok'}))\n" + events, reason, _, _ = await self._run(script, stop_after=99) + self.assertEqual(len(events), 1) + self.assertIsNone(reason) + + +class TestCasesDecidedByTheBudgetAreFlagged(unittest.TestCase): + """A case that stopped on its cap measured the cap, not the agent. + + Two runs of one engine disagreed on six cases where the noise floor was + one, and the explanation was that 26 of 67 cases sat within a single call + of a budget. Those are coin-flips: one more call either way moves them + across the line and takes the verdict with them. They look identical to + cases decided on their merits, which is what made the disagreement + unreadable. + + Reported rather than corrected. The budget is doing its job; the reader + just has to know how much of the run it decided. + """ + + def outcome(self, tool_calls=0, inspection_calls=0): + return routing.Outcome( + id="a", category="c", skill="s", prompt="p", expect=None, observed=None, + verdict="true_negative", passed=True, stop_reason="result", + elapsed_s=0.0, tool_calls=tool_calls, inspection_calls=inspection_calls, + ) + + CAPS = {"max_tool_calls": 4, "max_inspection_calls": 8} + + def test_a_case_that_stopped_on_the_cap_is_flagged(self) -> None: + self.assertTrue(routing.near_a_limit(self.outcome(tool_calls=4), self.CAPS)) + + def test_a_case_one_below_the_cap_is_flagged(self) -> None: + # One call is the resolution the threshold has. + self.assertTrue(routing.near_a_limit(self.outcome(tool_calls=3), self.CAPS)) + + def test_a_case_well_clear_of_the_cap_is_not(self) -> None: + self.assertFalse(routing.near_a_limit(self.outcome(tool_calls=1), self.CAPS)) + + def test_the_inspection_budget_counts_too(self) -> None: + self.assertTrue(routing.near_a_limit(self.outcome(inspection_calls=8), self.CAPS)) + + def test_a_cap_the_run_never_set_is_not_a_threshold(self) -> None: + # A leg that cannot enforce a budget reports none, and nothing should + # invent one for it. + self.assertFalse(routing.near_a_limit(self.outcome(tool_calls=99), {})) + self.assertFalse( + routing.near_a_limit(self.outcome(tool_calls=99), {"max_tool_calls": 0}) + ) + + def test_the_totals_carry_the_count(self) -> None: + outs = [self.outcome(tool_calls=4), self.outcome(tool_calls=0)] + totals = routing.summarize(outs, ["s"], {"skills": ["s"], **self.CAPS})["totals"] + self.assertEqual(totals["near_limit"], 1) + + def test_the_report_warns_before_the_reader_sees_the_score(self) -> None: + summary = routing.summarize( + [self.outcome(tool_calls=4)], + ["s"], + {"skills": ["s"], "model": "m", "effort": "e", **self.CAPS}, + ) + report = routing.render_markdown(summary) + self.assertIn("within one call of a budget", report) + self.assertLess(report.index("within one call"), report.index("| Verdict |")) + + +class TestProviderFailuresAreMarked(unittest.TestCase): + """A gateway error is not a routing result, and must not read as one. + + A 504 lands in a report as a lower score with nothing in the verdict + saying why, so a reader cannot tell it from the skill failing. At the rate + observed on one catalogue -- roughly a third of runs -- that makes an + unmarked provider error the most likely reason two runs of the same engine + disagree, which has to be ruled out before a difference between engines + means anything. + + Matching is deliberately generous: a false positive makes a reader look + twice at a run that was fine, a false negative lets an outage score as a + routing miss. The costs are not symmetric. + """ + + def test_the_shapes_actually_observed_are_recognised(self) -> None: + for message in ( + "API Error: 504 Exception trying to (AnthropicVertex) Chat Completions", + "API preflight timed out after 60s (is the network reachable?)", + "APIConnectionError: Connection error.", + "429 rate limit exceeded", + "502 Bad Gateway", + "upstream connect error", + ): + with self.subTest(message=message): + self.assertTrue(routing.is_provider_error(message)) + + def test_a_real_skill_failure_is_not_marked(self) -> None: + for message in ( + "the skill produced no plan.md", + "run ended without a routing decision (stopped after: tool_budget)", + None, + "", + ): + with self.subTest(message=message): + self.assertFalse(routing.is_provider_error(message)) + + def test_the_reason_a_result_event_gave_is_kept(self) -> None: + # It was being discarded. A 504 arrived as "result event reported an + # error", which no classifier and no reader can do anything with. + self.assertIn( + "504", + routing._result_error({"is_error": True, "result": "API Error: 504 upstream"}), + ) + + def test_the_subtype_is_kept_when_there_is_no_body(self) -> None: + # The CLI puts the reason in one field or the other depending on how + # it failed, and only one of them is ever populated. + self.assertIn( + "error_max_turns", + routing._result_error({"is_error": True, "subtype": "error_max_turns"}), + ) + + def test_an_agent_failure_is_not_blamed_on_the_provider(self) -> None: + # `error_max_turns` is the agent running out of road, not the gateway. + # Marking it degraded would excuse a real failure. + self.assertFalse(routing.is_provider_error( + routing._result_error({"is_error": True, "subtype": "error_max_turns"}) + )) + + def test_an_error_with_no_reason_is_not_guessed_at(self) -> None: + # Neither marked nor excused: we do not know, and saying so is the + # only honest option. + self.assertFalse(routing.is_provider_error( + routing._result_error({"is_error": True}) + )) + + def test_routing_totals_report_how_many_were_degraded(self) -> None: + # Beside the score, because it decides whether the score can be read. + outcomes = [ + routing.Outcome( + id=str(i), category="c", skill="s", prompt="p", expect=None, + observed=None, verdict="error", passed=False, stop_reason="result", + elapsed_s=0.0, tool_calls=0, error=err, + degraded=routing.is_provider_error(err), + ) + for i, err in enumerate(["API Error: 504 upstream", "the skill did nothing"]) + ] + totals = routing.summarize(outcomes, ["s"], {"skills": ["s"]})["totals"] + self.assertEqual(totals["errors"], 2) + self.assertEqual(totals["degraded"], 1) + + def test_behavioral_totals_report_it_too(self) -> None: + # Same vocabulary on both commands, or a degraded behavioral run reads + # as a failing skill. + outcomes = [ + behavior.BehaviorOutcome( + id="a", skill="s", prompt="p", passed=False, elapsed_s=0.0, + error="API Error: 504", degraded=True, + ) + ] + totals = behavior.summarize(outcomes, {"model": "m", "effort": "e"})["totals"] + self.assertEqual(totals["degraded"], 1) + + def test_the_report_warns_before_the_reader_sees_the_score(self) -> None: + # Underneath the table is too late: a reader has already formed a view. + outcomes = [ + routing.Outcome( + id="a", category="c", skill="s", prompt="p", expect=None, + observed=None, verdict="error", passed=False, stop_reason="result", + elapsed_s=0.0, tool_calls=0, error="API Error: 504", degraded=True, + ) + ] + summary = routing.summarize( + outcomes, ["s"], {"skills": ["s"], "model": "m", "effort": "e"} + ) + report = routing.render_markdown(summary) + self.assertIn("failed at the model provider", report) + self.assertLess(report.index("model provider"), report.index("| Verdict |")) + + +class TestRoutingStopsAtTheDecision(unittest.TestCase): + """The sandboxed leg stops when the decision is made, as legacy does. + + Legacy reads the CLI's stream and kills the process the moment a skill + activates, because everything after that is work the routing question does + not ask for and does pay for. The rule here runs earlier: it sees each tool + call the bridged CLI *proposes*, so the case stops without the work + happening at all. + + The skill's name goes into the reason on purpose. Approval runs before the + bridge adopts the assistant message, so the tool call that revealed the + decision may be absent from the transcript afterwards -- and a suppressed + activation reads exactly like an agent that correctly declined to route. + + Exercised through `routing_decision`, which is pure, rather than through + the approver that wraps it: the unit suite runs without the inspect extra + on purpose, and a rule that can only be tested with it would not be tested + at all in CI. + """ + + ROOM = ["alpha", "beta", "gamma"] + + def decide(self, function, arguments, tally=None, tools=None, inspections=None): + return engine_routing.routing_decision( + function, arguments, self.ROOM, + tally or engine_routing._Tally(), tools, inspections, + ) + + def test_activating_a_skill_stops_the_case(self) -> None: + decision, _ = self.decide("Skill", {"command": "alpha"}) + self.assertEqual(decision, "terminate") + + def test_the_skill_that_fired_survives_in_the_reason(self) -> None: + # The half that cannot go missing when the message does. + _, reason = self.decide("Skill", {"command": "alpha"}) + self.assertEqual( + engine_routing.activation_from_limit(reason, self.ROOM), "alpha" + ) + + def test_a_skill_nobody_installed_is_still_a_decision(self) -> None: + # A contaminated room is a routing result, not a non-event: the run + # has to be able to say a stranger fired. + decision, reason = self.decide("Skill", {"command": "dataviz"}) + self.assertEqual(decision, "terminate") + self.assertEqual( + engine_routing.activation_from_limit(reason, self.ROOM), "other:dataviz" + ) + + def test_ordinary_work_is_allowed_through(self) -> None: + self.assertEqual(self.decide("Bash", {"command": "ls"})[0], "approve") + + def test_a_skills_survey_spends_the_inspection_budget_not_the_work_one(self) -> None: + # Legacy's rule, and it matters: counting a survey against the work + # budget too ends a run mid-deliberation and scores it as a missed + # trigger. Observed doing exactly that on a real sandboxed run. + tally = engine_routing._Tally() + self.decide("Read", {"file_path": "/w/.claude/skills/alpha/SKILL.md"}, tally, 4, 8) + self.assertEqual((tally.tools, tally.inspections), (0, 1)) + + def test_unrelated_work_spends_the_work_budget(self) -> None: + tally = engine_routing._Tally() + self.decide("Bash", {"command": "pip install torch"}, tally, 4, 8) + self.assertEqual((tally.tools, tally.inspections), (1, 0)) + + def test_bookkeeping_spends_neither(self) -> None: + tally = engine_routing._Tally() + for name in sorted(routing.BOOKKEEPING_TOOLS): + self.decide(name, {}, tally, 4, 8) + self.assertEqual((tally.tools, tally.inspections), (0, 0)) + + def test_the_budget_is_counted_per_case_not_per_run(self) -> None: + # An approver is built once per task. A counter captured there counts + # every case in the run, creeping up until it crosses the budget and + # then terminating any case that makes a counted call before choosing. + # Seen as four cases terminated at tallies of 10, 11, 12 and 13 -- + # consecutive across different prompts. + self.assertIn("store()", inspect.getsource(engine_routing.routing_approver)) + self.assertNotIn( + "tally = _Tally()\n\n @approver", + inspect.getsource(engine_routing.routing_approver), + ) + + def test_the_tool_call_budget_stops_a_case_that_is_rummaging(self) -> None: + tally = engine_routing._Tally() + decisions = [ + self.decide("Bash", {"command": f"ls {i}"}, tally, tools=2)[0] + for i in range(4) + ] + self.assertEqual(decisions, ["approve", "approve", "terminate", "terminate"]) + + def test_bookkeeping_calls_do_not_count_against_the_budget(self) -> None: + # Same rule as legacy: the budget is about work, not housekeeping. + tally = engine_routing._Tally() + for name in sorted(routing.BOOKKEEPING_TOOLS): + self.decide(name, {}, tally, tools=1) + self.assertEqual(tally.tools, 0) + + def test_a_limit_reason_from_elsewhere_is_not_read_as_an_activation(self) -> None: + self.assertIsNone( + engine_routing.activation_from_limit("operator cancelled", self.ROOM) + ) + + def test_a_named_skill_outside_the_room_is_not_read_as_an_activation(self) -> None: + # The room is the authority. Otherwise a stray string becomes a verdict. + forged = engine_routing.ACTIVATION_MARK + "delta" + self.assertIsNone(engine_routing.activation_from_limit(forged, self.ROOM)) + + def test_the_approver_delegates_to_the_rule_rather_than_repeating_it(self) -> None: + self.assertIn( + "routing_decision(", inspect.getsource(engine_routing.routing_approver) + ) + + def test_the_host_leg_gets_no_approver(self) -> None: + # Its CLI buffers until exit, so there is nothing to approve in time; + # attaching one would suggest a bound that does not exist. + self.assertIn( + "if engine == CLAUDE_CODE", inspect.getsource(engine_routing.build_task) + ) + + +class TestRoutingCaseTimeoutBinds(unittest.TestCase): + """`--case-timeout` has to reach the new legs, or it is a cap in name only. + + The flag exists so one hung prompt cannot spend the whole run -- `deadline` + says so in its own docstring. The inspect legs originally bounded a sample + only by the command's remaining budget, which is precisely the thing the + flag guards against, while the report recorded `case_timeout` as though it + had applied. A cap that is reported and not enforced is worse than none. + """ + + def tearDown(self) -> None: + deadline.use(None) + + def test_the_flag_is_the_bound_when_the_command_has_room(self) -> None: + deadline.use(deadline.Deadline(3000.0, command="routing")) + self.assertEqual(engine_routing.case_time_limit(90), 90) + + def test_the_command_deadline_clips_a_longer_case_cap(self) -> None: + # The command deadline ends the process outright, taking the report + # with it, so a per-case cap must not outlive it. + deadline.use(deadline.Deadline(200.0, command="routing")) + self.assertLess(engine_routing.case_time_limit(9999), 200) + + def test_no_case_cap_falls_back_to_the_command_budget(self) -> None: + deadline.use(deadline.Deadline(3000.0, command="routing")) + self.assertEqual( + engine_routing.case_time_limit(None), + engine_behavioral.task_time_limit(deadline.active()), + ) + + def test_an_unbounded_command_still_honours_the_case_cap(self) -> None: + deadline.use(None) + self.assertEqual(engine_routing.case_time_limit(45), 45) + + def test_the_cli_hands_the_flag_to_the_leg(self) -> None: + # Asserted on the source: the failure mode is the argument silently + # not being passed, which no unit of the leg can notice. + self.assertIn("case_timeout=args.case_timeout", inspect.getsource(cli.cmd_routing)) + + +class TestRoutingRoomIsTheRoomThatWasAskedFor(unittest.TestCase): + """The host routing leg isolates the config dir, or it does not run. + + The legacy engine warns and carries on, because it reads the CLI's `init` + event and can name a user-level skill that gate-crashed the room. Neither + inspect leg gets that event, so the same contamination would be invisible + -- and a stray skill is offered for every prompt, so it changes every + decision at once while the run still reports a clean accuracy. + + Observed rather than feared: a probe of this leg on a developer machine put + roughly forty user-level skills in the room and none of the three staged. + """ + + def setUp(self) -> None: + self.addCleanup(os.environ.pop, "ANTHROPIC_API_KEY", None) + + def test_the_host_leg_refuses_without_the_credential_that_isolates_it(self) -> None: + os.environ.pop("ANTHROPIC_API_KEY", None) + with self.assertRaises(SystemExit) as caught: + engine_routing.require_isolated_room("claude-code-no-sandbox") + message = str(caught.exception) + self.assertIn("ANTHROPIC_API_KEY", message) + self.assertIn("--engine claude-code", message) + + def test_the_host_leg_runs_when_it_can_isolate(self) -> None: + os.environ["ANTHROPIC_API_KEY"] = "sk-test" + self.assertIsNone(engine_routing.require_isolated_room("claude-code-no-sandbox")) + + def test_the_sandboxed_leg_needs_no_credential_to_have_a_clean_room(self) -> None: + # The guest has no `~/.claude` to keep out, which is the whole reason + # this leg is the one to prefer for routing. + os.environ.pop("ANTHROPIC_API_KEY", None) + self.assertIsNone(engine_routing.require_isolated_room("claude-code")) + + def test_the_host_solver_can_be_pointed_at_a_config_dir(self) -> None: + # Without this parameter the leg reads the runner's own config dir and + # nothing downstream can tell. + self.assertIn( + "config_dir", inspect.signature(engine_no_sandbox.claude_code_no_sandbox).parameters + ) + + +class TestRoutingReportsWhereItRan(unittest.TestCase): + """Each leg states what contained it; none may overstate it. + + Deriving this from the engine's name is what let a run that started no + container report `sandbox: docker, sandbox_isolated: true`. Hardcoding + `host` was right only while no routing leg had a sandbox. Now one does, so + the value is passed in by the leg that knows. + """ + + def setUp(self) -> None: + self.repo = Repo(self) + self.repo.skill("alpha", dataset=tier0_dataset("alpha")) + self.repo.activate() + self.written: dict = {} + patch = mock.patch.object( + cli, "_write_report", lambda summary, *a, **k: self.written.update(summary) + ) + patch.start() + self.addCleanup(patch.stop) + + def meta(self, engine: str, **containment) -> dict: + args = cli.build_parser().parse_args( + ["routing", "--engine", engine, "--skip-preflight"] + ) + cli._finish_routing( + args, [], {"alpha": None}, time.time(), isolated=True, **containment + ) + return self.written["meta"] + + def test_a_host_leg_says_host(self) -> None: + meta = self.meta( + "claude-code-no-sandbox", sandbox="host", sandbox_isolated=False + ) + self.assertEqual(meta["sandbox"], "host") + self.assertIs(meta["sandbox_isolated"], False) + + def test_the_sandboxed_leg_names_the_provider_it_used(self) -> None: + meta = self.meta("claude-code", sandbox="docker", sandbox_isolated=True) + self.assertEqual(meta["sandbox"], "docker") + self.assertIs(meta["sandbox_isolated"], True) + + def test_a_leg_that_does_not_say_what_contained_it_cannot_report(self) -> None: + # Required keywords, so a leg added later fails at the call site + # instead of quietly reporting a containment it never had. + args = cli.build_parser().parse_args(["routing", "--skip-preflight"]) + with self.assertRaises(TypeError): + cli._finish_routing(args, [], {"alpha": None}, time.time(), isolated=True) + + def test_a_legs_own_meta_cannot_overwrite_what_contained_it(self) -> None: + meta = self.meta( + "claude-code-no-sandbox", sandbox="host", sandbox_isolated=False, + ) + self.assertEqual(meta["sandbox"], "host") + + def test_the_report_does_not_ask_the_provider_what_contained_a_run(self) -> None: + # The defect in one line: `_sandbox_meta` returns `provider()`, which + # examines nothing and returns the default string. + self.assertNotIn("_sandbox_meta", inspect.getsource(cli._finish_routing)) + + +class TestClaudeCliPreflightChecksBothCredentials(unittest.TestCase): + """`claude-code-no-sandbox` uses two, so probing one proves nothing about the other. + + The real CLI is the agent and the inspect provider grades it, so a run dies + on whichever is missing. The preflight tested the provider alone -- and for + routing, which uses neither, it tested the provider and demanded the + inspect extra for a leg that runs no inspect code. + """ + + def setUp(self) -> None: + self.repo = Repo(self) + self.repo.skill("alpha", dataset=tier0_dataset("alpha")) + self.repo.activate() + self.probed: list[str] = [] + + def args(self, engine: str) -> argparse.Namespace: + return cli.build_parser().parse_args( + ["behavioral", "--engine", engine, "--model", "opus"] + ) + + def run_preflight(self, engine: str, cli_ok: bool = True) -> list[str]: + patches = [ + mock.patch.object(cli.engine, "require", lambda *a, **k: None), + mock.patch.object( + cli, "check_api_reachable", + lambda *a, **k: (self.probed.append("cli"), (cli_ok, "ok"))[1], + ), + ] + probe = mock.patch( + "skillscope.engine.models.check_reachable", + lambda *a, **k: (self.probed.append("provider"), (True, "ok"))[1], + ) + for patch in [*patches, probe]: + patch.start() + self.addCleanup(patch.stop) + cli._prepare_graded_run(self.args(engine)) + return self.probed + + def test_it_probes_the_cli_as_well_as_the_provider(self) -> None: + self.assertEqual(sorted(self.run_preflight("claude-code-no-sandbox")), ["cli", "provider"]) + + def test_a_sandboxed_engine_probes_only_the_provider(self) -> None: + # `claude-code` reaches the CLI inside the container through inspect's + # own bridge, so the host's CLI credential is not what it uses. + self.assertEqual(self.run_preflight("claude-code"), ["provider"]) + + def test_an_unreachable_cli_stops_the_run(self) -> None: + with self.assertRaises(SystemExit) as caught: + self.run_preflight("claude-code-no-sandbox", cli_ok=False) + self.assertIn("claude API not reachable", str(caught.exception)) + + +class TestTheModelProbeIsBounded(unittest.TestCase): + """The preflight must not become the hang it exists to prevent. + + Pointed at a closed port it was still running after four minutes: inspect's + `GenerateConfig.max_retries` defaults to `None`, which retries a connection + error without a limit, and nothing clipped the call to `--timeout`. The + legacy probe has had both guards from the start, and says why in its own + docstring. + """ + + def tearDown(self) -> None: + deadline.use(None) + + def test_retries_are_off(self) -> None: + self.assertEqual(engine_models.PROBE_RETRIES, 0) + + def test_the_probe_asks_for_that_and_a_per_attempt_timeout(self) -> None: + source = inspect.getsource(engine_models._probe) + self.assertIn("max_retries=PROBE_RETRIES", source) + self.assertIn("timeout=", source) + # Belt and braces: neither of the above covers a connect that stalls + # before the provider's own clock starts. + self.assertIn("fail_after", source) + + def test_the_probe_config_is_not_memoized_into_the_graded_run(self) -> None: + # Retries off is right for a one-shot check and wrong for the run it + # precedes, and inspect memoizes models by default. + self.assertIn("memoize=False", inspect.getsource(engine_models._probe)) + + def test_an_unbounded_command_gets_the_default(self) -> None: + deadline.use(None) + self.assertEqual( + engine_models.probe_bound(), (engine_models.PROBE_TIMEOUT_S, "") + ) + + def test_a_tighter_command_timeout_wins(self) -> None: + deadline.use(deadline.Deadline(5.0, command="behavioral")) + seconds, _ = engine_models.probe_bound() + self.assertLessEqual(seconds, 5.0) + + def test_a_looser_command_timeout_does_not_extend_it(self) -> None: + deadline.use(deadline.Deadline(10_000.0, command="behavioral")) + seconds, _ = engine_models.probe_bound() + self.assertEqual(seconds, engine_models.PROBE_TIMEOUT_S) + + def test_an_expired_command_probes_nothing_at_all(self) -> None: + deadline.use(deadline.Deadline(0.0, command="behavioral")) + seconds, why = engine_models.probe_bound() + self.assertIsNone(seconds) + self.assertIn("--timeout", why) + + def test_an_expired_command_is_reported_rather_than_dialled(self) -> None: + deadline.use(deadline.Deadline(0.0, command="behavioral")) + ok, detail = engine_models.check_reachable("anthropic/claude-x") + self.assertFalse(ok) + self.assertIn("--timeout", detail) + + def test_mockllm_still_costs_nothing(self) -> None: + self.assertEqual( + engine_models.check_reachable("mockllm/model"), + (True, "mockllm (no provider)"), + ) + + +class TestTheProbeSaysWhatWentWrong(unittest.TestCase): + """A preflight whose message is unreadable has not done its job. + + Turning retries off makes tenacity raise `RetryError`, whose string form is + `RetryError[]` -- it names neither the host nor the + failure. The point of probing early is a message on the first line. + """ + + class Future: + def __init__(self, error: BaseException | None) -> None: + self.error = error + + def exception(self) -> BaseException | None: + return self.error + + def wrapper(self, error: BaseException | None) -> Exception: + wrapped = RuntimeError("RetryError[]") + wrapped.last_attempt = self.Future(error) + return wrapped + + def test_the_wrapped_error_is_what_gets_reported(self) -> None: + cause = ConnectionError("Connection error.") + self.assertIs(engine_models._underlying(self.wrapper(cause)), cause) + + def test_a_plain_error_is_left_alone(self) -> None: + plain = ValueError("401 unauthorized") + self.assertIs(engine_models._underlying(plain), plain) + + def test_an_empty_wrapper_falls_back_to_itself(self) -> None: + empty = self.wrapper(None) + self.assertIs(engine_models._underlying(empty), empty) + + def test_unwrapping_never_raises_on_its_own(self) -> None: + # This runs on the failure path. An exception here would replace a + # useful message with a traceback from the reporting code. + class Exploding: + def exception(self): + raise RuntimeError("boom") + + hostile = RuntimeError("wrapped") + hostile.last_attempt = Exploding() + self.assertIs(engine_models._underlying(hostile), hostile) + + +class TestTheShippedSandboxExample(unittest.TestCase): + """The worked example has to keep working. + + `sandbox:` was documented with no example of what the file it names looks + like, which left the one thing a reader actually needs -- a device bound in + and egress granted -- as an exercise. An example that drifts from the + schema, or from what inspect requires of a compose file, is worse than + none, so it is checked here rather than trusted. + """ + + EXAMPLE = REPO_ROOT / "examples" / "skill-with-a-device" / "evals" + + def setUp(self) -> None: + import yaml + + self.machine = yaml.safe_load( + (self.EXAMPLE / "machine.yml").read_text(encoding="utf-8") + ) + self.compose = yaml.safe_load( + (self.EXAMPLE / "compose.yaml").read_text(encoding="utf-8") + ) + + def test_the_machine_file_uses_only_keys_the_parser_knows(self) -> None: + self.assertEqual(set(self.machine) - datasets.MACHINE_KEYS, set()) + + def test_it_names_the_compose_file_that_sits_beside_it(self) -> None: + # Resolved beside machine.yml, not at the skill root. An example that + # got this wrong would teach the one mistake the layout invites. + self.assertTrue((self.EXAMPLE / self.machine["sandbox"]).is_file()) + + def test_the_compose_file_offers_a_service_inspect_will_use(self) -> None: + services = self.compose["services"] + default = "default" in services or any( + spec.get("x-default") for spec in services.values() + ) + self.assertTrue(default, f"no default service among {sorted(services)}") + + def test_the_container_is_told_to_stay_up(self) -> None: + # inspect execs into a container that is already running. One that + # exits on start fails every case on a sandbox that is not there. + self.assertIn("command", self.compose["services"]["default"]) + + def test_it_demonstrates_the_two_things_the_default_withholds(self) -> None: + default = self.compose["services"]["default"] + self.assertIn("devices", default) + # Egress is granted by *not* setting this, which is worth asserting: + # an example that carried it would grant nothing and say it did. + self.assertNotIn("network_mode", default) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tools/benchmark_engines.py b/tools/benchmark_engines.py new file mode 100644 index 0000000..096b6ea --- /dev/null +++ b/tools/benchmark_engines.py @@ -0,0 +1,332 @@ +#!/usr/bin/env python3 +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Compare two engines on the same dataset. + +Answers the two questions a migration has to answer before it can be trusted. + +**Does the new engine agree?** Per case, not in aggregate: an accuracy figure +can match exactly while individual cases flip in both directions and cancel +out. Flips are reported by direction, and against a measured noise floor -- +routing and behavioral are both nondeterministic, so "these two runs differ" +means nothing until you know how much one engine differs from itself. + +**Does it pay for itself?** Wall clock and tokens per run, from the report +`meta` both engines now populate. + +Runs through the `skillscope` CLI rather than importing either engine, so what +is measured is what CI executes. + + tools/benchmark_engines.py routing --routing-room my-skill --noise + tools/benchmark_engines.py behavioral --skill my-skill + tools/benchmark_engines.py --compare legacy.json candidate.json + +Which pair is compared is an argument, because the question changes over the +migration. `legacy` against `claude-code-no-sandbox` asks the narrow, sharp question: both +drive the same CLI, so agreement says the framework around the agent is +faithful, and disagreement is a defect in the crossing rather than a property +of a different agent. `claude-code-no-sandbox` against `claude-code` asks the other one -- +same agent, host against container, which is where a contaminated runner shows +up as a disagreement neither engine could find alone. + + tools/benchmark_engines.py behavioral --candidate claude-code-no-sandbox --skill my-skill +""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +import tempfile +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +# Taken from the CLI rather than restated, so a new engine is offered here the +# moment it is offered there. +from skillscope.cli import ENGINES # noqa: E402 + +AGREE = "agree" +NEW_PASSES = "only the candidate passes" +NEW_FAILS = "only the baseline passes" + + +def run_leg(leg: str, engine: str, passthrough: list[str], label: str) -> dict: + """Run one leg on one engine and return its JSON report.""" + out = Path(tempfile.mkdtemp(prefix="benchmark-")) / f"{label}.json" + cmd = [ + sys.executable, "-m", "skillscope", leg, + "--engine", engine, "--output", str(out), *passthrough, + ] + print(f"[benchmark] {label}: {' '.join(cmd)}", flush=True) + # A failing leg is a result, not an error: a run where cases fail still + # produces the report this compares. + subprocess.run(cmd, check=False) + if not out.is_file(): + raise SystemExit(f"error: {label} produced no report at {out}") + return json.loads(out.read_text(encoding="utf-8")) + + +def cases_by_id(report: dict) -> dict[str, dict]: + return {str(case["id"]): case for case in report.get("cases", [])} + + +def compare(baseline: dict, candidate: dict) -> dict: + """Per-case comparison of two reports of the same dataset.""" + left, right = cases_by_id(baseline), cases_by_id(candidate) + shared = sorted(set(left) & set(right)) + + rows = [] + for case_id in shared: + a, b = left[case_id], right[case_id] + if a["passed"] == b["passed"]: + direction = AGREE + else: + direction = NEW_PASSES if b["passed"] else NEW_FAILS + rows.append( + { + "id": case_id, + "direction": direction, + "baseline_passed": a["passed"], + "candidate_passed": b["passed"], + # Routing carries the decision itself, which says more than + # pass/fail: two engines can both fail a case for different + # reasons, and that is not agreement. + "baseline_observed": a.get("observed"), + "candidate_observed": b.get("observed"), + "baseline_verdict": a.get("verdict"), + "candidate_verdict": b.get("verdict"), + } + ) + + agreed = sum(1 for r in rows if r["direction"] == AGREE) + return { + "compared": len(rows), + "agreed": agreed, + "agreement": round(agreed / len(rows), 4) if rows else None, + "flips": [r for r in rows if r["direction"] != AGREE], + "only_in_baseline": sorted(set(left) - set(right)), + "only_in_candidate": sorted(set(right) - set(left)), + "rows": rows, + } + + +def spend(report: dict, engine: str = "legacy") -> dict: + meta = report.get("meta", {}) + return { + "engine": meta.get("engine", engine), + "wall_time_s": meta.get("wall_time_s"), + "model_calls": meta.get("model_calls"), + "total_tokens": meta.get("total_tokens"), + "cost_usd": meta.get("cost_usd"), + } + + +def _cell(value) -> str: + return "n/a" if value is None else str(value) + + +def _spend_caveats(spend: dict) -> list[str]: + """Say which columns are comparable, because not all of them are. + + The two engines count different things and silently tabulating them side by + side invites the wrong conclusion. Wall time is always comparable. Tokens + are not: the legacy engine reads them from assistant events, which exclude + the system prompt and cached input, and a routing case is killed before the + totals arrive -- so its figure is a floor, not a total. Cost is the legacy + engine's trustworthy number, and the inspect_ai-backed engines only have + one when the provider supplies pricing, which a gateway generally does not. + """ + engines = {spend[label]["engine"] for label in ("baseline", "candidate")} + notes = [] + if "legacy" in engines: + notes.append( + "> Legacy token counts are a floor: they omit the system prompt and " + "cached input, and a killed case never reports its totals. Compare " + "cost and wall time, not tokens." + ) + if any(spend[label]["cost_usd"] is None for label in ("baseline", "candidate")): + notes.append( + "> One engine reported no cost -- the inspect_ai-backed engines only " + "have one " + "when the model provider supplies pricing, which a gateway generally " + "does not. Wall time and model calls are comparable on both sides; " + "model calls in particular is the like-for-like measure of how " + "much work each engine asks of the model per case." + ) + return notes + + +def render(result: dict) -> str: + comparison = result["comparison"] + base = result["spend"]["baseline"]["engine"] + cand = result["spend"]["candidate"]["engine"] + lines = [ + "## Engine benchmark", + "", + f"**{comparison['agreed']}/{comparison['compared']} cases agree** " + f"between the `{base}` and `{cand}` engines.", + "", + ] + + noise = result.get("noise") + if noise is not None: + lines += [ + f"Noise floor: the `{base}` engine agrees with itself on " + f"{noise['agreed']}/{noise['compared']} cases. Treat any difference " + "at or below that as run-to-run variance rather than engine drift.", + "", + ] + else: + lines += [ + "_No noise floor measured; re-run with `--noise` before reading the " + "flips below as engine differences._", + "", + ] + + lines += ["| Run | Wall time | Model calls | Tokens | Cost |", "| --- | --- | --- | --- | --- |"] + for label in ("baseline", "candidate"): + s = result["spend"][label] + lines.append( + f"| {label} (`{s['engine']}`) | {_cell(s['wall_time_s'])}s | " + f"{_cell(s['model_calls'])} | {_cell(s['total_tokens'])} | " + f"{_cell(s['cost_usd'])} |" + ) + lines += ["", *_spend_caveats(result["spend"])] + + lines += ["", "### Cases that flipped", ""] + if not comparison["flips"]: + lines.append("None. Every shared case reached the same verdict on both engines.") + else: + lines += [ + f"| Case | Direction | `{base}` | `{cand}` |", + "| --- | --- | --- | --- |", + ] + for flip in comparison["flips"]: + left = flip["baseline_verdict"] or ("pass" if flip["baseline_passed"] else "fail") + right = flip["candidate_verdict"] or ("pass" if flip["candidate_passed"] else "fail") + lines.append(f"| `{flip['id']}` | {flip['direction']} | {left} | {right} |") + + for key, heading in ( + ("only_in_baseline", f"Only the `{base}` run produced these cases"), + ("only_in_candidate", f"Only the `{cand}` run produced these cases"), + ): + missing = comparison[key] + if missing: + lines += ["", f"### {heading}", "", ", ".join(f"`{m}`" for m in missing)] + + return "\n".join(lines) + "\n" + + +def refuse_unrunnable_pair(parser, args) -> None: + """Stop before the first leg when the pair cannot produce a comparison. + + Checked up front because the cost is not symmetric: the baseline leg runs + first, and with `--noise` it runs twice, so a candidate the leg will refuse + is discovered only after a full routing run has been paid for. The refusal + then surfaces as `produced no report`, which blames a missing file rather + than naming the engine that was never going to run. + + Every engine has a routing leg now, so in practice this catches the same + engine named twice, which measures run-to-run variance rather than a + difference between engines. `--noise` already reports that, and reports it + as what it is. + """ + if args.leg != "routing": + return + + runnable = set(cli_routing_engines()) + unrunnable = sorted({args.baseline, args.candidate} - runnable) + if unrunnable: + parser.error( + f"routing has no leg for {', '.join(unrunnable)}. It runs on " + f"{', '.join(sorted(runnable))}." + ) + if args.baseline == args.candidate: + parser.error( + f"--baseline and --candidate are both {args.baseline!r}, which " + "measures run-to-run variance rather than a difference between " + "engines. That is what --noise already reports, and it labels the " + "result as the noise floor rather than as a flip list." + ) + + +def cli_routing_engines() -> tuple[str, ...]: + """The engines the CLI will actually run a routing leg on.""" + from skillscope.cli import ROUTING_ENGINES + + return ROUTING_ENGINES + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("leg", nargs="?", choices=["routing", "behavioral"]) + parser.add_argument( + "--compare", + nargs=2, + metavar=("BASELINE", "CANDIDATE"), + help="Compare two reports that already exist instead of running the legs.", + ) + parser.add_argument( + "--baseline", + default="legacy", + choices=ENGINES, + help="The engine to measure against. Default: legacy.", + ) + parser.add_argument( + "--candidate", + default="claude-code-no-sandbox", + choices=ENGINES, + help="The engine under test. Default: claude-code-no-sandbox.", + ) + parser.add_argument( + "--noise", + action="store_true", + help=( + "Run the baseline engine twice to measure how much it disagrees " + "with itself. Without this the flip list cannot be read as engine " + "drift." + ), + ) + parser.add_argument("--output", default="", help="Write the JSON result here.") + args, passthrough = parser.parse_known_args(argv) + + if args.compare: + baseline = json.loads(Path(args.compare[0]).read_text(encoding="utf-8")) + candidate = json.loads(Path(args.compare[1]).read_text(encoding="utf-8")) + noise = None + else: + if not args.leg: + parser.error("give a leg to run (routing or behavioral), or --compare") + refuse_unrunnable_pair(parser, args) + baseline = run_leg(args.leg, args.baseline, passthrough, args.baseline) + noise_run = ( + run_leg(args.leg, args.baseline, passthrough, f"{args.baseline}-again") + if args.noise + else None + ) + candidate = run_leg(args.leg, args.candidate, passthrough, args.candidate) + noise = compare(baseline, noise_run) if noise_run is not None else None + + result = { + "comparison": compare(baseline, candidate), + "noise": noise, + "spend": { + "baseline": spend(baseline, getattr(args, "baseline", "legacy")), + "candidate": spend(candidate, getattr(args, "candidate", "claude-code-no-sandbox")), + }, + } + + report = render(result) + print(report) + if args.output: + Path(args.output).write_text(json.dumps(result, indent=2), encoding="utf-8") + print(f"[benchmark] JSON result: {args.output}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/sandbox_smoketest.py b/tools/sandbox_smoketest.py new file mode 100644 index 0000000..42194be --- /dev/null +++ b/tools/sandbox_smoketest.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Prove the sandbox machinery holds, without grading anything. + +Every engine that runs on `inspect_ai` shares a layer beneath the agent: a +sandbox is started, a case's `workspace:` fixtures are staged into it, the +guest is listed, and the listing is matched against what the case asked for. +That layer is where every cross-platform bug so far has been -- a container +that would not start, a Windows guest that could not be listed, a fixture that +landed somewhere the scorer never looked. + +It used to be covered by grading a fixture skill on the harness-independent +engine with a mock model. That engine is gone, and the two that replace it both +drive the real `claude` CLI, so neither can run key-free on a fork's pull +request. Rather than lose the coverage, this exercises the same layer directly: +a stub solver that does nothing at all stands in for the agent. + +Doing nothing is the point. The case's expectation is a file the *case* seeded, +so it passes only if the fixture was staged, the sandbox was listed, and the +listing was matched -- and it cannot be passed by an agent that got lucky. No +model is ever called, so this costs nothing and is deterministic. + + tools/sandbox_smoketest.py [--skill demo-skill] [--expect-sandbox docker] + +Exits non-zero, loudly, on the first thing that did not hold. +""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from skillscope import config, datasets # noqa: E402 +from skillscope.engine import behavioral, sandbox as sandbox_spec # noqa: E402 + +# Never reached -- the stub solver returns before anything generates -- but +# `eval()` requires a model, and this is the one that cannot bill anybody. +MODEL = "mockllm/model" + + +def stub_solver(skill_dir: Path): + """An agent that does nothing, so only the machinery can pass the case.""" + from inspect_ai.solver import solver + + @solver + def _noop(): + async def solve(state, generate): + return state + + return solve + + return _noop() + + +def fail(message: str) -> None: + print(f"FAIL: {message}", file=sys.stderr) + raise SystemExit(1) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("repo", help="Repo holding the fixture skill.") + parser.add_argument("--skill", default="demo-skill") + parser.add_argument( + "--expect-sandbox", + default="", + help=( + "The provider this run must resolve to, e.g. 'docker' or 'local'. " + "Supplied by the caller rather than read back from the harness: " + "asking the harness what it chose and then checking its own answer " + "cannot fail, and the failure that matters is a run that quietly " + "got no container at all." + ), + ) + args = parser.parse_args() + + config.use(config.build(Path(args.repo).resolve(), skills_dir="*")) + + cases = [ + case + for case in datasets.load_dataset(args.skill) + if case.has_behavior + ] + if not cases: + fail(f"{args.skill} has no case asserting anything, so nothing is proven") + + provider = sandbox_spec.provider() + print(f"[smoketest] {len(cases)} case(s), sandbox provider {provider!r}", flush=True) + + # Before anything runs: a graded run that silently dropped its sandbox + # reports the same shape as one that kept it, and `local` is the host + # filesystem, so every check below would pass with nothing isolated. + if args.expect_sandbox and provider != args.expect_sandbox: + fail( + f"expected the {args.expect_sandbox!r} sandbox, got {provider!r}. " + f"Check SKILLSCOPE_SANDBOX and the platform detection -- an " + f"unsandboxed run passes every check below for the wrong reason." + ) + + sandbox_spec.require_provider() + from inspect_ai import eval as inspect_eval + + logs = inspect_eval( + behavioral.build_task(args.skill, cases, MODEL, solver_factory=stub_solver), + model=MODEL, + log_dir=str(Path(".skillscope") / "logs"), + log_realtime=behavioral.realtime_logging(), + display="plain", + ) + + outcomes = [] + for log in logs: + outcomes.extend(behavioral._outcomes(log, args.skill, cases)) + + # An infrastructure failure is not a result. This is what catches a + # sandbox that never started. + errored = [o for o in outcomes if o.error] + if errored: + fail(f"{len(errored)} case(s) errored: {[o.error for o in errored]}") + + checks = [check for outcome in outcomes for check in outcome.checks] + if not checks: + fail("nothing was graded, so nothing was proven") + + # A guest that cannot be listed reports the same shape as an agent that + # produced nothing, so the difference is asserted rather than assumed. + unlistable = [c for c in checks if "could not list the sandbox" in (c.get("detail") or "")] + if unlistable: + fail(f"the sandbox could not be listed: {unlistable}") + + seeded = [c for c in checks if c.get("kind") == "files_exist"] + if not seeded: + fail("the seeded-file check did not run, so staging was never exercised") + unmet = [c for c in seeded if not c.get("passed")] + if unmet: + fail(f"a file the case seeded was not found in the sandbox: {unmet}") + + print( + f"[smoketest] ok -- {len(checks)} check(s) graded, " + f"{len(seeded)} seeded fixture(s) found, sandbox {provider!r}." + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())