Skip to content

Commit 785913e

Browse files
committed
infra: Revert comment-based trigger and use pull_request_target
This commit reverts a previous, non-functional attempt to run CI on forked PRs and replaces it with the correct, standard approach. The previous commit introduced a system using a `/check` comment to trigger workflows. This approach was fundamentally flawed due to limitations in the GitHub API, which could not resolve commit SHAs or refs from a forked repository that do not exist in the base repository's history. This resulted in `HTTP 422` errors. This commit removes that system entirely and implements the `pull_request_target` event trigger instead. This is the GitHub-recommended method for this scenario. The new implementation: 1. Uses the `pull_request_target` event, which runs in the context of the base repository and has access to secrets. 2. Leverages the existing manual "Approve and run" security step for new contributors. 3. Explicitly checks out the contributor's code using `github.event.pull_request.head.sha`. 4. Removes the now-obsolete `pr-comment-trigger.yml` workflow and its associated documentation. This solution is simpler, more robust, and correctly handles CI for external contributions.
1 parent d6ef6d4 commit 785913e

File tree

4 files changed

+8
-95
lines changed

4 files changed

+8
-95
lines changed

.github/workflows/playground-e2e-tests.yml

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@ on:
99
- ".notes/**"
1010
- "*.md"
1111
- "src/**/*.test.*"
12-
pull_request:
13-
branches:
14-
- main
12+
pull_request_target:
13+
types: [opened, synchronize, reopened]
1514
paths-ignore:
1615
- "docs/**"
1716
- ".notes/**"
@@ -88,18 +87,6 @@ jobs:
8887
echo 'matrix={"include":[{"os":"ubuntu-latest","package-manager":"npm"}]}' >> $GITHUB_OUTPUT
8988
fi
9089
91-
e2e-tests-pending:
92-
runs-on: ubuntu-latest
93-
if: |
94-
github.event_name == 'pull_request' &&
95-
github.event.pull_request.head.repo.full_name != github.repository
96-
steps:
97-
- name: Report pending status
98-
run: |
99-
echo "E2E tests for this PR have not been run."
100-
echo "A maintainer must comment '/check' to start them."
101-
exit 1
102-
10390
playground-e2e:
10491
needs: setup-matrix
10592
name: Playground E2E Test
@@ -127,13 +114,13 @@ jobs:
127114
github.event_name == 'push' ||
128115
github.event_name == 'schedule' ||
129116
github.event_name == 'workflow_dispatch' ||
130-
(github.event.pull_request.head.repo.full_name == github.repository)
117+
github.event_name == 'pull_request_target'
131118
132119
steps:
133120
- name: Checkout repository
134121
uses: actions/checkout@v5
135122
with:
136-
ref: ${{ github.event.inputs.ref || github.ref }}
123+
ref: ${{ github.event.pull_request.head.sha || github.event.inputs.ref || github.ref }}
137124

138125
- name: Enable Corepack
139126
run: corepack enable

.github/workflows/pr-comment-trigger.yml

Lines changed: 0 additions & 46 deletions
This file was deleted.

.github/workflows/smoke-test.yml

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@ on:
99
- ".notes/**"
1010
- "*.md"
1111
- "src/**/*.test.*"
12-
pull_request:
13-
branches:
14-
- main
12+
pull_request_target:
13+
types: [opened, synchronize, reopened]
1514
paths-ignore:
1615
- "docs/**"
1716
- ".notes/**"
@@ -86,17 +85,6 @@ jobs:
8685
echo 'matrix={"include":[{"os":"ubuntu-latest","package-manager":"npm"}]}' >> $GITHUB_OUTPUT
8786
fi
8887
89-
smoke-tests-pending:
90-
runs-on: ubuntu-latest
91-
if: |
92-
github.event_name == 'pull_request' &&
93-
github.event.pull_request.head.repo.full_name != github.repository
94-
steps:
95-
- name: Report pending status
96-
run: |
97-
echo "Smoke tests for this PR have not been run."
98-
echo "A maintainer must comment '/check' to start them."
99-
exit 1
10088
smoke-test:
10189
needs: setup-matrix
10290
name: Starter Smoke Test
@@ -122,13 +110,13 @@ jobs:
122110
github.event_name == 'push' ||
123111
github.event_name == 'schedule' ||
124112
github.event_name == 'workflow_dispatch' ||
125-
(github.event.pull_request.head.repo.full_name == github.repository)
113+
github.event_name == 'pull_request_target'
126114
127115
steps:
128116
- name: Checkout repository
129117
uses: actions/checkout@v5
130118
with:
131-
ref: ${{ github.event.inputs.ref || github.ref }}
119+
ref: ${{ github.event.pull_request.head.sha || github.event.inputs.ref || github.sha }}
132120

133121
- name: Enable Corepack
134122
run: corepack enable

CONTRIBUTING.md

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -518,22 +518,6 @@ export RWSDK_FORCE_FULL_SYNC=1
518518
npx rwsync --watch "npm run dev"
519519
```
520520
521-
## CI for External Contributions
522-
523-
For security reasons, CI workflows that require secrets (e.g., Smoke and E2E tests) do not automatically run on pull requests from forks. This prevents malicious PRs from accessing sensitive credentials.
524-
525-
When a PR is opened from a fork, the main test jobs are skipped. To prevent merging untested code, a separate status check will fail, indicating that the core tests are pending a maintainer's approval. This failing check is expected and blocks the PR from being merged until tests are run.
526-
527-
To run the tests for an external PR, a core contributor must:
528-
529-
1. **Review the code** to ensure it's safe to execute.
530-
2. Add a comment to the PR with the command:
531-
```
532-
/check
533-
```
534-
535-
This command triggers the full test suite. Only repository owners, members, and collaborators can use it.
536-
537521
## Releasing (for Core Contributors)
538522
539523
Releases are managed by a series of automated GitHub Actions workflows that handle versioning, smoke testing, publishing to npm, and packaging of release artifacts for the SDK, starter, and addons.

0 commit comments

Comments
 (0)