docs: exit the poll loop on a jq failure too, not just a gh failure
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017nLLLTGc37hQiC8s3mNyhq Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
e8a822bff0
commit
114717ffc2
1 changed files with 10 additions and 4 deletions
|
|
@ -255,7 +255,9 @@ while :; do
|
||||||
raw=$(gh api --paginate --slurp \
|
raw=$(gh api --paginate --slurp \
|
||||||
repos/modelcontextprotocol/inspector/pulls/<N>/reviews) || {
|
repos/modelcontextprotocol/inspector/pulls/<N>/reviews) || {
|
||||||
echo "gh api failed ($?) — not retrying blind" >&2; exit 1; }
|
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
|
[ "$n" -ge "$EXPECTED" ] && break
|
||||||
sleep 30
|
sleep 30
|
||||||
done
|
done
|
||||||
|
|
@ -264,9 +266,13 @@ done
|
||||||
`EXPECTED` is the review **count** you are waiting to reach, so it is `1` only
|
`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
|
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
|
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
|
above sets. **Every step that can fail exits the loop rather than
|
||||||
count straight into `awk` would make an auth or API error read as a count of
|
retrying.** Piping the count straight into `awk` would make an auth or API error
|
||||||
`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
|
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).
|
arrive late (see step 8).
|
||||||
|
|
||||||
## 8. Respond to the review
|
## 8. Respond to the review
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue