feat(vps): port auth middleware to HttpApiMiddleware (Step 3b)

apps/vps/src/app.ts

@@ -1,6 +1,4 @@
-import { sql } from 'drizzle-orm'
-import { Data, Duration, Effect, Schedule } from 'effect'
-import type { Context } from 'hono'
+import { Duration, Effect, Schedule } from 'effect'
 import configureOpenAPI from '@/lib/configure-open-api'
 import { createAppEffect } from '@/lib/create-app'
 import admin from '@/routes/admin/admin.index'
@@ -22,25 +20,12 @@ import spotify from '@/routes/spotify/spotify.index'
 import upload from '@/routes/upload/upload.index'
 import uploadMultipart from '@/routes/upload-multipart/upload-multipart.index'
 import user from '@/routes/user/user.index'
-import { db } from './db'
 import { regenerateSitemap } from './routes/redirect/seo/sitemap'
 import { runApp, runAppFork } from './runtime'
 import { processPendingReminders, queryNextDueReminder } from './services/reminder-processor'
 import { ReminderSignalService } from './services/reminder-signal.service'
 import { SentryService } from './services/sentry.service'
 
-class HealthCheckError extends Data.TaggedError('HealthCheckError')<{
-  cause?: unknown
-}> {}
-
-const healthCheckEffect = Effect.tryPromise({
-  try: () => db.execute(sql.raw('SELECT 1')),
-  catch: (cause) => new HealthCheckError({ cause })
-})
-
-const READINESS_CACHE_MS = 5_000
-let readinessCache: { checkedAt: number; status: 200 | 500 } | undefined
-
 const setupRoutesEffect = Effect.gen(function* () {
   yield* SentryService
   const app = yield* createAppEffect
@@ -66,38 +51,12 @@ const setupRoutesEffect = Effect.gen(function* () {
   app.route('/api/music-reminders', musicReminders)
 
   // Kept at root, not under /api: these are externally-referenced public URLs.
-  // /auth is handled by the Effect router directly (apps/vps/src/http/routes.ts,
-  // step 2c) -- not mounted here.
+  // /auth and /health are handled by the Effect router directly
+  // (apps/vps/src/http/routes.ts, steps 2c/3a) -- not mounted here.
   app.route('/s', shareRouter)
   app.route('', rss)
   app.route('', seoRouter)
 
-  app.get('/health/live', (c) => c.json({ ok: true }, 200))
-
-  const readinessHealthRoute = async (c: Context) => {
-    const cache = readinessCache
-    const cachedStatus = cache && Date.now() - cache.checkedAt < READINESS_CACHE_MS
-
-    if (cachedStatus) {
-      return c.json({ dbConnected: cache.status === 200 }, cache.status)
-    }
-
-    const program = healthCheckEffect.pipe(
-      Effect.map(() => ({ data: { dbConnected: true }, status: 200 as const })),
-      Effect.catch(() => Effect.succeed({ data: { dbConnected: false }, status: 500 as const }))
-    )
-
-    const result = await runApp(program)
-    readinessCache = {
-      checkedAt: Date.now(),
-      status: result.status
-    }
-    return c.json(result.data, result.status)
-  }
-
-  app.get('/health/ready', readinessHealthRoute)
-  app.get('/health', readinessHealthRoute)
-
   return app
 })
 

apps/vps/src/http/health.handlers.failure.test.ts

@@ -0,0 +1,71 @@
+import { ReadinessCheckFailedError } from '@gbfm/api/errors'
+import { decodeResponseBody } from '@gbfm/api/testing'
+import { Effect } from 'effect'
+import { describe, expect, it } from 'vitest'
+import type { AppType } from '@/app'
+import { createWebHandler } from './routes'
+
+// Separate file (docs/migration-effect-http-api.md, step 3a): each
+// createWebHandler builds its own cached readiness check (health.handlers.ts,
+// Effect.cachedWithTTL inside makeHealthHandlers), so this doesn't strictly
+// need isolation from routes.blackbox.test.ts anymore -- kept in its own file
+// as a dedicated home for the failure/concurrency paths.
+describe('health readiness failure + cache', () => {
+  it('caches a failing readiness check and does not re-run it within the window', async () => {
+    const mod = await import('@/app')
+    const app: AppType = mod.default
+
+    let checks = 0
+    const scoped = createWebHandler(app, {
+      healthDatabaseCheck: Effect.sync(() => {
+        checks += 1
+      }).pipe(
+        Effect.flatMap(() => Effect.fail(new ReadinessCheckFailedError({ dbConnected: false })))
+      )
+    })
+
+    try {
+      const first = await scoped.handler(new Request('http://localhost/health/ready'))
+      const second = await scoped.handler(new Request('http://localhost/health/ready'))
+
+      expect(first.status).toBe(500)
+      expect(second.status).toBe(500)
+
+      const decoded = await decodeResponseBody(ReadinessCheckFailedError, first)
+      expect(decoded._tag).toBe('ReadinessCheckFailedError')
+      expect(decoded.dbConnected).toBe(false)
+
+      // Second call must reuse the cached failure, not re-run the check.
+      expect(checks).toBe(1)
+    } finally {
+      await scoped.dispose()
+    }
+  })
+
+  it('concurrent requests on a cold cache share one in-flight check, not one each', async () => {
+    const mod = await import('@/app')
+    const app: AppType = mod.default
+
+    let checks = 0
+    const scoped = createWebHandler(app, {
+      healthDatabaseCheck: Effect.sync(() => {
+        checks += 1
+      }).pipe(Effect.delay('20 millis'))
+    })
+
+    try {
+      const [first, second] = await Promise.all([
+        scoped.handler(new Request('http://localhost/health/ready')),
+        scoped.handler(new Request('http://localhost/health/ready'))
+      ])
+
+      expect(first.status).toBe(200)
+      expect(second.status).toBe(200)
+      // Both requests arrived before the (delayed) check completed -- they
+      // must share the one in-flight computation, not race to run it twice.
+      expect(checks).toBe(1)
+    } finally {
+      await scoped.dispose()
+    }
+  })
+})

apps/vps/src/http/health.handlers.ts

@@ -0,0 +1,60 @@
+import { Api } from '@gbfm/api/api'
+import { ReadinessCheckFailedError } from '@gbfm/api/errors'
+import { sql } from 'drizzle-orm'
+import { Effect, Layer } from 'effect'
+import { HttpApiBuilder } from 'effect/unstable/httpapi'
+import { db } from '@/db'
+
+const READINESS_CACHE_MS = 5_000
+
+// Injectable so tests can force the failure path without a real DB outage
+// (docs/migration-effect-http-api.md, step 1b noted this gap; step 3a closes it).
+// The cause is logged server-side only -- it must never reach the response
+// body (a public /health endpoint leaking internal DB error detail is a real
+// information-disclosure risk), but it also must not be silently discarded.
+export const checkDatabase = Effect.tryPromise({
+  try: () => db.execute(sql.raw('SELECT 1')),
+  catch: (cause) => cause
+}).pipe(
+  Effect.tapError((cause) => Effect.logError('[health] readiness check failed', cause)),
+  Effect.mapError(() => new ReadinessCheckFailedError({ dbConnected: false })),
+  Effect.asVoid
+)
+
+type ReadinessResult = { readonly dbConnected: true } | { readonly dbConnected: false }
+
+// Effect.cachedWithTTL memoizes the *pending* computation, not just its result:
+// concurrent requests that arrive while a check is in flight share that one
+// fiber instead of each independently hitting the database and racing to
+// write a cache slot (the module-level `let` this replaced had that race).
+const readinessResult = (check: Effect.Effect<void, ReadinessCheckFailedError>) =>
+  check.pipe(
+    Effect.as<ReadinessResult>({ dbConnected: true }),
+    Effect.catch(() => Effect.succeed<ReadinessResult>({ dbConnected: false })),
+    Effect.cachedWithTTL(`${READINESS_CACHE_MS} millis`)
+  )
+
+const readiness = (cachedCheck: Effect.Effect<ReadinessResult>) =>
+  Effect.gen(function* () {
+    const result = yield* cachedCheck
+    if (!result.dbConnected) {
+      return yield* new ReadinessCheckFailedError({ dbConnected: false })
+    }
+    return result
+  })
+
+export const makeHealthHandlers = (check: Effect.Effect<void, ReadinessCheckFailedError>) =>
+  Layer.unwrap(
+    Effect.gen(function* () {
+      const cachedCheck = yield* readinessResult(check)
+
+      return HttpApiBuilder.group(Api, 'health', (handlers) =>
+        handlers
+          .handle('live', () => Effect.succeed({ ok: true as const }))
+          .handle('ready', () => readiness(cachedCheck))
+          .handle('check', () => readiness(cachedCheck))
+      )
+    })
+  )
+
+export const HealthHandlersLive = makeHealthHandlers(checkDatabase)

apps/vps/src/http/internal.handlers.test.ts

@@ -0,0 +1,40 @@
+import { afterAll, beforeAll, describe, expect, it } from 'vitest'
+import type { AppType } from '@/app'
+import { createWebHandler } from './routes'
+
+// Step 3b (docs/migration-effect-http-api.md): validates AuthMiddleware's
+// cookie-reading and 401 path against a real HttpApiEndpoint, ahead of any
+// real authed route depending on it. The 200/authenticated path isn't
+// covered here -- no existing test in this codebase creates a real
+// better-auth session, and building that harness is bigger than this step's
+// scope (validating rejection, not building session-creation tooling).
+let app: AppType
+let webHandler: ReturnType<typeof createWebHandler>
+
+beforeAll(async () => {
+  const mod = await import('@/app')
+  app = mod.default
+  webHandler = createWebHandler(app)
+})
+
+afterAll(async () => {
+  await webHandler.dispose()
+})
+
+describe('AuthMiddleware (internal group)', () => {
+  it('GET /api/internal/whoami returns 401 without a session cookie', async () => {
+    const res = await webHandler.handler(new Request('http://localhost/api/internal/whoami'))
+
+    expect(res.status).toBe(401)
+  })
+
+  it('GET /api/internal/whoami returns 401 with an invalid session cookie', async () => {
+    const res = await webHandler.handler(
+      new Request('http://localhost/api/internal/whoami', {
+        headers: { cookie: 'better-auth.session_token=not-a-real-session' }
+      })
+    )
+
+    expect(res.status).toBe(401)
+  })
+})

apps/vps/src/http/internal.handlers.ts

@@ -0,0 +1,13 @@
+import { Api } from '@gbfm/api/api'
+import { AuthSession } from '@gbfm/api/middleware/auth'
+import { Effect } from 'effect'
+import { HttpApiBuilder } from 'effect/unstable/httpapi'
+
+export const InternalHandlersLive = HttpApiBuilder.group(Api, 'internal', (handlers) =>
+  handlers.handle('whoami', () =>
+    Effect.gen(function* () {
+      const { user } = yield* AuthSession
+      return { userId: user.id, email: user.email }
+    })
+  )
+)

apps/vps/src/http/routes.blackbox.test.ts

@@ -4,9 +4,9 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'
 import type { AppType } from '@/app'
 import { createWebHandler } from './routes'
 
-// Step 2a (docs/migration-effect-http-api.md): the Effect toWebHandler + Hono
-// fallback must behave identically to the plain Hono app it wraps. Reuses the
-// same @gbfm/api schemas as the 1b blackbox suite so both point at one contract.
+// Blackbox suite asserting only the wire contract, so these assertions keep
+// working as more groups move from the Hono fallback onto the Effect router
+// (docs/migration-effect-http-api.md).
 let app: AppType
 let webHandler: ReturnType<typeof createWebHandler>
 
@@ -21,22 +21,6 @@ afterAll(async () => {
 })
 
 describe('Effect toWebHandler fallback', () => {
-  it('GET /health/live matches the Hono app', async () => {
-    const res = await webHandler.handler(new Request('http://localhost/health/live'))
-
-    expect(res.status).toBe(200)
-    await expect(decodeResponseBody(HealthLiveResponse, res)).resolves.toEqual({ ok: true })
-  })
-
-  it('GET /health/ready matches the Hono app', async () => {
-    const res = await webHandler.handler(new Request('http://localhost/health/ready'))
-
-    expect(res.status).toBe(200)
-    await expect(decodeResponseBody(HealthReadyResponse, res)).resolves.toEqual({
-      dbConnected: true
-    })
-  })
-
   it('GET /api/music/artists returns the same response as the plain Hono app', async () => {
     const [viaHandler, viaHono] = await Promise.all([
       webHandler.handler(new Request('http://localhost/api/music/artists')),
@@ -75,3 +59,42 @@ describe('better-auth route (Step 2c)', () => {
     expect(await withAuth.text()).not.toEqual(await withoutAuth.text())
   })
 })
+
+describe('health (HttpApiBuilder group, Step 3a)', () => {
+  it('GET /health/live returns 200 without checking the database', async () => {
+    const res = await webHandler.handler(new Request('http://localhost/health/live'))
+
+    expect(res.status).toBe(200)
+    await expect(decodeResponseBody(HealthLiveResponse, res)).resolves.toEqual({ ok: true })
+  })
+
+  it('GET /health/ready and /health return 200 when the database check succeeds', async () => {
+    const res = await webHandler.handler(new Request('http://localhost/health/ready'))
+    const checkRes = await webHandler.handler(new Request('http://localhost/health'))
+
+    expect(res.status).toBe(200)
+    expect(checkRes.status).toBe(200)
+    await expect(decodeResponseBody(HealthReadyResponse, res)).resolves.toEqual({
+      dbConnected: true
+    })
+    await expect(decodeResponseBody(HealthReadyResponse, checkRes)).resolves.toEqual({
+      dbConnected: true
+    })
+  })
+
+  it('repeated readiness calls within the cache window keep returning 200', async () => {
+    const first = await webHandler.handler(new Request('http://localhost/health/ready'))
+    const second = await webHandler.handler(new Request('http://localhost/health/ready'))
+
+    expect(first.status).toBe(200)
+    expect(second.status).toBe(200)
+  })
+
+  it('responds 404 to unsupported methods on health paths', async () => {
+    const res = await webHandler.handler(
+      new Request('http://localhost/health/live', { method: 'POST' })
+    )
+
+    expect(res.status).toBe(404)
+  })
+})