ci(#5441): enable parallel godog concurrency (default 12)

[fullsend-ai-coder]

Summary

Enable parallel godog scenario execution in the behaviour suite, defaulting to GODOG_CONCURRENCY=12 (matching the repo pool size). This removes the serial-only guard that was in place while per-scenario isolation prerequisites were being built.

Related Issue

Closes #5441 (child of epic #3454)

Changes

• e2e/behaviour/suite_test.go: Remove the serial-only guard that fataled on GODOG_CONCURRENCY != 1. Parse GODOG_CONCURRENCY from the environment as an integer (default 12) and pass it to godog.Options.Concurrency.
• Makefile: Add -race to the behaviour-test target so concurrent scenarios are validated under the Go race detector.
• docs/guides/dev/behaviour-testing.md: Document GODOG_CONCURRENCY env var, the default of 12, and how to use serial mode (GODOG_CONCURRENCY=1) for debugging.

Testing

• go vet -tags behaviour ./e2e/behaviour/ passes
• go test -tags behaviour -run "^$" ./e2e/behaviour/ compiles successfully
• make go-test passes for all packages (pre-existing infra failures in internal/binary, internal/cli, internal/fetch are unrelated)
• GODOG_CONCURRENCY=12 make behaviour-test — requires live GitHub infra (CI validation)
• GODOG_CONCURRENCY=1 make behaviour-test — serial fallback (CI validation)

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• No workflow file changes (avoiding GitHub App workflows permission limitation)
• CI picks up default concurrency of 12 without needing e2e.yml env var changes

---

Closes #5441

Post-script verification

• Branch is not main/master (agent/5441-enable-godog-concurrency)
• Secret scan passed (gitleaks — 70e5d9132d67e0afd75632ceeed7f77c738d8ba8..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:30 AM UTC · Completed 10:44 AM UTC
Commit: f902ffe · View workflow run →

[github-actions]

Site preview

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

Commit: 4cee34ea49f21e506b899c728331a11ab1666e09

[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

• [pool-concurrency mismatch] e2e/behaviour/suite_test.go:80 — The repo pool size is hardcoded to 12 (NewRepoPool(12)) while godog concurrency is configurable via GODOG_CONCURRENCY. If a user sets GODOG_CONCURRENCY above 12, extra godog workers will block in pool.Acquire waiting for a lease, silently capping real parallelism at 12 with no error or warning.
  Remediation: Add a validation check after computing concurrency that fatals or warns if concurrency > 12. Do NOT use NewRepoPool(concurrency) as the sole fix because the pool org only has 12 enrolled repos (test-repo-01 through test-repo-12) with per-repo mint enrollment — creating pool entries for test-repo-13+ would fail at runtime due to missing enrollment. The simplest correct fix is: if concurrency > 12 { t.Fatalf("GODOG_CONCURRENCY=%d exceeds repo pool size 12", concurrency) }. Alternatively, derive a const for the pool size and use it in both places.

• [stale-doc] docs/guides/dev/behaviour-drivers.md:35 — Line 35 states that numbered test-repo-01 through test-repo-12 "are also enrolled for planned parallelization; the driver does not select them yet." This is now incorrect: the PR enables parallel execution by default and NewRepoPool(12) actively leases these repos to concurrent scenarios.
  Remediation: Update the sentence to reflect that parallel execution is now enabled and the numbered repos are actively leased from the RepoPool.

---

Labels: PR modifies e2e behaviour test suite and test documentation

[fullsend-ai-review]

See the review comment for full details.

---

Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:

• e2e/behaviour/suite_test.go (file-level): Line 80 · [medium] pool-concurrency mismatch

The repo pool size is hardcoded to 12 (NewRepoPool(12)) while godog concurrency is configurable via GODOG_CONCURRENCY. If a user sets GODOG_CONCURRENCY above 12, extra godog workers will block in pool.Acquire waiting for a lease, silently capping real parallelism at 12 with no error or warning.

Suggested fix: Add a validation check after computing concurrency that fatals or warns if concurrency > 12. Do NOT use NewRepoPool(concurrency) as the sole fix because the pool org only has 12 enrolled repos with per-repo mint enrollment. The simplest correct fix is: if concurrency > 12 { t.Fatalf("GODOG_CONCURRENCY=%d exceeds repo pool size 12", concurrency) }.

[ifireball]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 11:13 AM UTC · Completed 11:24 AM UTC
Commit: f902ffe · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed both review findings: (1) added poolSize const and validation guard preventing GODOG_CONCURRENCY from exceeding repo pool size, and (2) updated stale documentation in behaviour-drivers.md to reflect that parallel execution is now enabled.

Fixed (2):

1. pool-concurrency mismatch (e2e/behaviour/suite_test.go): Added poolSize const (12) used by both the GODOG_CONCURRENCY validation and NewRepoPool call. Added a t.Fatalf guard when concurrency exceeds poolSize so extra godog workers no longer silently block.
2. stale-doc (docs/guides/dev/behaviour-drivers.md): Updated line 35 to state that numbered test-repo-01 through test-repo-12 are now actively leased from world.RepoPool during parallel execution, replacing the outdated 'planned parallelization' language.

Tests: passed

_{Updated by fullsend fix agent}

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 11:52 AM UTC · Completed 12:07 PM UTC
Commit: 4cee34e · View workflow run →

[fullsend-ai-retro]

Retro: PR #5636 — enable parallel godog concurrency

Verdict: Workflow executed well. The code→review→fix pipeline completed in ~1.5 hours with one rework iteration. All improvement areas are already tracked by existing open issues — no new proposals.

Timeline

Time (UTC)	Event
10:15	Code agent dispatched via /fs-code on #5441
10:28	PR #5636 created (14-min run)
10:29–10:44	Review agent ran (run 30258248835), found 2 medium findings
11:12	Human (ifireball) triggered /fs-fix (~28 min after review)
11:12–11:24	Fix agent ran (run 30261028296), fixed both findings ($1.34, 34 turns)
11:19	Human approved — no additional findings
11:49	PR merged

What went well

• Review quality was strong. The review agent found two genuine issues: (1) a pool-concurrency mismatch where GODOG_CONCURRENCY > 12 would cause workers to silently block in pool.Acquire, and (2) stale documentation in behaviour-drivers.md claiming parallelization was "planned" when the PR enables it. Both findings were validated by the human choosing to fix them.
• Challenger sub-agent worked correctly. It filtered out 2 noise findings (trivial code-comment-clarity and duplicate documentation accuracy) while retaining the 2 genuine ones.
• Fix agent was efficient. Addressed both findings in a single iteration: added a poolSize const with validation guard and updated the stale doc line. One rework cycle, $1.34.
• No false positives. The human approved without additional comments, confirming the review and fix were both correct.

Evidence for existing issues

• Code agent should compare against referenced source pattern and apply defensive coding to suggested code #4306, Code agent should preserve existing safety guards when modifying conditional expressions #1984, Code agent should trace all consumers of modified gating variables before committing #2873 (code agent defensive coding): The code agent made GODOG_CONCURRENCY configurable but didn't add a bounds check against the pool size, despite the issue description stating "CI default 12, matching pool size." The coupled constraint was hinted in the issue text but not enforced in code.
• Review agent should request changes for correctness bugs regardless of severity rating #1476 (review should request changes for correctness bugs): The review agent found a genuine correctness gap (silent blocking at GODOG_CONCURRENCY > 12) but chose comment-only, reasoning that "neither finding would break the PR's core functionality at default values." However, the PR's purpose was to make concurrency configurable, so non-default values are within its intended scope. Had the review agent chosen request_changes, the fix pipeline could have auto-engaged.
• Auto-dispatch fix agent on critical review findings for bot-authored PRs #2596, Shouldn't require human to poke the agent when it breaks CI #5350 (auto-fix for bot PRs): The 28-minute gap between review posting and /fs-fix trigger was purely human latency — the human's only role was procedural. This provides evidence that the auto-fix threshold should extend beyond critical findings to medium correctness findings on bot-authored PRs.
• post-review.sh fails with 422 when REVIEW_TOKEN undefined after PR #2389 #2990 (review 422 on inline comments): The review agent hit a GitHub 422 when posting inline comments on suite_test.go. It fell back to posting the content as a regular comment, so the finding was still visible.

Non-issues investigated

• Dual code runs: Two code.yml runs around the same time were confirmed to be for different issues (Missing SECURITY.md #5585 and ci(e2e): enable GODOG_CONCURRENCY for behaviour suite (default 12) #5441). No redundant dispatch.
• Post-merge fix failure: The failed fix run (30263592039) was for PR feat(#5631)!: make --preview accept alias for real Wrangler preview upload #5639, not ci(#5441): enable parallel godog concurrency (default 12) #5636. Its eligibility check correctly rejected a bot-triggered fix on a human-authored PR without the fullsend-fix label.