ci(review): fold real-world test evidence into the PR review comment via the gaia-testing skill (strix-halo) #2413

Activity

1. itomek

   added this to the Test & CI Hardening — Strix Halo + Installer [OSS] milestone on Jul 22, 2026
2. itomek

   added

   devopsDevOps/infrastructure changesDevOps/infrastructure changes

   testsTest changesTest changes

   on Jul 22, 2026
3. github-actions commented on Jul 23, 2026

   

   github-actionsBot

   on Jul 23, 2026 – with GitHub Actions

   Contributor

   More actions

   This is a solid, well-scoped design — the "two jobs, one comment" split (cheap ubuntu review + strix-halo evidence, folded via needs:) is the right call over coupling every review to the self-hosted box's uptime. It's actionable as written. One security nuance worth pinning down explicitly before implementation, plus a couple of grounding notes below.

   The one thing I'd flag: the evidence job is the first place in this pipeline that executes PR-head code (the review jobs only read the diff). On a self-hosted Windows runner that's a real trust boundary — the maintainer-label gate you list handles forks, but the evidence job must also keep secrets out of scope and clean workspace state between runs, since self-hosted runners persist. Worth making that an explicit acceptance criterion. 🔒 cc @kovtcharov-amd since this touches the pull_request_target security model.

   🔍 Technical details

   Grounding against the current files:

   • .github/workflows/claude.yml — the review jobs (pr-review, pr-rereview) don't run inline; they call the reusable claude-run.yml via uses: ./.github/workflows/claude-run.yml (lines ~126, ~375). A needs:-wired evidence job producing an artifact the review job downloads works, but the review job is a reusable-workflow call, not steps: — so the "download bundle + extend prompt" wiring either needs the bundle passed as an input to claude-run.yml or the review job restructured to inline steps. Worth deciding which in the design.
   • The file header (lines 5–6, 47, 67) already documents that pull_request_target runs with base-repo secrets in scope even on fork PRs, and that the workflow file is sourced from base (main). The evidence job inherits that context, so: (a) it must not expose secrets to PR-head test execution, and (b) the "rubric/skill sourced from base branch" criterion is already the established pattern here — good, keep it.
   • install-lemonade action + ensure-lemonade-running.ps1 are the strix-halo (Windows/PowerShell) bring-up path referenced correctly; the many test_*.yml suites stay as status-check sources per the out-of-scope note.
   • Dependency: this correctly blocks on docs(testing): bind real-world evidence contract to the changed surface #2376 (gaia-testing skill + raw-URL evidence rule) — the gaia-testing skill exists at .claude/skills/gaia-testing, so the "invoke the skill, don't reimplement" criterion is enforceable.

   Suggested added acceptance criterion: the evidence job runs PR-head code with no repo secrets in its environment and resets the self-hosted workspace between runs (untrusted-code isolation on a persistent runner).
4. itomek commented on Jul 23, 2026

   

   itomek

   on Jul 23, 2026

   CollaboratorAuthor

   More actions

   Phase-1 implementation plan (branch tmi/ci-review-evidence set up off main)

   Security architecture (the crux): the evidence job must run the PR's code, so it runs on pull_request (no secrets, read-only token — fork-safe), uploads the evidence as an artifact, and the existing pull_request_target review job (which keeps the secrets and never runs PR code) pulls that artifact and folds it into its single comment. Cross-trigger artifact hand-off is the part that needs CI validation.

   Phase 1 — wiring + CLI/API lane (ubuntu, validatable without strix)

   1. New pr-evidence job (trigger pull_request, runs-on: ubuntu-latest, no secrets): checkout PR head → set up python/gaia → invoke claude-code-action prompted to use the gaia-testing skill to produce evidence only for the CLI/API/MCP surfaces the diff touches → write evidence-bundle.md → actions/upload-artifact.
   2. claude-run.yml: add optional evidence_artifact input; when set, download-artifact into the repo root before the Claude call so the reviewer sees evidence-bundle.md alongside pr-diff.txt.
   3. pr-review job: pass evidence_artifact; extend the prompt — "if evidence-bundle.md is present, evaluate it and fold its evidence (verbatim command output / request traces) into the single review comment; mark untouched surfaces N/A." Boundary unchanged: it reads evidence + diff, never executes PR code.

   Phase 2 — strix-halo UI/agent lane

   pr-evidence gains a self-hosted [self-hosted, strix-halo] variant (label-gated for forks): install-lemonade + gaia chat --ui + headless Playwright via the skill → screenshots pushed to a …-evidence branch (raw-URL rule) → same bundle. Windows/PowerShell specifics per ensure-lemonade-running.ps1.

   Skill: "CI runner mode" section

   Add to gaia-testing (AFTER #2376 merges, to avoid a SKILL.md conflict): the two-runner split, evidence sinks that work without R2 secrets on forks (artifact + raw evidence branch), the label gate.

   Recommended sequencing

   Merge #2376 first (one approval away) so this builds on the final skill and doesn't conflict; then execute Phase 1 and iterate on CI. Same-repo PRs run automatically; fork PRs label-gated for the code-execution job.
5. itomek

   added 2 commits that reference this issue on Jul 23, 2026

   ci(review): run gaia-testing evidence and fold it into the PR review …

   ...

   3b30186

   ci(review): standalone pull_request evidence workflow, not the shared…

   ...

   afc8cbe
6. pull

   added 2 commits that reference this issue on Jul 23, 2026

   ci(review): fold gaia-testing evidence into the PR review comment (Ph…

   ...

   59c736d

   ci(review): broaden evidence gate + require a verdict-linked evidence…

   ...

   ca50bf8
7. kovtcharov-amd

   modified the milestones: Installer release integration tests [OSS], vFutures on Oct 25, 2026