Skip to content

fix(e2e): use workflow_run trigger pattern to support fork PRs#10

Merged
derekmisler merged 4 commits into
mainfrom
fix/e2e-fork-pr-trigger
Jun 23, 2026
Merged

fix(e2e): use workflow_run trigger pattern to support fork PRs#10
derekmisler merged 4 commits into
mainfrom
fix/e2e-fork-pr-trigger

Conversation

@derekmisler

Copy link
Copy Markdown
Collaborator

Problem

test-pirate-agent and test-invalid-agent use secrets.OPENAI_API_KEY, which is empty on fork PRs. Tests silently fail (empty key → agent errors).

Solution

Three-part change mirroring the existing self-review-pr / self-review-pr-trigger pattern.

1. New test-e2e-trigger.yml

Lightweight pull_request workflow with permissions: {}. Writes PR number and head SHA to flat files and uploads as the e2e-test-context artifact (1-day retention). Runs in the base-repo context so the main workflow can download it.

2. test-e2e.yml converted to workflow_run

  • Drops pull_request trigger; adds workflow_run referencing "Test E2E Trigger"
  • New resolve-context job: fetches OIDC token via setup-credentials, downloads the artifact using GITHUB_APP_TOKEN, outputs pr-number and pr-head-sha
  • test-pirate-agent / test-invalid-agent: depend on resolve-context, checkout refs/pull/<N>/head, use OPENAI_API_KEY_FROM_SSM || secrets.OPENAI_API_KEY
  • test-output-extraction / test-job-summary: no secrets needed, run on both push and workflow_run
  • mention-reply jobs: unchanged (workflow_dispatch only)

3. self-review-pr-trigger.yml types narrowed

types: [ready_for_review, opened, review_requested]types: [review_requested] to reduce unnecessary trigger noise.

Fork PRs cannot access repository secrets, so agent E2E tests that require
an OpenAI API key silently skip (empty key). This patch applies the same
two-workflow trigger pattern already used by self-review-pr.

Change 1 — new test-e2e-trigger.yml:
  Lightweight pull_request workflow with zero permissions. Writes PR number
  and head SHA to flat files, uploads as the 'e2e-test-context' artifact
  (1-day retention). Runs in the base repo context so it can be referenced
  by workflow_run.

Change 2 — test-e2e.yml converted to workflow_run:
  - Remove pull_request trigger; add workflow_run referencing 'Test E2E Trigger'
  - Add resolve-context job: downloads e2e-test-context artifact via OIDC
    (setup-credentials + GITHUB_APP_TOKEN), outputs pr-number and pr-head-sha
  - test-pirate-agent and test-invalid-agent now depend on resolve-context,
    checkout the PR head ref, use setup-credentials for OIDC, and fall back
    through OPENAI_API_KEY_FROM_SSM before secrets.OPENAI_API_KEY
  - test-output-extraction and test-job-summary run on both push and
    workflow_run (no secrets needed); checkout PR head SHA on workflow_run
  - mention-reply jobs unchanged (workflow_dispatch only)

Change 3 — self-review-pr-trigger.yml:
  Narrow pull_request types from [ready_for_review, opened, review_requested]
  to [review_requested] to reduce unnecessary trigger noise.
@derekmisler derekmisler self-assigned this Jun 23, 2026
@derekmisler derekmisler marked this pull request as ready for review June 23, 2026 21:02
resolve-context has no checkout step so it must keep the pinned ref.
The four jobs that do check out the PR head (test-pirate-agent,
test-invalid-agent, test-mention-reply-toplevel, test-mention-reply-inline)
now use ./setup-credentials, consistent with how ./  and ./review-pr/mention-reply
are referenced in the same file.
./setup-credentials runs node dist/credentials.js, so dist/ must exist
first. Reorder steps in test-pirate-agent and test-invalid-agent:
checkout → pnpm setup → node setup → build → setup-credentials → run test.

The mention-reply jobs already had the correct order; no change there.
@derekmisler derekmisler enabled auto-merge (squash) June 23, 2026 21:07
Re-run E2E tests when a draft PR is marked ready for review.
@derekmisler derekmisler merged commit 2b632d2 into main Jun 23, 2026
8 checks passed
@derekmisler derekmisler deleted the fix/e2e-fork-pr-trigger branch June 23, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants