fix(#6518): restore config.yaml agents between pool slot leases - #6521
Conversation
CleanupScenario already restored kill_switch, runtime, and allowed_remote_resources between behaviour test scenarios, but did not restore the agents list. This caused custom agent entries (local or URL-sourced) registered by one scenario to leak into later scenarios that reused the same pool slot. Add snapshot/restore logic for the agents section of config.yaml, following the existing pattern used for allowed_remote_resources: - Add AgentsOverridden/AgentsOriginal fields to world.World - Add snapshotAgents() called before every cfg.SetAgents() in dispatch.go, url_dispatch.go, and base_dispatch.go - Add RestoreAgents() called by CleanupScenario during teardown - Reset both fields in resetScenarioWorld between scenarios Closes #6518
|
🤖 Finished Review · ✅ Success · Started 1:12 PM UTC · Completed 1:23 PM UTC Commit: |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Looks good to me |
|
🤖 Finished Retro · ✅ Success · Started 1:32 PM UTC · Completed 1:52 PM UTC Commit: |
Retro: PR #6521 — restore config.yaml agents between pool slot leasesWorkflow outcome: Merged cleanly in ~1h45m from issue creation. Well-scoped fix adding snapshot/restore logic for the Timeline
Main finding: first code run wasted $4.84 on completed-but-uncommitted workThe first code agent run completed ALL implementation work correctly — edited 9 files, wrote tests, all tests passed, staged files — but the model emitted Contributing factors:
The second run succeeded because it used Existing issues providing coverage (no new proposals needed)
Review qualityThe review agent approved a 277-line, 9-file change with "Looks good to me" and no inline comments. The human reviewer (who authored the issue) also approved without comments. Given the PR followed an established snapshot/restore pattern, was well-tested, and all CI passed (80.49% patch coverage), this level of review depth appears appropriate for the change class. |
Summary
agentssection of.fullsend/config.yamlinCleanupScenario, following the existing pattern used forkill_switch,runtime, andallowed_remote_resources.snapshotAgents()before everycfg.SetAgents()call acrossdispatch.go,url_dispatch.go, andbase_dispatch.goso the install-time agent list is captured before any scenario modifies it.RestoreAgents()is called duringCleanupScenarioteardown to reset agents to their pre-scenario values, preventing harness registrations and custom agent entries from leaking across pool slot leases within a run.Context
Pool
test-repo-NNslots are reused across behaviour scenarios.CleanupScenariorestoredkill_switch,runtime, andallowed_remote_resourcesbut did not restore theagentslist. This caused URL host prefixes and custom agent entries from one scenario to persist into later scenarios on the same slot, leading to flaky test behavior (e.g., allowlist-negative scenarios passing incorrectly on reused slots).Testing
snapshotAgents,RestoreAgents, andCleanupScenarioagent restore added incleanup_test.goandurl_dispatch_test.goresetScenarioWorldtest updated to verify new fields are clearedpkg/behaviourtest/...passgofmt,go vetpass;pre-commitcould not run due to sandbox network restrictions (post-script runs authoritative check)Closes #6518
Post-script verification
agent/6518-restore-config-agents)997021c226272037a8cb69b32c73e14f26073b5e..HEAD)