> 🤖 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`
← this PR
2. #26793 `fix(site/e2e): accept 404 from external auth reset hook`
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`

The externalAuth e2e suite has been skipped since #17235 because
`createServer` in `site/e2e/helpers.ts` started an express server but
never gave callers a way to close it. On retries or repeated runs, the
listener from the previous invocation was still bound to the hardcoded
port and the next `beforeAll` failed with `EADDRINUSE`, eventually
timing out in `waitForPort`.

`createServer` now returns a `{ app, close }` pair. The web flow closes
in `afterAll`; the device flow uses `try/finally`.
`closeAllConnections()` is called before `close()` so teardown stays
bounded if keep-alive connections linger.

The suite remains `test.describe.skip` here; #26648 flips the skip off
once the rest of the stack is in.

Refs https://linear.app/codercom/issue/DEVEX-413
Refs coder/internal#356

<details>
<summary>Decision log</summary>

Discussed the full options list with @jakehwll before drafting. Picked
option A (minimal teardown) because:

- Two prior PRs (#15537, #16528) attacked symptoms (port probing, longer
timeout) without addressing the leaked listener.
- Kayla's diagnosis on coder/internal#356 pointed at exactly this case:
nothing else in CI is grabbing the port, the listener from the previous
run is still bound.
- A is mechanical and orthogonal: it adds a real teardown without
changing port allocation, fixture wiring, or what gets mocked. If the
flake persists after A, we know to escalate to a worker-scoped fixture
or dynamically allocated ports.

Returning `close` rather than the raw `http.Server` encapsulates the
`closeAllConnections` + `close` choreography so callers don't repeat it.
`closeAllConnections` is optional-chained because it landed in Node
18.2; coder/coder runs newer, but the chain costs nothing.

The device test uses `try/finally` rather than a shared `afterEach` to
keep per-test state local. The web flow's `afterAll` mirrors its
`beforeAll`.

</details>

> 🤖 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 #19775

<details>
<summary>Why a separate PR</summary>

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.

</details>

)

> 🤖 This PR was modified 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`
3. #26795 `fix(site/src): refresh provider state after device-flow
exchange` ← this PR
4. #26798 `fix(site/e2e): reset both providers in external auth hook`
5. #26648 `chore(site/e2e): re-enable externalAuth suite`

#18039 (May 2025) upgraded `@tanstack/react-query` from v4 to v5, which
[removed `onSuccess`/`onError`/`onSettled` from
`useQuery`](https://tanstack.com/query/v5/docs/react/guides/migrating-to-v5#callbacks-on-usequery-and-queryobserver-have-been-removed).
The migration updated the `invalidateQueries` argument shape but left
the now-dead `onSuccess` in place on `exchangeExternalAuthDevice`, which
is consumed by `useQuery` in `ExternalAuthPage.tsx`.

The relevant lines from #18039 in
`site/src/api/queries/externalAuth.ts`:

```diff
 		queryKey: ["external-auth", providerId, "device", deviceCode],
 		onSuccess: async () => {
 			// Force a refresh of the Git auth status.
-			await queryClient.invalidateQueries(["external-auth", providerId]);
+			await queryClient.invalidateQueries({
+				queryKey: ["external-auth", providerId],
+			});
 		},
```

Result: after a successful device-flow exchange the
`externalAuthProvider` query is never invalidated, so
`externalAuthProviderQuery.data.authenticated` stays `false` and the UI
is stuck on "Checking for authentication..." until a manual page
refresh. This breaks real users who go through the device flow today,
not just the e2e suite that re-enables in #26648.

This PR drops the dead `onSuccess` (and the `queryClient` param it
depended on), and moves the invalidation into a `useEffect` in
`ExternalAuthPage.tsx` that fires when
`exchangeExternalAuthDeviceQuery.isSuccess` flips true. Matches the
existing pattern in `LoginOAuthDevicePage.tsx`.

Verified against the failing CI run on #26648 ([job
83970095566](https://github.com/coder/coder/actions/runs/28346242963/job/83970095566?pr=26648)):
the trace shows `POST /api/v2/external-auth/device/device` returning
`204` (exchange succeeded) followed by no subsequent `GET
/api/v2/external-auth/device` to refresh the provider state. With this
PR the `isSuccess` effect runs, the provider query refetches, and the UI
flips to the authorized state.

## Regression test

Added `site/src/pages/ExternalAuthPage/ExternalAuthPage.test.tsx` (the
first test for this page). It renders the device flow with a stateful
provider handler that returns `authenticated: false` until the exchange
`POST` lands, then asserts the UI flips from the "Authenticate with
GitHub" polling screen to "You've authenticated with GitHub!" without a
manual refresh, and that the provider endpoint is refetched after the
exchange. Confirmed red against the pre-fix code (stuck on the polling
screen) and green with the fix.

Refs https://linear.app/codercom/issue/DEVEX-413
Refs #18039

<details>
<summary>Why <code>useEffect</code> rather than a query-level
callback</summary>

react-query v5 removed `onSuccess`/`onError`/`onSettled` from `useQuery`
because the v4 behaviour was unsound: the callbacks fired per-observer
rather than per-query, so they ran twice when two components observed
the same query and not at all when a component unmounted and cached data
was reused. [TKDodo's "Breaking React Query's API on
Purpose"](https://tkdodo.eu/blog/breaking-react-querys-api-on-purpose)
and the [official v5 migration
guide](https://tanstack.com/query/v5/docs/react/guides/migrating-to-v5#callbacks-on-usequery-and-queryobserver-have-been-removed)
both recommend `useEffect` on `isSuccess` as the replacement.

Alternatives considered:

| Option | Why not |
| --- | --- |
| Side-effect in `queryFn` | Re-adds the `queryClient` dependency we
just removed, and fires on every retry and cached re-read, not just
first success. |
| Convert to `useMutation` | Wrong semantics. This is a polling query
with `retry: isExchangeErrorRetryable` and `retryDelay`; mutations are
one-shot and don't have retry-on-pending machinery. |
| Global `QueryCache` `onSuccess` | Runs for every query in the app;
filtering by queryKey or `meta` for a single per-page side effect is
more code than the effect it replaces. |
| Custom hook wrapping `useQuery` | Only one consumer, so the
abstraction would have one caller. |

Local precedent: `LoginOAuthDevicePage.tsx` already uses the same
`isSuccess` → `useEffect` pattern for the post-success `location.href`
redirect.

</details>

<details>
<summary>Why a separate PR</summary>

Keeps the bisection signal clean. #26575 fixes the EADDRINUSE flake,
#26793 fixes the 404 hook contract drift, this PR fixes the dropped
invalidation, and #26648 just flips `.skip`. Each PR addresses one
independent root cause that piled up while the externalAuth suite was
skipped.

</details>