Skip to content

ci: run fork-PR e2e shards on ubuntu-24.04 - #3590

Merged
brandon-pereira merged 1 commit into
mainfrom
claude/e2e-fork-runner
Nov 6, 2026
Merged

brandon-pereira merged 1 commit into
mainfrom
claude/e2e-fork-runner

Conversation

@brandon-pereira

Copy link
Copy Markdown
Member

Summary

Since RUNNER_E2E was switched to medium-runner-ubuntu-x64 (#3573), E2E shards on fork PRs stay queued forever and never get a runner. Same-repo branches run fine. Examples: #3554 (queued 2h, then cancelled by a new push) and #3582.

The runner group most likely doesn't allow fork PRs. That is also the safer default for a custom runner group: fork code shouldn't run on it.

This change sends fork PRs (github.event.pull_request.head.repo.fork) to the GitHub-hosted ubuntu-24.04 runner. Because that runner has only 4 vCPUs, the fork path also uses:

Same-repo PRs, main, and other events are unchanged.

CI-only. No changeset needed.

How to reproduce

On main: open the checks on a fork PR such as #3554. All four e2e-tests / E2E Tests - Shard N jobs stay Queued.

On this branch: once merged, rerun Main on a fork PR. The shards should start on ubuntu-24.04. This PR comes from a same-repo branch, so it only checks that the non-fork path still uses RUNNER_E2E.

@vercel

vercel Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated
clickstack-oss Ignored Ignored Nov 6, 2026 10:38pm UTC
clickstack-storybook Ignored Ignored Nov 6, 2026 10:38pm UTC

Request Review

@brandon-pereira
brandon-pereira requested a review from a team as a code owner November 6, 2026 22:38
@github-actions github-actions Bot added the review/tier-1 Trivial — auto-merge candidate once CI passes label Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

🟢 Tier 1 — Trivial

Docs, images, lock files, a dependency bump, or an automated release. No functional code changes detected.

Why this tier:

  • All files are docs / images / lock files

Review process: Auto-merge once CI passes. No human review required.
SLA: Resolves automatically.

Stats
  • Production files changed: 0
  • Production lines changed: 0
  • Branch: claude/e2e-fork-runner
  • Author: brandon-pereira

To override this classification, remove the review/tier-1 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

PR Review

✅ No issues found.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Duplicate code analysis

No new duplicate code introduced by this PR.


jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.21%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This is a clean, well-scoped CI change. Reviewers confirmed the GitHub Actions expression semantics are correct: && binds tighter than ||, every middle operand ('ubuntu-24.04', 25, '2') is truthy so there is no falsy-middle-value gotcha, and github.event.pull_request.head.repo.fork is null-safe on non-pull_request events (property access on a null pull_request yields null, routing main/push and same-repo PRs down the byte-identical original chain). The caller (.github/workflows/main.yml) invokes this via the pull_request trigger, so fork code already runs with a read-only token and no secrets; moving it off the self-hosted custom runner group onto ephemeral ubuntu-24.04 is security-positive.

🔵 P3 nitpicks (2)
  • .github/workflows/e2e-tests.yml:17 — Fork routing keys on head.repo.fork, which is true for a head repo that is a fork of any repository rather than specifically a cross-repo PR against this one; it is correct only because this repo is not itself a fork and same-repo PRs report fork == false.
    • Fix: Consider gating on github.event.pull_request.head.repo.full_name != github.repository to make the cross-repo intent explicit and robust to future repo-topology changes.
    • correctness-reviewer
  • .github/workflows/e2e-tests.yml:17 — The fork predicate is repeated across three expressions (runs-on line 17, timeout-minutes line 20, E2E_WORKERS line 39), each paired with its own hardcoded fork-mode constant that must stay in sync; GitHub Actions blocks the obvious DRY fix since runs-on cannot reference job-level env.
    • Fix: Add a short comment noting the three fork sites are intentionally coupled so a future editor updates all of them together.
    • maintainability-reviewer

Reviewers (5): correctness, reliability, security, project-standards, maintainability.

Testing gaps: The 25-minute timeout for 2-worker shards on a 4-vCPU runner is unproven and can only be validated by a live fork-PR run; with fail-fast: false, the slowest single shard — not the average — must fit under 25 minutes. No automated assertion locks in that fork PRs resolve runs-on to ubuntu-24.04 while non-fork runs resolve to the custom runner group, so a future reorder of the expression could silently route fork code back onto the self-hosted group.

No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 464 passed • 1 skipped • 1299s

Status Count
✅ Passed 464
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@brandon-pereira
brandon-pereira merged commit 6318094 into main Nov 6, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-1 Trivial — auto-merge candidate once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants