ci: persist fork approval labels across pushes (#2924)

This commit is contained in:
Carlos Florêncio
2026-08-22 15:35:06 +00:00
committed by GitHub
parent f753220ae1
commit 7835c9a02a
11 changed files with 360 additions and 88 deletions
@@ -0,0 +1,50 @@
#!/usr/bin/env python3
"""Contract tests for the OpenCode fork-review workflow."""
from pathlib import Path
import unittest
REPO_ROOT = Path(__file__).resolve().parents[2]
WORKFLOW = REPO_ROOT / ".github" / "workflows" / "opencode-code-review.yml"
LINT_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "lint-action-pinning.yml"
class OpenCodeCodeReviewWorkflowContractTest(unittest.TestCase):
def test_safe_to_review_approval_survives_follow_up_pushes(self) -> None:
workflow = WORKFLOW.read_text(encoding="utf-8")
_, separator, fork_job = workflow.partition(" opencode-review-fork:")
self.assertTrue(separator, "OpenCode fork-review job is missing")
self.assertIn("github.event_name == 'pull_request_target'", fork_job)
self.assertIn(
"github.event.pull_request.head.repo.full_name != github.repository",
fork_job,
)
self.assertIn(
"((contains(github.event.pull_request.labels.*.name, 'safe-to-review')) ||",
fork_job,
)
self.assertNotIn(
"github.event.action != 'synchronize' && contains("
"github.event.pull_request.labels.*.name, 'safe-to-review')",
fork_job,
)
def test_workflow_has_no_synchronize_time_label_cleanup(self) -> None:
workflow = WORKFLOW.read_text(encoding="utf-8")
self.assertNotIn(" strip-safe-to-review:", workflow)
self.assertNotIn("github.rest.issues.removeLabel", workflow)
def test_lint_workflow_runs_opencode_contract_test(self) -> None:
workflow = LINT_WORKFLOW.read_text(encoding="utf-8")
self.assertIn(
"python3 .github/scripts/opencode-code-review-workflow-contract_test.py",
workflow,
)
if __name__ == "__main__":
unittest.main()
@@ -31,6 +31,18 @@ class PreviewEnvironmentWorkflowContractTest(unittest.TestCase):
deploy_job,
)
def test_safe_to_test_approval_survives_follow_up_pushes(self) -> None:
workflow = WORKFLOW.read_text(encoding="utf-8")
_, separator, deploy_job = workflow.partition(" deploy-fork:")
self.assertTrue(separator, "Fork preview deploy job is missing")
self.assertIn(
"((contains(github.event.pull_request.labels.*.name, 'safe-to-test')) ||",
deploy_job,
)
self.assertNotIn(" strip-safe-to-test:", workflow)
self.assertNotIn("github.rest.issues.removeLabel", workflow)
if __name__ == "__main__":
unittest.main()
@@ -53,6 +53,9 @@ jobs:
- name: Test preview environment workflow contract
run: python3 .github/scripts/preview-env-workflow-contract_test.py
- name: Test OpenCode code review workflow contract
run: python3 .github/scripts/opencode-code-review-workflow-contract_test.py
- name: Test frontend tests workflow contract
run: python3 .github/scripts/frontend-tests-workflow-contract_test.py
+4 -33
View File
@@ -6,7 +6,7 @@ on:
pull_request_target:
# `labeled` is needed so a maintainer applying `safe-to-review` to a fork
# PR triggers a review against the current head SHA. Subsequent fork pushes
# do not re-use the stale label — see strip-safe-to-review below.
# re-use the maintainer's approval for the new head.
types: [opened, synchronize, ready_for_review, reopened, labeled]
concurrency:
@@ -239,48 +239,19 @@ jobs:
path: .opencode-review/
if-no-files-found: ignore
# Strip safe-to-review label on new pushes to fork PRs.
# Together with the `github.event.action != 'synchronize'` guard on the
# fork review job's label path, this gives per-commit maintainer opt-in.
strip-safe-to-review:
if: >
github.event_name == 'pull_request_target' &&
github.event.action == 'synchronize' &&
github.event.pull_request.head.repo.full_name != github.repository &&
contains(github.event.pull_request.labels.*.name, 'safe-to-review')
runs-on: ubuntu-latest
permissions:
pull-requests: write
steps:
- uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
try {
await github.rest.issues.removeLabel({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.payload.pull_request.number,
name: 'safe-to-review',
});
} catch (error) {
if (error.status !== 404) throw error;
core.info('safe-to-review label already absent; nothing to remove.');
}
opencode-review-fork:
# Fork PRs only, via pull_request_target so OPENCODE_API_KEY is available.
# Two approval paths:
# - OPENCODE_REVIEW_ALLOWLIST repo variable (trusted login list, all events;
# JSON array, e.g. ["alice","bob"])
# - `safe-to-review` label (maintainer opt-in, per-commit). Explicitly
# blocked on `synchronize` so a contributor push cannot re-use a stale
# label — the maintainer must re-apply it after each push.
# - `safe-to-review` label (maintainer opt-in, durable until removed).
# It authorizes the current and subsequent fork heads.
if: >
vars.OPENCODE_REVIEW_ENABLED == 'true' &&
github.event_name == 'pull_request_target' &&
github.event.pull_request.draft == false &&
github.event.pull_request.head.repo.full_name != github.repository &&
((github.event.action != 'synchronize' && contains(github.event.pull_request.labels.*.name, 'safe-to-review')) ||
((contains(github.event.pull_request.labels.*.name, 'safe-to-review')) ||
(vars.OPENCODE_REVIEW_ALLOWLIST != '' && contains(fromJSON(vars.OPENCODE_REVIEW_ALLOWLIST), github.event.pull_request.user.login)))
runs-on: ubuntu-latest
timeout-minutes: 15
+8 -44
View File
@@ -10,8 +10,7 @@ on:
pull_request_target:
# `labeled` is needed so a maintainer applying `safe-to-test` to a fork PR
# triggers a deploy against the current head SHA. Subsequent fork pushes
# (synchronize) do NOT re-trigger the label path — see deploy-fork's
# if-gate and strip-safe-to-test below.
# (synchronize) re-use the maintainer's approval for the new head.
types: [opened, synchronize, reopened, closed, labeled]
concurrency:
@@ -73,12 +72,10 @@ jobs:
# JSON array of logins)
# - CLAUDE_REVIEW_ALLOWLIST repo variable (trusted login list, all events;
# the Claude workflow also adds the approval labels on open)
# - `safe-to-test` label (maintainer opt-in, per-commit). Explicitly
# blocked on `synchronize` so a contributor's push cannot re-use a
# stale label — the maintainer must re-apply it after each push for
# the `labeled` event to trigger a fresh deploy of the new SHA. This
# matters more here than for claude-code-review.yml because deploy
# actually runs the fork's `cmd/preview` code with SPRITES_API_TOKEN.
# - `safe-to-test` label (maintainer opt-in, durable until removed). It
# authorizes the current and subsequent fork heads. This matters more
# here than for claude-code-review.yml because deploy actually runs the
# fork's `cmd/preview` code with SPRITES_API_TOKEN.
# -----------------------------------------------------------------------
# Branch order matters: keep cheap/safe checks before `fromJSON(vars.*)`,
# which throws (and skips the whole job) if the repo variable is
@@ -88,7 +85,7 @@ jobs:
github.event.action != 'closed' &&
github.event_name == 'pull_request_target' &&
github.event.pull_request.head.repo.full_name != github.repository &&
((github.event.action != 'synchronize' && contains(github.event.pull_request.labels.*.name, 'safe-to-test')) ||
((contains(github.event.pull_request.labels.*.name, 'safe-to-test')) ||
(vars.PREVIEW_ENV_ALLOWLIST != '' && contains(fromJSON(vars.PREVIEW_ENV_ALLOWLIST), github.event.pull_request.user.login)) ||
(vars.CLAUDE_REVIEW_ALLOWLIST != '' && contains(fromJSON(vars.CLAUDE_REVIEW_ALLOWLIST), github.event.pull_request.user.login)))
runs-on: ubuntu-latest
@@ -104,8 +101,8 @@ jobs:
# NOTE: The allowlist gate (safe-to-test, PREVIEW_ENV_ALLOWLIST,
# CLAUDE_REVIEW_ALLOWLIST)
# authorizes running the fork's cmd/preview code with SPRITES_API_TOKEN and GH_TOKEN.
# The safe-to-test label must be re-applied after each new push for
# the label-only path (see strip-safe-to-test job).
# The safe-to-test label remains an explicit maintainer approval for
# follow-up pushes until the maintainer removes it.
repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.sha }}
@@ -135,39 +132,6 @@ jobs:
SPRITES_API_TOKEN: ${{ secrets.SPRITES_API_TOKEN }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# -----------------------------------------------------------------------
# Strip safe-to-test label on new pushes to fork PRs.
# Together with the `github.event.action != 'synchronize'` guard on the
# label path in deploy-fork's if-gate, this gives true per-commit opt-in:
# the maintainer must re-apply `safe-to-test` after each push for the
# deploy job to run again. A new push on its own never re-triggers deploy
# on the label path.
# -----------------------------------------------------------------------
strip-safe-to-test:
if: >
github.event_name == 'pull_request_target' &&
github.event.action == 'synchronize' &&
github.event.pull_request.head.repo.full_name != github.repository &&
contains(github.event.pull_request.labels.*.name, 'safe-to-test')
runs-on: ubuntu-latest
permissions:
pull-requests: write
steps:
- uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
try {
await github.rest.issues.removeLabel({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.payload.pull_request.number,
name: 'safe-to-test',
});
} catch (error) {
if (error.status !== 404) throw error;
core.info('safe-to-test label already absent; nothing to remove.');
}
# -----------------------------------------------------------------------
# Cleanup (all closed PRs — no allowlist check)
# The sprite must be destroyed regardless of how the PR was closed or