E2E: cleanup-e2e-requests never removes journey-submitted requests, so the merge journey test passes only once per environment

[dfe-lance]

Summary

Post16MergeJourneyTests.HappyPath_SubmitsAndShowsAReference passes the first time the E2E suite runs against an environment and fails every time after that, deterministically. [RetryFact(3)] cannot rescue it, because the state that breaks it is already in the database before the first attempt.

It is currently red on more than one open PR and is not caused by any of them.

Root cause

The test resets state through SeedHelpers.CleanupDevRequestsAsync, which posts to POST /dev/queues/cleanup-e2e-requests. That endpoint deletes only:

// DevPipelineController.cs:78-81
var devIds = await dbContext.ChangeRequests
    .Where(r => EF.Functions.Like(r.ReferenceNumber, "DEV-%"))

DEV- references are minted in exactly one place — DevPipelineRunner.cs:64, the dev pipeline harness. A request submitted by a journey never has one. Journeys mint CYPMD_{type}_{uniqueId} (JourneyValidationService.cs:240); the merge test itself asserts CYPMD_16to19_[0-9A-F]{7}.

So the cleanup has never been able to remove a request that an E2E journey submitted. It is a no-op for that purpose and returns {"deleted":0}.

That is survivable for most journeys. It is fatal for the merge journey because of the duplicate guard: a student who already has a submitted request is refused with "A request has already been submitted for this student — Choose another student" (DuplicateRequestMessages.cs:55). The test always uses the same seeded student, Alice Smith / CYPMD ID 500001. So:

1. First run on a fresh environment — passes, and submits a merge request for 500001.
2. That request survives every subsequent cleanup.
3. Every later run on that environment is refused at the first Continue, the journey never reaches pupil-search/select-match-pupil, and Playwright reports a bare 30s navigation timeout that says nothing about why.

Evidence

Reproduced against the live review app for PR #447, twice, the second time immediately after calling the cleanup endpoint:

POST /dev/queues/cleanup-e2e-requests  ->  {"deleted":0}

form state after clicking the autocomplete option:
  selectedPupilId    = 00df42a2-3fa6-4e76-976c-d5552537b214
  selectedPupilLabel = Alice, Smith, (CYPMD ID:500001, ULN:9900000001, DOB:01/01/2007, INCLUDED)

after Continue -> still on .../pupil-search/select-pupil
  h1:            What is the name of the student for the first duplicate record to be merged?
  error summary: There is a problem
                 A request has already been submitted for this student
                 Choose another student

The autocomplete, the option click and the hidden field are all correct. The step is rejected by a business rule, not by a UI race.

Same SHA, same review app, three consecutive E2E jobs:

Job	Result
15 Sep 19:42	pass — 206/0 (and leaves the request behind)
15 Sep 20:02	fail — 205/1
16 Sep 08:54	fail — 205/1, identical stack

It also failed identically on the review app for PR #414's branch on 15 Sep, and — before it was merged — twice on its own PR, #431, on 10 Sep, going green once on 11 Sep before merging on 14 Sep.

Why nothing caught it

The E2E job is gated to per-PR review apps (build-and-deploy.yml:93) — main never runs it. A review app's first E2E run always passes, so the test looks fine on any PR whose job runs once and is never re-run. Only a re-run, or a second workflow run against the same review app, exposes it.

This is wider than one test

Six E2E classes call CleanupDevRequestsAsync expecting it to reset request state — Post16MergeJourneyTests, Ks4JourneyTests, IncludeHandoffConflictTests, JourneyAutocompleteRestoreTests and others. For all of them it is currently doing nothing. The merge journey is simply the first place where the leak has a guard to collide with.

Proposed fix

1. Make the cleanup restore the seeded baseline rather than filter by prefix.

The endpoint's purpose is "put change requests back to a known state for a test run", so it should do that rather than match a prefix that only one code path produces.

Two traps to avoid:

• The seeded Kingsmead requests are CYPMD_KS4June_SEED00n (SeedChangeRequests.cs:64-69), so widening the filter to everything beginning CYPMD would delete fixtures other tests depend on.
• _ is a single-character wildcard in SQL LIKE, so LIKE 'CYPMD_%' does not mean what it looks like. Any prefix match on these references needs an explicit escape.

Deleting all change requests and re-running SeedChangeRequests avoids both, and gives every caller the reset they already believe they are getting.

2. Stop the merge test consuming a shared fixture permanently.

Even with the cleanup fixed, the test is one unhandled failure away from leaving its own request behind. Either seed a student for the run, or have the test remove the request it created.

3. Fail with a diagnosis, not a timeout.

ChooseMatchStudentAsync should assert it is where it expects to be and surface the error summary. Thirty seconds of silence followed by "Timeout 30000ms exceeded" cost a full investigation to turn into one sentence that was on the screen the whole time.

Stopping it recurring

• A test over the cleanup endpoint that submits a request in the journey reference format, calls the cleanup, and asserts the row is gone — the direct guard on the predicate, and the thing whose absence let this ship.
• A test that the seeded CYPMD_KS4June_SEED00n requests survive the cleanup, so a future widening of the filter cannot quietly delete them.
• Run the E2E suite twice in a row against the same review app in CI, or at minimum re-run it before merge, so "passes once per environment" cannot pass for green again.

Acceptance criteria

• POST /dev/queues/cleanup-e2e-requests removes requests submitted through a journey, not only DEV- ones
• The seeded CYPMD_KS4June_SEED00n requests are still present after a cleanup
• Post16MergeJourneyTests.HappyPath_SubmitsAndShowsAReference passes twice in a row against the same environment
• The merge journey's step assertions report the on-screen error rather than a navigation timeout
• Automated coverage for the first two points, so the mismatch cannot return unnoticed