From a7ae2c1c8ef1d056e57972420dc48a750b4d1c05 Mon Sep 17 00:00:00 2001 From: Ronnie Geraghty Date: Wed, 11 Nov 2026 11:23:30 -0800 Subject: [PATCH 01/14] Compare changed skills before and after with the native evals Action Require protected-environment approval before paid same-repository runs; hold eval measurements fixed while switching only the target skill source and reuse the Action's verified same-job tooling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/scripts/validation.test.mjs | 146 +++++++++++++++++++++++++- .github/workflows/evals.yml | 3 +- .github/workflows/skill-evals.yml | 152 ++++++++++++++++++++++++++++ README.md | 4 +- docs/checks-and-evals.md | 30 +++++- 5 files changed, 329 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/skill-evals.yml diff --git a/.github/scripts/validation.test.mjs b/.github/scripts/validation.test.mjs index ba64bdf..ea2e230 100644 --- a/.github/scripts/validation.test.mjs +++ b/.github/scripts/validation.test.mjs @@ -2,7 +2,8 @@ import test from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; -import { copyFileSync, existsSync, mkdirSync, readFileSync, readdirSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { parse } from "yaml"; @@ -361,7 +362,7 @@ test("schema validation skips absent local manifests without downloading or inst } }); -test("validation calls CI scripts, and paid eval activation is manual-only", () => { +test("validation stays model-free and the smoke workflow remains manual-only", () => { const validate = readFileSync(resolve(root, ".github/workflows/validate.yml"), "utf8"); assert.doesNotMatch(validate, /node -e|run: \|/); for (const script of ["validate-schema.sh", "check-local-catalog.mjs", "check-remote-pins.mjs"]) { @@ -391,6 +392,147 @@ test("validation calls CI scripts, and paid eval activation is manual-only", () }); }); + test("PR skill evals require approval and use two sequential native Action runs with fixed measurement inputs", () => { + const workflow = parse(readFileSync(resolve(root, ".github/workflows/skill-evals.yml"), "utf8")); + assert.deepEqual(Object.keys(workflow.on), ["pull_request"]); + assert.deepEqual(workflow.on.pull_request.paths, ["plugins/*/skills/**"]); + assert.equal(workflow.jobs.select.if, "github.event.pull_request.head.repo.full_name == github.repository"); + assert.deepEqual(workflow.permissions, { contents: "read" }); + assert.equal(workflow.jobs.select.permissions, undefined); + const job = workflow.jobs.evals; + assert.equal(job.environment, "evals"); + assert.equal(job.strategy["fail-fast"], false); + assert.deepEqual(job.permissions, { contents: "read", "copilot-requests": "write" }); + const actions = job.steps.filter((step) => step.uses?.startsWith("github/evals-action@")); + assert.equal(actions.length, 2); + assert.equal(actions[0].uses, actions[1].uses); + const { "report-id": beforeId, ...before } = actions[0].with; + const { "report-id": afterId, ...after } = actions[1].with; + assert.deepEqual(before, after); + assert.equal(before.command, "run"); + assert.equal(before["eval-paths"], "${{ matrix.evalPath }}"); + assert.equal(before["vally-version"], "0.19.0"); + assert.equal(before.runs, 1); + assert.equal(before.workers, 1); + assert.equal(before["max-retries"], 0); + assert.equal(before["upload-transcripts"], false); + assert.notEqual(beforeId, afterId); + assert.equal(actions[1].if, "${{ !cancelled() && steps.after_source.outcome == 'success' }}"); + assert.equal(job.steps.find((step) => step.id === "after_source").if, + "${{ !cancelled() && steps.before_source.outcome == 'success' }}"); + for (const step of job.steps.filter((step) => step.uses?.startsWith("actions/checkout@"))) { + assert.equal(step.with["persist-credentials"], false); + assert.equal(step.with["fetch-depth"], 2); + assert.equal(step.with.ref, undefined, "HEAD must remain the merge SHA accepted by the Action"); + } + }); + + test("PR selection and sequential source restores compare only eligible target skills and fail closed without approval", (t) => { + const directory = mkdtempSync(resolve(tmpdir(), "skill-eval-workflow-")); + t.after(() => rmSync(directory, { recursive: true, force: true })); + const repository = resolve(directory, "repository"); + const runner = resolve(directory, "runner"); + const hooks = resolve(directory, "hooks"); + for (const path of [repository, runner, hooks]) mkdirSync(path); + const config = resolve(directory, "gitconfig"); + writeFileSync(config, ""); + const env = { + ...process.env, GIT_CONFIG_GLOBAL: config, GIT_CONFIG_NOSYSTEM: "1", + RUNNER_TEMP: runner, GITHUB_OUTPUT: resolve(directory, "outputs"), + GITHUB_STEP_SUMMARY: resolve(directory, "summary.md").replaceAll("\\", "/"), + }; + const git = (...args) => { + const result = spawnSync("git", [ + "-c", "user.name=Fixture", "-c", "user.email=fixture@example.test", + "-c", "commit.gpgsign=false", "-c", `core.hooksPath=${hooks}`, ...args, + ], { cwd: repository, env, encoding: "utf8", timeout: 10000 }); + assert.ifError(result.error); + assert.equal(result.status, 0, result.stderr); + return result.stdout.trim(); + }; + const file = (path, content) => { + const destination = resolve(repository, path); + mkdirSync(resolve(destination, ".."), { recursive: true }); + writeFileSync(destination, content); + }; + const target = "plugins/example/skills/changed"; + git("init", "--initial-branch=main"); + file(`${target}/SKILL.md`, "base skill\n"); + file(`${target}/old.md`, "base-only file\n"); + file("plugins/example/skills/neighbor/SKILL.md", "base neighbor\n"); + file("plugins/example/skills/deleted/SKILL.md", "deleted skill\n"); + file("evals/example/skills/changed/eval.yaml", "fixed measurement\n"); + git("add", "."); git("commit", "-m", "Base fixture"); + git("checkout", "-b", "candidate"); + file(`${target}/SKILL.md`, "head skill\n"); + file(`${target}/new.md`, "head-only file\n"); + rmSync(resolve(repository, `${target}/old.md`)); + rmSync(resolve(repository, "plugins/example/skills/deleted/SKILL.md")); + file("plugins/example/skills/neighbor/SKILL.md", "head neighbor without an eval\n"); + file("plugins/example/skills/added/SKILL.md", "new skill without a baseline\n"); + git("add", "."); git("commit", "-m", "Candidate fixture"); + env.HEAD_SHA = git("rev-parse", "HEAD"); + git("checkout", "main"); + file(`${target}/main.md`, "current-main-only file\n"); + git("add", "."); git("commit", "-m", "Advance main fixture"); + const base = git("rev-parse", "HEAD"); + git("merge", "--no-ff", "candidate", "-m", "Merge fixture"); + env.GITHUB_SHA = git("rev-parse", "HEAD"); + const workflow = parse(readFileSync(resolve(root, ".github/workflows/skill-evals.yml"), "utf8")); + const selection = workflow.jobs.select.steps.find((step) => step.id === "select").run; + const code = selection.match(/<<'JS'\n([\s\S]*)\nJS/)[1]; + const environment = resolve(runner, "evals-environment.json"); + writeFileSync(environment, JSON.stringify({ protection_rules: [] })); + const denied = spawnSync(process.execPath, ["--input-type=module", "-e", code], { + cwd: repository, env, encoding: "utf8", timeout: 10000, + }); + assert.ifError(denied.error); + assert.notEqual(denied.status, 0); + assert.match(denied.stderr, /required maintainer reviewers/); + assert.equal(existsSync(env.GITHUB_OUTPUT), false); + writeFileSync(environment, JSON.stringify({ protection_rules: [{ type: "required_reviewers", reviewers: [{ type: "Team" }] }] })); + const selected = spawnSync(process.execPath, ["--input-type=module", "-e", code], { + cwd: repository, env, encoding: "utf8", timeout: 10000, + }); + assert.ifError(selected.error); + assert.equal(selected.status, 0, selected.stderr); + assert.match(selected.stdout, /No before\/after pair for example\/added/); + assert.match(selected.stdout, /No before\/after pair for example\/deleted/); + assert.match(selected.stdout, /No before\/after pair for example\/neighbor/); + const matrix = JSON.parse(readFileSync(env.GITHUB_OUTPUT, "utf8").split("\n")[0].slice("matrix=".length)); + assert.equal(matrix.include.length, 1); + const item = matrix.include[0]; + assert.equal(item.directory, target); + assert.equal(item.base, base); + assert.equal(item.head, env.HEAD_SHA); + assert.match(item.id, /^skill-[a-f0-9]{12}$/); + for (const [id, source, expected] of [ + ["before_source", base, "base skill\n"], ["after_source", env.HEAD_SHA, "head skill\n"], + ]) { + const step = workflow.jobs.evals.steps.find((entry) => entry.id === id); + const restored = spawnSync("bash", ["-c", step.run], { + cwd: repository, env: { ...env, SOURCE_SHA: source, SKILL_DIR: target }, encoding: "utf8", timeout: 10000, + }); + assert.ifError(restored.error); + assert.equal(restored.status, 0, restored.stderr); + assert.equal(git("rev-parse", "HEAD"), env.GITHUB_SHA); + assert.equal(readFileSync(resolve(repository, `${target}/SKILL.md`), "utf8"), expected); + assert.equal(existsSync(resolve(repository, `${target}/old.md`)), id === "before_source"); + assert.equal(existsSync(resolve(repository, `${target}/new.md`)), id === "after_source"); + assert.equal(existsSync(resolve(repository, `${target}/main.md`)), id === "before_source"); + assert.equal(readFileSync(resolve(repository, "plugins/example/skills/neighbor/SKILL.md"), "utf8"), "head neighbor without an eval\n"); + assert.equal(readFileSync(resolve(repository, "evals/example/skills/changed/eval.yaml"), "utf8"), "fixed measurement\n"); + assert.ok(git("diff", "--name-only", "HEAD").split("\n").every((path) => path.startsWith(`${target}/`))); + } + file("evals/example/skills/changed/eval.yaml", "unexpected measurement change\n"); + const drift = spawnSync("bash", ["-c", workflow.jobs.evals.steps.find((entry) => entry.id === "after_source").run], { + cwd: repository, env: { ...env, SOURCE_SHA: env.HEAD_SHA, SKILL_DIR: target }, encoding: "utf8", timeout: 10000, + }); + assert.ifError(drift.error); + assert.notEqual(drift.status, 0); + assert.match(drift.stdout, /Measurement files changed outside the selected skill/); + }); + for (const [plugin, skillName, readInstruction] of [ ["marketplace-maintainer", "update-remote-pins", "Read its `AGENTS.md` and `marketplace.json`, and this skill's [remote pin rules](references/remote-pins.md)."], ["plugin-authoring", "submit-plugin", "Read its `AGENTS.md` and `CONTRIBUTING.md`."], diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index 732ff84..79a65f1 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -1,6 +1,7 @@ name: Evals # Runs the smoke evals with real Copilot sessions, which cost AI credits. -# Start by explicit manual dispatch only; pull requests never start paid runs. +# This smoke workflow starts only by explicit manual dispatch. +# Skill comparisons use the separate approval-gated skill-evals workflow. on: workflow_dispatch: permissions: diff --git a/.github/workflows/skill-evals.yml b/.github/workflows/skill-evals.yml new file mode 100644 index 0000000..ad238eb --- /dev/null +++ b/.github/workflows/skill-evals.yml @@ -0,0 +1,152 @@ +name: Skill evals (before and after) +# Paid runs require approval in the existing, protected evals environment. +on: + pull_request: + paths: + - plugins/*/skills/** +permissions: + contents: read +jobs: + select: + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + matrix: ${{ steps.select.outputs.matrix }} + count: ${{ steps.select.outputs.count }} + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + fetch-depth: 2 + persist-credentials: false + - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 + with: + node-version: 24 + - name: Read approval protection without creating an environment + env: + GH_TOKEN: ${{ github.token }} + run: gh api "repos/$GITHUB_REPOSITORY/environments/evals" > "$RUNNER_TEMP/evals-environment.json" + - name: Require approval protection and select changed skills + id: select + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + node --input-type=module <<'JS' + import { execFileSync, spawnSync } from 'node:child_process'; + import { createHash } from 'node:crypto'; + import { appendFileSync, existsSync, readFileSync } from 'node:fs'; + import { resolve } from 'node:path'; + const environment = JSON.parse(readFileSync(resolve(process.env.RUNNER_TEMP, 'evals-environment.json'), 'utf8')); + if (!environment.protection_rules?.some(rule => rule.type === 'required_reviewers' && rule.reviewers?.length > 0)) { + throw new Error('Configure the evals environment with required maintainer reviewers before enabling paid PR evals.'); + } + const git = (...args) => execFileSync('git', args, { encoding: 'utf8' }).trim(); + const head = process.env.HEAD_SHA; + if (!/^[a-f0-9]{40}$/.test(head ?? '') || git('rev-parse', 'HEAD^2') !== head) { + throw new Error('Expected the PR merge commit and its exact head parent.'); + } + const base = git('rev-parse', 'HEAD^1'); + const changed = execFileSync('git', ['diff', '--no-renames', '--name-only', '-z', base, 'HEAD', '--', 'plugins'], { encoding: 'utf8' }); + const keys = [...new Set(changed.split('\0').flatMap(file => { + const match = /^plugins\/([^/]+)\/skills\/([^/]+)\//.exec(file); + return match ? [`${match[1]}/${match[2]}`] : []; + }))].sort(); + const include = []; + for (const key of keys) { + if (!/^[a-z0-9][a-z0-9._-]*\/[a-z0-9][a-z0-9._-]*$/.test(key)) { + throw new Error('Unsupported plugin or skill path in PR changes.'); + } + const [plugin, skill] = key.split('/'); + const directory = `plugins/${plugin}/skills/${skill}`; + const evalPath = `evals/${plugin}/skills/${skill}/eval.yaml`; + const present = ref => { + const result = spawnSync('git', ['cat-file', '-e', `${ref}:${directory}/SKILL.md`], { stdio: 'ignore' }); + if (result.error) throw result.error; + if (result.status !== 0 && result.status !== 128) throw new Error('Could not inspect the skill source tree.'); + return result.status === 0; + }; + if (!present(base) || !present(head) || !existsSync(evalPath)) { + console.log(`::warning::No before/after pair for ${key}: both skill versions and ${evalPath} are required. No eval was run for this skill.`); + continue; + } + include.push({ plugin, skill, directory, evalPath, base, head, id: `skill-${createHash('sha256').update(key).digest('hex').slice(0, 12)}` }); + } + appendFileSync(process.env.GITHUB_OUTPUT, `matrix=${JSON.stringify({ include })}\ncount=${include.length}\n`); + JS + + evals: + needs: select + if: needs.select.outputs.count != '0' + name: ${{ matrix.plugin }}/${{ matrix.skill }} + runs-on: ubuntu-latest + timeout-minutes: 60 + environment: evals + permissions: + contents: read + copilot-requests: write + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.select.outputs.matrix) }} + env: + VALLY_TELEMETRY_OPTOUT: '1' + SKILL_DIR: ${{ matrix.directory }} + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + fetch-depth: 2 + persist-credentials: false + - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 + with: + node-version: 24 + - name: Select the target skill from current main + id: before_source + env: + SOURCE_SHA: ${{ matrix.base }} + run: | + set -euo pipefail + git --literal-pathspecs restore --source="$SOURCE_SHA" --staged --worktree -- "$SKILL_DIR" + git --literal-pathspecs diff --exit-code "$SOURCE_SHA" -- "$SKILL_DIR" + if [[ -n "$(git diff --name-only HEAD | awk -v dir="$SKILL_DIR/" 'index($0,dir)!=1')$(git ls-files --others --exclude-standard)" ]]; then + echo "::error::Measurement files changed outside the selected skill." + exit 1 + fi + echo "Before: $SKILL_DIR from $SOURCE_SHA; measurement revision $GITHUB_SHA" >> "$GITHUB_STEP_SUMMARY" + - name: Run before evals + uses: github/evals-action@6e4c077e92194c7e75663d8d782426841047e887 # v0.2.0 + with: + command: run + eval-paths: ${{ matrix.evalPath }} + vally-version: '0.19.0' + copilot-token: ${{ github.token }} + runs: 1 + workers: 1 + max-retries: 0 + upload-transcripts: false + report-id: ${{ matrix.id }}-before + - name: Select the target skill from the PR head + id: after_source + if: ${{ !cancelled() && steps.before_source.outcome == 'success' }} + env: + SOURCE_SHA: ${{ matrix.head }} + run: | + set -euo pipefail + git --literal-pathspecs restore --source="$SOURCE_SHA" --staged --worktree -- "$SKILL_DIR" + git --literal-pathspecs diff --exit-code "$SOURCE_SHA" -- "$SKILL_DIR" + if [[ -n "$(git diff --name-only HEAD | awk -v dir="$SKILL_DIR/" 'index($0,dir)!=1')$(git ls-files --others --exclude-standard)" ]]; then + echo "::error::Measurement files changed outside the selected skill." + exit 1 + fi + echo "After: $SKILL_DIR from $SOURCE_SHA; measurement revision $GITHUB_SHA" >> "$GITHUB_STEP_SUMMARY" + - name: Run after evals even if the before verdict failed + if: ${{ !cancelled() && steps.after_source.outcome == 'success' }} + uses: github/evals-action@6e4c077e92194c7e75663d8d782426841047e887 # v0.2.0 + with: + command: run + eval-paths: ${{ matrix.evalPath }} + vally-version: '0.19.0' + copilot-token: ${{ github.token }} + runs: 1 + workers: 1 + max-retries: 0 + upload-transcripts: false + report-id: ${{ matrix.id }}-after diff --git a/README.md b/README.md index 3ca8f26..4a002ab 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,9 @@ request. **Paid evals:** `npm run eval` runs real Copilot sessions and costs AI credits. [Evals](.github/workflows/evals.yml) runs only when manually started; pull -requests never trigger paid evals. +requests that change a plugin skill use the separate +[before/after workflow](.github/workflows/skill-evals.yml), with maintainer approval +required in the protected `evals` environment. Fork PRs do not run paid evals. See [Checks and evals](docs/checks-and-evals.md) for CI details, local test commands, and Bash and PowerShell eval setup. diff --git a/docs/checks-and-evals.md b/docs/checks-and-evals.md index a4d6988..0c76df2 100644 --- a/docs/checks-and-evals.md +++ b/docs/checks-and-evals.md @@ -124,5 +124,31 @@ npm run eval:compare -- --baseline evals/_shared/fixtures/pr-description/baselin [Evals](../.github/workflows/evals.yml) runs the smoke suite only when you explicitly start it with **Run workflow** in GitHub Actions -(`workflow_dispatch`). Pull requests never start paid evals. Review the -selected branch and its eval code before dispatching a run. +(`workflow_dispatch`). Review the selected branch and its eval code before +dispatching a run. + +[Skill evals](../.github/workflows/skill-evals.yml) compares each changed existing +plugin skill with its version on current `main`. It uses the same PR merge +revision's evals, graders, fixtures and shared marketplace context for both runs; +only the selected skill comes from the exact base-parent or PR-head commit. +Both Action invocations run sequentially in one job, reusing the Action's verified +Vally installation. They use one trial per case and no trial retries. A failed +before verdict does not prevent the after run; failures remain visible, without +a new regression-only gate or statistical claim. + +A maintainer must first configure the `evals` environment with required +reviewers and review the PR's workflow, eval code and billing effects before +approval. The read-only selection job refuses missing or unprotected +environments, rather than automatically creating an unprotected environment. +Same-repository PRs are trusted code with model-billing permission, not an +OS/network sandbox; fork PRs are excluded and `pull_request_target` is not used. +The organization's Copilot billing policy must also permit workflow-token usage. +The workflow changes none of these repository or organization settings. + +New or deleted skills and skills without their corresponding +`evals//skills//eval.yaml` cannot form a before/after pair. They are +reported as not run, not counted as passing, and do not make evals mandatory for +every catalog skill. Each Action invocation retains its structured report; +raw transcript uploads and PR-comment permissions are disabled. The Action +manages its own runtime dependencies, so parity with the locally pinned Copilot +CLI is not assumed. From 8b9a7be247bc8eaa6aabf0201654a224bd97cbca Mon Sep 17 00:00:00 2001 From: Ronnie Geraghty Date: Thu, 12 Nov 2026 00:06:52 -0800 Subject: [PATCH 02/14] Scope comparison artifacts to the workflow run attempt Avoid upload collisions and importing prior-attempt verdicts when a maintainer reruns the same workflow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/scripts/validation.test.mjs | 5 +++++ .github/workflows/skill-evals.yml | 8 ++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/scripts/validation.test.mjs b/.github/scripts/validation.test.mjs index e2a751a..04d93c3 100644 --- a/.github/scripts/validation.test.mjs +++ b/.github/scripts/validation.test.mjs @@ -427,6 +427,11 @@ test("validation stays model-free and the smoke workflow remains manual-only", ( assert.equal(comment.concurrency["cancel-in-progress"], false); assert.match(comment.if, /!cancelled\(\)/); assert.ok(comment.steps.some((step) => step.run?.includes("skill-eval-comment.mjs post"))); + assert.equal(comment.steps.find((step) => step.uses?.startsWith("actions/download-artifact@")).with.pattern, + "skill-comparison-${{ github.run_attempt }}-*"); + for (const step of [...workflow.jobs.select.steps, ...job.steps].filter((step) => step.uses?.startsWith("actions/upload-artifact@"))) { + assert.match(step.with.name, /github\.run_attempt/, "reruns must not collide with or import earlier-attempt artifacts"); + } assert.equal(job.permissions["pull-requests"], undefined); for (const step of job.steps.filter((step) => step.uses?.startsWith("actions/checkout@"))) { assert.equal(step.with["persist-credentials"], false); diff --git a/.github/workflows/skill-evals.yml b/.github/workflows/skill-evals.yml index 9bb4234..fa9e213 100644 --- a/.github/workflows/skill-evals.yml +++ b/.github/workflows/skill-evals.yml @@ -88,7 +88,7 @@ jobs: - name: Retain comparison selection uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: - name: skill-comparison-selection + name: skill-comparison-${{ github.run_attempt }}-selection path: ${{ runner.temp }}/selection.json if-no-files-found: error retention-days: 7 @@ -120,7 +120,7 @@ jobs: - name: Read comparison selection uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5 with: - name: skill-comparison-selection + name: skill-comparison-${{ github.run_attempt }}-selection path: ${{ runner.temp }}/skill-selection - name: Select the target skill from current main id: before_source @@ -200,7 +200,7 @@ jobs: if: ${{ !cancelled() }} uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: - name: skill-comparison-${{ matrix.id }} + name: skill-comparison-${{ github.run_attempt }}-${{ matrix.id }} path: ${{ runner.temp }}/skill-comparison/*.json if-no-files-found: error retention-days: 7 @@ -227,7 +227,7 @@ jobs: - name: Read this run's comparison evidence only uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5 with: - pattern: skill-comparison-* + pattern: skill-comparison-${{ github.run_attempt }}-* merge-multiple: true path: ${{ runner.temp }}/skill-comparison - name: Update the PR comparison comment without model access From d03c07a3f53ef8ae46a5336d80ec9ab02d69ed3c Mon Sep 17 00:00:00 2001 From: Ronnie Geraghty Date: Thu, 12 Nov 2026 08:45:39 -0800 Subject: [PATCH 03/14] Keep incomplete comparisons unknown and reject unsafe workflow retries Require matching complete grader evidence for transitions, redact parse failures, cancel stale PR runs, and reject partial retries before either paid arm. Run the publisher from the trusted base and document the still-open billing trust decision without changing triggers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/scripts/skill-eval-comment.mjs | 37 +++++++++++++++------ .github/scripts/skill-eval-comment.test.mjs | 23 +++++++++++++ .github/scripts/validation.test.mjs | 19 +++++++++++ .github/workflows/skill-evals.yml | 29 +++++++++++++++- docs/checks-and-evals.md | 23 +++++++++++++ 5 files changed, 120 insertions(+), 11 deletions(-) diff --git a/.github/scripts/skill-eval-comment.mjs b/.github/scripts/skill-eval-comment.mjs index 9b0fb2c..dc8bc4f 100644 --- a/.github/scripts/skill-eval-comment.mjs +++ b/.github/scripts/skill-eval-comment.mjs @@ -13,7 +13,10 @@ const text = (value) => stripVTControlCharacters(String(value ?? "")) .replaceAll("&", "&").replaceAll("<", "<").replaceAll(">", ">") .replaceAll("|", "|").replaceAll("`", "`").replaceAll("[", "[").replaceAll("]", "]") .replaceAll("@", "@").slice(0, 200); -const json = (path) => JSON.parse(readFileSync(path, "utf8")); +const json = (path) => { + const contents = readFileSync(path, "utf8"); + try { return JSON.parse(contents); } catch { throw new Error("Malformed comparison JSON input."); } +}; const writeJson = (path, value) => { mkdirSync(dirname(path), { recursive: true }); writeFileSync(path, `${JSON.stringify(value)}\n`, { mode: 0o600 }); @@ -116,7 +119,9 @@ export function collectArm(report, logs) { const matches = []; for (const log of logs) { if (!log.trimStart().startsWith("{")) continue; // The Action also retains its plain version check. - const records = log.split(/\r?\n/).filter((line) => line.trim()).map((line) => JSON.parse(line)); + let records; + try { records = log.split(/\r?\n/).filter((line) => line.trim()).map((line) => JSON.parse(line)); } + catch { throw new Error("Malformed native JSONL evidence."); } if (records[0]?.runId === report.nativeRunId) matches.push(records); } if (matches.length !== 1) throw new Error("Expected exactly one native JSONL log for this Action run."); @@ -131,6 +136,12 @@ const findRow = (arm, row) => arm?.rows.find((item) => item.eval === row.eval && item.stimulus === row.stimulus && item.model === row.model); const failures = (row) => new Set(row?.checks.filter((check) => !check.passed).map((check) => check.id) ?? []); const difference = (a, b) => [...a].filter((id) => !b.has(id)).length; +function comparable(pair, before, after) { + return pair.before?.state === "complete" && pair.after?.state === "complete" && + before?.state === "graded" && after?.state === "graded" && + before.checks.length === after.checks.length && + before.checks.every((check) => after.checks.some((other) => other.id === check.id)); +} function counts(arm, row) { if (arm?.state === "error") return "⚠️ Error"; if (arm?.state === "not-run") return "⏭️ Not run"; @@ -139,9 +150,9 @@ function counts(arm, row) { if (row.state === "not-run") return "⏭️ Not run"; return `${row.checks.filter((check) => check.passed).length}/${row.checks.length}`; } -function transition(before, after) { +function transition(pair, before, after) { const old = failures(before), current = failures(after); - if (before?.state !== "graded" || after?.state !== "graded") return ""; + if (!comparable(pair, before, after)) return ""; const added = difference(current, old), resolved = difference(old, current); if (added && resolved) return `🔄 `; if (added) return "📉 "; @@ -155,7 +166,7 @@ export function renderComparison(selection, pairs, evidenceUrl) { let added = 0, resolved = 0; for (const pair of pairs) for (const after of pair.after?.rows ?? []) { const before = findRow(pair.before, after); - if (before?.state === "graded" && after.state === "graded") { + if (comparable(pair, before, after)) { added += difference(failures(after), failures(before)); resolved += difference(failures(before), failures(after)); } @@ -168,7 +179,8 @@ export function renderComparison(selection, pairs, evidenceUrl) { "**Cells: Before → After.** A scenario is one case; graders are its individual checks.", "", ]; if (pairs.some((pair) => ["before", "after"].some((arm) => pair[arm]?.state !== "complete" || - pair[arm]?.rows.some((row) => row.state !== "graded")))) { + pair[arm]?.rows.some((row) => row.state !== "graded")) || + (pair.after?.rows ?? []).some((after) => !comparable(pair, findRow(pair.before, after), after)))) { lines.push("**⚠️ Incomplete/error evidence:** affected cells are not passing results. Check the linked workflow.", ""); } const rowIdentities = (pair) => [...new Map([...(pair.before?.rows ?? []), ...(pair.after?.rows ?? [])] @@ -187,7 +199,9 @@ export function renderComparison(selection, pairs, evidenceUrl) { const cells = models.map((model) => { const identity = { ...row, model }; const before = findRow(pair.before, identity), after = findRow(pair.after, identity); - return `${transition(before, after)}${counts(pair.before, before)} → ${counts(pair.after, after)}`; + const unknown = before?.state === "graded" && after?.state === "graded" && !comparable(pair, before, after) + ? " (comparison unknown)" : ""; + return `${transition(pair, before, after)}${counts(pair.before, before)} → ${counts(pair.after, after)}${unknown}`; }); const short = row.stimulus.startsWith(`${row.eval}-`) ? row.stimulus.slice(row.eval.length + 1) : row.stimulus; lines.push(`| ${row.eval} : ${short} | ${cells.join(" | ")} |`); @@ -211,7 +225,7 @@ export function renderComparison(selection, pairs, evidenceUrl) { for (const { arm, row, check } of rows) { const other = findRow(pair[arm === "before" ? "after" : "before"], row); const otherFailed = failures(other).has(check.id); - const change = other?.state !== "graded" ? "comparison unknown" : + const change = !comparable(pair, arm === "before" ? row : other, arm === "after" ? row : other) ? "comparison unknown" : arm === "after" ? otherFailed ? "already present" : "new" : otherFailed ? "persists After" : "resolved After"; output.push(`| ${arm === "after" ? "After" : "Before"} (${change}) | ${check.name} | ${row.model} | ${check.reason} | [Native run](${evidenceUrl}) |`); } @@ -244,12 +258,15 @@ export function renderComparison(selection, pairs, evidenceUrl) { let added = 0, resolved = 0; for (const after of pair.after?.rows.filter((row) => row.model === model) ?? []) { const before = findRow(pair.before, after); - if (before?.state === "graded" && after.state === "graded") { + if (comparable(pair, before, after)) { added += difference(failures(after), failures(before)); resolved += difference(failures(before), failures(after)); } } - return added && resolved ? `🔄 ${values} (${added} new, ${resolved} resolved grader failures)` : values; + const unknown = (pair.after?.rows.filter((row) => row.model === model) ?? []) + .some((after) => !comparable(pair, findRow(pair.before, after), after)); + return unknown ? `${values} (comparison unknown)` : + added && resolved ? `🔄 ${values} (${added} new, ${resolved} resolved grader failures)` : values; }); lines.push(`| ${text(skill.key)} | ${cells.join(" | ")} |`); } diff --git a/.github/scripts/skill-eval-comment.test.mjs b/.github/scripts/skill-eval-comment.test.mjs index 248309b..11727e0 100644 --- a/.github/scripts/skill-eval-comment.test.mjs +++ b/.github/scripts/skill-eval-comment.test.mjs @@ -81,6 +81,29 @@ test("equal aggregate counts expose changed failure identities and missing arms assert.doesNotMatch(missing, /📈 1\/2/); }); +test("partial arms and absent matching graders cannot create resolved or new failure claims", () => { + for (const change of ["partial", "missing-check"]) { + const sample = pair(); + if (change === "partial") sample.after.state = "error"; + else sample.after.rows[0].checks.pop(); + const rendered = renderComparison(selection, [sample], "https://example.test"); + assert.match(rendered, /0 resolved · 📉 0 new failures/); + assert.match(rendered, /Before \(comparison unknown\) \| routing/); + assert.match(rendered, /comparison unknown/); + assert.doesNotMatch(rendered, /resolved After|After \(new\)|📈 1\/2/); + if (change === "partial") assert.match(rendered, /1\/2 → ⚠️ Error/); + else assert.match(rendered, /1\/2 → 1\/1 \(comparison unknown\)/); + } +}); + +test("malformed native output never leaks parser excerpts", () => { + assert.throws(() => collectArm(report, [`${JSON.stringify(trial())}\nSECRET_RAW_INVALID_JSON`]), (error) => { + assert.equal(error.message, "Malformed native JSONL evidence."); + assert.doesNotMatch(error.message, /SECRET/); + return true; + }); +}); + test("catalog inventory enumerates local and exact pinned remote skill folders without executing them", (t) => { const root = mkdtempSync(join(tmpdir(), "comparison-inventory-")); t.after(() => rmSync(root, { recursive: true, force: true })); diff --git a/.github/scripts/validation.test.mjs b/.github/scripts/validation.test.mjs index 04d93c3..5cbc8a4 100644 --- a/.github/scripts/validation.test.mjs +++ b/.github/scripts/validation.test.mjs @@ -398,6 +398,9 @@ test("validation stays model-free and the smoke workflow remains manual-only", ( assert.deepEqual(workflow.on.pull_request.paths, ["plugins/*/skills/**"]); assert.equal(workflow.jobs.select.if, "github.event.pull_request.head.repo.full_name == github.repository"); assert.deepEqual(workflow.permissions, { contents: "read" }); + assert.deepEqual(workflow.concurrency, { + group: "skill-evals-${{ github.event.pull_request.number }}", "cancel-in-progress": true, + }); assert.deepEqual(workflow.jobs.select.permissions, { contents: "read", actions: "read" }); const job = workflow.jobs.evals; assert.equal(job.environment, "evals"); @@ -433,6 +436,22 @@ test("validation stays model-free and the smoke workflow remains manual-only", ( assert.match(step.with.name, /github\.run_attempt/, "reruns must not collide with or import earlier-attempt artifacts"); } assert.equal(job.permissions["pull-requests"], undefined); + assert.equal(comment.steps.find((step) => step.uses?.startsWith("actions/checkout@")).with.ref, + "${{ github.event.pull_request.base.sha }}"); + assert.match(comment.steps.find((step) => step.name?.includes("trusted base")).run, + /Merge the reviewed publisher into the base branch/); + for (const guarded of [job, comment]) { + assert.equal(guarded.steps[0].env.SELECTED_ATTEMPT, "${{ needs.select.outputs.attempt }}"); + for (const attempt of ["1", "2"]) { + const checked = spawnSync("bash", ["-c", guarded.steps[0].run], { + encoding: "utf8", timeout: 10000, + env: { ...process.env, SELECTED_ATTEMPT: "1", GITHUB_RUN_ATTEMPT: attempt }, + }); + assert.ifError(checked.error); + assert.equal(checked.status, attempt === "1" ? 0 : 1); + if (attempt === "2") assert.match(checked.stdout, /Re-run all jobs/); + } + } for (const step of job.steps.filter((step) => step.uses?.startsWith("actions/checkout@"))) { assert.equal(step.with["persist-credentials"], false); assert.equal(step.with["fetch-depth"], 2); diff --git a/.github/workflows/skill-evals.yml b/.github/workflows/skill-evals.yml index fa9e213..e0808a8 100644 --- a/.github/workflows/skill-evals.yml +++ b/.github/workflows/skill-evals.yml @@ -6,6 +6,9 @@ on: - plugins/*/skills/** permissions: contents: read +concurrency: + group: skill-evals-${{ github.event.pull_request.number }} + cancel-in-progress: true jobs: select: if: github.event.pull_request.head.repo.full_name == github.repository @@ -17,6 +20,7 @@ jobs: outputs: matrix: ${{ steps.select.outputs.matrix }} count: ${{ steps.select.outputs.count }} + attempt: ${{ steps.select.outputs.attempt }} steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: @@ -83,7 +87,7 @@ jobs: run: process.env.GITHUB_RUN_ID, attempt: process.env.GITHUB_RUN_ATTEMPT, }; writeFileSync(resolve(process.env.RUNNER_TEMP, 'selection.json'), JSON.stringify(selection)); - appendFileSync(process.env.GITHUB_OUTPUT, `matrix=${JSON.stringify({ include })}\ncount=${include.length}\n`); + appendFileSync(process.env.GITHUB_OUTPUT, `matrix=${JSON.stringify({ include })}\ncount=${include.length}\nattempt=${process.env.GITHUB_RUN_ATTEMPT}\n`); JS - name: Retain comparison selection uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 @@ -110,6 +114,14 @@ jobs: VALLY_TELEMETRY_OPTOUT: '1' SKILL_DIR: ${{ matrix.directory }} steps: + - name: Reject partial retries before either paid arm + env: + SELECTED_ATTEMPT: ${{ needs.select.outputs.attempt }} + run: | + if [[ "$SELECTED_ATTEMPT" != "$GITHUB_RUN_ATTEMPT" ]]; then + echo "::error::Partial job retries are unsupported. Use Re-run all jobs so selection and both arms share one attempt." + exit 1 + fi - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 2 @@ -218,12 +230,27 @@ jobs: actions: read pull-requests: write steps: + - name: Reject stale selection on comment-only retries + env: + SELECTED_ATTEMPT: ${{ needs.select.outputs.attempt }} + run: | + if [[ "$SELECTED_ATTEMPT" != "$GITHUB_RUN_ATTEMPT" ]]; then + echo "::error::Partial job retries are unsupported. Use Re-run all jobs so selection and both arms share one attempt." + exit 1 + fi - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: + ref: ${{ github.event.pull_request.base.sha }} persist-credentials: false - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 with: node-version: 24 + - name: Require the publisher on the trusted base revision + run: | + test -f .github/scripts/skill-eval-comment.mjs || { + echo "::error::Merge the reviewed publisher into the base branch before enabling comparison comments." + exit 1 + } - name: Read this run's comparison evidence only uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5 with: diff --git a/docs/checks-and-evals.md b/docs/checks-and-evals.md index dd6bdd6..d729f0a 100644 --- a/docs/checks-and-evals.md +++ b/docs/checks-and-evals.md @@ -178,6 +178,9 @@ graders fail. New, resolved and persistent failures are distinguished, including equal totals with changed failure identities. Errors, skipped cases and missing arms never become passing counts. These single-trial observations do not prove causation. +Transition labels require two complete arms and the same individual checks. +A missing counterpart is unknown, not a resolved failure. Partial failed-check +evidence stays visible without claiming an improvement or regression. The small presentation helper reads the pinned Action's local native JSONL logs and uploads only case/model identities, verdicts and safe check explanations. @@ -194,3 +197,23 @@ serialized publishing and run/attempt ordering prevent older runs replacing newer comments. No custom Run button, new trigger or branch-protection change is introduced. The layout supports multiple observed models without selecting or paying for additional models automatically. + +The publisher checks out the exact base SHA, never the PR's helper. Its reviewed +helper must already exist on that trusted revision; otherwise publication fails +closed. Workflow-level per-PR concurrency cancels stale runs before more work is +scheduled. Cancellation cannot undo already billed model requests. + +**Partial job reruns are unsupported.** Use **Re-run all jobs** so selection and +both paid arms share an attempt and approval. A retry reusing a successful +selection from a previous attempt is rejected before either paid arm, rather +than spending credits and then failing to collect its artifacts. This also +applies to comment-only retries. + +**Approval boundary:** required environment reviewers protect jobs that reference +that environment, not every job an authorized workflow editor could add. +`copilot-requests: write` is a workflow-token permission, not an environment +secret. These declarations are not an immutable organization-wide billing +policy. A trusted publishing checkout also does not make the surrounding +PR-editable workflow immutable. Do not treat this workflow as a security boundary +against untrusted workflow editors; the trusted-entrypoint/billing design remains +under review before enabling it. From 2c47a1983bbfa6b348de80f738ba4ea8b53211b3 Mon Sep 17 00:00:00 2001 From: Ronnie Geraghty Date: Thu, 12 Nov 2026 09:30:10 -0800 Subject: [PATCH 04/14] Publish PR comparison data from a trusted workflow_run workflow Remove write permissions from PR publication code; validate the authenticated producer workflow, run, attempt, PR and merge revisions, and treat bounded artifact JSON as untrusted display data. Preserve the existing native paid entrypoint and document its separate approval-boundary gap. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/scripts/skill-eval-comment.mjs | 153 ++++++++++++++++---- .github/scripts/skill-eval-comment.test.mjs | 84 +++++++++-- .github/scripts/validation.test.mjs | 25 ++-- .github/workflows/skill-eval-comment.yml | 38 +++++ .github/workflows/skill-evals.yml | 46 ------ README.md | 3 + docs/checks-and-evals.md | 64 +++++--- 7 files changed, 302 insertions(+), 111 deletions(-) create mode 100644 .github/workflows/skill-eval-comment.yml diff --git a/.github/scripts/skill-eval-comment.mjs b/.github/scripts/skill-eval-comment.mjs index dc8bc4f..2dcefd8 100644 --- a/.github/scripts/skill-eval-comment.mjs +++ b/.github/scripts/skill-eval-comment.mjs @@ -1,12 +1,16 @@ import { execFileSync } from "node:child_process"; -import { existsSync, mkdirSync, readFileSync, readdirSync, realpathSync, statSync, writeFileSync } from "node:fs"; -import { dirname, resolve } from "node:path"; +import { existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, realpathSync, statSync, writeFileSync } from "node:fs"; +import { dirname, isAbsolute, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; import { stripVTControlCharacters } from "node:util"; const marker = "`; + const key = `${marker}${context.repository}/${sourceWorkflow} -->`; const comments = []; for (let page = 1; ; page++) { - const batch = await api(`${endpoint}/issues/${env.PR_NUMBER}/comments?per_page=100&page=${page}`); + const batch = await api(`${endpoint}/issues/${context.pullNumber}/comments?per_page=100&page=${page}`); comments.push(...batch); if (batch.length < 100) break; } const existing = comments.find((comment) => comment.user?.login === "github-actions[bot]" && comment.body.startsWith(key)); - const order = [Number(env.GITHUB_RUN_NUMBER), Number(env.GITHUB_RUN_ATTEMPT)]; + const order = [context.runNumber, Number(context.attempt)]; if (!order.every((n) => Number.isSafeInteger(n) && n > 0)) throw new Error("Invalid workflow run order."); const previous = existing?.body.match(//); if (previous && (Number(previous[1]) > order[0] || Number(previous[1]) === order[0] && Number(previous[2]) > order[1])) return "stale"; const content = `${key}\n\n${body}`; if (content.length > 60000) throw new Error("Comparison exceeds the comment limit; no misleading truncated report published."); - return api(existing ? `${endpoint}/issues/comments/${existing.id}` : `${endpoint}/issues/${env.PR_NUMBER}/comments`, + return api(existing ? `${endpoint}/issues/comments/${existing.id}` : `${endpoint}/issues/${context.pullNumber}/comments`, existing ? "PATCH" : "POST", { body: content }); } @@ -332,23 +444,10 @@ async function main() { data: collectArm(report, logs), }); } else if (command === "post") { - const selection = json(resolve(path, "selection.json")); - validateIdentity(selection, process.env); - const pairs = selection.include.map((entry) => { - const pair = { key: `${entry.plugin}/${entry.skill}` }; - for (const arm of ["before", "after"]) { - const file = resolve(path, `${entry.id}-${arm}.json`); - if (!existsSync(file)) { pair[arm] = { state: "error", rows: [] }; continue; } - const saved = json(file); - if (saved.key !== pair.key || saved.id !== entry.id || saved.arm !== arm || - saved.repository !== selection.repository || saved.run !== selection.run || - saved.attempt !== selection.attempt || saved.merge !== selection.merge) throw new Error("Mixed comparison artifacts."); - pair[arm] = saved.data; - } - return pair; - }); + const context = await publisherContext(json(process.env.GITHUB_EVENT_PATH)); + const { selection, pairs } = await loadComparison(resolve(path), context); const evidence = `${process.env.GITHUB_SERVER_URL}/${selection.repository}/actions/runs/${selection.run}`; - console.log(JSON.stringify(await postComparison(selection, renderComparison(selection, pairs, evidence)))); + console.log(JSON.stringify(await postComparison(selection, renderComparison(selection, pairs, evidence), github, context))); } else throw new Error("Expected collect or post."); } diff --git a/.github/scripts/skill-eval-comment.test.mjs b/.github/scripts/skill-eval-comment.test.mjs index 11727e0..34bf43d 100644 --- a/.github/scripts/skill-eval-comment.test.mjs +++ b/.github/scripts/skill-eval-comment.test.mjs @@ -5,7 +5,7 @@ import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; -import { collectArm, inventorySkills, postComparison, projectTrials, renderComparison } from "./skill-eval-comment.mjs"; +import { collectArm, inventorySkills, loadComparison, postComparison, projectTrials, publisherContext, renderComparison, validateSelection } from "./skill-eval-comment.mjs"; const trial = (extra = {}) => ({ type: "trial-result", runId: "native", evalName: "draft", stimulus: "draft-positive", model: "model-a", variant: "main", @@ -128,10 +128,7 @@ test("catalog inventory enumerates local and exact pinned remote skill folders w }); test("comment publishing checks current PR head, bot ownership, run order and updates only one comment", async () => { - const env = { - GITHUB_REPOSITORY: "example/repo", PR_NUMBER: "2", GITHUB_WORKFLOW_REF: "example/repo/.github/workflows/skill-evals.yml@refs/pull/2/merge", - GITHUB_RUN_NUMBER: "5", GITHUB_RUN_ATTEMPT: "1", - }; + const context = { repository: "example/repo", pullNumber: 2, runNumber: 5, attempt: "1" }; const calls = []; let head = selection.head, comments = []; const api = async (endpoint, method = "GET", body) => { @@ -140,17 +137,86 @@ test("comment publishing checks current PR head, bot ownership, run order and up if (method === "GET") return comments; return { id: 8 }; }; - await postComparison(selection, "Report", api, env); + await postComparison(selection, "Report", api, context); assert.equal(calls.at(-1).method, "POST"); comments = [{ id: 8, user: { login: "github-actions[bot]" }, body: calls.at(-1).body.body }]; calls.length = 0; - await postComparison(selection, "Updated", api, env); + await postComparison(selection, "Updated", api, context); assert.equal(calls.at(-1).method, "PATCH"); assert.match(calls.at(-1).endpoint, /issues\/comments\/8$/); comments[0].body = comments[0].body.replace("run:5:1", "run:6:1"); - assert.equal(await postComparison(selection, "Older", api, env), "stale"); + assert.equal(await postComparison(selection, "Older", api, context), "stale"); head = "c".repeat(40); - await assert.rejects(postComparison(selection, "Wrong head", api, env), /head changed/); + await assert.rejects(postComparison(selection, "Wrong head", api, context), /head changed/); +}); + +function sourceFixture() { + const context = { repository: "example/repo", run: "11", attempt: "2", runNumber: 5, + pullNumber: 2, head: selection.head, runHead: "c".repeat(40) }; + const selected = { + ...selection, repository: context.repository, run: context.run, attempt: context.attempt, merge: context.runHead, + include: [{ id: "skill-aaaaaaaaaaaa", plugin: "local", skill: "draft", directory: "plugins/local/skills/draft", + evalPath: "evals/local/skills/draft/eval.yaml", base: selection.base, head: selection.head }], + }; + const source = { id: 11, run_attempt: 2, pull_requests: [{ number: 2 }] }; + const run = { ...source, repository: { full_name: context.repository }, head_repository: { full_name: context.repository }, + event: "pull_request", status: "completed", conclusion: "failure", head_sha: context.runHead, + run_number: 5, workflow_id: 7, pull_requests: [{ number: 2, head: { sha: selection.head } }] }; + const api = async (endpoint) => { + if (endpoint.endsWith("/actions/runs/11/attempts/2")) return run; + if (endpoint.endsWith("/actions/workflows/7")) return { path: ".github/workflows/skill-evals.yml" }; + if (endpoint.endsWith(`/git/commits/${selected.merge}`)) return { + sha: selected.merge, parents: [{ sha: selected.base }, { sha: selected.head }], + }; + throw new Error("Unexpected test API request."); + }; + return { context, selected, source, run, api }; +} + +test("publisher authenticates workflow, source attempt and PR independently from its own run", async () => { + const { context, source, run, api } = sourceFixture(); + const env = { GITHUB_EVENT_NAME: "workflow_run", GITHUB_REPOSITORY: context.repository, + GITHUB_RUN_ID: "999", GITHUB_RUN_ATTEMPT: "7", GITHUB_SHA: "d".repeat(40) }; + assert.deepEqual(await publisherContext({ workflow_run: source }, api, env), context); + await assert.rejects(publisherContext({ workflow_run: source }, api, { ...env, GITHUB_EVENT_NAME: "pull_request" }), /workflow_run/); + for (const change of [ + { event: "workflow_dispatch" }, { head_repository: { full_name: "fork/repo" } }, + { repository: { full_name: "other/repo" } }, { pull_requests: [] }, { run_attempt: 1 }, + ]) { + await assert.rejects(publisherContext({ workflow_run: source }, async (endpoint) => + endpoint.includes("/actions/runs/") ? { ...run, ...change } : api(endpoint), env)); + } + await assert.rejects(publisherContext({ workflow_run: source }, async (endpoint) => + endpoint.includes("/actions/workflows/") ? { path: ".github/workflows/spoofed.yml" } : api(endpoint), env), /workflow path/); +}); + +test("untrusted artifacts are bounded data, source-bound and re-sanitized before rendering", async (t) => { + const root = mkdtempSync(join(tmpdir(), "comparison-publisher-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + const { selected, context, api } = sourceFixture(); + writeFileSync(join(root, "selection.json"), JSON.stringify(selected)); + const arm = { + id: selected.include[0].id, key: "local/draft", arm: "before", repository: context.repository, + run: context.run, attempt: context.attempt, merge: selected.merge, + data: pair().before, + }; + arm.data.rows[0].model = "@everyone | "; + arm.data.rows[0].checks[1].reason = "SECRET_UNTRUSTED_EVIDENCE"; + writeFileSync(join(root, `${arm.id}-before.json`), JSON.stringify(arm)); + const loaded = await loadComparison(root, context, api); + const rendered = renderComparison(loaded.selection, loaded.pairs, "https://example.test"); + assert.doesNotMatch(rendered, /"; + arm.data.rows[0].eval = "A&B"; + arm.data.rows[0].checks[1].name = "A&B"; arm.data.rows[0].checks[1].reason = "SECRET_UNTRUSTED_EVIDENCE"; writeFileSync(join(root, `${arm.id}-before.json`), JSON.stringify(arm)); const loaded = await loadComparison(root, context, api); const rendered = renderComparison(loaded.selection, loaded.pairs, "https://example.test"); - assert.doesNotMatch(rendered, /"; - arm.data.rows[0].eval = "A&B"; - arm.data.rows[0].checks[1].name = "A&B"; - arm.data.rows[0].checks[1].reason = "SECRET_UNTRUSTED_EVIDENCE"; - writeFileSync(join(root, `${arm.id}-before.json`), JSON.stringify(arm)); - const loaded = await loadComparison(root, context, api, selected); - const rendered = renderComparison(loaded.selection, loaded.pairs, "https://example.test"); - assert.doesNotMatch(rendered, /