alibaba / alibaba/open-code-review

feat(action): optional pr_number input, for triggers that carry no pull request

Open Beginner friendly
#1,150 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
24.4k
Forks
1.8k
Avg merge
2d 4m
Merged PRs (30d)
102

Description

## Problem

The action resolves the pull request it operates on from the event payload, and
offers no way to override it:

- `action.yml` — the fetch step: `PR_NUM: ${{ github.event.pull_request.number || github.event.issue.number }}`
- `action.yml` — the post step: `prNumber: context.issue.number`

Both are correct for `pull_request`, `pull_request_target` and `issue_comment`.
Neither resolves for **`workflow_run`**, whose payload carries no pull request —
the PR is reachable only as `github.event.workflow_run.pull_requests[0].number`,
which the action never reads.

The result is not a clean failure. The review runs to completion and then the
post step requests:

```
/repos/OWNER/REPO/issues//comments
```

— note the empty path segment — and gets a `404`. Observed on v1.11.2 with a
review whose `terminal_state` was `complete`: the work was done and the findings
were lost.

## Why `workflow_run`

To review the head the tests passed on, rather than racing CI. Triggering on
`pull_request` starts a review the moment a commit lands, so reviewer quota gets
spent on heads CI is about to reject, and findings get posted against code that
never passed. `workflow_run` on the test workflow is the standard way to order
those, and it is the only trigger that expresses "after CI, on the same head".

The workarounds are all worse:

- an in-job wait on `pull_request` — occupies a runner for the duration of CI,
which is fatal on a small self-hosted pool;
- reimplementing the posting path from the run manifest — forks inline comments,
the sticky summary and the checkpoint marker out of the action.

## Proposed

An optional input, defaulting to today's behaviour:

```yaml
pr_number:
description: Pull request number. Defaults to the number in the event payload.
required: false
default: ''
```

used as `pr_number || ` at both sites. Callers already
have the number — a `workflow_run` payload names it, and one API read resolves
it for any event that does not.

Behaviour is unchanged for every current trigger, since an unset input falls
through to the existing resolution.

## Related

`base_ref` and `head_sha` already exist as inputs for exactly this reason —
letting the caller supply what its event cannot. `pr_number` is the third member
of that set, and its absence is what makes the other two insufficient on
`workflow_run`.

Happy to open a PR if the shape is agreeable.

Contributor guide

Open the contributing guide

Research direction

Start in action.yml and inspect the fetch and post steps named in the issue, along with the existing base_ref and head_sha inputs. Add the optional pr_number input with its fallback behavior at both sites, then verify that current triggers retain their existing resolution and workflow_run can supply a pull request number.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions
Domain
ci-cd
Issue type
Feature
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.