diff --git a/.agents/skills/pr-watch/SKILL.md b/.agents/skills/pr-watch/SKILL.md index c6a447a1..b8e39f2f 100644 --- a/.agents/skills/pr-watch/SKILL.md +++ b/.agents/skills/pr-watch/SKILL.md @@ -16,8 +16,8 @@ scripts/pr-watch [--expect ] [--timeout ] [--interval /dev/null 2>&1; then pinned=$(beads_pin) running=$(bd_running_version) - if [ -n "$pinned" ] && [ -n "$running" ] && [ "$running" != "$pinned" ]; then + # Fail closed: if we can't parse either version, warn rather than silently pass + if [ -z "$pinned" ]; then + printf '[session-start] WARNING: cannot parse beads pin from flake.nix\n' >&2 + printf '[session-start] The pin check is skipped, but mutating bd commands may be unsafe.\n' >&2 + exit 2 + fi + if [ -z "$running" ]; then + printf '[session-start] WARNING: cannot parse bd version from "bd version"\n' >&2 + printf '[session-start] bd: %s\n' "$(command -v bd)" >&2 + printf '[session-start] The pin check is skipped, but mutating bd commands may be unsafe.\n' >&2 + exit 2 + fi + if [ "$running" != "$pinned" ]; then printf '\n[session-start] ===== BEADS VERSION SKEW =====\n' >&2 printf '[session-start] pinned (flake.nix): %s\n' "$pinned" >&2 printf '[session-start] running: %s (%s)\n' "$running" "$(command -v bd)" >&2 diff --git a/scripts/agent-bootstrap.sh b/scripts/agent-bootstrap.sh index 7f2e328c..35e9be8d 100755 --- a/scripts/agent-bootstrap.sh +++ b/scripts/agent-bootstrap.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Stack-agnostic agent bootstrap for this repository. # # This script prepares any coding agent (Claude Code, Cursor, etc.) to work diff --git a/scripts/pr-watch b/scripts/pr-watch index cc5759cf..10844e1e 100755 --- a/scripts/pr-watch +++ b/scripts/pr-watch @@ -14,16 +14,16 @@ usage: pr-watch [--expect ] [--timeout ] [--interval --no-reviews skip the reviews/comments printout exit 0 every check COMPLETED, none failed, and GitHub says MERGEABLE with a - state of CLEAN, UNSTABLE or HAS_HOOKS + state of CLEAN, UNSTABLE or HAS_HOOKS (and reviews fetched if --reviews) exit 1 a check failed, or the PR is not mergeable (any other known state) exit 2 the head moved during the wait, or is not --expect exit 3 timed out with checks still running, or mergeability still UNKNOWN, or still BLOCKED with every registered check green exit 4 usage or environment error exit 5 the PR is MERGED or CLOSED, so there is nothing left to wait for +exit 6 checks green/mergeable, but pr-reviews failed (fetch error, not "no reviews") -After exit 0, reviews and comments are printed (unless --no-reviews). These do -not change the exit code -- answering them is your responsibility. Reviews are +After exit 0, reviews and comments are printed (unless --no-reviews). Reviews are a separate API surface from comments; Codex/ChatGPT reviews arrive as *reviews* and are easy to miss if you only read the comment thread. EOF @@ -146,21 +146,25 @@ case "$state" in CLEAN|UNSTABLE|HAS_HOOKS) ;; *) exit 1 ;; esac # ── Reviews and comments ─────────────────────────────────────────────────── # Checks are green and mergeable. Show reviews/comments so the agent sees any # feedback that needs answering before merge. This does not change the exit -# code -- answering reviews is your responsibility. +# code for success -- answering reviews is your responsibility. However, if +# pr-reviews itself fails (usage error, network, etc.), we warn and exit 6. if [ "$SHOW_REVIEWS" -eq 1 ]; then echo "" echo "pr-watch: fetching reviews and comments (--no-reviews to skip)…" - # pr-reviews' exit code is not propagated: reviews being present or absent - # does not change whether the PR is green/mergeable. The agent must read - # and answer them, but that is a human-in-the-loop obligation, not a gate. - # Fixture mode: pass through the fixture dir if set. + reviews_exit=0 if [ -n "${PR_WATCH_FIXTURE_DIR:-}" ]; then - PR_REVIEWS_FIXTURE_DIR="$PR_WATCH_FIXTURE_DIR" "$SCRIPT_DIR/pr-reviews" "$PR" || true + PR_REVIEWS_FIXTURE_DIR="$PR_WATCH_FIXTURE_DIR" "$SCRIPT_DIR/pr-reviews" "$PR" || reviews_exit=$? else - "$SCRIPT_DIR/pr-reviews" "$PR" || true + "$SCRIPT_DIR/pr-reviews" "$PR" || reviews_exit=$? fi echo "" - echo "pr-watch: reviews shown above. Answer substantive feedback before merging." + if [ "$reviews_exit" -eq 0 ]; then + echo "pr-watch: reviews shown above. Answer substantive feedback before merging." + else + echo "pr-watch: WARNING: pr-reviews failed (exit $reviews_exit). Reviews may not be shown." >&2 + echo "pr-watch: Run './scripts/pr-reviews $PR' manually to fetch reviews." >&2 + exit 6 + fi fi exit 0 diff --git a/scripts/pr-watch-test.sh b/scripts/pr-watch-test.sh index 3e64a714..685a034f 100755 --- a/scripts/pr-watch-test.sh +++ b/scripts/pr-watch-test.sh @@ -99,6 +99,18 @@ else ok "--no-reviews actually skips review output" fi +# pr-reviews failure: if pr-reviews fails, pr-watch exits 6 (not 0) +# Create a green fixture with MALFORMED review fixtures so pr-reviews fails +mk "$work/green-bad-reviews" last.json <<'EOF' +{"headRefOid":"aaaaaaaaaaaa","mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","statusCheckRollup":[ + {"name":"build","status":"COMPLETED","conclusion":"SUCCESS"}]} +EOF +# Create malformed JSON that will cause jq to fail in pr-reviews +echo 'NOT VALID JSON' > "$work/green-bad-reviews/last.pr.json" +echo '[]' > "$work/green-bad-reviews/last.inline.json" +echo '[]' > "$work/green-bad-reviews/last.issue.json" +run_case "pr-reviews failure exits 6" 6 "$work/green-bad-reviews" + mk "$work/red" last.json <<'EOF' {"headRefOid":"aaaaaaaaaaaa","mergeable":"MERGEABLE","mergeStateStatus":"UNSTABLE","statusCheckRollup":[ {"name":"build","status":"COMPLETED","conclusion":"FAILURE"}]}