From d56e6a04c49f7855485b33c4ae0677a16b0ce9ac Mon Sep 17 00:00:00 2001 From: Alexey Samoylov Date: Mon, 22 Jun 2026 21:22:35 +0600 Subject: [PATCH] feat: simplify review feedback inputs --- README.md | 7 +-- action.yml | 9 +--- scripts/run-diffpal-review.sh | 26 ++++++++-- scripts/smoke-test.sh | 91 ++++++++++++++++++++++++++++------- 4 files changed, 99 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index ee4faee..1c302b8 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,6 @@ jobs: permissions: contents: read pull-requests: write - checks: write steps: - uses: actions/checkout@v6 with: @@ -71,7 +70,7 @@ jobs: head: ${{ github.event.pull_request.head.sha }} repo: ${{ github.repository }} review-id: github-pr-${{ github.event.pull_request.number }} - feedback: balanced + feedback: review gate: true env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -113,8 +112,7 @@ they configure. | --- | --- | --- | | `gate` | `false` | `--gate` | | `block-on` | `high` | `--block-on` | -| `mode` | empty | `--mode` | -| `feedback` | `balanced` | `--feedback` | +| `feedback` | `review` | `--feedback` | | `summary-overview` | `true` | `--summary-overview` | | `review-channel` | `diffpal` | `--review-channel` | | `out` | empty | `--out` | @@ -157,7 +155,6 @@ Use these permissions when publishing PR feedback: permissions: contents: read pull-requests: write - checks: write ``` Keep provider secrets available only to trusted workflows. For pull requests, diff --git a/action.yml b/action.yml index ccdea77..cb85b19 100644 --- a/action.yml +++ b/action.yml @@ -39,14 +39,10 @@ inputs: description: Return non-zero when blocking findings exist. required: false default: "false" - mode: - description: Comma-separated GitHub publish modes. - required: false - default: "" feedback: description: Review feedback shape passed to diffpal review github. Pin diffpal-version for reproducible behavior across CLI releases. required: false - default: balanced + default: review summary-overview: description: Include a semantic change overview in review summaries. required: false @@ -64,7 +60,7 @@ inputs: required: false default: "" review-channel: - description: GitHub publishing channel for check runs and summary comments. + description: GitHub publishing channel for summary comments. required: false default: diffpal language: @@ -99,7 +95,6 @@ runs: INPUT_PROFILE: ${{ inputs.profile }} INPUT_BLOCK_ON: ${{ inputs.block-on }} INPUT_GATE: ${{ inputs.gate }} - INPUT_MODE: ${{ inputs.mode }} INPUT_FEEDBACK: ${{ inputs.feedback }} INPUT_SUMMARY_OVERVIEW: ${{ inputs.summary-overview }} INPUT_OUT: ${{ inputs.out }} diff --git a/scripts/run-diffpal-review.sh b/scripts/run-diffpal-review.sh index 38ecb7a..1829a9c 100755 --- a/scripts/run-diffpal-review.sh +++ b/scripts/run-diffpal-review.sh @@ -28,6 +28,10 @@ truthy() { esac } +is_review_blocked_failure() { + [[ "$1" == "10" ]] +} + require_input "base" "${INPUT_BASE:-}" require_input "head" "${INPUT_HEAD:-}" @@ -48,10 +52,7 @@ fi if truthy "${INPUT_GATE:-false}"; then argv+=(--gate) fi -if [[ -n "${INPUT_MODE:-}" ]]; then - argv+=(--mode "$INPUT_MODE") -fi -if [[ -z "${INPUT_MODE:-}" && -n "${INPUT_FEEDBACK:-}" ]]; then +if [[ -n "${INPUT_FEEDBACK:-}" ]]; then argv+=(--feedback "$INPUT_FEEDBACK") fi if ! truthy "${INPUT_SUMMARY_OVERVIEW:-true}"; then @@ -79,4 +80,19 @@ if [[ -n "${INPUT_INSTRUCTIONS_FILE:-}" ]]; then argv+=(--instructions-file "$INPUT_INSTRUCTIONS_FILE") fi -exec "${argv[@]}" +stdout_file="$(mktemp)" +stderr_file="$(mktemp)" +trap 'rm -f "$stdout_file" "$stderr_file"' EXIT + +if "${argv[@]}" > >(tee "$stdout_file") 2> >(tee "$stderr_file" >&2); then + exit 0 +else + code=$? +fi + +if truthy "${INPUT_GATE:-false}" && is_review_blocked_failure "$code"; then + threshold="${INPUT_BLOCK_ON:-high}" + printf '::error::DiffPal code review found blocking issues at or above the %s threshold.\n' "$threshold" +fi + +exit "$code" diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 645f87a..3519e7e 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -5,11 +5,21 @@ repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" make_fake_diffpal() { local bin_dir="$1" + local exit_code="${2:-0}" + local stdout_text="${3:-}" + local stderr_text="${4:-}" cat > "$bin_dir/diffpal" <<'SCRIPT' #!/usr/bin/env bash printf '%s\n' "$@" > "$DIFFPAL_ARGV_FILE" +printf '%b' "$DIFFPAL_STDOUT_TEXT" +printf '%b' "$DIFFPAL_STDERR_TEXT" >&2 +exit "$DIFFPAL_EXIT_CODE" SCRIPT chmod +x "$bin_dir/diffpal" + + export DIFFPAL_EXIT_CODE="$exit_code" + export DIFFPAL_STDOUT_TEXT="$stdout_text" + export DIFFPAL_STDERR_TEXT="$stderr_text" } assert_contains() { @@ -34,6 +44,17 @@ assert_not_contains() { fi } +assert_text_contains() { + local file="$1" + local expected="$2" + if ! grep -Fq -- "$expected" "$file"; then + echo "expected $file to contain text: $expected" >&2 + echo "--- $file ---" >&2 + cat "$file" >&2 + exit 1 + fi +} + test_installer_installs_requested_version() { local dir="$1" local fake_bin="$dir/fake-bin" @@ -132,10 +153,10 @@ test_wrapper_uses_installed_binary_and_feedback() { assert_contains "$argv" "diffpal-dev" } -test_wrapper_mode_overrides_feedback() { +test_wrapper_can_disable_summary_overview() { local dir="$1" - local bin_dir="$dir/path-bin" - local argv="$dir/argv-mode" + local bin_dir="$dir/overview-bin" + local argv="$dir/argv-overview" mkdir -p "$bin_dir" make_fake_diffpal "$bin_dir" @@ -144,32 +165,67 @@ test_wrapper_mode_overrides_feedback() { DIFFPAL_BIN=diffpal \ INPUT_BASE=base-sha \ INPUT_HEAD=head-sha \ - INPUT_MODE=check-run \ - INPUT_FEEDBACK=summary \ + INPUT_SUMMARY_OVERVIEW=false \ "$repo_root/scripts/run-diffpal-review.sh" - assert_contains "$argv" "--mode" - assert_contains "$argv" "check-run" - assert_not_contains "$argv" "--feedback" - assert_not_contains "$argv" "summary" + assert_contains "$argv" "--summary-overview=false" } -test_wrapper_can_disable_summary_overview() { +test_wrapper_prints_gate_annotation_for_blocked_exit_code() { local dir="$1" - local bin_dir="$dir/overview-bin" - local argv="$dir/argv-overview" + local bin_dir="$dir/gate-bin" + local argv="$dir/argv-gate" + local log="$dir/log-gate" mkdir -p "$bin_dir" - make_fake_diffpal "$bin_dir" + make_fake_diffpal "$bin_dir" 10 "" "review blocked: blocking findings detected: 2\n" + set +e PATH="$bin_dir:$PATH" \ DIFFPAL_ARGV_FILE="$argv" \ DIFFPAL_BIN=diffpal \ INPUT_BASE=base-sha \ INPUT_HEAD=head-sha \ - INPUT_SUMMARY_OVERVIEW=false \ - "$repo_root/scripts/run-diffpal-review.sh" + INPUT_GATE=true \ + INPUT_BLOCK_ON=high \ + "$repo_root/scripts/run-diffpal-review.sh" >"$log" 2>&1 + local status=$? + set -e + + if [[ "$status" != "10" ]]; then + echo "expected exit code 10, got $status" >&2 + cat "$log" >&2 + exit 1 + fi - assert_contains "$argv" "--summary-overview=false" + assert_text_contains "$log" "::error::DiffPal code review found blocking issues at or above the high threshold." +} + +test_wrapper_keeps_non_gate_failures_generic() { + local dir="$1" + local bin_dir="$dir/generic-bin" + local argv="$dir/argv-generic" + local log="$dir/log-generic" + mkdir -p "$bin_dir" + make_fake_diffpal "$bin_dir" 4 "" "platform publish failed\n" + + set +e + PATH="$bin_dir:$PATH" \ + DIFFPAL_ARGV_FILE="$argv" \ + DIFFPAL_BIN=diffpal \ + INPUT_BASE=base-sha \ + INPUT_HEAD=head-sha \ + INPUT_GATE=true \ + "$repo_root/scripts/run-diffpal-review.sh" >"$log" 2>&1 + local status=$? + set -e + + if [[ "$status" != "4" ]]; then + echo "expected exit code 4, got $status" >&2 + cat "$log" >&2 + exit 1 + fi + + assert_not_contains "$log" "::error::DiffPal code review found blocking issues at or above the high threshold." } tmpdir="$(mktemp -d)" @@ -179,7 +235,8 @@ test_installer_installs_requested_version "$tmpdir/install" test_installer_selects_custom_path "$tmpdir/custom" test_installer_selects_path_when_install_disabled "$tmpdir/disabled" test_wrapper_uses_installed_binary_and_feedback "$tmpdir/wrapper-installed" -test_wrapper_mode_overrides_feedback "$tmpdir/wrapper-mode" test_wrapper_can_disable_summary_overview "$tmpdir/wrapper-overview" +test_wrapper_prints_gate_annotation_for_blocked_exit_code "$tmpdir/wrapper-gate" +test_wrapper_keeps_non_gate_failures_generic "$tmpdir/wrapper-generic" echo "github action smoke tests passed"