From ca91586222d8d4e73d2de0a47fcedc30aae64dd3 Mon Sep 17 00:00:00 2001 From: marcie Date: Fri, 28 Aug 2026 18:17:30 -0500 Subject: [PATCH] framework tools/git: pr-review body-required actions fail fast (codex suggestion on PR #1464) request-changes and comment both validate their required body immediately after argument parsing (rc 2, stderr, zero provider contact). The repo-host-override suite's action-message expectation updated to the new wording; suite green. --- packages/mosaic/framework/tools/git/pr-review.sh | 7 +++++++ .../tools/git/test-pr-review-repo-host-override.sh | 2 +- .../framework/tools/git/test-pr-review-usage-contract.sh | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/mosaic/framework/tools/git/pr-review.sh b/packages/mosaic/framework/tools/git/pr-review.sh index 0eb717ec..c5e9028b 100755 --- a/packages/mosaic/framework/tools/git/pr-review.sh +++ b/packages/mosaic/framework/tools/git/pr-review.sh @@ -124,6 +124,13 @@ case "$ACTION" in *) usage_error "unknown action '$ACTION': approve, request-changes, comment" ;; esac +# Body-required actions fail fast too (codex follow-up on PR #1464): +# request-changes and comment both require a body; validate before any +# provider contact. +if [[ ( "$ACTION" == "request-changes" || "$ACTION" == "comment" ) && -z "$COMMENT" ]]; then + usage_error "comment required for $ACTION (-b/--body)" +fi + 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 / diff --git a/packages/mosaic/framework/tools/git/test-pr-review-repo-host-override.sh b/packages/mosaic/framework/tools/git/test-pr-review-repo-host-override.sh index f5699ce4..789fa410 100755 --- a/packages/mosaic/framework/tools/git/test-pr-review-repo-host-override.sh +++ b/packages/mosaic/framework/tools/git/test-pr-review-repo-host-override.sh @@ -218,7 +218,7 @@ if grep -q 'Unknown option' "$OUTPUT_FILE"; then cat "$OUTPUT_FILE" >&2 exit 1 fi -grep -q 'Unknown action: bogus-action' "$OUTPUT_FILE" +grep -q "unknown action 'bogus-action'" "$OUTPUT_FILE" # --- Case 2: -h/--help documents both overrides. HELP_TEXT="$("$SCRIPT_DIR/pr-review.sh" -h)" 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 1c4f8d69..a66b2075 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,8 @@ 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 "comment without body exits 2 pre-detection" -n 5 -a comment +expect_stderr "comment required for comment" "comment-without-body 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