What happened
PR #6199 was ejected from the merge queue on its first attempt (run 31744447910). The Fork PR kill switch blocks all harnesses behaviour test itself passed, but its after-scenario cleanup hook failed: the hook called fullsend inference status to clean up a leased test repo, and that command failed because the GCP IAM Credentials generateAccessToken endpoint returned HTTP 503 ("upstream connect error or disconnect/reset before headers. reset reason: overflow"). This is a transient GCP-side error — the same test passed cleanly ~48 minutes later on the second merge queue attempt. The human reviewer (ralphbean) had to manually re-queue the PR. This is the second documented instance of after-scenario hook flakiness causing merge queue ejections — #5774 documents a different failure mode (GitHub 422 Tree SHA error) in the same hooks.
What could go better
After-scenario hooks perform cleanup operations (releasing test repos, checking inference status) that are not part of the test assertion itself. When these cleanup calls hit transient infrastructure errors, the test is marked as failed even though the actual test assertion passed. This creates a fragile point in the merge queue pipeline: any transient GCP or GitHub API error during cleanup can eject a PR that would otherwise merge cleanly.
I am fairly confident this is a recurring pattern rather than a one-off, based on: (1) the existence of #5774 documenting a different after-scenario hook error causing the same outcome, (2) the general frequency of transient GCP 503 errors in shared CI environments, and (3) the fact that merge queue attempts run under tighter time pressure where retries are less likely to happen naturally.
I am less certain about the best fix approach — retry with backoff vs. making cleanup failures non-fatal — and the test infrastructure owners should decide based on whether cleanup failure can leave orphaned resources that affect later test runs.
Proposed change
In the behaviour test suite's after-scenario hooks (likely in the test setup/teardown code under the behaviour test directory), add resilience for transient API errors in cleanup operations. Two approaches to evaluate:
-
Retry with backoff: Wrap after-scenario API calls (especially fullsend inference status and any GCP IAM token acquisition) in a retry loop with exponential backoff for transient HTTP errors (429, 502, 503). This is consistent with the approach proposed in #3902 for harness resource fetching.
-
Non-fatal cleanup: Make after-scenario hook failures emit a warning log rather than failing the test, since the test assertion itself already passed. This is the simpler approach but risks orphaned resources if cleanup is silently skipped.
A hybrid approach (retry 2-3 times, then warn-and-continue) would provide the best balance. This should also address the 422 Tree SHA failure in #5774 if the retry/tolerance is applied broadly to after-scenario hooks rather than to a single error code.
Validation criteria
After the change, scan the next 20 merge queue ejections on fullsend-ai/fullsend. None should be caused by transient errors (HTTP 429, 502, 503) in behaviour test after-scenario hooks. Additionally, #5774-style 422 Tree SHA failures in after-scenario hooks should no longer cause test failures if the broader tolerance approach is taken.
Generated by retro agent from #6199
What happened
PR #6199 was ejected from the merge queue on its first attempt (run 31744447910). The
Fork PR kill switch blocks all harnessesbehaviour test itself passed, but its after-scenario cleanup hook failed: the hook calledfullsend inference statusto clean up a leased test repo, and that command failed because the GCP IAM CredentialsgenerateAccessTokenendpoint returned HTTP 503 ("upstream connect error or disconnect/reset before headers. reset reason: overflow"). This is a transient GCP-side error — the same test passed cleanly ~48 minutes later on the second merge queue attempt. The human reviewer (ralphbean) had to manually re-queue the PR. This is the second documented instance of after-scenario hook flakiness causing merge queue ejections — #5774 documents a different failure mode (GitHub 422 Tree SHA error) in the same hooks.What could go better
After-scenario hooks perform cleanup operations (releasing test repos, checking inference status) that are not part of the test assertion itself. When these cleanup calls hit transient infrastructure errors, the test is marked as failed even though the actual test assertion passed. This creates a fragile point in the merge queue pipeline: any transient GCP or GitHub API error during cleanup can eject a PR that would otherwise merge cleanly.
I am fairly confident this is a recurring pattern rather than a one-off, based on: (1) the existence of #5774 documenting a different after-scenario hook error causing the same outcome, (2) the general frequency of transient GCP 503 errors in shared CI environments, and (3) the fact that merge queue attempts run under tighter time pressure where retries are less likely to happen naturally.
I am less certain about the best fix approach — retry with backoff vs. making cleanup failures non-fatal — and the test infrastructure owners should decide based on whether cleanup failure can leave orphaned resources that affect later test runs.
Proposed change
In the behaviour test suite's after-scenario hooks (likely in the test setup/teardown code under the behaviour test directory), add resilience for transient API errors in cleanup operations. Two approaches to evaluate:
Retry with backoff: Wrap after-scenario API calls (especially
fullsend inference statusand any GCP IAM token acquisition) in a retry loop with exponential backoff for transient HTTP errors (429, 502, 503). This is consistent with the approach proposed in #3902 for harness resource fetching.Non-fatal cleanup: Make after-scenario hook failures emit a warning log rather than failing the test, since the test assertion itself already passed. This is the simpler approach but risks orphaned resources if cleanup is silently skipped.
A hybrid approach (retry 2-3 times, then warn-and-continue) would provide the best balance. This should also address the 422 Tree SHA failure in #5774 if the retry/tolerance is applied broadly to after-scenario hooks rather than to a single error code.
Validation criteria
After the change, scan the next 20 merge queue ejections on fullsend-ai/fullsend. None should be caused by transient errors (HTTP 429, 502, 503) in behaviour test after-scenario hooks. Additionally, #5774-style 422 Tree SHA failures in after-scenario hooks should no longer cause test failures if the broader tolerance approach is taken.
Generated by retro agent from #6199