ci(e2e): delete leaked repo-level FULLSEND_MINT_URL from test-repo

[ralphbean]

Summary

• cfmint (the behaviour-test CF Worker preview mint driver from ci(#5346): add CF Worker preview mint to BT driver #6037) writes FULLSEND_MINT_URL as a repo-level variable on test-repo via fullsend github setup
• Repo-level variables take precedence over org-level ones, and nothing ever deleted this variable — since e2e admin tests and behaviour tests share the same org pool and both use a repo named test-repo, a leaked value from a prior behaviour-test run shadows the org-level FULLSEND_MINT_URL that admin install sets
• Because the CF Worker preview mint is ephemeral (torn down at the end of its run), the leaked URL is dead, so dispatch fails with a DNS resolution error for any run that later reuses the org — this is currently breaking TestAdminInstallUninstall's triage-dispatch phase intermittently across PRs
• Delete the repo variable in TeardownPerRepoInstall (immediate cleanup) and CleanupStaleResources (defensive cleanup so already-poisoned orgs in the pool self-heal)

Test plan

• go build ./pkg/e2etest/... ./pkg/behaviourtest/...
• go test ./pkg/e2etest/... ./pkg/behaviourtest/... ./internal/forge/...
• pre-commit run --files pkg/e2etest/cleanup.go

Assisted-by: Claude Opus 4.6 noreply@anthropic.com

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:24 PM UTC · Completed 6:34 PM UTC

Commit: 4144510 · View workflow run →

[qodo-code-review]

PR Summary by Qodo

CI(e2e): remove leaked repo-level FULLSEND_MINT_URL to stop pooled-org dispatch flakes

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Delete repo-level FULLSEND_MINT_URL during per-repo teardown to avoid shadowing org variables.
• Add defensive stale-resource cleanup for already-poisoned pooled orgs’ test-repo.
• Prevent intermittent triage/dispatch failures caused by dead, ephemeral preview mint URLs.

Diagram

graph TD
  A["E2E test runner"] --> B["CleanupStaleResources()"] --> D["forge.Client"] --> E[("Repo Actions variables")]
  A --> C["TeardownPerRepoInstall()"] --> D --> E

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Fix at the source (cfmint) to avoid writing repo-level FULLSEND_MINT_URL

• ➕ Eliminates the root cause; no cleanup required to prevent future leaks
• ➕ Reduces hidden precedence issues between org vs repo variables
• ➖ Doesn’t self-heal orgs already polluted in the pool
• ➖ Requires coordinating changes across test drivers and potentially other tooling

2. Make repo variable driver-scoped (e.g., FULLSEND_MINT_URL_CFMINT)

• ➕ Avoids clobbering the admin-install variable name
• ➕ Allows multiple drivers to coexist without precedence collisions
• ➖ Requires downstream consumers/dispatch logic to handle multiple names or selection rules
• ➖ Still needs cleanup to prevent accumulation of stale values

3. Avoid shared repo identity in the pool (unique repo per run)

• ➕ Prevents cross-run contamination from any repo-scoped artifacts, not just this variable
• ➖ Increases resource churn and cleanup complexity
• ➖ May be slower/costlier and require broader harness changes

Recommendation: This PR’s defense-in-depth cleanup is the right immediate fix because it both prevents new leaks (per-repo teardown) and self-heals already-poisoned pooled orgs (stale-resource cleanup). A follow-up to address the root cause in the per-repo driver (stop writing the shared variable name or reliably delete it) would further reduce future risk, but shouldn’t replace the defensive cleanup given the shared-org pool realities.

Files changed (1) +18 / -0

Bug fix (1) +18 / -0

cleanup.goDelete leaked FULLSEND_MINT_URL repo variable during e2e cleanup/teardown +18/-0

Delete leaked FULLSEND_MINT_URL repo variable during e2e cleanup/teardown

• Adds deletion of the repo-level FULLSEND_MINT_URL variable in both pooled-org stale cleanup (for test-repo) and per-repo driver teardown. This prevents a stale repo-scoped variable from overriding the org-level FULLSEND_MINT_URL and breaking later dispatch when the leaked value points at an ephemeral, already-destroyed mint endpoint.

pkg/e2etest/cleanup.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Looks good to me

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 9:41 PM UTC · Completed 9:54 PM UTC

Commit: 4144510 · View workflow run →

[fullsend-ai-retro]

Retro: PR #6112 — ci(e2e): delete leaked repo-level FULLSEND_MINT_URL from test-repo

What happened

PR #6037 (authored by fullsend-ai-coder, merged 2026-08-11 12:27 UTC) introduced the cfmint behaviour-test driver. This driver calls fullsend github setup --mint-url which writes FULLSEND_MINT_URL as a repo-level variable on test-repo, but the driver's Teardown() only tears down the CF Worker preview — it never deletes the repo-level variable. Because repo-level variables shadow org-level variables, and the preview mint is ephemeral, the leaked variable pointed to a dead URL, causing intermittent DNS failures in TestAdminInstallUninstall for subsequent test runs reusing the same org.

PR #6112 (authored by ralphbean, assisted by Claude Opus 4.6) added cleanup in both TeardownPerRepoInstall (immediate) and CleanupStaleResources (self-healing for already-poisoned orgs). The fix is surgical and defensive — errors are logged as warnings but don't fail cleanup.

Timeline

Time (UTC)	Event
18:23	PR #6112 created by ralphbean
18:24	Review agent started (run 31522475917)
18:25	Qodo review: 0 bugs, 0 violations
18:31	Human review (ifireball): APPROVED (~8 min from creation)
18:34	Review agent: APPROVED ("Looks good to me"), ready-for-merge label applied
18:35	First merge queue attempt bounced after ~79 seconds
21:16	Second merge queue attempt
21:39	Merged

Workflow quality

PR #6112's review went well. The review agent ran 4 sub-agents (correctness, style-conventions, intent-coherence, docs-currency) in parallel. The docs sub-agent identified two real documentation gaps (e2e-testing.md and behaviour-drivers.md don't document the FULLSEND_MINT_URL repo-variable precedence or cleanup requirement), but the challenger sub-agent correctly removed them as scope creep for a bug fix PR. The final verdict was a clean APPROVE with no actionable findings — the correct outcome.

The interesting quality gap is upstream, on PR #6037's review. The review agent on #6037 caught several resource-leak issues (Worker preview leak on partial failure, teardownPreview naming asymmetry), but missed the cross-boundary repo variable leak. The miss is understandable: the variable is created as a side effect of calling fullsend github setup (defined in internal/repos/install.go, not in the diff), and catching it required tracing CLI command side effects across subsystems plus understanding the shared-pool interaction between behaviour tests and admin tests.

Existing issues that cover the identified gaps

All improvement opportunities from this retro are already tracked:

• Review correctness sub-agent should detect missing cleanup for test-shared-state mutations agents#640 — "Review correctness sub-agent should detect missing cleanup for test-shared-state mutations." This retro provides additional evidence: the cfmint driver on PR ci(#5346): add CF Worker preview mint to BT driver #6037 created a shared-state mutation (repo-level FULLSEND_MINT_URL) without cleanup, and the review agent did not detect it.
• Correctness sub-agent should check cleanup/init consistency across all exit paths agents#669 — "Correctness sub-agent should check cleanup/init consistency across all exit paths." Also directly relevant: the Teardown() method cleaned up the Worker preview but not the repo variable created during Install().
• test(e2e): behaviour suite must not install or allowlist singular test-repo #6119 — "test(e2e): behaviour suite must not install or allowlist singular test-repo" (filed by the reviewer ifireball). Addresses the architectural root cause: if behaviour tests stop sharing test-repo with admin tests, this class of variable-leak bug is eliminated.
• Post-review script should suppress APPROVE when human review comments arrive during the review run agents#386 — "Post-review script should suppress APPROVE when human review comments arrive during the review run." Relevant because the agent's approval arrived 3.5 minutes after the human had already approved, providing no incremental value on this PR.

Assessment

No new proposals are warranted. The workflow on PR #6112 executed correctly — the human diagnosed the flake, wrote a clean fix with good documentation, and the review agent reached the right verdict. The upstream gap (PR #6037's missed cleanup) is already tracked by existing improvement issues in the agents repo. The reviewer's follow-up issue (#6119) demonstrates good engineering practice by addressing the architectural root cause.