ci(e2e): always report status on PRs, short-circuit for irrelevant paths

[ralphbean]

Summary

• Remove paths: filter from pull_request_target so the e2e workflow triggers on all PRs
• Add a path-relevance check that short-circuits when no e2e-relevant files changed
• Docs-only and config-only PRs now get a passing e2e status instead of no status at all

This restores the approach from #1988 which was lost when the e2e workflow was refactored to use pull_request_target with a gate/e2e job split. Without this fix, any PR that doesn't touch Go files is blocked from the merge queue because e2e is a required check that never starts.

Fixes #1989

Test plan

• Open a docs-only PR and verify e2e reports a passing status with the "No e2e-relevant files changed" notice
• Open a Go-touching PR and verify e2e runs the full test suite as before
• Verify a docs-only PR can be enqueued in the merge queue

🤖 Generated with Claude Code

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:07 PM UTC · Completed 9:19 PM UTC
Commit: fe94a21 · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] .github/workflows/e2e.yml — This file is under the .github/ protected path. The PR links to issue ci: add test, e2e, commit-lint, and DCO as required status checks #1989 and the description explains the rationale (making e2e always report a status so it can be a required check). Human approval is required for protected-path changes regardless of context.

Low

• [error-handling] .github/workflows/e2e.yml:91 — The shell script in the "Check for e2e-relevant changes" step does not use set -euo pipefail. The explicit || { ... } handler covers gh api failure correctly (fails safe by setting relevant=true). However, if the pipe echo "$FILES" | grep -qE ... encounters an unexpected error in echo, grep receives no input, returns 1, and the else branch sets relevant=false, which would skip tests. This is extremely unlikely in practice.

• [pattern-consistency] .github/workflows/e2e.yml:91 — The inline grep pattern and the push.paths filter must be kept in sync manually. The PR adds SYNC-WITH comments in both locations, which is a reasonable mitigation. No further action needed beyond maintaining the comments.

Info

• [workflow-trigger-scope] .github/workflows/e2e.yml:25 — Removing the paths: filter from pull_request_target causes this workflow to trigger on every PR event. However, the existing gate job with its authorization check runs before any fork code is checked out, and the new path-relevance check short-circuits before secrets are accessed. The fail-open direction defaults to running tests (not skipping), which is the safer behavior. No net security regression.

• [intent-scope-alignment] .github/workflows/e2e.yml — The PR correctly implements the prerequisite portion of issue ci: add test, e2e, commit-lint, and DCO as required status checks #1989. The issue has broader scope (also adding test, commit-lint, and DCO as required checks). This PR is a prerequisite, not the full fix.

Previous run

Review

Findings

Medium

• [protected-path] .github/workflows/e2e.yml — This file is under the .github/ protected path. The PR links to issue ci: add test, e2e, commit-lint, and DCO as required status checks #1989 and the description explains the rationale (making e2e always report a status so it can be a required check). Human approval is required for protected-path changes regardless of context.

Low

• [edge-case] .github/workflows/e2e.yml:91 — The grep pattern for .github/workflows/e2e.yml and .github/actions/check-e2e-authorization/ lack a ^ anchor, unlike the other patterns. A file like vendor/.github/workflows/e2e.yml would over-match. This only causes unnecessary test runs (safe direction), not missed runs.
  Remediation: Anchor the patterns with ^.

• [pattern-consistency] .github/workflows/e2e.yml:92 — The inline grep pattern duplicates the paths: filter that remains on the push trigger. When e2e-relevant paths change, both locations must be kept in sync manually.
  Remediation: Add a comment noting the sync requirement, or extract the path list to a reusable form.

• [error-handling] .github/workflows/e2e.yml:91 — The new step does not use set -euo pipefail. However, the failure mode is benign: if gh api fails, FILES is empty, grep won't match, and relevant=false is set — a safe default that skips tests rather than blocking the PR.

• [security-timing-window] .github/workflows/e2e.yml:92 — The path-relevance check runs in the e2e job rather than the gate job. It runs before checkout and uses only github.token (not secrets), so the actual risk is minimal.

Info

• [workflow-trigger-scope] .github/workflows/e2e.yml:25 — The paths: filter is removed from pull_request_target, broadening the trigger to all PRs. The authorization gate job still guards secret access, so this does not expand the attack surface.

• [intent-traceability] .github/workflows/e2e.yml — The PR states "Fixes ci: add test, e2e, commit-lint, and DCO as required status checks #1989" but issue ci: add test, e2e, commit-lint, and DCO as required status checks #1989 has broader scope (also adding test, commit-lint, and DCO as required checks). This PR is a prerequisite, not the full fix.

[waynesun09]

Review Squad — 4 findings (MEDIUM+)

Agents: 5 (Claude x3, Gemini, Codex) · Models: Claude, Gemini, Codex

Two findings below are inline on the changed lines. Two more reference unchanged code (not suitable for inline):

---

MEDIUM — Regex pattern divergence risk with push.paths filter

Lines: push.paths (L12-24) vs grep regex (L92)

The push.paths glob list and the grep -qE regex encode the same path set in two syntaxes. They must stay synchronized manually — adding a path to one but not the other causes silent divergence.

Suggestion: Add sync comments above both locations:

# SYNC-WITH: grep regex in "Check for e2e-relevant changes" step
paths:

# SYNC-WITH: push.paths filter above
if echo "$FILES" | grep -qE '...'; then

Flagged by 1/5 agents

---

MEDIUM — Gate job runs on all PRs including docs-only

Lines: gate job (L45-78, unchanged)

Removing paths: from pull_request_target means the gate job now runs checkout + authorization (~1-2 min) for every PR, including docs-only PRs that will short-circuit in the e2e job.

Suggestion: Acceptable trade-off for the initial fix. Can optimize in a follow-up by moving the path-relevance check into gate and propagating a relevant output.
Flagged by 3/5 agents

---

Assisted-by: Claude (review x3), Gemini (review), Codex (review)

[waynesun09]

HIGH — Missing ^ anchor on .github/ regex patterns

Two patterns lack start anchors: \.github/workflows/e2e\.yml$ and \.github/actions/check-e2e-authorization/. All other exact-match patterns are correctly anchored (^go\.(mod|sum)$, ^Makefile$, etc.).

The original paths: globs were implicitly root-anchored by GitHub — the regex should match. A path like vendor/.github/workflows/e2e.yml would incorrectly trigger a full e2e run.

Suggestion:

^\.github/workflows/e2e\.yml$|^\.github/actions/check-e2e-authorization/

Flagged by 4/5 agents (strong consensus)

[waynesun09]

MEDIUM — No error handling if gh api fails

If the API is unavailable, rate-limited, or errors, $FILES is empty → grep doesn't match → relevant=false is set — silently skipping e2e tests on a PR that may need them.

Suggestion: Default to running tests on API failure:

FILES=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/files" --paginate --jq '.[].filename') || {
  echo "::warning::Failed to fetch PR files — running e2e tests as a precaution"
  echo "relevant=true" >> "$GITHUB_OUTPUT"
  exit 0
}

Flagged by 3/5 agents (consensus)

[github-actions]

Site preview

Preview: https://657bcdf5-site.fullsend-ai.workers.dev

Commit: 6f20434fea6ca73384eecde9d105ad425be6ce69

[waynesun09]

All 4 findings from the review squad addressed. LGTM.

Assisted-by: Claude (review x3), Gemini (review), Codex (review)

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:31 PM UTC · Completed 9:42 PM UTC
Commit: 6f20434 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 12:06 AM UTC · Completed 12:14 AM UTC
Commit: 6f20434 · View workflow run →

[fullsend-ai-retro]

Retro: PR #2398 — ci(e2e): always report status on PRs, short-circuit for irrelevant paths

Workflow went well. This was a clean, efficient cycle: human-authored CI fix → review agent + review squad caught real issues (missing regex anchor, missing error handling) → author fixed in one iteration → merged within ~3 hours.

Timeline

Time (UTC)	Event
21:03	PR opened by ralphbean (1 file, +30/−17)
21:04–21:20	First review run (27719653650) finds edge-case about missing ^ anchor, error-handling, pattern-consistency
21:12	ifireball approves
21:23	waynesun09 posts review squad findings — elevates missing ^ anchor to HIGH (4/5 agents consensus)
21:27	ralphbean pushes fix commit addressing all findings
21:28–21:42	Second review run (27720917064) confirms fixes
21:31	waynesun09 approves
00:02	PR merged

Observations

1. Severity calibration gap (existing issue): The review agent found the missing ^ regex anchor on the first pass but categorized it as a low-priority edge case. The review squad correctly elevated it to HIGH with strong consensus (4/5 agents). This pattern — correctness bugs under-rated as low — is already tracked by #1144.

2. Full re-review on fix commits (existing issue): The second review run took 14 minutes to re-evaluate a small fix commit from scratch rather than doing targeted verification of prior findings. This is tracked by #1552.

Proposals

No new proposals filed — the two improvement areas identified are already covered by existing open issues (#1144, #1552). The workflow performed well overall with a single rework iteration and meaningful review findings.