Phase 2 stage 1 — Foundations: provider, contracts, constants, and the error handler fix

server/constants.ts

@@ -0,0 +1,76 @@
+/**
+ * Tunable values for the server.
+ *
+ * These were previously scattered as literals across services and routes,
+ * several of them redefined independently in more than one file. Each name
+ * here records what the number means, so a future change is a one-line edit
+ * instead of a grep-and-replace that risks missing a copy.
+ *
+ * Some constants share a value today (e.g. AI_GENERATION_TIMEOUT_MS and
+ * GENERATION_STREAM_CLEANUP_MS are both five minutes) but are kept separate
+ * because they mean different things and would be tuned independently.
+ */
+
+// --- AI generation ---
+
+/** Abort an AI SDK call (chapter, quiz, TOC, cover prompt, etc.) after this long. */
+export const AI_GENERATION_TIMEOUT_MS = 300_000 // 5 minutes
+
+/** How long a finished or failed SSE generation stream stays in memory, after its last subscriber leaves, before its state is evicted. */
+export const GENERATION_STREAM_CLEANUP_MS = 300_000 // 5 minutes
+
+/** How long a finished or failed background task (cover, epub, audiobook, etc.) stays in memory before eviction. */
+export const TASK_CLEANUP_DELAY_MS = 60_000 // 1 minute
+
+/** Timeout for fetching the available-models list from a provider's API. */
+export const MODEL_LIST_TIMEOUT_MS = 10_000
+
+/** Timeout for rendering a single mermaid diagram via the kroki.io fallback renderer. */
+export const DIAGRAM_RENDER_TIMEOUT_MS = 30_000
+
+// --- Book generation defaults ---
+
+/** Chapter count used when a create-book request doesn't specify one. */
+export const DEFAULT_CHAPTER_COUNT = 12
+
+/** Hard ceiling on chapter count, used when truncating an AI-generated or AI-revised table of contents. */
+export const MAX_CHAPTERS = 500
+
+/** Quiz question count used when a request doesn't specify one. */
+export const DEFAULT_QUIZ_LENGTH = 3
+
+/** Model ID used as a fallback for on-demand quiz generation when the request doesn't specify one (e.g. reconnecting to a chapter whose quiz file went missing). */
+export const DEFAULT_MODEL = 'claude-sonnet-4-6'
+
+// --- Context window sizing ---
+
+/** Characters kept from each chapter when summarizing a book for profile-suggestion prompts. */
+export const PROFILE_EXCERPT_CHARS = 300
+
+/** Characters of chapter content sent as context to the inline chat endpoint. */
+export const CHAT_CONTEXT_CHARS = 4000
+
+/** Characters of the previous chapter's tail included for continuity when generating the next chapter. */
+export const PREV_CHAPTER_TAIL_CHARS = 500
+
+/** Characters of surrounding text included on each side of a full-text search match, when building a result snippet. */
+export const SEARCH_SNIPPET_RADIUS = 60
+
+/** Characters of raw model output echoed in the error message when table-of-contents parsing fails. */
+export const TOC_ERROR_SNIPPET_CHARS = 300
+
+// --- HTTP caching ---
+
+/** Cache-Control max-age, in seconds, for served book cover images. */
+export const COVER_CACHE_MAX_AGE_S = 3600 // 1 hour
+
+/** Cache-Control max-age, in seconds, for synthesized audiobook voice preview clips. */
+export const VOICE_PREVIEW_CACHE_MAX_AGE_S = 2_592_000 // 30 days
+
+// --- Misc ---
+
+/** Max request body size, in bytes, for EPUB import endpoints (the request carries the EPUB as base64). */
+export const IMPORT_BODY_LIMIT_BYTES = 20 * 1024 * 1024 // 20MB, to accommodate ~10MB EPUB as base64
+
+/** Audio bitrate ffmpeg encodes with when muxing the final M4B audiobook. */
+export const M4B_BITRATE = '64k'

server/http/error-handler.test.ts

@@ -0,0 +1,76 @@
+import { describe, it, expect } from 'vitest'
+import { toErrorResponse } from './error-handler.js'
+import { RequestValidationError } from './parse.js'
+
+/**
+ * The error handler is expressed as a pure function so its behaviour can be
+ * pinned without booting a server.
+ *
+ * Context: until this phase the handler was registered AFTER every route
+ * plugin had been awaited, so it never applied to any route and every error
+ * fell through to Fastify's default handler. Phase 0's characterization tests
+ * recorded that broken behaviour deliberately. These tests describe the
+ * intended behaviour that registering it correctly now delivers.
+ */
+
+describe('toErrorResponse', () => {
+  it('renders a validation failure as the exact body the routes used to send', () => {
+    const issues = [{ code: 'custom' as const, path: ['topic'], message: 'Required' }]
+    const res = toErrorResponse(new RequestValidationError(issues))
+    expect(res.status).toBe(400)
+    expect(res.body).toEqual({ error: 'Invalid request', details: issues })
+    expect(res.logAsServerError).toBe(false)
+  })
+
+  it('turns a missing file into a 404 without echoing the path', () => {
+    const err = Object.assign(
+      new Error("ENOENT: no such file or directory, open '/Users/someone/Library/tutor/books/abc/meta.yml'"),
+      { code: 'ENOENT' },
+    )
+    const res = toErrorResponse(err)
+    expect(res.status).toBe(404)
+    expect(res.body).toEqual({ error: 'Not found' })
+    // The whole point: no filesystem path may reach the client.
+    expect(JSON.stringify(res.body)).not.toContain('/Users/')
+    expect(JSON.stringify(res.body)).not.toContain('meta.yml')
+  })
+
+  it('passes through a deliberate client error using its own message', () => {
+    const err = Object.assign(new Error('Chapter 99 out of range (1-12)'), { statusCode: 400 })
+    const res = toErrorResponse(err)
+    expect(res.status).toBe(400)
+    expect(res.body).toEqual({ error: 'Chapter 99 out of range (1-12)' })
+    expect(res.logAsServerError).toBe(false)
+  })
+
+  it('preserves other 4xx codes such as a conflict', () => {
+    const err = Object.assign(new Error('Book is generating'), { statusCode: 409 })
+    expect(toErrorResponse(err)).toMatchObject({ status: 409, body: { error: 'Book is generating' } })
+  })
+
+  it('hides the detail of an unexpected server error and flags it for logging', () => {
+    const res = toErrorResponse(new Error('connection string user:hunter2@db failed'))
+    expect(res.status).toBe(500)
+    expect(res.body).toEqual({ error: 'Internal server error' })
+    expect(JSON.stringify(res.body)).not.toContain('hunter2')
+    expect(res.logAsServerError).toBe(true)
+  })
+
+  it('treats an explicit 5xx statusCode as a server error too', () => {
+    const err = Object.assign(new Error('upstream exploded'), { statusCode: 502 })
+    const res = toErrorResponse(err)
+    expect(res.status).toBe(500)
+    expect(res.body).toEqual({ error: 'Internal server error' })
+    expect(res.logAsServerError).toBe(true)
+  })
+
+  it('falls back to a generic message when a client error carries none', () => {
+    const err = Object.assign(new Error(''), { statusCode: 400 })
+    expect(toErrorResponse(err).body).toEqual({ error: 'Internal server error' })
+  })
+
+  it('checks ENOENT before statusCode, so a tagged ENOENT still 404s', () => {
+    const err = Object.assign(new Error('nope'), { code: 'ENOENT', statusCode: 500 })
+    expect(toErrorResponse(err).status).toBe(404)
+  })
+})

server/http/error-handler.ts

@@ -0,0 +1,79 @@
+import type { FastifyInstance } from 'fastify'
+import { RequestValidationError } from './parse.js'
+
+/**
+ * The single place an error becomes a response.
+ *
+ * This handler existed before but never ran. It was registered on the root
+ * instance AFTER every route plugin had already been awaited through
+ * `register()`, and Fastify only propagates an error handler to child
+ * encapsulation contexts created after it is set. So every route booted
+ * against Fastify's default handler instead, which meant a missing book
+ * returned 500 with an absolute filesystem path in the message rather than a
+ * clean 404. Registering it ahead of the route plugins is the fix.
+ */
+
+/** What the client receives, plus whether the server should log it as a fault. */
+export interface ErrorResponse {
+  status: number
+  body: { error: string; details?: unknown }
+  logAsServerError: boolean
+}
+
+type AppError = Error & { code?: string; statusCode?: number }
+
+/**
+ * Pure mapping from a thrown error to a response, kept separate from Fastify
+ * so it can be unit tested without booting a server.
+ *
+ * Order matters. ENOENT is checked before `statusCode` so a missing file 404s
+ * even when something upstream tagged it 500.
+ */
+export function toErrorResponse(error: AppError): ErrorResponse {
+  if (error instanceof RequestValidationError) {
+    return {
+      status: 400,
+      body: { error: 'Invalid request', details: error.issues },
+      logAsServerError: false,
+    }
+  }
+
+  // Never echo the message here. It contains the absolute path of the file
+  // that was missing, which is exactly what leaked before this was fixed.
+  if (error.code === 'ENOENT') {
+    return { status: 404, body: { error: 'Not found' }, logAsServerError: false }
+  }
+
+  const statusCode = error.statusCode ?? 500
+
+  // An unexpected failure may carry anything in its message, including
+  // credentials from a connection string, so the client gets a generic body
+  // and the detail goes to the log instead.
+  if (statusCode >= 500) {
+    return { status: 500, body: { error: 'Internal server error' }, logAsServerError: true }
+  }
+
+  return {
+    status: statusCode,
+    body: { error: error.message || 'Internal server error' },
+    logAsServerError: false,
+  }
+}
+
+/**
+ * Registers the handler. MUST be called before any route plugin is registered,
+ * otherwise the plugins inherit Fastify's default handler and this one never
+ * runs. See the note at the top of this file.
+ */
+export function registerErrorHandler(fastify: FastifyInstance): void {
+  fastify.setErrorHandler((error: AppError, request, reply) => {
+    const { status, body, logAsServerError } = toErrorResponse(error)
+    if (logAsServerError) {
+      fastify.log.error(
+        { err: error, req: { method: request.method, url: request.url } },
+        'Unhandled server error',
+      )
+    }
+    return reply.status(status).send(body)
+  })
+}

server/http/parse.test.ts

@@ -0,0 +1,70 @@
+import { describe, it, expect } from 'vitest'
+import { z } from 'zod'
+import { parseBody, RequestValidationError } from './parse.js'
+
+/**
+ * parseBody replaces roughly thirty hand-written try/catch blocks that each
+ * did the same thing. The exact 400 body those blocks produced is a published
+ * contract the client reads, so these tests pin it rather than describe it.
+ */
+
+const Schema = z.object({
+  topic: z.string().min(1),
+  count: z.number().int().optional(),
+  tags: z.array(z.string()).default([]),
+})
+
+describe('parseBody', () => {
+  it('returns the parsed value when the body is valid', () => {
+    expect(parseBody(Schema, { topic: 'effect', count: 3 })).toEqual({
+      topic: 'effect',
+      count: 3,
+      tags: [],
+    })
+  })
+
+  it('applies schema defaults, which Fastify ajv validation would not', () => {
+    // This is why the refactor keeps Zod rather than moving to ajv `schema.body`.
+    expect(parseBody(Schema, { topic: 'effect' }).tags).toEqual([])
+  })
+
+  it('throws RequestValidationError rather than ZodError', () => {
+    expect(() => parseBody(Schema, {})).toThrow(RequestValidationError)
+  })
+
+  it('carries statusCode 400 so the error handler needs no special casing', () => {
+    try {
+      parseBody(Schema, {})
+      expect.unreachable('should have thrown')
+    } catch (err) {
+      expect((err as RequestValidationError).statusCode).toBe(400)
+    }
+  })
+
+  it('exposes the Zod issues verbatim, which the 400 body echoes as `details`', () => {
+    try {
+      parseBody(Schema, { topic: '' })
+      expect.unreachable('should have thrown')
+    } catch (err) {
+      const issues = (err as RequestValidationError).issues
+      expect(Array.isArray(issues)).toBe(true)
+      expect(issues.length).toBeGreaterThan(0)
+      expect(issues[0]).toHaveProperty('path')
+      expect(issues[0]).toHaveProperty('message')
+    }
+  })
+
+  it('uses the message the routes previously sent, so the body is unchanged', () => {
+    try {
+      parseBody(Schema, {})
+      expect.unreachable('should have thrown')
+    } catch (err) {
+      expect((err as Error).message).toBe('Invalid request')
+    }
+  })
+
+  it('rejects a null or undefined body the same way as a malformed one', () => {
+    expect(() => parseBody(Schema, null)).toThrow(RequestValidationError)
+    expect(() => parseBody(Schema, undefined)).toThrow(RequestValidationError)
+  })
+})

server/http/parse.ts

@@ -0,0 +1,43 @@
+import type { z } from 'zod'
+
+/**
+ * One way to validate a request body.
+ *
+ * Every route used to wrap `Schema.parse(request.body)` in its own try/catch,
+ * check `err instanceof ZodError`, and hand-write the same 400 response. That
+ * was repeated about thirty times, and the eight MCP authoring routes forgot
+ * the guard entirely, so bad input there surfaced as a 500.
+ *
+ * Fastify's own ajv `schema.body` validation was considered and rejected. It
+ * produces a different 400 payload, which the client already depends on, and
+ * it drops the Zod defaults and transforms the handlers rely on, such as
+ * `.default([])`.
+ */
+
+/**
+ * Thrown when a request body does not match its schema. Carries `statusCode`
+ * so the error handler needs no special casing, and `issues` so the response
+ * can echo them as `details`, exactly as the hand-written blocks did.
+ */
+export class RequestValidationError extends Error {
+  readonly statusCode = 400
+  readonly issues: z.core.$ZodIssue[]
+
+  constructor(issues: z.core.$ZodIssue[]) {
+    super('Invalid request')
+    this.name = 'RequestValidationError'
+    this.issues = issues
+  }
+}
+
+/**
+ * Parses a request body, throwing {@link RequestValidationError} on failure.
+ * Returns the parsed value, so schema defaults and transforms still apply.
+ */
+export function parseBody<T>(schema: z.ZodType<T>, body: unknown): T {
+  const result = schema.safeParse(body)
+  if (!result.success) {
+    throw new RequestValidationError(result.error.issues)
+  }
+  return result.data
+}

server/http/route-params.ts

@@ -0,0 +1,41 @@
+import { PROVIDERS } from '@shared/provider.js'
+
+/**
+ * Fastify JSON-schema fragments for route `:params`, shared by every route
+ * file that needs them. Before this module, `books.ts` and `covers.ts` each
+ * hand-rolled an identical book-id schema, and `models.ts` restated the
+ * provider list as a separate regex literal.
+ *
+ * The book id and chapter patterns must stay byte-identical to what shipped
+ * before — the Phase 0 characterization tests assert the exact 400 responses
+ * they produce for malformed params.
+ */
+
+/** Matches a route `:id` that must be a book id, e.g. `/api/books/:id`. */
+export const bookIdSchema = {
+  type: 'object' as const,
+  properties: { id: { type: 'string' as const, pattern: '^[a-z0-9-]{1,50}$' } },
+  required: ['id'] as const,
+}
+
+/** Matches a route with both a book `:id` and a chapter `:num`, e.g. `/api/books/:id/chapters/:num`. */
+export const bookChapterSchema = {
+  type: 'object' as const,
+  properties: {
+    id: { type: 'string' as const, pattern: '^[a-z0-9-]{1,50}$' },
+    num: { type: 'string' as const, pattern: '^[1-9][0-9]{0,2}$' },
+  },
+  required: ['id', 'num'] as const,
+}
+
+/**
+ * Matches a route `:provider` that must be one of the known AI providers.
+ * Built from the shared provider list rather than a restated regex literal,
+ * so a provider added to `shared/provider.ts` doesn't also need updating
+ * here.
+ */
+export const providerParamSchema = {
+  type: 'object' as const,
+  properties: { provider: { type: 'string' as const, pattern: `^(${PROVIDERS.join('|')})$` } },
+  required: ['provider'] as const,
+}