§6 test-cleanup (todo §6); unified the genuine OAuth2/Hydra test overlaps. The stale-4xx→400 / outage-5xx→500 degrade was triplicated across the app.test.ts /oauth2/login, /consent, /logout tests with near-identical app-spin-up boilerplate — production aims for byte-identical degrade across the three, so it's now one parametrized test iterating the three endpoints × {410→400, 503→500} (removes ~27 lines, makes the shared contract explicit/enforced); the three endpoint tests keep their happy-path + missing-challenge→400. oauth-consent.test.ts: merged the two consent-screen view tests (account named when signed in / omitted otherwise — same view surface) and the two acceptConsent grant tests (scope re-read + id_token on subject-match / omitted on mismatch — same method's grant body). Pure test refactor, no production code touched, every assertion preserved. The per-module matrices (hydra-admin/oauth-login/admin-clients, one contract per test) carry no fat. 279 → 278 units; typecheck + tests green.

src/app.test.ts

@@ -531,20 +531,6 @@ test("OAuth2 login challenge (/oauth2/login): a Kratos session accepts via Hydra
   // Missing login_challenge → 400 (someone hit the endpoint directly).
   assert.equal((await fetch(base + "/oauth2/login", { redirect: "manual" })).status, 400);
 
-  // A stale/invalid/consumed challenge (Hydra 4xx — back button, slow login) degrades to a
-  // recoverable 400, not a 500. A genuine Hydra outage (5xx) still surfaces as a 500.
-  const staleHydra = stubHydra({ getLoginRequest: async () => { throw new HydraError("gone", 410, ""); } });
-  const stale = createApp({ hydra: staleHydra, kratos: withWhoami(async () => null) });
-  await new Promise<void>((r) => stale.listen(0, r));
-  t.after(() => stale.close());
-  const staleBase = `http://localhost:${(stale.address() as AddressInfo).port}`;
-  assert.equal((await fetch(staleBase + "/oauth2/login?login_challenge=gone", { redirect: "manual" })).status, 400);
-  const downHydra = stubHydra({ getLoginRequest: async () => { throw new HydraError("down", 503, ""); } });
-  const down = createApp({ hydra: downHydra, kratos: withWhoami(async () => null) });
-  await new Promise<void>((r) => down.listen(0, r));
-  t.after(() => down.close());
-  assert.equal((await fetch(`http://localhost:${(down.address() as AddressInfo).port}/oauth2/login?login_challenge=x`, { redirect: "manual" })).status, 500);
-
   // Not signed in: bounce to the themed login, return_to carrying an absolute URL back to here.
   const anon = createApp({ hydra: stubHydra(), kratos: withWhoami(async () => null) });
   await new Promise<void>((r) => anon.listen(0, r));
@@ -570,7 +556,7 @@ test("/login?return_to=… bakes the return target into the Kratos flow init (§
   assert.equal(seenReturnTo, returnTo);
 });
 
-test("OAuth2 consent challenge (/oauth2/consent): skip auto-accepts; a third-party shows the screen; allow/deny POST; CSRF-guarded; missing/stale challenge", async (t) => {
+test("OAuth2 consent challenge (/oauth2/consent): skip auto-accepts; a third-party shows the screen; allow/deny POST; CSRF-guarded; missing challenge", async (t) => {
   const csrfSecret = "consent-secret";
   let granted: { grant_scope?: string[]; session?: unknown } | undefined;
   const hydra = stubHydra({
@@ -623,19 +609,9 @@ test("OAuth2 consent challenge (/oauth2/consent): skip auto-accepts; a third-par
   const auto = await fetch(`http://localhost:${(skip.address() as AddressInfo).port}/oauth2/consent?consent_challenge=cons1`, { redirect: "manual" });
   assert.equal(auto.status, 303);
   assert.match(auto.headers.get("location") ?? "", /consent_verifier=v/);
-
-  // A stale challenge (Hydra 4xx) degrades to 400; a genuine outage (5xx) surfaces as 500.
-  const stale = createApp({ hydra: stubHydra({ getConsentRequest: async () => { throw new HydraError("gone", 410, ""); } }), kratos: withWhoami(async () => null) });
-  await new Promise<void>((r) => stale.listen(0, r));
-  t.after(() => stale.close());
-  assert.equal((await fetch(`http://localhost:${(stale.address() as AddressInfo).port}/oauth2/consent?consent_challenge=gone`, { redirect: "manual" })).status, 400);
-  const down = createApp({ hydra: stubHydra({ getConsentRequest: async () => { throw new HydraError("down", 503, ""); } }), kratos: withWhoami(async () => null) });
-  await new Promise<void>((r) => down.listen(0, r));
-  t.after(() => down.close());
-  assert.equal((await fetch(`http://localhost:${(down.address() as AddressInfo).port}/oauth2/consent?consent_challenge=x`, { redirect: "manual" })).status, 500);
 });
 
-test("OAuth2 RP-initiated logout (/oauth2/logout): accepts the logout challenge → 303 to Hydra; missing → 400; stale 4xx → 400, outage 5xx → 500", async (t) => {
+test("OAuth2 RP-initiated logout (/oauth2/logout): accepts the logout challenge → 303 to Hydra; missing → 400", async (t) => {
   let acceptedChallenge: string | undefined;
   const hydra = stubHydra({ acceptLogoutRequest: async (c) => { acceptedChallenge = c; return { redirect: "http://acme.example/post-logout" }; } });
   const app = createApp({ hydra, kratos: withWhoami(async () => null) });
@@ -649,16 +625,26 @@ test("OAuth2 RP-initiated logout (/oauth2/logout): accepts the logout challenge
   assert.equal(acceptedChallenge, "lc1");
 
   assert.equal((await fetch(base + "/oauth2/logout", { redirect: "manual" })).status, 400);
+});
 
-  const stale = createApp({ hydra: stubHydra({ acceptLogoutRequest: async () => { throw new HydraError("gone", 404, ""); } }), kratos: withWhoami(async () => null) });
+// All three OAuth2 challenge endpoints share one degrade contract (the documented "byte-identical"
-  await new Promise<void>((r) => stale.listen(0, r));
+// behaviour): a stale/consumed challenge (Hydra 4xx — back button, slow login) → recoverable 400,
-  t.after(() => stale.close());
+// a genuine Hydra outage (5xx) → 500.
-  assert.equal((await fetch(`http://localhost:${(stale.address() as AddressInfo).port}/oauth2/logout?logout_challenge=gone`, { redirect: "manual" })).status, 400);
+test("OAuth2 challenge endpoints degrade identically: stale Hydra 4xx → 400, outage 5xx → 500", async (t) => {
-
+  const endpoints: { make: (status: number) => Partial<HydraAdmin>; path: string }[] = [
-  const down = createApp({ hydra: stubHydra({ acceptLogoutRequest: async () => { throw new HydraError("down", 503, ""); } }), kratos: withWhoami(async () => null) });
+    { make: (s) => ({ getLoginRequest: async () => { throw new HydraError("x", s, ""); } }), path: "/oauth2/login?login_challenge=x" },
-  await new Promise<void>((r) => down.listen(0, r));
+    { make: (s) => ({ getConsentRequest: async () => { throw new HydraError("x", s, ""); } }), path: "/oauth2/consent?consent_challenge=x" },
-  t.after(() => down.close());
+    { make: (s) => ({ acceptLogoutRequest: async () => { throw new HydraError("x", s, ""); } }), path: "/oauth2/logout?logout_challenge=x" },
-  assert.equal((await fetch(`http://localhost:${(down.address() as AddressInfo).port}/oauth2/logout?logout_challenge=x`, { redirect: "manual" })).status, 500);
+  ];
+  for (const { make, path } of endpoints) {
+    for (const [status, expected] of [[410, 400], [503, 500]] as const) {
+      const app = createApp({ hydra: stubHydra(make(status)), kratos: withWhoami(async () => null) });
+      await new Promise<void>((r) => app.listen(0, r));
+      t.after(() => app.close());
+      const res = await fetch(`http://localhost:${(app.address() as AddressInfo).port}${path}`, { redirect: "manual" });
+      assert.equal(res.status, expected, `${path} ${status} → ${expected}`);
+    }
+  }
 });
 
 // Built-in Users admin screen (§5): gate + every CRUD action over HTTP against a mock Kratos admin.

src/oauth-consent.test.ts

@@ -60,38 +60,29 @@ test("a first-party client (metadata.first_party) auto-accepts even without skip
   assert.equal(granted?.session, undefined);
 });
 
-test("a third-party client shows the consent screen carrying the signed-in account (no auto-accept)", async () => {
+test("a third-party client shows the consent screen (no auto-accept); the account is named when signed in, omitted otherwise", async () => {
   let accepted = false;
   const hydra = stubHydra(consent(), () => { accepted = true; });
-  const kratos = stubKratos(async () => sessionWith({ email: "ada@x.io" }));
+  const signedIn = await resolveConsentChallenge({ hydra, kratos: stubKratos(async () => sessionWith({ email: "ada@x.io" })) }, CHALLENGE, "plainpages_session=s");
-  const out = await resolveConsentChallenge({ hydra, kratos }, CHALLENGE, "plainpages_session=s");
+  assert.equal(signedIn.redirect, undefined);
-  assert.equal(out.redirect, undefined);
+  assert.deepEqual(signedIn.view, { account: "ada@x.io", challenge: CHALLENGE, client: "Acme Reports", scopes: ["openid", "profile"] });
-  assert.deepEqual(out.view, { account: "ada@x.io", challenge: CHALLENGE, client: "Acme Reports", scopes: ["openid", "profile"] });
   assert.equal(accepted, false);
+  // No session ⇒ the screen still renders but names no account.
+  const anon = await resolveConsentChallenge({ hydra: stubHydra(consent()), kratos: stubKratos(async () => null) }, CHALLENGE, undefined);
+  assert.equal(anon.view?.account, undefined);
 });
 
-test("the consent screen omits the account when there's no session", async () => {
+test("acceptConsent re-reads the challenge's scopes (never client-supplied) and projects id_token only when the session subject matches", async () => {
-  const out = await resolveConsentChallenge({ hydra: stubHydra(consent()), kratos: stubKratos(async () => null) }, CHALLENGE, undefined);
+  let matched: AcceptConsent | undefined;
-  assert.equal(out.view?.account, undefined);
+  const redirect = await acceptConsent({ hydra: stubHydra(consent(), (b) => { matched = b; }), kratos: stubKratos(async () => sessionWith({ email: "ada@x.io" })) }, CHALLENGE, "plainpages_session=s");
-});
+  assert.equal(redirect, REDIRECT);
-
+  assert.deepEqual(matched?.grant_scope, ["openid", "profile"]); // re-read from the challenge, not the form
-test("id_token claims are only projected when the session subject matches the challenge (else omitted)", async () => {
+  assert.deepEqual(matched?.session, { id_token: { email: "ada@x.io" } });
-  let granted: AcceptConsent | undefined;
-  const hydra = stubHydra(consent(), (b) => { granted = b; });
   // A session whose identity differs from the challenge subject must not leak its claims into the grant.
+  let mismatched: AcceptConsent | undefined;
   const other: Session = { active: true, identity: { id: "01902d5e-0000-7e3a-9f21-3c8d1e0a4b55", traits: { email: "mallory@x.io" } } };
-  const redirect = await acceptConsent({ hydra, kratos: stubKratos(async () => other) }, CHALLENGE, "plainpages_session=s");
+  await acceptConsent({ hydra: stubHydra(consent(), (b) => { mismatched = b; }), kratos: stubKratos(async () => other) }, CHALLENGE, "plainpages_session=s");
-  assert.equal(redirect, REDIRECT);
+  assert.equal(mismatched?.session, undefined);
-  assert.equal(granted?.session, undefined);
-});
-
-test("acceptConsent re-fetches the challenge and grants its scopes (never client-supplied)", async () => {
-  let granted: AcceptConsent | undefined;
-  const hydra = stubHydra(consent(), (b) => { granted = b; });
-  const redirect = await acceptConsent({ hydra, kratos: stubKratos(async () => sessionWith({ email: "ada@x.io" })) }, CHALLENGE, "plainpages_session=s");
-  assert.equal(redirect, REDIRECT);
-  assert.deepEqual(granted?.grant_scope, ["openid", "profile"]);
-  assert.deepEqual(granted?.session, { id_token: { email: "ada@x.io" } });
 });
 
 test("rejectConsent rejects with access_denied → the client's error redirect", async () => {

todo.md

@@ -107,7 +107,7 @@ everything via Docker.
 - [x] OAuth2 client registration (admin UI or CLI). → Built-in **OAuth2 clients** admin screen (`src/admin-clients.ts`, `/admin/clients`) — the §6 client side of Hydra (apps that log in *through* us). `src/hydra-admin.ts` gains the client half of the admin API: `createClient`/`listClients`/`getClient`/`deleteClient` over `/admin/clients` (+ a `nextPageToken` Link parser, mirrors kratos-admin) and the registration fields on `OAuth2Client`. The screen mirrors the §5 Users/Roles pattern — pure builders (`toClientView`, `clientPayload`, `validateClientInput`, `parseRedirectUris`, `buildClients{List,Form,Detail}Model`) + `handleAdminClients` (the imperative shell app.ts dispatches `/admin/clients*` to). Routes: `GET /admin/clients` (list — search/paginate over one Hydra page), `GET|POST /admin/clients/new`+`/` (register), `GET /admin/clients/:id` (read-only detail), `GET|POST …/:id/delete` (confirm + delete). Register builds a standard authorization-code client (+ refresh_token), confidential (`client_secret_basic`) or public (PKCE, `none`), with an optional first-party (auto-consent) flag; **Hydra returns the `client_secret` once**, so the register POST renders the new client's detail page with the one-time secret directly (no PRG) — never re-shown (`getClient` carries no secret; detail asserts it). Writes go **only to Hydra**; gated admin-only (anon→/login, non-admin→403) + every mutation CSRF-guarded, like §5; a Hydra 4xx (bad redirect/scope) re-renders the form (400), a 5xx → 500 (mirrors `oauth-login.ts`); `:id` via `safeDecode` (malformed→404). Wired into the shared `adminSection` (Users·Groups·Roles·**OAuth2 clients**, `i-globe`) so it shows for admins, invisible otherwise. New views (`admin/clients`,`client-form`,`client-detail` + `partials/client-{form,detail}-body`) reuse the shell/filter-bar/data-table/field blocks; one `.detail-list` CSS rule. Tests-first: `hydra-admin.test.ts` (client CRUD contracts incl. Link pagination/404→null/204), `admin-clients.test.ts` (builder/validation/payload matrix), `app.test.ts` HTTP integration (gate/list/register-shows-secret-once/invalid+CSRF-reject/detail-hides-secret/delete + malformed-`%`→404). Stability-reviewer run as a local PR: APPROVE, no Critical/High; addressed its one nit (dropped a dead `URL.protocol` check in `validateClientInput`). Boot-verified the client CRUD live against real Hydra v26.2.0 (create→201 w/ one-time secret → list finds it → get → delete → get null); torn down. typecheck + 274 units green. Review/comment/test-cleanup are the next §6 items.
 - [x] Run the architecture and the product reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §6 Hydra OAuth2 surfaces). Architecture: **no Critical**; Product: **no Critical** this checkpoint. **Fixed now (tests-first):** (1) HIGH (arch) — `/oauth2/logout` was published to Hydra (`hydra.yml` `urls.logout`) and asserted in `hydra.test.ts`, but had **no handler** (a dead/published contract — Hydra's RP-initiated logout would 404). Added `hydra-admin.acceptLogoutRequest` (PUT logout/accept, folded into the shared `reqUrl(kind…)`) + a `GET /oauth2/logout` branch: accept the `logout_challenge` → 303 to Hydra's post-logout redirect; missing challenge → 400; a stale/consumed 4xx → recoverable 400, a 5xx outage → 500 (byte-identical degrade to the login/consent siblings). GET-accept is safe — the challenge is Hydra-minted + single-use; the first-party `POST /logout` still owns ending the Kratos session + our JWT cookie. (2) HIGH (arch) — added `oauth2` to `RESERVED_PLUGIN_IDS` (a `plugins/oauth2/` folder would silently shadow the provider routes — the one route surface the §4 reserved-id fix didn't cover; discovery now refuses it loud). (3) Product **Blocker** — the consent screen never told the user *whose* account they were authorizing (informed-consent gap on shared devices). It now renders "Signed in as `<email>`" (`ConsentView.account` from `whoami`) + a "Not you? Sign out" form (CSRF-guarded by the same signed double-submit). (4) MEDIUM (arch) — consent `accept()` now projects id_token claims **only when** the live Kratos session subject `===` the challenge subject Hydra bound at login (never leak a mismatched session's email/name into the issued token; guards the auto-accept path too). (5) Product nits — register-form confidential-vs-public guidance ("Browser/mobile apps can't keep a secret — choose Public…") and a client-detail "to change a client, delete and re-register — the secret is shown only once" note (covers the no-edit friction + lost-secret-on-reload). Stability-reviewer run as a local PR: **APPROVE, no Critical/High**; addressed its actionable follow-ups (README §6 now documents the logout handler + the consent identity line; a comment notes the GET-accept is Hydra-validated). Extended `e2e/oauth-login.spec.ts` to assert the consent screen names the signed-in account; **boot-verified the full OAuth2 login+consent flow live against real Hydra v26.2.0** (E2E green) then torn down. typecheck + 279 units green. **Deferred (reviewer-scoped, not the §6 checkpoint):** the host **internal route-table** (fold the admin/oauth if-ladder into one `{method,prefix,handler}` table, derive `RESERVED_PLUGIN_IDS`/`allowedMethods` from it — arch M1, the long-deferred §2/§5 item) → **§9**: H1/H2 are now point-fixed, so M1 is reduced to a pure dedup/structural improvement (Medium) best done as a focused standalone change to the central dispatcher, not bundled into a review-fix; the **RP-initiated-logout browser/live E2E** (needs token-exchange + `id_token_hint` + `post_logout_redirect_uris`) → **§8** (owns full E2E — the handler is unit+HTTP covered and reuses the `complete()` path already live-verified by the login/consent accepts); the redirect-URI **scheme allowlist** + the `safeUrl()` href helper (arch L1) → **§7** (first untrusted-URL flow); full **client edit** (registration-only by design — the detail page now says delete+re-register), the blank **empty-list** state (known §5 deferral), and success-flash after writes → §8/polish; unconditional `refresh_token` + raw custom-scope labels (product 🟢) → future.
 - [x] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. → Pass over the §6 Hydra/OAuth2 accretion. The §6 modules (`hydra-admin`/`oauth-login`/`oauth-consent`/`admin-clients`) were authored dense, so the wins are in `app.ts`, where the three sibling OAuth2 handlers had drifted toward repeating the same prose: trimmed the `/oauth2/login` block header (dropped "Challenge looked up over Hydra's admin API" — derivable from `resolveLoginChallenge` — and condensed the provider-role sentence to "Provider-only.", matching the consent/logout blocks); collapsed the `/oauth2/consent` degrade comment to the same one-liner the `/oauth2/logout` sibling already uses, so the canonical 4xx→400 / 5xx→500 explanation lives once (the `/oauth2/login` block). Left intact: the logout block's GET-accept safety rationale (reviewer-requested, todo line 108), the EJS view config-doc headers (the only schema for untyped locals), and the §6 README **OAuth2 provider (Hydra)** + admin-clients sections (authored concise in §6, todo line 108). The README **Status** note / `_(planned)_` markers / Layout refresh stays §9's (line 133). typecheck + 279 units green.
-- [ ] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us.
+- [x] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us. → Pass over the §6 Hydra/OAuth2 accretion (the per-module matrices in `hydra-admin`/`oauth-login`/`admin-clients` are one-contract-per-test — no fat). Removed the genuine §6 overlaps: (1) the stale-4xx→400 / outage-5xx→500 degrade was **triplicated** across the `app.test.ts` `/oauth2/login`, `/consent`, `/logout` tests with near-identical app-spin-up boilerplate — production aims for "byte-identical" degrade across the three, so it's now one parametrized test (`OAuth2 challenge endpoints degrade identically`) iterating the three endpoints × {410→400, 503→500}, which both removes ~27 lines and makes the shared contract explicit/enforced; the three endpoint tests keep their happy-path + missing-challenge→400. (2) `oauth-consent.test.ts`: merged the two consent-screen view tests (account named when signed in / omitted when not — same `view` surface, one variable) and the two `acceptConsent` grant tests (scope re-read + id_token on subject-match / omitted on mismatch — same method's grant body). Pure test refactor, no production code touched; every assertion preserved. 279 → 278 units; typecheck + tests green.
 
 ## 7. Example plugin (reference)
 - [ ] Reference plugin (e.g. people directory or scheduling): list page fetching upstream data, a form that forwards writes upstream, permission-gated nav.