From da0b739435a39a9cf9070c39f1899acbf5df3c3a Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Fri, 23 Oct 2026 18:12:33 +0800 Subject: [PATCH 1/8] feat: automate skill PR evaluation planning and feedback Add revision-bound planning, explicit operator approval and retained-result PR summaries around the existing comparison tool. Agent-assisted-by: Codex / unknown [effort=unknown] (implementation) Signed-off-by: GUOGUO --- .github/workflows/skill-eval-pr.yml | 163 +++++++++ docs/operations.md | 1 + docs/skill-eval-comparison.md | 3 + docs/skill-eval-pr.md | 182 ++++++++++ tests/fixtures/skill_eval_pr_demo.py | 35 ++ tests/fixtures/skill_eval_pr_support.py | 95 ++++++ tests/test_skill_eval_pr.py | 262 ++++++++++++++ tools/skill_eval_pr.py | 436 ++++++++++++++++++++++++ tools/skill_eval_pr_cases.json | 19 ++ 9 files changed, 1196 insertions(+) create mode 100644 .github/workflows/skill-eval-pr.yml create mode 100644 docs/skill-eval-pr.md create mode 100644 tests/fixtures/skill_eval_pr_demo.py create mode 100644 tests/fixtures/skill_eval_pr_support.py create mode 100644 tests/test_skill_eval_pr.py create mode 100644 tools/skill_eval_pr.py create mode 100644 tools/skill_eval_pr_cases.json diff --git a/.github/workflows/skill-eval-pr.yml b/.github/workflows/skill-eval-pr.yml new file mode 100644 index 00000000..0852c433 --- /dev/null +++ b/.github/workflows/skill-eval-pr.yml @@ -0,0 +1,163 @@ +name: Skill PR evaluation +run-name: >- + ${{ github.event_name == 'workflow_dispatch' && inputs.action == 'execute' + && format('skill-eval execute PR {0} plan {1}', inputs.pr, inputs.plan_sha256) + || format('skill-eval plan or publish PR {0}', inputs.pr || github.event.pull_request.number) }} + +on: + pull_request_target: + branches: [main] + types: [opened, synchronize, reopened] + paths: + - src/inferrsi/data/skill_library/** + - tools/skill_eval_* + - docs/examples/skill-comparison/** + - .github/workflows/skill-eval-pr.yml + workflow_dispatch: + inputs: + action: + type: choice + options: [execute, publish] + required: true + description: Execute consumes authorization once; publish reuses retained results + pr: + required: true + description: PR number + plan_sha256: + required: true + description: Exact PR plan SHA-256 shown by automatic planning + version: + required: true + description: Exact trusted implementation SHA shown in the plan + base: + required: true + description: Exact compared base SHA + head: + required: true + description: Exact compared head SHA + budget: + required: true + description: Explicit approval of the complete Budget JSON shown in the plan + source_run: + description: Execution workflow run ID, required for publish-only retries + +permissions: {} + +# Every writer and execution for a PR is serialized; never cancel paid work. +concurrency: + group: skill-eval-pr-${{ inputs.pr || github.event.pull_request.number }} + cancel-in-progress: false + +env: + PYTHONPATH: src + +jobs: + plan: + if: github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + pull-requests: read + actions: read + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.workflow_sha }} + persist-credentials: false + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - run: python -m pip install '.[skills]' + - name: Prepare from trusted code and raw PR Git objects + id: prepare + env: + GITHUB_TOKEN: ${{ github.token }} + run: python -m tools.skill_eval_pr prepare --output prepared + - name: Validate explicit operator authorization and reserve execution + if: github.event_name == 'workflow_dispatch' && inputs.action == 'execute' + env: + GITHUB_TOKEN: ${{ github.token }} + run: python -m tools.skill_eval_pr authorize --output prepared + - uses: actions/upload-artifact@v4 + with: + name: skill-eval-plan + path: prepared + retention-days: 30 + + execute: + if: github.event_name == 'workflow_dispatch' && inputs.action == 'execute' + needs: plan + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + contents: read + actions: read + pull-requests: read + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.workflow_sha }} + persist-credentials: false + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - run: python -m pip install '.[skills]' + - uses: actions/download-artifact@v4 + with: + name: skill-eval-plan + path: prepared + - name: Existing client preflight (requires separately provisioned model access) + run: codex --version + - name: Recheck live revision and authorization before execution + env: + GITHUB_TOKEN: ${{ github.token }} + run: python -m tools.skill_eval_pr authorize --output prepared + - name: Execute existing comparison without publication credentials + run: python -m tools.skill_eval_pr execute --output prepared + - name: Package retained results even after interruption + if: always() + run: python -m tools.skill_eval_pr pack --output prepared + - uses: actions/upload-artifact@v4 + if: always() + with: + name: skill-eval-results + path: prepared/results.zip + retention-days: 30 + + publish: + if: always() && needs.plan.result == 'success' + needs: [plan, execute] + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + actions: read + pull-requests: read + issues: write + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.workflow_sha }} + persist-credentials: false + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - run: python -m pip install '.[skills]' + - name: Rebuild trusted inputs; never execute artifact code + env: + GITHUB_TOKEN: ${{ github.token }} + run: python -m tools.skill_eval_pr prepare --output prepared + - name: Update the single revision-bound summary from retained data + env: + GITHUB_TOKEN: ${{ github.token }} + run: python -m tools.skill_eval_pr publish --output prepared + - uses: actions/upload-artifact@v4 + if: always() + with: + name: skill-eval-report + path: | + prepared/pr-plan.json + prepared/*/report.json + prepared/*/report.md + retention-days: 30 diff --git a/docs/operations.md b/docs/operations.md index 87be846a..6737a4dc 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -28,6 +28,7 @@ Effect classes used below: | Execute/inspect experiments | CLI `run`; `status`; `report` | Experiment-executing / read-only / report-generating | `run` executes caller-supplied commands and records evidence; `status` reads recorded progress; `report` generates HTML from recorded evidence | | Evaluate selected skill revisions | [Bounded skill-evaluation driver](skill-evaluation.md) | Experiment-executing | Explicitly authorized real Codex calls, independent task verdicts and private evidence; separate from numeric engine-performance scoring | | Compare skill additions or updates | [Configured skill comparison](skill-eval-comparison.md), `python -m tools.skill_eval_compare` | Read-only preparation / experiment-executing / report-generating | Fixed Git snapshots and one shared exam; explicit plan authorization for execution; JSON + Markdown regeneration makes no model calls | +| Plan and report skill PR evaluations | [Skill PR integration](skill-eval-pr.md), `python -m tools.skill_eval_pr` | Read-only planning / authorized experiment-executing / PR-publishing | Trusted mapping, exact operator approval, consumed execution identity and credential-separated publication from retained evidence; no automatic paid trials | The `collect`, `read` and `resource` commands share the reader's rules; they are distinct from `knowledge search`, whose package catalog filters diff --git a/docs/skill-eval-comparison.md b/docs/skill-eval-comparison.md index acfade0f..ccff0d8e 100644 --- a/docs/skill-eval-comparison.md +++ b/docs/skill-eval-comparison.md @@ -5,6 +5,9 @@ generates JSON and Markdown reports. It reuses the existing reader, `CodexClient`, `run_trial`, verifier and error handling. The original [pilot](skill-evaluation.md) remains available with its original interfaces. +The [PR integration](skill-eval-pr.md) adds automatic planning, explicit operator +authorization and retained-result publication around these interfaces. + ```mermaid flowchart LR A["New: small JSON config"] --> B["New: fixed Git snapshots"] diff --git a/docs/skill-eval-pr.md b/docs/skill-eval-pr.md new file mode 100644 index 00000000..43aa5818 --- /dev/null +++ b/docs/skill-eval-pr.md @@ -0,0 +1,182 @@ +# Skill PR planning and feedback + +The report-only [Skill PR workflow](../.github/workflows/skill-eval-pr.yml) connects +skill changes to the existing [comparison command](skill-eval-comparison.md). +It does not add an execution engine or a required efficacy gate. Refs #101; parent #88. + +```mermaid +flowchart LR + A["PR Git objects"] --> B["Trusted mapping → frozen plan"] + B --> C["Authorized operator: versions + hash + budget"] + C --> D["Existing comparison client; no publication credentials"] + D --> E["Retained results: 30 days"] + E --> F["Trusted publisher → one PR summary"] +``` + +## Automatic planning + +`pull_request_target` uses the workflow implementation on trusted main. It reads +PR commits as raw Git objects without checking out PR files, importing PR Python, +running PR graders, or invoking a model. The small +[mapping](../tools/skill_eval_pr_cases.json) owns skill/resource paths and the +existing case file. A new entrypoint selects absent/new; an existing entrypoint +selects old/new. Declared resource/dependency paths can select the same comparison. +Add mappings only when the named cases actually cover that skill's behavior. +Unmapped paths remain `uncovered`. Deletions and unsupported snapshot modes are +explicitly `unsupported`; they cannot count as passes. Rename detection is off: +a rename is a deletion plus an addition and retains the deletion gap. + +The initial mapping covers only `inference-benchmark-design` and the existing +`audit-threshold` holdout case. Library navigation/index changes are uncovered; +coverage of one skill is not library-wide qualification. Changes to the mapping, +cases, workflow or comparison implementation also trigger planning. + +The PR plan contains compared base/head SHAs, trusted implementation SHA, mapping +and adapter hashes, selected cases/splits, frozen grader/schema/case hashes, +model/effort, limits, estimated calls and coverage gaps. Its SHA-256 is computed +over canonical JSON (sorted keys, compact separators). Each comparison retains +its existing `plan/freeze.json` SHA-256. The PR plan hash binds those hashes; it +does not replace or modify the comparison format. Execution identity is +`skill-eval-`. Repeated preparation is deterministic. + +Plans and reports have one identifiable bot comment beginning with +``. A new head/base, trusted implementation, +mapping, exam, model or budget produces a new plan. Earlier authorization and +results are stale. The planner replaces the summary with the current plan and +missing pairs. A publisher checks live revisions twice and refuses older writer +runs, so delayed jobs cannot replace a newer summary. All plan/execute/publish +runs for one PR share non-cancelling Actions concurrency. + +## Explicit execution authorization + +Planning grants no execution budget. An operator with repository `write`, +`maintain` or `admin` permission dispatches **Skill PR evaluation** on **main**: + +| Dispatch input | Required approval | +| --- | --- | +| `action=execute`, `pr` | The PR being evaluated | +| `plan_sha256` | The exact PR plan hash displayed in its summary | +| `version` | The full trusted implementation SHA from that summary | +| `base`, `head` | Both full compared revisions | +| `budget` | The complete displayed Budget JSON, including limits per comparison and total estimated calls | + +The authenticated dispatch actor's current collaborator permission is queried; +knowledge of a hash or a PR comment is insufficient. The explicit inputs must +match the newly reconstructed plan exactly. Authorization and current revisions +are rechecked in the execution job immediately before starting the existing +comparison command. No shell evaluates user-provided inputs. + +The selected model is **`gpt-6.1-sol`, effort `xhigh`**, requested by the owner. +The initial mapping schedules **2 task calls**: one absent/old and one new arm, +one case, one repeat. Limits are 180 seconds per trial, 600 seconds per comparison, +and an observed-token stop threshold of 20,000. This stops new trials; it is not +a provider token or monetary hard cap. One trial can cross the threshold, and +unreported usage/cost remain unknown. Mapping more skills increases the total; +the exact total is shown in the plan and requires renewed authorization. + +### Execution environment prerequisite + +Execution reuses `CodexClient` and the current process cleanup path. The existing +client executable and separately authorized model access must already be +available in that job's environment. The repository currently has **no such +hosted-job model provisioning**. The workflow fails its `codex --version` preflight +when the client is absent; it never substitutes fixed outputs for real execution. +This delivery adds no runner, model credential, secret or repository security +setting. Real-agent end-to-end acceptance remains blocked on owner-approved +access to the selected client in the execution environment. Do not dispatch a +paid trial until that prerequisite and its budget are separately authorized. + +The execute job has read-only repository permissions, no publication token in +its model step, and checkout uses `persist-credentials: false`. The privileged +publisher is a separate fresh job. Model access must not also grant PR publishing. +The existing client's controlled skill exposure is not OS read confinement. + +## Duplicate prevention and interrupted execution + +Actions records the authorization dispatch as +`skill-eval execute PR plan `. Under the shared concurrency +group, only the earliest matching execution run may proceed. Every status is +considered, including queued, failed and cancelled runs. Attempts other than +`run_attempt=1` cannot execute. An exclusive local claim file additionally prevents +two callers sharing the same retained directory from entering the comparison. + +The Actions run is the durable execution tombstone, not the expiring artifact. +An interrupted or failed attempt is consumed even if it might have stopped +before any paid work. There is no automatic paid retry/resume. Do not delete +execution workflow runs; inaccessible/incomplete run history fails closed. +Deleting this coordination history removes duplicate-prevention evidence and +requires human reconciliation before any further execution of the same plan. +Workflow reruns may republish results but must not rerun evaluation. + +## Publication and retention + +Use `action=publish` with the original **execution `source_run` ID** and the current +plan inputs to retry publication. This mode contains no call to the client or +comparison executor. A lost publication response is safe: the next attempt finds +and updates the same marked bot comment instead of creating another summary. + +The publisher reconstructs trusted inputs from Git and the maintained mapping. +It requires the source run's workflow path, event, trusted revision and exact +plan identity. It downloads only that run's uniquely named result artifact. +ZIP members must be bounded, unique, regular files at explicitly scheduled +result paths; traversal, symlinks, executable scripts and replacement plans or +Markdown are rejected before extraction. GitHub authorization is stripped on +artifact-download redirects. The publisher imports no artifact Python and runs +no artifact scripts. It regenerates the existing report from retained data and +cross-checks accepted grades with the trusted default audit verifier in process. +Custom graders are outside this small mapping's support boundary. + +Reports show fix, regression, unchanged-correct, unchanged-incorrect, +inconclusive and missing-pair counts, plus uncovered/unsupported changes, +revisions, settings, limits, exam identity and an evidence link. Discovery stays +separate. Raw model text and supplied Markdown never become PR comment content. +Malformed or inconsistent evidence fails closed or remains inconclusive; it +cannot turn into a positive result. These integrity checks are not cryptographic +attestation of a hostile executor's observations. + +| Artifact | Content | Retention | +| --- | --- | --- | +| `skill-eval-plan` | PR plan, comparison freezes, bundles and validator inputs; generated authorization receipt for execution | 30 days | +| `skill-eval-results` | `results.zip`, containing retained trial/status records, answers, hashes and raw evidence | 30 days | +| `skill-eval-report` | Trusted regenerated JSON/Markdown reports and PR plan | 30 days | + +Results remain generated Actions artifacts, not routine commits to main. The +summary labels outcomes historical and gives their verification expiry. They +are verifiable only while the artifact remains available and before its expiry; +the comment alone is not retained evidence. A publication retry after expiry +replaces the outcome counts with **EXPIRED / unverified** and never reevaluates. +Missing/deleted artifacts cannot substantiate results. Repository retention +limits can shorten the requested 30 days. There is no background expiry monitor. + +## Offline verification and real acceptance + +Run the integration controls and original pilot regressions: + +```bash +python -m pytest tests/test_skill_eval_pr.py tests/test_skill_eval_compare.py \ + tests/test_skill_eval_pipeline.py tests/test_skill_eval_verifier.py \ + tests/test_skill_evaluation.py +ruff check src tests tools +pytest +``` + +Generate a complete offline example for either mode: + +```bash +python -m tests.fixtures.skill_eval_pr_demo --mode addition --output +python -m tests.fixtures.skill_eval_pr_demo --mode update --output +``` + +These commands use temporary real Git commits, the existing verifier, fixed-output +clients and a simulated GitHub interface. They make no GitHub requests or model +calls. Each generates a fix, regression, unchanged-correct, unchanged-incorrect, +inconclusive and missing pair, a single simulated PR summary, and retained +evidence. They validate integration behavior only. The summary's example GitHub +link is simulated; inspect the corresponding local `publication/` evidence. + +The workflow becomes active after this implementation is on main. Draft PR CI +tests its offline behavior; it does not activate an unmerged privileged workflow. +#101/#88 remain open until one **real addition** and one **real update** complete +the separately authorized agent execution and automatic PR feedback path. +Neutral/negative outcomes are valid. Neither fixed outputs nor green CI establish +skill efficacy, authorize paid trials, merge a PR or close either issue. diff --git a/tests/fixtures/skill_eval_pr_demo.py b/tests/fixtures/skill_eval_pr_demo.py new file mode 100644 index 00000000..62c07e4f --- /dev/null +++ b/tests/fixtures/skill_eval_pr_demo.py @@ -0,0 +1,35 @@ +import argparse +import tempfile +from pathlib import Path + +from tests.fixtures.skill_eval_compare_support import FixedOutputClient +from tests.fixtures.skill_eval_pr_support import SimulatedGitHub, approval, setup +from tools import skill_eval_pr as integration + + +def main(): + parser = argparse.ArgumentParser(description="Offline PR integration with fixed outputs") + parser.add_argument("--mode", choices=("addition", "update"), default="update") + parser.add_argument("--output", type=Path, required=True) + args = parser.parse_args() + output = args.output.resolve() + output.mkdir(parents=True, exist_ok=False) + with tempfile.TemporaryDirectory(prefix="se-pr-demo-") as temporary: + root = Path(temporary) + repo, mapping, identity = setup(root, args.mode, token_stop_new_trials=200) + plan = integration.prepare(repo, mapping, identity, output / "execution") + client = FixedOutputClient(root, {("fix", "baseline"): "incorrect", + ("regression", "candidate"): "incorrect", ("wrong", "baseline"): "incorrect", + ("wrong", "candidate"): "incorrect", ("error", "candidate"): "invalid_json"}) + integration.execute(output / "execution", approval(plan), client=client) + data = integration.result_archive(output / "execution").read_bytes() + integration.prepare(repo, mapping, identity, output / "publication") + api = SimulatedGitHub(plan, data) + integration.publish_retained(api, output / "publication", 10, 11, 1) + (output / "pr-summary.md").write_text(api.comments[0]["body"], encoding="utf-8") + print({"mode": args.mode, "fixed_output_calls": len(client.calls), + "pr_plan_sha256": integration.fingerprint(plan), "simulated_comments": 1}) + + +if __name__ == "__main__": + main() diff --git a/tests/fixtures/skill_eval_pr_support.py b/tests/fixtures/skill_eval_pr_support.py new file mode 100644 index 00000000..2c4fdf72 --- /dev/null +++ b/tests/fixtures/skill_eval_pr_support.py @@ -0,0 +1,95 @@ +import io +import shutil +import zipfile + +from inferrsi.infra.artifacts import write_json +from tests.fixtures.skill_eval_compare_support import configuration, git +from tools import skill_eval_compare as compare +from tools import skill_eval_pr as integration + + +def setup(root, mode="update", ids=("fix", "regression", "same", "wrong", "error", "missing"), + **limits): + config_path, repo = configuration(root, ids=ids, **limits) + config = compare.read_json(config_path) + before = config["snapshots"]["baseline"]["git_revision"] + head = config["snapshots"]["candidate"]["git_revision"] + if mode == "addition": + addition = root / "addition" + addition.mkdir() + git(addition, "init", "--quiet") + (addition / "seed.txt").write_text("Before skills", encoding="utf-8") + git(addition, "add", "seed.txt") + git(addition, "commit", "--quiet", "-m", "Fixture before skill addition") + before = git(addition, "rev-parse", "HEAD") + shutil.copytree(repo / "library", addition / "library") + git(addition, "add", "library") + git(addition, "commit", "--quiet", "-m", "Fixture skill addition") + repo, head = addition, git(addition, "rev-parse", "HEAD") + mapping = {"skill_root": "library", "model": config["model"], "limits": config["limits"], + "skills": [{"skill": "selected", "entrypoint": "selected/SKILL.md", + "paths": ["library/selected", "library/shared/notes.md"], + "cases": "cases.json"}]} + path = root / "mapping.json" + write_json(path, mapping) + identity = {"repository": "example/fixture", "pr": 7, "base": before, + "head": head, "version": head} + return repo, path, identity + + +def approval(plan): + request = {"plan_sha256": integration.fingerprint(plan), "version": plan["version"], + "base": plan["base"], "head": plan["head"], "budget": integration.budget(plan)} + current = {key: plan[key] for key in ("base", "head")} + return integration.authorize(plan, request, "fixture-operator", "write", current) + + +def envelope(data): + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w", zipfile.ZIP_DEFLATED) as archive: + archive.writestr("results.zip", data) + return buffer.getvalue() + + +class SimulatedGitHub(integration.GitHub): + def __init__(self, plan, data=b"", run_id=10): + super().__init__(plan["repository"], "offline-not-a-credential") + self.current = {key: plan[key] for key in ("base", "head")} + self.comments, self.writes, self.fail_response = [], [], False + self.change_during_read = False + self.run = {"id": run_id, "path": integration.WORKFLOW, "event": "workflow_dispatch", + "display_title": integration.run_title(plan["pr"], + integration.fingerprint(plan)), + "head_sha": plan["version"], "status": "completed"} + self.artifact = {"id": 90, "name": "skill-eval-results", "expired": False, + "expires_at": "2030-01-01T00:00:00Z", "size_in_bytes": len(data)} + self.data = envelope(data) + + def request(self, method, path, data=None, *, binary=False): + if method == "GET": + if path.startswith("/pulls/"): + return {"state": "open", **{key: {"sha": value} + for key, value in self.current.items()}} + if path.startswith("/actions/runs/") and "/artifacts" not in path: + return self.run + if path.startswith("/actions/runs/") and "/artifacts" in path: + return {"artifacts": [self.artifact]} + if path.startswith("/actions/artifacts/"): + assert binary + return self.data + if path.startswith("/issues/") and "/comments?" in path: + if self.change_during_read: + self.current["head"] = "f" * 40 + return self.comments + if method in ("POST", "PATCH"): + self.writes.append((method, path, data)) + if method == "POST": + self.comments.append({"id": 1, "user": {"login": "github-actions[bot]"}, + "body": data["body"]}) + else: + self.comments[0]["body"] = data["body"] + if self.fail_response: + self.fail_response = False + raise ConnectionError("Offline lost response after server accepted publication") + return {"html_url": "https://github.com/example/fixture/pull/7#issuecomment-1"} + raise AssertionError((method, path)) diff --git a/tests/test_skill_eval_pr.py b/tests/test_skill_eval_pr.py new file mode 100644 index 00000000..8c800b49 --- /dev/null +++ b/tests/test_skill_eval_pr.py @@ -0,0 +1,262 @@ +import copy +import io +import stat +import threading +import urllib.request +import zipfile +from concurrent.futures import ThreadPoolExecutor + +import pytest +import yaml + +from inferrsi.infra.artifacts import sha256, write_json +from tests.fixtures.skill_eval_compare_support import ROOT, FixedOutputClient, git +from tests.fixtures.skill_eval_pr_support import SimulatedGitHub, approval, envelope, setup +from tools import skill_eval_compare as compare +from tools import skill_eval_pr as integration + + +@pytest.mark.parametrize("mode", ["addition", "update"]) +def test_plan_selects_addition_update_resources_without_execution(tmp_path, mode): + repo, mapping, identity = setup(tmp_path, mode) + output = tmp_path / "planned" + plan = integration.prepare(repo, mapping, identity, output) + item = plan["comparisons"][0] + assert item["mode"] == mode and item["calls"] == 12 + assert item["exam"]["grader"] == sha256(ROOT / "tools/skill_eval_verifier.py") + frozen = compare.load_plan(output / "selected") + assert (frozen["arms"]["baseline"]["bundle"] is None) == (mode == "addition") + assert frozen["arms"]["candidate"]["git_revision"] == identity["head"] + assert all(c["coverage"] == "covered" for c in plan["coverage"] + if c["path"].startswith("library/selected/")) + if mode == "update": + assert {c["coverage"] for c in plan["coverage"] if c["path"] == "exam.txt"} == {"uncovered"} + assert list((output / "selected/results").iterdir()) == [] + second = integration.prepare(repo, mapping, identity, tmp_path / "same-plan") + assert integration.fingerprint(plan) == integration.fingerprint(second) + assert integration.execution_id(plan) == integration.execution_id(second) + + +def test_deleted_and_unmapped_skill_changes_are_visible_not_passed(tmp_path): + repo, mapping, identity = setup(tmp_path) + git(repo, "rm", "library/selected/resource.txt") + (repo / "library/unknown").mkdir() + (repo / "library/unknown/SKILL.md").write_text("Unmapped skill", encoding="utf-8") + git(repo, "add", "library") + git(repo, "commit", "--quiet", "-m", "Fixture unsupported deletion and unknown skill") + identity.update(base=identity["head"], head=git(repo, "rev-parse", "HEAD")) + plan = integration.prepare(repo, mapping, identity, tmp_path / "plan") + assert plan["comparisons"] == [] and plan["estimated_calls"] == 0 + assert {c["coverage"] for c in plan["coverage"]} == {"uncovered", "unsupported"} + body = integration.summary(plan, [], "https://github.com/example/fixture/actions/runs/1", 10, 1) + assert "uncovered" in body and "unsupported" in body and '"fix":0' in body + with pytest.raises(ValueError, match="No supported comparisons"): + approval(plan) + + +def test_unsupported_file_mode_does_not_disappear_or_become_a_pass(tmp_path): + repo, mapping, identity = setup(tmp_path) + git(repo, "update-index", "--chmod=+x", "library/selected/resource.txt") + git(repo, "commit", "--quiet", "-m", "Fixture executable resource") + identity["head"] = git(repo, "rev-parse", "HEAD") + plan = integration.prepare(repo, mapping, identity, tmp_path / "plan") + assert plan["estimated_calls"] == 0 + assert "Executable snapshot entries" in plan["comparisons"][0]["unsupported"] + + +@pytest.mark.parametrize("field", ["plan_sha256", "version", "base", "head", "budget"]) +def test_authorization_requires_exact_revisions_and_budget(tmp_path, field): + repo, mapping, identity = setup(tmp_path) + plan = integration.prepare(repo, mapping, identity, tmp_path / "plan") + request = {k: v for k, v in approval(plan).items() if k not in ("actor", "execution_id")} + request[field] = "wrong" + with pytest.raises(ValueError, match="Explicit approval"): + integration.authorize(plan, request, "operator", "write", + {k: plan[k] for k in ("base", "head")}) + + +def test_hash_knowledge_and_old_heads_do_not_grant_execution(tmp_path): + repo, mapping, identity = setup(tmp_path) + plan = integration.prepare(repo, mapping, identity, tmp_path / "plan") + approved = approval(plan) + request = {k: v for k, v in approved.items() if k not in ("actor", "execution_id")} + for permission in ("read", "triage", "none"): + with pytest.raises(ValueError, match="operator"): + integration.authorize(plan, request, "reader", permission, + {k: plan[k] for k in ("base", "head")}) + with pytest.raises(ValueError, match="PR changed"): + integration.authorize(plan, request, "writer", "write", + {"base": plan["base"], "head": "f" * 40}) + # A changed exam/model/controller version produces another identity and approval. + for field in ("model", "limits", "comparisons", "version", "head", "base"): + changed = copy.deepcopy(plan) + changed[field] = {"changed": True} + assert integration.execution_id(changed) != approved["execution_id"] + + +def test_dispatch_history_blocks_duplicate_failed_cancelled_and_rerun_attempts(tmp_path): + repo, mapping, identity = setup(tmp_path) + plan = integration.prepare(repo, mapping, identity, tmp_path / "plan") + title = integration.run_title(7, integration.fingerprint(plan)) + runs = [{"id": 1, "event": "workflow_dispatch", "display_title": title, + "status": "completed", "conclusion": "cancelled"}, + {"id": 2, "event": "workflow_dispatch", "display_title": title}] + assert integration.claim(plan, runs, 1, 1) == integration.execution_id(plan) + for run, attempt in ((2, 1), (1, 2), (2, 2)): + with pytest.raises(ValueError): + integration.claim(plan, runs, run, attempt) + with pytest.raises(ValueError): + integration.claim(plan, [], 1, 1) + + +def test_concurrent_duplicate_calls_execute_only_one_paid_schedule(tmp_path): + repo, mapping, identity = setup(tmp_path, ids=("one",)) + output = tmp_path / "plan" + plan = integration.prepare(repo, mapping, identity, output) + approved = approval(plan) + client = FixedOutputClient(tmp_path) + barrier = threading.Barrier(2) + + def trigger(): + barrier.wait() + try: + integration.execute(output, approved, client=client) + return "executed" + except FileExistsError: + return "duplicate" + + with ThreadPoolExecutor(max_workers=2) as pool: + outcomes = list(pool.map(lambda _: trigger(), range(2))) + assert sorted(outcomes) == ["duplicate", "executed"] and len(client.calls) == 2 + with pytest.raises(FileExistsError): + integration.execute(output, approved, client=client) + + +@pytest.mark.parametrize("mode", ["addition", "update"]) +def test_retained_results_publish_once_retry_without_model_and_reject_stale_jobs(tmp_path, mode): + repo, mapping, identity = setup(tmp_path, mode, token_stop_new_trials=200) + output = tmp_path / "execution" + plan = integration.prepare(repo, mapping, identity, output) + client = FixedOutputClient(tmp_path, {("fix", "baseline"): "incorrect", + ("regression", "candidate"): "incorrect", ("wrong", "baseline"): "incorrect", + ("wrong", "candidate"): "incorrect", ("error", "candidate"): "invalid_json"}) + integration.execute(output, approval(plan), client=client) + data = integration.result_archive(output).read_bytes() + publisher = tmp_path / "publisher" + integration.prepare(repo, mapping, identity, publisher) + api = SimulatedGitHub(plan, data) + api.fail_response = True + with pytest.raises(ConnectionError): + integration.publish_retained(api, publisher, 10, 11, 1) + calls = len(client.calls) + receipt = integration.publish_retained(api, publisher, 10, 11, 1) + assert receipt["html_url"].endswith("issuecomment-1") + assert len(api.comments) == 1 and len(client.calls) == calls + body = api.comments[0]["body"] + assert '"fix":1' in body and '"regression":1' in body + assert '"unchanged_correct":1' in body and '"unchanged_incorrect":1' in body + assert '"inconclusive":1' in body and '"missing_pair":1' in body + assert identity["head"] in body and "Uncovered/unsupported" in body + with pytest.raises(ValueError, match="Older workflow"): + integration.publish_retained(api, publisher, 10, 9, 1) + api.current["head"] = "f" * 40 + with pytest.raises(ValueError, match="Stale report"): + integration.publish_retained(api, publisher, 10, 12, 1) + assert len(api.writes) == 2 and len(client.calls) == calls + + +def test_new_inputs_reject_old_artifact_and_expired_evidence_loses_verifiability(tmp_path): + repo, mapping, identity = setup(tmp_path, ids=("one",)) + output = tmp_path / "plan" + plan = integration.prepare(repo, mapping, identity, output) + api = SimulatedGitHub(plan) + api.artifact["expired"] = True + integration.publish_retained(api, output, 10, 11, 1) + assert "EXPIRED" in api.comments[0]["body"] + assert "Outcomes: `" not in api.comments[0]["body"] + api.run["head_sha"] = "f" * 40 + with pytest.raises(ValueError, match="Untrusted or mismatched"): + integration.publish_retained(api, output, 10, 12, 1) + api.run["head_sha"] = plan["version"] + api.change_during_read = True + with pytest.raises(ValueError, match="PR changed before publication"): + integration.publish_retained(api, output, 10, 12, 1) + assert len(api.writes) == 1 + + +@pytest.mark.parametrize("name", ["../escape", "/absolute", "selected/results/../../escape", + "selected/results/script.py", "report.md", "pr-plan.json", "selected/validator/verify.py", + "selected\\results\\status.json"]) +def test_untrusted_archive_paths_and_scripts_are_rejected(tmp_path, name): + repo, mapping, identity = setup(tmp_path) + output = tmp_path / "plan" + integration.prepare(repo, mapping, identity, output) + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + archive.writestr(name, "Untrusted content must never execute") + with pytest.raises(ValueError): + integration.import_results(buffer.getvalue(), output) + assert list((output / "selected/results").iterdir()) == [] + + +def test_symlinks_and_forged_grades_are_rejected_without_executing_artifact_scripts(tmp_path): + repo, mapping, identity = setup(tmp_path, ids=("one",)) + output = tmp_path / "plan" + plan = integration.prepare(repo, mapping, identity, output) + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + entry = zipfile.ZipInfo("selected/results/status.json") + entry.external_attr = (stat.S_IFLNK | 0o777) << 16 + archive.writestr(entry, "/private") + with pytest.raises(ValueError, match="regular"): + integration.import_results(buffer.getvalue(), output) + client = FixedOutputClient(tmp_path) + integration.execute(output, approval(plan), client=client) + path = output / "selected/results/one-candidate-1/result.json" + record = compare.read_json(path) + record["checks"] = {"passed": True, "assertions": {"fabricated": True}} + write_json(path, record) + write_json(path.parent / "receipt.json", {"result_sha256": sha256(path)}) + with pytest.raises(ValueError, match="trusted grader"): + integration.reports(output) + assert len(client.calls) == 2 + + +def test_artifact_envelope_and_run_provenance_are_validated(tmp_path): + repo, mapping, identity = setup(tmp_path) + output = tmp_path / "plan" + plan = integration.prepare(repo, mapping, identity, output) + api = SimulatedGitHub(plan) + for field, value in (("path", "attacker.yml"), ("event", "pull_request"), + ("display_title", "wrong plan"), ("status", "in_progress")): + original = api.run[field] + api.run[field] = value + with pytest.raises(ValueError, match="Untrusted or mismatched"): + integration.publish_retained(api, output, 10, 11, 1) + api.run[field] = original + api.data = envelope(b"not a zip") + with pytest.raises(zipfile.BadZipFile): + integration.publish_retained(api, output, 10, 11, 1) + assert api.writes == [] + + +def test_workflow_separates_execution_and_publication_credentials(): + workflow = yaml.safe_load((ROOT / integration.WORKFLOW).read_text(encoding="utf-8")) + assert workflow["concurrency"]["cancel-in-progress"] is False + executor = workflow["jobs"]["execute"] + assert all(value != "write" for value in executor["permissions"].values()) + assert workflow["jobs"]["publish"]["permissions"]["issues"] == "write" + for job in workflow["jobs"].values(): + checkout = next(step for step in job["steps"] if step.get("uses") == "actions/checkout@v4") + assert checkout["with"]["persist-credentials"] is False + assert checkout["with"]["ref"] == "${{ github.workflow_sha }}" + step = next(s for s in executor["steps"] if "execute --output" in s.get("run", "")) + assert "env" not in step + + +def test_download_redirect_drops_the_github_token(): + request = urllib.request.Request("https://api.github.com/artifact", + headers={"Authorization": "Bearer offline-only"}) + redirected = integration.ArtifactRedirect().redirect_request( + request, None, 302, "Found", {}, "https://storage.example.invalid/signed-artifact") + assert not redirected.has_header("Authorization") diff --git a/tools/skill_eval_pr.py b/tools/skill_eval_pr.py new file mode 100644 index 00000000..68a0e819 --- /dev/null +++ b/tools/skill_eval_pr.py @@ -0,0 +1,436 @@ +import argparse +import hashlib +import html +import io +import json +import os +import re +import stat +import subprocess +import time +import urllib.request +import zipfile +from pathlib import Path, PurePosixPath + +from inferrsi.infra.artifacts import sha256, write_json +from tools import skill_eval_compare as compare +from tools.skill_eval_verifier import verify + +ROOT = Path(__file__).resolve().parents[1] +MAPPING = ROOT / "tools/skill_eval_pr_cases.json" +WORKFLOW = ".github/workflows/skill-eval-pr.yml" +MARKER = "" +RETENTION_DAYS = 30 +MAX_ARCHIVE = 100 * 1024 * 1024 +SHA = r"[0-9a-f]{40}" + + +def canonical(value): + return json.dumps(value, sort_keys=True, separators=(",", ":"), allow_nan=False) + + +def fingerprint(value): + return hashlib.sha256(canonical(value).encode()).hexdigest() + + +def git(repo, *args): + return subprocess.check_output([*compare.GIT_READ, *args], cwd=repo) + + +def changed_files(repo, base, head): + fields = git(repo, "diff", "--no-renames", "--name-status", "-z", base, head).split(b"\0") + return [{"status": fields[i].decode(), "path": fields[i + 1].decode()} + for i in range(0, len(fields) - 1, 2)] + + +def blob_exists(repo, revision, path): + return bool(git(repo, "ls-tree", revision, "--", path).strip()) + + +def prepare(repo, mapping_path, identity, output): + # Read only trusted configuration and raw Git objects, never PR Python or graders. + for key in ("base", "head", "version"): + if not re.fullmatch(SHA, identity[key]): + raise ValueError("Expected immutable revision: " + key) + mapping = compare.read_json(mapping_path) + changes = changed_files(repo, identity["base"], identity["head"]) + output.mkdir(parents=True, exist_ok=False) + coverage, selected = [], set() + for change in changes: + matches = [item for item in mapping["skills"] if any( + change["path"] == path or change["path"].startswith(path.rstrip("/") + "/") + for path in item["paths"])] + status = "uncovered" + if matches: + status = "unsupported" if change["status"] not in ("A", "M") else "covered" + if status == "covered": + selected.update(item["skill"] for item in matches) + coverage.append({**change, "coverage": status}) + comparisons = [] + for item in mapping["skills"]: + if item["skill"] not in selected: + continue + entrypoint = mapping["skill_root"] + "/" + item["entrypoint"] + if not blob_exists(repo, identity["head"], entrypoint): + for entry in coverage: + if any(entry["path"].startswith(p.rstrip("/") + "/") + or entry["path"] == p for p in item["paths"]): + entry["coverage"] = "unsupported" + continue + config = {"mode": "update" if blob_exists(repo, identity["base"], entrypoint) + else "addition", "skill": item["skill"], "skill_root": mapping["skill_root"], + "cases": str((mapping_path.parent / item["cases"]).resolve()), + "snapshots": {"baseline": {"git_revision": identity["base"]}, + "candidate": {"git_revision": identity["head"]}}, + "model": mapping["model"], "limits": mapping["limits"]} + config_path = output / (item["skill"] + ".json") + write_json(config_path, config) + destination = output / item["skill"] + try: + digest = compare.prepare(config_path, repo, destination) + except ValueError as error: + # Unsupported snapshots remain visible; they are never task passes. + comparisons.append({"skill": item["skill"], "unsupported": str(error)}) + continue + plan = compare.load_plan(destination) + comparisons.append({"skill": item["skill"], "mode": plan["mode"], + "plan_sha256": digest, "cases": [ + {"id": t["id"], "split": t["split"]} + for t in compare.read_json(destination / "plan/tasks.json")], + "exam": plan["exam"], "calls": len(plan["schedule"])}) + plan = {"format_version": 1, **identity, "mapping_sha256": sha256(mapping_path), + "adapter_sha256": sha256(Path(__file__)), "coverage": coverage, + "comparisons": comparisons, "model": mapping["model"], "limits": mapping["limits"], + "estimated_calls": sum(c.get("calls", 0) for c in comparisons), + "retention_days": RETENTION_DAYS} + write_json(output / "pr-plan.json", plan) + return plan + + +def budget(plan): + return {"estimated_calls": plan["estimated_calls"], "limits_per_comparison": plan["limits"], + "comparisons": sum("plan_sha256" in c for c in plan["comparisons"])} + + +def execution_id(plan): + return "skill-eval-" + fingerprint(plan) + + +def run_title(pr, digest): + return f"skill-eval execute PR {pr} plan {digest}" + + +def authorize(plan, request, actor, permission, current): + expected = {"plan_sha256": fingerprint(plan), "version": plan["version"], + "base": plan["base"], "head": plan["head"], "budget": budget(plan)} + if permission not in ("write", "maintain", "admin") or not actor: + raise ValueError("Execution requires an operator with repository write permission") + if request != expected: + raise ValueError("Explicit approval must match the exact plan, versions and budget") + if current != {"base": plan["base"], "head": plan["head"]}: + raise ValueError("PR changed; prepare and authorize a new plan") + if not plan["estimated_calls"]: + raise ValueError("No supported comparisons selected") + return {**expected, "actor": actor, "execution_id": execution_id(plan)} + + +def claim(plan, runs, run_id, attempt): + # Actions' serialized workflow and durable run history are the execution tombstone. + # Failed/cancelled/rerun attempts remain consumed, even after evidence expires. + if attempt != 1: + raise ValueError("Execution attempts cannot be rerun; publish retained evidence only") + title = run_title(plan["pr"], fingerprint(plan)) + owners = [r["id"] for r in runs if r["event"] == "workflow_dispatch" + and r["display_title"] == title] + if not owners or min(owners) != run_id: + raise ValueError("Duplicate or uncertain execution; paid work will not be restarted") + return execution_id(plan) + + +def execute(output, approval, *, client=None): + plan = compare.read_json(output / "pr-plan.json") + if approval["execution_id"] != execution_id(plan): + raise ValueError("Approval belongs to another plan") + # Exclusive creation also protects local/offline callers sharing retained storage. + with (output / "execution-claim.json").open("x", encoding="utf-8") as handle: + handle.write(canonical(approval)) + for item in plan["comparisons"]: + if "plan_sha256" in item: + compare.execute(output / item["skill"], item["plan_sha256"], client=client) + + +def result_archive(output): + plan = compare.read_json(output / "pr-plan.json") + archive = output / "results.zip" + with zipfile.ZipFile(archive, "w", zipfile.ZIP_DEFLATED) as zipped: + for item in plan["comparisons"]: + folder = output / item["skill"] / "results" + if folder.is_dir(): + for path in sorted(folder.rglob("*")): + if path.is_file() and not path.is_symlink(): + zipped.write(path, path.relative_to(output).as_posix()) + return archive + + +def import_results(data, output): + # No extractall, artifact imports, or artifact commands in the trusted publisher. + if len(data) > MAX_ARCHIVE: + raise ValueError("Result archive too large") + plan = compare.read_json(output / "pr-plan.json") + allowed = set() + for item in plan["comparisons"]: + if "plan_sha256" not in item: + continue + prefix = item["skill"] + "/results/" + allowed.update(prefix + p for p in ("start.json", "status.json", "status.receipt.json")) + frozen = compare.load_plan(output / item["skill"]) + for entry in frozen["schedule"]: + trial = compare.trial_directory(output / item["skill"], entry).name + files = ("answer.json", "result.json", "receipt.json", *compare.EVIDENCE) + allowed.update(prefix + trial + "/" + p for p in files) + with zipfile.ZipFile(io.BytesIO(data)) as zipped: + members = zipped.infolist() + names = [m.filename for m in members] + if (len(names) != len(set(names)) or set(names) - allowed + or sum(m.file_size for m in members) > MAX_ARCHIVE): + raise ValueError("Unexpected, duplicate or oversized result archive entries") + for member in members: + mode = member.external_attr >> 16 + if stat.S_ISLNK(mode) or (stat.S_IFMT(mode) not in (0, stat.S_IFREG)): + raise ValueError("Only regular result files are accepted") + path = PurePosixPath(member.filename) + if path.is_absolute() or ".." in path.parts or "\\" in member.filename: + raise ValueError("Unsafe result path") + for member in members: + target = output / member.filename + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(zipped.read(member)) + + +def reports(output): + plan = compare.read_json(output / "pr-plan.json") + results = [] + for item in plan["comparisons"]: + if "plan_sha256" not in item: + continue + directory = output / item["skill"] + result = compare.report(directory) + tasks = {t["id"]: t for t in compare.read_json(directory / "plan/tasks.json")} + for trial in [p[r] for p in result["pairs"] for r in compare.ROLES] + result["discovery"]: + if trial["verdict"] in ("PASS", "FAIL"): + # Recheck the supported grader using trusted code, not artifact verify.py. + checks = verify(tasks[trial["task_id"]], json.loads(trial["output"])) + if checks != trial["checks"] or checks["passed"] != (trial["verdict"] == "PASS"): + raise ValueError("Untrusted grading disagrees with the trusted grader") + results.append(result) + return results + + +def summary(plan, results, evidence_url, expires_at, now): + expired = now >= expires_at + lines = [MARKER, "## Skill evaluation", "", f"Plan SHA-256: `{fingerprint(plan)}`", + f"Base: `{plan['base']}` · Head: `{plan['head']}`", + f"Trusted implementation: `{plan['version']}`", "", + "Model/settings: `" + canonical(plan["model"]) + "`", + "Budget: `" + canonical(budget(plan)) + "`", "", + "Token usage stops new trials; it is not a hard provider token or monetary cap.", + "Cost remains unknown.", "", "| Skill | Comparison | Cases | Estimated calls |", + "| --- | --- | --- | --- |"] + for item in plan["comparisons"]: + if "plan_sha256" in item: + cases = ", ".join(c["id"] + " (" + c["split"] + ")" for c in item["cases"]) + lines.append(f"| {item['skill']} | {item['mode']} | {cases} | {item['calls']} |") + lines.append("\nFrozen exam hashes: `" + canonical(item["exam"]) + "`\n") + else: + lines.append(f"| {item['skill']} | unsupported snapshot | — | 0 |") + counts = {k: 0 for k in ("fix", "regression", "unchanged_correct", "unchanged_incorrect", + "inconclusive", "missing_pair")} + if expired: + lines += ["", "**EXPIRED: retained evidence is no longer verifiable.**"] + elif results: + for result in results: + for key in counts: + counts[key] += result["counts"][key] + lines += ["", "Outcomes: `" + canonical(counts) + "`", "", + "Discovery is reported separately in retained evidence."] + else: + counts["missing_pair"] = sum( + sum(c["split"] != "discovery" for c in item["cases"]) * plan["limits"]["repeats"] + for item in plan["comparisons"] if "plan_sha256" in item) + lines += ["", "**PLANNED: execution is not authorized by knowing this hash.**", + "Outcomes: `" + canonical(counts) + "`", + "Earlier revision results are stale for this plan."] + gaps = [c for c in plan["coverage"] if c["coverage"] != "covered"] + lines += ["", f"Uncovered/unsupported changes: **{len(gaps)}**. " + "Gaps never count as passes.", ""] + # Encode untrusted Git paths as data, so Markdown/mentions cannot escape into the comment. + for gap in gaps: + lines.append("- " + gap["coverage"] + ": " + + html.escape(canonical(gap["path"])).replace("@", "@") + "") + lines += ["", f"[Evidence and logs]({evidence_url}) · Retention: {RETENTION_DAYS} days.", + f"Outcomes are historical; verifiable only before Unix time `{int(expires_at)}` " + "while the linked artifact is available. Expired evidence is unverified.", + "", "Pipeline evidence only; offline fixed outputs do not establish skill efficacy."] + return "\n".join(lines) + + +class GitHub: + def __init__(self, repository, token): + if not re.fullmatch(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+", repository): + raise ValueError("Invalid repository") + self.repository, self.token = repository, token + + def request(self, method, path, data=None, *, binary=False): + url = "https://api.github.com/repos/" + self.repository + path + request = urllib.request.Request(url, method=method, headers={ + "Authorization": "Bearer " + self.token, "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", "Content-Type": "application/json"}, + data=None if data is None else canonical(data).encode()) + with urllib.request.build_opener(ArtifactRedirect()).open(request, timeout=60) as response: + raw = response.read(MAX_ARCHIVE + 1) + return raw if binary else json.loads(raw) + + def pages(self, path, key=None): + records, page = [], 1 + while True: + data = self.request("GET", path + ("&" if "?" in path else "?") + + f"per_page=100&page={page}") + batch = data[key] if key else data + records.extend(batch) + if len(batch) < 100: + return records + page += 1 + + def pr(self, number): + result = self.request("GET", f"/pulls/{number}") + if result["state"] != "open": + raise ValueError("PR must be open") + return {"base": result["base"]["sha"], "head": result["head"]["sha"]} + + def publish(self, plan, body, run_id): + current = self.pr(plan["pr"]) + if current != {"base": plan["base"], "head": plan["head"]}: + raise ValueError("Stale report cannot replace the current PR summary") + comments = self.pages(f"/issues/{plan['pr']}/comments") + matches = [c for c in comments if c["user"]["login"] == "github-actions[bot]" + and c["body"].startswith(MARKER)] + if len(matches) > 1: + raise ValueError("Multiple owned summaries; refusing an ambiguous update") + stamp = f"\n" + if matches: + previous = re.search(r"", matches[0]["body"]) + if previous and int(previous[1]) > run_id: + raise ValueError("Older workflow cannot replace a newer summary") + # A second live read covers head changes while fetching the comment pages. + if self.pr(plan["pr"]) != current: + raise ValueError("PR changed before publication") + if matches: + return self.request("PATCH", f"/issues/comments/{matches[0]['id']}", + {"body": body + stamp}) + return self.request("POST", f"/issues/{plan['pr']}/comments", {"body": body + stamp}) + + +class ArtifactRedirect(urllib.request.HTTPRedirectHandler): + def redirect_request(self, request, *args, **kwargs): + redirected = super().redirect_request(request, *args, **kwargs) + if redirected is not None: + redirected.remove_header("Authorization") + return redirected + + +def artifact_zip(api, run_id, name, now): + artifacts = api.pages(f"/actions/runs/{run_id}/artifacts", "artifacts") + selected = [a for a in artifacts if a["name"] == name] + if len(selected) != 1: + raise ValueError("Evidence artifact missing or ambiguous") + artifact = selected[0] + if artifact["size_in_bytes"] > MAX_ARCHIVE: + raise ValueError("Artifact too large") + from datetime import datetime + + expires = datetime.fromisoformat(artifact["expires_at"].replace("Z", "+00:00")).timestamp() + if artifact["expired"] or now >= expires: + return None, min(expires, now) + raw = api.request("GET", f"/actions/artifacts/{artifact['id']}/zip", binary=True) + with zipfile.ZipFile(io.BytesIO(raw)) as zipped: + if zipped.namelist() != ["results.zip"]: + raise ValueError("Unexpected artifact envelope") + member = zipped.infolist()[0] + if member.file_size > MAX_ARCHIVE: + raise ValueError("Oversized artifact payload") + return zipped.read(member), expires + + +def publish_retained(api, output, source_run, writer_run, now): + plan = compare.read_json(output / "pr-plan.json") + run = api.request("GET", f"/actions/runs/{source_run}") + if (run["path"] != WORKFLOW or run["event"] != "workflow_dispatch" + or run["display_title"] != run_title(plan["pr"], fingerprint(plan)) + or run["head_sha"] != plan["version"] + or run["status"] != "completed" and source_run != writer_run): + raise ValueError("Untrusted or mismatched execution workflow") + data, expires = artifact_zip(api, source_run, "skill-eval-results", now) + results = [] + if data is not None: + import_results(data, output) + results = reports(output) + url = f"https://github.com/{plan['repository']}/actions/runs/{source_run}" + return api.publish(plan, summary(plan, results, url, expires, now), writer_run) + + +def main(): + parser = argparse.ArgumentParser(description="Revision-bound skill PR integration") + parser.add_argument("mode", choices=("prepare", "authorize", "execute", "pack", "publish")) + parser.add_argument("--output", required=True, type=Path) + args = parser.parse_args() + output = args.output.resolve() + event = compare.read_json(Path(os.environ["GITHUB_EVENT_PATH"])) + inputs = event.get("inputs", {}) + pr = int(inputs.get("pr") or event["pull_request"]["number"]) + repository = os.environ["GITHUB_REPOSITORY"] + run_id = int(os.environ["GITHUB_RUN_ID"]) + if args.mode == "execute": + execute(output, compare.read_json(output / "approval.json")) + return + if args.mode == "pack": + result_archive(output) + return + api = GitHub(repository, os.environ["GITHUB_TOKEN"]) + if args.mode == "prepare": + current = api.pr(pr) + subprocess.run(["git", "fetch", "--no-tags", "--no-recurse-submodules", + "https://github.com/" + repository + ".git", current["base"], + f"refs/pull/{pr}/head"], cwd=ROOT, check=True) + version = git(ROOT, "rev-parse", "HEAD").decode().strip() + plan = prepare(ROOT, MAPPING, {"repository": repository, "pr": pr, + "version": version, **current}, output) + with Path(os.environ["GITHUB_OUTPUT"]).open("a", encoding="utf-8") as handle: + handle.write(f"pr={pr}\nplan_sha256={fingerprint(plan)}\n") + return + plan = compare.read_json(output / "pr-plan.json") + if args.mode == "authorize": + actor = os.environ["GITHUB_ACTOR"] + permission = api.request("GET", f"/collaborators/{actor}/permission")["permission"] + request = {key: inputs[key] for key in ("plan_sha256", "version", "base", "head")} + request["budget"] = json.loads(inputs["budget"]) + approval = authorize(plan, request, actor, permission, api.pr(pr)) + runs = api.pages("/actions/workflows/skill-eval-pr.yml/runs?event=workflow_dispatch", + "workflow_runs") + claim(plan, runs, run_id, int(os.environ["GITHUB_RUN_ATTEMPT"])) + write_json(output / "approval.json", approval) + return + if inputs.get("action") in ("execute", "publish"): + if inputs["action"] == "publish" and not inputs.get("source_run"): + raise ValueError("Publish-only retry requires the original execution run ID") + receipt = publish_retained(api, output, int(inputs.get("source_run") or run_id), + run_id, time.time()) + else: + now = time.time() + url = f"https://github.com/{repository}/actions/runs/{run_id}" + body = summary(plan, [], url, now + RETENTION_DAYS * 86400, now) + receipt = api.publish(plan, body, run_id) + print(receipt["html_url"]) + + +if __name__ == "__main__": + main() diff --git a/tools/skill_eval_pr_cases.json b/tools/skill_eval_pr_cases.json new file mode 100644 index 00000000..ea206ff0 --- /dev/null +++ b/tools/skill_eval_pr_cases.json @@ -0,0 +1,19 @@ +{ + "skill_root": "src/inferrsi/data/skill_library", + "model": {"name": "gpt-6.1-sol", "effort": "xhigh"}, + "limits": { + "repeats": 1, + "max_task_calls": 2, + "timeout_seconds": 180, + "campaign_seconds": 600, + "token_stop_new_trials": 20000 + }, + "skills": [ + { + "skill": "inference-benchmark-design", + "entrypoint": "methods/inference-benchmark-design/SKILL.md", + "paths": ["src/inferrsi/data/skill_library/methods/inference-benchmark-design"], + "cases": "../docs/examples/skill-comparison/cases.json" + } + ] +} From 6b93c54b82e9dbd51d48ea0729f8d59413c9cc70 Mon Sep 17 00:00:00 2001 From: Dong1017 <55723162+Dong1017@users.noreply.github.com> Date: Fri, 23 Oct 2026 03:30:09 -0700 Subject: [PATCH 2/8] fix: validate retained PR evidence and include offline summary example Reject malformed input-integrity flags and count unsupported snapshot changes as coverage gaps. Remove transient configuration paths from retained plans and include a fixed-output summary excerpt. Agent-assisted-by: Codex / unknown [effort=unknown] (implementation) Signed-off-by: GUOGUO --- .../examples/skill-eval-pr/summary-example.md | 38 +++++++++++++++++++ docs/skill-eval-pr.md | 2 + tests/test_skill_eval_pr.py | 7 ++++ tools/skill_eval_pr.py | 14 +++++++ 4 files changed, 61 insertions(+) create mode 100644 docs/examples/skill-eval-pr/summary-example.md diff --git a/docs/examples/skill-eval-pr/summary-example.md b/docs/examples/skill-eval-pr/summary-example.md new file mode 100644 index 00000000..26c9dc7d --- /dev/null +++ b/docs/examples/skill-eval-pr/summary-example.md @@ -0,0 +1,38 @@ +# Offline skill PR summary example + +Fixed-output pipeline control, not a model measurement or a skill-efficacy claim. +Revision/hash values below are placeholders for the temporary Git fixture. + +PR plan: `` + +Base: `` · Head: `` + +Comparison: **old/new** (`addition` uses **absent/new**) + +Client: **FixedOutputClient**, model label `offline-fixture`, effort `test` + +| Pair outcome | Count | +| --- | ---: | +| fix | 1 | +| regression | 1 | +| unchanged_correct | 1 | +| unchanged_incorrect | 1 | +| inconclusive | 1 | +| missing_pair | 1 | + +One unrelated source change remains **uncovered** and never counts as a pass. +The deliberately incorrect candidate is graded as a regression; malformed JSON +is inconclusive. The final pair is missing after the observed-token stop. + +Budget: 12 scheduled calls, one repeat, 60 seconds/trial, 900 seconds/comparison, +observed-token stop threshold 200. The fixed client records **10 calls**; no real +model calls occur. Monetary cost is unknown. Publication retry updates the same +single simulated comment with **zero additional evaluation calls**. + +The [offline integration control](../../../tests/test_skill_eval_pr.py) verifies +these outcomes, including addition/update, lost publication response, stale +heads and retained evidence validation. The +[demo command](../../../tests/fixtures/skill_eval_pr_demo.py) saves the complete +plan, results, regenerated reports and simulated summary for local inspection. +Simulated GitHub links are not live evidence. Real Actions evidence is requested +for 30 days; after expiry the comment alone cannot substantiate its outcomes. diff --git a/docs/skill-eval-pr.md b/docs/skill-eval-pr.md index 43aa5818..e2cd67b4 100644 --- a/docs/skill-eval-pr.md +++ b/docs/skill-eval-pr.md @@ -162,6 +162,8 @@ pytest Generate a complete offline example for either mode: +See the [offline summary excerpt](examples/skill-eval-pr/summary-example.md). + ```bash python -m tests.fixtures.skill_eval_pr_demo --mode addition --output python -m tests.fixtures.skill_eval_pr_demo --mode update --output diff --git a/tests/test_skill_eval_pr.py b/tests/test_skill_eval_pr.py index 8c800b49..1c23554d 100644 --- a/tests/test_skill_eval_pr.py +++ b/tests/test_skill_eval_pr.py @@ -62,6 +62,8 @@ def test_unsupported_file_mode_does_not_disappear_or_become_a_pass(tmp_path): plan = integration.prepare(repo, mapping, identity, tmp_path / "plan") assert plan["estimated_calls"] == 0 assert "Executable snapshot entries" in plan["comparisons"][0]["unsupported"] + assert all(c["coverage"] == "unsupported" for c in plan["coverage"] + if c["path"].startswith("library/selected/")) @pytest.mark.parametrize("field", ["plan_sha256", "version", "base", "head", "budget"]) @@ -219,6 +221,11 @@ def test_symlinks_and_forged_grades_are_rejected_without_executing_artifact_scri write_json(path.parent / "receipt.json", {"result_sha256": sha256(path)}) with pytest.raises(ValueError, match="trusted grader"): integration.reports(output) + record["inputs_unchanged"] = "False" + write_json(path, record) + write_json(path.parent / "receipt.json", {"result_sha256": sha256(path)}) + with pytest.raises(ValueError, match="must be a boolean"): + integration.reports(output) assert len(client.calls) == 2 diff --git a/tools/skill_eval_pr.py b/tools/skill_eval_pr.py index 68a0e819..c547f0b2 100644 --- a/tools/skill_eval_pr.py +++ b/tools/skill_eval_pr.py @@ -91,7 +91,13 @@ def prepare(repo, mapping_path, identity, output): except ValueError as error: # Unsupported snapshots remain visible; they are never task passes. comparisons.append({"skill": item["skill"], "unsupported": str(error)}) + for entry in coverage: + if any(entry["path"] == p or entry["path"].startswith(p.rstrip("/") + "/") + for p in item["paths"]): + entry["coverage"] = "unsupported" continue + finally: + config_path.unlink() plan = compare.load_plan(destination) comparisons.append({"skill": item["skill"], "mode": plan["mode"], "plan_sha256": digest, "cases": [ @@ -214,6 +220,13 @@ def reports(output): if "plan_sha256" not in item: continue directory = output / item["skill"] + for entry in compare.load_plan(directory)["schedule"]: + path = compare.trial_directory(directory, entry) / "result.json" + if path.is_file(): + record = compare.read_json(path) + if (not isinstance(record, dict) + or type(record.get("inputs_unchanged")) is not bool): + raise ValueError("Untrusted input-integrity flag must be a boolean") result = compare.report(directory) tasks = {t["id"]: t for t in compare.read_json(directory / "plan/tasks.json")} for trial in [p[r] for p in result["pairs"] for r in compare.ROLES] + result["discovery"]: @@ -231,6 +244,7 @@ def summary(plan, results, evidence_url, expires_at, now): lines = [MARKER, "## Skill evaluation", "", f"Plan SHA-256: `{fingerprint(plan)}`", f"Base: `{plan['base']}` · Head: `{plan['head']}`", f"Trusted implementation: `{plan['version']}`", "", + "Applicable only while these revisions and inputs match the current PR.", "", "Model/settings: `" + canonical(plan["model"]) + "`", "Budget: `" + canonical(budget(plan)) + "`", "", "Token usage stops new trials; it is not a hard provider token or monetary cap.", From 57dffe4c7ee0e0ab7b56ea9b993177f707fb0ad5 Mon Sep 17 00:00:00 2001 From: Dong1017 <55723162+Dong1017@users.noreply.github.com> Date: Fri, 23 Oct 2026 06:12:43 -0700 Subject: [PATCH 3/8] fix: require complete workflow history before skill evaluation Reject capped, inconsistent or overlapping execution history before emitting approval. Cover the omitted older execution through the authorization entry point. Agent-assisted-by: Codex / unknown [effort=unknown] (implementation) Signed-off-by: GUOGUO --- docs/skill-eval-pr.md | 6 ++ tests/fixtures/skill_eval_pr_support.py | 22 ++++++ tests/test_skill_eval_pr.py | 100 +++++++++++++++++++++++- tools/skill_eval_pr.py | 25 +++++- 4 files changed, 149 insertions(+), 4 deletions(-) diff --git a/docs/skill-eval-pr.md b/docs/skill-eval-pr.md index e2cd67b4..287046fa 100644 --- a/docs/skill-eval-pr.md +++ b/docs/skill-eval-pr.md @@ -104,6 +104,12 @@ The Actions run is the durable execution tombstone, not the expiring artifact. An interrupted or failed attempt is consumed even if it might have stopped before any paid work. There is no automatic paid retry/resume. Do not delete execution workflow runs; inaccessible/incomplete run history fails closed. +Authorization requires a stable integer `total_count`, exactly that many unique +run IDs and no overlapping pages. GitHub's filtered workflow-run searches have a +[1,000-result limit](https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28#list-workflow-runs-for-a-workflow). +Reaching that limit is ambiguous, so a reported total of 1,000 or more blocks +execution immediately. Missing/changing totals or an early short/empty page +also block execution; a short page alone never proves complete history. Deleting this coordination history removes duplicate-prevention evidence and requires human reconciliation before any further execution of the same plan. Workflow reruns may republish results but must not rerun evaluation. diff --git a/tests/fixtures/skill_eval_pr_support.py b/tests/fixtures/skill_eval_pr_support.py index 2c4fdf72..0578973d 100644 --- a/tests/fixtures/skill_eval_pr_support.py +++ b/tests/fixtures/skill_eval_pr_support.py @@ -1,6 +1,7 @@ import io import shutil import zipfile +from urllib.parse import parse_qs, urlsplit from inferrsi.infra.artifacts import write_json from tests.fixtures.skill_eval_compare_support import configuration, git @@ -93,3 +94,24 @@ def request(self, method, path, data=None, *, binary=False): raise ConnectionError("Offline lost response after server accepted publication") return {"html_url": "https://github.com/example/fixture/pull/7#issuecomment-1"} raise AssertionError((method, path)) + + +class SimulatedWorkflowHistory(integration.GitHub): + def __init__(self, plan, responses): + super().__init__(plan["repository"], "offline-not-a-credential") + self.plan, self.responses, self.pages_requested = plan, responses, [] + + def request(self, method, path, data=None, *, binary=False): + assert method == "GET" and not binary and data is None + if path.startswith("/collaborators/"): + return {"permission": "write"} + if path.startswith("/pulls/"): + return {"state": "open", **{key: {"sha": self.plan[key]} + for key in ("base", "head")}} + if path.startswith("/actions/workflows/skill-eval-pr.yml/runs?"): + query = parse_qs(urlsplit(path).query) + assert query["event"] == ["workflow_dispatch"] and query["per_page"] == ["100"] + page = int(query["page"][0]) + self.pages_requested.append(page) + return self.responses[page - 1] + raise AssertionError((method, path)) diff --git a/tests/test_skill_eval_pr.py b/tests/test_skill_eval_pr.py index 1c23554d..2398142f 100644 --- a/tests/test_skill_eval_pr.py +++ b/tests/test_skill_eval_pr.py @@ -1,6 +1,7 @@ import copy import io import stat +import sys import threading import urllib.request import zipfile @@ -11,7 +12,13 @@ from inferrsi.infra.artifacts import sha256, write_json from tests.fixtures.skill_eval_compare_support import ROOT, FixedOutputClient, git -from tests.fixtures.skill_eval_pr_support import SimulatedGitHub, approval, envelope, setup +from tests.fixtures.skill_eval_pr_support import ( + SimulatedGitHub, + SimulatedWorkflowHistory, + approval, + envelope, + setup, +) from tools import skill_eval_compare as compare from tools import skill_eval_pr as integration @@ -111,6 +118,97 @@ def test_dispatch_history_blocks_duplicate_failed_cancelled_and_rerun_attempts(t integration.claim(plan, [], 1, 1) +@pytest.fixture +def history_plan(): + return {"repository": "example/fixture", "pr": 7, "base": "a" * 40, "head": "b" * 40, + "version": "c" * 40, "estimated_calls": 2, + "limits": {"repeats": 1, "max_task_calls": 2, "timeout_seconds": 60, + "campaign_seconds": 900, "token_stop_new_trials": 20000}, + "comparisons": [{"plan_sha256": "d" * 64}]} + + +@pytest.mark.parametrize("reported_total", [1000, 1001]) +def test_filtered_history_cap_cannot_authorize_an_omitted_older_execution( + tmp_path, monkeypatch, history_plan, reported_total, +): + title = integration.run_title(7, integration.fingerprint(history_plan)) + runs = [{"id": i, "event": "workflow_dispatch", + "display_title": title if i in (1, 1001) else "another execution"} + for i in range(1001, 0, -1)] + visible = runs[:1000] + assert runs[-1]["id"] == 1 and runs[-1]["display_title"] == title + assert all(r["id"] != 1 for r in visible) + responses = [{"total_count": reported_total, "workflow_runs": visible[i:i + 100]} + for i in range(0, 1100, 100)] + api = SimulatedWorkflowHistory(history_plan, responses) + inputs = {key: history_plan[key] for key in ("version", "base", "head")} + inputs.update(pr="7", plan_sha256=integration.fingerprint(history_plan), + budget=integration.canonical(integration.budget(history_plan))) + write_json(tmp_path / "event.json", {"inputs": inputs}) + write_json(tmp_path / "pr-plan.json", history_plan) + for key, value in {"GITHUB_EVENT_PATH": str(tmp_path / "event.json"), + "GITHUB_REPOSITORY": "example/fixture", "GITHUB_RUN_ID": "1001", + "GITHUB_RUN_ATTEMPT": "1", "GITHUB_ACTOR": "fixture-operator", + "GITHUB_TOKEN": "offline-not-a-credential"}.items(): + monkeypatch.setenv(key, value) + monkeypatch.setattr(integration, "GitHub", lambda repository, token: api) + monkeypatch.setattr(sys, "argv", ["skill_eval_pr", "authorize", "--output", str(tmp_path)]) + with pytest.raises(ValueError, match="history"): + integration.main() + assert not (tmp_path / "approval.json").exists() + assert api.pages_requested == [1] + + +@pytest.mark.parametrize("total", [0, 100, 101, 999]) +def test_counted_workflow_history_accepts_complete_pages(history_plan, total): + runs = [{"id": i + 1} for i in range(total)] + responses = [{"total_count": total, "workflow_runs": runs[i:i + 100]} + for i in range(0, max(1, total), 100)] + api = SimulatedWorkflowHistory(history_plan, responses) + actual = api.pages("/actions/workflows/skill-eval-pr.yml/runs?event=workflow_dispatch", + "workflow_runs", require_complete=True) + assert actual == runs + assert api.pages_requested == list(range(1, len(responses) + 1)) + + +def test_complete_workflow_history_keeps_an_older_duplicate_in_the_claim(history_plan): + title = integration.run_title(7, integration.fingerprint(history_plan)) + runs = [{"id": i, "event": "workflow_dispatch", + "display_title": title if i in (1, 101) else "another execution"} + for i in range(101, 0, -1)] + api = SimulatedWorkflowHistory(history_plan, [ + {"total_count": 101, "workflow_runs": runs[:100]}, + {"total_count": 101, "workflow_runs": runs[100:]}, + ]) + actual = api.pages("/actions/workflows/skill-eval-pr.yml/runs?event=workflow_dispatch", + "workflow_runs", require_complete=True) + with pytest.raises(ValueError, match="Duplicate or uncertain"): + integration.claim(history_plan, actual, 101, 1) + assert integration.claim(history_plan, actual, 1, 1) == integration.execution_id(history_plan) + + +@pytest.mark.parametrize("responses", [ + [{"workflow_runs": []}], + [{"total_count": None, "workflow_runs": []}], + [{"total_count": True, "workflow_runs": [{"id": 1}]}], + [{"total_count": -1, "workflow_runs": []}], + [{"total_count": 2, "workflow_runs": [{"id": 1}]}], + [{"total_count": 0, "workflow_runs": [{"id": 1}]}], + [{"total_count": 101, "workflow_runs": [{"id": i} for i in range(100)]}, + {"total_count": 101, "workflow_runs": []}], + [{"total_count": 101, "workflow_runs": [{"id": i} for i in range(100)]}, + {"total_count": 102, "workflow_runs": [{"id": 100}]}], + [{"total_count": 101, "workflow_runs": [{"id": i} for i in range(100)]}, + {"total_count": 101, "workflow_runs": [{"id": 99}]}], +], ids=["missing-total", "null-total", "boolean-total", "negative-total", "short-page", + "extra-record", "empty-last-page", "changing-total", "overlapping-pages"]) +def test_workflow_history_rejects_incomplete_or_uncertain_pages(history_plan, responses): + api = SimulatedWorkflowHistory(history_plan, responses) + with pytest.raises(ValueError, match="history"): + api.pages("/actions/workflows/skill-eval-pr.yml/runs?event=workflow_dispatch", + "workflow_runs", require_complete=True) + + def test_concurrent_duplicate_calls_execute_only_one_paid_schedule(tmp_path): repo, mapping, identity = setup(tmp_path, ids=("one",)) output = tmp_path / "plan" diff --git a/tools/skill_eval_pr.py b/tools/skill_eval_pr.py index c547f0b2..a6481272 100644 --- a/tools/skill_eval_pr.py +++ b/tools/skill_eval_pr.py @@ -304,14 +304,33 @@ def request(self, method, path, data=None, *, binary=False): raw = response.read(MAX_ARCHIVE + 1) return raw if binary else json.loads(raw) - def pages(self, path, key=None): - records, page = [], 1 + def pages(self, path, key=None, *, require_complete=False): + records, page, total, seen = [], 1, None, set() while True: data = self.request("GET", path + ("&" if "?" in path else "?") + f"per_page=100&page={page}") batch = data[key] if key else data + if require_complete: + count = data.get("total_count") + # Filtered workflow-run searches stop at 1,000; that boundary is ambiguous. + if type(count) is not int or not 0 <= count < 1000: + raise ValueError( + "Workflow history total is unknown or reaches the search limit") + if total is not None and count != total: + raise ValueError("Workflow history changed during pagination") + total = count + identities = {record["id"] for record in batch} + if len(identities) != len(batch) or seen & identities: + raise ValueError("Workflow history pages contain duplicate records") + if len(records) + len(batch) > total: + raise ValueError("Workflow history exceeds its reported total") + seen.update(identities) records.extend(batch) + if require_complete and len(records) == total: + return records if len(batch) < 100: + if require_complete: + raise ValueError("Workflow history ended before its reported total") return records page += 1 @@ -429,7 +448,7 @@ def main(): request["budget"] = json.loads(inputs["budget"]) approval = authorize(plan, request, actor, permission, api.pr(pr)) runs = api.pages("/actions/workflows/skill-eval-pr.yml/runs?event=workflow_dispatch", - "workflow_runs") + "workflow_runs", require_complete=True) claim(plan, runs, run_id, int(os.environ["GITHUB_RUN_ATTEMPT"])) write_json(output / "approval.json", approval) return From 2ad3e4fdd89d1805a6717df0bcf3d549ce068d81 Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Sat, 24 Oct 2026 21:59:11 +0800 Subject: [PATCH 4/8] fix: restore comparison results after plan artifact download Recreate empty result directories in the PR adapter before invoking the unchanged executor. Cover addition and update through a file-only artifact round trip while retaining duplicate-attempt rejection. Agent-assisted-by: Codex / unknown [effort=unknown] (implementation) Signed-off-by: GUOGUO --- docs/skill-eval-pr.md | 4 ++++ tests/test_skill_eval_pr.py | 27 +++++++++++++++++++++++++++ tools/skill_eval_pr.py | 2 ++ 3 files changed, 33 insertions(+) diff --git a/docs/skill-eval-pr.md b/docs/skill-eval-pr.md index 287046fa..733f1165 100644 --- a/docs/skill-eval-pr.md +++ b/docs/skill-eval-pr.md @@ -90,6 +90,10 @@ The execute job has read-only repository permissions, no publication token in its model step, and checkout uses `persist-credentials: false`. The privileged publisher is a separate fresh job. Model access must not also grant PR publishing. The existing client's controlled skill exposure is not OS read confinement. +Plan artifacts contain files rather than empty directories. The adapter restores +each comparison's empty results directory after download; the existing executor +still rejects nonempty results, and the exclusive execution claim still rejects +duplicate attempts. ## Duplicate prevention and interrupted execution diff --git a/tests/test_skill_eval_pr.py b/tests/test_skill_eval_pr.py index 2398142f..38bb9e96 100644 --- a/tests/test_skill_eval_pr.py +++ b/tests/test_skill_eval_pr.py @@ -232,6 +232,33 @@ def trigger(): integration.execute(output, approved, client=client) +@pytest.mark.parametrize("mode", ["addition", "update"]) +def test_file_only_plan_artifact_round_trip_can_execute(tmp_path, mode): + repo, mapping, identity = setup(tmp_path, mode, ids=("one",)) + prepared = tmp_path / "prepared" + plan = integration.prepare(repo, mapping, identity, prepared) + write_json(prepared / "approval.json", approval(plan)) + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + for path in prepared.rglob("*"): + if path.is_file(): + archive.write(path, path.relative_to(prepared).as_posix()) + downloaded = tmp_path / "downloaded" + with zipfile.ZipFile(io.BytesIO(buffer.getvalue())) as archive: + assert all(not entry.is_dir() for entry in archive.infolist()) + archive.extractall(downloaded) + assert not (downloaded / "selected/results").exists() + assert compare.load_plan(downloaded / "selected") == compare.load_plan(prepared / "selected") + client = FixedOutputClient(tmp_path) + integration.execute(downloaded, compare.read_json(downloaded / "approval.json"), client=client) + status = compare.read_json(downloaded / "selected/results/status.json") + assert status["execution_state"] == "completed" and status["recorded_trials"] == 2 + assert len(client.calls) == 2 + with pytest.raises(FileExistsError): + integration.execute(downloaded, approval(plan), client=client) + assert len(client.calls) == 2 + + @pytest.mark.parametrize("mode", ["addition", "update"]) def test_retained_results_publish_once_retry_without_model_and_reject_stale_jobs(tmp_path, mode): repo, mapping, identity = setup(tmp_path, mode, token_stop_new_trials=200) diff --git a/tools/skill_eval_pr.py b/tools/skill_eval_pr.py index a6481272..5da6456c 100644 --- a/tools/skill_eval_pr.py +++ b/tools/skill_eval_pr.py @@ -162,6 +162,8 @@ def execute(output, approval, *, client=None): handle.write(canonical(approval)) for item in plan["comparisons"]: if "plan_sha256" in item: + # Artifact downloads omit empty directories required by the executor. + (output / item["skill"] / "results").mkdir(exist_ok=True) compare.execute(output / item["skill"], item["plan_sha256"], client=client) From 24b9c54a45a7acafe901881a447551b81c2c1cab Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Sat, 24 Oct 2026 22:38:30 +0800 Subject: [PATCH 5/8] feat: map the planned skill addition to the existing audit case Reserve the fixed-workload audit owner in the existing PR mapping. Verify the maintained nested paths select absent/new or old/new without model execution and keep canonical index changes uncovered. Agent-assisted-by: Codex / unknown [effort=unknown] (implementation) Signed-off-by: GUOGUO --- docs/skill-eval-pr.md | 8 ++++-- tests/test_skill_eval_pr.py | 48 ++++++++++++++++++++++++++++++++++ tools/skill_eval_pr_cases.json | 6 +++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/docs/skill-eval-pr.md b/docs/skill-eval-pr.md index 733f1165..d60f65df 100644 --- a/docs/skill-eval-pr.md +++ b/docs/skill-eval-pr.md @@ -26,8 +26,12 @@ Unmapped paths remain `uncovered`. Deletions and unsupported snapshot modes are explicitly `unsupported`; they cannot count as passes. Rename detection is off: a rename is a deletion plus an addition and retains the deletion gap. -The initial mapping covers only `inference-benchmark-design` and the existing -`audit-threshold` holdout case. Library navigation/index changes are uncovered; +The initial mapping associates `inference-benchmark-design` and the prospective +`audit-fixed-workload-claim` addition with the existing `audit-threshold` holdout +case. The latter reserves the path for the bounded fixed-workload audit skill +proposed for real addition acceptance; it does not add that skill or authorize +execution. Until a PR adds that path, it selects no comparison. Library +navigation/index changes are uncovered; coverage of one skill is not library-wide qualification. Changes to the mapping, cases, workflow or comparison implementation also trigger planning. diff --git a/tests/test_skill_eval_pr.py b/tests/test_skill_eval_pr.py index 38bb9e96..17971359 100644 --- a/tests/test_skill_eval_pr.py +++ b/tests/test_skill_eval_pr.py @@ -44,6 +44,54 @@ def test_plan_selects_addition_update_resources_without_execution(tmp_path, mode assert integration.execution_id(plan) == integration.execution_id(second) +@pytest.mark.parametrize("skill,mode", [ + ("audit-fixed-workload-claim", "addition"), + ("inference-benchmark-design", "update"), +]) +def test_maintained_mapping_selects_real_acceptance_paths(tmp_path, skill, mode): + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init", "--quiet") + (repo / "seed.txt").write_text("Before acceptance change", encoding="utf-8") + mapping = compare.read_json(integration.MAPPING) + owner = next(item for item in mapping["skills"] if item["skill"] == skill) + path = repo / mapping["skill_root"] / owner["entrypoint"] + index = repo / mapping["skill_root"] / "skill-index.json" + registration = {"schema_version": 1, "skills": { + skill: {"path": path.parent.relative_to(index.parent).as_posix(), "backend": "common"}}} + body = f"---\nname: {skill}\ndescription: Fixed-workload audit fixture\n---\n" + if mode == "update": + path.parent.mkdir(parents=True) + path.write_text(body + "Old audit guidance\n", encoding="utf-8") + write_json(index, registration) + git(repo, "add", mapping["skill_root"]) + git(repo, "add", "seed.txt") + git(repo, "commit", "--quiet", "-m", "Fixture acceptance baseline") + before = git(repo, "rev-parse", "HEAD") + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body + "Audit every request and retain late outliers\n", encoding="utf-8") + write_json(index, registration) + git(repo, "add", mapping["skill_root"]) + git(repo, "commit", "--quiet", "-m", "Fixture acceptance candidate") + head = git(repo, "rev-parse", "HEAD") + plan = integration.prepare(repo, integration.MAPPING, + {"repository": "example/fixture", "pr": 7, "base": before, + "head": head, "version": head}, tmp_path / "planned") + assert len(plan["comparisons"]) == 1 and plan["estimated_calls"] == 2 + selected = plan["comparisons"][0] + assert selected["skill"] == skill and selected["mode"] == mode + assert selected["cases"] == [{"id": "audit-threshold", "split": "holdout"}] + assert selected["exam"]["grader"] == sha256(ROOT / "tools/skill_eval_verifier.py") + assert {"status": "A" if mode == "addition" else "M", + "path": path.relative_to(repo).as_posix(), "coverage": "covered"} in plan["coverage"] + assert [change for change in plan["coverage"] if change["coverage"] != "covered"] == ( + [{"status": "A", "path": index.relative_to(repo).as_posix(), "coverage": "uncovered"}] + if mode == "addition" else []) + assert plan["model"] == {"name": "gpt-6.1-sol", "effort": "xhigh"} + assert integration.budget(plan)["comparisons"] == 1 + assert not (tmp_path / "planned/execution-claim.json").exists() + + def test_deleted_and_unmapped_skill_changes_are_visible_not_passed(tmp_path): repo, mapping, identity = setup(tmp_path) git(repo, "rm", "library/selected/resource.txt") diff --git a/tools/skill_eval_pr_cases.json b/tools/skill_eval_pr_cases.json index ea206ff0..9d7cbe6c 100644 --- a/tools/skill_eval_pr_cases.json +++ b/tools/skill_eval_pr_cases.json @@ -14,6 +14,12 @@ "entrypoint": "methods/inference-benchmark-design/SKILL.md", "paths": ["src/inferrsi/data/skill_library/methods/inference-benchmark-design"], "cases": "../docs/examples/skill-comparison/cases.json" + }, + { + "skill": "audit-fixed-workload-claim", + "entrypoint": "methods/audit-fixed-workload-claim/SKILL.md", + "paths": ["src/inferrsi/data/skill_library/methods/audit-fixed-workload-claim"], + "cases": "../docs/examples/skill-comparison/cases.json" } ] } From f1c422c14792a335f7c29275b71a403b2052ccc0 Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Sat, 24 Oct 2026 22:39:49 +0800 Subject: [PATCH 6/8] docs: clarify call budgets per selected comparison State the combined budget when both mapped owners change. Agent-assisted-by: Codex / unknown [effort=unknown] (documentation) Signed-off-by: GUOGUO --- docs/skill-eval-pr.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/skill-eval-pr.md b/docs/skill-eval-pr.md index d60f65df..adeeb6e3 100644 --- a/docs/skill-eval-pr.md +++ b/docs/skill-eval-pr.md @@ -71,11 +71,12 @@ are rechecked in the execution job immediately before starting the existing comparison command. No shell evaluates user-provided inputs. The selected model is **`gpt-6.1-sol`, effort `xhigh`**, requested by the owner. -The initial mapping schedules **2 task calls**: one absent/old and one new arm, +Each selected comparison schedules **2 task calls**: one absent/old and one new arm, one case, one repeat. Limits are 180 seconds per trial, 600 seconds per comparison, and an observed-token stop threshold of 20,000. This stops new trials; it is not a provider token or monetary hard cap. One trial can cross the threshold, and -unreported usage/cost remain unknown. Mapping more skills increases the total; +unreported usage/cost remain unknown. Selecting both mapped owners schedules four +calls. Mapping more skills increases the total; the exact total is shown in the plan and requires renewed authorization. ### Execution environment prerequisite From c716307df7c9dbd0674638ecea894bd912790882 Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Mon, 26 Oct 2026 11:19:27 +0800 Subject: [PATCH 7/8] Validate original result archive member names Check raw ZIP member names against the approved result paths before the platform normalizes them. Preserve unsafe spelling in the existing archive regression fixture so Windows exercises the same rejection as other hosts. Agent-assisted-by: Codex / unknown [effort=unknown] (implementation) Signed-off-by: GUOGUO --- tests/test_skill_eval_pr.py | 7 ++++++- tools/skill_eval_pr.py | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_skill_eval_pr.py b/tests/test_skill_eval_pr.py index 17971359..f2b4f29f 100644 --- a/tests/test_skill_eval_pr.py +++ b/tests/test_skill_eval_pr.py @@ -368,7 +368,12 @@ def test_untrusted_archive_paths_and_scripts_are_rejected(tmp_path, name): integration.prepare(repo, mapping, identity, output) buffer = io.BytesIO() with zipfile.ZipFile(buffer, "w") as archive: - archive.writestr(name, "Untrusted content must never execute") + member = zipfile.ZipInfo(name) + # Preserve unsafe archive spelling even when the host normalizes ZipInfo filenames. + member.filename = name + archive.writestr(member, "Untrusted content must never execute") + with zipfile.ZipFile(io.BytesIO(buffer.getvalue())) as archive: + assert archive.infolist()[0].orig_filename == name with pytest.raises(ValueError): integration.import_results(buffer.getvalue(), output) assert list((output / "selected/results").iterdir()) == [] diff --git a/tools/skill_eval_pr.py b/tools/skill_eval_pr.py index 5da6456c..82095a94 100644 --- a/tools/skill_eval_pr.py +++ b/tools/skill_eval_pr.py @@ -198,7 +198,8 @@ def import_results(data, output): allowed.update(prefix + trial + "/" + p for p in files) with zipfile.ZipFile(io.BytesIO(data)) as zipped: members = zipped.infolist() - names = [m.filename for m in members] + # Validate archive spelling before ZipInfo's platform-dependent normalization. + names = [m.orig_filename for m in members] if (len(names) != len(set(names)) or set(names) - allowed or sum(m.file_size for m in members) > MAX_ARCHIVE): raise ValueError("Unexpected, duplicate or oversized result archive entries") From 7952ae73476cfefadcb3f40a933090e7e51b7989 Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Mon, 26 Oct 2026 11:53:27 +0800 Subject: [PATCH 8/8] Preserve hidden resources in skill plan artifacts Include hidden files only in the prepared-plan upload so frozen skill resources survive the artifact round trip. Cover dotfiles and resources under dot directories in the existing addition and update controls. Agent-assisted-by: Codex / unknown [effort=unknown] (implementation) Signed-off-by: GUOGUO --- .github/workflows/skill-eval-pr.yml | 1 + tests/test_skill_eval_pr.py | 25 +++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/.github/workflows/skill-eval-pr.yml b/.github/workflows/skill-eval-pr.yml index 0852c433..e8221910 100644 --- a/.github/workflows/skill-eval-pr.yml +++ b/.github/workflows/skill-eval-pr.yml @@ -83,6 +83,7 @@ jobs: with: name: skill-eval-plan path: prepared + include-hidden-files: true retention-days: 30 execute: diff --git a/tests/test_skill_eval_pr.py b/tests/test_skill_eval_pr.py index f2b4f29f..0d8ecf4f 100644 --- a/tests/test_skill_eval_pr.py +++ b/tests/test_skill_eval_pr.py @@ -283,14 +283,35 @@ def trigger(): @pytest.mark.parametrize("mode", ["addition", "update"]) def test_file_only_plan_artifact_round_trip_can_execute(tmp_path, mode): repo, mapping, identity = setup(tmp_path, mode, ids=("one",)) + hidden_resources = { + ".audit-rules": "Retain every request\n", + ".audit/threshold.txt": "Reject a fixed-workload claim above the threshold\n", + } + for name, content in hidden_resources.items(): + resource = repo / "library/selected" / name + resource.parent.mkdir(parents=True, exist_ok=True) + resource.write_text(content, encoding="utf-8") + git(repo, "add", "library/selected") + git(repo, "commit", "--quiet", "-m", "Fixture hidden skill resources") + identity["head"] = git(repo, "rev-parse", "HEAD") prepared = tmp_path / "prepared" plan = integration.prepare(repo, mapping, identity, prepared) + for name, content in hidden_resources.items(): + frozen_resource = prepared / "selected/plan/bundles/candidate/selected" / name + assert frozen_resource.read_text(encoding="utf-8") == content write_json(prepared / "approval.json", approval(plan)) + workflow = yaml.safe_load((ROOT / integration.WORKFLOW).read_text(encoding="utf-8")) + upload = next(step["with"] for step in workflow["jobs"]["plan"]["steps"] + if step.get("uses") == "actions/upload-artifact@v4") + assert upload["path"] == prepared.name buffer = io.BytesIO() with zipfile.ZipFile(buffer, "w") as archive: for path in prepared.rglob("*"): - if path.is_file(): - archive.write(path, path.relative_to(prepared).as_posix()) + relative = path.relative_to(prepared) + # Match upload-artifact's documented dotfile and dot-directory filtering. + hidden = any(part.startswith(".") for part in relative.parts) + if path.is_file() and (upload.get("include-hidden-files", False) or not hidden): + archive.write(path, relative.as_posix()) downloaded = tmp_path / "downloaded" with zipfile.ZipFile(io.BytesIO(buffer.getvalue())) as archive: assert all(not entry.is_dir() for entry in archive.infolist())