Claude finished @Yeraze's task —— View job

PR Review

• Read changed file
• Analyze code quality, bugs, performance, security
• Post review feedback

Summary

This is a well-targeted fix for a real operational problem. The script is clean, defensive, and well-commented. Below are a few observations:

Positive Observations

• headRefOid vs headRefName for PRs — using headRefOid (the commit SHA) directly from the PR metadata is fork-safe and avoids a second API call. Good choice.
• Fallback to LAST_SHA on transient API failures is a nice resilience touch.
• Inline jq filter with headSha cleanly filters stale runs without adding a second gh call per cycle.
• Fixed failure hint — the old gh run view --log-failed --branch <branch> was invalid syntax; the two-step suggestion is correct.

Issues / Suggestions

1. jq inline string interpolation (potential injection risk, line 118)

-q ".[] | select(.headSha == \"$HEAD_SHA\") | ..."

HEAD_SHA comes from GitHub API (headRefOid / .sha) and will always be a 40-char hex string, so in practice there's no injection risk. However, it's more idiomatic and safer to pass it as a --arg:

-q --arg sha "$HEAD_SHA" '.[] | select(.headSha == $sha) | "\(.name)|\(.status)|\(.conclusion)"'

This also avoids the nested quote escaping, improving readability.

2. Early exit on first failure may miss concurrent failures (line 162–169)

The "fail fast" logic exits as soon as FAILED_NAME is set, even if multiple workflows failed in the same cycle. This is by design (documented in the header), but worth noting: users only see the first failed workflow name, not all of them. The existing behaviour is acceptable for a "stop the watch" tool, but the failure message at line 164 could iterate all failures rather than just the first one.

3. LIMIT may be insufficient for large branches (line 117)

--limit 20 (default) combined with a busy branch could mean the current SHA's runs are beyond position 20 in the list. The filter is correct but silently returns empty results, which falls through to "No CI runs yet — waiting…". This could cause indefinite waiting. A brief comment noting this edge case (or bumping the default) would be helpful.

4. REPO variable unused when PR_NUMBER is set (lines 62–66)

The guard correctly exits only when PR_NUMBER is empty and REPO is empty, so the logic is sound. Minor: the comment on line 61 says "REPO is optional there" which is accurate but could be slightly confusing. Not a bug.

5. set -euo pipefail interaction with resolve_sha || true (line 102)

The || true guard prevents set -e from killing the script when resolve_sha returns non-zero. This is correct and intentional. Just confirming it's handled properly — it is.

Verdict

Approved with minor suggestions. The fix directly addresses the reported problem (stale failed-run reporting), is safe, well-commented, and doesn't break the existing -q / branch / PR modes. The --arg sha refactor (item 1) is the only change I'd recommend before merge; the others are nice-to-haves.