From 953293c4c03f297686df41299a58745e8d9f4bf6 Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Mon, 15 Jun 2026 00:38:23 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20implement=20issue=20#747=20?= =?UTF-8?q?=E2=80=94=20[Phase=201]=20Make=20the=20Skill=20Eval=20Report=20?= =?UTF-8?q?visible=20=E2=80=94=20surface=20regression/error=20outcomes=20(?= =?UTF-8?q?no=20longer=20silent)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/lint.yml | 3 +- .github/workflows/skill-eval-report.yml | 17 ++- scripts/evals/notify-eval-health.sh | 152 +++++++++++++++++++++ tests/test_eval_health_notify.bats | 173 ++++++++++++++++++++++++ tests/test_skill_eval_report.bats | 56 ++++++++ 5 files changed, 399 insertions(+), 2 deletions(-) create mode 100644 scripts/evals/notify-eval-health.sh create mode 100644 tests/test_eval_health_notify.bats diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 423aa9a61..7435869d1 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -63,7 +63,8 @@ jobs: tests/test_review_registry.bats \ tests/test_skill_evals.bats \ tests/test_holdout_guard.bats \ - tests/test_skill_eval_report.bats + tests/test_skill_eval_report.bats \ + tests/test_eval_health_notify.bats validate-agent-profiles: runs-on: ubuntu-latest diff --git a/.github/workflows/skill-eval-report.yml b/.github/workflows/skill-eval-report.yml index 63527e792..6c0f28c40 100644 --- a/.github/workflows/skill-eval-report.yml +++ b/.github/workflows/skill-eval-report.yml @@ -44,7 +44,8 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 permissions: - contents: read # actions/checkout only — the report is an artifact, no issue/PR writes + contents: read # actions/checkout + issues: write # notify step opens/updates/closes the single eval-health tracking issue (#747) env: CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} SKILL: ${{ inputs.skill || 'triage' }} @@ -146,3 +147,17 @@ jobs: regression) echo "::warning::Skill eval '${SKILL}': held-out regression detected (report-only — not blocking)." ;; *) echo "::warning::Skill eval '${SKILL}': scorer hard-errored (report-only — not blocking)." ;; esac + + - name: Notify eval health (open/update/close tracking issue) + # Visibility, not gating (#747): on regression/error open-or-update a + # single de-duplicated eval-health tracking issue; on a pass that follows + # a prior open issue, comment + close it; a healthy run with no open + # issue makes zero noise. Runs always() so a recovery still closes the + # issue even though the eval step exits 0. + if: always() + env: + GH_TOKEN: ${{ github.token }} + OUTCOME: ${{ steps.eval.outputs.outcome }} + REPO: ${{ github.repository }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: bash scripts/evals/notify-eval-health.sh diff --git a/scripts/evals/notify-eval-health.sh b/scripts/evals/notify-eval-health.sh new file mode 100644 index 000000000..25507ace3 --- /dev/null +++ b/scripts/evals/notify-eval-health.sh @@ -0,0 +1,152 @@ +#!/usr/bin/env bash +set -euo pipefail +# notify-eval-health.sh — surface the daily Skill Eval outcome as a single, +# de-duplicated tracking issue (issue #747, epic #581). +# +# The Skill Eval Report (skill-eval-report.yml) runs the deterministic holdout +# scorer daily and is intentionally NON-BLOCKING: a regression or a scorer +# hard-error never fails the workflow. Before this script its only output was a +# step summary / artifact / annotation — effectively invisible. This notifier +# makes the outcome visible WITHOUT making it blocking: +# +# outcome=regression | error -> open OR update a single `eval-health` tracking +# issue (idempotent: the same issue is rewritten +# in place, never a new one per day). +# outcome=pass -> if a tracking issue is currently open, comment +# + close it (recovery); otherwise do NOTHING +# (a healthy run must create zero noise). +# +# De-dup key: the single open issue carrying $EVAL_HEALTH_LABEL whose title +# matches the stable per-skill title below. Stable title => one issue, rewritten. +# +# This script only adds visibility; it never changes the scorer's pass/fail +# decision or gates any merge. +# +# Env: +# REPO owner/repo (required) +# OUTCOME pass | regression | error (required) +# SKILL skill whose holdout was scored (default: triage) +# REPORT_PATH eval-report.json from the scorer (default: ./eval-report.json) +# RUN_URL link back to the workflow run (optional but recommended) +# EVAL_HEALTH_LABEL tracking-issue label (default: eval-health) +# DRY_RUN "1"/"true" => render + log intent, never mutate via gh +# +# Exit codes: 0 success (including the no-op healthy path); 1 bad usage / gh error. + +REPO="${REPO:?REPO required}" +OUTCOME="${OUTCOME:?OUTCOME required (pass|regression|error)}" +SKILL="${SKILL:-triage}" +REPORT_PATH="${REPORT_PATH:-eval-report.json}" +RUN_URL="${RUN_URL:-}" +EVAL_HEALTH_LABEL="${EVAL_HEALTH_LABEL:-eval-health}" + +case "$OUTCOME" in + pass|regression|error) ;; + *) echo "::error::notify-eval-health: unknown OUTCOME '$OUTCOME' (expected pass|regression|error)" >&2; exit 1 ;; +esac + +# Stable, per-skill title so the lookup always lands on the same issue. +TITLE="Skill eval health — \`${SKILL}\` (holdout)" + +_is_dry_run() { [[ "${DRY_RUN:-0}" == "1" || "${DRY_RUN:-0}" == "true" ]]; } + +# Locate the single open tracking issue (scoped by label) whose title matches. +# gh emits the candidate list as JSON; system jq does the title de-dup so the +# expression is unambiguous (no reliance on gh's embedded jq arg passing). +find_existing() { + if _is_dry_run; then + printf '%s' "${DRY_RUN_EXISTING:-}" + return 0 + fi + gh issue list --repo "$REPO" --label "$EVAL_HEALTH_LABEL" --state open \ + --json number,title 2>/dev/null \ + | jq -r --arg t "$TITLE" 'map(select(.title == $t)) | (.[0].number // "")' 2>/dev/null \ + || true +} + +# Render the tracking-issue body for a regression/error outcome to stdout. +render_body() { + echo "## Skill eval health — \`${SKILL}\` (holdout)" + echo "" + if [ "$OUTCOME" = "error" ]; then + echo "**Outcome:** \`error\` — the scorer hard-errored (bad usage / missing tooling)." + echo "" + echo "_The eval run could not produce a score. This is report-only (Phase 1): it does not block any merge — but it does mean the held-out set is currently un-scored and needs a look._" + else + echo "**Outcome:** \`regression\` — at least one held-out case regressed." + echo "" + echo "_Report-only (Phase 1): a regression does not fail CI or block any merge._" + fi + echo "" + + if jq -e . "$REPORT_PATH" >/dev/null 2>&1; then + local score passed total failed + score=$(jq -r '.score' "$REPORT_PATH") + passed=$(jq -r '.passed' "$REPORT_PATH") + total=$(jq -r '.total' "$REPORT_PATH") + failed=$(jq -r '.failed' "$REPORT_PATH") + echo "| skill | score | passed | failed | total |" + echo "|-------|-------|--------|--------|-------|" + echo "| \`${SKILL}\` | ${score} | ${passed} | ${failed} | ${total} |" + if [ "${failed}" != "0" ]; then + echo "" + echo "### Regressed cases" + jq -r '.cases[] | select(.pass | not) | "- `\(.id)` — expected \(.expected|tojson), got \(.actual|tojson)"' "$REPORT_PATH" + fi + else + echo "Scorer produced no parseable JSON report (outcome: \`${OUTCOME}\`)." + fi + + echo "" + if [ -n "$RUN_URL" ]; then + echo "[View the workflow run](${RUN_URL})" + fi + echo "" + echo "Auto-managed by \`notify-eval-health.sh\` — this issue is updated in place each run and closed automatically on recovery." +} + +existing="$(find_existing)" + +case "$OUTCOME" in + pass) + if [ -z "$existing" ]; then + echo "eval health pass: no open tracking issue — nothing to do." + exit 0 + fi + comment="$(mktemp)" + trap 'rm -f "$comment"' EXIT + { + echo "✅ **Recovered** — the \`${SKILL}\` holdout eval passed; all held-out cases are green again." + echo "" + [ -n "$RUN_URL" ] && echo "[View the workflow run](${RUN_URL})" + } >"$comment" + if _is_dry_run; then + echo "[dry-run] would comment on and close eval-health issue #${existing}" + exit 0 + fi + gh issue comment "$existing" --repo "$REPO" --body-file "$comment" >/dev/null + gh issue close "$existing" --repo "$REPO" >/dev/null + echo "closed eval-health issue #${existing} (recovered)" + ;; + + regression|error) + body="$(mktemp)" + trap 'rm -f "$body"' EXIT + render_body >"$body" + if _is_dry_run; then + echo "[dry-run] would $( [ -n "$existing" ] && echo "update #${existing}" || echo "create" ) eval-health issue '${TITLE}'" + exit 0 + fi + if [ -n "$existing" ]; then + gh issue edit "$existing" --repo "$REPO" --body-file "$body" >/dev/null + echo "updated eval-health issue #${existing}" + else + # Ensure the label exists (ignore failure if it already does). + gh label create "$EVAL_HEALTH_LABEL" --repo "$REPO" \ + --description "Skill-eval holdout health tracking" --color d93f0b 2>/dev/null || true + url="$(gh issue create --repo "$REPO" --title "$TITLE" \ + --label "$EVAL_HEALTH_LABEL" --body-file "$body")" + echo "created eval-health issue ${url}" + fi + ;; +esac diff --git a/tests/test_eval_health_notify.bats b/tests/test_eval_health_notify.bats new file mode 100644 index 000000000..b6d1d6c10 --- /dev/null +++ b/tests/test_eval_health_notify.bats @@ -0,0 +1,173 @@ +#!/usr/bin/env bats +# Offline tests for the eval-health notifier (scripts/evals/notify-eval-health.sh). +# +# This is the visibility layer for the Skill Eval Report (issue #747): on a +# held-out regression OR a scorer hard-error it opens-or-updates a single, +# de-duplicated `eval-health` tracking issue; on recovery (pass with a prior +# open issue) it comments + closes that issue; a healthy run with no open issue +# makes ZERO noise. +# +# These tests drive the script against a STUB `gh` placed first on PATH, so the +# open/update/close + de-dupe branching is exercised with NO network — mirroring +# the stub-engine discipline of tests/test_skill_evals.bats. + +bats_require_minimum_version 1.5.0 + +setup() { + ROOT="$(cd "$BATS_TEST_DIRNAME/.." && pwd)" + NOTIFY="$ROOT/scripts/evals/notify-eval-health.sh" + TMP="$(mktemp -d)" + GH_LOG="$TMP/gh.log" + : >"$GH_LOG" + + # A regression report: one pass, one fail (escalate case mis-approved). + REPORT="$TMP/eval-report.json" + cat >"$REPORT" <<'JSON' +{ + "skill": "triage", + "total": 2, + "passed": 1, + "failed": 1, + "score": 0.5, + "cases": [ + {"id": "case-approve", "pass": true, "expected": {"escalate": false, "risk": "LOW"}, "actual": {"escalate": false, "risk": "LOW"}}, + {"id": "case-escalate", "pass": false, "expected": {"escalate": true, "risk": "HIGH"}, "actual": {"escalate": false, "risk": "LOW"}} + ] +} +JSON + + # The script's de-dup looks up the single open tracking issue by stable title. + # Keep this title in lock-step with notify-eval-health.sh (per-skill TITLE). + TITLE='Skill eval health — `triage` (holdout)' + # JSON the stub returns for `issue list`: empty by default; tests opt into an + # "existing open issue" by setting GH_LIST_JSON to a matching-title array. + EXISTING_JSON="$(jq -nc --arg t "$TITLE" '[{"number":42,"title":$t}]')" + + # Stub gh: logs every invocation + any --body-file content to GH_LOG, returns + # the candidate list JSON from $GH_LIST_JSON for `issue list` (default []), and + # a fixed URL for `issue create`. No network. + BIN="$TMP/bin" + mkdir -p "$BIN" + cat >"$BIN/gh" <<'SH' +#!/usr/bin/env bash +echo "ARGS: $*" >>"$GH_LOG" +prev="" +for a in "$@"; do + if [ "$prev" = "--body-file" ]; then + { echo "BODY<<"; cat "$a"; echo ">>BODY"; } >>"$GH_LOG" + fi + prev="$a" +done +case "$1 $2" in + "issue list") printf '%s' "${GH_LIST_JSON:-[]}" ;; + "issue create") echo "https://github.com/petry-projects/.github-private/issues/123" ;; +esac +exit 0 +SH + chmod +x "$BIN/gh" + export GH_LOG +} + +teardown() { rm -rf "$TMP"; } + +# Extra VAR=value pairs (OUTCOME=..., GH_LIST_JSON=..., DRY_RUN=1) are passed +# through `env` so they take effect even though they are runtime-expanded args. +run_notify() { + run env "PATH=$BIN:$PATH" "GH_LOG=$GH_LOG" \ + "REPO=petry-projects/.github-private" "SKILL=triage" \ + "REPORT_PATH=$REPORT" "RUN_URL=https://github.com/o/r/actions/runs/999" \ + "$@" bash "$NOTIFY" +} + +# A `gh issue list` JSON payload with one matching-title open issue numbered $1. +mklist() { jq -nc --arg t "$TITLE" --argjson n "$1" '[{number:$n,title:$t}]'; } + +# --- regression: open / update / de-dupe ------------------------------------ + +@test "regression with no existing issue creates exactly one eval-health issue" { + run_notify OUTCOME="regression" + [ "$status" -eq 0 ] + grep -q "ARGS: issue create" "$GH_LOG" + grep -q -- "--label eval-health" "$GH_LOG" + [ "$(grep -c 'ARGS: issue create' "$GH_LOG")" -eq 1 ] +} + +@test "regression with an existing open issue updates in place (no new issue — de-dupe)" { + run_notify OUTCOME="regression" "GH_LIST_JSON=$(mklist 42)" + [ "$status" -eq 0 ] + grep -q "ARGS: issue edit 42" "$GH_LOG" + ! grep -q "ARGS: issue create" "$GH_LOG" +} + +@test "the tracking issue is looked up de-duped by label and open state" { + run_notify OUTCOME="regression" + [ "$status" -eq 0 ] + grep -q "ARGS: issue list" "$GH_LOG" + grep -q -- "--label eval-health" "$GH_LOG" + grep -q -- "--state open" "$GH_LOG" +} + +@test "regression body carries the score table, regressed case detail and run link" { + run_notify OUTCOME="regression" + [ "$status" -eq 0 ] + # score table figures + grep -q "0.5" "$GH_LOG" + # regressed case id and the actual-vs-expected detail + grep -q "case-escalate" "$GH_LOG" + # link back to the workflow run + grep -q "actions/runs/999" "$GH_LOG" +} + +# --- error: still visible (never silent) ------------------------------------ + +@test "error outcome opens the tracking issue even with an unparseable report" { + echo 'not json' >"$REPORT" + run_notify OUTCOME="error" + [ "$status" -eq 0 ] + grep -q "ARGS: issue create" "$GH_LOG" +} + +@test "error outcome with an existing issue updates it (de-dupe, not silent)" { + echo 'not json' >"$REPORT" + run_notify OUTCOME="error" "GH_LIST_JSON=$(mklist 7)" + [ "$status" -eq 0 ] + grep -q "ARGS: issue edit 7" "$GH_LOG" + ! grep -q "ARGS: issue create" "$GH_LOG" +} + +# --- pass: recovery closes; healthy makes no noise -------------------------- + +@test "pass with a prior open issue comments and closes it (recovery)" { + run_notify OUTCOME="pass" "GH_LIST_JSON=$(mklist 55)" + [ "$status" -eq 0 ] + grep -q "ARGS: issue comment 55" "$GH_LOG" + grep -q "ARGS: issue close 55" "$GH_LOG" +} + +@test "pass with no existing issue makes zero noise (no create/comment/close)" { + run_notify OUTCOME="pass" + [ "$status" -eq 0 ] + ! grep -q "ARGS: issue create" "$GH_LOG" + ! grep -q "ARGS: issue comment" "$GH_LOG" + ! grep -q "ARGS: issue close" "$GH_LOG" +} + +# --- guards ------------------------------------------------------------------ + +@test "missing REPO is a hard error" { + run env -u REPO "PATH=$BIN:$PATH" "GH_LOG=$GH_LOG" OUTCOME="regression" \ + SKILL="triage" "REPORT_PATH=$REPORT" bash "$NOTIFY" + [ "$status" -ne 0 ] +} + +@test "an unknown OUTCOME is a hard error" { + run_notify OUTCOME="banana" + [ "$status" -ne 0 ] +} + +@test "DRY_RUN never invokes gh mutations" { + run_notify OUTCOME="regression" DRY_RUN=1 + [ "$status" -eq 0 ] + ! grep -q "ARGS: issue create" "$GH_LOG" + ! grep -q "ARGS: issue edit" "$GH_LOG" +} diff --git a/tests/test_skill_eval_report.bats b/tests/test_skill_eval_report.bats index 341d9634c..9621f22c3 100644 --- a/tests/test_skill_eval_report.bats +++ b/tests/test_skill_eval_report.bats @@ -121,6 +121,62 @@ PY [ "$output" = "True" ] } +@test "#747: a job grants issues:write for the notify step (least-privilege add)" { + run python3 - "$WF" <<'PY' +import sys, yaml +with open(sys.argv[1], encoding="utf-8") as fh: + doc = yaml.safe_load(fh) +ok = False +for job in (doc.get("jobs") or {}).values(): + perms = job.get("permissions") or {} + if isinstance(perms, dict) and str(perms.get("issues")) == "write": + ok = True +print(ok) +PY + [ "$status" -eq 0 ] + [ "$output" = "True" ] +} + +@test "#747: job permissions stay scoped (no contents:write, no write-all)" { + run python3 - "$WF" <<'PY' +import sys, yaml +with open(sys.argv[1], encoding="utf-8") as fh: + doc = yaml.safe_load(fh) +ok = True +for job in (doc.get("jobs") or {}).values(): + perms = job.get("permissions") + if isinstance(perms, str): # write-all / read-all scalar => too broad + ok = False + elif isinstance(perms, dict): + if str(perms.get("contents")) == "write": + ok = False +print(ok) +PY + [ "$status" -eq 0 ] + [ "$output" = "True" ] +} + +@test "#747: wires the eval-health notifier step" { + grep -q 'scripts/evals/notify-eval-health.sh' "$WF" +} + +@test "#747: the notify step runs always() so recovery can close the issue" { + run python3 - "$WF" <<'PY' +import sys, yaml +with open(sys.argv[1], encoding="utf-8") as fh: + doc = yaml.safe_load(fh) +ok = False +for job in (doc.get("jobs") or {}).values(): + for step in (job.get("steps") or []): + if "notify-eval-health.sh" in str(step.get("run", "")): + if str(step.get("if", "")).strip() == "always()": + ok = True +print(ok) +PY + [ "$status" -eq 0 ] + [ "$output" = "True" ] +} + @test "AC#4: every third-party action is SHA-pinned (40-hex)" { run python3 - "$WF" <<'PY' import re, sys From 3bdd445c73b45216f23a43093254efd3569a9dd2 Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Mon, 15 Jun 2026 00:41:20 +0000 Subject: [PATCH 2/3] fix(bot): address bot feedback [skip ci-relay] --- scripts/evals/notify-eval-health.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/evals/notify-eval-health.sh b/scripts/evals/notify-eval-health.sh index 25507ace3..ec2b2cb0c 100644 --- a/scripts/evals/notify-eval-health.sh +++ b/scripts/evals/notify-eval-health.sh @@ -81,17 +81,17 @@ render_body() { if jq -e . "$REPORT_PATH" >/dev/null 2>&1; then local score passed total failed - score=$(jq -r '.score' "$REPORT_PATH") - passed=$(jq -r '.passed' "$REPORT_PATH") - total=$(jq -r '.total' "$REPORT_PATH") - failed=$(jq -r '.failed' "$REPORT_PATH") + score=$(jq -r '.score // "N/A"' "$REPORT_PATH") + passed=$(jq -r '.passed // "0"' "$REPORT_PATH") + total=$(jq -r '.total // "0"' "$REPORT_PATH") + failed=$(jq -r '.failed // "0"' "$REPORT_PATH") echo "| skill | score | passed | failed | total |" echo "|-------|-------|--------|--------|-------|" echo "| \`${SKILL}\` | ${score} | ${passed} | ${failed} | ${total} |" if [ "${failed}" != "0" ]; then echo "" echo "### Regressed cases" - jq -r '.cases[] | select(.pass | not) | "- `\(.id)` — expected \(.expected|tojson), got \(.actual|tojson)"' "$REPORT_PATH" + jq -r '(.cases | select(type == "array") // []) | .[] | select(.pass | not) | "- `\(.id // \"unknown\")` — expected \(.expected|tojson), got \(.actual|tojson)"' "$REPORT_PATH" fi else echo "Scorer produced no parseable JSON report (outcome: \`${OUTCOME}\`)." From 4790a13898d16504292c4f193ba1df83176d528a Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Mon, 15 Jun 2026 00:50:05 +0000 Subject: [PATCH 3/3] chore: apply manual instructions [skip ci-relay] --- scripts/evals/notify-eval-health.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/evals/notify-eval-health.sh b/scripts/evals/notify-eval-health.sh index ec2b2cb0c..0ce2f5d1b 100644 --- a/scripts/evals/notify-eval-health.sh +++ b/scripts/evals/notify-eval-health.sh @@ -91,7 +91,7 @@ render_body() { if [ "${failed}" != "0" ]; then echo "" echo "### Regressed cases" - jq -r '(.cases | select(type == "array") // []) | .[] | select(.pass | not) | "- `\(.id // \"unknown\")` — expected \(.expected|tojson), got \(.actual|tojson)"' "$REPORT_PATH" + jq -r --arg def "unknown" '(.cases | select(type == "array") // []) | .[] | select(.pass | not) | "- `\(.id // $def)` — expected \(.expected|tojson), got \(.actual|tojson)"' "$REPORT_PATH" fi else echo "Scorer produced no parseable JSON report (outcome: \`${OUTCOME}\`)."