From da4a5b2f1910a1dddabcdce8009591a5926fd5cb Mon Sep 17 00:00:00 2001 From: marcie Date: Fri, 28 Aug 2026 18:14:57 -0500 Subject: [PATCH] framework tools/git: pr-review validates --action before provider contact (codex blocker on PR #1464) An unsupported --action previously reached platform detection (and could touch the provider) before failing with a provider-class status; comment-without-body exited 1 mid-switch. Both now fail fast with usage_error (rc 2, stderr) immediately after argument parsing. Test arms added including the zero-provider-contact assertion for invalid-action runs. --- packages/mosaic/framework/tools/git/pr-review.sh | 14 ++++++++++---- .../tools/git/test-pr-review-usage-contract.sh | 9 +++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/mosaic/framework/tools/git/pr-review.sh b/packages/mosaic/framework/tools/git/pr-review.sh index b36a6a66..0eb717ec 100755 --- a/packages/mosaic/framework/tools/git/pr-review.sh +++ b/packages/mosaic/framework/tools/git/pr-review.sh @@ -116,6 +116,14 @@ if [[ -z "$ACTION" ]]; then usage_error "Action is required (-a/--action): approve, request-changes, comment" fi +# Validate the action BEFORE any provider contact (codex review of PR #1464: +# an unsupported --action previously reached platform detection and could +# touch the provider before failing with a provider-class status). +case "$ACTION" in + approve|request-changes|comment) ;; + *) usage_error "unknown action '$ACTION': approve, request-changes, comment" ;; +esac + if [[ -n "$REPO_OVERRIDE" ]]; then # An explicit --repo is the whole point of a reviewer worktree whose origin # is nonstandard or missing (#867 convention, mirrored from pr-view.sh / @@ -715,8 +723,7 @@ if [[ "$PLATFORM" == "github" ]]; then echo "Added review comment to GitHub PR #$PR_NUMBER" ;; *) - echo "Error: Unknown action: $ACTION" - exit 1 + usage_error "unknown action: $ACTION" ;; esac elif [[ "$PLATFORM" == "gitea" ]]; then @@ -809,8 +816,7 @@ elif [[ "$PLATFORM" == "gitea" ]]; then echo "Added and verified comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)" ;; *) - echo "Error: Unknown action: $ACTION" - exit 1 + usage_error "unknown action: $ACTION" ;; esac else diff --git a/packages/mosaic/framework/tools/git/test-pr-review-usage-contract.sh b/packages/mosaic/framework/tools/git/test-pr-review-usage-contract.sh index 8841553c..1c4f8d69 100755 --- a/packages/mosaic/framework/tools/git/test-pr-review-usage-contract.sh +++ b/packages/mosaic/framework/tools/git/test-pr-review-usage-contract.sh @@ -97,6 +97,15 @@ expect_rc 2 "missing -a exits 2" -n 5 expect_stderr "Action is required" "missing -a message on stderr" expect_rc 2 "request-changes without comment exits 2" -n 5 -a request-changes expect_stderr "comment required for request-changes" "request-changes message on stderr" +expect_rc 2 "invalid action exits 2 pre-detection" -n 5 -a bogus +expect_stderr "unknown action" "invalid action message on stderr" +# Invalid-action arms must not contact any provider (validation precedes +# detect_platform): probe log empty at this point. +if [[ -s "$PROBE_LOG" ]]; then + echo "FAIL: an invalid-action arm contacted a provider:" >&2 + cat "$PROBE_LOG" >&2 + exit 1 +fi # 4. Value-less flags: rc 2 with "requires a value" on stderr. for flag in -n -a -b -c -l -r --number --action --body --comment --login --repo; do