From 114717ffc26f69da9e96139fda2ef0ee8320b417 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 6 Sep 2026 12:26:52 -0400 Subject: [PATCH] docs: exit the poll loop on a jq failure too, not just a gh failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review round 2 on #2272: a jq failure on an unexpected shape leaves n empty, `[ "" -ge 1 ]` then exits non-zero, break never fires, and the loop sleeps and retries forever — the same unbounded wait as the awk pipe, reached from the other end. Verified locally: jq exits 5 and the test exits 1. Propagate the jq failure, guard that n is numeric, and generalize the prose to say every step which can fail exits rather than retries. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017nLLLTGc37hQiC8s3mNyhq Signed-off-by: cliffhall --- .claude/skills/pr-flow/SKILL.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.claude/skills/pr-flow/SKILL.md b/.claude/skills/pr-flow/SKILL.md index d94feaad..1cc421f7 100644 --- a/.claude/skills/pr-flow/SKILL.md +++ b/.claude/skills/pr-flow/SKILL.md @@ -255,7 +255,9 @@ while :; do raw=$(gh api --paginate --slurp \ repos/modelcontextprotocol/inspector/pulls//reviews) || { echo "gh api failed ($?) — not retrying blind" >&2; exit 1; } - n=$(jq '[.[][] | select(.user.login | startswith("copilot-pull-request-reviewer"))] | length' <<<"$raw") + n=$(jq '[.[][] | select(.user.login | startswith("copilot-pull-request-reviewer"))] | length' <<<"$raw") || { + echo "jq failed ($?) on an unexpected response shape" >&2; exit 1; } + case $n in '' | *[!0-9]*) echo "not a count: '$n'" >&2; exit 1 ;; esac [ "$n" -ge "$EXPECTED" ] && break sleep 30 done @@ -264,9 +266,13 @@ done `EXPECTED` is the review **count** you are waiting to reach, so it is `1` only on the first round — on round two the first round's review is still there and an existence check returns immediately. `sleep 30` is the remote-API floor the rule -above sets. The loop **exits on a `gh` failure rather than retrying**: piping the -count straight into `awk` would make an auth or API error read as a count of -`0`, and the job would then wait forever on a poll that can never succeed. Give the inline comments a further ~60s after the body lands; they +above sets. **Every step that can fail exits the loop rather than +retrying.** Piping the count straight into `awk` would make an auth or API error +read as a count of `0`; and a `jq` failure on an unexpected shape leaves `n` +empty, whereupon `[ "" -ge 1 ]` exits non-zero, `break` never fires, and the job +sleeps and retries forever — the same unbounded wait, reached from the other +end. A background task that can never succeed is worse than one that never +started, because it looks like progress. Give the inline comments a further ~60s after the body lands; they arrive late (see step 8). ## 8. Respond to the review