From c4d638b7f023244fea0bc84ab741a10656ab164d Mon Sep 17 00:00:00 2001 From: Amp Date: Tue, 8 Sep 2026 17:02:46 +0000 Subject: [PATCH 1/3] feat: configure reviews by pull request state Amp-Thread-ID: https://ampcode.com/threads/T-01a081f0-878f-711e-b4ff-07114b8ea68a Co-authored-by: Ivan Milev --- .github/workflows/codeboarding.yml | 12 ++++++------ README.md | 18 +++++++++++++++-- action.yml | 11 +++++++++++ scripts/action/guard.sh | 15 +++++++++++++++ tests/test_action_inputs.py | 8 ++++++++ tests/test_action_sync.py | 31 ++++++++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 8 deletions(-) diff --git a/.github/workflows/codeboarding.yml b/.github/workflows/codeboarding.yml index 5b868b7..322bff4 100644 --- a/.github/workflows/codeboarding.yml +++ b/.github/workflows/codeboarding.yml @@ -2,11 +2,11 @@ name: CodeBoarding review on: pull_request: - # Generate once, when the PR becomes reviewable. Reusing this PR's previous - # analysis makes per-push runs affordable, so `synchronize` is a reasonable - # addition now; /codeboarding still refreshes on demand. 'closed' only - # cancels an in-flight review (see concurrency), it doesn't start one. - types: [opened, reopened, ready_for_review, closed] + # Generate when the PR opens or changes draft state. Reusing this PR's + # previous analysis makes `synchronize` a reasonable addition if per-push + # updates are wanted; /codeboarding still refreshes on demand. 'closed' + # only cancels an in-flight review (see concurrency), it doesn't start one. + types: [opened, reopened, ready_for_review, converted_to_draft, closed] issue_comment: types: [created] @@ -37,7 +37,7 @@ jobs: # changes generated files, so a diff comment would be noise. Scope this to # this repository so a fork using the same branch name is still reviewed. if: > - (github.event_name == 'pull_request' && github.event.action != 'closed' && github.event.pull_request.draft == false && + (github.event_name == 'pull_request' && github.event.action != 'closed' && github.event.pull_request.head.repo.full_name == github.repository && !(github.head_ref == 'codeboarding/sync' && github.event.pull_request.head.repo.full_name == github.repository)) || (github.event_name == 'issue_comment' && github.event.issue.pull_request != null && diff --git a/README.md b/README.md index 78d7081..19885b2 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ name: CodeBoarding review on: pull_request: - types: [opened, reopened, ready_for_review, synchronize] + types: [opened, reopened, ready_for_review, converted_to_draft, synchronize] issue_comment: types: [created] @@ -42,7 +42,7 @@ concurrency: jobs: review: if: > - (github.event_name == 'pull_request' && github.event.pull_request.draft == false && + (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) || (github.event_name == 'issue_comment' && github.event.issue.pull_request != null && startsWith(github.event.comment.body, '/codeboarding') && @@ -57,6 +57,18 @@ jobs: Automatic runs update one sticky **CodeBoarding review** comment. A trusted repository owner, member, or collaborator can comment `/codeboarding` to analyze the current PR head again, including on fork PRs; every command creates a new result comment. +By default, automatic reviews run for both non-draft and draft pull requests. The two states can be selected independently: + +```yaml + - uses: CodeBoarding/CodeBoarding-action@v1 + with: + llm: hosted + run_on_pull_requests: true + run_on_draft_pull_requests: false # review only non-draft PRs +``` + +Set `run_on_pull_requests: false` and `run_on_draft_pull_requests: true` to review only drafts. Set both to `false` to disable automatic PR-event reviews while retaining trusted `/codeboarding` commands. Keep the PR-state check out of the job-level `if:` expression so the action can apply these inputs. Include `ready_for_review` and `converted_to_draft` in `pull_request.types` if changing either state should trigger a review immediately. + `synchronize` re-runs the review on every push to the branch. Each of those runs covers only the commits pushed since the previous one, so a push costs a fraction of a first analysis — and a pushed commit is the only thing that builds the reusable analysis, since GitHub gives comment-triggered runs a read-only cache. Drop `synchronize` from the list if you would rather spend one analysis per pull request than one per push. Keep the `concurrency` block if you keep `synchronize`: it is what makes a push continue from the push before it, and what stops a slower run for an older commit from overwriting the review comment for a newer one. Set `cancel-in-progress: true` instead to abandon a superseded run rather than queue it, which costs less when branches are pushed to rapidly, at the price of no analysis for the commits in between. @@ -296,6 +308,8 @@ With the default `github.token`, the repository or organization must allow GitHu | Input | Mode | Default | Description | |---|---|---|---| | `mode` | both | `review` | `review` or `sync`. | +| `run_on_pull_requests` | review | `true` | Run automatic reviews for non-draft PR events. | +| `run_on_draft_pull_requests` | review | `true` | Run automatic reviews for draft PR events. | | `llm` | both | **required** | `hosted`, `license`, or a provider name. No default. | | `_api_key` | both | empty | That provider's key, e.g. `anthropic_api_key`. See [Providers](#providers). | | `_base_url` | both | empty | That provider's endpoint, where it has one. | diff --git a/action.yml b/action.yml index cf4f57b..9d156ec 100644 --- a/action.yml +++ b/action.yml @@ -9,6 +9,14 @@ inputs: description: 'review posts a PR architecture diff; sync updates the versioned analysis baseline.' required: false default: 'review' + run_on_pull_requests: + description: 'Run automatic reviews for non-draft pull requests.' + required: false + default: 'true' + run_on_draft_pull_requests: + description: 'Run automatic reviews for draft pull requests.' + required: false + default: 'true' llm: description: 'Required. Where analysis credentials come from: hosted, license, or a provider name (anthropic, aws_bedrock, cerebras, deepseek, glm, google, kimi, litellm, ollama, openai, openrouter, orcarouter, vercel).' required: true @@ -201,6 +209,8 @@ runs: GH_HOST: ${{ github.server_url }} MODE: ${{ inputs.mode }} EVENT: ${{ github.event_name }} + RUN_ON_PULL_REQUESTS: ${{ inputs.run_on_pull_requests }} + RUN_ON_DRAFT_PULL_REQUESTS: ${{ inputs.run_on_draft_pull_requests }} REF_NAME: ${{ github.ref_name }} REF_TYPE: ${{ github.ref_type }} EVENT_SHA: ${{ github.sha }} @@ -216,6 +226,7 @@ runs: PULL_BASE_REF: ${{ github.event.pull_request.base.ref }} PULL_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} PULL_BASE_REPO: ${{ github.event.pull_request.base.repo.full_name }} + PULL_IS_DRAFT: ${{ github.event.pull_request.draft }} REPOSITORY: ${{ github.repository }} run: "$GITHUB_ACTION_PATH/scripts/action/guard.sh" diff --git a/scripts/action/guard.sh b/scripts/action/guard.sh index 84d9261..af152ec 100755 --- a/scripts/action/guard.sh +++ b/scripts/action/guard.sh @@ -42,6 +42,21 @@ fi case "$EVENT" in pull_request|pull_request_target) + run_on_pull_requests="${RUN_ON_PULL_REQUESTS:-true}" + run_on_draft_pull_requests="${RUN_ON_DRAFT_PULL_REQUESTS:-true}" + case "${run_on_pull_requests,,}" in + true|false) ;; + *) fail "run_on_pull_requests must be true or false." ;; + esac + case "${run_on_draft_pull_requests,,}" in + true|false) ;; + *) fail "run_on_draft_pull_requests must be true or false." ;; + esac + if [ "${PULL_IS_DRAFT:-false}" = true ]; then + [ "${run_on_draft_pull_requests,,}" = true ] || skip "Automatic reviews are disabled for draft pull requests." + else + [ "${run_on_pull_requests,,}" = true ] || skip "Automatic reviews are disabled for non-draft pull requests." + fi pr_number="$EVENT_PR_NUMBER" base_sha="$PULL_BASE_SHA" head_sha="$PULL_HEAD_SHA" diff --git a/tests/test_action_inputs.py b/tests/test_action_inputs.py index ec5d246..d9c9a84 100644 --- a/tests/test_action_inputs.py +++ b/tests/test_action_inputs.py @@ -62,6 +62,14 @@ def test_llm_is_required_and_has_no_default(self) -> None: self.assertIn("required: true", block) self.assertNotIn("default:", block) + def test_pull_request_state_inputs_default_to_enabled_and_reach_the_guard(self) -> None: + for name in ("run_on_pull_requests", "run_on_draft_pull_requests"): + with self.subTest(input=name): + self.assertIn("default: 'true'", self.inputs[name]) + self.assertIn(f"{name.upper()}: ${{{{ inputs.{name} }}}}", ACTION) + + self.assertIn("PULL_IS_DRAFT: ${{ github.event.pull_request.draft }}", ACTION) + def test_the_inferred_credential_inputs_are_gone(self) -> None: """`llm_api_key`/`llm_provider` are what made a fallback expressible at all.""" for stale in ("llm_api_key", "llm_provider"): diff --git a/tests/test_action_sync.py b/tests/test_action_sync.py index b0b87aa..d5d7026 100644 --- a/tests/test_action_sync.py +++ b/tests/test_action_sync.py @@ -16,6 +16,35 @@ class ActionSyncTests(unittest.TestCase): + def test_review_guard_honors_the_pull_request_state_inputs(self) -> None: + cases = ( + ("false", "true", "false", "non-draft"), + ("true", "false", "true", "draft"), + ) + for run_on_prs, run_on_drafts, is_draft, label in cases: + with self.subTest(state=label), tempfile.TemporaryDirectory() as tmp: + output = Path(tmp) / "github-output" + result = subprocess.run( + [str(GUARD)], + env={ + "PATH": os.environ["PATH"], + "GITHUB_OUTPUT": str(output), + "MODE": "review", + "EVENT": "pull_request", + "RUN_ON_PULL_REQUESTS": run_on_prs, + "RUN_ON_DRAFT_PULL_REQUESTS": run_on_drafts, + "PULL_IS_DRAFT": is_draft, + }, + capture_output=True, + text=True, + check=False, + ) + + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + values = output.read_text(encoding="utf-8") + self.assertTrue(values.endswith("skip=true\n")) + self.assertIn(f"disabled for {label} pull requests", result.stdout) + def test_review_guard_rejects_fork_before_checkout(self) -> None: with tempfile.TemporaryDirectory() as tmp: output = Path(tmp) / "github-output" @@ -135,6 +164,8 @@ def test_review_guard_accepts_trusted_fork_command(self) -> None: "AUTHOR_ASSOCIATION": "COLLABORATOR", "ISSUE_PR_URL": "repos/owner/repo/pulls/42", "GH_HOST": "github.com", + "RUN_ON_PULL_REQUESTS": "false", + "RUN_ON_DRAFT_PULL_REQUESTS": "false", }, capture_output=True, text=True, From 0c1db71ee5222a75369ebcd779f448d55e54f1cb Mon Sep 17 00:00:00 2001 From: Amp Date: Tue, 8 Sep 2026 17:21:15 +0000 Subject: [PATCH 2/3] fix: model pull request selection as states Amp-Thread-ID: https://ampcode.com/threads/T-01a081f0-878f-711e-b4ff-07114b8ea68a Co-authored-by: Ivan Milev --- README.md | 8 +++----- action.yml | 13 ++++--------- scripts/action/guard.sh | 29 +++++++++++++++++------------ tests/test_action_inputs.py | 9 +++------ tests/test_action_sync.py | 35 ++++++++++++++++++++++++++++------- 5 files changed, 55 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 19885b2..88f8d26 100644 --- a/README.md +++ b/README.md @@ -63,11 +63,10 @@ By default, automatic reviews run for both non-draft and draft pull requests. Th - uses: CodeBoarding/CodeBoarding-action@v1 with: llm: hosted - run_on_pull_requests: true - run_on_draft_pull_requests: false # review only non-draft PRs + pull_request_states: 'ready' # review only non-draft PRs ``` -Set `run_on_pull_requests: false` and `run_on_draft_pull_requests: true` to review only drafts. Set both to `false` to disable automatic PR-event reviews while retaining trusted `/codeboarding` commands. Keep the PR-state check out of the job-level `if:` expression so the action can apply these inputs. Include `ready_for_review` and `converted_to_draft` in `pull_request.types` if changing either state should trigger a review immediately. +Set `pull_request_states: 'draft'` to review only drafts, or `pull_request_states: ''` to disable automatic PR-event reviews while retaining trusted `/codeboarding` commands. The default, `'ready,draft'`, reviews both states. Values are comma-separated; surrounding whitespace is ignored, and unknown values fail the run rather than silently changing which pull requests are reviewed. Keep the PR-state check out of the job-level `if:` expression so the action can apply this input. Include `ready_for_review` and `converted_to_draft` in `pull_request.types` if changing either state should trigger a review immediately. `synchronize` re-runs the review on every push to the branch. Each of those runs covers only the commits pushed since the previous one, so a push costs a fraction of a first analysis — and a pushed commit is the only thing that builds the reusable analysis, since GitHub gives comment-triggered runs a read-only cache. Drop `synchronize` from the list if you would rather spend one analysis per pull request than one per push. @@ -308,8 +307,7 @@ With the default `github.token`, the repository or organization must allow GitHu | Input | Mode | Default | Description | |---|---|---|---| | `mode` | both | `review` | `review` or `sync`. | -| `run_on_pull_requests` | review | `true` | Run automatic reviews for non-draft PR events. | -| `run_on_draft_pull_requests` | review | `true` | Run automatic reviews for draft PR events. | +| `pull_request_states` | review | `ready,draft` | Comma-separated automatic-review states: `ready`, `draft`, both, or empty for neither. | | `llm` | both | **required** | `hosted`, `license`, or a provider name. No default. | | `_api_key` | both | empty | That provider's key, e.g. `anthropic_api_key`. See [Providers](#providers). | | `_base_url` | both | empty | That provider's endpoint, where it has one. | diff --git a/action.yml b/action.yml index 9d156ec..54e80f0 100644 --- a/action.yml +++ b/action.yml @@ -9,14 +9,10 @@ inputs: description: 'review posts a PR architecture diff; sync updates the versioned analysis baseline.' required: false default: 'review' - run_on_pull_requests: - description: 'Run automatic reviews for non-draft pull requests.' + pull_request_states: + description: 'Comma-separated pull request states to review automatically: ready and/or draft; empty disables automatic PR reviews.' required: false - default: 'true' - run_on_draft_pull_requests: - description: 'Run automatic reviews for draft pull requests.' - required: false - default: 'true' + default: 'ready,draft' llm: description: 'Required. Where analysis credentials come from: hosted, license, or a provider name (anthropic, aws_bedrock, cerebras, deepseek, glm, google, kimi, litellm, ollama, openai, openrouter, orcarouter, vercel).' required: true @@ -209,8 +205,7 @@ runs: GH_HOST: ${{ github.server_url }} MODE: ${{ inputs.mode }} EVENT: ${{ github.event_name }} - RUN_ON_PULL_REQUESTS: ${{ inputs.run_on_pull_requests }} - RUN_ON_DRAFT_PULL_REQUESTS: ${{ inputs.run_on_draft_pull_requests }} + PULL_REQUEST_STATES: ${{ inputs.pull_request_states }} REF_NAME: ${{ github.ref_name }} REF_TYPE: ${{ github.ref_type }} EVENT_SHA: ${{ github.sha }} diff --git a/scripts/action/guard.sh b/scripts/action/guard.sh index af152ec..e0acd41 100755 --- a/scripts/action/guard.sh +++ b/scripts/action/guard.sh @@ -42,20 +42,25 @@ fi case "$EVENT" in pull_request|pull_request_target) - run_on_pull_requests="${RUN_ON_PULL_REQUESTS:-true}" - run_on_draft_pull_requests="${RUN_ON_DRAFT_PULL_REQUESTS:-true}" - case "${run_on_pull_requests,,}" in - true|false) ;; - *) fail "run_on_pull_requests must be true or false." ;; - esac - case "${run_on_draft_pull_requests,,}" in - true|false) ;; - *) fail "run_on_draft_pull_requests must be true or false." ;; - esac + pull_request_states="${PULL_REQUEST_STATES-ready,draft}" + review_ready=false + review_draft=false + if [ -n "$pull_request_states" ]; then + IFS=',' read -ra states <<< "$pull_request_states" + for state in "${states[@]}"; do + state="${state#"${state%%[![:space:]]*}"}" + state="${state%"${state##*[![:space:]]}"}" + case "$state" in + ready) review_ready=true ;; + draft) review_draft=true ;; + *) fail "pull_request_states must contain only ready and draft." ;; + esac + done + fi if [ "${PULL_IS_DRAFT:-false}" = true ]; then - [ "${run_on_draft_pull_requests,,}" = true ] || skip "Automatic reviews are disabled for draft pull requests." + [ "$review_draft" = true ] || skip "Automatic reviews are disabled for draft pull requests." else - [ "${run_on_pull_requests,,}" = true ] || skip "Automatic reviews are disabled for non-draft pull requests." + [ "$review_ready" = true ] || skip "Automatic reviews are disabled for ready pull requests." fi pr_number="$EVENT_PR_NUMBER" base_sha="$PULL_BASE_SHA" diff --git a/tests/test_action_inputs.py b/tests/test_action_inputs.py index d9c9a84..1d24528 100644 --- a/tests/test_action_inputs.py +++ b/tests/test_action_inputs.py @@ -62,12 +62,9 @@ def test_llm_is_required_and_has_no_default(self) -> None: self.assertIn("required: true", block) self.assertNotIn("default:", block) - def test_pull_request_state_inputs_default_to_enabled_and_reach_the_guard(self) -> None: - for name in ("run_on_pull_requests", "run_on_draft_pull_requests"): - with self.subTest(input=name): - self.assertIn("default: 'true'", self.inputs[name]) - self.assertIn(f"{name.upper()}: ${{{{ inputs.{name} }}}}", ACTION) - + def test_pull_request_states_default_to_both_and_reach_the_guard(self) -> None: + self.assertIn("default: 'ready,draft'", self.inputs["pull_request_states"]) + self.assertIn("PULL_REQUEST_STATES: ${{ inputs.pull_request_states }}", ACTION) self.assertIn("PULL_IS_DRAFT: ${{ github.event.pull_request.draft }}", ACTION) def test_the_inferred_credential_inputs_are_gone(self) -> None: diff --git a/tests/test_action_sync.py b/tests/test_action_sync.py index d5d7026..c9b630e 100644 --- a/tests/test_action_sync.py +++ b/tests/test_action_sync.py @@ -18,10 +18,12 @@ class ActionSyncTests(unittest.TestCase): def test_review_guard_honors_the_pull_request_state_inputs(self) -> None: cases = ( - ("false", "true", "false", "non-draft"), - ("true", "false", "true", "draft"), + ("draft", "false", "ready"), + ("ready", "true", "draft"), + ("", "false", "ready"), + ("", "true", "draft"), ) - for run_on_prs, run_on_drafts, is_draft, label in cases: + for states, is_draft, label in cases: with self.subTest(state=label), tempfile.TemporaryDirectory() as tmp: output = Path(tmp) / "github-output" result = subprocess.run( @@ -31,8 +33,7 @@ def test_review_guard_honors_the_pull_request_state_inputs(self) -> None: "GITHUB_OUTPUT": str(output), "MODE": "review", "EVENT": "pull_request", - "RUN_ON_PULL_REQUESTS": run_on_prs, - "RUN_ON_DRAFT_PULL_REQUESTS": run_on_drafts, + "PULL_REQUEST_STATES": states, "PULL_IS_DRAFT": is_draft, }, capture_output=True, @@ -45,6 +46,26 @@ def test_review_guard_honors_the_pull_request_state_inputs(self) -> None: self.assertTrue(values.endswith("skip=true\n")) self.assertIn(f"disabled for {label} pull requests", result.stdout) + def test_review_guard_rejects_an_unknown_pull_request_state(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + output = Path(tmp) / "github-output" + result = subprocess.run( + [str(GUARD)], + env={ + "PATH": os.environ["PATH"], + "GITHUB_OUTPUT": str(output), + "MODE": "review", + "EVENT": "pull_request", + "PULL_REQUEST_STATES": "ready,unknown", + }, + capture_output=True, + text=True, + check=False, + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("pull_request_states must contain only ready and draft", result.stdout) + def test_review_guard_rejects_fork_before_checkout(self) -> None: with tempfile.TemporaryDirectory() as tmp: output = Path(tmp) / "github-output" @@ -120,6 +141,7 @@ def test_review_guard_ignores_a_distance_it_cannot_anchor(self) -> None: "PULL_HEAD_SHA": "head-sha", "PULL_BASE_REPO": "owner/repo", "PULL_HEAD_REPO": "owner/repo", + "PULL_IS_DRAFT": "true", }, capture_output=True, text=True, @@ -164,8 +186,7 @@ def test_review_guard_accepts_trusted_fork_command(self) -> None: "AUTHOR_ASSOCIATION": "COLLABORATOR", "ISSUE_PR_URL": "repos/owner/repo/pulls/42", "GH_HOST": "github.com", - "RUN_ON_PULL_REQUESTS": "false", - "RUN_ON_DRAFT_PULL_REQUESTS": "false", + "PULL_REQUEST_STATES": "", }, capture_output=True, text=True, From 15b0e593e3cbbaf856a2d20d2867a75b645addf6 Mon Sep 17 00:00:00 2001 From: Amp Date: Tue, 8 Sep 2026 19:17:54 +0000 Subject: [PATCH 3/3] feat: review draft pull requests by default Amp-Thread-ID: https://ampcode.com/threads/T-01a081f0-878f-711e-b4ff-07114b8ea68a Co-authored-by: Ivan Milev --- .github/workflows/codeboarding.yml | 9 +++--- README.md | 16 ++------- action.yml | 6 ---- scripts/action/guard.sh | 20 ------------ tests/test_action_inputs.py | 15 ++++++--- tests/test_action_sync.py | 52 ------------------------------ 6 files changed, 16 insertions(+), 102 deletions(-) diff --git a/.github/workflows/codeboarding.yml b/.github/workflows/codeboarding.yml index 322bff4..3495800 100644 --- a/.github/workflows/codeboarding.yml +++ b/.github/workflows/codeboarding.yml @@ -2,11 +2,10 @@ name: CodeBoarding review on: pull_request: - # Generate when the PR opens or changes draft state. Reusing this PR's - # previous analysis makes `synchronize` a reasonable addition if per-push - # updates are wanted; /codeboarding still refreshes on demand. 'closed' - # only cancels an in-flight review (see concurrency), it doesn't start one. - types: [opened, reopened, ready_for_review, converted_to_draft, closed] + # Analyze when the PR opens or receives a commit, whether it is ready or draft. + # Changing only the draft state does not rerun analysis. 'closed' only cancels + # an in-flight review (see concurrency), it doesn't start one. + types: [opened, reopened, closed, synchronize] issue_comment: types: [created] diff --git a/README.md b/README.md index 5a90996..fb5a194 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ name: CodeBoarding review on: pull_request: - types: [opened, reopened, ready_for_review, converted_to_draft, synchronize] + types: [opened, reopened, synchronize] issue_comment: types: [created] @@ -55,18 +55,7 @@ jobs: llm: hosted # or license, or a provider name -- see Authentication ``` -Automatic runs update one sticky **CodeBoarding review** comment. A trusted repository owner, member, or collaborator can comment `/codeboarding` to analyze the current PR head again, including on fork PRs; every command creates a new result comment. - -By default, automatic reviews run for both non-draft and draft pull requests. The two states can be selected independently: - -```yaml - - uses: CodeBoarding/CodeBoarding-action@v1 - with: - llm: hosted - pull_request_states: 'ready' # review only non-draft PRs -``` - -Set `pull_request_states: 'draft'` to review only drafts, or `pull_request_states: ''` to disable automatic PR-event reviews while retaining trusted `/codeboarding` commands. The default, `'ready,draft'`, reviews both states. Values are comma-separated; surrounding whitespace is ignored, and unknown values fail the run rather than silently changing which pull requests are reviewed. Keep the PR-state check out of the job-level `if:` expression so the action can apply this input. Include `ready_for_review` and `converted_to_draft` in `pull_request.types` if changing either state should trigger a review immediately. +Automatic runs review both draft and non-draft pull requests and update one sticky **CodeBoarding review** comment. Opening, reopening, or pushing a commit runs analysis; changing only the draft state does not. A trusted repository owner, member, or collaborator can comment `/codeboarding` to analyze the current PR head again, including on fork PRs; every command creates a new result comment. `synchronize` re-runs the review on every push to the branch. Each of those runs covers only the commits pushed since the previous one, so a push costs a fraction of a first analysis — and a pushed commit is the only thing that builds the reusable analysis, since GitHub gives comment-triggered runs a read-only cache. Drop `synchronize` from the list if you would rather spend one analysis per pull request than one per push. @@ -307,7 +296,6 @@ With the default `github.token`, the repository or organization must allow GitHu | Input | Mode | Default | Description | |---|---|---|---| | `mode` | both | `review` | `review` or `sync`. | -| `pull_request_states` | review | `ready,draft` | Comma-separated automatic-review states: `ready`, `draft`, both, or empty for neither. | | `llm` | both | **required** | `hosted`, `license`, or a provider name. No default. | | `_api_key` | both | empty | That provider's key, e.g. `anthropic_api_key`. See [Providers](#providers). | | `_base_url` | both | empty | That provider's endpoint, where it has one. | diff --git a/action.yml b/action.yml index 8ca8dfd..2d815f7 100644 --- a/action.yml +++ b/action.yml @@ -9,10 +9,6 @@ inputs: description: 'review posts a PR architecture diff; sync updates the versioned analysis baseline.' required: false default: 'review' - pull_request_states: - description: 'Comma-separated pull request states to review automatically: ready and/or draft; empty disables automatic PR reviews.' - required: false - default: 'ready,draft' llm: description: 'Required. Where analysis credentials come from: hosted, license, or a provider name (anthropic, aws_bedrock, cerebras, deepseek, glm, google, kimi, litellm, ollama, openai, openrouter, orcarouter, vercel).' required: true @@ -209,7 +205,6 @@ runs: GH_HOST: ${{ github.server_url }} MODE: ${{ inputs.mode }} EVENT: ${{ github.event_name }} - PULL_REQUEST_STATES: ${{ inputs.pull_request_states }} REF_NAME: ${{ github.ref_name }} REF_TYPE: ${{ github.ref_type }} EVENT_SHA: ${{ github.sha }} @@ -225,7 +220,6 @@ runs: PULL_BASE_REF: ${{ github.event.pull_request.base.ref }} PULL_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} PULL_BASE_REPO: ${{ github.event.pull_request.base.repo.full_name }} - PULL_IS_DRAFT: ${{ github.event.pull_request.draft }} REPOSITORY: ${{ github.repository }} run: "$GITHUB_ACTION_PATH/scripts/action/guard.sh" diff --git a/scripts/action/guard.sh b/scripts/action/guard.sh index e0acd41..84d9261 100755 --- a/scripts/action/guard.sh +++ b/scripts/action/guard.sh @@ -42,26 +42,6 @@ fi case "$EVENT" in pull_request|pull_request_target) - pull_request_states="${PULL_REQUEST_STATES-ready,draft}" - review_ready=false - review_draft=false - if [ -n "$pull_request_states" ]; then - IFS=',' read -ra states <<< "$pull_request_states" - for state in "${states[@]}"; do - state="${state#"${state%%[![:space:]]*}"}" - state="${state%"${state##*[![:space:]]}"}" - case "$state" in - ready) review_ready=true ;; - draft) review_draft=true ;; - *) fail "pull_request_states must contain only ready and draft." ;; - esac - done - fi - if [ "${PULL_IS_DRAFT:-false}" = true ]; then - [ "$review_draft" = true ] || skip "Automatic reviews are disabled for draft pull requests." - else - [ "$review_ready" = true ] || skip "Automatic reviews are disabled for ready pull requests." - fi pr_number="$EVENT_PR_NUMBER" base_sha="$PULL_BASE_SHA" head_sha="$PULL_HEAD_SHA" diff --git a/tests/test_action_inputs.py b/tests/test_action_inputs.py index 68f91ce..4757bc7 100644 --- a/tests/test_action_inputs.py +++ b/tests/test_action_inputs.py @@ -16,6 +16,7 @@ ROOT = Path(__file__).resolve().parent.parent ACTION = (ROOT / "action.yml").read_text(encoding="utf-8") TABLE = json.loads((ROOT / "scripts" / "action" / "supported-providers.json").read_text(encoding="utf-8")) +DOGFOOD = (ROOT / ".github" / "workflows" / "codeboarding.yml").read_text(encoding="utf-8") def declared_inputs() -> dict[str, str]: @@ -62,11 +63,6 @@ def test_llm_is_required_and_has_no_default(self) -> None: self.assertIn("required: true", block) self.assertNotIn("default:", block) - def test_pull_request_states_default_to_both_and_reach_the_guard(self) -> None: - self.assertIn("default: 'ready,draft'", self.inputs["pull_request_states"]) - self.assertIn("PULL_REQUEST_STATES: ${{ inputs.pull_request_states }}", ACTION) - self.assertIn("PULL_IS_DRAFT: ${{ github.event.pull_request.draft }}", ACTION) - def test_depth_is_wired_to_state_identity_and_both_analysis_modes(self) -> None: self.assertIn("default: '2'", self.inputs["depth_cap"]) self.assertNotIn("depth_level", self.inputs) @@ -75,6 +71,15 @@ def test_depth_is_wired_to_state_identity_and_both_analysis_modes(self) -> None: block = ACTION[start : ACTION.index("\n run:", start)] self.assertIn("DEPTH_CAP: ${{ inputs.depth_cap }}", block) + def test_default_workflow_reviews_drafts_on_open_and_new_commits(self) -> None: + self.assertNotIn("github.event.pull_request.draft", DOGFOOD) + start = DOGFOOD.index("types:") + types = DOGFOOD[start : DOGFOOD.index("\n", start)] + for event in ("opened", "reopened", "synchronize"): + self.assertIn(event, types) + for state_change in ("ready_for_review", "converted_to_draft"): + self.assertNotIn(state_change, types) + def test_the_inferred_credential_inputs_are_gone(self) -> None: """`llm_api_key`/`llm_provider` are what made a fallback expressible at all.""" for stale in ("llm_api_key", "llm_provider"): diff --git a/tests/test_action_sync.py b/tests/test_action_sync.py index c9b630e..b0b87aa 100644 --- a/tests/test_action_sync.py +++ b/tests/test_action_sync.py @@ -16,56 +16,6 @@ class ActionSyncTests(unittest.TestCase): - def test_review_guard_honors_the_pull_request_state_inputs(self) -> None: - cases = ( - ("draft", "false", "ready"), - ("ready", "true", "draft"), - ("", "false", "ready"), - ("", "true", "draft"), - ) - for states, is_draft, label in cases: - with self.subTest(state=label), tempfile.TemporaryDirectory() as tmp: - output = Path(tmp) / "github-output" - result = subprocess.run( - [str(GUARD)], - env={ - "PATH": os.environ["PATH"], - "GITHUB_OUTPUT": str(output), - "MODE": "review", - "EVENT": "pull_request", - "PULL_REQUEST_STATES": states, - "PULL_IS_DRAFT": is_draft, - }, - capture_output=True, - text=True, - check=False, - ) - - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - values = output.read_text(encoding="utf-8") - self.assertTrue(values.endswith("skip=true\n")) - self.assertIn(f"disabled for {label} pull requests", result.stdout) - - def test_review_guard_rejects_an_unknown_pull_request_state(self) -> None: - with tempfile.TemporaryDirectory() as tmp: - output = Path(tmp) / "github-output" - result = subprocess.run( - [str(GUARD)], - env={ - "PATH": os.environ["PATH"], - "GITHUB_OUTPUT": str(output), - "MODE": "review", - "EVENT": "pull_request", - "PULL_REQUEST_STATES": "ready,unknown", - }, - capture_output=True, - text=True, - check=False, - ) - - self.assertEqual(result.returncode, 1) - self.assertIn("pull_request_states must contain only ready and draft", result.stdout) - def test_review_guard_rejects_fork_before_checkout(self) -> None: with tempfile.TemporaryDirectory() as tmp: output = Path(tmp) / "github-output" @@ -141,7 +91,6 @@ def test_review_guard_ignores_a_distance_it_cannot_anchor(self) -> None: "PULL_HEAD_SHA": "head-sha", "PULL_BASE_REPO": "owner/repo", "PULL_HEAD_REPO": "owner/repo", - "PULL_IS_DRAFT": "true", }, capture_output=True, text=True, @@ -186,7 +135,6 @@ def test_review_guard_accepts_trusted_fork_command(self) -> None: "AUTHOR_ASSOCIATION": "COLLABORATOR", "ISSUE_PR_URL": "repos/owner/repo/pulls/42", "GH_HOST": "github.com", - "PULL_REQUEST_STATES": "", }, capture_output=True, text=True,