ci(e2e): bootstrap behaviour job on main for pull_request_target

[ifireball]

Why this PR looks strange

This is a bootstrap PR, not the behaviour test feature itself. The real work lives in #1982 (Gherkin suite, install drivers, dummy runtime, docs).

The pull_request_target problem

Our e2e workflow uses pull_request_target so fork PRs can receive OIDC/GCP secrets after the authorization gate. GitHub always loads the workflow definition from the PR base branch (main), not from the PR head.

PR #1982 adds a behaviour job to .github/workflows/e2e.yml on its branch, but that job cannot run in CI on #1982 until the workflow change is merged to main. Without this bootstrap, #1982 would merge blind — the behaviour job would only start working for PRs opened after #1982 lands.

What this PR does

Minimal wiring only:

• behaviour job in e2e.yml — same gate authorization as e2e, behaviour-specific path relevance filter
• make behaviour-test target
• Placeholder test TestBehaviourCIWiring — empty pass, //go:build behaviour, no godog deps

Once this merges, #1982's head will execute real behaviour tests under the new job. The placeholder is replaced by the full suite in #1982.

Merge order

1. This PR → puts behaviour job on main
2. ci(e2e): add Gherkin behaviour tests with dummy runtime #1982 → runs full behaviour tests in CI before merge

Test plan

• make lint (pre-commit on staged files; gofmt, go vet, actionlint)
• make behaviour-test — placeholder passes locally
• make go-test — no regressions
• CI behaviour job passes on this PR (will exercise the new job once merged to main's workflow — on this PR itself the base workflow may not include the job yet; after merge, re-run ci(e2e): add Gherkin behaviour tests with dummy runtime #1982)

Made with Cursor

[qodo-code-review]

PR Summary by Qodo

Bootstrap behaviour CI job in e2e workflow for pull_request_target

⚙️ Configuration changes 🧪 Tests ✨ Enhancement 🕐 20-40 Minutes

AI Description

• Add a new behaviour job to the e2e workflow with the same authorization gate as e2e.
• Run behaviour tests only when behaviour-relevant paths change, to avoid unnecessary CI.
• Provide a make behaviour-test target and a placeholder build-tagged test to wire CI end-to-end.

Diagram

flowchart TD
  A["GitHub event"] --> B["Authz gate"] --> C["Behaviour job"] --> D["Check changes"] --> G["GCP OIDC auth"] --> E["Run behaviour tests"] --> F["Go placeholder test"] --> H["Upload artifacts"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Run behaviour steps inside existing `e2e` job

• ➕ Avoids adding another job and duplicating gate/checkout/auth logic
• ➕ Simplifies parallelism and artifact handling
• ➖ Couples behaviour suite rollout to admin e2e cadence and timeouts
• ➖ Harder to path-skip independently; behaviour may bloat the e2e job duration

2. Use `pull_request` trigger and avoid secrets (mock/stub GCP)

• ➕ Workflow definition can live entirely in the PR head (no bootstrap needed)
• ➕ Reduced risk from running untrusted code with secrets
• ➖ Does not exercise the real end-to-end path that requires OIDC/GCP
• ➖ Would require parallel test strategy and may miss integration failures

3. Run behaviour suite post-merge via `workflow_run` / scheduled CI

• ➕ No need for pull_request_target secret exposure to PR code
• ➕ Keeps PR checks fast
• ➖ Removes pre-merge safety net; regressions are detected after landing
• ➖ Adds operational overhead (reverts/fixes) and longer feedback cycle

Recommendation: Keep the current bootstrap approach. Given pull_request_target loads workflows from main, landing a minimal behaviour job first is the most reliable way to ensure the real suite in #1982 is exercised pre-merge while preserving the existing authorization gate and secret-handling model.

Files changed (3) +118 / -3

Tests (1) +9 / -0

suite_test.goAdd placeholder behaviour build-tag test to validate CI wiring +9/-0

Add placeholder behaviour build-tag test to validate CI wiring

• Introduces a minimal '//go:build behaviour' test that always passes. This acts as a bootstrap placeholder so the new CI job compiles and runs until the full Gherkin suite is introduced in a follow-up PR.

e2e/behaviour/suite_test.go

Other (2) +109 / -3

e2e.ymlAdd gated 'behaviour' CI job with relevance filtering and artifact upload +104/-2

Add gated 'behaviour' CI job with relevance filtering and artifact upload

• Extends the workflow path filters and introduces a new 'behaviour' job gated by the existing authorization step. The job checks for behaviour-relevant file changes via the GitHub API, checks out the PR head when relevant, authenticates to GCP via OIDC, runs 'make behaviour-test', and uploads debug artifacts on failure.

.github/workflows/e2e.yml

MakefileAdd 'behaviour-test' target for behaviour-tagged Go tests +5/-1

Add 'behaviour-test' target for behaviour-tagged Go tests

• Adds a 'behaviour-test' phony target and help text. The target runs 'go test' in 'e2e/behaviour' with the 'behaviour' build tag and a 30-minute timeout.

Makefile

[github-actions]

Site preview

Preview: https://f2a24aae-site.fullsend-ai.workers.dev

Commit: ff7fa369a7cfd387d06e6ffa5d5b4700395236df

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 5:57 PM UTC · Ended 6:04 PM UTC
Commit: 104508d · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 51 rules

✅ Skills: writing-user-docs, writing-adrs

1. CLI changes skip behaviour ✗ Dismissed 🐞 Bug ☼ Reliability

Description

The behaviour job’s change-detection regex only matches internal/cli/github.go and
internal/cli/run.go, so changes to other CLI entrypoints like internal/cli/root.go are treated as
irrelevant and the behaviour job is skipped. This creates CI blind spots for behaviour tests
whenever CLI wiring/commands change.

Code

.github/workflows/e2e.yml[R216-221]

+          if echo "$FILES" | grep -qE '^e2e/behaviour/|^e2e/admin/|^internal/runtime/|^internal/sandbox/|^internal/config/|^internal/cli/github\.go$|^internal/cli/run\.go$|^internal/layers/|^internal/scaffold/|^internal/forge/|^internal/harness/|^internal/dispatch/|^internal/security/hooks/|^internal/mintclient/|^cmd/fullsend/|^go\.(mod|sum)$|^Makefile$|^\.github/workflows/e2e\.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"
+            echo "relevant=false" >> "$GITHUB_OUTPUT"
+          fi

Relevance

⭐⭐⭐ High

Team previously accepted expanding workflow relevance grep filters to avoid skipping needed tests
(e2e/functional).

PR-#2398
PR-#2534
PR-#2617

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The behaviour job’s filter only includes two specific internal/cli files, while the CLI is wired
via internal/cli/root.go (and other command files). Therefore changes to internal/cli/root.go
will not match the filter and will skip the behaviour job on PR/merge_group runs.

.github/workflows/e2e.yml[185-221]
internal/cli/root.go[33-54]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The behaviour job decides whether to run by grepping changed file paths, but the regex only includes `internal/cli/github.go` and `internal/cli/run.go`. Changes to other CLI wiring/commands (e.g. `internal/cli/root.go`) will incorrectly mark the PR as not relevant and skip the behaviour job.

### Issue Context
The fullsend CLI is assembled in `internal/cli/root.go` by registering many subcommands. Skipping behaviour tests on changes to these files undermines the job’s purpose once the real behaviour suite lands.

### Fix
Update the behaviour job’s `grep -qE` to treat all `internal/cli/` changes as relevant (e.g. replace the two file-specific entries with `^internal/cli/`), or explicitly enumerate all CLI files that should trigger behaviour tests.

### Fix Focus Areas
- .github/workflows/e2e.yml[216-221]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[fullsend-ai-review]

🤖 Finished Review · ❌ Failure · Started 5:57 PM UTC · Completed 6:04 PM UTC
Commit: 26fcbde · View workflow run →

[waynesun09]

Review Squad Report — 9 agents (Claude, Gemini, Codex)

5 verified MEDIUM findings posted inline. 3 false positives removed during verification. Security model is sound — gate authorization, minimal permissions, SHA-pinned actions, and persist-credentials: false all correctly replicate the existing e2e pattern. No critical or high issues.

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

[waynesun09]

Review Squad — 9 agents (Claude, Gemini, Codex)

5 verified MEDIUM findings posted inline. 3 false positives removed during verification. Security model is sound — gate authorization, minimal permissions, SHA-pinned actions, persist-credentials: false all correctly replicate the existing e2e pattern. No critical or high issues.

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:38 AM UTC · Completed 5:48 AM UTC
Commit: ff7fa36 · View workflow run →

[ifireball]

Babysit status

All CI checks are green on ff7fa369 (including codecov/patch and e2e).

Review feedback from the first pass was addressed in ff7fa369:

• Dropped redundant e2e/behaviour/** push.paths entry (e2e/** already covers it)
• Clarified SYNC-WITH: push.paths is the union of e2e + behaviour filters; each job grep may be narrower
• Broadened behaviour grep to ^internal/cli/ (fixes CLI blind-spot); narrowed scaffold to fullsend-repo/; documented e2e/admin/ shared helpers
• Aligned behaviour-test Makefile target with e2e-test (./e2e/behaviour/)

Branch is up to date with main (no merge conflicts).

Note: The behaviour job will not appear in this PR's E2E workflow run — expected for pull_request_target bootstrap PRs (workflow definition loads from base main until this merges).

Awaiting human review/approval.

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] .github/workflows/e2e.yml — This file is under the .github/ protected path. The PR provides sufficient context (references ci(e2e): add Gherkin behaviour tests with dummy runtime #1982 and Add Gherkin behaviour tests with dummy runtime and CI job #2276, explains the pull_request_target bootstrap need), but human approval is always required for protected-path changes regardless of justification.

---

Labels: PR adds a behaviour test CI job and e2e test scaffolding.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:34 AM UTC · Completed 7:41 AM UTC
Commit: ff7fa36 · View workflow run →

[fullsend-ai-retro]

Retro: PR #2793 — Bootstrap behaviour CI job

Workflow went well. This human-authored PR by ifireball added a behaviour CI job to the e2e workflow, a Makefile target, and a placeholder test. The review pipeline caught 5 valid MEDIUM-severity findings (redundant path entries, unclear grep patterns, Makefile inconsistency, comment clarity). ifireball addressed all findings in a single follow-up push. The second review passed, and rh-hemartin approved.

Timeline

1. Jun 30 17:55 — PR created (commit 104508d)
2. Jun 30 17:57–18:04 — Review run 28464973112 completed with Failure verdict (status comment initially showed "Terminated" mid-flight, then updated to "Failure" on completion)
3. Jun 30 18:01 — qodo-code-review posted 1 finding (dismissed)
4. Jun 30 18:29–18:31 — Review squad (waynesun09) posted 5 MEDIUM inline findings
5. Jul 1 05:32 — ifireball addressed all 5 findings, pushed commit ff7fa36
6. Jul 1 05:35–05:48 — Review run 28496018104 succeeded; protected-path finding posted (human approval required for .github/workflows/)
7. Jul 1 07:10 — rh-hemartin approved

Quality assessment

• Review quality: All 5 findings were valid and actionable. No false positives wasted human time. The protected-path gate for workflow files was appropriate.
• Rework rate: 1 iteration — all findings addressed in a single push. Efficient.
• Token cost: 2 review runs (one Failure, one Success). No obvious waste beyond the standard review-after-fix cycle.
• Time to resolution: ~13 hours from PR creation to approval, dominated by human timezone gap (overnight). Pipeline itself was responsive.

No new proposals

All potential improvement areas identified are already covered by existing open issues:

• Review termination/dedup on rapid pushes: Deduplicate review runs on rapid successive pushes #1418, Review status comment shows Terminated then posts a second Success comment for the same run #2572, Cancel in-flight review workflow when PR merges during review #2388, Remove stale-head re-dispatch: redundant with concurrency group, causes self-triggering #2399
• Protected-path severity tuning for human PRs: Review agent: downgrade protected-path severity for human-authored PRs #1551, Suppress protected-path finding when all changed files are in protected paths #1392, Bot gives identical "protected-path" review comments on every workflow run #2794, Refine .github/ protected path to target specific subdirectories instead of the entire prefix #2614
• Review coordination between systems: Review agent misses cross-file impact analysis that Review Squad catches #1525, Consolidate overlapping review-verdict escalation issues (#1453 and #1476) #1481