diff --git a/.github/scripts/skill-eval-comment.mjs b/.github/scripts/skill-eval-comment.mjs new file mode 100644 index 0000000..9b0fb2c --- /dev/null +++ b/.github/scripts/skill-eval-comment.mjs @@ -0,0 +1,340 @@ +import { execFileSync } from "node:child_process"; +import { existsSync, mkdirSync, readFileSync, readdirSync, realpathSync, statSync, writeFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { stripVTControlCharacters } from "node:util"; + +const marker = "`; + const comments = []; + for (let page = 1; ; page++) { + const batch = await api(`${endpoint}/issues/${env.PR_NUMBER}/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)]; + 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`, + existing ? "PATCH" : "POST", { body: content }); +} + +function validateIdentity(selection, env) { + if (!sha.test(selection.base) || !sha.test(selection.head) || selection.merge !== env.GITHUB_SHA || + selection.repository !== env.GITHUB_REPOSITORY || selection.run !== env.GITHUB_RUN_ID || + selection.attempt !== env.GITHUB_RUN_ATTEMPT) throw new Error("Comparison artifact context changed."); +} + +async function main() { + const [command, path] = process.argv.slice(2); + if (command === "collect") { + const selection = json(resolve(process.env.SELECTION_FILE)); + validateIdentity(selection, process.env); + const entry = selection.include.find((item) => item.id === process.env.COMPARISON_ID); + if (!entry || !["before", "after"].includes(process.env.ARM)) throw new Error("Unknown comparison arm."); + const report = process.env.ACTION_REPORT && existsSync(process.env.ACTION_REPORT) ? json(process.env.ACTION_REPORT) : undefined; + if (report && (report.reportId !== `${entry.id}-${process.env.ARM}` || + report.revision !== selection.merge || String(report.workflowRunId) !== selection.run || + String(report.workflowRunAttempt) !== selection.attempt || report.repository !== selection.repository)) throw new Error("Action report identity changed."); + const logs = process.env.NATIVE_DIRECTORY && existsSync(process.env.NATIVE_DIRECTORY) + ? readdirSync(process.env.NATIVE_DIRECTORY).filter((file) => file.endsWith(".stdout.log")).map((file) => { + const source = resolve(process.env.NATIVE_DIRECTORY, file); + if (statSync(source).size > 64 * 1024 * 1024) throw new Error("Native log exceeds supported size."); + return readFileSync(source, "utf8"); + }) : []; + writeJson(resolve(path, `${entry.id}-${process.env.ARM}.json`), { + id: entry.id, key: `${entry.plugin}/${entry.skill}`, arm: process.env.ARM, + repository: selection.repository, run: selection.run, attempt: selection.attempt, merge: selection.merge, + 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 evidence = `${process.env.GITHUB_SERVER_URL}/${selection.repository}/actions/runs/${selection.run}`; + console.log(JSON.stringify(await postComparison(selection, renderComparison(selection, pairs, evidence)))); + } else throw new Error("Expected collect or post."); +} + +if (process.argv[1] && realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url))) { + try { await main(); } catch (error) { console.error(error.message); process.exitCode = 1; } +} diff --git a/.github/scripts/skill-eval-comment.test.mjs b/.github/scripts/skill-eval-comment.test.mjs new file mode 100644 index 0000000..248309b --- /dev/null +++ b/.github/scripts/skill-eval-comment.test.mjs @@ -0,0 +1,165 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +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"; + +const trial = (extra = {}) => ({ + type: "trial-result", runId: "native", evalName: "draft", stimulus: "draft-positive", model: "model-a", variant: "main", + status: "success", gradeResult: { passed: false, details: [ + { graderType: "completed", name: "completed", passed: true }, + { graderType: "skill-invocation", name: "routing", passed: false, evidence: "SECRET_RAW_GRADER_OUTPUT" }, + ] }, trajectory: { output: "SECRET_RAW_ANSWER", events: [] }, ...extra, +}); +const footer = { type: "run-summary", runId: "native", hadExecutionErrors: false }; +const report = { schemaVersion: 2, command: "run", nativeRunId: "native", outcome: "failed", completeness: "complete", code: "native-failed" }; +const selection = { + base: "a".repeat(40), head: "b".repeat(40), changed: ["local/draft"], + skills: [{ key: "local/draft", hasEval: true }, { key: "local/neighbor", hasEval: true }, { key: "remote/run", hasEval: false }], +}; +const pair = () => ({ + key: "local/draft", + before: { state: "complete", rows: projectTrials([trial(), footer], "native") }, + after: { state: "complete", rows: projectTrials([trial({ gradeResult: { passed: true, details: [ + { graderType: "completed", name: "completed", passed: true }, { graderType: "skill-invocation", name: "routing", passed: true }, + ] } }), footer], "native") }, +}); + +test("native grader projection preserves counts and errors, rejects mixed/repeated trials, and omits raw output", () => { + const data = collectArm(report, ["0.21.0\n", `${JSON.stringify(trial())}\n${JSON.stringify(footer)}\n`]); + assert.equal(data.state, "complete"); + assert.equal(data.rows[0].checks.filter((check) => check.passed).length, 1); + assert.match(data.rows[0].checks[1].reason, /activation/); + assert.doesNotMatch(JSON.stringify(data), /SECRET/); + assert.throws(() => projectTrials([trial(), trial(), footer], "native"), /one trial/); + assert.throws(() => projectTrials([trial({ runId: "other" }), footer], "native"), /identity/); + assert.throws(() => projectTrials([trial(), { ...footer, runId: "other" }], "native"), /completion/); + assert.throws(() => collectArm(report, []), /exactly one/); + assert.equal(collectArm(undefined, []).state, "error"); + assert.equal(collectArm({ ...report, outcome: "error", completeness: "partial" }, + [`${JSON.stringify(trial())}\n${JSON.stringify(footer)}`]).state, "error"); + const errored = projectTrials([trial({ gradeResult: { passed: false, details: [{ status: "error" }] } }), footer], "native"); + assert.equal(errored[0].state, "error"); + assert.equal(errored[0].passed, false); + assert.equal(projectTrials([trial({ status: "skipped" }), footer], "native")[0].state, "not-run"); +}); + +test("approved hierarchy shows grader versus scenario denominators, failed-check reasons and honest coverage", () => { + const sample = pair(); + const rendered = renderComparison(selection, [sample], "https://github.com/example/repo/actions/runs/1"); + assert.match(rendered, /## Changed skill: local\/draft/); + assert.match(rendered, /draft : positive \| 📈 1\/2 → 2\/2/); + assert.match(rendered, /passing \/ total graders/); + assert.match(rendered, /local\/draft \| 0\/1 → 1\/1/); + assert.match(rendered, /passing scenarios \/ total recorded scenarios/); + assert.match(rendered, /Before \(resolved After\) \| routing/); + assert.match(rendered, /Required activation/); + assert.match(rendered, /local\/neighbor \| ⏭️ Not run/); + assert.match(rendered, /remote\/run \| ⏭️ Not evaluated — no repo eval/); + assert.match(rendered, /does not measure unchanged-skill regressions/); + assert.ok(rendered.indexOf("Before failures") < rendered.indexOf("All marketplace skills")); + assert.ok(rendered.indexOf("All marketplace skills") < rendered.indexOf("Marketplace failure details")); + assert.equal((rendered.match(/
/g) ?? []).length, 3); + assert.doesNotMatch(rendered, /SECRET/); +}); + +test("equal aggregate counts expose changed failure identities and missing arms never become passes", () => { + const sample = pair(); + sample.after.rows[0].passed = false; + sample.after.rows[0].checks[0].passed = false; + const rendered = renderComparison(selection, [sample], "https://example.test"); + assert.match(rendered, /🔄 0\/1 → 0\/1 \(1 new, 1 resolved/); + assert.match(rendered, /After \(new\) \| completed/); + assert.match(rendered, /Before \(resolved After\) \| routing/); + sample.after = { state: "error", rows: [] }; + const missing = renderComparison(selection, [sample], "https://example.test"); + assert.match(missing, /Incomplete\/error evidence/); + assert.match(missing, /1\/2 → ⚠️ Error/); + assert.doesNotMatch(missing, /📈 1\/2/); +}); + +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 })); + mkdirSync(join(root, "plugins/local/skills/draft"), { recursive: true }); + writeFileSync(join(root, "plugins/local/skills/draft/SKILL.md"), "not executed"); + mkdirSync(join(root, "evals/local/skills/draft"), { recursive: true }); + writeFileSync(join(root, "evals/local/skills/draft/eval.yaml"), "not executed"); + writeFileSync(join(root, "marketplace.json"), JSON.stringify({ plugins: [ + { name: "local", source: "./plugins/local" }, + { name: "remote", source: { source: "github", repo: "example/plugin", sha: "c".repeat(40), path: "package" } }, + ] })); + const readTree = (repo, commit) => { + assert.equal(repo, "example/plugin"); assert.equal(commit, "c".repeat(40)); + return { truncated: false, tree: [ + { type: "blob", path: "package/skills/run/SKILL.md" }, { type: "blob", path: "other/skills/unrelated/SKILL.md" }, + ] }; + }; + assert.deepEqual(inventorySkills(root, readTree), [ + { key: "local/draft", hasEval: true }, { key: "remote/run", hasEval: false }, + ]); + assert.throws(() => inventorySkills(root, () => ({ truncated: true, tree: [] })), /Incomplete/); +}); + +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 calls = []; + let head = selection.head, comments = []; + const api = async (endpoint, method = "GET", body) => { + calls.push({ endpoint, method, body }); + if (endpoint.endsWith("/pulls/2")) return { state: "open", head: { sha: head, repo: { full_name: "example/repo" } } }; + if (method === "GET") return comments; + return { id: 8 }; + }; + await postComparison(selection, "Report", api, env); + 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); + 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"); + head = "c".repeat(40); + await assert.rejects(postComparison(selection, "Wrong head", api, env), /head changed/); +}); + +test("collect CLI binds native Action diagnostics to this selection without publishing raw evidence", (t) => { + const root = mkdtempSync(join(tmpdir(), "comparison-collect-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + const context = { + repository: "example/repo", run: "1", attempt: "1", merge: "c".repeat(40), + base: selection.base, head: selection.head, include: [{ id: "skill-id", plugin: "local", skill: "draft" }], + }; + writeFileSync(join(root, "selection.json"), JSON.stringify(context)); + const action = { + ...report, reportId: "skill-id-before", revision: context.merge, repository: context.repository, + workflowRunId: 1, workflowRunAttempt: 1, + }; + writeFileSync(join(root, "report.json"), JSON.stringify(action)); + writeFileSync(join(root, "native.stdout.log"), `${JSON.stringify(trial())}\n${JSON.stringify(footer)}\n`); + const options = { + encoding: "utf8", timeout: 10000, + env: { ...process.env, GITHUB_REPOSITORY: context.repository, GITHUB_RUN_ID: "1", GITHUB_RUN_ATTEMPT: "1", + GITHUB_SHA: context.merge, COMPARISON_ID: "skill-id", ARM: "before", SELECTION_FILE: join(root, "selection.json"), + ACTION_REPORT: join(root, "report.json"), NATIVE_DIRECTORY: root }, + }; + const args = [fileURLToPath(new URL("./skill-eval-comment.mjs", import.meta.url)), "collect", join(root, "derived")]; + const result = spawnSync(process.execPath, args, options); + assert.ifError(result.error); + assert.equal(result.status, 0, result.stderr); + const saved = readFileSync(join(root, "derived/skill-id-before.json"), "utf8"); + assert.equal(JSON.parse(saved).data.rows[0].checks.length, 2); + assert.doesNotMatch(saved, /SECRET/); + writeFileSync(join(root, "report.json"), JSON.stringify({ ...action, reportId: "wrong-report" })); + const mixed = spawnSync(process.execPath, args, options); + assert.ifError(mixed.error); + assert.notEqual(mixed.status, 0); + assert.match(mixed.stderr, /identity changed/); +}); diff --git a/.github/scripts/validation.test.mjs b/.github/scripts/validation.test.mjs index ba64bdf..04d93c3 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,162 @@ 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.deepEqual(workflow.jobs.select.permissions, { contents: "read", actions: "read" }); + 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.21.0"); + assert.equal(before["working-directory"], "."); + 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' }}"); + const comment = workflow.jobs.comment; + assert.deepEqual(comment.permissions, { contents: "read", actions: "read", "pull-requests": "write" }); + assert.deepEqual(comment.needs, ["select", "evals"]); + 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); + 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"); + file("marketplace.json", JSON.stringify({ plugins: [{ name: "example", source: "./plugins/example" }] })); + file(".github/scripts/skill-eval-comment.mjs", readFileSync(resolve(root, ".github/scripts/skill-eval-comment.mjs"), "utf8")); + 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..fa9e213 --- /dev/null +++ b/.github/workflows/skill-evals.yml @@ -0,0 +1,237 @@ +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 + permissions: + contents: read + actions: read + 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 }} + GH_TOKEN: ${{ github.token }} + run: | + node --input-type=module <<'JS' + import { execFileSync, spawnSync } from 'node:child_process'; + import { createHash } from 'node:crypto'; + import { appendFileSync, existsSync, readFileSync, writeFileSync } from 'node:fs'; + import { resolve } from 'node:path'; + import { inventorySkills } from './.github/scripts/skill-eval-comment.mjs'; + 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)}` }); + } + const selection = { + include, changed: keys, skills: inventorySkills(process.cwd()), + base, head, merge: process.env.GITHUB_SHA, + repository: process.env.GITHUB_REPOSITORY, + 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`); + JS + - name: Retain comparison selection + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: skill-comparison-${{ github.run_attempt }}-selection + path: ${{ runner.temp }}/selection.json + if-no-files-found: error + retention-days: 7 + + 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: Read comparison selection + uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5 + with: + name: skill-comparison-${{ github.run_attempt }}-selection + path: ${{ runner.temp }}/skill-selection + - 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 + id: before + uses: github/evals-action@6e4c077e92194c7e75663d8d782426841047e887 # v0.2.0 + with: + command: run + working-directory: . + eval-paths: ${{ matrix.evalPath }} + vally-version: '0.21.0' + copilot-token: ${{ github.token }} + runs: 1 + workers: 1 + max-retries: 0 + upload-transcripts: false + report-id: ${{ matrix.id }}-before + - name: Keep only before grader verdicts + if: ${{ !cancelled() && steps.before_source.outcome == 'success' }} + env: + ARM: before + COMPARISON_ID: ${{ matrix.id }} + SELECTION_FILE: ${{ runner.temp }}/skill-selection/selection.json + ACTION_REPORT: ${{ steps.before.outputs.result-path }} + NATIVE_DIRECTORY: ${{ steps.before.outputs.native-diagnostics-path }} + run: node .github/scripts/skill-eval-comment.mjs collect "$RUNNER_TEMP/skill-comparison" + - 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 + id: after + if: ${{ !cancelled() && steps.after_source.outcome == 'success' }} + uses: github/evals-action@6e4c077e92194c7e75663d8d782426841047e887 # v0.2.0 + with: + command: run + working-directory: . + eval-paths: ${{ matrix.evalPath }} + vally-version: '0.21.0' + copilot-token: ${{ github.token }} + runs: 1 + workers: 1 + max-retries: 0 + upload-transcripts: false + report-id: ${{ matrix.id }}-after + - name: Keep only after grader verdicts + if: ${{ !cancelled() && steps.after_source.outcome == 'success' }} + env: + ARM: after + COMPARISON_ID: ${{ matrix.id }} + SELECTION_FILE: ${{ runner.temp }}/skill-selection/selection.json + ACTION_REPORT: ${{ steps.after.outputs.result-path }} + NATIVE_DIRECTORY: ${{ steps.after.outputs.native-diagnostics-path }} + run: node .github/scripts/skill-eval-comment.mjs collect "$RUNNER_TEMP/skill-comparison" + - name: Retain minimal comparison evidence + if: ${{ !cancelled() }} + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: skill-comparison-${{ github.run_attempt }}-${{ matrix.id }} + path: ${{ runner.temp }}/skill-comparison/*.json + if-no-files-found: error + retention-days: 7 + + comment: + needs: [select, evals] + if: ${{ !cancelled() && needs.select.result == 'success' }} + runs-on: ubuntu-latest + timeout-minutes: 5 + concurrency: + group: skill-eval-comment-${{ github.event.pull_request.number }} + cancel-in-progress: false + permissions: + contents: read + actions: read + pull-requests: write + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + persist-credentials: false + - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 + with: + node-version: 24 + - name: Read this run's comparison evidence only + uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5 + with: + pattern: skill-comparison-${{ github.run_attempt }}-* + merge-multiple: true + path: ${{ runner.temp }}/skill-comparison + - name: Update the PR comparison comment without model access + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: node .github/scripts/skill-eval-comment.mjs post "$RUNNER_TEMP/skill-comparison" diff --git a/README.md b/README.md index 3ca8f26..8179784 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,12 @@ 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. +The workflow updates one PR comment with per-case grader counts, failure +details, and an all-skills coverage table. Unmeasured skills are marked not run; +the comment does not imply a full-marketplace or multi-model run. 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 3604cec..dd6bdd6 100644 --- a/docs/checks-and-evals.md +++ b/docs/checks-and-evals.md @@ -125,5 +125,72 @@ 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. + +The workflow uses Vally 0.21, so native project Docker defaults are honored +when configured. It does not configure or bypass the Copilot Evals plugin's +separate Run entrypoint. The Action manages its runtime dependencies; parity +with the locally pinned Copilot CLI is not assumed. + +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. + +### PR comparison comment + +The comment shows one visible table per changed skill. Rows are eval/case +identities; columns are models actually observed in native results. Cells show +Before → After **passing/total graders**. After failures stay visible; Before +failures are collapsed. A separate collapsed marketplace table shows +**passing/total recorded cases**, with nested failure details. + +The current workflow evaluates each eligible changed skill separately using its +saved model, not a new model roster or all marketplace evals. Other skill +sources stay fixed at the merge revision in each comparison. Unchanged skills +and remote skills without repo evals remain explicitly not evaluated; loading +their context does not measure regressions. Local `skills//SKILL.md` +folders and equivalent folders in each remote plugin's exact pinned GitHub tree +provide the inventory, without installing those plugins. Nonstandard remote +skill layouts need an inventory adaptation. + +One scenario can pass under its native scoring rule even if some individual +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. + +The small presentation helper reads the pinned Action's local native JSONL logs +and uploads only case/model identities, verdicts and safe check explanations. +Raw trajectories, outputs, grader evidence, exceptions and transcripts are not +uploaded or copied into comments. Failure reasons describe the check that +failed, not a generated diagnosis; the evidence link leads to this workflow's +native summaries and derived artifacts. Structured/derived artifacts use seven-day +retention, subject to repository policy. + +The comment job has `actions: read` and `pull-requests: write`, but no model-billing +permission. Paid jobs have no comment-write permission. Publishing checks the +live PR head, refuses forks/closed PRs, and updates only its own bot comment; +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.