dispatcher.sh: || true suppresses errors in get_pr_merger / get_pr_reviews, making error handlers dead code #189

New issue

Closed

opened 2026-04-03 13:00:57 +00:00 by dev-bot · 0 comments

dev-bot commented

2026-04-03 13:00:57 +00:00

Collaborator

Flagged by AI reviewer in PR #188.

Problem

Both get_pr_merger and get_pr_reviews end with || true, meaning they always return exit code 0. The callers pattern-match on the exit code:

reviews_json=$(get_pr_reviews "$pr_number") || {
  log "WARNING: Could not fetch reviews..."
  return 1
}

Because the function always succeeds, this || { ... } block is unreachable dead code. If the underlying curl call fails, reviews_json is empty. The subsequent jq 'length // 0' on empty input produces empty output; [ "" -eq 0 ] then throws a bash integer error instead of the intended warning log. The fallback to the merger check still occurs, but via an uncontrolled error path.

Same pre-existing issue in get_pr_merger.

Fix

Remove || true from both helpers so curl failures propagate, letting the || { log ...; return 1; } handlers fire correctly.

Auto-created from AI review

Acceptance criteria

get_pr_merger does not end with || true
get_pr_reviews does not end with || true
When curl fails inside get_pr_merger, the caller || { ... } error handler fires
When curl fails inside get_pr_reviews, the caller || { ... } error handler fires
ShellCheck passes on docker/edge/dispatcher.sh

Affected files

docker/edge/dispatcher.sh — get_pr_merger() and get_pr_reviews() functions

Flagged by AI reviewer in PR #188. ## Problem Both `get_pr_merger` and `get_pr_reviews` end with `|| true`, meaning they always return exit code 0. The callers pattern-match on the exit code: ```bash reviews_json=$(get_pr_reviews "$pr_number") || { log "WARNING: Could not fetch reviews..." return 1 } ``` Because the function always succeeds, this `|| { ... }` block is unreachable dead code. If the underlying `curl` call fails, `reviews_json` is empty. The subsequent `jq 'length // 0'` on empty input produces empty output; `[ "" -eq 0 ]` then throws a bash integer error instead of the intended warning log. The fallback to the merger check still occurs, but via an uncontrolled error path. Same pre-existing issue in `get_pr_merger`. ## Fix Remove `|| true` from both helpers so curl failures propagate, letting the `|| { log ...; return 1; }` handlers fire correctly. --- *Auto-created from AI review* ## Acceptance criteria - [ ] `get_pr_merger` does not end with `|| true` - [ ] `get_pr_reviews` does not end with `|| true` - [ ] When curl fails inside `get_pr_merger`, the caller `|| { ... }` error handler fires - [ ] When curl fails inside `get_pr_reviews`, the caller `|| { ... }` error handler fires - [ ] ShellCheck passes on `docker/edge/dispatcher.sh` ## Affected files - `docker/edge/dispatcher.sh` — `get_pr_merger()` and `get_pr_reviews()` functions