From 39da38b18991b14e2ffeae82db5c19a96e53fd66 Mon Sep 17 00:00:00 2001 From: Jake Howell Date: Mon, 6 Jul 2026 13:56:50 +1000 Subject: [PATCH] fix(site/e2e): accept 404 from external auth reset hook (#26793) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > 🤖 This PR was written by Coder Agents on behalf of Jake Howell. Stack: 1. #26575 `fix(site/e2e): close mock external-auth servers in teardown` 2. #26793 `fix(site/e2e): accept 404 from external auth reset hook` ← this PR 3. #26795 `fix(site/src): refresh provider state after device-flow exchange` 4. #26798 `fix(site/e2e): reset both providers in external auth hook` 5. #26648 `chore(site/e2e): re-enable externalAuth suite` `deleteExternalAuthByID` used to be inverted: `sql.ErrNoRows` (link doesn't exist for this user/provider) fell through to the `500` path, while non-`ErrNoRows` DB errors went to `httpapi.ResourceNotFound`. #19775 (Sep 2025) refactored it to return `404` for not-found and `500` for real DB errors, which is the contract you'd expect. The relevant lines from #19775 in `coderd/externalauth.go`: ```diff - err := api.Database.DeleteExternalAuthLink(ctx, ...) + link, err := api.Database.GetExternalAuthLink(ctx, ...) if err != nil { - if !errors.Is(err, sql.ErrNoRows) { + if errors.Is(err, sql.ErrNoRows) { httpapi.ResourceNotFound(w) return } httpapi.Write(ctx, w, http.StatusInternalServerError, ...) return } ``` `resetExternalAuthKey` in `site/e2e/hooks.ts` still treats `500` as the not-found code, so the first `beforeEach` in the externalAuth suite throws. The suite was skipped at the time #19775 landed (#17235), so nobody noticed the contract drift until #26648 tried to re-enable it. This just flips the accepted status codes to `200 || 404` and rewrites the stale comment. The 401/403/500 paths still surface as failures, which is what we want. Refs https://linear.app/codercom/issue/DEVEX-413 Refs https://github.com/coder/coder/pull/19775
Why a separate PR Keeps the bisection signal clean: #26575 proves the EADDRINUSE flake is fixed, this PR fixes the hook contract drift surfaced by re-enabling the suite, and #26648 just flips `.skip`. Squashing into #26648 would conflate two unrelated fixes. The CI run on #26648 already confirms the flake fix is doing its job: `successful external auth from workspace` passes (5.6s) and the `beforeAll`/`afterAll` mock servers come up and tear down cleanly with no EADDRINUSE. The only failures are this 404 hook drift.
--- site/e2e/hooks.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/e2e/hooks.ts b/site/e2e/hooks.ts index 8065bc40d3..e8ad7658bd 100644 --- a/site/e2e/hooks.ts +++ b/site/e2e/hooks.ts @@ -71,8 +71,8 @@ export const resetExternalAuthKey = async (context: BrowserContext) => { }); res.on("end", () => { - // Both 200 (key deleted successfully) and 500 (key was not found) are valid responses. - if (res.statusCode !== 200 && res.statusCode !== 500) { + // 200 = link deleted; 404 = no link existed for this provider. + if (res.statusCode !== 200 && res.statusCode !== 404) { console.error("failed to delete external auth link", data); throw new Error( `failed to delete external auth link: HTTP response ${res.statusCode}`,