diff --git a/.env.example b/.env.example index fcaaa53..ca0f553 100644 --- a/.env.example +++ b/.env.example @@ -1,6 +1,47 @@ +# --- Secrets (required) --- + # OpenAI API key used server-side only (never sent to the browser). OPENAI_API_KEY= # Secret used to sign short-lived browser session tokens for the realtime WS handshake. # Generate one with: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))" REALTIME_TOKEN_SECRET= + +# --- Server --- + +# HTTP port the production server (server.js) listens on. +# PORT=3000 + +# --- Analytics (see docs/ANALYTICS.md and docs/CONFIGURATION.md) --- + +# Master switch. Set to false to disable analytics recording entirely — no +# data/analytics.db file is created at all. +# ANALYTICS_ENABLED=true + +# Where the local SQLite analytics db lives. +# ANALYTICS_DB_PATH=./data/analytics.db + +# Per-category toggles, independent of ANALYTICS_ENABLED and each other. +# Set any of these to false to stop recording that category's data while +# still recording session start/end (model, voice, timing) — that volume +# signal is always recorded whenever analytics is enabled at all. +# ANALYTICS_LOG_MESSAGES=true # conversation text (both roles) +# ANALYTICS_LOG_TOOL_CALLS=true # tool call arguments/output +# ANALYTICS_LOG_EVENTS=true # errors and session.closed detail + +# --- Session --- + +# Overrides the model configured in src/lib/project/persona.ts. Leave unset +# to use that file's default. +# SESSION_MODEL= + +# Voice used when the browser's session.start doesn't specify one. Leave +# unset to let the provider pick its own default. +# SESSION_DEFAULT_VOICE= + +# How long a client-executed tool call may go unanswered before the session +# gives up on it and resolves it with an error. +# CLIENT_TOOL_TIMEOUT_MS=10000 + +# TTL for the short-lived token minted by POST /api/realtime/session. +# REALTIME_TOKEN_TTL_MS=60000 diff --git a/docs/ANALYTICS.md b/docs/ANALYTICS.md index 982aa0b..231ff7c 100644 --- a/docs/ANALYTICS.md +++ b/docs/ANALYTICS.md @@ -37,7 +37,11 @@ Foreign keys are declared for documentation but explicitly not enforced (`PRAGMA ## Configuration -`ANALYTICS_DB_PATH` (optional) — defaults to `./data/analytics.db`. The parent directory is created automatically if missing. `/data` is gitignored. +Env vars are documented in full in [docs/CONFIGURATION.md](CONFIGURATION.md) — `ANALYTICS_DB_PATH` (parent directory created automatically if missing; `/data` is gitignored), plus `ANALYTICS_ENABLED` and three per-category toggles, described here since the selection logic is analytics-specific: + +`ANALYTICS_ENABLED=false` selects `NoopAnalyticsRecorder` instead of `SqliteAnalyticsRecorder` at the composition root (`server.js` / `dev-plugin.ts`) — every method is a no-op, so no `data/analytics.db` file is created at all, not just left empty. + +With analytics enabled, three independent toggles gate what `SqliteAnalyticsRecorder` actually writes: `ANALYTICS_LOG_MESSAGES` (the `messages` table), `ANALYTICS_LOG_TOOL_CALLS` (the `tool_calls` table), `ANALYTICS_LOG_EVENTS` (the `events` table). Each is a guard at the top of the corresponding recorder method — `session.ts`'s call sites are unaffected and still call every method unconditionally at the same lifecycle hook points described above. There is no toggle for the `sessions` row (start/end, model, voice, timing): that base volume signal is always recorded whenever the recorder is real, regardless of the three category toggles. ## PII and retention diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 19ca58b..957c2dd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -26,5 +26,6 @@ Routes (`src/routes/`) stay thin and delegate into these — a route file is glu | Realtime session management (OpenAI Realtime API today, extensible to other models/pipelines) | [REALTIME_SESSION.md](REALTIME_SESSION.md) | | Tool calling (server- and client-executed) and the core/project ownership boundary | [TOOLS.md](TOOLS.md) | | Analytics recording (sessions, messages, tool calls, errors) to a local SQLite database | [ANALYTICS.md](ANALYTICS.md) | +| Configuration (env vars, validation, the core/project split for persona content) | [CONFIGURATION.md](CONFIGURATION.md) | Add a row here (and a corresponding doc) for each new subsystem as it's built — see the "Documentation" section in [AGENTS.md](../AGENTS.md) for when and how. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md new file mode 100644 index 0000000..3339135 --- /dev/null +++ b/docs/CONFIGURATION.md @@ -0,0 +1,57 @@ +# Configuration + +Every env var this app reads is parsed and validated in one place: [`src/lib/server/core/config.ts`](../src/lib/server/core/config.ts)'s `loadConfig()`. It reads `process.env` directly (not `$env/static/private`) for the same reason `tokens.ts` and `dev-plugin.ts` already do — this module tree is compiled standalone (`tsc -p tsconfig.server.json`) for `server.js` to import outside Vite's module graph. See [`.env.example`](../.env.example) for the full list with inline comments. + +## `AppConfig` shape + +```ts +interface AppConfig { + port: number; + openaiApiKey: string; + analytics: AnalyticsConfig; // { enabled, dbPath, messages, toolCalls, events } + session: SessionRuntimeConfig; // { modelOverride?, defaultVoice?, clientToolTimeoutMs, tokenTtlMs } +} +``` + +`AnalyticsConfig` extends `AnalyticsCategoryConfig` (defined in `analytics.ts` itself, not here — it's the recorder's own config shape, the same way `SessionDefaults` lives in `session.ts`). + +## Env vars + +| Var | Default | Required | +| -------------------------- | ----------------------------------- | -------- | +| `OPENAI_API_KEY` | — | yes | +| `REALTIME_TOKEN_SECRET` | — | yes | +| `PORT` | `3000` | no | +| `ANALYTICS_ENABLED` | `true` | no | +| `ANALYTICS_DB_PATH` | `./data/analytics.db` | no | +| `ANALYTICS_LOG_MESSAGES` | `true` | no | +| `ANALYTICS_LOG_TOOL_CALLS` | `true` | no | +| `ANALYTICS_LOG_EVENTS` | `true` | no | +| `SESSION_MODEL` | `SESSION_PERSONA.model` (see below) | no | +| `SESSION_DEFAULT_VOICE` | unset — provider picks | no | +| `CLIENT_TOOL_TIMEOUT_MS` | `10000` | no | +| `REALTIME_TOKEN_TTL_MS` | `60000` | no | + +Booleans accept exactly `"true"`/`"false"`; anything else (if the var is set at all) is a validation error. Integers are validated for range where one applies (e.g. `PORT` must be `1–65535`). + +## Fail-fast, in both dev and prod + +`loadConfig()` collects every problem — missing required var, malformed boolean/integer, out-of-range integer — into a single thrown `Error` listing all of them, rather than failing on the first one found. It's called once, eagerly, at both composition roots (`server.js`'s top level; `dev-plugin.ts`'s `configureServer`, right after `.env` is loaded), and memoized after that (env vars don't change mid-process, and this gets called again from the session-token route to read `tokenTtlMs`). + +This means `npm run dev` now requires a fully-populated `.env` to start at all — previously, a missing `OPENAI_API_KEY` only produced a console warning in dev and the failure surfaced later, confusingly, inside a WS handshake attempt. Prod already threw eagerly; this just makes dev match it, consistent with this repo's existing "fail loud at boot" precedent (see `createToolRegistry`'s doc comment in `server/core/tools.ts`). + +`REALTIME_TOKEN_SECRET` is a deliberate partial exception: `loadConfig()` checks it's _present_ (so a missing secret is caught at the same startup point as everything else) but does not store its value on `AppConfig` — `tokens.ts` remains the sole reader of the actual secret, reading `process.env` directly and lazily, exactly as before. This is what keeps `tokens.spec.ts`'s existing pattern of mutating `process.env.REALTIME_TOKEN_SECRET` per test working unchanged. + +## The `vite.config.ts` / Vitest gotcha + +**Do not** move a throwing, env-validating call to `vite.config.ts`'s top level, and don't have it build an already-resolved session-defaults object either. Both Vitest projects in `vite.config.ts`'s `test.projects` use `extends: './vite.config.ts'`, so this file's top-level code — and any plugin `configureServer` hook — runs on every `npm run test:unit`, not just `npm run dev`. `dev-plugin.ts`'s `configureServer` already has to guard itself with `if (process.env.VITEST) return` for exactly this reason. + +Concretely: `vite.config.ts` passes only raw, env-independent pieces into the dev plugin — `realtimeDevWsPlugin({ persona: SESSION_PERSONA, toolRegistry })` — never a pre-resolved `defaults`/`config` object. The `loadConfig()` call, `resolveSessionDefaults(...)`, and the `NoopAnalyticsRecorder`/`SqliteAnalyticsRecorder` choice all happen _inside_ `configureServer`, below the `VITEST` guard. If a future change needs another env-driven value in dev, thread it through the same way — resolve it inside `configureServer`, not at `vite.config.ts` module scope — or `npm run test:unit` will start requiring a populated `.env` for no reason, and may pick up a developer's real local secrets into `process.env` during test runs. + +## Session persona vs. runtime overrides + +The actual persona — model choice and system instructions — is project-owned content in [`src/lib/project/persona.ts`](../src/lib/project/persona.ts), not part of this config system (see [`docs/TOOLS.md`](TOOLS.md) for why core/project content is split this way). `config.ts`'s `session.modelOverride` (from `SESSION_MODEL`) lets a deployment swap the model without touching that file; `instructions` deliberately has **no** env override — it's prose, and belongs in a commented TypeScript file, not a `.env` value. + +## What's not configurable (on purpose) + +`src/lib/server/core/providers/openai.ts`'s hardcoded upstream URL, its always-`['audio']` `output_modalities`, and its `tool_choice: 'auto'` are deliberately out of scope here — these are provider-tuning knobs nobody asked to change yet, and adding config surface for them now would be speculative. diff --git a/docs/REALTIME_SESSION.md b/docs/REALTIME_SESSION.md index c0d8afa..100037b 100644 --- a/docs/REALTIME_SESSION.md +++ b/docs/REALTIME_SESSION.md @@ -64,7 +64,7 @@ The critical constraint: a `ProviderSession` speaks **only** the normalized `Cli ## The trust boundary -The browser's `session.start` message can only set `voice` (see `SessionStartConfig` in `protocol.ts`). `instructions`, `tools`, and `model` are always server-owned — set in [`defaults.ts`](../src/lib/server/core/defaults.ts) and merged in `resolveSessionConfig()` (`session.ts`). This is deliberate: this relay is shared infrastructure that other people's browsers connect to, so a client project should never let an untrusted browser dictate what the model is told to do or which model/tools it can invoke. If a client project needs more client-controllable config, extend `SessionStartConfig` and `resolveSessionConfig()` explicitly and think about what's safe to expose — don't just pass the browser's config straight through. +The browser's `session.start` message can only set `voice` (see `SessionStartConfig` in `protocol.ts`). `instructions`, `tools`, and `model` are always server-owned — sourced from the project's persona ([`src/lib/project/persona.ts`](../src/lib/project/persona.ts), `model` optionally overridden by `SESSION_MODEL`; see [docs/CONFIGURATION.md](CONFIGURATION.md)) via `resolveSessionDefaults()` (`defaults.ts`), and merged with the client-visible config in `resolveSessionConfig()` (`session.ts`). A server-side `defaultVoice` (`SESSION_DEFAULT_VOICE`) fills in when the browser's `session.start` doesn't specify a voice, but the browser's own choice still wins when it sends one — this doesn't loosen the trust boundary, since voice was already the one client-controllable field. This is deliberate: this relay is shared infrastructure that other people's browsers connect to, so a client project should never let an untrusted browser dictate what the model is told to do or which model/tools it can invoke. If a client project needs more client-controllable config, extend `SessionStartConfig` and `resolveSessionConfig()` explicitly and think about what's safe to expose — don't just pass the browser's config straight through. ## SvelteKit WebSocket plumbing diff --git a/server.js b/server.js index c2fc40d..da64422 100644 --- a/server.js +++ b/server.js @@ -7,33 +7,41 @@ // provide OPENAI_API_KEY / REALTIME_TOKEN_SECRET via your deployment's real // secret mechanism. import { createServer } from 'node:http'; -import { join } from 'node:path'; import { handler } from './build/handler.js'; -import { SqliteAnalyticsRecorder } from './build-server/server/core/analytics.js'; -import { DEFAULT_SESSION_DEFAULTS } from './build-server/server/core/defaults.js'; +import { + NoopAnalyticsRecorder, + SqliteAnalyticsRecorder +} from './build-server/server/core/analytics.js'; +import { loadConfig } from './build-server/server/core/config.js'; +import { resolveSessionDefaults } from './build-server/server/core/defaults.js'; import { OpenAiRealtimeProvider } from './build-server/server/core/providers/openai.js'; import { handleRealtimeUpgrade } from './build-server/server/core/ws-handler.js'; import { toolRegistry } from './build-server/server/project/tools/index.js'; +import { SESSION_PERSONA } from './build-server/project/persona.js'; -const apiKey = process.env.OPENAI_API_KEY; -if (!apiKey) { - throw new Error('OPENAI_API_KEY is not set. See .env.example.'); -} +// Parses/validates every env var this app reads and throws one aggregate +// error if anything required is missing or invalid — see docs/CONFIGURATION.md. +const config = loadConfig(); -const provider = new OpenAiRealtimeProvider({ apiKey }); -const dbPath = process.env.ANALYTICS_DB_PATH ?? join(process.cwd(), 'data', 'analytics.db'); -const recorder = new SqliteAnalyticsRecorder(dbPath); +const provider = new OpenAiRealtimeProvider({ apiKey: config.openaiApiKey }); +const recorder = config.analytics.enabled + ? new SqliteAnalyticsRecorder(config.analytics.dbPath, config.analytics) + : new NoopAnalyticsRecorder(); // server/core never imports project code directly (see docs/TOOLS.md) — this // composition root is one of the two places (alongside vite.config.ts) that -// merges core defaults with the project's tool registry. -const defaults = { ...DEFAULT_SESSION_DEFAULTS, toolRegistry }; +// merges core defaults with the project's persona and tool registry. +const defaults = { ...resolveSessionDefaults(SESSION_PERSONA, config.session), toolRegistry }; const server = createServer(handler); server.on('upgrade', (req, socket, head) => { - handleRealtimeUpgrade(req, socket, head, { provider, defaults, recorder }); + handleRealtimeUpgrade(req, socket, head, { + provider, + defaults, + recorder, + clientToolTimeoutMs: config.session.clientToolTimeoutMs + }); }); -const port = process.env.PORT ? Number(process.env.PORT) : 3000; -server.listen(port, () => { - console.log(`listening on port ${port}`); +server.listen(config.port, () => { + console.log(`listening on port ${config.port}`); }); diff --git a/src/lib/project/persona.ts b/src/lib/project/persona.ts new file mode 100644 index 0000000..96ad706 --- /dev/null +++ b/src/lib/project/persona.ts @@ -0,0 +1,22 @@ +/** + * Project-owned session persona — the model and system instructions a + * branched client project actually wants. Isomorphic (no server/client-only + * imports), mirroring `$lib/project/tools.ts`: core never imports this file + * directly (see docs/TOOLS.md) — `resolveSessionDefaults` in + * `$lib/server/core/defaults.ts` takes it as a parameter instead, and the + * composition roots (server.js, vite.config.ts) are what wire it in. + * + * A branched project edits this file freely. `model` can also be overridden + * per-deployment via the `SESSION_MODEL` env var (see + * $lib/server/core/config.ts); `instructions` deliberately has no env + * override — prose doesn't belong in a .env value, and this file is its home. + */ +export interface SessionPersona { + model: string; + instructions: string; +} + +export const SESSION_PERSONA: SessionPersona = { + model: 'gpt-realtime', + instructions: 'You are a helpful voice assistant for a webavatar demo. Keep responses concise.' +}; diff --git a/src/lib/server/core/analytics.spec.ts b/src/lib/server/core/analytics.spec.ts index fe518c3..476c01f 100644 --- a/src/lib/server/core/analytics.spec.ts +++ b/src/lib/server/core/analytics.spec.ts @@ -2,11 +2,18 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; -import { SqliteAnalyticsRecorder } from './analytics.js'; +import type { AnalyticsCategoryConfig, AnalyticsRecorder } from './analytics.js'; +import { NoopAnalyticsRecorder, SqliteAnalyticsRecorder } from './analytics.js'; + +const ALL_CATEGORIES_ON: AnalyticsCategoryConfig = { + messages: true, + toolCalls: true, + events: true +}; describe('SqliteAnalyticsRecorder', () => { it('creates all four tables on construction', () => { - const recorder = new SqliteAnalyticsRecorder(':memory:'); + const recorder = new SqliteAnalyticsRecorder(':memory:', ALL_CATEGORIES_ON); const tables = recorder.rawDb .prepare("SELECT name FROM sqlite_master WHERE type = 'table'") .all() @@ -17,7 +24,7 @@ describe('SqliteAnalyticsRecorder', () => { }); it('persists a started session and leaves ended_at/end_reason null', () => { - const recorder = new SqliteAnalyticsRecorder(':memory:'); + const recorder = new SqliteAnalyticsRecorder(':memory:', ALL_CATEGORIES_ON); recorder.sessionStarted({ sessionId: 'session-1', model: 'test-model', @@ -37,7 +44,7 @@ describe('SqliteAnalyticsRecorder', () => { }); it('updates the same session row on sessionEnded, leaving other columns unchanged', () => { - const recorder = new SqliteAnalyticsRecorder(':memory:'); + const recorder = new SqliteAnalyticsRecorder(':memory:', ALL_CATEGORIES_ON); recorder.sessionStarted({ sessionId: 'session-1', model: 'test-model', @@ -57,7 +64,7 @@ describe('SqliteAnalyticsRecorder', () => { }); it('logs messages for both roles', () => { - const recorder = new SqliteAnalyticsRecorder(':memory:'); + const recorder = new SqliteAnalyticsRecorder(':memory:', ALL_CATEGORIES_ON); recorder.messageLogged({ sessionId: 'session-1', role: 'user', text: 'hi', loggedAt: 1 }); recorder.messageLogged({ sessionId: 'session-1', @@ -76,7 +83,7 @@ describe('SqliteAnalyticsRecorder', () => { }); it('round-trips a logged tool call', () => { - const recorder = new SqliteAnalyticsRecorder(':memory:'); + const recorder = new SqliteAnalyticsRecorder(':memory:', ALL_CATEGORIES_ON); recorder.toolCallLogged({ sessionId: 'session-1', callId: 'call-1', @@ -99,7 +106,7 @@ describe('SqliteAnalyticsRecorder', () => { }); it('logs an event with a null session_id for a pre-session error', () => { - const recorder = new SqliteAnalyticsRecorder(':memory:'); + const recorder = new SqliteAnalyticsRecorder(':memory:', ALL_CATEGORIES_ON); recorder.eventLogged({ sessionId: null, type: 'error', @@ -116,7 +123,7 @@ describe('SqliteAnalyticsRecorder', () => { }); it('logs an event against a real session_id once a session exists', () => { - const recorder = new SqliteAnalyticsRecorder(':memory:'); + const recorder = new SqliteAnalyticsRecorder(':memory:', ALL_CATEGORIES_ON); recorder.eventLogged({ sessionId: 'session-1', type: 'session.closed', @@ -143,9 +150,101 @@ describe('SqliteAnalyticsRecorder', () => { // mean anything — asserting it against the ':memory:' recorder used // everywhere else in this file would be misleading. tmpDir = mkdtempSync(join(tmpdir(), 'analytics-test-')); - const recorder = new SqliteAnalyticsRecorder(join(tmpDir, 'analytics.db')); + const recorder = new SqliteAnalyticsRecorder(join(tmpDir, 'analytics.db'), ALL_CATEGORIES_ON); expect(recorder.rawDb.pragma('journal_mode', { simple: true })).toBe('wal'); recorder.close(); }); }); + + describe('category toggles', () => { + it('skips writing to messages when the messages category is off', () => { + const recorder = new SqliteAnalyticsRecorder(':memory:', { + ...ALL_CATEGORIES_ON, + messages: false + }); + recorder.messageLogged({ sessionId: 'session-1', role: 'user', text: 'hi', loggedAt: 1 }); + + expect(recorder.rawDb.prepare('SELECT * FROM messages').all()).toEqual([]); + }); + + it('skips writing to tool_calls when the toolCalls category is off', () => { + const recorder = new SqliteAnalyticsRecorder(':memory:', { + ...ALL_CATEGORIES_ON, + toolCalls: false + }); + recorder.toolCallLogged({ + sessionId: 'session-1', + callId: 'call-1', + name: 'get_current_time', + arguments: '{}', + output: '{}', + durationMs: 1, + loggedAt: 1 + }); + + expect(recorder.rawDb.prepare('SELECT * FROM tool_calls').all()).toEqual([]); + }); + + it('skips writing to events when the events category is off', () => { + const recorder = new SqliteAnalyticsRecorder(':memory:', { + ...ALL_CATEGORIES_ON, + events: false + }); + recorder.eventLogged({ sessionId: null, type: 'error', detail: '{}', loggedAt: 1 }); + + expect(recorder.rawDb.prepare('SELECT * FROM events').all()).toEqual([]); + }); + + it('never gates sessionStarted/sessionEnded, even with every category off', () => { + const recorder = new SqliteAnalyticsRecorder(':memory:', { + messages: false, + toolCalls: false, + events: false + }); + recorder.sessionStarted({ + sessionId: 'session-1', + model: 'test-model', + voice: undefined, + startedAt: 1000 + }); + recorder.sessionEnded({ sessionId: 'session-1', endedAt: 2000, reason: 'client_requested' }); + + const row = recorder.rawDb.prepare('SELECT * FROM sessions WHERE id = ?').get('session-1'); + expect(row).toMatchObject({ + id: 'session-1', + ended_at: 2000, + end_reason: 'client_requested' + }); + }); + }); +}); + +describe('NoopAnalyticsRecorder', () => { + it('does not throw for any recorder method, and touches no storage', () => { + // Typed as the interface, not the concrete class: NoopAnalyticsRecorder's + // own methods take no parameters (still valid — a function taking fewer + // params satisfies a wider interface type), but calling through the + // interface is what this test actually wants to exercise. + const recorder: AnalyticsRecorder = new NoopAnalyticsRecorder(); + expect(() => { + recorder.sessionStarted({ + sessionId: 'session-1', + model: 'test-model', + voice: undefined, + startedAt: 1 + }); + recorder.sessionEnded({ sessionId: 'session-1', endedAt: 2, reason: 'client_requested' }); + recorder.messageLogged({ sessionId: 'session-1', role: 'user', text: 'hi', loggedAt: 1 }); + recorder.toolCallLogged({ + sessionId: 'session-1', + callId: 'call-1', + name: 'get_current_time', + arguments: '{}', + output: '{}', + durationMs: 1, + loggedAt: 1 + }); + recorder.eventLogged({ sessionId: null, type: 'error', detail: '{}', loggedAt: 1 }); + }).not.toThrow(); + }); }); diff --git a/src/lib/server/core/analytics.ts b/src/lib/server/core/analytics.ts index bcedbc2..eef4afe 100644 --- a/src/lib/server/core/analytics.ts +++ b/src/lib/server/core/analytics.ts @@ -8,6 +8,18 @@ import { dirname } from 'node:path'; * API — every call site in `session.ts` stays fire-and-forget with no * `await`/error-handling threaded through the session's hot path. */ +/** + * Which categories of data `SqliteAnalyticsRecorder` actually writes. + * `sessions` volume (start/end, model, voice) is never gated by this — only + * the three data categories that can carry sensitive content are. See + * `$lib/server/core/config.ts` for where these are sourced from env vars. + */ +export interface AnalyticsCategoryConfig { + messages: boolean; + toolCalls: boolean; + events: boolean; +} + export interface AnalyticsRecorder { sessionStarted(params: { sessionId: string; @@ -105,7 +117,10 @@ export class SqliteAnalyticsRecorder implements AnalyticsRecorder { private readonly insertToolCall: Database.Statement; private readonly insertEvent: Database.Statement; - constructor(dbPath: string) { + constructor( + dbPath: string, + private readonly categories: AnalyticsCategoryConfig + ) { // better-sqlite3 does not create the parent directory itself — on a // fresh checkout `data/` doesn't exist yet. No-op for ':memory:' // (dirname resolves to '.'). @@ -160,6 +175,7 @@ export class SqliteAnalyticsRecorder implements AnalyticsRecorder { text, loggedAt }: Parameters[0]): void { + if (!this.categories.messages) return; this.insertMessage.run(sessionId, role, text, loggedAt); } @@ -175,6 +191,7 @@ export class SqliteAnalyticsRecorder implements AnalyticsRecorder { durationMs, loggedAt }: Parameters[0]): void { + if (!this.categories.toolCalls) return; this.insertToolCall.run(sessionId, callId, name, args, output, durationMs, loggedAt); } @@ -184,6 +201,7 @@ export class SqliteAnalyticsRecorder implements AnalyticsRecorder { detail, loggedAt }: Parameters[0]): void { + if (!this.categories.events) return; this.insertEvent.run(sessionId, type, detail, loggedAt); } @@ -196,3 +214,18 @@ export class SqliteAnalyticsRecorder implements AnalyticsRecorder { this.db.close(); } } + +/** + * Selected instead of `SqliteAnalyticsRecorder` when analytics is disabled + * entirely (`ANALYTICS_ENABLED=false` — see config.ts). No constructor args, + * no filesystem/DB interaction of any kind: choosing this at a composition + * root guarantees the analytics db file is never created, not merely left + * empty. + */ +export class NoopAnalyticsRecorder implements AnalyticsRecorder { + sessionStarted(): void {} + sessionEnded(): void {} + messageLogged(): void {} + toolCallLogged(): void {} + eventLogged(): void {} +} diff --git a/src/lib/server/core/config.spec.ts b/src/lib/server/core/config.spec.ts new file mode 100644 index 0000000..c01a0ea --- /dev/null +++ b/src/lib/server/core/config.spec.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Every env var loadConfig() reads — snapshotted/restored per test so this +// file can't leak state into (or pick up state from) other spec files. +const ENV_KEYS = [ + 'OPENAI_API_KEY', + 'REALTIME_TOKEN_SECRET', + 'PORT', + 'ANALYTICS_ENABLED', + 'ANALYTICS_DB_PATH', + 'ANALYTICS_LOG_MESSAGES', + 'ANALYTICS_LOG_TOOL_CALLS', + 'ANALYTICS_LOG_EVENTS', + 'SESSION_MODEL', + 'SESSION_DEFAULT_VOICE', + 'CLIENT_TOOL_TIMEOUT_MS', + 'REALTIME_TOKEN_TTL_MS' +] as const; + +const REQUIRED_ENV = { + OPENAI_API_KEY: 'test-key', + REALTIME_TOKEN_SECRET: 'test-secret' +}; + +let originalEnv: Partial>; + +beforeEach(() => { + originalEnv = {}; + for (const key of ENV_KEYS) { + originalEnv[key] = process.env[key]; + delete process.env[key]; + } +}); + +afterEach(() => { + for (const key of ENV_KEYS) { + if (originalEnv[key] === undefined) delete process.env[key]; + else process.env[key] = originalEnv[key]; + } +}); + +/** + * loadConfig() memoizes its result at module scope, so every test needs a + * fresh module instance to see its own process.env setup rather than a + * previous test's cached result. + */ +async function freshLoadConfig() { + vi.resetModules(); + const mod = await import('./config.js'); + return mod.loadConfig; +} + +describe('loadConfig', () => { + it('applies every default when only the required secrets are set', async () => { + Object.assign(process.env, REQUIRED_ENV); + const loadConfig = await freshLoadConfig(); + + expect(loadConfig()).toEqual({ + port: 3000, + openaiApiKey: 'test-key', + analytics: { + enabled: true, + dbPath: expect.stringContaining('analytics.db'), + messages: true, + toolCalls: true, + events: true + }, + session: { + modelOverride: undefined, + defaultVoice: undefined, + clientToolTimeoutMs: 10_000, + tokenTtlMs: 60_000 + } + }); + }); + + it('does not expose REALTIME_TOKEN_SECRET anywhere on the returned config', async () => { + Object.assign(process.env, REQUIRED_ENV); + const loadConfig = await freshLoadConfig(); + + expect(JSON.stringify(loadConfig())).not.toContain('test-secret'); + }); + + it('reads every override when set', async () => { + Object.assign(process.env, REQUIRED_ENV, { + PORT: '8080', + ANALYTICS_ENABLED: 'false', + ANALYTICS_DB_PATH: '/tmp/custom.db', + ANALYTICS_LOG_MESSAGES: 'false', + ANALYTICS_LOG_TOOL_CALLS: 'false', + ANALYTICS_LOG_EVENTS: 'false', + SESSION_MODEL: 'custom-model', + SESSION_DEFAULT_VOICE: 'alloy', + CLIENT_TOOL_TIMEOUT_MS: '5000', + REALTIME_TOKEN_TTL_MS: '30000' + }); + const loadConfig = await freshLoadConfig(); + + expect(loadConfig()).toEqual({ + port: 8080, + openaiApiKey: 'test-key', + analytics: { + enabled: false, + dbPath: '/tmp/custom.db', + messages: false, + toolCalls: false, + events: false + }, + session: { + modelOverride: 'custom-model', + defaultVoice: 'alloy', + clientToolTimeoutMs: 5000, + tokenTtlMs: 30000 + } + }); + }); + + it('memoizes: a later process.env change is not picked up by the same loaded module', async () => { + Object.assign(process.env, REQUIRED_ENV); + const loadConfig = await freshLoadConfig(); + + expect(loadConfig().port).toBe(3000); + process.env.PORT = '9999'; + expect(loadConfig().port).toBe(3000); + }); + + it('throws mentioning every missing required var when none are set', async () => { + const loadConfig = await freshLoadConfig(); + expect(() => loadConfig()).toThrow(/OPENAI_API_KEY/); + expect(() => loadConfig()).toThrow(/REALTIME_TOKEN_SECRET/); + }); + + it('throws a descriptive error for an invalid boolean value', async () => { + Object.assign(process.env, REQUIRED_ENV, { ANALYTICS_ENABLED: 'yes' }); + const loadConfig = await freshLoadConfig(); + expect(() => loadConfig()).toThrow(/ANALYTICS_ENABLED must be "true" or "false"/); + }); + + it('throws a descriptive error for a non-integer value', async () => { + Object.assign(process.env, REQUIRED_ENV, { PORT: 'not-a-number' }); + const loadConfig = await freshLoadConfig(); + expect(() => loadConfig()).toThrow(/PORT must be an integer/); + }); + + it('throws a descriptive error when an integer is out of range', async () => { + Object.assign(process.env, REQUIRED_ENV, { PORT: '99999' }); + const loadConfig = await freshLoadConfig(); + expect(() => loadConfig()).toThrow(/PORT must be <= 65535/); + }); + + it('aggregates multiple simultaneous problems into one thrown error', async () => { + Object.assign(process.env, { + // OPENAI_API_KEY and REALTIME_TOKEN_SECRET both left unset. + PORT: 'not-a-number', + ANALYTICS_ENABLED: 'maybe' + }); + const loadConfig = await freshLoadConfig(); + + let thrown: unknown; + try { + loadConfig(); + } catch (err) { + thrown = err; + } + + expect(thrown).toBeInstanceOf(Error); + const message = (thrown as Error).message; + expect(message).toContain('OPENAI_API_KEY'); + expect(message).toContain('REALTIME_TOKEN_SECRET'); + expect(message).toContain('PORT'); + expect(message).toContain('ANALYTICS_ENABLED'); + }); +}); diff --git a/src/lib/server/core/config.ts b/src/lib/server/core/config.ts new file mode 100644 index 0000000..d5a85eb --- /dev/null +++ b/src/lib/server/core/config.ts @@ -0,0 +1,128 @@ +import { join } from 'node:path'; +import type { AnalyticsCategoryConfig } from './analytics.js'; + +/** + * Central env-var parsing/validation for this app. Lives in `server/core` + * (not `core/`) because it reads `process.env` directly rather than + * SvelteKit's `$env/static/private` — same reasoning as `tokens.ts`: this + * module tree is compiled standalone (`tsc -p tsconfig.server.json`) for + * `server.js` to import, outside Vite's module graph. + */ + +export interface AnalyticsConfig extends AnalyticsCategoryConfig { + enabled: boolean; + dbPath: string; +} + +export interface SessionRuntimeConfig { + /** Overrides `SessionPersona.model` (see $lib/project/persona.ts) when set. */ + modelOverride?: string; + defaultVoice?: string; + clientToolTimeoutMs: number; + tokenTtlMs: number; +} + +export interface AppConfig { + port: number; + openaiApiKey: string; + analytics: AnalyticsConfig; + session: SessionRuntimeConfig; +} + +let cached: AppConfig | null = null; + +/** + * Parses and validates every env var this app reads, once, into a single + * typed object — memoized, since env vars don't change mid-process and this + * is called from both composition roots (server.js, dev-plugin.ts) plus the + * session-token route. Throws one Error listing every problem found, not + * just the first, so a misconfigured deployment fails loudly and completely + * at startup rather than one env var at a time. + * + * `REALTIME_TOKEN_SECRET` is checked for presence here (so a missing secret + * fails at the same startup point as everything else) but its value is + * deliberately NOT stored on `AppConfig` — `tokens.ts` remains the sole + * reader of the actual secret, reading `process.env` directly and lazily. + * Routing it through this memoized singleton would break `tokens.spec.ts`'s + * pattern of mutating `process.env.REALTIME_TOKEN_SECRET` per test to + * exercise `mintSessionToken`/`verifySessionToken` against different + * secrets — don't "simplify" this away. + */ +export function loadConfig(): AppConfig { + if (cached) return cached; + + const errors: string[] = []; + + const port = parseIntEnv('PORT', 3000, { min: 1, max: 65535 }, errors); + const openaiApiKey = requireEnv('OPENAI_API_KEY', errors); + requireEnv('REALTIME_TOKEN_SECRET', errors); // presence only — see doc comment above + + const analytics: AnalyticsConfig = { + enabled: parseBoolEnv('ANALYTICS_ENABLED', true, errors), + dbPath: process.env.ANALYTICS_DB_PATH || join(process.cwd(), 'data', 'analytics.db'), + messages: parseBoolEnv('ANALYTICS_LOG_MESSAGES', true, errors), + toolCalls: parseBoolEnv('ANALYTICS_LOG_TOOL_CALLS', true, errors), + events: parseBoolEnv('ANALYTICS_LOG_EVENTS', true, errors) + }; + + const session: SessionRuntimeConfig = { + modelOverride: optionalEnv('SESSION_MODEL'), + defaultVoice: optionalEnv('SESSION_DEFAULT_VOICE'), + clientToolTimeoutMs: parseIntEnv('CLIENT_TOOL_TIMEOUT_MS', 10_000, { min: 1 }, errors), + tokenTtlMs: parseIntEnv('REALTIME_TOKEN_TTL_MS', 60_000, { min: 1 }, errors) + }; + + if (errors.length > 0) { + throw new Error(`Invalid configuration:\n${errors.map((e) => ` - ${e}`).join('\n')}`); + } + + cached = { port, openaiApiKey, analytics, session }; + return cached; +} + +function parseBoolEnv(name: string, def: boolean, errors: string[]): boolean { + const raw = process.env[name]; + if (raw === undefined || raw === '') return def; + if (raw === 'true') return true; + if (raw === 'false') return false; + errors.push(`${name} must be "true" or "false" if set (got "${raw}").`); + return def; +} + +function parseIntEnv( + name: string, + def: number, + opts: { min?: number; max?: number }, + errors: string[] +): number { + const raw = process.env[name]; + if (raw === undefined || raw === '') return def; + const n = Number(raw); + if (!Number.isInteger(n)) { + errors.push(`${name} must be an integer if set (got "${raw}").`); + return def; + } + if (opts.min !== undefined && n < opts.min) { + errors.push(`${name} must be >= ${opts.min} (got ${n}).`); + return def; + } + if (opts.max !== undefined && n > opts.max) { + errors.push(`${name} must be <= ${opts.max} (got ${n}).`); + return def; + } + return n; +} + +function requireEnv(name: string, errors: string[]): string { + const raw = process.env[name]; + if (!raw) { + errors.push(`${name} is not set. See .env.example.`); + return ''; + } + return raw; +} + +function optionalEnv(name: string): string | undefined { + const raw = process.env[name]; + return raw === undefined || raw === '' ? undefined : raw; +} diff --git a/src/lib/server/core/defaults.spec.ts b/src/lib/server/core/defaults.spec.ts new file mode 100644 index 0000000..10cc1dd --- /dev/null +++ b/src/lib/server/core/defaults.spec.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; +import { resolveSessionDefaults } from './defaults.js'; + +const PERSONA = { model: 'persona-model', instructions: 'persona instructions' }; + +describe('resolveSessionDefaults', () => { + it('uses the persona model/instructions when no overrides are given', () => { + expect(resolveSessionDefaults(PERSONA, {})).toEqual({ + model: 'persona-model', + instructions: 'persona instructions', + defaultVoice: undefined + }); + }); + + it('prefers modelOverride over the persona model when set', () => { + expect(resolveSessionDefaults(PERSONA, { modelOverride: 'override-model' })).toEqual({ + model: 'override-model', + instructions: 'persona instructions', + defaultVoice: undefined + }); + }); + + it('passes defaultVoice through unchanged', () => { + expect(resolveSessionDefaults(PERSONA, { defaultVoice: 'coral' })).toEqual({ + model: 'persona-model', + instructions: 'persona instructions', + defaultVoice: 'coral' + }); + }); +}); diff --git a/src/lib/server/core/defaults.ts b/src/lib/server/core/defaults.ts index 55fd55e..9595568 100644 --- a/src/lib/server/core/defaults.ts +++ b/src/lib/server/core/defaults.ts @@ -1,16 +1,24 @@ +import type { SessionPersona } from '../../project/persona.js'; import type { SessionDefaults } from './session.js'; /** - * Server-owned session policy shared by the dev-mode Vite plugin and the - * production custom server, so both environments start sessions identically. - * A real client project will likely replace this with its own - * persona/instructions, possibly per-route rather than a single constant. + * Merges a project's persona (`$lib/project/persona.ts`) with env-driven + * overrides (`$lib/server/core/config.ts`'s `SessionRuntimeConfig`) into the + * session policy shared by the dev-mode Vite plugin and the production + * custom server, so both environments start sessions identically. * - * Deliberately missing `toolRegistry`: this file is core, and core never - * imports project code (see docs/TOOLS.md) — the composition roots - * (server.js, vite.config.ts) merge this with the project's tool registry. + * This file is core and never imports project code directly (see + * docs/TOOLS.md) — `persona` is taken as a parameter, and the composition + * roots (server.js, vite.config.ts) are what supply it alongside the + * project's tool registry. */ -export const DEFAULT_SESSION_DEFAULTS: Omit = { - model: 'gpt-realtime', - instructions: 'You are a helpful voice assistant for a webavatar demo. Keep responses concise.' -}; +export function resolveSessionDefaults( + persona: SessionPersona, + overrides: { modelOverride?: string; defaultVoice?: string } +): Omit { + return { + model: overrides.modelOverride ?? persona.model, + instructions: persona.instructions, + defaultVoice: overrides.defaultVoice + }; +} diff --git a/src/lib/server/core/dev-plugin.ts b/src/lib/server/core/dev-plugin.ts index 8229522..a82fa2b 100644 --- a/src/lib/server/core/dev-plugin.ts +++ b/src/lib/server/core/dev-plugin.ts @@ -1,18 +1,25 @@ -import { join } from 'node:path'; import { loadEnv, type Plugin } from 'vite'; -import { SqliteAnalyticsRecorder } from './analytics.js'; +import type { SessionPersona } from '../../project/persona.js'; +import { NoopAnalyticsRecorder, SqliteAnalyticsRecorder } from './analytics.js'; +import { loadConfig } from './config.js'; +import { resolveSessionDefaults } from './defaults.js'; import { OpenAiRealtimeProvider } from './providers/openai.js'; -import type { SessionDefaults } from './session.js'; +import type { ToolRegistry } from './tools.js'; import { handleRealtimeUpgrade } from './ws-handler.js'; export interface RealtimeDevWsPluginOptions { /** - * Merged session defaults (core defaults + project tool registry) — taken - * as a parameter rather than imported here so this module (server/core) - * never has to import project code directly. See vite.config.ts, the - * composition root that does the merge, and docs/TOOLS.md. + * Raw, env-independent pieces (project persona + tool registry) — taken as + * parameters rather than imported here so this module (server/core) never + * has to import project code directly. See vite.config.ts, the + * composition root that supplies these, and docs/TOOLS.md. + * + * Deliberately NOT pre-resolved against env vars by the caller: resolving + * them happens inside configureServer below, gated by the VITEST check — + * see the comment on that check for why. */ - defaults: SessionDefaults; + persona: SessionPersona; + toolRegistry: ToolRegistry; } /** @@ -30,7 +37,12 @@ export function realtimeDevWsPlugin(options: RealtimeDevWsPluginOptions): Plugin name: 'webavatar-realtime-ws-dev', configureServer(server) { // vitest's browser-mode project boots its own Vite dev server for - // component tests; this plugin has nothing to do there. + // component tests; this plugin has nothing to do there. Critically, + // this guard is also what keeps loadConfig()'s fail-fast validation + // (below) from running during `npm run test:unit` — that suite's + // `test.projects` entries both `extends: './vite.config.ts'`, so this + // plugin's config gets evaluated there too, and unit tests need no env + // vars set at all. Don't move config resolution above this line. if (process.env.VITEST) return; const loaded = loadEnv('development', process.cwd(), ''); @@ -38,21 +50,22 @@ export function realtimeDevWsPlugin(options: RealtimeDevWsPluginOptions): Plugin if (process.env[key] === undefined) process.env[key] = value; } - const apiKey = process.env.OPENAI_API_KEY; - if (!apiKey) { - console.warn( - '[realtime] OPENAI_API_KEY is not set (see .env.example) — realtime sessions will fail to start.' - ); - } - const provider = new OpenAiRealtimeProvider({ apiKey: apiKey ?? '' }); - const dbPath = process.env.ANALYTICS_DB_PATH ?? join(process.cwd(), 'data', 'analytics.db'); - const recorder = new SqliteAnalyticsRecorder(dbPath); + const config = loadConfig(); + const provider = new OpenAiRealtimeProvider({ apiKey: config.openaiApiKey }); + const recorder = config.analytics.enabled + ? new SqliteAnalyticsRecorder(config.analytics.dbPath, config.analytics) + : new NoopAnalyticsRecorder(); + const defaults = { + ...resolveSessionDefaults(options.persona, config.session), + toolRegistry: options.toolRegistry + }; server.httpServer?.on('upgrade', (req, socket, head) => { handleRealtimeUpgrade(req, socket, head, { provider, - defaults: options.defaults, - recorder + defaults, + recorder, + clientToolTimeoutMs: config.session.clientToolTimeoutMs }); }); } diff --git a/src/lib/server/core/session.spec.ts b/src/lib/server/core/session.spec.ts index c5eff6a..0bbb16a 100644 --- a/src/lib/server/core/session.spec.ts +++ b/src/lib/server/core/session.spec.ts @@ -14,6 +14,9 @@ const DEFAULTS = { toolRegistry: createToolRegistry([], {}) }; +/** Fixture value for RealtimeSession's clientToolTimeoutMs — arbitrary but matches the old hardcoded default, since most tests don't care about its exact value. */ +const TEST_TOOL_TIMEOUT_MS = 10_000; + /** Minimal stand-in for a `ws` WebSocket — only what RealtimeSession touches. */ class FakeBrowserSocket extends EventEmitter { readyState = 1; // WebSocket.OPEN @@ -132,6 +135,26 @@ describe('resolveSessionConfig', () => { tools: [] }); }); + + it('falls back to defaults.defaultVoice when the client does not specify one', () => { + expect(resolveSessionConfig({ ...DEFAULTS, defaultVoice: 'alloy' })).toEqual({ + model: 'test-model', + instructions: 'be helpful', + voice: 'alloy', + tools: [] + }); + }); + + it('prefers the client-specified voice over defaults.defaultVoice when both are set', () => { + expect( + resolveSessionConfig({ ...DEFAULTS, defaultVoice: 'alloy' }, { voice: 'coral' }) + ).toEqual({ + model: 'test-model', + instructions: 'be helpful', + voice: 'coral', + tools: [] + }); + }); }); describe('RealtimeSession', () => { @@ -139,7 +162,7 @@ describe('RealtimeSession', () => { const provider = new FakeProvider(); const recorder = new FakeAnalyticsRecorder(); const { socket, asWs } = makeSocket(); - createRealtimeSession(asWs, provider, DEFAULTS, recorder); + createRealtimeSession(asWs, provider, DEFAULTS, recorder, TEST_TOOL_TIMEOUT_MS); socket.receive({ type: 'session.start' }); await vi.waitFor(() => expect(provider.sessions).toHaveLength(1)); @@ -155,7 +178,7 @@ describe('RealtimeSession', () => { const provider = new FakeProvider(); const recorder = new FakeAnalyticsRecorder(); const { socket, asWs } = makeSocket(); - createRealtimeSession(asWs, provider, DEFAULTS, recorder); + createRealtimeSession(asWs, provider, DEFAULTS, recorder, TEST_TOOL_TIMEOUT_MS); socket.receive({ type: 'input_text.send', text: 'too early' }); await vi.waitFor(() => @@ -173,7 +196,7 @@ describe('RealtimeSession', () => { const provider = new FakeProvider(); const recorder = new FakeAnalyticsRecorder(); const { socket, asWs } = makeSocket(); - createRealtimeSession(asWs, provider, DEFAULTS, recorder); + createRealtimeSession(asWs, provider, DEFAULTS, recorder, TEST_TOOL_TIMEOUT_MS); socket.receive({ type: 'session.start' }); await vi.waitFor(() => expect(provider.sessions).toHaveLength(1)); @@ -191,7 +214,7 @@ describe('RealtimeSession', () => { const provider = new FakeProvider(); const recorder = new FakeAnalyticsRecorder(); const { socket, asWs } = makeSocket(); - createRealtimeSession(asWs, provider, DEFAULTS, recorder); + createRealtimeSession(asWs, provider, DEFAULTS, recorder, TEST_TOOL_TIMEOUT_MS); socket.receive({ type: 'session.start' }); await vi.waitFor(() => expect(provider.sessions).toHaveLength(1)); @@ -245,7 +268,7 @@ describe('RealtimeSession', () => { const provider = new FakeProvider(); const recorder = new FakeAnalyticsRecorder(); const { socket, asWs } = makeSocket(); - createRealtimeSession(asWs, provider, DEFAULTS, recorder); + createRealtimeSession(asWs, provider, DEFAULTS, recorder, TEST_TOOL_TIMEOUT_MS); socket.receive({ type: 'session.start' }); await vi.waitFor(() => expect(provider.sessions).toHaveLength(1)); @@ -262,7 +285,7 @@ describe('RealtimeSession', () => { const provider = new FakeProvider(); const recorder = new FakeAnalyticsRecorder(); const { socket, asWs } = makeSocket(); - createRealtimeSession(asWs, provider, DEFAULTS, recorder); + createRealtimeSession(asWs, provider, DEFAULTS, recorder, TEST_TOOL_TIMEOUT_MS); socket.receive({ type: 'session.start' }); await vi.waitFor(() => expect(provider.sessions).toHaveLength(1)); @@ -279,7 +302,7 @@ describe('RealtimeSession', () => { const provider = new FakeProvider({ deferred: true }); const recorder = new FakeAnalyticsRecorder(); const { socket, asWs } = makeSocket(); - createRealtimeSession(asWs, provider, DEFAULTS, recorder); + createRealtimeSession(asWs, provider, DEFAULTS, recorder, TEST_TOOL_TIMEOUT_MS); socket.receive({ type: 'session.start' }); socket.emit('close'); // disconnect while the provider handshake is still pending @@ -324,11 +347,11 @@ const CLIENT_TOOL: ToolDefinition = { location: 'client' }; -async function startActiveSession(defaults: typeof DEFAULTS) { +async function startActiveSession(defaults: typeof DEFAULTS, timeoutMs = TEST_TOOL_TIMEOUT_MS) { const provider = new FakeProvider(); const recorder = new FakeAnalyticsRecorder(); const { socket, asWs } = makeSocket(); - createRealtimeSession(asWs, provider, defaults, recorder); + createRealtimeSession(asWs, provider, defaults, recorder, timeoutMs); socket.receive({ type: 'session.start' }); await vi.waitFor(() => expect(provider.sessions).toHaveLength(1)); return { provider, session: provider.sessions[0], socket, recorder }; @@ -448,7 +471,7 @@ describe('RealtimeSession tool calls', () => { expect(session.sent).toContainEqual({ type: 'response.trigger' }); }); - it('resolves an unanswered client tool call with an error after the timeout, without hanging', async () => { + it('resolves an unanswered client tool call with an error after the configured timeout, without hanging', async () => { const toolRegistry = createToolRegistry([CLIENT_TOOL], {}); const { session, socket } = await startActiveSession({ ...DEFAULTS, toolRegistry }); @@ -466,7 +489,7 @@ describe('RealtimeSession tool calls', () => { arguments: '{}' }); - await vi.advanceTimersByTimeAsync(10_000); + await vi.advanceTimersByTimeAsync(TEST_TOOL_TIMEOUT_MS); expect(session.sent).toContainEqual({ type: 'tool.result', @@ -476,6 +499,33 @@ describe('RealtimeSession tool calls', () => { expect(session.sent).toContainEqual({ type: 'response.trigger' }); }); + it('honors a client tool timeout other than the default', async () => { + const toolRegistry = createToolRegistry([CLIENT_TOOL], {}); + const shortTimeoutMs = 5_000; + const { session } = await startActiveSession({ ...DEFAULTS, toolRegistry }, shortTimeoutMs); + + vi.useFakeTimers(); + session.emitToOrchestrator({ + type: 'tool.call', + callId: 'call-1', + name: 'client_tool', + arguments: '{}' + }); + + // Not yet — the configured (shorter) timeout hasn't elapsed. + await vi.advanceTimersByTimeAsync(shortTimeoutMs - 1); + expect(session.sent).not.toContainEqual( + expect.objectContaining({ type: 'tool.result', callId: 'call-1' }) + ); + + await vi.advanceTimersByTimeAsync(1); + expect(session.sent).toContainEqual({ + type: 'tool.result', + callId: 'call-1', + output: JSON.stringify({ error: 'Client tool call timed out.' }) + }); + }); + it('resolves immediately with an error for an unknown tool name', async () => { const { session } = await startActiveSession(DEFAULTS); @@ -515,6 +565,6 @@ describe('RealtimeSession tool calls', () => { // now-null providerSession — sendToProvider's optional-chaining guard // (rather than a throw) is what's being verified here: awaiting this // without the test failing means the timer callback didn't throw. - await vi.advanceTimersByTimeAsync(10_000); + await vi.advanceTimersByTimeAsync(TEST_TOOL_TIMEOUT_MS); }); }); diff --git a/src/lib/server/core/session.ts b/src/lib/server/core/session.ts index 9230898..3db6ce5 100644 --- a/src/lib/server/core/session.ts +++ b/src/lib/server/core/session.ts @@ -8,13 +8,12 @@ import type { ToolRegistry } from './tools.js'; export type RealtimeSessionState = 'idle' | 'connecting' | 'active' | 'closing' | 'closed'; -/** How long a client-executed tool call may go unanswered before we give up on it. */ -const CLIENT_TOOL_TIMEOUT_MS = 10_000; - /** Server-owned policy for a realtime session. Never client-controlled. */ export interface SessionDefaults { model: string; instructions: string; + /** Used only when the browser's `session.start` doesn't specify a voice. */ + defaultVoice?: string; toolRegistry: ToolRegistry; } @@ -31,7 +30,7 @@ export function resolveSessionConfig( return { model: defaults.model, instructions: defaults.instructions, - voice: clientConfig?.voice, + voice: clientConfig?.voice ?? defaults.defaultVoice, tools: defaults.toolRegistry.definitions }; } @@ -63,7 +62,9 @@ export class RealtimeSession { private readonly browserSocket: WebSocket, private readonly provider: RealtimeProvider, private readonly defaults: SessionDefaults, - private readonly recorder: AnalyticsRecorder + private readonly recorder: AnalyticsRecorder, + /** How long a client-executed tool call may go unanswered before we give up on it. */ + private readonly clientToolTimeoutMs: number ) { this.browserSocket.on('message', (data, isBinary) => { void this.handleBrowserMessage(data, isBinary); @@ -257,7 +258,7 @@ export class RealtimeSession { const timeout = setTimeout(() => { this.pendingClientToolCalls.delete(event.callId); this.resolveToolCall(event.callId, formatToolError('Client tool call timed out.')); - }, CLIENT_TOOL_TIMEOUT_MS); + }, this.clientToolTimeoutMs); this.pendingClientToolCalls.set(event.callId, timeout); return; } @@ -360,7 +361,8 @@ export function createRealtimeSession( browserSocket: WebSocket, provider: RealtimeProvider, defaults: SessionDefaults, - recorder: AnalyticsRecorder + recorder: AnalyticsRecorder, + clientToolTimeoutMs: number ): RealtimeSession { - return new RealtimeSession(browserSocket, provider, defaults, recorder); + return new RealtimeSession(browserSocket, provider, defaults, recorder, clientToolTimeoutMs); } diff --git a/src/lib/server/core/ws-handler.ts b/src/lib/server/core/ws-handler.ts index ecec483..1f5c80c 100644 --- a/src/lib/server/core/ws-handler.ts +++ b/src/lib/server/core/ws-handler.ts @@ -11,6 +11,7 @@ export interface RealtimeUpgradeDeps { provider: RealtimeProvider; defaults: SessionDefaults; recorder: AnalyticsRecorder; + clientToolTimeoutMs: number; } // `noServer: true` + `handleUpgrade` below is what lets this share an @@ -44,6 +45,12 @@ export function handleRealtimeUpgrade( } wss.handleUpgrade(req, socket, head, (browserSocket) => { - createRealtimeSession(browserSocket, deps.provider, deps.defaults, deps.recorder); + createRealtimeSession( + browserSocket, + deps.provider, + deps.defaults, + deps.recorder, + deps.clientToolTimeoutMs + ); }); } diff --git a/src/routes/api/realtime/session/+server.ts b/src/routes/api/realtime/session/+server.ts index 66e87ab..2da36c7 100644 --- a/src/routes/api/realtime/session/+server.ts +++ b/src/routes/api/realtime/session/+server.ts @@ -1,8 +1,9 @@ import { json } from '@sveltejs/kit'; +import { loadConfig } from '$lib/server/core/config.js'; import { mintSessionToken } from '$lib/server/core/tokens.js'; import type { RequestHandler } from './$types'; export const POST: RequestHandler = () => { - const { token, expiresAt } = mintSessionToken(); + const { token, expiresAt } = mintSessionToken(loadConfig().session.tokenTtlMs); return json({ token, expiresAt }); }; diff --git a/vite.config.ts b/vite.config.ts index 18af2d5..41adc1a 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -2,14 +2,9 @@ import { defineConfig } from 'vitest/config'; import { playwright } from '@vitest/browser-playwright'; import adapter from '@sveltejs/adapter-node'; import { sveltekit } from '@sveltejs/kit/vite'; -import { DEFAULT_SESSION_DEFAULTS } from './src/lib/server/core/defaults.js'; import { realtimeDevWsPlugin } from './src/lib/server/core/dev-plugin.js'; import { toolRegistry } from './src/lib/server/project/tools/index.js'; - -// Composition root: merges core defaults with the project's tool registry. -// server/core never imports project code directly (see docs/TOOLS.md) — this -// is one of the two places (alongside server.js) that's allowed to. -const realtimeDefaults = { ...DEFAULT_SESSION_DEFAULTS, toolRegistry }; +import { SESSION_PERSONA } from './src/lib/project/persona.js'; export default defineConfig({ plugins: [ @@ -27,7 +22,12 @@ export default defineConfig({ }), // Attaches the same WS upgrade handling used in production (server.js) // to Vite's dev server, so `npm run dev` supports it without a build. - realtimeDevWsPlugin({ defaults: realtimeDefaults }) + // Passes only raw, env-independent pieces (persona + tool registry) — + // NOT a fully-resolved `defaults` object — because this plugin's config + // gets evaluated under `npm run test:unit` too (see the VITEST guard in + // dev-plugin.ts's configureServer); resolving env-driven config here at + // module scope would run it there as well, with no env vars available. + realtimeDevWsPlugin({ persona: SESSION_PERSONA, toolRegistry }) ], test: { expect: { requireAssertions: true },