Skip to content

chore(scripts): add scripts/request-review.sh to formalize fork-PR reviewer pings#497

Closed
Senorespecial wants to merge 1 commit into
MERIDIAN-CITY:mainfrom
Senorespecial:chore/scripts-fork-pr-review-request
Closed

chore(scripts): add scripts/request-review.sh to formalize fork-PR reviewer pings#497
Senorespecial wants to merge 1 commit into
MERIDIAN-CITY:mainfrom
Senorespecial:chore/scripts-fork-pr-review-request

Conversation

@Senorespecial

@Senorespecial Senorespecial commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

What

Adds scripts/request-review.sh (the fork-PR + token-403 reviewer-request workaround that was hand-rolled across PRs #491 and #495) plus a top-level Makefile recipe request-review and a CONTRIBUTING.md one-liner exposing it, so future contributors find the script on day one without rediscovering the 403 dance.

Files

  • scripts/request-review.sh (~178 lines, executable) — main artifact.
  • Makefile — top-level umbrella make request-review ARGS="..." recipe delegating to the script; includes a make help so contributors can discover available recipes.
  • CONTRIBUTING.md — one-line section pointing future contributors at make request-review.

Why

A fork-PR contributor bot can't gh pr edit --add-reviewer — it returns Resource not accessible by integration (same constraint observed on PR #491). Until now each fork-PR hand-crafted the workaround in a comment. Codifying it in a tagged script + wiring it into the repo's tooling surface (Makefile + CONTRIBUTING one-liner) means any future chore(ci): ... PR can just run:

make request-review ARGS="<PR_NUMBER> <REVIEWER_LOGIN> [<REVIEWER_LOGIN> ...]"

without re-deriving the 403 dance.

Flags

scripts/request-review.sh exposes:

  • --dry-run — echo what each gh call would do without making any GitHub API mutations; exits 4.
  • --report-json — emit one JSON summary line to stdout on every exit (suitable for jq . capture).
  • --help | -h — print the header docstring and exit 0.

Exit codes: 0 direct success, 1 bad args, 2 fallback comment posted, 3 idempotent skip, 4 dry-run end.

Polish rounds applied

  • Sentinel-driven --help (no hardcoded line range).
  • Split exit codes (3 = idempotent_skip, 4 = dry_run_noop).
  • >/dev/null on Steps 1/2 (was 2>&1 >&2, leaked run_gh's --dry-run echoes into stdout and contaminated JSON contract).
  • Pre-scan for --report-json in $@ BEFORE the main flag-parse loop (so --typo --report-json emits JSON instead of 0 stdout lines).
  • Pre-scan uses case style (consistency with main loop's flag-dispatch arms).
  • Function-definition ordering: emit_json defined BEFORE flag-parse loop (bash does NOT hoist function defs).
  • Defensive Python heredoc: _int_or_none for empty PR_NUMBER / exit_code (called from bad_args / unknown-flag early-exit paths).
  • Step 3 cross-reference comment: >&2 (preserve gh stdout for operator visibility) vs Steps 1/2 >/dev/null (suppress gh stdout to protect JSON contract).
  • ...and wiring it all into the repo's tooling surface (Makefile + CONTRIBUTING one-liner) so future contributors find it on day one.

Sanity coverage

bash -n clean. make -n help and make -n request-review ARGS="..." echo without invoking. make request-review ARGS="--dry-run --report-json 495 Qoder-Undefined" runs the script end-to-end via the Makefile and produces correct JSON-on-stdout + dry-run notices on stderr. Manual test matrix covers: --help, no-args, --report-json --dry-run, --dry-run --report-json, --typo --report-json, --report-json --typo, multiple unknowns + --report-json, --typo --dry-run no-JSON, --report-json --typo --dry-run.

@Senorespecial Senorespecial force-pushed the chore/scripts-fork-pr-review-request branch 2 times, most recently from 4a735f3 to e32b257 Compare June 20, 2026 14:22
…viewer pings

When this repo is opened from a fork (e.g. Senorespecial/Meridians-verse -> MERIDIAN-CITY/Meridians-verse), the fork-scoped gh token returns 403 on the direct paths that formally request reviewers (gh pr edit --add-reviewer returns GraphQL requestReviewsByLogin permission error; REST POST /pulls/:n/requested_reviewers returns 403).

This script encapsulates the workaround that has been used manually on PR MERIDIAN-CITY#491 and PR MERIDIAN-CITY#495: try the direct paths first (works from upstream-write-token env), and on 403 fall back to posting a PR comment containing a ready-to-paste maintainer command plus a TL;DR for the human reviewer. The fallback comment is dedup-aware via a single DEDUP_MARKER shell constant used both for grep-marker-matching and as the comment-header text, so re-running the script on a PR with an existing fallback comment exits 3 without reposting.

Also includes --help (prints the header docstring), --dry-run (logs what each gh call would do without side effects, exits 3 with no mutation), REQUEST_REVIEW_REPO envvar override, and documented exit codes (0/1/2/3) for downstream automation. No production code change; scripts/ is a new directory at the repo root.
@Senorespecial Senorespecial force-pushed the chore/scripts-fork-pr-review-request branch from e32b257 to 78ac295 Compare June 20, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants