diff --git a/.gitignore b/.gitignore index 122d2e7e..363994d8 100644 --- a/.gitignore +++ b/.gitignore @@ -7,7 +7,14 @@ dist/ .env .agentworkforce/ .cargo-home/ -.review-target + +# .review-target/ carries the PR the swarm is reviewing. It IS uploaded to +# the cloud sandbox (that is the point), so it must NOT be gitignored — the +# earlier `.review-target` entry silently dropped the file from `git ls-files` +# and every cloud swarm run hit `FETCH_FAILED: .review-target missing`. +# scripts/review-swarm-prepare.sh writes and stages the files under it; the +# `git add` there needs no `-f`. Commit them to your review branch and let +# the launcher upload them like any other tracked file. # Toolchains materialize inside the workspace in a cloud sandbox and must never # be committed or delivered. Run f18ec684's patch carried .rustup-home/ files; diff --git a/scripts/review-swarm-prepare.sh b/scripts/review-swarm-prepare.sh new file mode 100755 index 00000000..c4cb221d --- /dev/null +++ b/scripts/review-swarm-prepare.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Prepare the .review-target/ packet the cloud review swarm consumes. +# +# The cloud sandbox has NO GitHub token and NO git remote — the earlier +# workflow tried to run `gh pr view/diff` from inside the sandbox and got +# 0-line diffs because gh was unauthenticated ("To get started with GitHub +# CLI, please run: gh auth login"). This helper runs on the LAUNCHING host +# (GitHub Actions runner, or your laptop), where `gh` IS authenticated, +# and drops the diff + metadata into `.review-target/` so `agent-relay +# cloud run` uploads them like any other tracked file. +# +# Files written (all consumed by workflows/review-swarm.yaml): +# .review-target/pr-number one-line PR number +# .review-target/pr.diff output of `gh pr diff ` +# .review-target/pr.json output of `gh pr view --json ...` +# +# Usage: +# scripts/review-swarm-prepare.sh +# Then: +# agent-relay cloud run workflows/review-swarm.yaml +set -euo pipefail + +pr="${1:-}" +if [[ -z "$pr" || ! "$pr" =~ ^[0-9]+$ ]]; then + echo "usage: $0 " >&2 + exit 2 +fi + +if ! command -v gh >/dev/null 2>&1; then + echo "PREPARE_FAILED: gh not on PATH — the launching host must have GitHub CLI." >&2 + exit 3 +fi + +if ! gh auth status >/dev/null 2>&1; then + echo "PREPARE_FAILED: gh is unauthenticated on this host." >&2 + echo " Either run \`gh auth login\` or export GH_TOKEN (CI does the latter)." >&2 + exit 4 +fi + +mkdir -p .review-target +printf '%s\n' "$pr" > .review-target/pr-number + +# `gh pr diff` and `gh pr view` both fail loudly on missing PRs — do NOT +# swallow their exit code. The earlier workflow ran `gh` under `set -u` only +# and reported FETCHED with 0 lines when gh failed. +gh pr diff "$pr" > .review-target/pr.diff +gh pr view "$pr" --json number,headRefName,baseRefName,title,url,author \ + > .review-target/pr.json + +diff_lines=$(wc -l < .review-target/pr.diff | tr -d ' ') +if [[ "$diff_lines" -eq 0 ]]; then + echo "PREPARE_FAILED: gh pr diff $pr produced 0 lines — refusing to stage a nothing-review." >&2 + exit 5 +fi + +# Stage so `agent-relay cloud run` (which uploads `git ls-files`) picks them +# up. `.review-target/` is no longer in .gitignore, so plain `git add` works. +git add .review-target/pr-number .review-target/pr.diff .review-target/pr.json + +echo "PREPARED: PR #$pr, $diff_lines diff lines, staged for upload" diff --git a/workflows/review-swarm.yaml b/workflows/review-swarm.yaml index 6bd1a73c..7ea732d0 100644 --- a/workflows/review-swarm.yaml +++ b/workflows/review-swarm.yaml @@ -7,7 +7,13 @@ description: > trial expired, both reporting SUCCESS. Our own review must not depend on someone else's quota. - Invoke with PR_NUMBER set. Each lens persists its own transcript to + Invoke by first running `scripts/review-swarm-prepare.sh ` on the + LAUNCHING host (which has `gh` auth). That fetches the PR diff + metadata + into `.review-target/` and stages the files so `agent-relay cloud run` + uploads them with the rest of the tree. Each lens then reads the diff from + the workspace — never from `/tmp` (per-step sandboxes do NOT share it) and + never through `gh` (the cloud sandbox has NO GitHub token and NO remote, + same as drive-cloud.yaml). Each lens persists its own transcript to ops/reviews/; the aggregate step fails the run if ANY lens rejects, so a single honest refusal blocks the merge. @@ -39,18 +45,34 @@ workflows: - name: fetch type: deterministic command: | - # Deterministic steps do not inherit the launching shell's env, so the - # target is read from a file the operator writes before the run: - # echo 8 > .review-target - set -u - if [ ! -f .review-target ]; then - echo "FETCH_FAILED: .review-target missing — write the PR number to it first"; exit 1 + # Validate the pre-fetched review packet. The launching host runs + # scripts/review-swarm-prepare.sh , which fetches the diff and + # metadata (it has `gh` auth) and stages `.review-target/pr.diff`, + # `.review-target/pr.json`, `.review-target/pr-number`. The cloud + # sandbox has NEITHER a GitHub token NOR a git remote (same as + # drive-cloud.yaml), so re-fetching here is not an option — the + # earlier `gh pr view/diff` call silently returned 0-line diffs + # (run 84874db1: "To get started with GitHub CLI, please run: gh + # auth login"). Files under `/tmp` also don't cross the per-step + # sandbox boundary, so the diff has to live in the workspace. + set -eu + for required in .review-target/pr-number .review-target/pr.diff .review-target/pr.json; do + if [ ! -s "$required" ]; then + echo "FETCH_FAILED: $required missing or empty" >&2 + echo " Run scripts/review-swarm-prepare.sh on the launching host" >&2 + echo " and commit (or force-add) the .review-target/ files before" >&2 + echo " \`agent-relay cloud run workflows/review-swarm.yaml\`." >&2 + exit 1 + fi + done + PR=$(tr -dc '0-9' < .review-target/pr-number) + [ -n "$PR" ] || { echo "FETCH_FAILED: .review-target/pr-number holds no PR number" >&2; exit 1; } + diff_lines=$(wc -l < .review-target/pr.diff | tr -d ' ') + if [ "$diff_lines" -eq 0 ]; then + echo "FETCH_FAILED: .review-target/pr.diff has 0 lines — refusing to review nothing" >&2 + exit 1 fi - PR=$(tr -dc '0-9' < .review-target) - [ -n "$PR" ] || { echo "FETCH_FAILED: .review-target holds no PR number"; exit 1; } - gh pr view "$PR" --json headRefName,title,url > /tmp/pr-$PR.json - gh pr diff "$PR" > /tmp/pr-$PR.diff - echo "target PR #$PR, $(wc -l < /tmp/pr-$PR.diff) diff lines" + echo "target PR #$PR, $diff_lines diff lines" echo FETCHED - name: lens-maintainability @@ -58,16 +80,20 @@ workflows: agent: maintainability dependsOn: [fetch] task: | - Review the PR whose number is in .review-target (diff at - /tmp/pr-.diff, metadata at /tmp/pr-.json) through ONE lens: maintainability. + Review the PR whose number is in .review-target/pr-number + (diff at .review-target/pr.diff, metadata at .review-target/pr.json) + through ONE lens: maintainability. Ask: could a stranger read this in six months and change it safely? Name unclear boundaries, implicit contracts, missing failure handling, comments that assert what the code does not do, and tests that would not fail if the behavior broke. Read AGENTS.md and docs/RFC-0001-everything-is-a-relayflow.md first. Write your complete review to - ops/reviews/$(date +%Y%m%d-%H%M)-pr$(cat .review-target)-maintainability.md - and `git add` it. End your output with REVIEW_PASSED or REVIEW_FAILED. + ops/reviews/$(date +%Y%m%d-%H%M)-pr$(cat .review-target/pr-number)-maintainability.md + (create the file — do not `git add`; the persist step does that once + the orchestrator has re-collected every lens's workspace, since a + per-step sandbox's stage does not survive across steps). + End your output with REVIEW_PASSED or REVIEW_FAILED. verification: type: output_contains value: "REVIEW_" @@ -79,16 +105,20 @@ workflows: agent: history dependsOn: [fetch] task: | - Review the PR whose number is in .review-target (diff at /tmp/pr-.diff) through ONE - lens: does this change fit the story of the code? + Review the PR whose number is in .review-target/pr-number + (diff at .review-target/pr.diff) through ONE lens: does this change + fit the story of the code? Run `git log --oneline -40` and read ops/DRIVE-LOG.md, ops/NEXT.md and ops/DIRECTIVES.md if present. Ask: does it repeat a mistake the log already records? Does it contradict a settled decision in RFC-0001? Does it reintroduce something a previous commit deliberately removed? Does the commit message tell the truth about the diff? Write your complete review to - ops/reviews/$(date +%Y%m%d-%H%M)-pr$(cat .review-target)-history.md and - `git add` it. End your output with REVIEW_PASSED or REVIEW_FAILED. + ops/reviews/$(date +%Y%m%d-%H%M)-pr$(cat .review-target/pr-number)-history.md + (create the file — do not `git add`; the persist step does that once + the orchestrator has re-collected every lens's workspace, since a + per-step sandbox's stage does not survive across steps). + End your output with REVIEW_PASSED or REVIEW_FAILED. verification: type: output_contains value: "REVIEW_" @@ -100,15 +130,19 @@ workflows: agent: structure dependsOn: [fetch] task: | - Review the PR whose number is in .review-target (diff at /tmp/pr-.diff) through ONE - lens: structure. Boundaries, coupling, file size and single purpose, + Review the PR whose number is in .review-target/pr-number + (diff at .review-target/pr.diff) through ONE lens: structure. + Boundaries, coupling, file size and single purpose, whether the shape matches RFC-0001 (closed kernel vocabulary, helpers over primitives, fail-closed, completionReason discipline) and AGENTS.md. Name anything that puts product logic in the kernel, adds a primitive instead of a helper, or grows a file past its purpose. Write your complete review to - ops/reviews/$(date +%Y%m%d-%H%M)-pr$(cat .review-target)-structure.md and - `git add` it. End your output with REVIEW_PASSED or REVIEW_FAILED. + ops/reviews/$(date +%Y%m%d-%H%M)-pr$(cat .review-target/pr-number)-structure.md + (create the file — do not `git add`; the persist step does that once + the orchestrator has re-collected every lens's workspace, since a + per-step sandbox's stage does not survive across steps). + End your output with REVIEW_PASSED or REVIEW_FAILED. verification: type: output_contains value: "REVIEW_" @@ -119,14 +153,24 @@ workflows: type: deterministic dependsOn: [lens-maintainability, lens-history, lens-structure] command: | - # Lens verdicts are evidence even when the aggregate rejects. Commit - # exactly the review files the lenses staged before any later reset - # can destroy them. - set -u - PR=$(tr -dc '0-9' < .review-target 2>/dev/null) - if ! git diff --cached --quiet -- ops/reviews/; then - git commit -m "ops(review): persist PR #${PR} swarm transcripts" -- ops/reviews/ + # Lens verdicts are evidence even when the aggregate rejects. Stage + # and commit every transcript that landed in the workspace before + # any later reset can destroy it. Do the `git add` here — a per-step + # sandbox is the wrong place to stage, because staged state is not + # part of the orchestrator archive that seeds the next step. + set -eu + PR=$(tr -dc '0-9' < .review-target/pr-number 2>/dev/null || true) + mkdir -p ops/reviews + git add -- ops/reviews/ 2>/dev/null || true + if ! git diff --cached --quiet -- ops/reviews/ 2>/dev/null; then + git -c user.email=review-swarm@relayflows.local \ + -c user.name='review-swarm' \ + commit -m "ops(review): persist PR #${PR:-unknown} swarm transcripts" \ + -- ops/reviews/ || echo "PERSIST_NOTE: commit refused" + else + echo "PERSIST_NOTE: no new transcripts to commit" fi + timeoutMs: 120000 - name: aggregate type: deterministic @@ -134,21 +178,34 @@ workflows: command: | # Any single honest refusal blocks the merge. A missing transcript is # a refusal too: an unpersisted verdict is not evidence. - set -u - PR=$(tr -dc '0-9' < .review-target 2>/dev/null) + # + # Sort lexicographically by filename (YYYYMMDD-HHMM prefix), not by + # mtime. Fresh checkouts give transcripts uniform mtimes and + # mtime-sort picked stale verdicts (learned in + # .github/workflows/scripts/swarm-post.sh; same rule applies here). + # + # Verdict = LAST non-empty line's token, not a whole-file grep. + # A whole-file grep of REVIEW_FAILED misclassifies a passing review + # that quotes the token in prose. + set -eu + PR=$(tr -dc '0-9' < .review-target/pr-number 2>/dev/null || true) + if [ -z "$PR" ]; then + echo "SWARM_FAILED: .review-target/pr-number missing at aggregate" >&2 + exit 1 + fi fail=0 for lens in maintainability history structure; do - f=$(ls -t ops/reviews/*-pr${PR}-${lens}.md 2>/dev/null | head -1) - if [ -z "$f" ]; then + f=$(printf '%s\n' ops/reviews/*-pr${PR}-${lens}.md 2>/dev/null \ + | grep -v '\*' | sort | tail -1) + if [ -z "${f:-}" ] || [ ! -f "$f" ]; then echo "SWARM_FAILED: $lens produced no transcript"; fail=1; continue fi - if grep -q "REVIEW_FAILED" "$f"; then - echo "SWARM_FAILED: $lens rejected — see $f"; fail=1 - elif grep -q "REVIEW_PASSED" "$f"; then - echo "ok: $lens passed ($f)" - else - echo "SWARM_FAILED: $lens transcript carries no verdict ($f)"; fail=1 - fi + last=$(awk 'NF { last=$0 } END { print last }' "$f") + case "$last" in + *REVIEW_PASSED*) echo "ok: $lens passed ($f)" ;; + *REVIEW_FAILED*) echo "SWARM_FAILED: $lens rejected — see $f"; fail=1 ;; + *) echo "SWARM_FAILED: $lens transcript carries no verdict ($f)"; fail=1 ;; + esac done - [ $fail -eq 0 ] && echo SWARM_PASSED || exit 1 + [ $fail -eq 0 ] && echo SWARM_PASSED || { echo SWARM_FAILED; exit 1; } timeoutMs: 120000