From cd71e5749ff6f96cd3606e372b8c173c7a94777a Mon Sep 17 00:00:00 2001 From: Barak Korren Date: Thu, 23 Jul 2026 09:00:35 +0300 Subject: [PATCH 1/2] ci(e2e): split ok-to-test label trigger into caller workflows Non-ok-to-test labels no longer start E2E/Functional suite runs. ok-to-test is handled by thin callers via workflow_call. Labeled events never cancel an in-progress suite (auth only, not a retest). Signed-off-by: Barak Korren Co-authored-by: Cursor --- .github/workflows/e2e-ok-to-test.yml | 20 +++++++++++ .github/workflows/e2e.yml | 26 +++++++-------- .github/workflows/functional-ok-to-test.yml | 20 +++++++++++ .github/workflows/functional-tests.yml | 23 ++++++------- docs/guides/dev/e2e-testing.md | 37 ++++++++++++--------- 5 files changed, 83 insertions(+), 43 deletions(-) create mode 100644 .github/workflows/e2e-ok-to-test.yml create mode 100644 .github/workflows/functional-ok-to-test.yml diff --git a/.github/workflows/e2e-ok-to-test.yml b/.github/workflows/e2e-ok-to-test.yml new file mode 100644 index 000000000..282ddfbfb --- /dev/null +++ b/.github/workflows/e2e-ok-to-test.yml @@ -0,0 +1,20 @@ +name: E2E ok-to-test + +# Thin labeled-only entry point for fork / ok-to-test authorization. +# Calls E2E Tests via workflow_call so non-ok-to-test labels (e.g. ready-for-review) +# do not start skipped runs under the "E2E Tests" workflow name. +# +# No caller-level concurrency on the e2e-{n} group — sharing that group between +# this caller and the reusable e2e.yml deadlocks. + +permissions: {} + +on: + pull_request_target: + types: [labeled] + +jobs: + run: + if: github.event.label.name == 'ok-to-test' + uses: ./.github/workflows/e2e.yml + secrets: inherit diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index d1d093b3b..c4e757ca3 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -37,29 +37,29 @@ on: - 'Makefile' - '.github/scripts/**' - '.github/workflows/e2e.yml' + - '.github/workflows/e2e-ok-to-test.yml' - '.github/workflows/reusable-dispatch.yml' - 'action.yml' - '.github/actions/check-e2e-authorization/**' - 'scripts/check-e2e-authorization.sh' pull_request_target: - types: [opened, synchronize, reopened, labeled] + # labeled / ok-to-test is handled by e2e-ok-to-test.yml via workflow_call. + types: [opened, synchronize, reopened] merge_group: workflow_dispatch: + workflow_call: -# Non-ok-to-test labeled events still start a workflow run (GitHub cannot filter -# by label name at on:), but must not cancel an in-progress opened/synchronize -# run — same actionable filter as the gate job if: below. concurrency: group: >- ${{ github.event_name == 'pull_request_target' && format('e2e-{0}', github.event.pull_request.number) || format('{0}-{1}', github.workflow, github.ref) }} - # Explicit outer parens: (A && B) || (C && D). Relies on && binding tighter - # than ||; keep the grouping obvious so a future || clause cannot silently - # disable cancel for non-pull_request_target triggers. + # Label events are auth, not a code change — never cancel an in-progress run. + # Cancel for opened/synchronize/reopened and for non-main non-PR triggers. + # Explicit outer parens: (A && B) || (C && D). cancel-in-progress: >- ${{ (github.event_name == 'pull_request_target' - && (github.event.action != 'labeled' || github.event.label.name == 'ok-to-test')) + && github.event.action != 'labeled') || (github.event_name != 'pull_request_target' && github.ref != 'refs/heads/main') }} jobs: @@ -67,9 +67,8 @@ jobs: # Separate job (not steps in e2e) so pull-requests: write stays out of the # job that checks out fork head and runs make e2e-test with secrets. # Never checkout github.event.pull_request.head.sha here. - if: >- - github.event_name == 'pull_request_target' && - (github.event.action != 'labeled' || github.event.label.name == 'ok-to-test') + # workflow_call from e2e-ok-to-test.yml still has event_name pull_request_target. + if: github.event_name == 'pull_request_target' runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: @@ -95,7 +94,6 @@ jobs: e2e: # For pull_request_target, runs only when gate sets authorized=true. - # Do not treat a skipped gate as authorized (e.g. labeled events for non-ok-to-test labels). # This job checks out untrusted PR head code — no pull-requests: write here. needs: gate if: >- @@ -138,7 +136,7 @@ jobs: exit 0 } fi - if echo "$FILES" | grep -qE '\.go$|^go\.(mod|sum)$|^e2e/|^pkg/e2etest/|^internal/scaffold/fullsend-repo/|^internal/security/hooks/|^internal/dispatch/gcf/mintsrc/|^internal/sentencetoken/english\.json$|^Makefile$|^\.github/scripts/|^\.github/workflows/e2e\.yml$|^action\.yml$|^\.github/actions/check-e2e-authorization/|^scripts/check-e2e-authorization\.sh$'; then + if echo "$FILES" | grep -qE '\.go$|^go\.(mod|sum)$|^e2e/|^pkg/e2etest/|^internal/scaffold/fullsend-repo/|^internal/security/hooks/|^internal/dispatch/gcf/mintsrc/|^internal/sentencetoken/english\.json$|^Makefile$|^\.github/scripts/|^\.github/workflows/e2e\.yml$|^\.github/workflows/e2e-ok-to-test\.yml$|^action\.yml$|^\.github/actions/check-e2e-authorization/|^scripts/check-e2e-authorization\.sh$'; then echo "relevant=true" >> "$GITHUB_OUTPUT" else echo "::notice::No e2e-relevant files changed — skipping tests" @@ -226,7 +224,7 @@ jobs: } fi # pkg/e2etest: shared pool/CLI/cleanup; pkg/behaviourtest: framework; e2e/admin: admin-only helpers - if echo "$FILES" | grep -qE '^e2e/behaviour/|^e2e/admin/|^pkg/e2etest/|^pkg/behaviourtest/|^internal/runtime/|^internal/sandbox/|^internal/config/|^internal/cli/|^internal/layers/|^internal/scaffold/fullsend-repo/|^internal/forge/|^internal/harness/|^internal/harnessdispatch/|^internal/normevent/|^internal/dispatch/|^internal/security/hooks/|^internal/mintclient/|^cmd/fullsend/|^go\.(mod|sum)$|^Makefile$|^\.github/workflows/e2e\.yml$|^\.github/workflows/reusable-dispatch\.yml$|^\.github/actions/check-e2e-authorization/|^scripts/check-e2e-authorization\.sh$'; then + if echo "$FILES" | grep -qE '^e2e/behaviour/|^e2e/admin/|^pkg/e2etest/|^pkg/behaviourtest/|^internal/runtime/|^internal/sandbox/|^internal/config/|^internal/cli/|^internal/layers/|^internal/scaffold/fullsend-repo/|^internal/forge/|^internal/harness/|^internal/harnessdispatch/|^internal/normevent/|^internal/dispatch/|^internal/security/hooks/|^internal/mintclient/|^cmd/fullsend/|^go\.(mod|sum)$|^Makefile$|^\.github/workflows/e2e\.yml$|^\.github/workflows/e2e-ok-to-test\.yml$|^\.github/workflows/reusable-dispatch\.yml$|^\.github/actions/check-e2e-authorization/|^scripts/check-e2e-authorization\.sh$'; then echo "relevant=true" >> "$GITHUB_OUTPUT" else echo "::notice::No behaviour-relevant files changed — skipping behaviour tests" diff --git a/.github/workflows/functional-ok-to-test.yml b/.github/workflows/functional-ok-to-test.yml new file mode 100644 index 000000000..2973ba2ea --- /dev/null +++ b/.github/workflows/functional-ok-to-test.yml @@ -0,0 +1,20 @@ +name: Functional ok-to-test + +# Thin labeled-only entry point for fork / ok-to-test authorization. +# Calls Functional Tests via workflow_call so non-ok-to-test labels do not start +# skipped runs under the "Functional Tests" workflow name. +# +# No caller-level concurrency on the functional-{n} group — sharing that group +# between this caller and the reusable functional-tests.yml deadlocks. + +permissions: {} + +on: + pull_request_target: + types: [labeled] + +jobs: + run: + if: github.event.label.name == 'ok-to-test' + uses: ./.github/workflows/functional-tests.yml + secrets: inherit diff --git a/.github/workflows/functional-tests.yml b/.github/workflows/functional-tests.yml index 4b67da727..ebd4d3232 100644 --- a/.github/workflows/functional-tests.yml +++ b/.github/workflows/functional-tests.yml @@ -15,26 +15,25 @@ on: - 'internal/scaffold/**' - 'internal/cli/run.go' - '.github/workflows/functional-tests.yml' + - '.github/workflows/functional-ok-to-test.yml' - '.github/scripts/**' pull_request_target: - types: [opened, synchronize, reopened, labeled] + # labeled / ok-to-test is handled by functional-ok-to-test.yml via workflow_call. + types: [opened, synchronize, reopened] merge_group: workflow_dispatch: + workflow_call: -# Non-ok-to-test labeled events still start a workflow run (GitHub cannot filter -# by label name at on:), but must not cancel an in-progress opened/synchronize -# run — same actionable filter as the gate job if: below. concurrency: group: >- ${{ github.event_name == 'pull_request_target' && format('functional-{0}', github.event.pull_request.number) || format('{0}-{1}', github.workflow, github.ref) }} - # Explicit outer parens: (A && B) || (C && D). Relies on && binding tighter - # than ||; keep the grouping obvious so a future || clause cannot silently - # disable cancel for non-pull_request_target triggers. Keep in sync with e2e.yml. + # Label events are auth, not a code change — never cancel an in-progress run. + # Keep in sync with e2e.yml. Explicit outer parens: (A && B) || (C && D). cancel-in-progress: >- ${{ (github.event_name == 'pull_request_target' - && (github.event.action != 'labeled' || github.event.label.name == 'ok-to-test')) + && github.event.action != 'labeled') || (github.event_name != 'pull_request_target' && github.ref != 'refs/heads/main') }} jobs: @@ -42,9 +41,8 @@ jobs: # Separate job so pull-requests: write stays out of the job that checks # out fork head and runs tests with secrets. # Never checkout github.event.pull_request.head.sha here. - if: >- - github.event_name == 'pull_request_target' && - (github.event.action != 'labeled' || github.event.label.name == 'ok-to-test') + # workflow_call from functional-ok-to-test.yml still has event_name pull_request_target. + if: github.event_name == 'pull_request_target' runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: @@ -70,7 +68,6 @@ jobs: functional-tests: # For pull_request_target, runs only when gate sets authorized=true. - # Do not treat a skipped gate as authorized. # This job checks out untrusted PR head code — no pull-requests: write here. needs: gate if: >- @@ -113,7 +110,7 @@ jobs: exit 0 } fi - if echo "$FILES" | grep -qE '^eval/|^internal/scaffold/|^internal/cli/run\.go$|^\.github/workflows/functional-tests\.yml$|^\.github/scripts/'; then + if echo "$FILES" | grep -qE '^eval/|^internal/scaffold/|^internal/cli/run\.go$|^\.github/workflows/functional-tests\.yml$|^\.github/workflows/functional-ok-to-test\.yml$|^\.github/scripts/'; then echo "relevant=true" >> "$GITHUB_OUTPUT" else echo "::notice::No functional-test-relevant files changed — skipping tests" diff --git a/docs/guides/dev/e2e-testing.md b/docs/guides/dev/e2e-testing.md index 2c3f7df28..bc1082516 100644 --- a/docs/guides/dev/e2e-testing.md +++ b/docs/guides/dev/e2e-testing.md @@ -162,33 +162,38 @@ automatically and e2e is skipped until a maintainer re-applies it after reviewing the latest changes. Freshness compares the label timestamp against the frozen PR `updated_at` from the workflow event (`PR_UPDATED_AT`); the live API fallback may over-reject when non-push activity bumped `updated_at`. -Applying the label triggers immediate authorization on `labeled` events. +Applying the label triggers the **E2E ok-to-test** / **Functional ok-to-test** +caller workflows, which `workflow_call` into the main suites. + +The main **E2E Tests** and **Functional Tests** workflows do **not** subscribe to +`labeled` events. Only `opened` / `synchronize` / `reopened` cancel in-progress +work in the per-PR concurrency group (code changed). Label events are +authorization only: they **never** cancel an in-progress suite. If a suite is +already running when `ok-to-test` is applied, GitHub may queue a second run +behind it (no expression-only “skip if busy”); that is accepted. Other labels (for example `ready-for-review`, `requires-manual-review`, or -`component/*`) do **not** authorize e2e and do **not** cancel an in-progress -e2e or functional-test run for that PR. Only `opened` / `synchronize` / -`reopened`, and `labeled` when the label is `ok-to-test`, cancel in-progress -work in the per-PR concurrency group. GitHub still starts a workflow run for -every `labeled` event (labels cannot be filtered at `on:`). For non-actionable -labels the gate job is skipped entirely and no sticky `` -comment is posted. If an actionable run is already in progress, that new run -sits **pending** until the active run finishes (it does not skip immediately). -If several non-actionable labels land in a burst, GitHub cancels earlier -pending runs in the concurrency group; only the last dequeued run executes and -skips. Either way the original in-progress run is left alone. +`component/*`) do **not** authorize e2e. They may start the thin ok-to-test +caller with a skipped `run` job (GitHub cannot filter by label name at `on:`), +but they do not start skipped checks under the **E2E Tests** / **Functional +Tests** workflow names, and they never enter the suite concurrency group. ### Blocked runs When the gate **runs** and denies authorization, a sticky PR comment (marker ``) explains why and what to do. That is distinct from a -non-actionable label event, where the gate never runs and no comment appears. -Re-run the workflow or add/re-apply `ok-to-test` as appropriate. +non-`ok-to-test` label event, where the thin caller skips without invoking the +gate. Re-run the workflow or add/re-apply `ok-to-test` as appropriate. ## CI architecture -1. **Gate** — authorize the PR author or a fresh `ok-to-test` label (base +1. **PR open/sync** — **E2E Tests** / **Functional Tests** run gate then suite + jobs (trusted authors authorized immediately) +2. **`ok-to-test` label** — thin **E2E ok-to-test** / **Functional ok-to-test** + workflows call the same suites via `workflow_call` (fork / external path) +3. **Gate** — authorize the PR author or a fresh `ok-to-test` label (base checkout only; never checks out PR head) -2. **E2E** — checkout PR head SHA, authenticate to GCP via WIF, mint cross-org +4. **E2E** — checkout PR head SHA, authenticate to GCP via WIF, mint cross-org tokens per pool org, `make e2e-test` Pushes to `main`, merge queue, and `workflow_dispatch` skip the gate and run e2e From 15ac2929bc86575887335c6f88b042d45b56fd53 Mon Sep 17 00:00:00 2001 From: Barak Korren Date: Thu, 23 Jul 2026 13:03:06 +0300 Subject: [PATCH 2/2] ci(e2e): grant caller permissions for ok-to-test workflow_call Reusable workflows cannot elevate GITHUB_TOKEN beyond the caller's ceiling. Grant contents/pull-requests/id-token on the thin callers. Signed-off-by: Barak Korren Co-authored-by: Cursor --- .github/workflows/e2e-ok-to-test.yml | 6 ++++++ .github/workflows/functional-ok-to-test.yml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/.github/workflows/e2e-ok-to-test.yml b/.github/workflows/e2e-ok-to-test.yml index 282ddfbfb..96ed853d1 100644 --- a/.github/workflows/e2e-ok-to-test.yml +++ b/.github/workflows/e2e-ok-to-test.yml @@ -16,5 +16,11 @@ on: jobs: run: if: github.event.label.name == 'ok-to-test' + # Caller permissions are a ceiling for the reusable workflow — must grant + # the union of job perms in e2e.yml (gate / e2e / behaviour). + permissions: + contents: read + pull-requests: write + id-token: write uses: ./.github/workflows/e2e.yml secrets: inherit diff --git a/.github/workflows/functional-ok-to-test.yml b/.github/workflows/functional-ok-to-test.yml index 2973ba2ea..b516129cb 100644 --- a/.github/workflows/functional-ok-to-test.yml +++ b/.github/workflows/functional-ok-to-test.yml @@ -16,5 +16,11 @@ on: jobs: run: if: github.event.label.name == 'ok-to-test' + # Caller permissions are a ceiling for the reusable workflow — must grant + # the union of job perms in functional-tests.yml (gate / functional-tests). + permissions: + contents: read + pull-requests: write + id-token: write uses: ./.github/workflows/functional-tests.yml secrets: inherit