[Skills] Add code-review: run review-pr under Copilot code review

[zhiding512]

Adds .github/skills/code-review/SKILL.md, a thin adapter that runs the review-pr skill inside
Copilot code review, and .github/workflows/copilot-code-review.yml, which prepares its environment.
The main ruleset already requests a Copilot review when a PR to main is opened by an author with
Copilot access; once this merges, those reviews load this skill. Coverage is unchanged by this PR.

Copilot code review reads skills from .github/skills/. Its sandbox blocks api.github.com, so
fetch.sh cannot run there, and its own checkout has no usable history. The adapter keeps
review-pr's rules, triage.py evidence and seven gates, and changes:

• Evidence: copilot-code-review.yml fetches the full history and the base branches before the
  firewall starts. The block then builds fetch.sh's artifacts from git diff origin/<base>...HEAD
  with no network, in a mktemp directory per session. It stops at the first failed command,
  including an empty diff, and writes rules.txt only when every command succeeded, so a failed
  evidence step leaves the ledger gate red.
• Title: read through a quoted here-document, so a Revert "..." title or one containing $(...)
  stays data.
• Step 8.7: no independent reader exists; independent.txt says NONE AVAILABLE and the card says
  "not independently refuted", which review-pr allows.
• Gates: run on the session's own directory; a red gate is fixed once, then the session stops, and a
  📝 comment names any gate still red.
• Output: findings are posted as review comments. Copilot combines three sessions, votes on medium
  and low findings and grades severity itself, so a card finding can be dropped or re-graded.
  Copilot leaves a Comment review by default, so the review does not block merging.

The CI and Perf lines will usually read PENDING and NOT MEASURED: Copilot reviews within minutes,
long before the GPU test jobs finish, and the default GitHub MCP server may not read job logs.

Checked locally on #1290 (head 3a7115f), with the remote URL pointed at a dead path to stand in for
the firewall:

• Full history: the block matches fetch.sh byte for byte (pr.diff, rules.txt,
  rules_expanded.txt, scan.txt, core_expected.txt, bounds.txt, ci_coverage.txt, and the
  benchcov part of bench_compare.txt).
• Depth-1 clone after the setup step's commands: same result.
• Depth-1 clone without the setup step: stops at the unshallow (exit 128), writes no rules.txt,
  and the ledger gate reports MISSING rules.txt.
• Empty diff: stops (exit 1), writes no rules.txt, and the ledger gate reports MISSING rules.txt.
• Two sessions at once, both titled Revert "x $(printf INJECTED)" plus a backquoted command: two
  directories, both match fetch.sh, and nothing in the title ran.

Not yet verified: that Copilot code review runs copilot-code-review.yml from this PR's head. The
docs say copilot-setup-steps.yml only runs from the default branch, so the workflow may have to
reach main first; the job log of this PR's next Copilot review will show which.

review-pr itself still never posts.

[Copilot]

🟡 Changes recommended

The evidence setup requires unsupported Git/network access, and literal PR-title interpolation permits shell command execution.

2 open findings

• PR title interpolation enables shell command injection · New
• Requires unavailable Git history and network fetch instead of supported evidence · New

What changed in this PR

Adds a skill adapter intended to bring FlyDSL’s existing review-pr workflow into automatic Copilot code reviews.

Changes:

• Defines evidence collection through Git commands and GitHub MCP.
• Retains review gates while marking independent refutation unavailable.
• Converts review-card findings into posted comments.

File	Description
.github/​skills/​code-review/​SKILL.md	Adds the Copilot adapter for evidence collection, review gates, and output.

🧠 Review effort: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jhinpan]

Requesting changes. The adapter keeps review-pr's rules and the seven gates, and the command block works in a full clone. But in this PR's own Copilot run, the evidence step did not run, so the gates did not control what was posted. The inline comments cover three smaller problems in the block.

Works as described

• Copilot loads this skill. The job log shows invoked: name="code-review" in all three ensemble sessions and in the aggregation session.
• In a full clone of #1290 (head 3a7115f), the block writes the same pr.diff, rules.txt, rules_expanded.txt, scan.txt, core_expected.txt, bounds.txt and ci_coverage.txt as fetch.sh, byte for byte. Only bench_compare.txt differs (no CI-log rows), as the description says.
• The gates accept both edits. gate_card matches the prefix Review (advisory (triage.py#L1069), and NONE AVAILABLE -- ... passes gate_independent when the card says "not independently refuted".

Blocking: the evidence step did not run in Copilot

The description calls this PR's own review the first real test. In that run:

• Each of the three ensemble sessions stored a finding on lines 23-24. The posted version (thread) says the sandbox has no Git history and no network, so it cannot build pr.diff or merge-target/.
• All four sessions logged "github-mcp-server"=connected/invocations=0. No session read CI status, human comments or benchmark rows.
• The overview uses Copilot's fixed format. It has no Review, CI or Perf line.

If the block cannot run, the ledger gate cannot pass (MISSING rules.txt). Every session still finished, and the review was posted. So in Copilot the gates are not enforced, and that is the property this PR says it keeps.

Author must make the evidence exist in the environment that Copilot provides, then link one session log (PR timeline, "View session") where the block ran and triage.py gates printed 7 green / 0 red. One option to test: setup steps in .github/workflows/copilot-code-review.yml run before the agent firewall starts (docs), so a fetch-depth: 0 checkout and the git lines of the block could run there. The log line Repository is not checked out at /home/runner/work/FlyDSL/FlyDSL. Checking out a fresh copy. suggests that Copilot reuses an existing checkout. I have not verified this for code review.

Not blocking

• The posted review is not one card. The detector runs three sessions (EnsembleConfig=[{"count":3,"role":"general"}]), needs two votes for medium and low findings (MinNumberOfVotesMediumSev=2;MinNumberOfVotesLowSev=2), and sets severity with its own classifier. A ⚠️ or 📝 that only one session finds can be dropped. Lines 49-51 and the description should not promise that each card finding is posted.
• The CI and Perf lines will usually read PENDING and NOT MEASURED. Copilot posted this review about 4 minutes after the request, while the Fly DSL test run for #1290 took about 1 h 52 min. Also, the MCP docs show actions only as an opt-in X-MCP-Toolsets value, so the default GitHub MCP server may not read job logs at all (unverified).

[jhinpan]

⚠️ [inferred] /tmp/review is one directory for every session in the job. The job log shows three ensemble sessions loading this skill from the same checkout at 03:35:52.40, .55 and .58, and an aggregation session loading it again at 03:37:36. With one shared directory, the second git worktree add fails with already exists (I hit this on a local retry). The sessions also overwrite each other's answers.txt, verdicts.txt and card.md, so one session can pass the gates on files that another session wrote. fetch.sh uses mktemp for this reason (fetch.sh#L22-L23). Author must create one directory per session with W=$(mktemp -d /tmp/review-XXXXXX) and use $W everywhere, including line 44.

[jhinpan]

⚠️ [verified] A failed git diff does not stop the block. On a depth-1 clone of #1290's head, this line printed fatal: FETCH_HEAD...HEAD: no merge base and left pr.diff empty. The next lines still ran, and rules.txt became [none] NONE. From that state, a ✅ NO FINDINGS card plus the step files passed all seven gates (7 green / 0 red), because no gate checks that the diff is non-empty. fetch.sh refuses this case (fetch.sh#L63). Here the only signal is one stderr line, which line 34 expects the agent to notice. Author must stop the block on the first failed command and on an empty diff, so that the ledger gate reports MISSING rules.txt.

The block below passed locally on #1290. The shallow attempt stops with no rules.txt. After git fetch --unshallow origin, a fresh run matches fetch.sh byte for byte. It also gives each run its own directory (line 22) and passes the title as data. That fixes Copilot's line-25 finding, which also has a benign trigger: GitHub's default revert title Revert "..." (for example #1189) makes the current line 25 a bash syntax error, so rules.txt is never written.

W=$(mktemp -d /tmp/review-XXXXXX); T=.claude/skills/review-pr/triage.py; echo "WORK=$W"
cat > $W/title.txt <<'EOF'
<PR title>
EOF
(
set -euo pipefail
git fetch -q origin <PR base branch>
git diff --no-color --no-ext-diff FETCH_HEAD...HEAD > $W/pr.diff
[ -s $W/pr.diff ]
git worktree add -q --detach $W/merge-target FETCH_HEAD
python3 $T rules $W/pr.diff "$(cat $W/title.txt)" . > $W/rules.txt
# the other six triage.py lines, unchanged
)
echo "evidence exit=$?"

Do not append || ... to the subshell: bash ignores set -e inside a command on the left of ||.

[jhinpan]

⚠️ [inferred] Two problems in the gate step. First, the fixed /tmp/review path checks whatever the last session wrote there (see line 22). Second, "go back to the failing step until every gate passes" has no end state. The detector runs with TimeSoftThresholdSeconds=480, TimeHardThresholdSeconds=660 and CostBudgetUsd=5 (job log). The one full worker run published in #1279 took 783 s and $5.50 on #950; it used a different model and CLI, so it is only a rough guide. When a gate cannot pass, for example because the evidence step failed, a session either hits the limit or posts anyway, and the posted review does not show which. Author must run triage.py gates "$W" . and define the end state, for example: after one retry, post the review with the red gate names on the Review line.

[Copilot]

🔵 Needs a closer look

Workflow execution and availability of the prepared history inside Copilot’s review sandbox still need end-to-end verification.

0 open findings

2 resolved since last review

• PR title interpolation enables shell command injection
• Requires unavailable Git history and network fetch instead of supported evidence

🧠 Review effort: Balanced