Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/e2e-ok-to-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
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'
# 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
Comment thread
ifireball marked this conversation as resolved.
secrets: inherit
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
26 changes: 12 additions & 14 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,39 +37,38 @@ 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:
gate:
# 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:
Expand All @@ -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: >-
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
26 changes: 26 additions & 0 deletions .github/workflows/functional-ok-to-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
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'
# 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
23 changes: 10 additions & 13 deletions .github/workflows/functional-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,36 +15,34 @@ 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:
gate:
# 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:
Expand All @@ -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: >-
Expand Down Expand Up @@ -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"
Expand Down
37 changes: 21 additions & 16 deletions docs/guides/dev/e2e-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<!-- e2e-gate -->`
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
`<!-- e2e-gate -->`) 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
Expand Down
Loading