test(#6169): suite uses unified Driver for repo allocate/deallocate/finalize by fullsend-ai-coder[bot] · Pull Request #6260 · fullsend-ai/fullsend

fullsend-ai-coder · 2026-08-16T13:50:15Z

Summary

Refactors the behaviour test suite to use a unified install.Driver (AllocateRepo/DeallocateRepo/Finalize/Capacity) instead of manually composing RepoEnsurer + RepoPool + MintDriver. The suite now takes a single Factory/Driver and does not construct or thread those components itself.

Related Issue

Implements #6169 (child of #6135).

Changes

install.Driver renamed to install.MintDriver to free the Driver name for the unified interface (transitional until test(behaviour): fold RepoPool and RepoEnsurer into environment install drivers #6170)
Added install.Factory type and unified install.Driver interface with AllocateRepo/DeallocateRepo/Finalize/Capacity per the contract defined in test(behaviour): consolidate install driver, RepoEnsurer, and RepoPool behind one driver interface #6135
Added composedDriver (internal helper) that wraps MintDriver + RepoEnsurer + channel-based pool into a unified Driver
Added cfmint.NewFactory that closes over PEM/pool config, deploys the preview mint, creates the ensurer, and returns the composed Driver
Simplified suite_test.go — uses factory instead of ad-hoc composition; no direct RepoPool, RepoEnsurer, or mint URL threading
Changed suite.InitScenario signature from (sc, template, pool) to (sc, template) — pool is internal to the driver
Replaced world.Ensurer with world.Driver (unified driver field)
Updated givenEnrolledTestRepository to call Driver.AllocateRepo with double-allocation guard
After hook calls Driver.DeallocateRepo instead of pool.Release
Concurrency > Capacity is an advisory t.Logf warning, not a fatal (per test(behaviour): consolidate install driver, RepoEnsurer, and RepoPool behind one driver interface #6135)
Finalize uses errors.Join for leak + mint teardown errors
Updated docs in behaviour-drivers.md and behaviour-testing.md for Factory/Driver, API changes

Testing

All existing pkg/behaviourtest/... unit tests pass with -race
New composed_test.go covers: allocate/deallocate, double-dealloc guard, unknown name guard, blocking when at capacity, ensure error returns slot to pool, finalize with/without outstanding leases, errors.Join behavior, concurrent allocate/deallocate under -race
Updated suite/init_test.go tests for new Before/After hooks with unified driver
go vet passes for all changed packages (including -tags behaviour)
Secret scan clean

Checklist

PR title follows Conventional Commits (correct type, ! for breaking changes)
Tests added/updated for new or modified logic

Closes #6169

Post-script verification

Branch is not main/master (agent/6169-unified-driver-suite)
Secret scan passed (gitleaks — 53456d472dcba81bcea79f8d715e9d6159cefa5c..HEAD)
PR body secret scan passed (gitleaks — no-git)

…inalize Refactor the behaviour test suite to speak only to a unified install.Driver surface (AllocateRepo/DeallocateRepo/Finalize/ Capacity) instead of manually composing RepoEnsurer + RepoPool + MintDriver. Key changes: - Rename install.Driver → install.MintDriver to free the name for the unified interface. - Add install.Factory type and install.Driver interface per #6135. - Add composedDriver (internal) that wraps MintDriver + RepoEnsurer + channel-based pool into a unified Driver. - Add cfmint.NewFactory that closes over PEM/pool config, deploys the mint, creates the ensurer, and returns the composed Driver. - Update suite_test.go to use the factory — no ad-hoc composition of RepoEnsurer, RepoPool, or mint URL threading. - Update suite.InitScenario signature from (sc, template, pool) to (sc, template) — pool is internal to the driver. - Replace world.Ensurer with world.Driver (unified driver). - Update givenEnrolledTestRepository to call Driver.AllocateRepo with double-allocation guard. - After hook calls Driver.DeallocateRepo instead of pool.Release. - Concurrency > Capacity is an advisory t.Logf, not a fatal. - Finalize uses errors.Join for leak + mint teardown errors. - Update behaviour-drivers.md and behaviour-testing.md docs. Closes #6169

fullsend-ai-review · 2026-08-16T13:51:31Z

🤖 Review · ⚠️ Cancelled · Started 1:51 PM UTC · Ended 2:04 PM UTC

Commit: 8ecf8d8 · View workflow run →

github-actions · 2026-08-16T13:51:59Z

Site preview

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

Commit: d5382c788f498b81bea5777e1830fae7d5c9a404

ifireball · 2026-08-16T13:56:20Z

/fs-fix CI test failed on make lint-all → lint-md-links.

Stale fragment #lazy-createinstall-repoensurer is linked from:

docs/guides/dev/e2e-testing.md (~line 81)
docs/guides/infrastructure/mint-administration.md (~line 79)

The behaviour-testing heading was renamed away from RepoEnsurer, so that anchor no longer exists.

Update both links (and any nearby prose) to the current heading slug for the AllocateRepo / unified-driver lazy create+install section in docs/guides/dev/behaviour-testing.md. Run/fix until lint-md-links is clean.

fullsend-ai-coder · 2026-08-16T13:57:38Z

🤖 Finished Fix · ✅ Success · Started 1:57 PM UTC · Completed 2:04 PM UTC

Commit: 8ecf8d8 · View workflow run →

Update two markdown links from #lazy-createinstall-repoensurer (removed heading) to #repo-allocation-via-unified-driver (current heading) and align nearby prose with the unified install.Driver terminology introduced in this PR. Addresses review feedback on #6260

fullsend-ai-coder · 2026-08-16T14:04:06Z

🔧 Fix agent — iteration 1 (human-triggered)

Fixed 2 stale markdown fragment links in e2e-testing.md and mint-administration.md that pointed to the removed #lazy-createinstall-repoensurer heading. Updated both to #repo-allocation-via-unified-driver and aligned nearby prose with the unified Driver terminology. lint-md-links now passes cleanly.

Fixed (2):

stale fragment #lazy-createinstall-repoensurer in e2e-testing.md (docs/guides/dev/e2e-testing.md): Updated link anchor from #lazy-createinstall-repoensurer to #repo-allocation-via-unified-driver and aligned prose to reference the unified install.Driver instead of RepoPool/RepoEnsurer
stale fragment #lazy-createinstall-repoensurer in mint-administration.md (docs/guides/infrastructure/mint-administration.md): Updated link anchor from #lazy-createinstall-repoensurer to #repo-allocation-via-unified-driver and aligned prose to reference the unified install.Driver instead of RepoEnsurer

Tests: passed

Next steps:

/fs-review — request a re-review of the changes
/fs-fix <your instruction> — run another fix pass with specific guidance
Push commits directly — review re-runs automatically on push
_{Updated by fullsend fix agent}

fullsend-ai-review · 2026-08-16T14:05:42Z

🤖 Finished Review · ✅ Success · Started 2:05 PM UTC · Completed 2:23 PM UTC

Commit: 93c1ef9 · View workflow run →

codecov · 2026-08-16T14:09:55Z

Codecov Report

❌ Patch coverage is 98.86364% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/behaviourtest/drivers/install/cfmint/driver.go	96.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

fullsend-ai-review · 2026-08-16T14:23:21Z

Review

Findings

Medium

[interface-rename-collision] pkg/behaviourtest/drivers/install/driver.go — The existing install.Driver interface is renamed to install.MintDriver while a new install.Driver with a different contract (AllocateRepo/DeallocateRepo/Finalize/Capacity) is introduced. This is intentional per test(behaviour): suite uses one install Driver for repo allocate/deallocate/finalize #6169/test(behaviour): consolidate install driver, RepoEnsurer, and RepoPool behind one driver interface #6135, and migration guidance is provided in behaviour-testing.md, but the PR title should carry the ! suffix per COMMITS.md since this breaks exported API surface consumed by external repos (e.g., fullsend-ai/agents).
Remediation: update PR title to test(#6169)!: suite uses unified Driver for repo allocate/deallocate/finalize
[function-signature-change] pkg/behaviourtest/suite/init.go — suite.InitScenario signature changed from (sc, template, pool) to (sc, template), removing the *world.RepoPool parameter. External behaviour test suites calling this function with three arguments will fail to compile. Migration guidance is provided in behaviour-testing.md.
[struct-field-removed] pkg/behaviourtest/world/world.go — world.World.Ensurer (type install.RepoEnsurer) is replaced with world.World.Driver (type install.Driver). External code referencing w.Ensurer will fail to compile. Migration documented in behaviour-testing.md.

Low

[naming-consistency] pkg/behaviourtest/drivers/install/cfmint/driver_test.go — The test helper type testMintDriver uses a different naming convention from fakeMintDriver in composed_test.go. The author notes these are in different packages (cfmint vs install), but the inconsistency may cause confusion when reading cross-package test code.
[resource-leak] pkg/behaviourtest/drivers/install/cfmint/driver.go — In buildFromMint, if mintDriver.Install succeeds but NewComposedDriver fails (e.g., poolSize <= 0), the deployed mint worker is leaked because no driver is returned to call Finalize on. Practical risk is low since the only failure path is a programming error (invalid capacity).
[dead-code] pkg/behaviourtest/world/pool.go — world.RepoPool is now dead code after this PR. No callers of NewRepoPool, Acquire, or Release remain. Transitional until test(behaviour): fold RepoPool and RepoEnsurer into environment install drivers #6170.

Previous run

Review

Findings

Medium

[interface-rename-collision] pkg/behaviourtest/drivers/install/driver.go — The existing install.Driver interface is renamed to install.MintDriver while a new install.Driver with a different contract (AllocateRepo/DeallocateRepo/Finalize/Capacity) is introduced. This is intentional per test(behaviour): suite uses one install Driver for repo allocate/deallocate/finalize #6169/test(behaviour): consolidate install driver, RepoEnsurer, and RepoPool behind one driver interface #6135, and migration guidance is provided in behaviour-testing.md, but the PR title should carry the ! suffix per COMMITS.md since this breaks exported API surface consumed by external repos (e.g., fullsend-ai/agents).
Remediation: update PR title to test(#6169)!: suite uses unified Driver for repo allocate/deallocate/finalize
[function-signature-change] pkg/behaviourtest/suite/init.go — suite.InitScenario signature changed from (sc, template, pool) to (sc, template), removing the *world.RepoPool parameter. External behaviour test suites calling this function with three arguments will fail to compile. Migration guidance is provided in behaviour-testing.md.
[struct-field-removed] pkg/behaviourtest/world/world.go — world.World.Ensurer (type install.RepoEnsurer) is replaced with world.World.Driver (type install.Driver). External code referencing w.Ensurer will fail to compile. Migration documented in behaviour-testing.md.

Low

[dead-code] pkg/behaviourtest/world/pool.go — world.RepoPool is now dead code after this PR. No callers of NewRepoPool, Acquire, or Release remain. Transitional until test(behaviour): fold RepoPool and RepoEnsurer into environment install drivers #6170.
[edge-case] pkg/behaviourtest/drivers/install/composed.go:108 — In Finalize, outstanding lease names are reclaimed via channel sends while holding d.mu. Safe in practice (Finalize runs after godog finishes), but if called concurrently with AllocateRepo's error-recovery path, a blocking send under the lock is theoretically possible.
[new-required-interface] pkg/behaviourtest/drivers/install/driver.go — The new install.Factory type is the recommended way to construct unified Driver instances. External repos adopting the unified driver flow will need to migrate to the factory pattern. Migration guidance in behaviour-testing.md.

Labels: PR modifies e2e behaviour test infrastructure (pkg/behaviourtest/, e2e/behaviour/) and introduces new install.Driver unified interface

ifireball · 2026-08-16T14:39:51Z

/fs-fix get code coverage to 80% or above

fullsend-ai-coder · 2026-08-16T14:41:06Z

🤖 Finished Fix · ✅ Success · Started 2:41 PM UTC · Completed 2:54 PM UTC

Commit: 93c1ef9 · View workflow run →

Extract the deploy-and-compose path from NewFactory into a package- internal buildFromMint helper so it can be tested with a fake MintDriver (NewFactory hard-codes NewDriver, which needs real PEM files and an external binary). New tests cover: - NewFactory returns a non-nil factory - NewFactory error path (invalid config → NewDriver fails) - buildFromMint happy path with MintURLProvider state - buildFromMint install failure - buildFromMint with state that lacks MintURLProvider - buildFromMint with invalid pool size Coverage for cfmint/driver.go NewFactory: 0% → 80%. Package cfmint overall: 76.3% → 93.5%. Addresses review feedback on #6260

fullsend-ai-coder · 2026-08-16T14:54:25Z

🔧 Fix agent — iteration 2 (human-triggered)

Addressed coverage gap in cfmint/driver.go by extracting buildFromMint helper and adding 7 tests. NewFactory coverage 0% → 80%, package cfmint 76.3% → 93.5%. All changed files are well above the 80% patch coverage threshold.

Fixed (1):

get code coverage to 80% or above (pkg/behaviourtest/drivers/install/cfmint/driver.go): Extracted post-NewDriver logic from NewFactory into a package-internal buildFromMint helper to make the deploy-and-compose path testable with a fake MintDriver. Added 7 tests covering happy path, error paths, and edge cases. NewFactory coverage: 0% → 80%. Package cfmint overall: 76.3% → 93.5%.

Tests: passed

Decision points

Chose to extract buildFromMint helper rather than testing NewFactory end-to-end (alternatives: Test NewFactory with real PEM files and mock binary, Inject MintDriver via functional option on NewFactory; rationale: NewFactory hard-codes NewDriver (which needs PEMs) and the created driver's cliRunner (which calls e2etest.TryRunCLI). Extracting the composition logic into buildFromMint keeps the public API unchanged while making the deploy+compose path testable with a fake MintDriver.)

Next steps:

/fs-review — request a re-review of the changes
/fs-fix <your instruction> — run another fix pass with specific guidance
Push commits directly — review re-runs automatically on push
_{Updated by fullsend fix agent}

fullsend-ai-review · 2026-08-16T14:55:43Z

🤖 Finished Review · ✅ Success · Started 2:55 PM UTC · Completed 3:12 PM UTC

Commit: d5382c7 · View workflow run →

fullsend-ai-retro · 2026-08-16T15:41:45Z

🤖 Finished Retro · ✅ Success · Started 3:41 PM UTC · Completed 3:53 PM UTC

Commit: d5382c7 · View workflow run →

fullsend-ai-retro · 2026-08-16T15:53:25Z

Retro: PR #6260 — unified Driver for behaviour test suite

Outcome: Merged after 2 fix iterations (6 agent runs on this PR). However, this was the second attempt — the predecessor PR #6172 was abandoned after 15 agent runs over 3 days due to a gitleaks scan-range bug after rebase. Total cost across both PRs: 21 agent runs for an issue that should have required ~6.

What went well

The human decision to abandon PR test(#6169): suite uses unified Driver for repo allocate/deallocate/finalize #6172 and restart fresh from current main was correct — PR test(#6169): suite uses unified Driver for repo allocate/deallocate/finalize #6260 succeeded cleanly with far fewer iterations.
The 3 low-severity review findings (naming inconsistency, theoretical resource leak, dead code) were all factually accurate and appropriately categorized.
The fix agent handled both stale-link repair and coverage-gap backfill efficiently (one commit each).
The code agent's second attempt produced a clean, well-structured 886-line refactor across 16 files.

What could go better

Review quality — 3/3 medium findings were false positives. The review agent flagged interface-rename-collision, function-signature-change, and struct-field-removed at medium severity, claiming these break "exported API surface consumed by external repos." GitHub code search confirms zero external importers of pkg/behaviourtest/ — these packages are internal test infrastructure. The human reviewer (ifireball) was correct to approve without addressing them. See Proposal 1 below.

Evidence for existing open issues

fullsend#1719 (code agent should run CI before pushing): The initial commit broke lint-md-links by renaming a heading without updating fragment links in two other docs. CI caught it; a human-triggered /fs-fix was needed.
agents#230 (fix agent should proactively meet coverage requirements): The codecov/patch check failed after the first fix, requiring a second human-triggered /fs-fix to add tests for cfmint.NewFactory.
agents#318 / fullsend#6259 (gitleaks scan range after rebase): PR test(#6169): suite uses unified Driver for repo allocate/deallocate/finalize #6172 was terminally blocked when the fix agent's git merge origin/main expanded the gitleaks scan range to include ~70 upstream commits containing a false-positive test-fixture RSA PEM. This is the second documented instance of this bug causing complete PR abandonment.
agents#565 (fix agent won't proactively rebase): Over 3 days and 5+ iterations on PR test(#6169): suite uses unified Driver for repo allocate/deallocate/finalize #6172, the branch was never rebased. By the time it was attempted, the branch was 108 commits behind main, and the agent invented unnecessary abstractions to work around already-merged changes from sibling PR test(#6168): stop BT steps from using install.State config indirection #6171.
agents#235 (detect significant base-branch divergence early): The review agent never flagged that PR test(#6169): suite uses unified Driver for repo allocate/deallocate/finalize #6172 had diverged 108 commits from main, allowing wasteful fix iterations on a stale base.
agents#106 / fullsend#4682 (review agent re-raises dismissed findings): On PR test(#6169): suite uses unified Driver for repo allocate/deallocate/finalize #6172, the GODOG_CONCURRENCY t.Logf vs t.Errorf finding was raised 4 consecutive times despite explicit human dismissals with rationale.

Proposals filed

Cross-repo-contracts sub-agent should verify external consumers before flagging breaking API changes (in fullsend-ai/agents)

fullsend-ai-coder Bot requested a review from a team as a code owner August 16, 2026 13:50

fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Aug 16, 2026

fullsend-ai-coder Bot assigned ifireball Aug 16, 2026

github-actions Bot deployed to site-preview August 16, 2026 13:51 View deployment

github-actions Bot deployed to site-preview August 16, 2026 14:06 View deployment

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/e2e End-to-end tests labels Aug 16, 2026

github-actions Bot deployed to site-preview August 16, 2026 14:56 View deployment

ifireball approved these changes Aug 16, 2026

View reviewed changes

ifireball added this pull request to the merge queue Aug 16, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 16, 2026

ifireball added this pull request to the merge queue Aug 16, 2026

Merged via the queue into main with commit dc87d7d Aug 16, 2026
16 checks passed

ifireball deleted the agent/6169-unified-driver-suite branch August 16, 2026 15:39

fullsend-ai-retro Bot mentioned this pull request Aug 16, 2026

Cross-repo-contracts sub-agent should verify external consumers before flagging breaking API changes fullsend-ai/agents#824

Open

Uh oh!

Conversation

fullsend-ai-coder Bot commented Aug 16, 2026

Summary

Related Issue

Changes

Testing

Checklist

Post-script verification

Uh oh!

fullsend-ai-review Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

ifireball commented Aug 16, 2026

Uh oh!

fullsend-ai-coder Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-coder Bot commented Aug 16, 2026

🔧 Fix agent — iteration 1 (human-triggered)

Tests: passed

Uh oh!

fullsend-ai-review Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fullsend-ai-review Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Uh oh!

This comment was marked as outdated.

Uh oh!

ifireball commented Aug 16, 2026

Uh oh!

fullsend-ai-coder Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-coder Bot commented Aug 16, 2026

🔧 Fix agent — iteration 2 (human-triggered)

Uh oh!

fullsend-ai-review Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fullsend-ai-retro Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-retro Bot commented Aug 16, 2026

Retro: PR #6260 — unified Driver for behaviour test suite

What went well

What could go better

Evidence for existing open issues

Proposals filed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fullsend-ai-review Bot commented Aug 16, 2026 •

edited

Loading

github-actions Bot commented Aug 16, 2026 •

edited

Loading

fullsend-ai-coder Bot commented Aug 16, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 16, 2026 •

edited

Loading

codecov Bot commented Aug 16, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 16, 2026 •

edited

Loading

fullsend-ai-coder Bot commented Aug 16, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 16, 2026 •

edited

Loading

fullsend-ai-retro Bot commented Aug 16, 2026 •

edited

Loading