From e8a822bff058d546c4567a76c252f36ae05dbaef Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 6 Sep 2026 12:19:50 -0400 Subject: [PATCH] docs: make the review poll fail loudly, and honor the 30s floor Copilot review round 1 on #2272: piping the count through awk made awk's status the loop's, so an auth or API failure read as a count of 0 and the job would retry forever on a poll that can never succeed. The example also used sleep 20 against the 30s remote-API floor the same PR adds to AGENTS.md. Capture the response first and exit non-zero on a gh failure; jq runs separately because --slurp cannot be combined with --jq. sleep 30. 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 | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/.claude/skills/pr-flow/SKILL.md b/.claude/skills/pr-flow/SKILL.md index 10b67201..d94feaad 100644 --- a/.claude/skills/pr-flow/SKILL.md +++ b/.claude/skills/pr-flow/SKILL.md @@ -248,14 +248,25 @@ work](../../../AGENTS.md#waiting-on-long-running-work) — and exactly where the poll belongs when one is needed. ```sh -until [ "$(gh api --paginate repos/modelcontextprotocol/inspector/pulls//reviews \ - --jq '[.[]|select(.user.login|startswith("copilot-pull-request-reviewer"))]|length' \ - | awk '{s+=$1} END{print s+0}')" -ge "$EXPECTED" ]; do sleep 20; done +EXPECTED=1 # the review COUNT you are waiting to reach — see below +while :; do + # Capture first, so a gh failure stops the loop instead of being swallowed by + # a pipeline. --slurp cannot be combined with --jq, hence the separate jq. + 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" -ge "$EXPECTED" ] && break + sleep 30 +done ``` -`EXPECTED` is the review **count** you are waiting to reach, not `1` — on round -two the first round's review is still there, so an existence check returns -immediately. Give the inline comments a further ~60s after the body lands; they +`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 arrive late (see step 8). ## 8. Respond to the review