From d564a663cf585b0f1af78a1c574c782cb5358f34 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 07:18:00 +0000 Subject: [PATCH 01/33] Add inspect_ai engine behind --engine flag The legacy engine shells out to the claude CLI with permissions bypassed on the runner, and hand-rolls workspace staging, LLM judging and routing termination around it. This adds a second engine that hands that work to inspect_ai, selected with `--engine inspect` and off by default until a benchmark shows what it changes. Its agent is harness-independent rather than the claude CLI, so a skill is graded on whether its instructions work rather than on one harness's reading of them. That also removes what blocked Windows: with no CLI in the sandbox there is no agent bridge, and the bridge is Linux-only. Both engines produce the same outcome objects, so `summarize`, `render_markdown` and the report writers are untouched. Three pieces are not thin wrappers. The tools are ours because inspect's assume a POSIX guest, which the Windows legs do not have. The judge sees artifacts rather than the agent's prose, attaches images so "did it draw a cat" is answerable, and reports whether a requirement is satisfied so a "must not" is never scored by negating a verdict. Routing is a single model call with no agent loop: the decision is a tool call, visible in the first turn, so there is no turn to pay for and kill. machine.yml gains a `sandbox` key naming a compose file, so a skill that must reach the network to pull a model can say so. --- pyproject.toml | 14 +- skillscope/cli.py | 102 +++++++++--- skillscope/datasets.py | 12 +- skillscope/engine/__init__.py | 38 +++++ skillscope/engine/behavioral.py | 145 +++++++++++++++++ skillscope/engine/convert.py | 91 +++++++++++ skillscope/engine/judge.py | 206 ++++++++++++++++++++++++ skillscope/engine/models.py | 76 +++++++++ skillscope/engine/routing.py | 182 +++++++++++++++++++++ skillscope/engine/sandbox.py | 70 ++++++++ skillscope/engine/scorers.py | 109 +++++++++++++ skillscope/engine/tools.py | 223 ++++++++++++++++++++++++++ skillscope/schema/machine.schema.json | 6 + tests/test_skillscope.py | 212 +++++++++++++++++++++++- 14 files changed, 1452 insertions(+), 34 deletions(-) create mode 100644 skillscope/engine/__init__.py create mode 100644 skillscope/engine/behavioral.py create mode 100644 skillscope/engine/convert.py create mode 100644 skillscope/engine/judge.py create mode 100644 skillscope/engine/models.py create mode 100644 skillscope/engine/routing.py create mode 100644 skillscope/engine/sandbox.py create mode 100644 skillscope/engine/scorers.py create mode 100644 skillscope/engine/tools.py diff --git a/pyproject.toml b/pyproject.toml index d05674d..d4cf227 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,16 +15,24 @@ 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 +# The legacy runner is standard library only, so a graded 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"] +# The inspect-backed engine (`--engine inspect`). Kept an extra so the default +# path keeps the stdlib-only property while both engines are shipped: a repo +# that has not migrated installs nothing new. +[project.optional-dependencies] +# `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. +inspect = ["inspect-ai>=0.3.263", "anthropic>=0.40"] + [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/cli.py b/skillscope/cli.py index f9d93b4..215cfd2 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -72,7 +72,7 @@ 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 from . import selection as select_module from .agent import check_api_reachable, enforce_model_policy @@ -334,6 +334,11 @@ 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", "legacy") == "inspect": + # The inspect engine never shells out to `claude`, so the CLI-based + # reachability probe would be testing something this run does not use. + engine.require() + return selected if not args.skip_preflight: ok, detail = check_api_reachable(args.model) if not ok: @@ -341,6 +346,43 @@ def _prepare_graded_run( return selected +def _finish_routing( + args: argparse.Namespace, + outcomes: list, + routing_set: dict, + started: float, + *, + isolated: bool, + extra: dict | None = None, +) -> int: + """Summarize, report, and gate a routing run. Shared by both engines.""" + 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, + "isolated_config_dir": isolated, + "github_run_id": os.environ.get("GITHUB_RUN_ID"), + **(extra or {}), + }, + ) + _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,6 +404,15 @@ def cmd_routing(args: argparse.Namespace) -> int: elif args.skill: cases = datasets.filter_cases(cases, args.skill) + if args.engine == "inspect": + from .engine import models as engine_models + from .engine import routing as inspect_routing + + outcomes = inspect_routing.run( + cases, routing_set, engine_models.resolve(args.model) + ) + return _finish_routing(args, outcomes, routing_set, started, isolated=True) + routing_config = routing.RoutingConfig( model=args.model, effort=args.effort, @@ -392,34 +443,20 @@ def cmd_routing(args: argparse.Namespace) -> int: else: outcomes = [routing.run_case(case, routing_set, routing_config) for case in cases] - summary = routing.summarize( + 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, + routing_set, + started, + isolated=routing_config.isolate_config, + extra={ "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"), }, ) - _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,11 +479,21 @@ def cmd_behavioral(args: argparse.Namespace) -> int: ) return 0 - outcomes = behavior.run(skills, gradable, args.model, args.effort) + if args.engine == "inspect": + from .engine import behavioral as inspect_behavioral + from .engine import models as engine_models + + outcomes = inspect_behavioral.run( + skills, gradable, engine_models.resolve(args.model), args.effort + ) + else: + outcomes = behavior.run(skills, gradable, args.model, args.effort) + summary = behavior.summarize( outcomes, { "model": args.model, + "engine": args.engine, "effort": args.effort, "skills": skills, "extended": args.extended, @@ -565,6 +612,17 @@ 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", "legacy"), + choices=["legacy", "inspect"], + help=( + "Which eval engine runs the cases. `legacy` drives the claude CLI " + "directly; `inspect` runs a harness-independent agent through " + "inspect_ai (needs `pip install 'skillscope[inspect]'`). Default: " + "legacy, or $SKILLSCOPE_ENGINE." + ), + ) _add_timeout_argument(parser) diff --git a/skillscope/datasets.py b/skillscope/datasets.py index 17a75b8..a8ada70 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 the inspect engine):: 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..13b2207 --- /dev/null +++ b/skillscope/engine/__init__.py @@ -0,0 +1,38 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""The inspect-backed eval engine (``--engine inspect``). + +The legacy engine drives the `claude` CLI directly; this one hands the work to +`inspect_ai`. Both 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()` before touching a submodule +to turn a missing wheel into an actionable message rather than a traceback. +""" + +from __future__ import annotations + +INSTALL_HINT = ( + "error: --engine inspect needs the inspect extra. Install it with:\n" + " pip install 'skillscope[inspect]'" +) + + +def require() -> 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) 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..31c0cc4 --- /dev/null +++ b/skillscope/engine/behavioral.py @@ -0,0 +1,145 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Behavioral evals on the inspect engine. + +`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 + +from pathlib import Path + +from .. import config, deadline +from ..behavior import BehaviorOutcome +from ..datasets import Case +from . import convert, models, sandbox as sandbox_spec, scorers, tools + +# 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 + + +def _tools(skill_dir: Path) -> list: + """Tools the agent gets for a behavioral run. + + The skill under test, plus the cross-platform set from `engine/tools.py` -- + inspect's own `bash()` and `text_editor()` assume a POSIX guest, which the + Windows legs do not have. + """ + from inspect_ai.tool import skill + + return [skill([skill_dir]), *tools.toolset()] + + +def build_task(skill: str, cases: list[Case], ctx: dict | None = None): + """One inspect `Task` per skill: its cases, its skill installed, its scorer.""" + from inspect_ai import Task + from inspect_ai.agent import react + + skill_dir = config.active().skill_path(skill) + samples = [convert.sample_from_case(c, skill_dir, ctx) for c in cases] + + bound = deadline.active() + return Task( + name=f"behavioral-{skill}", + dataset=samples, + solver=react(tools=_tools(skill_dir)), + scorer=scorers.expectations(), + sandbox=sandbox_spec.for_skill(skill), + message_limit=MESSAGE_LIMIT, + time_limit=int(bound.remaining()) if bound is not None else None, + ) + + +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 [ + BehaviorOutcome( + id=case.id, + skill=skill, + prompt=case.prompt, + passed=False, + elapsed_s=0.0, + error=f"inspect task failed: {detail}", + ) + for case in cases + ] + + 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, + ) + ) + return outcomes + + +def run( + skills: list[str], cases: list[Case], model: str, effort: str +) -> list[BehaviorOutcome]: + """Run every behavioral case, grouped by skill. Mirrors `behavior.run`.""" + from inspect_ai import eval as inspect_eval + + 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) + logs = inspect_eval( + build_task(skill, skill_cases), + model=model, + model_args=models.model_args(), + log_dir=str(Path(".skillscope") / "logs"), + # 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", + ) + for log in logs: + 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..7143a17 --- /dev/null +++ b/skillscope/engine/convert.py @@ -0,0 +1,91 @@ +# 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}") + + files: dict[str, str] = {} + for path in sorted(seed.rglob("*")): + if path.is_file(): + target = path.relative_to(seed).as_posix() + files[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/judge.py b/skillscope/engine/judge.py new file mode 100644 index 0000000..330401b --- /dev/null +++ b/skillscope/engine/judge.py @@ -0,0 +1,206 @@ +# 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 = 6_000 + +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 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): + parts.append(content) + text = "\n".join(parts) + if len(text) > MAX_TRANSCRIPT: + text = text[:MAX_TRANSCRIPT] + "\n...[truncated]..." + return text + + +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 + + described: list[str] = [] + images: list[tuple[str, bytes]] = [] + + for path in paths[:MAX_FILES]: + if is_image(path): + try: + images.append((path, await sandbox().read_file(path, 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(path, 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, + ) + + paths = await tools_list_paths() + described, images = await artifacts(paths) + + evidence = "\n".join( + [ + f"Files the agent left behind: {paths or 'none'}", + "", + "--- what the agent did ---", + transcript_of(state), + "", + "--- artifacts ---", + *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" + 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..1feca0d --- /dev/null +++ b/skillscope/engine/models.py @@ -0,0 +1,76 @@ +# 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 + +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}") + + +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() -> 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. + """ + 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/routing.py b/skillscope/engine/routing.py new file mode 100644 index 0000000..80e06b6 --- /dev/null +++ b/skillscope/engine/routing.py @@ -0,0 +1,182 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Routing evals on the inspect engine. + +Routing asks one question: with the whole catalog installed, does this prompt +activate the skill it should, and stay quiet when it should not? The room +matters -- a skill tested alone happily answers its neighbour's prompts -- so +every skill is offered on every case, exactly as `routing.stage_workspace` did. + +Two things get much simpler than the legacy engine. + +**Activation is observed, not inferred.** With the `skill()` tool the agent +names the skill it wants, so the decision is a tool call rather than something +reconstructed from a stream of events. `routing.detect_activation` and its +helpers exist because that signal was not available. + +**Stopping is free.** A routing decision is visible in the first assistant turn, +so a case is exactly one model call: the skills are offered as tools and the +reply either names one or does not. Nothing is executed, so there is no agent +loop to bound and no sandbox to start -- the legacy engine's stream reader, +process group and SIGKILL all exist to end a turn it had already paid for. + +Keeping the scaffolding out is also a measurement decision: no submit tool and +no agent system prompt sit between the descriptions and the decision, which is +what makes the result about the descriptions. + +What this measures is how well a description discriminates against its +neighbours, which is the part a skill author controls. It is not a measurement +of any particular product harness's discovery machinery, and the numbers are +not interchangeable with one. +""" + +from __future__ import annotations + +import time +from pathlib import Path + +from .. import config, deadline +from ..datasets import Case +from ..routing import PASSING_VERDICTS, Outcome, classify +from . import convert, models + +SKILL_TOOL = "skill" + + +def activation_of(messages) -> str | None: + """The skill the agent asked for, or None if it never asked for one.""" + for message in messages: + for call in getattr(message, "tool_calls", None) or []: + if call.function != SKILL_TOOL: + continue + named = (call.arguments or {}).get("command") + if isinstance(named, str) and named.strip(): + return named.strip() + return None + + +def tool_call_count(messages) -> int: + return sum(len(getattr(m, "tool_calls", None) or []) for m in messages) + + +def decide(routing_set: dict[str, Path]): + """Solver: offer the room as tools, take one turn, record what was named. + + The tool is never executed -- only its definition matters, which is the + skill's name and description. That is the whole input to a routing decision. + """ + from inspect_ai.solver import solver + from inspect_ai.tool import skill + + @solver + def _decide(): + tools = [skill(list(routing_set.values()))] + + async def solve(state, generate): + from inspect_ai.model import get_model + + output = await get_model().generate(input=state.messages, tools=tools) + state.messages.append(output.message) + state.output = output + return state + + return solve + + return _decide() + + +def build_task(cases: list[Case], routing_set: dict[str, Path]): + """One task holding every routing case, with the whole room offered.""" + from inspect_ai import Task + + cfg = config.active() + samples = [ + convert.sample_from_case( + case, cfg.skill_path(case.skill) if case.skill else Path(".") + ) + for case in cases + ] + + bound = deadline.active() + return Task( + name="routing", + dataset=samples, + solver=decide(routing_set), + # No sandbox: nothing is executed, so there is nothing to isolate. + time_limit=int(bound.remaining()) if bound is not None else None, + ) + + +def _outcome(sample, case: Case, room: list[str]) -> Outcome: + """Map one inspect sample onto the outcome `routing.summarize` expects.""" + messages = sample.messages or [] + observed = activation_of(messages) + calls = tool_call_count(messages) + + if sample.error is not None: + verdict, error, stop_reason = "error", sample.error.message, "error" + else: + verdict, error = classify(case.expect_skill, observed), None + stop_reason = "decided" if observed else "completed" + + return 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(getattr(sample, "total_time", None) or 0.0, 2), + tool_calls=calls, + # The legacy engine counted reads of a skill body separately because a + # skill could be inspected without being activated. The tool makes that + # distinction disappear: naming the skill *is* the activation. + inspection_calls=0, + visible_skills=room, + # Nothing beyond the room can leak in: the tool is constructed from the + # room, so there is no user-level config dir for a stray skill to arrive + # from. That is why `routing.can_isolate_config` has no counterpart here. + extra_skills=[], + error=error, + ) + + +def run(cases: list[Case], routing_set: dict[str, Path], model: str) -> list[Outcome]: + """Run every routing case against the whole room.""" + from inspect_ai import eval as inspect_eval + + if not cases: + return [] + + room = list(routing_set) + print(f"[routing] installed together: {', '.join(room) or '(none)'}") + print(f"[routing] {len(cases)} cases, model={model}", flush=True) + + started = time.perf_counter() + logs = inspect_eval( + build_task(cases, routing_set), + model=model, + model_args=models.model_args(), + log_dir=str(Path(".skillscope") / "logs"), + display="plain", + ) + + by_id = {case.id: case for case in cases} + outcomes: list[Outcome] = [] + for log in logs: + if log.status == "error" or not log.samples: + detail = getattr(log.error, "message", None) or "no samples" + raise SystemExit(f"error: routing task failed: {detail}") + for sample in log.samples: + case = by_id.get(str(sample.id)) + if case is not None: + outcomes.append(_outcome(sample, case, room)) + + elapsed = round(time.perf_counter() - started, 1) + print(f"[routing] {len(outcomes)} decisions in {elapsed}s", flush=True) + return outcomes diff --git a/skillscope/engine/sandbox.py b/skillscope/engine/sandbox.py new file mode 100644 index 0000000..55a8251 --- /dev/null +++ b/skillscope/engine/sandbox.py @@ -0,0 +1,70 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Which sandbox a skill's cases run in. + +Docker on Linux, `local` on Windows. inspect's sandbox layer -- and every tool +built on it -- assumes a POSIX guest, so there is no Windows container option +here; the Windows legs trade isolation for running on the platform they are +meant to test. DevLab's ephemeral, off-network runners are what covers that gap. + +A skill declares its needs in `evals/machine.yml`, which already exists to say +what class of machine a skill wants. An optional `sandbox:` key names a compose +file relative to the skill directory, so a skill that must reach the network to +pull a model, or needs `/dev/dri`, says so instead of every skill paying for it. +""" + +from __future__ import annotations + +import os +import sys + +from .. import datasets + +# Escape hatch for local development and wiring runs: `SKILLSCOPE_SANDBOX=local` +# skips the container entirely. 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" + +# Hardware-free skills get no network. Skills that need egress ship their own +# compose file and opt out of this default. +DEFAULT_COMPOSE = "compose.yaml" + + +def is_windows() -> bool: + return sys.platform.startswith("win") + + +def for_skill(skill: str): + """The `sandbox` spec for a skill's task, or None to use inspect's default. + + Returns a `(type, config)` tuple when a compose file is declared, a bare + type name otherwise -- both are accepted as `Task(sandbox=...)`. + """ + override = os.environ.get(SANDBOX_ENV, "").strip() + if override: + return override + + if is_windows(): + return "local" + + compose = _declared_compose(skill) + if compose is not None: + return ("docker", str(compose)) + return "docker" + + +def _declared_compose(skill: str): + """Path to the compose file a skill's `machine.yml` names, if any.""" + name = (datasets._read_machine(skill) or {}).get("sandbox") + if not name: + return None + + path = datasets.skill_path(skill) / 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..58f0dea --- /dev/null +++ b/skillscope/engine/scorers.py @@ -0,0 +1,109 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Grading for the inspect engine. + +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: + files = await tools.list_paths() + 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/tools.py b/skillscope/engine/tools.py new file mode 100644 index 0000000..71f8630 --- /dev/null +++ b/skillscope/engine/tools.py @@ -0,0 +1,223 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""A tool set that works on a non-POSIX guest. + +inspect's own tools assume one: `bash()` execs `["bash", "--login", "-c", ...]`, +`text_editor()` needs a Linux-only helper binary, and `list_files()`/`grep()` +shell out to `find`/`grep`. On a Windows host with the `local` sandbox that +leaves an agent that can read and think but cannot write a file or run a +command -- not enough to grade a behavioral case with. + +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" + +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) + + probe = await sandbox().exec(["bash", "-lc", "exit 0"], concurrency=False) + prefix = POSIX_SHELL if probe.success else WINDOWS_SHELL + store().set(SHELL_KEY, prefix) + return list(prefix) + + +async def run(command: str, timeout: int | None = None): + """Run `command` through whichever shell the sandbox has.""" + from inspect_ai.util import sandbox + + prefix = await shell_prefix() + return await sandbox().exec(prefix + [command], 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) + + +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: + return [] + return normalize_listing(result.stdout) + + +def _text(result) -> str: + """`stdout` plus `stderr`, which is where a failing command says why.""" + parts = [result.stdout.strip(), result.stderr.strip()] + body = "\n".join(p for p in parts if p) + if result.success: + return body or "(no output)" + return f"exit code {result.returncode}\n{body}".strip() + + +def shell(timeout: int = 300): + """Run shell commands in the sandbox, on whichever platform it is.""" + from inspect_ai.tool import Tool, tool + + @tool(name="shell") + def _shell() -> Tool: + async def execute(command: str) -> str: + """Run a command in the sandbox and return its output. + + Uses bash on Linux and macOS, and PowerShell on Windows, so write + commands for the platform you find yourself on. Check with `uname` + or `$PSVersionTable` if you are unsure. + + Args: + command: The command line to run. + + Returns: + The command's combined output, or its exit code and error output + when it fails. + """ + return _text(await run(command, timeout=timeout)) + + return execute + + return _shell() + + +def write_file(): + """Create or overwrite a file, without going through a shell.""" + from inspect_ai.tool import Tool, tool + + @tool(name="write_file") + def _write_file() -> Tool: + async def execute(path: str, content: str) -> str: + """Write text to a file in the sandbox, replacing it if it exists. + + Prefer this over shell redirection: it needs no quoting or escaping + and behaves the same on every platform. + + Args: + path: File to write, relative to the working directory. + content: The full text the file should contain. + + Returns: + Confirmation of what was written. + """ + from inspect_ai.util import sandbox + + await sandbox().write_file(path, content) + return f"wrote {len(content)} characters to {path}" + + return execute + + return _write_file() + + +def edit_file(): + """Replace one exact occurrence of a string in a file.""" + from inspect_ai.tool import Tool, tool + + @tool(name="edit_file") + def _edit_file() -> Tool: + async def execute(path: str, old_text: str, new_text: str) -> str: + """Replace an exact snippet in a file. + + `old_text` must appear exactly once, so include enough surrounding + context to make it unique. To create a file, use write_file. + + Args: + path: File to edit, relative to the working directory. + old_text: The exact text to replace. + new_text: What to put in its place. + + Returns: + Confirmation, or an explanation of why the edit was refused. + """ + from inspect_ai.util import sandbox + + current = await sandbox().read_file(path, text=True) + found = current.count(old_text) + if found == 0: + return f"no edit made: {path} does not contain that text" + if found > 1: + return ( + f"no edit made: that text appears {found} times in {path}. " + "Include more surrounding context so it matches once." + ) + await sandbox().write_file(path, current.replace(old_text, new_text, 1)) + return f"edited {path}" + + return execute + + return _edit_file() + + +def list_files(): + """List the files the sandbox working directory holds.""" + from inspect_ai.tool import Tool, tool + + @tool(name="list_files") + def _list_files() -> Tool: + async def execute() -> str: + """List every file in the working directory, recursively. + + Returns: + One relative path per line. + """ + paths = await list_paths() + return "\n".join(paths) if paths else "(no files)" + + return execute + + return _list_files() + + +def toolset() -> list: + """The tools a behavioral run gives the agent. + + inspect's `think()` is reused as-is -- it never touches the sandbox, so it + is already platform-neutral. Its `bash()`, `text_editor()`, `list_files()` + and `grep()` are the ones replaced above. + """ + from inspect_ai.tool import think + + return [shell(), write_file(), edit_file(), list_files(), think()] diff --git a/skillscope/schema/machine.schema.json b/skillscope/schema/machine.schema.json index 8f5997e..f3f55a5 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, relative to the skill directory, describing the sandbox the behavioral cases need under the inspect engine. 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/tests/test_skillscope.py b/tests/test_skillscope.py index 11cdee6..a3d0a0d 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -48,6 +48,10 @@ ) from skillscope import selection as select_module from skillscope.datasets import EVALUATIONS_KEY, TRIGGER_KEY +from skillscope.engine import judge as engine_judge +from skillscope.engine import models as engine_models +from skillscope.engine import routing as engine_routing +from skillscope.engine import tools as engine_tools REPO_ROOT = datasets.PACKAGE_DIR.parent SCHEMA_DIR = datasets.PACKAGE_DIR / "schema" @@ -328,13 +332,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(): @@ -2501,5 +2508,198 @@ def test_an_empty_room_leaves_only_the_shared_pool(self) -> None: self.assertTrue(all(case.skill is None for case in cases)) +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(), {}) + + def test_headers_are_parsed_into_default_headers(self) -> None: + os.environ[engine_models.CUSTOM_HEADERS_ENV] = ( + "Ocp-Apim-Subscription-Key: secret\nuser: a1_ucicd\n" + ) + self.assertEqual( + engine_models.model_args(), + { + "default_headers": { + "Ocp-Apim-Subscription-Key": "secret", + "user": "a1_ucicd", + } + }, + ) + + 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_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() + 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 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 _Call: + def __init__(self, function: str, arguments: dict) -> None: + self.function = function + self.arguments = arguments + + +class _Message: + def __init__(self, tool_calls: list | None = None) -> None: + self.tool_calls = tool_calls + + +class TestEngineRoutingActivation(unittest.TestCase): + """Naming a skill through the tool *is* the activation, so it is observed.""" + + def test_a_skill_call_is_the_decision(self) -> None: + messages = [_Message([_Call("skill", {"command": "local-ai-use"})])] + self.assertEqual(engine_routing.activation_of(messages), "local-ai-use") + + def test_no_tool_call_means_nothing_activated(self) -> None: + self.assertIsNone(engine_routing.activation_of([_Message(), _Message([])])) + + def test_another_tool_is_not_an_activation(self) -> None: + messages = [_Message([_Call("think", {"thought": "skill demo-skill?"})])] + self.assertIsNone(engine_routing.activation_of(messages)) + + def test_the_first_skill_named_wins(self) -> None: + messages = [ + _Message([_Call("skill", {"command": "first"})]), + _Message([_Call("skill", {"command": "second"})]), + ] + self.assertEqual(engine_routing.activation_of(messages), "first") + + def test_a_blank_command_is_not_an_activation(self) -> None: + messages = [_Message([_Call("skill", {"command": " "})])] + self.assertIsNone(engine_routing.activation_of(messages)) + + def test_tool_calls_are_counted_across_messages(self) -> None: + messages = [ + _Message([_Call("think", {}), _Call("skill", {"command": "x"})]), + _Message(), + ] + self.assertEqual(engine_routing.tool_call_count(messages), 2) + + if __name__ == "__main__": unittest.main(verbosity=2) From 3f76a2f41496f2fa371702b17f54a172bbb7bce2 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 07:26:12 +0000 Subject: [PATCH 02/33] Record what a graded run spends, and compare engines Neither engine reported its own cost. The legacy engine discards the `total_cost_usd` the CLI returns with every result, and inspect keeps usage in its own log where skillscope's report never looks. That was fine with one engine and nothing to compare it against. Both now record into `usage`, which the report meta carries, and `tools/benchmark_engines.py` runs the same dataset through both and reports per-case agreement plus what each run spent. Agreement is compared case by case rather than in aggregate: two runs can post identical accuracy while individual cases flip in both directions and cancel out. `--noise` runs the legacy engine twice so flips can be read against how much it already disagrees with itself. The spend columns are not equally trustworthy and the report says so rather than tabulating them as though they were. Legacy token counts are a floor -- assistant events omit the system prompt and cached input, and a routing case is killed before its totals arrive -- so cost is its reliable figure. inspect has a cost only when the provider supplies pricing, which a gateway does not, so wall time is often the only column comparable on both sides. --- skillscope/agent.py | 14 +- skillscope/cli.py | 14 +- skillscope/engine/behavioral.py | 5 +- skillscope/engine/routing.py | 3 +- skillscope/engine/stats.py | 33 +++++ skillscope/routing.py | 27 +++- skillscope/usage.py | 73 +++++++++ tools/benchmark_engines.py | 254 ++++++++++++++++++++++++++++++++ 8 files changed, 416 insertions(+), 7 deletions(-) create mode 100644 skillscope/engine/stats.py create mode 100644 skillscope/usage.py create mode 100644 tools/benchmark_engines.py diff --git a/skillscope/agent.py b/skillscope/agent.py index 53bce6d..6302348 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") @@ -387,8 +387,18 @@ def __init__(self, *, workspace: Path, events: list[dict], judge_model: str | No result_text = "" for ev in events: - if ev.get("type") == "result" and isinstance(ev.get("result"), str): + if ev.get("type") != "result": + continue + if isinstance(ev.get("result"), str): result_text = ev["result"] + # The CLI reports what the turn cost; recording it is what lets a + # run be compared against the same cases on the other engine. + tokens = ev.get("usage") or {} + usage.record( + input_tokens=tokens.get("input_tokens", 0), + output_tokens=tokens.get("output_tokens", 0), + cost_usd=ev.get("total_cost_usd"), + ) self.workspace = workspace self.judge_model = judge_model diff --git a/skillscope/cli.py b/skillscope/cli.py index 215cfd2..942a127 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -72,7 +72,17 @@ from concurrent.futures import ThreadPoolExecutor from pathlib import Path -from . import behavior, config, datasets, deadline, engine, 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 @@ -369,6 +379,7 @@ def _finish_routing( "timeout": args.timeout, "isolated_config_dir": isolated, "github_run_id": os.environ.get("GITHUB_RUN_ID"), + **usage.snapshot().as_meta(), **(extra or {}), }, ) @@ -500,6 +511,7 @@ def cmd_behavioral(args: argparse.Namespace) -> int: "wall_time_s": round(time.time() - started, 1), "timeout": args.timeout, "github_run_id": os.environ.get("GITHUB_RUN_ID"), + **usage.snapshot().as_meta(), }, ) _write_report(summary, behavior.render_markdown(summary), args, "behavioral") diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index 31c0cc4..e6d6a42 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -13,10 +13,10 @@ from pathlib import Path -from .. import config, deadline +from .. import config, deadline, usage from ..behavior import BehaviorOutcome from ..datasets import Case -from . import convert, models, sandbox as sandbox_spec, scorers, tools +from . import convert, models, sandbox as sandbox_spec, scorers, stats, tools # 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 @@ -132,6 +132,7 @@ def run( display="plain", ) for log in logs: + stats.record_log(log) outcomes.extend(_outcomes(log, skill, skill_cases)) for outcome in outcomes: diff --git a/skillscope/engine/routing.py b/skillscope/engine/routing.py index 80e06b6..3b42f0e 100644 --- a/skillscope/engine/routing.py +++ b/skillscope/engine/routing.py @@ -40,7 +40,7 @@ from .. import config, deadline from ..datasets import Case from ..routing import PASSING_VERDICTS, Outcome, classify -from . import convert, models +from . import convert, models, stats SKILL_TOOL = "skill" @@ -169,6 +169,7 @@ def run(cases: list[Case], routing_set: dict[str, Path], model: str) -> list[Out by_id = {case.id: case for case in cases} outcomes: list[Outcome] = [] for log in logs: + stats.record_log(log) if log.status == "error" or not log.samples: detail = getattr(log.error, "message", None) or "no samples" raise SystemExit(f"error: routing task failed: {detail}") diff --git a/skillscope/engine/stats.py b/skillscope/engine/stats.py new file mode 100644 index 0000000..631c0ed --- /dev/null +++ b/skillscope/engine/stats.py @@ -0,0 +1,33 @@ +# 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, + ) + + # One "call" per sample is the comparable unit across engines: the legacy + # engine reports once per case, and per-request counts are not visible on + # both sides. + usage.record(calls=len(getattr(log, "samples", None) or [])) diff --git a/skillscope/routing.py b/skillscope/routing.py index fbaac65..ec58b73 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 @@ -308,6 +308,29 @@ def _init_skills(event: dict, skills: list[str]) -> list[str] | None: return seen +def _record_usage(event: dict) -> None: + """Record what one stream event says the run has spent. + + Tokens come from assistant events, one per model response, because a + routing case is normally killed the moment its decision is visible and the + result event that would total them up never arrives. Cost comes only from + the result event, where it is a run total -- so a case that was killed + reports its tokens and no cost, which is the truth about what the legacy + engine can observe rather than an omission. + """ + kind = event.get("type") + if kind == "assistant": + message = event.get("message") + counts = (message or {}).get("usage") if isinstance(message, dict) else None + if isinstance(counts, dict): + usage.record( + input_tokens=counts.get("input_tokens", 0), + output_tokens=counts.get("output_tokens", 0), + ) + elif kind == "result": + usage.record(cost_usd=event.get("total_cost_usd"), calls=0) + + def _init_tools(event: dict) -> set[str] | None: """Tool names the CLI reported at session init, if this is that event. @@ -524,6 +547,7 @@ def run_case(case: Case, routing_set: dict[str, Path], config: RoutingConfig) -> except json.JSONDecodeError: continue events.append(event) + _record_usage(event) reported = _init_skills(event, skills) if reported is not None: @@ -543,6 +567,7 @@ def run_case(case: Case, routing_set: dict[str, Path], config: RoutingConfig) -> if event.get("type") == "result": stop_reason = "result" + _record_usage(event) if event.get("is_error"): error = str(event.get("result") or "result event reported an error")[:400] break diff --git a/skillscope/usage.py b/skillscope/usage.py new file mode 100644 index 0000000..c76ab26 --- /dev/null +++ b/skillscope/usage.py @@ -0,0 +1,73 @@ +# 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) diff --git a/tools/benchmark_engines.py b/tools/benchmark_engines.py new file mode 100644 index 0000000..7b1d43f --- /dev/null +++ b/tools/benchmark_engines.py @@ -0,0 +1,254 @@ +#!/usr/bin/env python3 +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Compare the legacy and inspect 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 inspect.json +""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +import tempfile +from pathlib import Path + +AGREE = "agree" +NEW_PASSES = "only the inspect engine passes" +NEW_FAILS = "only the legacy engine 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) -> dict: + meta = report.get("meta", {}) + return { + "engine": meta.get("engine", "legacy"), + "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 inspect only has 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 -- inspect only has one when the " + "model provider supplies pricing, which a gateway generally does " + "not. Wall time is the comparable column here." + ) + return notes + + +def render(result: dict) -> str: + comparison = result["comparison"] + lines = [ + "## Engine benchmark", + "", + f"**{comparison['agreed']}/{comparison['compared']} cases agree** " + f"between the legacy and inspect engines.", + "", + ] + + noise = result.get("noise") + if noise is not None: + lines += [ + f"Noise floor: the legacy 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 += [ + "| Case | Direction | Legacy | Inspect |", + "| --- | --- | --- | --- |", + ] + 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", "Only the legacy run produced these cases"), + ("only_in_candidate", "Only the inspect 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 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=("LEGACY", "INSPECT"), + help="Compare two reports that already exist instead of running the legs.", + ) + parser.add_argument( + "--noise", + action="store_true", + help=( + "Run the legacy 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") + baseline = run_leg(args.leg, "legacy", passthrough, "legacy") + noise_run = ( + run_leg(args.leg, "legacy", passthrough, "legacy-again") + if args.noise + else None + ) + candidate = run_leg(args.leg, "inspect", passthrough, "inspect") + noise = compare(baseline, noise_run) if noise_run is not None else None + + result = { + "comparison": compare(baseline, candidate), + "noise": noise, + "spend": {"baseline": spend(baseline), "candidate": spend(candidate)}, + } + + 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()) From fd52c3ef40f8a1a4757fe509c01b6e97cc9fc81d Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 07:42:47 +0000 Subject: [PATCH 03/33] Add the Claude Code verification leg The inspect engine grades a skill with a harness-independent agent, which tests whether the skill's instructions work rather than how one product reads them. That choice raises an obvious question, so `--engine claude-code` answers it: run the real harness in the sandbox and see whether it agrees. It slots into the existing flag rather than adding a command, so the benchmark tool can diff its report against either other engine with no new machinery. Reporting only -- harness runs are nondeterministic and the harness is not what we grade, so a divergence is a question about the skill rather than a build failure. Linux only, because inspect_swe shells `bash -c` to find the CLI and the proxy it installs in the guest is a Linux binary. Also fixes how the inspect engines count model calls. They recorded one per sample while the legacy engine records one per assistant response: identical for routing, where a case is a single turn, but a large undercount for behavioral, where the agent loops. The benchmark puts the two columns side by side and names that one comparable, so it had to mean the same thing on both sides. Counted from assistant messages now. --- pyproject.toml | 5 ++ skillscope/cli.py | 14 +++-- skillscope/engine/stats.py | 16 +++-- skillscope/engine/verify.py | 113 ++++++++++++++++++++++++++++++++++++ tools/benchmark_engines.py | 4 +- 5 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 skillscope/engine/verify.py diff --git a/pyproject.toml b/pyproject.toml index d4cf227..66205fa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,11 @@ dependencies = ["pyyaml>=6.0"] # optional, so installing it alone gets you a harness that cannot reach a model. inspect = ["inspect-ai>=0.3.263", "anthropic>=0.40"] +# The Claude Code verification leg (`--engine claude-code`). Separate from +# `inspect` because it is a reporting-only cross-check, not something a graded +# run needs -- and because it only works on a POSIX guest. +verify = ["skillscope[inspect]", "inspect-swe>=0.2.70"] + [project.scripts] skillscope = "skillscope.cli:main" diff --git a/skillscope/cli.py b/skillscope/cli.py index 942a127..701d1e3 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -344,7 +344,7 @@ 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", "legacy") == "inspect": + if getattr(args, "engine", "legacy") in ("inspect", "claude-code"): # The inspect engine never shells out to `claude`, so the CLI-based # reachability probe would be testing something this run does not use. engine.require() @@ -490,11 +490,15 @@ def cmd_behavioral(args: argparse.Namespace) -> int: ) return 0 - if args.engine == "inspect": - from .engine import behavioral as inspect_behavioral + if args.engine in ("inspect", "claude-code"): from .engine import models as engine_models - outcomes = inspect_behavioral.run( + if args.engine == "inspect": + from .engine import behavioral as runner + else: + from .engine import verify as runner + + outcomes = runner.run( skills, gradable, engine_models.resolve(args.model), args.effort ) else: @@ -627,7 +631,7 @@ def _add_graded_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument( "--engine", default=os.environ.get("SKILLSCOPE_ENGINE", "legacy"), - choices=["legacy", "inspect"], + choices=["legacy", "inspect", "claude-code"], help=( "Which eval engine runs the cases. `legacy` drives the claude CLI " "directly; `inspect` runs a harness-independent agent through " diff --git a/skillscope/engine/stats.py b/skillscope/engine/stats.py index 631c0ed..c28160c 100644 --- a/skillscope/engine/stats.py +++ b/skillscope/engine/stats.py @@ -27,7 +27,15 @@ def record_log(log) -> None: calls=0, ) - # One "call" per sample is the comparable unit across engines: the legacy - # engine reports once per case, and per-request counts are not visible on - # both sides. - usage.record(calls=len(getattr(log, "samples", None) or [])) + # 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/verify.py b/skillscope/engine/verify.py new file mode 100644 index 0000000..707f903 --- /dev/null +++ b/skillscope/engine/verify.py @@ -0,0 +1,113 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""The Claude Code verification leg (`--engine claude-code`). + +The `inspect` engine grades a skill with a harness-independent agent, which is +a deliberate choice: it tests whether a skill's *instructions* work rather than +how one product reads them. This leg exists to answer the question that choice +raises -- do the results still hold under the real thing? + +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, models, sandbox as sandbox_spec, scorers, stats + +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." + ) + try: + import inspect_swe # noqa: F401 + except ModuleNotFoundError as exc: # pragma: no cover -- environment shape + raise SystemExit(INSTALL_HINT) from exc + + +def build_task(skill: str, cases: list[Case], ctx: dict | None = None): + """One task per skill, solved by real Claude Code rather than our agent.""" + from inspect_ai import Task + 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] + + bound = deadline.active() + return Task( + name=f"claude-code-{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. + solver=claude_code(skills=[skill_dir]), + scorer=scorers.expectations(), + sandbox=sandbox_spec.for_skill(skill), + message_limit=behavioral.MESSAGE_LIMIT, + time_limit=int(bound.remaining()) if bound is not None else None, + ) + + +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() + + 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=model, + model_args=models.model_args(), + log_dir=str(Path(".skillscope") / "logs"), + 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/tools/benchmark_engines.py b/tools/benchmark_engines.py index 7b1d43f..2064e00 100644 --- a/tools/benchmark_engines.py +++ b/tools/benchmark_engines.py @@ -136,7 +136,9 @@ def _spend_caveats(spend: dict) -> list[str]: notes.append( "> One engine reported no cost -- inspect only has one when the " "model provider supplies pricing, which a gateway generally does " - "not. Wall time is the comparable column here." + "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 From ef3365f93d570418e7761177f51b423c37218f50 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 07:58:12 +0000 Subject: [PATCH 04/33] Count legacy model calls the same way in both commands The two legacy commands read the same `claude` stream but recorded from different events: routing counted assistant replies, behavioral counted only the result event, which is one per case. A behavioral run of three cases reported three model calls while costing $0.72 -- far more than three small calls -- and the benchmark puts that column beside the other engine's and calls it comparable. Both now go through one recorder in `usage`. The same three cases report fourteen calls against the new engine's twelve, which is the like-for- like reading: two agents looping to similar depth. --- skillscope/agent.py | 15 ++++----------- skillscope/routing.py | 27 ++------------------------- skillscope/usage.py | 24 ++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 36 deletions(-) diff --git a/skillscope/agent.py b/skillscope/agent.py index 6302348..88165c6 100644 --- a/skillscope/agent.py +++ b/skillscope/agent.py @@ -387,18 +387,11 @@ def __init__(self, *, workspace: Path, events: list[dict], judge_model: str | No result_text = "" for ev in events: - if ev.get("type") != "result": - continue - if isinstance(ev.get("result"), str): + # 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"] - # The CLI reports what the turn cost; recording it is what lets a - # run be compared against the same cases on the other engine. - tokens = ev.get("usage") or {} - usage.record( - input_tokens=tokens.get("input_tokens", 0), - output_tokens=tokens.get("output_tokens", 0), - cost_usd=ev.get("total_cost_usd"), - ) self.workspace = workspace self.judge_model = judge_model diff --git a/skillscope/routing.py b/skillscope/routing.py index ec58b73..6b28d83 100644 --- a/skillscope/routing.py +++ b/skillscope/routing.py @@ -308,29 +308,6 @@ def _init_skills(event: dict, skills: list[str]) -> list[str] | None: return seen -def _record_usage(event: dict) -> None: - """Record what one stream event says the run has spent. - - Tokens come from assistant events, one per model response, because a - routing case is normally killed the moment its decision is visible and the - result event that would total them up never arrives. Cost comes only from - the result event, where it is a run total -- so a case that was killed - reports its tokens and no cost, which is the truth about what the legacy - engine can observe rather than an omission. - """ - kind = event.get("type") - if kind == "assistant": - message = event.get("message") - counts = (message or {}).get("usage") if isinstance(message, dict) else None - if isinstance(counts, dict): - usage.record( - input_tokens=counts.get("input_tokens", 0), - output_tokens=counts.get("output_tokens", 0), - ) - elif kind == "result": - usage.record(cost_usd=event.get("total_cost_usd"), calls=0) - - def _init_tools(event: dict) -> set[str] | None: """Tool names the CLI reported at session init, if this is that event. @@ -547,7 +524,7 @@ def run_case(case: Case, routing_set: dict[str, Path], config: RoutingConfig) -> except json.JSONDecodeError: continue events.append(event) - _record_usage(event) + usage.record_stream_event(event) reported = _init_skills(event, skills) if reported is not None: @@ -567,7 +544,7 @@ def run_case(case: Case, routing_set: dict[str, Path], config: RoutingConfig) -> if event.get("type") == "result": stop_reason = "result" - _record_usage(event) + usage.record_stream_event(event) if event.get("is_error"): error = str(event.get("result") or "result event reported an error")[:400] break diff --git a/skillscope/usage.py b/skillscope/usage.py index c76ab26..2160a1c 100644 --- a/skillscope/usage.py +++ b/skillscope/usage.py @@ -71,3 +71,27 @@ def record( _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) From a97d447ec0f2e8d897936f769952db3660cd6ff0 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 07:58:54 +0000 Subject: [PATCH 05/33] Document the engine choice `--engine` changes what drives the agent while leaving the dataset, the CLI and the reports identical, so it belongs in usage rather than being discoverable only from --help. Says what each engine needs, why the inspect one is cheaper, why the claude-code leg never gates, and where the sandbox does and does not exist. --- docs/usage.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/docs/usage.md b/docs/usage.md index 68164ef..81b6998 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -208,6 +208,41 @@ 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 | Needs | +| --- | --- | --- | +| `legacy` (default) | The `claude` CLI, driven directly | the CLI on `PATH` | +| `inspect` | A harness-independent agent through `inspect_ai` | `pip install 'skillscope[inspect]'` | +| `claude-code` | Real Claude Code inside the sandbox, to cross-check the other two | `skillscope[verify]`, Linux only | + +`inspect` grades a skill on whether its *instructions* work rather than on how +one product reads them, which is the stronger claim and the one a product repo +can adopt. It is also much cheaper: a routing case is a single model call, +because the decision is visible in the first reply and nothing needs executing. + +`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. + +Under `inspect`, behavioral cases run in a Docker container on Linux and +unsandboxed on Windows -- inspect's sandbox layer assumes a POSIX guest, so the +Windows legs trade isolation for running on the platform they are meant to +test. A skill that needs network egress or a device names a compose file with +`sandbox:` in its `evals/machine.yml`. `SKILLSCOPE_SANDBOX=local` skips the +container entirely, which is for working locally rather than for CI: a graded +run that quietly dropped its sandbox would report the same numbers with none of +the isolation. + +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 From ff499f79a0c0ad02ad7752782cda5f0622f07109 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 08:16:27 +0000 Subject: [PATCH 06/33] Make the sandbox provider selectable, and stop it eating the skill's config Which provider to use is a property of the runner -- docker, podman on a host that has that instead, local where there is no container -- and whoever runs the job knows it while a skill does not. What the sandbox has to provide is the opposite: the skill declares a compose file when it needs network egress or a device. `SKILLSCOPE_SANDBOX` already selected the provider but returned it bare, discarding any compose file the skill had declared. A skill that needs the network would then run without it and fail for a reason nothing in the report explains. The two are now resolved independently. podman works with no registration: `inspect-podman` publishes an `inspect_ai` entry point and inspect resolves the bare name, so installing the extra is the whole setup. --- docs/usage.md | 28 ++++++++++---- pyproject.toml | 5 +++ skillscope/engine/sandbox.py | 71 +++++++++++++++++++++++------------- tests/test_skillscope.py | 53 +++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 33 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 81b6998..6cb157e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -229,14 +229,26 @@ because the decision is visible in the first reply and nothing needs executing. 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. -Under `inspect`, behavioral cases run in a Docker container on Linux and -unsandboxed on Windows -- inspect's sandbox layer assumes a POSIX guest, so the -Windows legs trade isolation for running on the platform they are meant to -test. A skill that needs network egress or a device names a compose file with -`sandbox:` in its `evals/machine.yml`. `SKILLSCOPE_SANDBOX=local` skips the -container entirely, which is for working locally rather than for CI: a graded -run that quietly dropped its sandbox would report the same numbers with none of -the isolation. +### Where an `inspect` 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 (`pip install 'skillscope[podman]'` -- the provider registers itself, so +installing it is the whole setup); `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. + +**What the sandbox must provide** is a property of the skill, declared as +`sandbox: compose.yaml` in its `evals/machine.yml`. 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. + +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 diff --git a/pyproject.toml b/pyproject.toml index 66205fa..e849eb6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,11 @@ dependencies = ["pyyaml>=6.0"] # optional, so installing it alone gets you a harness that cannot reach a model. inspect = ["inspect-ai>=0.3.263", "anthropic>=0.40"] +# 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`). Separate from # `inspect` because it is a reporting-only cross-check, not something a graded # run needs -- and because it only works on a POSIX guest. diff --git a/skillscope/engine/sandbox.py b/skillscope/engine/sandbox.py index 55a8251..c47aaa2 100644 --- a/skillscope/engine/sandbox.py +++ b/skillscope/engine/sandbox.py @@ -4,15 +4,24 @@ """Which sandbox a skill's cases run in. -Docker on Linux, `local` on Windows. inspect's sandbox layer -- and every tool -built on it -- assumes a POSIX guest, so there is no Windows container option -here; the Windows legs trade isolation for running on the platform they are -meant to test. DevLab's ephemeral, off-network runners are what covers that gap. - -A skill declares its needs in `evals/machine.yml`, which already exists to say -what class of machine a skill wants. An optional `sandbox:` key names a compose -file relative to the skill directory, so a skill that must reach the network to -pull a model, or needs `/dev/dri`, says so instead of every skill paying for it. +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 @@ -22,37 +31,49 @@ from .. import datasets -# Escape hatch for local development and wiring runs: `SKILLSCOPE_SANDBOX=local` -# skips the container entirely. Not for CI -- a graded run that quietly dropped -# its sandbox would report the same numbers with none of the isolation. +# 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" -# Hardware-free skills get no network. Skills that need egress ship their own -# compose file and opt out of this default. -DEFAULT_COMPOSE = "compose.yaml" +DEFAULT_PROVIDER = "docker" + +# Providers that take no configuration, so a skill's compose file cannot apply. +UNCONFIGURED = {"local"} def is_windows() -> bool: return sys.platform.startswith("win") -def for_skill(skill: str): - """The `sandbox` spec for a skill's task, or None to use inspect's default. - - Returns a `(type, config)` tuple when a compose file is declared, a bare - type name otherwise -- both are accepted as `Task(sandbox=...)`. - """ +def provider() -> str: + """The sandbox provider for this run.""" override = os.environ.get(SANDBOX_ENV, "").strip() 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) - if compose is not None: - return ("docker", str(compose)) - return "docker" + return (name, str(compose)) if compose is not None else name def _declared_compose(skill: str): diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index a3d0a0d..f71788d 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -51,6 +51,7 @@ from skillscope.engine import judge as engine_judge from skillscope.engine import models as engine_models from skillscope.engine import routing as engine_routing +from skillscope.engine import sandbox as engine_sandbox from skillscope.engine import tools as engine_tools REPO_ROOT = datasets.PACKAGE_DIR.parent @@ -2701,5 +2702,57 @@ def test_tool_calls_are_counted_across_messages(self) -> None: self.assertEqual(engine_routing.tool_call_count(messages), 2) +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) + + def _skill(self, machine: str | None = None, compose: bool = False) -> None: + folder = self.repo.skill( + "boxed", dataset=tier0_dataset("boxed"), machine=machine + ) + if compose: + (folder / "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_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)) + + if __name__ == "__main__": unittest.main(verbosity=2) From 7b54f6870715d7ca0dede5474a000dc2bcaebddd Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 08:28:24 +0000 Subject: [PATCH 07/33] Say in the report whether the run was isolated A behavioral report showed the same numbers whether the agent had been contained or had worked directly in the harness's filesystem with permissions bypassed, which invites the reader to assume the former. Both engines now record what held the run, and the markdown says it outright -- the legacy engine reports `host`, and the Windows legs, which have no sandbox available at all, no longer look like the Linux ones. Routing under the inspect engine reports `none` rather than a provider: it offers the skill tool and never calls it, so nothing is executed and there is nothing to isolate. That is not the same claim as unprotected. Also stops the gateway-versus-federation guard firing on runs that reach no provider. `--model mockllm/model` is the wiring check that costs nothing, and refusing it because the shell holds both Anthropic variables broke it on exactly the machines most likely to have an OAuth token. --- skillscope/behavior.py | 21 +++++++++++++++++++++ skillscope/cli.py | 24 ++++++++++++++++++++++++ skillscope/engine/behavioral.py | 2 +- skillscope/engine/models.py | 10 +++++++++- skillscope/engine/routing.py | 2 +- skillscope/engine/sandbox.py | 16 ++++++++++++++++ skillscope/engine/verify.py | 2 +- tests/test_skillscope.py | 14 +++++++++++--- 8 files changed, 84 insertions(+), 7 deletions(-) diff --git a/skillscope/behavior.py b/skillscope/behavior.py index 154f48c..cac3a15 100644 --- a/skillscope/behavior.py +++ b/skillscope/behavior.py @@ -210,6 +210,25 @@ def summarize(outcomes: list[BehaviorOutcome], meta: dict) -> dict: } +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 +239,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 701d1e3..8a54818 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -356,6 +356,20 @@ def _prepare_graded_run( return selected +def _sandbox_meta(args: argparse.Namespace) -> dict: + """What contained this run, recorded so the report does not have to imply it. + + The legacy engine runs the agent on the host with permissions bypassed, and + saying so in the artifact is the point: the same numbers mean different + things depending on whether anything was isolated. + """ + if getattr(args, "engine", "legacy") == "legacy": + return {"sandbox": "host", "sandbox_isolated": False} + from .engine import sandbox as engine_sandbox + + return engine_sandbox.describe() + + def _finish_routing( args: argparse.Namespace, outcomes: list, @@ -380,6 +394,15 @@ def _finish_routing( "isolated_config_dir": isolated, "github_run_id": os.environ.get("GITHUB_RUN_ID"), **usage.snapshot().as_meta(), + # Routing under the inspect engine executes nothing -- the skill + # tool is offered and never called -- so there is no sandbox and + # nothing to isolate. Saying "none" is not the same as saying the + # run was unprotected. + **( + {"sandbox": "none", "sandbox_isolated": None} + if args.engine == "inspect" + else _sandbox_meta(args) + ), **(extra or {}), }, ) @@ -516,6 +539,7 @@ def cmd_behavioral(args: argparse.Namespace) -> int: "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") diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index e6d6a42..142e7fd 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -124,7 +124,7 @@ def run( logs = inspect_eval( build_task(skill, skill_cases), model=model, - model_args=models.model_args(), + model_args=models.model_args(model), log_dir=str(Path(".skillscope") / "logs"), # skillscope's own progress lines are the report; inspect's rich # display takes over the terminal and produces nothing useful when diff --git a/skillscope/engine/models.py b/skillscope/engine/models.py index 1feca0d..3026361 100644 --- a/skillscope/engine/models.py +++ b/skillscope/engine/models.py @@ -49,13 +49,21 @@ def custom_headers() -> dict[str, str]: return headers -def model_args() -> dict: +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 {} diff --git a/skillscope/engine/routing.py b/skillscope/engine/routing.py index 3b42f0e..1d6ed3b 100644 --- a/skillscope/engine/routing.py +++ b/skillscope/engine/routing.py @@ -161,7 +161,7 @@ def run(cases: list[Case], routing_set: dict[str, Path], model: str) -> list[Out logs = inspect_eval( build_task(cases, routing_set), model=model, - model_args=models.model_args(), + model_args=models.model_args(model), log_dir=str(Path(".skillscope") / "logs"), display="plain", ) diff --git a/skillscope/engine/sandbox.py b/skillscope/engine/sandbox.py index c47aaa2..aef67b6 100644 --- a/skillscope/engine/sandbox.py +++ b/skillscope/engine/sandbox.py @@ -42,6 +42,22 @@ # 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} + def is_windows() -> bool: return sys.platform.startswith("win") diff --git a/skillscope/engine/verify.py b/skillscope/engine/verify.py index 707f903..725ef91 100644 --- a/skillscope/engine/verify.py +++ b/skillscope/engine/verify.py @@ -94,7 +94,7 @@ def run( logs = inspect_eval( build_task(skill, skill_cases), model=model, - model_args=models.model_args(), + model_args=models.model_args(model), log_dir=str(Path(".skillscope") / "logs"), display="plain", ) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index f71788d..ab7eccf 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2535,14 +2535,14 @@ def setUp(self) -> None: os.environ.pop(var, None) def test_no_headers_configured_means_no_provider_arguments(self) -> None: - self.assertEqual(engine_models.model_args(), {}) + 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] = ( "Ocp-Apim-Subscription-Key: secret\nuser: a1_ucicd\n" ) self.assertEqual( - engine_models.model_args(), + engine_models.model_args("anthropic/claude-opus-5"), { "default_headers": { "Ocp-Apim-Subscription-Key": "secret", @@ -2561,13 +2561,21 @@ 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() + engine_models.model_args("anthropic/claude-opus-5") self.assertIn(engine_models.AUTH_TOKEN_ENV, str(caught.exception)) From 991ac6dc519a93b138af2d42765321025709a733 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 08:33:39 +0000 Subject: [PATCH 08/33] Check the model answers before starting containers A graded run on the inspect engine starts a sandbox and installs a skill before it first reaches a provider, so a bad key surfaced as a task that failed after all that work -- a 401 buried in a sample error, eighteen seconds and a container in. One tiny call up front turns that into a message on the first line. The legacy engine already had this; the inspect one skipped it on the grounds that the CLI-based probe tested the wrong thing, which was true and left nothing in its place. mockllm reaches no provider, so it is skipped rather than charged for a round trip that proves nothing. --- skillscope/cli.py | 12 ++++++++++-- skillscope/engine/models.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/skillscope/cli.py b/skillscope/cli.py index 8a54818..182b3de 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -345,9 +345,17 @@ def _prepare_graded_run( _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", "legacy") in ("inspect", "claude-code"): - # The inspect engine never shells out to `claude`, so the CLI-based - # reachability probe would be testing something this run does not use. + # 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() + 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}") return selected if not args.skip_preflight: ok, detail = check_api_reachable(args.model) diff --git a/skillscope/engine/models.py b/skillscope/engine/models.py index 3026361..9c2fbff 100644 --- a/skillscope/engine/models.py +++ b/skillscope/engine/models.py @@ -37,6 +37,36 @@ def resolve(model: str) -> str: return ALIASES.get(model.lower(), f"anthropic/{model}") +async def _probe(model: str): + from inspect_ai.model import get_model + + resolved = get_model(model, **model_args(model)) + return await resolved.generate("Reply with the single word: ok") + + +def check_reachable(model: str) -> 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. + + 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)" + + import anyio + + try: + output = anyio.run(_probe, model) + except Exception as exc: # noqa: BLE001 -- the reason is the return value + return False, f"{type(exc).__name__}: {exc}"[:400] + return True, (output.completion or "").strip()[:40] + + def custom_headers() -> dict[str, str]: """Parse ``ANTHROPIC_CUSTOM_HEADERS`` (newline-separated ``Key: value``).""" headers: dict[str, str] = {} From 4df25049cc06a998964129480e51e359f697be02 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 08:44:06 +0000 Subject: [PATCH 09/33] Stop a broken sandbox looking like an idle agent `list_paths` returned an empty list when the listing command failed, so a sandbox that could not be listed was indistinguishable from one the agent had left empty. That is not a cosmetic difference: `files_exist` fails, and the judge builds its evidence from the same listing, so every judged expectation fails too with "no files were produced". Nine checks blame the skill for what the harness did. It now raises. The scorer reports that it could not list the sandbox, and the judge declines to rule rather than being handed an empty workspace as fact -- a judge told "no files" will confidently conclude the agent did nothing. --- skillscope/engine/judge.py | 11 ++++++++++- skillscope/engine/scorers.py | 32 +++++++++++++++++++++----------- skillscope/engine/tools.py | 20 ++++++++++++++++++-- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/skillscope/engine/judge.py b/skillscope/engine/judge.py index 330401b..02efaee 100644 --- a/skillscope/engine/judge.py +++ b/skillscope/engine/judge.py @@ -150,7 +150,16 @@ async def grade( get_model, ) - paths = await tools_list_paths() + 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( diff --git a/skillscope/engine/scorers.py b/skillscope/engine/scorers.py index 58f0dea..e465f57 100644 --- a/skillscope/engine/scorers.py +++ b/skillscope/engine/scorers.py @@ -75,17 +75,27 @@ async def score(state, target) -> "Score": wanted = meta.get(convert.FILES_EXIST, []) if wanted: - files = await tools.list_paths() - 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) - ) + 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. diff --git a/skillscope/engine/tools.py b/skillscope/engine/tools.py index 71f8630..f191f90 100644 --- a/skillscope/engine/tools.py +++ b/skillscope/engine/tools.py @@ -77,13 +77,26 @@ def normalize_listing(stdout: str) -> list[str]: 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: - return [] + raise ListingFailed( + f"`{listing}` failed in the sandbox (exit {result.returncode}). " + f"stderr: {result.stderr.strip()[:200] or '(none)'}" + ) return normalize_listing(result.stdout) @@ -203,7 +216,10 @@ async def execute() -> str: Returns: One relative path per line. """ - paths = await list_paths() + try: + paths = await list_paths() + except ListingFailed as exc: + return f"could not list the directory: {exc}" return "\n".join(paths) if paths else "(no files)" return execute From 6a3719d597a0b59d025649a01a2d50713cecb893 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 09:56:23 +0000 Subject: [PATCH 10/33] Give a containerised case a working directory A container sandbox starts at `/`. So `write_file("router.json")` landed at `/router.json`, `find . -type f` walked the whole image and died on `/proc`, and the judge -- had the listing survived -- would have read twenty arbitrary system files as the case's artifacts. An agent left to guess reasonably tried `/app`, then `~`, and scattered its output. Everything a case does now happens in `/workspace`: fixtures are seeded there, the file tools resolve against it, shell commands run in it, and it is what gets listed. The agent is told so, because where its output lands should not be something the dataset has to predict. `local` keeps the harness's own working directory -- it already has a sensible one, and creating `/workspace` on someone's machine would not be. inspect_swe solves the same problem the same way; its agent cwd falls back to the home directory when the sandbox default is `/`. --- skillscope/engine/behavioral.py | 19 ++++++++++- skillscope/engine/convert.py | 10 +++++- skillscope/engine/tools.py | 58 ++++++++++++++++++++++++++++++--- 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index 142e7fd..ec71c66 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -37,6 +37,23 @@ def _tools(skill_dir: Path) -> list: return [skill([skill_dir]), *tools.toolset()] +def _prompt() -> str | None: + """Tell the agent where its work belongs, when that is not obvious. + + A container sandbox starts at `/`, and an agent left to guess reasonably + tries `/app`, then `~`, and scatters its output. What a case produced then + depends on where the agent happened to `cd`, which is not something the + dataset should have to predict. + """ + if not tools.containerized(): + return None + return ( + f"Your working directory is {tools.WORKDIR}. Create and edit files " + "there, using paths relative to it, so the work you produce can be " + "found afterwards." + ) + + def build_task(skill: str, cases: list[Case], ctx: dict | None = None): """One inspect `Task` per skill: its cases, its skill installed, its scorer.""" from inspect_ai import Task @@ -49,7 +66,7 @@ def build_task(skill: str, cases: list[Case], ctx: dict | None = None): return Task( name=f"behavioral-{skill}", dataset=samples, - solver=react(tools=_tools(skill_dir)), + solver=react(prompt=_prompt(), tools=_tools(skill_dir)), scorer=scorers.expectations(), sandbox=sandbox_spec.for_skill(skill), message_limit=MESSAGE_LIMIT, diff --git a/skillscope/engine/convert.py b/skillscope/engine/convert.py index 7143a17..75b9a63 100644 --- a/skillscope/engine/convert.py +++ b/skillscope/engine/convert.py @@ -44,11 +44,19 @@ def seed_files(seed: Path) -> dict[str, str]: 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[target] = str(path) + files[prefix + target] = str(path) return files diff --git a/skillscope/engine/tools.py b/skillscope/engine/tools.py index f191f90..eb9d873 100644 --- a/skillscope/engine/tools.py +++ b/skillscope/engine/tools.py @@ -20,6 +20,15 @@ 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"] @@ -49,12 +58,50 @@ async def shell_prefix() -> list[str]: 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 + + +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.""" + """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], timeout=timeout) + return await sandbox().exec( + prefix + [command], cwd=await workdir(), timeout=timeout + ) def normalize_listing(stdout: str) -> list[str]: @@ -157,7 +204,7 @@ async def execute(path: str, content: str) -> str: """ from inspect_ai.util import sandbox - await sandbox().write_file(path, content) + await sandbox().write_file(await resolve(path), content) return f"wrote {len(content)} characters to {path}" return execute @@ -187,7 +234,8 @@ async def execute(path: str, old_text: str, new_text: str) -> str: """ from inspect_ai.util import sandbox - current = await sandbox().read_file(path, text=True) + target = await resolve(path) + current = await sandbox().read_file(target, text=True) found = current.count(old_text) if found == 0: return f"no edit made: {path} does not contain that text" @@ -196,7 +244,7 @@ async def execute(path: str, old_text: str, new_text: str) -> str: f"no edit made: that text appears {found} times in {path}. " "Include more surrounding context so it matches once." ) - await sandbox().write_file(path, current.replace(old_text, new_text, 1)) + await sandbox().write_file(target, current.replace(old_text, new_text, 1)) return f"edited {path}" return execute From 1400d9054f1ca5b9de916c1495a9df469148c5bb Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 10:02:45 +0000 Subject: [PATCH 11/33] Let the judge read the artifacts, and hear what the agent said Two reasons a case could not pass in a container. The judge read artifacts by the path `list_paths` reported, which is relative to the case's working directory, while `read_file` resolves against the sandbox's own -- `/`. Every artifact came back unreadable and the judge concluded, reasonably, that nothing had been produced. It now resolves them the same way the tools do. And it never saw the agent's final message, so an expectation about what the agent *told* the user -- "output the curl commands they need" -- was unanswerable however well the agent had done. The legacy judge was given both halves and this one was not. They stay labelled apart, because the distinction is what stops an agent writing "I won't call the cloud API" from settling an expectation that it avoided doing so: actions are judged from tool calls and artifacts, and only what the agent told the user is judged from its message. --- skillscope/engine/judge.py | 50 +++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/skillscope/engine/judge.py b/skillscope/engine/judge.py index 02efaee..277aa60 100644 --- a/skillscope/engine/judge.py +++ b/skillscope/engine/judge.py @@ -88,6 +88,33 @@ def parse_verdict(text: str) -> tuple[bool, str] | None: 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. + """ + for message in reversed(state.messages): + if getattr(message, "role", None) != "assistant": + continue + content = getattr(message, "content", None) + if isinstance(content, str) and content.strip(): + return content.strip()[:MAX_TRANSCRIPT] + if isinstance(content, list): + texts = [ + part.text + for part in content + if isinstance(getattr(part, "text", None), str) + ] + if any(t.strip() for t in texts): + return "\n".join(texts).strip()[:MAX_TRANSCRIPT] + return "(the agent said nothing)" + + def transcript_of(state) -> str: """What the agent did: tool calls and their results, never its prose.""" parts: list[str] = [] @@ -108,13 +135,20 @@ 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(path, text=False))) + 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 @@ -122,7 +156,7 @@ async def artifacts(paths: list[str]) -> tuple[list[str], list[tuple[str, bytes] described.append(f"--- {path} (binary) ---") continue try: - body = await sandbox().read_file(path, text=True) + body = await sandbox().read_file(target, text=True) except Exception as exc: # noqa: BLE001 described.append(f"--- {path} (unreadable: {exc}) ---") continue @@ -166,10 +200,13 @@ async def grade( [ f"Files the agent left behind: {paths or 'none'}", "", - "--- what the agent did ---", + "--- what the agent DID (tool calls and their results) ---", transcript_of(state), "", - "--- artifacts ---", + "--- what the agent SAID to the user (its final message) ---", + final_message_of(state), + "", + "--- artifacts it produced ---", *described, ] ) @@ -179,6 +216,11 @@ async def grade( 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" From 27660aeeefc94b236e0251ac548c7fb17af8b932 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 10:08:21 +0000 Subject: [PATCH 12/33] Stop truncation hiding the action a check turns on The judge's transcript was cut at the end, so the agent's most recent tool calls were the first to go -- and those are usually the ones an expectation is about. An agent writes a file, validates it, and reports the result; losing the validation makes the run look like it claimed something it never did, which is precisely how the last failing check read: "only the agent's unsupported claim". Tool calls are now kept whole, since each is short and the list of them is the record of what happened. Only results are capped, individually, because they are what grows without bound. If the whole still overflows, the middle goes rather than the end. --- skillscope/engine/judge.py | 33 +++++++++++++++++++++++++++------ tests/test_skillscope.py | 17 +++++++++++++++++ 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/skillscope/engine/judge.py b/skillscope/engine/judge.py index 277aa60..6416be0 100644 --- a/skillscope/engine/judge.py +++ b/skillscope/engine/judge.py @@ -28,7 +28,13 @@ # about, not everything on disk. MAX_FILES = 20 MAX_FILE_BYTES = 20_000 -MAX_TRANSCRIPT = 6_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"} @@ -115,6 +121,21 @@ def final_message_of(state) -> str: return "(the agent said nothing)" +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] = [] @@ -124,11 +145,11 @@ def transcript_of(state) -> str: if getattr(message, "role", "") == "tool": content = getattr(message, "content", None) if isinstance(content, str): - parts.append(content) - text = "\n".join(parts) - if len(text) > MAX_TRANSCRIPT: - text = text[:MAX_TRANSCRIPT] + "\n...[truncated]..." - return text + 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]]]: diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index ab7eccf..118e1b7 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2655,6 +2655,23 @@ def test_a_must_not_requirement_asks_whether_it_was_avoided(self) -> None: 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 TestEngineJudgeArtifacts(unittest.TestCase): def test_images_are_recognised_by_suffix(self) -> None: self.assertTrue(engine_judge.is_image("out.PNG")) From 98ca77fb900e23295dfb2981bd3f5b8e055b1625 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 10:45:05 +0000 Subject: [PATCH 13/33] Use neutral values in the header-parsing fixture This repository is public and the fixture named a real internal CI service account alongside the gateway's header, which together describe how model access is fronted. The test is about parsing `Key: value` lines; the values were never the point. --- tests/test_skillscope.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 118e1b7..14741c0 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2539,14 +2539,14 @@ def test_no_headers_configured_means_no_provider_arguments(self) -> None: def test_headers_are_parsed_into_default_headers(self) -> None: os.environ[engine_models.CUSTOM_HEADERS_ENV] = ( - "Ocp-Apim-Subscription-Key: secret\nuser: a1_ucicd\n" + "X-Subscription-Key: secret\nuser: ci-runner\n" ) self.assertEqual( engine_models.model_args("anthropic/claude-opus-5"), { "default_headers": { - "Ocp-Apim-Subscription-Key": "secret", - "user": "a1_ucicd", + "X-Subscription-Key": "secret", + "user": "ci-runner", } }, ) From de64e5963a1e1893ecc4a4ae7c46d12936f04693 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 10:50:23 +0000 Subject: [PATCH 14/33] Exercise the inspect engine in CI, on both sandboxes Every bug in this branch was found by running against a real container, and none of them would have been caught by the unit suite. `mockllm` reaches no provider, so the machinery around the model can be driven for free, with no key, on a pull request from a fork -- on Ubuntu, where Docker is the default sandbox, and on Windows, where there is none and the cross-platform tools are the only thing that works. The assertions are about the harness rather than the skill, because the mock satisfies nothing. It checks that no case errored, that the sandbox that ran is the one the platform should have chosen, and that no check failed because the sandbox could not be listed -- which otherwise reports identically to an agent that did nothing. The load-bearing one is the seeded fixture. The file exists because the case staged it, not because the agent acted, so `files_exist` passing proves the whole path: staged into the working directory, listed there, and matched against what the case asked for. That is exactly the chain that was broken in a container an hour ago. --- .github/workflows/selftest.yml | 98 ++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index 5a68438..8431b79 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -44,6 +44,104 @@ jobs: - name: Run the suite run: python -m unittest discover -s tests -t . --verbose + engine: + name: Inspect engine (${{ 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 tools are + # exercised at all -- inspect's own assume a POSIX guest. + os: [ubuntu-latest, windows-latest] + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install with the inspect extra + run: python -m pip install --upgrade pip && python -m pip install ".[inspect]" + + # `mockllm` reaches no provider, so this needs no key and runs on a pull + # request from a fork. It cannot tell us whether a skill is any good -- + # the mock does no work and every judged expectation fails. What it does + # tell us is whether the machinery around the model still holds together + # on both platforms, which is where every 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 + + # Exits non-zero because the mock satisfies nothing; the report is the + # artifact under test, not the exit code. + - name: Grade the fixture on the inspect engine + continue-on-error: true + working-directory: fixture + run: > + python -m skillscope behavioral --engine inspect + --model mockllm/model --skills-dir '*' --skill demo-skill + --output ../engine-report.json + + - name: Check the machinery held + shell: python + env: + EXPECTED_SANDBOX: ${{ matrix.os == 'windows-latest' && 'local' || 'docker' }} + run: | + import json + import os + + report = json.load(open("engine-report.json", encoding="utf-8")) + meta, totals = report["meta"], report["totals"] + print(json.dumps(meta, indent=2)) + + # An infrastructure failure is not a graded result. This is what + # catches a sandbox that would not start. + assert totals["errors"] == 0, report["cases"] + + expected = os.environ["EXPECTED_SANDBOX"] + assert meta["sandbox"] == expected, f"ran in {meta['sandbox']!r}, wanted {expected!r}" + + checks = [c for case in report["cases"] for c in case["checks"]] + assert checks, "nothing was graded, so nothing was proven" + + # A sandbox that cannot be listed reports the same shape as an agent + # that produced nothing, so the difference is asserted explicitly. + broken = [c for c in checks if "could not list the sandbox" in (c["detail"] or "")] + assert not broken, broken + + # The seeded file exists because the case put it there, not because + # the mock did anything. It passing proves the whole path: the fixture + # was staged into the working directory, the directory was listed, and + # the listing was matched against what the case asked for. + seeded = [c for c in checks if c["kind"] == "files_exist"] + assert seeded, "the seeded-file check did not run" + assert all(c["passed"] for c in seeded), seeded + print(f"{len(checks)} checks graded, seeded fixture found, sandbox {expected}.") + action: name: Action against a throwaway repo runs-on: ubuntu-latest From f7d5124dfabb8adea3cd0dd6b62f9043211e297e Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 11:03:28 +0000 Subject: [PATCH 15/33] Fix what CI found: platform-blind tests and a misplaced fixture root Two failures on the first run of the new job, both worth having. The sandbox-selection tests asserted `docker` on any host, which is only true on a POSIX one -- on the Windows runners the answer is `local`, because inspect's sandbox layer assumes a POSIX guest. They pin the platform now and assert both answers, which is what they should have said in the first place. The engine job pointed at its fixture with `working-directory`, but a `--skills-dir` glob that was passed is resolved against the repo root, and `find_root` takes that from the nearest `.git`. Running from a subdirectory of a checkout still globbed the checkout, so the fixture was never found. `SKILLSCOPE_REPO` is the documented way to say which repo is under test, and is what the action already uses. The local reproduction missed this because the throwaway fixture was not inside a git repository, so the root happened to be the fixture. --- .github/workflows/selftest.yml | 11 +++++++++-- tests/test_skillscope.py | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index 8431b79..81e3736 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -96,15 +96,22 @@ jobs: } EOF + # `SKILLSCOPE_REPO` rather than `working-directory`, because a `--skills-dir` + # glob that was passed is resolved against the repo root and `find_root` + # takes that from the nearest `.git`. Running from a subdirectory of a + # checkout would therefore still glob the checkout. Same thing `repo:` + # does for the action. + # # Exits non-zero because the mock satisfies nothing; the report is the # artifact under test, not the exit code. - name: Grade the fixture on the inspect engine continue-on-error: true - working-directory: fixture + env: + SKILLSCOPE_REPO: fixture run: > python -m skillscope behavioral --engine inspect --model mockllm/model --skills-dir '*' --skill demo-skill - --output ../engine-report.json + --output engine-report.json - name: Check the machinery held shell: python diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 14741c0..ef32c0a 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2734,6 +2734,20 @@ 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( @@ -2772,6 +2786,14 @@ def test_local_takes_no_configuration(self) -> None: 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_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: From 00d11100d17e96fafe9ff71cb1e6d91f2eb20fb3 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 11:11:48 +0000 Subject: [PATCH 16/33] Do not pin a mock model to opus under CI The CI pin coerces any non-opus model to opus so paid runs stay comparable between runs. `mockllm` reaches no provider, grades nothing and costs nothing, so pinning it turned the free wiring check into a run that needed a key -- in the one environment where not needing a key is the entire point. The new engine job failed on exactly that. --- skillscope/agent.py | 8 ++++++++ tests/test_skillscope.py | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/skillscope/agent.py b/skillscope/agent.py index 88165c6..d5ebca5 100644 --- a/skillscope/agent.py +++ b/skillscope/agent.py @@ -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." diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index ef32c0a..b0dc147 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2509,6 +2509,26 @@ 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 TestEngineModelNames(unittest.TestCase): """`--model` speaks the claude CLI's aliases; inspect wants provider names.""" From 86283df182cb231ff74a48d456e58732cfb4efea Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 14:16:06 +0000 Subject: [PATCH 17/33] Stop the wiring run paying for a mock that cannot finish `mockllm` never calls the submit tool, so the agent looped to the full 120-message budget and every turn was a real sandbox round trip. The CI grade step took 43 seconds to learn what it knows in three turns, and the same thing made a mock run over a real catalogue take 157 seconds for three cases. A model that reaches no provider now gets six turns. The job's grade step drops to about two seconds and the assertion it rests on -- the seeded fixture being found -- is unchanged. Also caches the pip download. inspect-ai pulls in the order of eighty packages, which was the other twenty seconds. --- .github/workflows/selftest.yml | 5 +++++ skillscope/engine/behavioral.py | 21 +++++++++++++++++---- skillscope/engine/verify.py | 6 +++--- tests/test_skillscope.py | 19 +++++++++++++++++++ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index 81e3736..adb4bf2 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -61,6 +61,11 @@ jobs: - 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]" diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index ec71c66..cc10b51 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -13,7 +13,7 @@ from pathlib import Path -from .. import config, deadline, usage +from .. import agent, config, deadline, usage from ..behavior import BehaviorOutcome from ..datasets import Case from . import convert, models, sandbox as sandbox_spec, scorers, stats, tools @@ -24,6 +24,19 @@ # 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 + + +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 _tools(skill_dir: Path) -> list: """Tools the agent gets for a behavioral run. @@ -54,7 +67,7 @@ def _prompt() -> str | None: ) -def build_task(skill: str, cases: list[Case], ctx: dict | None = None): +def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = None): """One inspect `Task` per skill: its cases, its skill installed, its scorer.""" from inspect_ai import Task from inspect_ai.agent import react @@ -69,7 +82,7 @@ def build_task(skill: str, cases: list[Case], ctx: dict | None = None): solver=react(prompt=_prompt(), tools=_tools(skill_dir)), scorer=scorers.expectations(), sandbox=sandbox_spec.for_skill(skill), - message_limit=MESSAGE_LIMIT, + message_limit=message_limit_for(model), time_limit=int(bound.remaining()) if bound is not None else None, ) @@ -139,7 +152,7 @@ def run( print(f"[behavioral] {skill}: {len(skill_cases)} case(s)", flush=True) logs = inspect_eval( - build_task(skill, skill_cases), + build_task(skill, skill_cases, model), model=model, model_args=models.model_args(model), log_dir=str(Path(".skillscope") / "logs"), diff --git a/skillscope/engine/verify.py b/skillscope/engine/verify.py index 725ef91..00ff853 100644 --- a/skillscope/engine/verify.py +++ b/skillscope/engine/verify.py @@ -53,7 +53,7 @@ def require() -> None: raise SystemExit(INSTALL_HINT) from exc -def build_task(skill: str, cases: list[Case], ctx: dict | None = None): +def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = None): """One task per skill, solved by real Claude Code rather than our agent.""" from inspect_ai import Task from inspect_swe import claude_code @@ -71,7 +71,7 @@ def build_task(skill: str, cases: list[Case], ctx: dict | None = None): solver=claude_code(skills=[skill_dir]), scorer=scorers.expectations(), sandbox=sandbox_spec.for_skill(skill), - message_limit=behavioral.MESSAGE_LIMIT, + message_limit=behavioral.message_limit_for(model), time_limit=int(bound.remaining()) if bound is not None else None, ) @@ -92,7 +92,7 @@ def run( print(f"[claude-code] {skill}: {len(skill_cases)} case(s)", flush=True) logs = inspect_eval( - build_task(skill, skill_cases), + build_task(skill, skill_cases, model), model=model, model_args=models.model_args(model), log_dir=str(Path(".skillscope") / "logs"), diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index b0dc147..82698f6 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -48,6 +48,7 @@ ) 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 judge as engine_judge from skillscope.engine import models as engine_models from skillscope.engine import routing as engine_routing @@ -2529,6 +2530,24 @@ def test_nothing_is_pinned_outside_ci(self) -> None: 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 TestEngineModelNames(unittest.TestCase): """`--model` speaks the claude CLI's aliases; inspect wants provider names.""" From b987db7a2d2c2d04dafbd0139cc907b0c29170b2 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 14:45:41 +0000 Subject: [PATCH 18/33] Say what to install when a sandbox provider will not resolve A trial on a self-hosted runner installed the podman binary, selected `SKILLSCOPE_SANDBOX=podman`, and failed with a ValueError from inspect's registry naming neither the variable that chose the provider nor the package that supplies it. 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, and that is the `[podman]` extra rather than `[inspect]`. Checked once before any container starts, with the install line in the message. The resolver is injectable so this is testable without the inspect extra, which the unit suite runs without on purpose. --- skillscope/engine/behavioral.py | 2 ++ skillscope/engine/sandbox.py | 31 +++++++++++++++++++++++++++++++ skillscope/engine/verify.py | 2 ++ tests/test_skillscope.py | 16 ++++++++++++++++ 4 files changed, 51 insertions(+) diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index cc10b51..8ac6afd 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -144,6 +144,8 @@ def run( """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] diff --git a/skillscope/engine/sandbox.py b/skillscope/engine/sandbox.py index aef67b6..011a91e 100644 --- a/skillscope/engine/sandbox.py +++ b/skillscope/engine/sandbox.py @@ -59,10 +59,41 @@ def describe() -> dict: 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 provider() -> str: """The sandbox provider for this run.""" override = os.environ.get(SANDBOX_ENV, "").strip() diff --git a/skillscope/engine/verify.py b/skillscope/engine/verify.py index 00ff853..caf8a82 100644 --- a/skillscope/engine/verify.py +++ b/skillscope/engine/verify.py @@ -84,6 +84,8 @@ def run( 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] diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 82698f6..dc49534 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2833,6 +2833,22 @@ def test_windows_has_no_sandbox_available(self) -> None: 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: From 24f476d2219bb29324350fb0810240f957f83d6c Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 14:56:40 +0000 Subject: [PATCH 19/33] Grade the answer the agent submitted, not the sentence introducing it A `react` agent delivers its answer through the submit tool, which lands in `output.completion`. The judge read the last assistant message instead, and that is often the preamble -- so an expectation about what the agent told the user could be graded against "the commands are below" rather than the commands. On a real skill this showed up as the judge reporting that the final message "only describes having printed registration/test commands but contains no actual curl commands", which is exactly what it was shown. Falls back to the last assistant message for agents with no submit tool. --- skillscope/engine/judge.py | 9 ++++++++ tests/test_skillscope.py | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/skillscope/engine/judge.py b/skillscope/engine/judge.py index 6416be0..c329396 100644 --- a/skillscope/engine/judge.py +++ b/skillscope/engine/judge.py @@ -104,6 +104,15 @@ def final_message_of(state) -> str: the cloud API" must neither satisfy nor fail an expectation that it avoided doing so. The prompt says which to use for which. """ + # A `react` agent delivers its answer through the submit tool, and that is + # what lands in `output.completion`. The last assistant *message* can be + # the preamble that introduces it -- "the commands are below" -- so reading + # only that can show the judge a description of an answer instead of the + # answer. + completion = getattr(getattr(state, "output", None), "completion", None) + if isinstance(completion, str) and completion.strip(): + return completion.strip()[:MAX_TRANSCRIPT] + for message in reversed(state.messages): if getattr(message, "role", None) != "assistant": continue diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index dc49534..fae7a4e 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2711,6 +2711,49 @@ def test_the_end_survives(self) -> None: 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_wins(self) -> None: + # The last assistant message is often the preamble that introduces the + # answer. Grading that instead shows the judge a description of the + # work rather than the work. + state = _State( + [_Assistant("Here are the commands you need:")], + _Output("curl -X POST /api/v1/pull -d '{...}'"), + ) + self.assertIn("curl -X POST", engine_judge.final_message_of(state)) + + def test_it_falls_back_to_the_last_assistant_message(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")) From f983879b6b798d956d6db75af3907b41770b52f0 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Fri, 11 Sep 2026 15:03:34 +0000 Subject: [PATCH 20/33] Document what podman actually needs Three things, each found by the previous one failing on a real runner: the provider package, podman's own compose rather than the shim that delegates to Docker's, and a search registry because podman will not guess one for an unqualified image name. Worth the setup where the runner's user cannot reach the Docker socket, which is the case this came from. --- docs/usage.md | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 6cb157e..8a47d86 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -235,10 +235,26 @@ 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 (`pip install 'skillscope[podman]'` -- the provider registers itself, so -installing it is the whole setup); `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. +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`. Skills get a container with From bb3a32bd8e44a72f88e3da9e3f3fbcc1517493d4 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 14 Sep 2026 06:41:09 +0000 Subject: [PATCH 21/33] Resolve a declared compose file beside machine.yml It resolved against the skill root, which is what gets published -- so eval infrastructure would ship with the skill. A path written in a file is also most usefully relative to that file. Both point at evals/, beside the machine.yml that names it. Found by writing the first real one. --- docs/usage.md | 2 +- skillscope/engine/sandbox.py | 9 +++++++-- skillscope/schema/machine.schema.json | 2 +- tests/test_skillscope.py | 6 +++++- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 8a47d86..fce2972 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -257,7 +257,7 @@ 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`. Skills get a container with +`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. diff --git a/skillscope/engine/sandbox.py b/skillscope/engine/sandbox.py index 011a91e..537bec1 100644 --- a/skillscope/engine/sandbox.py +++ b/skillscope/engine/sandbox.py @@ -124,12 +124,17 @@ def for_skill(skill: str): def _declared_compose(skill: str): - """Path to the compose file a skill's `machine.yml` names, if any.""" + """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.skill_path(skill) / name + path = datasets.machine_path(skill).parent / name if not path.is_file(): raise SystemExit( f"error: {skill}: evals/machine.yml names sandbox '{name}', " diff --git a/skillscope/schema/machine.schema.json b/skillscope/schema/machine.schema.json index f3f55a5..0ba208d 100644 --- a/skillscope/schema/machine.schema.json +++ b/skillscope/schema/machine.schema.json @@ -23,7 +23,7 @@ "sandbox": { "type": "string", "minLength": 1, - "description": "Compose file, relative to the skill directory, describing the sandbox the behavioral cases need under the inspect engine. 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.", + "description": "Compose file, resolved beside this machine.yml in evals/, describing the sandbox the behavioral cases need under the inspect engine. 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/tests/test_skillscope.py b/tests/test_skillscope.py index fae7a4e..c558035 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2836,7 +2836,11 @@ def _skill(self, machine: str | None = None, compose: bool = False) -> None: "boxed", dataset=tier0_dataset("boxed"), machine=machine ) if compose: - (folder / "compose.yaml").write_text("services: {}\n", encoding="utf-8") + # 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: From b6fc1976c42336d67a964ddd459efe4d83858954 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 14 Sep 2026 06:56:03 +0000 Subject: [PATCH 22/33] Contain a skill that cannot be run to that skill A malformed sandbox declaration raised out of the whole behavioral command, so one skill's broken setup discarded results for skills already graded and paid for. On a real run that threw away three passing cases to report that a fourth skill named a compose file that was not there. It reports failed outcomes for that skill and carries on, which is the same rule the structural gate already follows: a neighbour's mistake says nothing about whether this run can proceed. --- skillscope/engine/behavioral.py | 57 +++++++++++++++++++++------------ tests/test_skillscope.py | 17 ++++++++++ 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index 8ac6afd..f152608 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -87,6 +87,21 @@ def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = Non ) +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, + ) + for case in cases + ] + + def _outcomes(log, skill: str, cases: list[Case]) -> list[BehaviorOutcome]: """Map one inspect `EvalLog` back onto skillscope's outcome objects. @@ -99,17 +114,7 @@ def _outcomes(log, skill: str, cases: list[Case]) -> list[BehaviorOutcome]: if log.status == "error" or not log.samples: detail = getattr(log.error, "message", None) or "the task produced no samples" - return [ - BehaviorOutcome( - id=case.id, - skill=skill, - prompt=case.prompt, - passed=False, - elapsed_s=0.0, - error=f"inspect task failed: {detail}", - ) - for case in cases - ] + return _failed(skill, cases, f"inspect task failed: {detail}") for sample in log.samples: case_id = str(sample.id) @@ -153,16 +158,26 @@ def run( continue print(f"[behavioral] {skill}: {len(skill_cases)} case(s)", flush=True) - logs = inspect_eval( - build_task(skill, skill_cases, model), - model=model, - model_args=models.model_args(model), - log_dir=str(Path(".skillscope") / "logs"), - # 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", - ) + try: + logs = inspect_eval( + build_task(skill, skill_cases, model), + model=model, + model_args=models.model_args(model), + log_dir=str(Path(".skillscope") / "logs"), + # 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)) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index c558035..0266fb5 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2548,6 +2548,23 @@ def test_a_mock_gets_a_short_one(self) -> None: ) +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 TestEngineModelNames(unittest.TestCase): """`--model` speaks the claude CLI's aliases; inspect wants provider names.""" From 89386822ead44f98cfd212d1a89208f85582a3ca Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 14 Sep 2026 08:41:09 +0000 Subject: [PATCH 23/33] Show the judge 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. Reading only the final turn credited the agent with its closing summary and called the work missing -- on a real skill the judge reported that the agent "only claims curl commands were delivered as text" when it had printed them a turn earlier. The submitted answer still comes last and is marked, because it is the answer where the rest is working. The split the prompt relies on is unchanged: actions are judged from tool calls and artifacts, and only what the agent told the user is judged from what it said. --- skillscope/engine/judge.py | 42 +++++++++++++++++++++++--------------- tests/test_skillscope.py | 23 ++++++++++++++++----- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/skillscope/engine/judge.py b/skillscope/engine/judge.py index c329396..f4f0a30 100644 --- a/skillscope/engine/judge.py +++ b/skillscope/engine/judge.py @@ -104,30 +104,40 @@ def final_message_of(state) -> str: the cloud API" must neither satisfy nor fail an expectation that it avoided doing so. The prompt says which to use for which. """ - # A `react` agent delivers its answer through the submit tool, and that is - # what lands in `output.completion`. The last assistant *message* can be - # the preamble that introduces it -- "the commands are below" -- so reading - # only that can show the judge a description of an answer instead of the - # answer. - completion = getattr(getattr(state, "output", None), "completion", None) - if isinstance(completion, str) and completion.strip(): - return completion.strip()[:MAX_TRANSCRIPT] - - for message in reversed(state.messages): + # 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(): - return content.strip()[:MAX_TRANSCRIPT] - if isinstance(content, list): + said.append(content.strip()) + elif isinstance(content, list): texts = [ part.text for part in content - if isinstance(getattr(part, "text", None), str) + if isinstance(getattr(part, "text", None), str) and part.text.strip() ] - if any(t.strip() for t in texts): - return "\n".join(texts).strip()[:MAX_TRANSCRIPT] - return "(the agent said nothing)" + 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: diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 0266fb5..ef7072d 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2749,17 +2749,30 @@ def __init__(self, content: str) -> None: class TestEngineJudgeFinalMessage(unittest.TestCase): """A `react` agent answers through submit, not through a chat message.""" - def test_the_submitted_answer_wins(self) -> None: - # The last assistant message is often the preamble that introduces the - # answer. Grading that instead shows the judge a description of the - # work rather than the work. + 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_falls_back_to_the_last_assistant_message(self) -> None: + 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" From facfb2ecf7e30efbebe8a907a05b63a1184f1ea0 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 14 Sep 2026 10:59:00 +0000 Subject: [PATCH 24/33] Run the real CLI inside inspect's framework `inspect_swe` runs Claude Code inside the sandbox and reaches the model through a bridge whose proxy is a Linux binary, so it cannot run on Windows at all. This drives the same CLI on the host, the way the legacy engine does, and maps what it did into inspect's messages -- so the scorers, the artifact-reading judge and the .eval transcript all work unchanged. The point is fidelity. A skill is written for this harness, and grading a different agent measures something customers will not experience. With this the real harness runs on both platforms and only the isolation differs: a container on Linux via --engine claude-code, the host on Windows via --engine claude-cli, with the report saying which. Unsandboxed by construction, which is what the legacy engine already does, so it is not a regression. Refuses a container provider outright rather than running the CLI in one filesystem and scoring another. `build_task` gains a solver seam: what drives the agent swaps while staging, scoring, judging and reporting stay put. --- skillscope/cli.py | 34 +++++-- skillscope/engine/behavioral.py | 25 ++++- skillscope/engine/cli_agent.py | 165 ++++++++++++++++++++++++++++++++ tests/test_skillscope.py | 19 ++++ 4 files changed, 230 insertions(+), 13 deletions(-) create mode 100644 skillscope/engine/cli_agent.py diff --git a/skillscope/cli.py b/skillscope/cli.py index 182b3de..433274c 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -344,7 +344,7 @@ 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", "legacy") in ("inspect", "claude-code"): + if getattr(args, "engine", "legacy") in ("inspect", "claude-code", "claude-cli"): # 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 @@ -524,14 +524,32 @@ def cmd_behavioral(args: argparse.Namespace) -> int: if args.engine in ("inspect", "claude-code"): from .engine import models as engine_models - if args.engine == "inspect": - from .engine import behavioral as runner - else: + from .engine import behavioral as inspect_behavioral + + if args.engine == "claude-code": from .engine import verify as runner - outcomes = runner.run( - skills, gradable, engine_models.resolve(args.model), args.effort - ) + outcomes = runner.run( + skills, gradable, engine_models.resolve(args.model), args.effort + ) + elif args.engine == "claude-cli": + # 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 cli_agent + + cli_agent.require_local() + outcomes = inspect_behavioral.run( + skills, + gradable, + engine_models.resolve(args.model), + args.effort, + solver=cli_agent.claude_cli(args.model, args.effort), + ) + else: + outcomes = inspect_behavioral.run( + skills, gradable, engine_models.resolve(args.model), args.effort + ) else: outcomes = behavior.run(skills, gradable, args.model, args.effort) @@ -663,7 +681,7 @@ def _add_graded_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument( "--engine", default=os.environ.get("SKILLSCOPE_ENGINE", "legacy"), - choices=["legacy", "inspect", "claude-code"], + choices=["legacy", "inspect", "claude-code", "claude-cli"], help=( "Which eval engine runs the cases. `legacy` drives the claude CLI " "directly; `inspect` runs a harness-independent agent through " diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index f152608..e02d1fb 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -67,8 +67,19 @@ def _prompt() -> str | None: ) -def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = None): - """One inspect `Task` per skill: its cases, its skill installed, its scorer.""" +def build_task( + skill: str, + cases: list[Case], + model: str, + ctx: dict | None = None, + solver=None, +): + """One inspect `Task` per skill: its cases, its skill installed, its scorer. + + `solver` swaps what drives the agent while everything around it -- staging, + scoring, judging, reporting -- stays the same. That is the seam the engine + choice turns on. + """ from inspect_ai import Task from inspect_ai.agent import react @@ -79,7 +90,7 @@ def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = Non return Task( name=f"behavioral-{skill}", dataset=samples, - solver=react(prompt=_prompt(), tools=_tools(skill_dir)), + solver=solver or react(prompt=_prompt(), tools=_tools(skill_dir)), scorer=scorers.expectations(), sandbox=sandbox_spec.for_skill(skill), message_limit=message_limit_for(model), @@ -144,7 +155,11 @@ def _outcomes(log, skill: str, cases: list[Case]) -> list[BehaviorOutcome]: def run( - skills: list[str], cases: list[Case], model: str, effort: str + skills: list[str], + cases: list[Case], + model: str, + effort: str, + solver=None, ) -> list[BehaviorOutcome]: """Run every behavioral case, grouped by skill. Mirrors `behavior.run`.""" from inspect_ai import eval as inspect_eval @@ -160,7 +175,7 @@ def run( print(f"[behavioral] {skill}: {len(skill_cases)} case(s)", flush=True) try: logs = inspect_eval( - build_task(skill, skill_cases, model), + build_task(skill, skill_cases, model, solver=solver), model=model, model_args=models.model_args(model), log_dir=str(Path(".skillscope") / "logs"), diff --git a/skillscope/engine/cli_agent.py b/skillscope/engine/cli_agent.py new file mode 100644 index 0000000..8340639 --- /dev/null +++ b/skillscope/engine/cli_agent.py @@ -0,0 +1,165 @@ +# 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, and an eval that +grades a different agent is measuring something customers will not experience -- +which matters most for routing, where the whole question is what fires. So the +real harness runs everywhere, and only the isolation differs by platform: +`inspect_swe` in a container on Linux, this on Windows, where 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 json +import shutil + +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" + + +def require_local() -> None: + """This driver runs on the host, so the sandbox has to be the host.""" + from . import sandbox as sandbox_spec + + provider = sandbox_spec.provider() + if provider not in sandbox_spec.NOT_ISOLATED: + raise SystemExit( + f"error: --engine claude-cli runs the CLI on the host, so it needs " + f"SKILLSCOPE_SANDBOX=local, not {provider!r}. For a sandboxed run of " + "the real harness on Linux, use --engine claude-code." + ) + 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 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, + ) + ) + + final = "" + for event in events: + if event.get("type") == "result" and isinstance(event.get("result"), str): + final = event["result"] + if final: + messages.append(ChatMessageAssistant(content=final)) + return messages, final + + +def claude_cli(model: str | None, effort: str | None): + """Solver: run the real CLI once, and record what it did.""" + from inspect_ai.model import ModelOutput + from inspect_ai.solver import solver + + @solver + def _claude_cli(): + async def solve(state, generate): + from inspect_ai.util import subprocess as sandbox_subprocess + + workspace = await _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] + + result = await sandbox_subprocess( + cmd, input=prompt, cwd=workspace, env=legacy_agent.claude_env() + ) + + events: list[dict] = [] + for line in (result.stdout or "").splitlines(): + line = line.strip() + if not line: + continue + try: + events.append(json.loads(line)) + except json.JSONDecodeError: + continue + + if not events: + raise RuntimeError( + "claude produced no parseable stream-json output " + f"(exit {result.returncode}). stderr: {(result.stderr or '')[: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_cli() diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index ef7072d..be18c46 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -49,6 +49,7 @@ 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 cli_agent as engine_cli_agent from skillscope.engine import judge as engine_judge from skillscope.engine import models as engine_models from skillscope.engine import routing as engine_routing @@ -2565,6 +2566,24 @@ def test_a_skill_that_cannot_run_becomes_failed_outcomes(self) -> None: self.assertTrue(all(o.checks == [] for o in outcomes)) +class TestEngineCliAgentGuard(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_cli_agent.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.""" From ecf082588dbf8a2b909bba5a92e1defd471933a3 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 14 Sep 2026 11:12:57 +0000 Subject: [PATCH 25/33] Let the benchmark compare any two engines Legacy against inspect asks whether a different agent reaches the same verdicts. Legacy against claude-cli asks something narrower and sharper: both drive the same CLI, so agreement means the framework around the agent is faithful, and a flip is a defect in the crossing rather than a property of a different agent. That is the measurement that decides whether the legacy path is redundant or merely superseded. The engine list now lives in the CLI and is imported, so a new engine is offered in both places at once rather than in whichever was remembered. On the fixture the two agree on every case at 9 model calls and ~27s each, which is what the same binary run twice should look like. --- skillscope/cli.py | 15 ++++++-- tools/benchmark_engines.py | 71 ++++++++++++++++++++++++++++---------- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/skillscope/cli.py b/skillscope/cli.py index 433274c..286f3d7 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -86,6 +86,17 @@ from . import selection as select_module from .agent import check_api_reachable, enforce_model_policy +# The engines a graded run can be driven by, in the order they were added. +# legacy -- the claude CLI, driven directly, no framework +# inspect -- a harness-independent agent under inspect_ai +# claude-code -- the real CLI inside the sandbox, via inspect_swe (Linux only) +# claude-cli -- the real CLI on the host, under inspect_ai (any platform) +ENGINES = ["legacy", "inspect", "claude-code", "claude-cli"] + +# Every engine but the first runs under inspect_ai, and they share what follows +# from that: a preflight of their own, and a report that names the engine. +INSPECT_ENGINES = tuple(e for e in ENGINES if e != "legacy") + # 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" @@ -344,7 +355,7 @@ 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", "legacy") in ("inspect", "claude-code", "claude-cli"): + if getattr(args, "engine", "legacy") 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 @@ -681,7 +692,7 @@ def _add_graded_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument( "--engine", default=os.environ.get("SKILLSCOPE_ENGINE", "legacy"), - choices=["legacy", "inspect", "claude-code", "claude-cli"], + choices=ENGINES, help=( "Which eval engine runs the cases. `legacy` drives the claude CLI " "directly; `inspect` runs a harness-independent agent through " diff --git a/tools/benchmark_engines.py b/tools/benchmark_engines.py index 2064e00..b4d37a2 100644 --- a/tools/benchmark_engines.py +++ b/tools/benchmark_engines.py @@ -3,7 +3,7 @@ # # SPDX-License-Identifier: MIT -"""Compare the legacy and inspect engines on the same dataset. +"""Compare two engines on the same dataset. Answers the two questions a migration has to answer before it can be trusted. @@ -22,6 +22,15 @@ 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 inspect.json + +Which pair is compared is an argument, because the question changes over the +migration. `legacy` against `inspect` asks whether a different agent reaches +the same verdicts. `legacy` against `claude-cli` asks something narrower and +sharper: both drive the same CLI, so agreement there says the framework around +the agent is faithful, and disagreement is a defect in the crossing rather than +a property of a different agent. + + tools/benchmark_engines.py behavioral --candidate claude-cli --skill my-skill """ from __future__ import annotations @@ -33,9 +42,15 @@ 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 inspect engine passes" -NEW_FAILS = "only the legacy engine passes" +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: @@ -98,10 +113,10 @@ def compare(baseline: dict, candidate: dict) -> dict: } -def spend(report: dict) -> dict: +def spend(report: dict, engine: str = "legacy") -> dict: meta = report.get("meta", {}) return { - "engine": meta.get("engine", "legacy"), + "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"), @@ -134,9 +149,9 @@ def _spend_caveats(spend: dict) -> list[str]: ) if any(spend[label]["cost_usd"] is None for label in ("baseline", "candidate")): notes.append( - "> One engine reported no cost -- inspect only has one when the " - "model provider supplies pricing, which a gateway generally does " - "not. Wall time and model calls are comparable on both sides; " + "> One engine reported no cost -- the inspect 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." ) @@ -145,18 +160,20 @@ def _spend_caveats(spend: dict) -> list[str]: 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 legacy and inspect engines.", + f"between the `{base}` and `{cand}` engines.", "", ] noise = result.get("noise") if noise is not None: lines += [ - f"Noise floor: the legacy engine agrees with itself on " + 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.", "", @@ -183,7 +200,7 @@ def render(result: dict) -> str: lines.append("None. Every shared case reached the same verdict on both engines.") else: lines += [ - "| Case | Direction | Legacy | Inspect |", + f"| Case | Direction | `{base}` | `{cand}` |", "| --- | --- | --- | --- |", ] for flip in comparison["flips"]: @@ -192,8 +209,8 @@ def render(result: dict) -> str: lines.append(f"| `{flip['id']}` | {flip['direction']} | {left} | {right} |") for key, heading in ( - ("only_in_baseline", "Only the legacy run produced these cases"), - ("only_in_candidate", "Only the inspect run produced these cases"), + ("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: @@ -211,12 +228,25 @@ def main(argv: list[str] | None = None) -> int: metavar=("LEGACY", "INSPECT"), 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="inspect", + choices=ENGINES, + help="The engine under test. Default: inspect.", + ) parser.add_argument( "--noise", action="store_true", help=( - "Run the legacy engine twice to measure how much it disagrees with " - "itself. Without this the flip list cannot be read as engine drift." + "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.") @@ -229,19 +259,22 @@ def main(argv: list[str] | None = None) -> int: else: if not args.leg: parser.error("give a leg to run (routing or behavioral), or --compare") - baseline = run_leg(args.leg, "legacy", passthrough, "legacy") + baseline = run_leg(args.leg, args.baseline, passthrough, args.baseline) noise_run = ( - run_leg(args.leg, "legacy", passthrough, "legacy-again") + run_leg(args.leg, args.baseline, passthrough, f"{args.baseline}-again") if args.noise else None ) - candidate = run_leg(args.leg, "inspect", passthrough, "inspect") + 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), "candidate": spend(candidate)}, + "spend": { + "baseline": spend(baseline, getattr(args, "baseline", "legacy")), + "candidate": spend(candidate, getattr(args, "candidate", "inspect")), + }, } report = render(result) From dbe49a1a8290c77f7f6fc838b97306d5d5121aa1 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 14 Sep 2026 13:03:29 +0000 Subject: [PATCH 26/33] Run Claude Code where the scorers look Every files_exist check on the claude-code leg of the first trial run failed with "sandbox holds: nothing" -- nine of nine, across every skill that asserts files. The agent was not doing nothing: a container starts at /, so it worked there while the scorers read /workspace. The other engines say this in a prompt, because their agent takes instructions. This one takes a cwd. The directory is created before the solver runs, since this leg's agent brings its own tools and so never touches ours, which is what would otherwise have made it. Read as a comparison, the old numbers said real Claude Code performs worse than a harness-independent agent on these skills. They said nothing of the kind. --- skillscope/engine/tools.py | 10 +++++++++ skillscope/engine/verify.py | 43 +++++++++++++++++++++++++++++++++++-- tests/test_skillscope.py | 17 +++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/skillscope/engine/tools.py b/skillscope/engine/tools.py index eb9d873..f1de14e 100644 --- a/skillscope/engine/tools.py +++ b/skillscope/engine/tools.py @@ -65,6 +65,16 @@ def containerized() -> bool: 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. diff --git a/skillscope/engine/verify.py b/skillscope/engine/verify.py index caf8a82..d233f12 100644 --- a/skillscope/engine/verify.py +++ b/skillscope/engine/verify.py @@ -31,7 +31,15 @@ from .. import config, deadline from ..behavior import BehaviorOutcome from ..datasets import Case -from . import behavioral, convert, models, sandbox as sandbox_spec, scorers, stats +from . import ( + behavioral, + convert, + models, + sandbox as sandbox_spec, + scorers, + stats, + tools, +) INSTALL_HINT = ( "error: --engine claude-code needs the verify extra. Install it with:\n" @@ -53,14 +61,42 @@ def require() -> None: raise SystemExit(INSTALL_HINT) from exc +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): """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". The other engines say this in a prompt, because + # their agent takes instructions; this one takes a working directory. bound = deadline.active() return Task( name=f"claude-code-{skill}", @@ -68,7 +104,10 @@ def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = Non # `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. - solver=claude_code(skills=[skill_dir]), + solver=chain( + _ensure_workdir(), + claude_code(skills=[skill_dir], cwd=tools.workdir_path()), + ), scorer=scorers.expectations(), sandbox=sandbox_spec.for_skill(skill), message_limit=behavioral.message_limit_for(model), diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index be18c46..8f1b0b8 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2566,6 +2566,23 @@ def test_a_skill_that_cannot_run_becomes_failed_outcomes(self) -> None: self.assertTrue(all(o.checks == [] for o in outcomes)) +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 TestEngineCliAgentGuard(unittest.TestCase): """The CLI runs on the host, so the sandbox has to be the host.""" From 00e54386a2de70e8522e39b4e646dd536c4cd7dc Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Tue, 15 Sep 2026 14:01:35 +0000 Subject: [PATCH 27/33] Name the engine that was actually asked for Every engine but `legacy` runs on inspect_ai, so any of them can hit the missing-extra path -- but the hint said `--engine inspect` regardless. A Windows CI job that had passed `--engine claude-cli` was told to go look at a flag it had never used. The remedy is the same extra either way, so this only costs a reader their bearings. That is enough. --- skillscope/cli.py | 2 +- skillscope/engine/__init__.py | 23 +++++++++++++++++------ tests/test_skillscope.py | 13 +++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/skillscope/cli.py b/skillscope/cli.py index 286f3d7..af0ca03 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -360,7 +360,7 @@ def _prepare_graded_run( # 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() + engine.require(args.engine) if not args.skip_preflight: from .engine import models as engine_models diff --git a/skillscope/engine/__init__.py b/skillscope/engine/__init__.py index 13b2207..cb3f385 100644 --- a/skillscope/engine/__init__.py +++ b/skillscope/engine/__init__.py @@ -15,18 +15,29 @@ from __future__ import annotations -INSTALL_HINT = ( - "error: --engine inspect needs the inspect extra. Install it with:\n" - " pip install 'skillscope[inspect]'" -) +def install_hint(engine: str = "inspect") -> str: + """Why this run cannot start, naming the engine that was actually asked for. + Every engine but `legacy` runs on inspect_ai, so any of them can raise + this. Naming `inspect` regardless sent a CI job looking for a flag it had + not passed. + """ + return ( + f"error: --engine {engine} needs the inspect extra. Install it with:\n" + " pip install 'skillscope[inspect]'" + ) -def require() -> None: + +# Kept for callers that predate the engine argument. +INSTALL_HINT = install_hint() + + +def require(engine: str = "inspect") -> 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) from exc + raise SystemExit(install_hint(engine)) from exc def available() -> bool: diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 8f1b0b8..f97b716 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -42,6 +42,7 @@ credentials, datasets, deadline, + engine as engine_module, references, routing, structure, @@ -2566,6 +2567,18 @@ def test_a_skill_that_cannot_run_becomes_failed_outcomes(self) -> None: self.assertTrue(all(o.checks == [] for o in outcomes)) +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-cli. + self.assertIn("--engine claude-cli", engine_module.install_hint("claude-cli")) + + def test_it_still_points_at_the_one_extra_that_fixes_all_of_them(self) -> None: + self.assertIn("skillscope[inspect]", engine_module.install_hint("claude-code")) + + class TestEngineWorkdirPath(unittest.TestCase): """The working directory has to be knowable before a sandbox exists.""" From 8da6ee6c5abc841bd80b729db2af9995f2fbc2e7 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Wed, 16 Sep 2026 09:43:48 +0000 Subject: [PATCH 28/33] Send claude-cli to the engine it asked for The guard around the inspect dispatch was a literal tuple listing only inspect and claude-code. `claude-cli` was added to the chain inside it but never to the tuple, so it fell through to the legacy engine: every run that asked for it silently got something else, while the report recorded `engine: claude-cli` throughout. The preflight, which does key off INSPECT_ENGINES, meanwhile demanded an extra the run never used -- which is how a Windows job failed on a flag it had not passed. The absent .eval transcripts were the symptom that gave it away: legacy does not write them. This invalidates every claude-cli measurement taken so far, including the benchmark that reported legacy and claude-cli agreeing 4/4 with cost within 3%. They agreed because they were the same engine; the figure matched the noise floor exactly, which should have been the tell. Tests now assert which runner a flag reaches, and that the guard and the choices list cannot drift apart again. --- skillscope/cli.py | 7 ++++++- tests/test_skillscope.py | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/skillscope/cli.py b/skillscope/cli.py index af0ca03..1da1956 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -532,7 +532,12 @@ def cmd_behavioral(args: argparse.Namespace) -> int: ) return 0 - if args.engine in ("inspect", "claude-code"): + # INSPECT_ENGINES rather than a literal tuple: a literal was already wrong + # once. `claude-cli` was added to the branch below but not to this guard, + # so it fell through to the legacy engine and every run that asked for it + # silently got something else -- while the preflight above, which does key + # off INSPECT_ENGINES, still demanded the inspect extra it never used. + if args.engine in INSPECT_ENGINES: from .engine import models as engine_models from .engine import behavioral as inspect_behavioral diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index f97b716..8e75968 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -19,6 +19,7 @@ from __future__ import annotations import argparse +import inspect import contextlib import io import json @@ -2567,6 +2568,32 @@ def test_a_skill_that_cannot_run_becomes_failed_outcomes(self) -> None: self.assertTrue(all(o.checks == [] for o in outcomes)) +class TestBehavioralEngineDispatch(unittest.TestCase): + """Only `legacy` may bypass the inspect path. + + `claude-cli` was added to the dispatch chain but not to the guard around + it, so it fell through to the legacy engine: runs that asked for one agent + silently got another, while the reports said `engine: claude-cli` + throughout. Nothing in the suite noticed, because nothing asserted which + runner a flag actually reaches. + """ + + def test_the_guard_covers_every_engine_but_legacy(self) -> None: + self.assertEqual(set(cli.INSPECT_ENGINES), set(cli.ENGINES) - {"legacy"}) + + def test_the_dispatch_reads_that_set_rather_than_a_literal(self) -> None: + # The bug was a literal tuple that fell behind the choices list. A + # literal here is the defect itself, so the source is what to assert. + source = inspect.getsource(cli.cmd_behavioral) + self.assertIn("if args.engine in INSPECT_ENGINES:", source) + + def test_the_preflight_uses_the_same_set(self) -> None: + # These disagreed: the preflight demanded the inspect extra for + # claude-cli while the dispatch sent it to an engine that never uses + # it, which is how a Windows job failed on a flag it had not passed. + 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.""" From 22b214fcb52d4d5d15d253c3778eb5003b5490cb Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Wed, 16 Sep 2026 10:10:33 +0000 Subject: [PATCH 29/33] Launch the CLI on Windows, where it is not an executable npm installs `claude` as a .cmd shim. The legacy engine starts it with the synchronous subprocess.run, which copes, so this never surfaced while --engine claude-cli was silently running legacy. Under inspect's asyncio subprocess the path is exec'd directly and Windows answers WinError 2 -- which reads as "the CLI is not installed" when it plainly is, and which the report surfaced as the task failing rather than the launch failing. --- skillscope/engine/cli_agent.py | 17 ++++++++++++++++- tests/test_skillscope.py | 25 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/skillscope/engine/cli_agent.py b/skillscope/engine/cli_agent.py index 8340639..0a62206 100644 --- a/skillscope/engine/cli_agent.py +++ b/skillscope/engine/cli_agent.py @@ -26,6 +26,7 @@ from __future__ import annotations import json +import os import shutil from .. import agent as legacy_agent @@ -35,6 +36,19 @@ _CALL_PREFIX = "cli" +def launch_argv(argv: list[str]) -> list[str]: + """How to start the CLI, given that on Windows it is not an executable. + + npm installs `claude` as a `.cmd` shim. The legacy engine launches it with + the synchronous `subprocess.run`, which copes; inspect's asyncio subprocess + execs the path directly and Windows answers `WinError 2`, which reads as + "the CLI is not installed" when it plainly is. + """ + if os.name == "nt" and argv[0].lower().endswith((".cmd", ".bat")): + return ["cmd.exe", "/c", *argv] + return argv + + def require_local() -> None: """This driver runs on the host, so the sandbox has to be the host.""" from . import sandbox as sandbox_spec @@ -131,7 +145,8 @@ async def solve(state, generate): cmd += ["--effort", effort] result = await sandbox_subprocess( - cmd, input=prompt, cwd=workspace, env=legacy_agent.claude_env() + launch_argv(cmd), input=prompt, cwd=workspace, + env=legacy_agent.claude_env(), ) events: list[dict] = [] diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 8e75968..2f6c48a 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2623,6 +2623,31 @@ def test_a_local_run_has_none_so_the_agent_keeps_its_own(self) -> None: self.assertIsNone(engine_tools.workdir_path()) +class TestEngineCliAgentLaunch(unittest.TestCase): + """npm ships `claude` as a .cmd shim, which Windows will not exec.""" + + def test_windows_runs_a_cmd_shim_through_the_interpreter(self) -> None: + with mock.patch.object(engine_cli_agent.os, "name", "nt"): + argv = engine_cli_agent.launch_argv([r"C:\npm\claude.CMD", "-p"]) + # Without this the failure is WinError 2, which reads as "the CLI is + # not installed" when it plainly is. + self.assertEqual(argv, ["cmd.exe", "/c", r"C:\npm\claude.CMD", "-p"]) + + def test_a_real_executable_is_left_alone(self) -> None: + with mock.patch.object(engine_cli_agent.os, "name", "nt"): + self.assertEqual( + engine_cli_agent.launch_argv([r"C:\bin\claude.exe"]), + [r"C:\bin\claude.exe"], + ) + + def test_posix_is_untouched(self) -> None: + with mock.patch.object(engine_cli_agent.os, "name", "posix"): + self.assertEqual( + engine_cli_agent.launch_argv(["/usr/bin/claude", "-p"]), + ["/usr/bin/claude", "-p"], + ) + + class TestEngineCliAgentGuard(unittest.TestCase): """The CLI runs on the host, so the sandbox has to be the host.""" From a7ada23bd15833f829d51f9803d4e542a2f20408 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Wed, 16 Sep 2026 10:51:58 +0000 Subject: [PATCH 30/33] Give the CLI the skill it is supposed to be testing The react agent installs the skill through inspect's skill() tool, which arrives as part of its toolset. A solver that replaces the react agent replaces that too -- so claude-cli ran the real CLI against an empty workspace and the agent answered from the prompt alone. It scored 4/21 on a skill every other engine passed 21/21, and finished faster doing it, which was the only visible sign. The seam is now a factory taking the skill's directory, because staging is the driver's job: legacy copies the tree into .claude/skills, and inspect_swe does the same through its own skills= argument. This does what both already do. Third invalid claude-cli measurement in a row. The first two were the dispatch falling through to legacy and the CLI failing to launch on Windows; this one produced plausible-looking numbers rather than an error, which is why it needed the comparison to catch. --- skillscope/cli.py | 4 +++- skillscope/engine/behavioral.py | 20 +++++++++++++------- skillscope/engine/cli_agent.py | 21 +++++++++++++++++++-- tests/test_skillscope.py | 26 ++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/skillscope/cli.py b/skillscope/cli.py index 1da1956..400dee4 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -560,7 +560,9 @@ def cmd_behavioral(args: argparse.Namespace) -> int: gradable, engine_models.resolve(args.model), args.effort, - solver=cli_agent.claude_cli(args.model, args.effort), + solver_factory=lambda d: cli_agent.claude_cli( + args.model, args.effort, d + ), ) else: outcomes = inspect_behavioral.run( diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index e02d1fb..77a45fd 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -72,13 +72,15 @@ def build_task( cases: list[Case], model: str, ctx: dict | None = None, - solver=None, + solver_factory=None, ): """One inspect `Task` per skill: its cases, its skill installed, its scorer. - `solver` swaps what drives the agent while everything around it -- staging, - scoring, judging, reporting -- stays the same. That is the seam the engine - choice turns on. + `solver_factory` swaps what drives the agent while everything around it -- + scoring, judging, reporting -- stays the same. It receives the skill's + directory because staging is the driver's job: the react agent installs the + skill through inspect's `skill()` tool, and a solver that replaces the react + agent replaces that too. """ from inspect_ai import Task from inspect_ai.agent import react @@ -90,7 +92,11 @@ def build_task( return Task( name=f"behavioral-{skill}", dataset=samples, - solver=solver or react(prompt=_prompt(), tools=_tools(skill_dir)), + solver=( + solver_factory(skill_dir) + if solver_factory + else react(prompt=_prompt(), tools=_tools(skill_dir)) + ), scorer=scorers.expectations(), sandbox=sandbox_spec.for_skill(skill), message_limit=message_limit_for(model), @@ -159,7 +165,7 @@ def run( cases: list[Case], model: str, effort: str, - solver=None, + solver_factory=None, ) -> list[BehaviorOutcome]: """Run every behavioral case, grouped by skill. Mirrors `behavior.run`.""" from inspect_ai import eval as inspect_eval @@ -175,7 +181,7 @@ def run( print(f"[behavioral] {skill}: {len(skill_cases)} case(s)", flush=True) try: logs = inspect_eval( - build_task(skill, skill_cases, model, solver=solver), + build_task(skill, skill_cases, model, solver_factory=solver_factory), model=model, model_args=models.model_args(model), log_dir=str(Path(".skillscope") / "logs"), diff --git a/skillscope/engine/cli_agent.py b/skillscope/engine/cli_agent.py index 0a62206..792e65f 100644 --- a/skillscope/engine/cli_agent.py +++ b/skillscope/engine/cli_agent.py @@ -28,6 +28,7 @@ import json import os import shutil +from pathlib import Path from .. import agent as legacy_agent @@ -120,8 +121,23 @@ def events_to_messages(events: list[dict], prompt: str) -> tuple[list, str]: return messages, final -def claude_cli(model: str | None, effort: str | None): - """Solver: run the real CLI once, and record what it did.""" +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 the legacy engine has always done and what + `inspect_swe` does via its own `skills=` argument. The react agent reaches + the same place differently, through inspect's `skill()` tool -- so a solver + that replaces the react agent has to do this itself or the agent runs with + no skill at all, answering from the prompt and scoring like it. + """ + 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) + + +def claude_cli(model: str | None, effort: str | None, skill_dir: Path): + """Solver: install the skill, run the real CLI once, record what it did.""" from inspect_ai.model import ModelOutput from inspect_ai.solver import solver @@ -131,6 +147,7 @@ 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 = [ diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 2f6c48a..521883b 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2623,6 +2623,32 @@ def test_a_local_run_has_none_so_the_agent_keeps_its_own(self) -> None: self.assertIsNone(engine_tools.workdir_path()) +class TestEngineCliAgentInstallsSkill(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_cli_agent.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 TestEngineCliAgentLaunch(unittest.TestCase): """npm ships `claude` as a .cmd shim, which Windows will not exec.""" From 928414c43fd84a2799017ce4b58fe0cd9578c8e1 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Thu, 17 Sep 2026 09:41:40 +0000 Subject: [PATCH 31/33] Let the shell probe survive a guest with no bash The probe ran `bash -lc exit 0` and read `success` to decide between bash and PowerShell. But a guest without bash does not answer "that failed" -- there is nothing to exec, so it raises before any result exists. On a Windows host under the `local` sandbox that surfaced as WinError 2 and took the whole task down, reporting 0/0 expectations, which reads as the harness being broken rather than the probe learning exactly what it asked. Also drops the cmd.exe wrapper added last commit. It was reasoned from npm shipping `claude` as a .cmd shim that Windows cannot exec directly. The runner says otherwise: `which` resolves to claude.CMD and async exec starts it with rc=0, as do sync Popen and cmd.exe. The wrapper fixed nothing and added a shell layer between us and the agent's arguments. --- skillscope/engine/cli_agent.py | 17 +--------- skillscope/engine/tools.py | 13 ++++++-- tests/test_skillscope.py | 59 ++++++++++++++++++++++------------ 3 files changed, 51 insertions(+), 38 deletions(-) diff --git a/skillscope/engine/cli_agent.py b/skillscope/engine/cli_agent.py index 792e65f..dd97508 100644 --- a/skillscope/engine/cli_agent.py +++ b/skillscope/engine/cli_agent.py @@ -26,7 +26,6 @@ from __future__ import annotations import json -import os import shutil from pathlib import Path @@ -37,19 +36,6 @@ _CALL_PREFIX = "cli" -def launch_argv(argv: list[str]) -> list[str]: - """How to start the CLI, given that on Windows it is not an executable. - - npm installs `claude` as a `.cmd` shim. The legacy engine launches it with - the synchronous `subprocess.run`, which copes; inspect's asyncio subprocess - execs the path directly and Windows answers `WinError 2`, which reads as - "the CLI is not installed" when it plainly is. - """ - if os.name == "nt" and argv[0].lower().endswith((".cmd", ".bat")): - return ["cmd.exe", "/c", *argv] - return argv - - def require_local() -> None: """This driver runs on the host, so the sandbox has to be the host.""" from . import sandbox as sandbox_spec @@ -162,8 +148,7 @@ async def solve(state, generate): cmd += ["--effort", effort] result = await sandbox_subprocess( - launch_argv(cmd), input=prompt, cwd=workspace, - env=legacy_agent.claude_env(), + cmd, input=prompt, cwd=workspace, env=legacy_agent.claude_env(), ) events: list[dict] = [] diff --git a/skillscope/engine/tools.py b/skillscope/engine/tools.py index f1de14e..ffcce72 100644 --- a/skillscope/engine/tools.py +++ b/skillscope/engine/tools.py @@ -52,8 +52,17 @@ async def shell_prefix() -> list[str]: if cached: return list(cached) - probe = await sandbox().exec(["bash", "-lc", "exit 0"], concurrency=False) - prefix = POSIX_SHELL if probe.success else WINDOWS_SHELL + # 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) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 521883b..89ed2bb 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -19,6 +19,8 @@ from __future__ import annotations import argparse +import sys +import asyncio import inspect import contextlib import io @@ -2649,29 +2651,46 @@ def test_the_skill_lands_where_the_harness_looks(self) -> None: self.assertTrue((staged / "scripts" / "validate.py").is_file()) -class TestEngineCliAgentLaunch(unittest.TestCase): - """npm ships `claude` as a .cmd shim, which Windows will not exec.""" +class TestShellPrefixProbe(unittest.TestCase): + """A guest without bash raises rather than answering.""" - def test_windows_runs_a_cmd_shim_through_the_interpreter(self) -> None: - with mock.patch.object(engine_cli_agent.os, "name", "nt"): - argv = engine_cli_agent.launch_argv([r"C:\npm\claude.CMD", "-p"]) - # Without this the failure is WinError 2, which reads as "the CLI is - # not installed" when it plainly is. - self.assertEqual(argv, ["cmd.exe", "/c", r"C:\npm\claude.CMD", "-p"]) + def _probe(self, exc: Exception | None): + from skillscope.engine import tools as t - def test_a_real_executable_is_left_alone(self) -> None: - with mock.patch.object(engine_cli_agent.os, "name", "nt"): - self.assertEqual( - engine_cli_agent.launch_argv([r"C:\bin\claude.exe"]), - [r"C:\bin\claude.exe"], - ) + class _Result: + success = True - def test_posix_is_untouched(self) -> None: - with mock.patch.object(engine_cli_agent.os, "name", "posix"): - self.assertEqual( - engine_cli_agent.launch_argv(["/usr/bin/claude", "-p"]), - ["/usr/bin/claude", "-p"], - ) + 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 TestEngineCliAgentGuard(unittest.TestCase): From 9015640d47e0503d67e977d77e4dab74e8db7280 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Thu, 17 Sep 2026 10:36:21 +0000 Subject: [PATCH 32/33] Drop the live sample buffer on Windows inspect keeps a sqlite buffer so `inspect view` can watch a run in progress. It lives under the user data directory, named after the task, and on this runner that directory is the service account's profile -- 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. Measured rather than inferred this time: 241 characters writes, 260 does not, on the same machine in the same run. 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. LOCALAPPDATA does not help -- platformdirs asks Windows, not the environment, so the short path I set last commit was ignored. That commit's reasoning was right and its remedy did nothing. --- skillscope/engine/behavioral.py | 19 +++++++++++++++++++ skillscope/engine/verify.py | 1 + tests/test_skillscope.py | 12 ++++++++++++ 3 files changed, 32 insertions(+) diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index 77a45fd..713616f 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -11,6 +11,7 @@ from __future__ import annotations +import sys from pathlib import Path from .. import agent, config, deadline, usage @@ -31,6 +32,23 @@ MOCK_MESSAGE_LIMIT = 6 +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): @@ -185,6 +203,7 @@ def run( 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. diff --git a/skillscope/engine/verify.py b/skillscope/engine/verify.py index d233f12..1e8c121 100644 --- a/skillscope/engine/verify.py +++ b/skillscope/engine/verify.py @@ -137,6 +137,7 @@ def run( 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: diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 89ed2bb..1c76f45 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2651,6 +2651,18 @@ def test_the_skill_lands_where_the_harness_looks(self) -> None: self.assertTrue((staged / "scripts" / "validate.py").is_file()) +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.""" From 3c3bd5e04a445504e4061d9641eca505283ee377 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Thu, 17 Sep 2026 12:42:31 +0000 Subject: [PATCH 33/33] Make a timeout say where it got to The `--timeout` deadline ends the process with os._exit, which takes the report and the transcript with it, so an overrun says only that it overran. inspect has its own per-sample limit and we were handing it the whole budget -- so the two fired together and the hard kill won the race. Keep two minutes back. inspect stops the sample, scores what exists and writes the log; the deadline stays as the backstop for a genuinely hung process, which is what it is for. Eight Instinct runs have now overrun. Not one of them said whether the agent was working, waiting, or stuck, and two of the three diagnoses that followed were wrong because there was nothing to read. --- skillscope/engine/behavioral.py | 19 ++++++++++++++++++- skillscope/engine/verify.py | 2 +- tests/test_skillscope.py | 19 +++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py index 713616f..b7eeb96 100644 --- a/skillscope/engine/behavioral.py +++ b/skillscope/engine/behavioral.py @@ -32,6 +32,23 @@ 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. @@ -118,7 +135,7 @@ def build_task( scorer=scorers.expectations(), sandbox=sandbox_spec.for_skill(skill), message_limit=message_limit_for(model), - time_limit=int(bound.remaining()) if bound is not None else None, + time_limit=task_time_limit(bound), ) diff --git a/skillscope/engine/verify.py b/skillscope/engine/verify.py index 1e8c121..9c2efc5 100644 --- a/skillscope/engine/verify.py +++ b/skillscope/engine/verify.py @@ -111,7 +111,7 @@ def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = Non scorer=scorers.expectations(), sandbox=sandbox_spec.for_skill(skill), message_limit=behavioral.message_limit_for(model), - time_limit=int(bound.remaining()) if bound is not None else None, + time_limit=behavioral.task_time_limit(bound), ) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 1c76f45..06e5f57 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2651,6 +2651,25 @@ def test_the_skill_lands_where_the_harness_looks(self) -> None: 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."""