feat(voice): audio-native voice — the model hears you and answers in the same breath - #2399
Conversation
Ports the proven core from the pagespace-voice prototype into apps/web/src/lib/ai/realtime/ as three pure modules — no I/O, no clock, no randomness, no module-level mutable state, no React: - events.ts: function calls out of `response.done`, transcripts under BOTH `response.output_audio_transcript.done` and the older `response.audio_transcript.done` (renamed across releases, deployments straddle it), the `function_call_output` client event, and a `parseEvent` that returns undefined on a malformed frame rather than throwing. - session.ts: the `client_secrets` body shape, the two endpoint URLs, the `oai-events` channel name, and defensive `value` narrowing on the mint response. Default model is `gpt-realtime` — access is per-account, `client_secrets` accepts any string, and `/v1/realtime/calls` 403s `model_not_found` at connect time — with OPENAI_REALTIME_MODEL as the upgrade path, resolved purely from a passed-in env bag. - session-state.ts: the session reducer. A failure keeps the transcript; a drop mid-conversation must not erase the record. Tool definitions are deliberately not ported: PageSpace has its own tool registry, so only the transport-shaped `RealtimeTool` type lives here. 68 tests, 100% statements/branches/functions/lines on all three files, mutation-checked (34 mutations, each verified to turn the suite red). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VuzQWypmJU8b8pKvqt3BgW
feat(voice): port the pure realtime core
Realtime carries tools as flat {type,name,description,parameters} with plain
JSON Schema; PageSpace holds them as AI SDK tool() objects. buildRealtimeTools
bridges the two by reusing the text stack's own machinery rather than curating
a voice-specific list: splitToolsForExposure picks the upfront half, and
tool_search/execute_tool come from the same factories the Global Assistant
uses, so voice and text cannot describe the same tool differently or drift as
tools are added.
Schema conversion is the same z.toJSONSchema call tool_search already makes.
$schema is dropped — it describes the document, not the parameter contract, and
strict schema validation rejects unsupported top-level keywords.
No always-upfront set: that rescue exists for composer-toggled tools facing
execute_tool's allowlist re-check on the text routes, and a voice call has no
composer toggles.
toRealtimeTool is exported so the conversion guard can run over every tool in
the real registry — buildRealtimeTools converts only the upfront half, so
testing through it alone would pass vacuously for the deferred majority
(mutation-checked: a z.date() on a deferred tool now goes red and names it).
Pure: the registry is a parameter, never a module-load import.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011yZkd6fkVQ4bJVoUAqCHH4
feat(voice): adapt the tool registry to realtime function definitions (W1-A2)
…constants Voice bills deterministically today by audio-second (whisper) and input-character (tts). The audio-native realtime path bills TOKENS, per modality, in a shape different enough to need its own table. Additive only: the whisper/tts rates and behaviour are untouched, since on-demand TTS (Read Aloud) is not being retired. voice-pricing.ts - REALTIME_RATES: published gpt-realtime rates as USD per token, every rate env-overridable via the file's existing envFloat, defaults pinned by tests. - calculateRealtimeCostDollars(model, usage): pre-markup provider cost for one response.done frame; the credit pipeline applies MARKUP_BPS as it does for every other call. A call's total is the sum across its responses, since usage is reported per RESPONSE, not per call. Three wire asymmetries are modelled rather than smoothed over: - Caching is input-only, so the output rate type has no cached member to fill in wrongly, and no image member either. - cached_tokens is a SUBSET of input_tokens: the cached slice bills at the cache rate and only the REMAINDER at the full input rate. Each modality's cached count is clamped to its own gross count, so a malformed payload cannot drive the fresh remainder negative and subtract money from the bill. - Modality is the cost driver (one short spoken sentence measured 30 text + 86 audio output tokens, audio being ~2.7x the text rate), so each side is priced per modality rather than blended. Unknown model, absent usage, or missing/negative/NaN quantities bill 0 — never a negative or NaN charge. A usage object carrying only totals also bills 0: an unattributed token cannot be priced (audio input is 8x text input) and guessing a modality would over-charge, so the metering layer is left to notice that shape. credit-pricing.ts — realtime session constants beside the existing voice ones: - REALTIME_SESSION_HOLD_ESTIMATE_CENTS (10c). Realtime settles CONTINUOUSLY as usage arrives per response, so the hold need only cover the window between settles rather than the whole call. - REALTIME_MAX_SESSION_SECONDS (600). Derived, not picked: it must stay inside CREDIT_HOLD_TTL_SECONDS (900) or the reconcile cron reclaims a live call's own reservation mid-session. A test asserts a >=300s margin. - REALTIME_IDLE_TIMEOUT_SECONDS (120). Reaps the abandoned call, which otherwise holds a slot and streams ambient audio into a per-token-billed model. - REALTIME_MAX_INFLIGHT (2 per user). Voice is physically exclusive, so 1 is the semantically correct cap; 2 keeps a not-yet-reaped zombie session from locking a user out of reconnecting. - REALTIME_MAX_GLOBAL_SESSIONS (8). The per-user cap structurally cannot enforce the binding constraint: the OpenAI account is limited to 40,000 tokens/MINUTE across every session on the key, which a talking session approaches at roughly 4k tokens/min — putting the ceiling near 10. Pure: no route wiring, that is the metering leaf's job. Verified: voice-pricing.ts at 100% statement/branch/function/line coverage, with 7 mutations (double-count guard, cached clamp, a published rate, the negative quantity guard, the pro-rata share cap, and both session constants) each confirmed to turn the suite red. Root typecheck 17/17 and lint 15/15. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCyj6kA4A7bwmmpwDhHrer
feat(voice): price gpt-realtime token usage and add realtime session constants
The 2.1 default was reverted to gpt-realtime because /v1/realtime/calls returned 403 model_not_found for it. That was a genuine entitlement gap, not a bad model id: 2.1 has since been added to the project allowlist, GET /v1/models now lists gpt-realtime-2.1 and -mini, and the WebRTC calls endpoint returns 201 for it (verified with a real browser SDP offer). Two durable lessons kept in the comment, because both cost real debugging time: mint returns 200 for a model the calls endpoint will reject, so model-config errors must be read from the calls response; and a plain wss://...?model=X socket opens even for an unentitled model, so it cannot be used as an entitlement probe. OPENAI_REALTIME_MODEL stays the per-environment override — a deployment whose project lacks 2.1 still needs gpt-realtime.
fix(voice): pin gpt-realtime-2.1 now that the project allowlists it
…d hold it The server-side call plane, end to end: the browser POSTs an SDP offer to our route and gets an answer back, while our server holds the call id and that call's ephemeral secret and hands both to apps/realtime over an HMAC-signed internal request. apps/realtime attaches a WebSocket to the same session, lands session.update with the real tool set, and registers the socket for its whole life. PART 1 — apps/web - POST /api/voice/realtime/call: auth + tier gate copied from the transcribe route, mint, SDP relay, Location parse, signed handoff, answer SDP back. - The model is resolved with resolveRealtimeModel(process.env) and passed explicitly, never left to buildSessionConfig's default. - The mint carries NO tools: tools arrive with the process that executes them, so a call with no server attached cannot advertise tools nobody will answer. - Upstream failures surface as 502 carrying the upstream status and body — /v1/realtime/calls is the only place a bad OPENAI_REALTIME_MODEL is visible, since the mint 200s for a model the project cannot use. - INTERNAL_REALTIME_URL unset degrades to a working audio call (attached:false) rather than failing; a missing/malformed Location fails loudly instead. - The ephemeral secret is never returned, never logged, never persisted. PART 2 — apps/realtime - POST /api/realtime/attach, beside /api/broadcast, HMAC-verified before the body is parsed — it carries a live OpenAI credential. - Readiness is socket OPEN then session.updated, never session.created, which an attached socket never sends. - Named failures for every outcome, including the API-key-instead-of-ephemeral -secret case, refused before a socket is opened. - Registry keyed by callId following socket-registry.ts, with a synchronously reserved slot so the concurrency cap holds across the attach await. - Teardown on close, error, explicit end, and a hard duration cap. Shared contract in packages/lib/src/realtime/voice-bridge-contract.ts, parsed by both sides. Tools are converted web-side (the registry lives there and apps/realtime has no dependency edge to apps/web) and travel in the signed payload, so no tool schema reaches the browser. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012pYq7haTAw3E2zVKDp3t9W
feat(voice): get our server onto the realtime call, authenticated, and hold it
C-A got our server onto the realtime socket and holds it open. This is everything that happens on it: the session is seeded from the bound conversation, tool calls are dispatched against PageSpace's real registry with real permissions, both sides of the exchange are written into that conversation as ordinary messages, and every response's usage is metered. One chunk, because it is one event loop. A `response.done` carries a tool call AND the usage for the turn that produced it; the same silence that means "no transcript to write" means "the idle timer should be running". Split apart, each would need its own copy of the frame plumbing and its own theory of when the call ended. PART 1 — the seed (apps/web/src/lib/ai/realtime/seed.ts, pure) - `buildRealtimeSeed(messages, opts?)` -> ordered `conversation.item.create` events. Hard-capped at 20 turns / ~4k estimated tokens, both configurable, most-recent kept, oldest dropped first. - The cap IS the feature. Realtime cannot take assistant audio as history, so prior turns can only be injected as text, and injecting a large text history into a session with audio output is a documented way to get no audio at all. - The content-part type is NOT symmetric and fails silently if you get it wrong: `input_text` for a user item, `text` for an assistant item. Verified against the conversations guide and openai/openai-realtime-api-beta#57. - A single most-recent turn larger than the whole budget is TRUNCATED, not dropped: returning an empty seed for a thread that plainly has context is the one outcome nobody wants. The cap still holds absolutely. - Token estimate is a documented chars/4 heuristic. No tokenizer dependency. - Loaded web-side (`seed-loader.ts`) behind `canAccessConversation` and shipped in the attach payload, so it is in the realtime server's hand before the model speaks rather than a round trip later. Every unhappy path yields an empty seed: a seed is never a reason to fail a call. PART 2 — tool dispatch (apps/web/src/lib/ai/realtime/tool-dispatch.ts) - `function_call.arguments` is a newline-laden STRING and is not guaranteed to parse. Parsed defensively, with brace-counting recovery for the common streamed-delta corruption (trailing junk after a complete object) that ignores braces inside strings. - Runs against `buildRealtimeToolSet` — extracted from `buildRealtimeTools` so what the session ADVERTISES and what the dispatcher RUNS are one expression evaluated twice, in two processes. `tool_search`/`execute_tool` work unchanged. - Builds a real `ToolExecutionContext` carrying the acting `userId`, `conversationId`, `timezone` and `locationContext`. Permissions are NOT re-implemented: every PageSpace tool enforces access internally against that userId, and a second layer here would be a second thing to keep in step. - Always returns a STRING, on every path including failure — the model is blocked until `function_call_output` arrives. - Results are truncated generically (word boundary + how much was omitted + how to get the rest). The prototype's per-tool presenters do not scale to a registry of dozens, and a curated list would be a second tool surface. PART 3 — transcript persistence - Written through `messageRepository`, so the write, the in-transaction `conversations.rev` bump and the `conversation:*` emit are the SAME ones the typed path uses. The sidebar, GlobalAssistantView and page-agent chat update live with ZERO new client wiring — because this is not a second writer. - Global -> `saveGlobalMessage`; page -> `savePageMessage` with the agent page, assistant rows attributed `userId: null` + `sourceAgentId` per the table's attribution rule; drive/client threads refused rather than guessed at. - Lazy first-message creation via `resolveOrCreateConversation`, as typing does. - New nullable `messages.source` column (migration 0256) marks voice-authored rows, so the UI can show a mic glyph and the seed can tell spoken turns from typed ones. Set on INSERT only — a later terminal write does not change how a message was made. PART 4 — metering (apps/realtime, no HTTP hop) - Usage is per RESPONSE, so `usage-accumulator.ts` folds it per modality; summing only the totals would produce a tidy number that prices at zero, because `calculateRealtimeCostDollars` prices from the detail blocks. Cached tokens stay a subset, never an addition. - Continuous settlement: hold -> talk -> settle that window exactly -> re-hold. `trackUsage` takes ownership of the hold, so a long call that did not re-hold would run unreserved; a refused re-hold ends the call mid-conversation rather than at hangup. - Idle timeout, duration cap (bounded by CREDIT_HOLD_TTL_SECONDS), per-user and per-deployment concurrency caps. `MAX_CALL_DURATION_MS` stays what its doc says: a socket-leak backstop an order of magnitude looser, not a duplicate. - `gpt-realtime-2.1` had NO pricing row, so the model the app pins would have billed $0. Added (rates verified identical to `gpt-realtime`) and the stale "not served over WebRTC" comment corrected. THE PROCESS BOUNDARY, decided deliberately `apps/realtime` now calls BACK into web at `/api/internal/voice/bridge`, signed with the same symmetric HMAC. Tools and transcripts cross because their owners — the tool registry behind `@/` aliases, and `messageRepository` with its rev invariant — genuinely cannot move. Metering deliberately does NOT cross: its whole dependency set already lives in `@pagespace/lib`, which this app depends on directly, so an HTTP hop would add a failure mode and buy nothing. Same rule C-A applied to tools: the work runs where its owner lives. This makes false a statement in docs/2.0-architecture/agent-sessions.md §3d ("realtime makes no outbound HTTP calls to web at all"), so that section is amended in this change. The deploy ORDER is unchanged — every bridge hop is best-effort, so new realtime against old web degrades to audio without tools or transcripts rather than dropping calls. `events.ts` moved to `packages/lib/src/realtime/voice-events.ts` and gained `extractUsage`/`extractRateLimits`: the process that reads this event stream is `apps/realtime`, which cannot import from the web app. Gate: monorepo typecheck (16/16 non-web packages clean under --force; web clean under direct `tsc --noEmit` — turbo's web#typecheck races its own build and prints TS6053 for `.next/types`), lint 15/15, realtime 1093 tests, lib 9244, web 16868. Remaining failures are DB-only suites ("DATABASE_URL must point at a migrated Postgres") in a worktree with no test database. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015sdoa1UNkm5vqMowkytc39
feat(voice): what the server does while it is on the call
# Conflicts: # packages/db/drizzle/meta/0256_snapshot.json # packages/db/drizzle/meta/_journal.json
…yncing master master added 0256_parched_bloodscream and 0257_absurd_grim_reaper while this epic branch sat at 0255, so the generated 0256_violet_blur collided on both the journal and the filename. Took master's journal, dropped the stale 0256, and regenerated: the migration is byte-identical in effect (one nullable additive column on messages) and now lands as 0258.
The server side of the audio-native call has been merged and working since #2393/#2395 — mint, relay, HMAC handoff, seeding, tool dispatch, transcript persistence, metering — and nothing had ever driven it from a browser. This is that half: the page opens the peer connection, adds the mic track, opens the `oai-events` data channel, POSTs its SDP offer to our relay route, and applies the answer. It never contacts api.openai.com, never holds an ephemeral secret, and is never sent a tool schema. VoiceSessionProvider is mounted in Layout, ABOVE RightPanel. That placement is load-bearing, not incidental: `rightPanelVisible && …` unmounts the right sidebar outright when it closes, so a session owned by the panel would hang up every time somebody collapsed it. The sidebar is voice's home, not its owner — and the test starts a call from inside the panel and then unmounts the panel, so the property is asserted rather than merely arranged. Every decision is a pure module the provider only wires together: `voice-target` (navigating is not rebinding — walking to another page moves locationContext, choosing another agent moves the conversation), `chain-schedule` (when to hand off), and the already-merged `sessionReducer` (what the UI shows). CHAINING. A call has a server-enforced ceiling; a conversation does not. Before the cap lands, the client mints a fresh call on the SAME conversationId — which the server reseeds from that thread, because the transcript is the durable layer — and swaps make-before-break, so there is no moment with no session. The microphone is handed over as an independent track clone: no second permission prompt, no blink in the recording indicator, and stopping the outgoing call cannot take the incoming call's audio with it. The replacement negotiates muted, so two live sessions cannot both hear the same sentence. The ceiling had to come from the server. `REALTIME_MAX_SESSION_SECONDS` is per-deployment env the browser cannot read, so the call route now reports `maxDurationMs`. The alternative was a client-side copy of a server env var, whose failure mode is a user cut off mid-sentence on the one deployment that tuned it. `getMicPermissionErrorMessage` moved out of useVoiceMode into `lib/voice/mic-errors` rather than being ported: both paths ask the same API and hit the same five failures, and its desktop-Electron branch (System Settings, not "browser settings", which an Electron shell does not have) is not one to keep two copies of. Denied and missing stay different outcomes with different advice. Gate: monorepo `bun run typecheck` 17/17, `bun run lint` 15/15, 345 voice tests green. Teardown claims are mutation-checked, not asserted on faith. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PhbBndG131JyacZCqXrF5w
…vives
Review caught a real hole. `VoiceSessionContext.test.tsx` mounts its own
provider, so it proves a session survives a child unmounting — but replace
`<VoiceSessionProvider>` in Layout with a passthrough and all 25 of those tests
still pass, while every call in production would hang up the moment the user
closed the sidebar. The one load-bearing fact of this chunk was unguarded.
This asserts it against the REAL Layout tree. Everything heavy is mocked EXCEPT
the thing under test: the provider is the real one, mounted by the real Layout,
and `RightPanel` is replaced by a probe that CONSUMES the session — so a
provider that is missing, passthrough, or moved inside the panel fails at render
rather than subtly. The call is then started from inside that probe and the
sidebar gate is closed underneath it.
Verified by breaking Layout three ways and watching it go red, then restoring:
1. provider replaced with a passthrough (the exact review mutation) — 3 red
2. provider moved inside the `rightPanelVisible &&` region — 3 red
3. provider kept as an ancestor but re-keyed on `rightPanelVisible`, so it
remounts on toggle — 2 red, on `stop` having been called and the reopened
panel finding no call. That one isolates the SURVIVAL assertion, proving it
is not decorative: the hooks never throw, only the call dies.
No existing test was weakened to make this work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PhbBndG131JyacZCqXrF5w
feat(voice): the browser holds the mic and the speaker, and nothing else
The first user-visible piece of the audio-native path. Everything beneath it was merged and untested by a human; this is the part a person actually touches. THE SHAPE, as settled: - ONE trigger, in the nav bar, on every route. Nothing connects until it is pressed. It doubles as the live indicator and as the way back to a minimized call — and it NEVER hangs up, because a toggle there means pressing the live indicator hides the call you pressed it to see. Ending a call is the End button on the call. - It binds to whatever assistant is in view: the sidebar's selected agent on a page route, the GlobalAssistantView conversation on the dashboard (where the sidebar has no chat tab at all). There is no second target picker anywhere. - Voice is a MODE on the chat surface, not a fourth tab and not an overlay: the same message list, with a live call header above it. Spoken turns arrive in that list as ordinary messages — the realtime server writes them through messageRepository, so they ride the `conversation:*` events every surface already subscribes to. No new client wiring, and no second history. - Closing the sidebar minimizes; navigating updates locationContext and does NOT rebind; the sidebar's agent switcher DOES rebind. Every decision is a pure module beside the wiring — `resolveVoiceBinding`, `rebindAction`, `decideReveal`, `describeCall`, `toVoiceLocationContext` — so the rules are testable without a browser, a peer connection or a microphone. `rebindAction` is the load-bearing one. Navigating and switching agent arrive as the SAME change to the derived target, so a "watch the target and call start" effect cannot tell them apart and would hang up the user's call every time they opened a page. A rebind is therefore driven by the switch EVENT: the switcher records an intent, and the intent is applied once the newly chosen agent's conversation has resolved. Navigation records nothing. TWO THINGS THE MERGED SEAM WAS MISSING, added at the provider rather than worked around in the UI: - `failure` — the classified reason beside `error`. A sentence cannot be branched on, and a denied prompt and an absent capture device need opposite affordances: one offers Try again, the other must not, because a retry that cannot conjure hardware teaches the user that voice is broken. - `muted`/`setMuted` — mute has to live with the microphone. Held by the chrome it would return un-muted every time the sidebar was collapsed. It also fixes a latent bug: the swap hardcoded `setMicrophoneEnabled(true)`, so a chain at the duration ceiling un-muted a deliberately muted call. `messages.source` now reaches the UI (conversion AND broadcast, so the glyph does not appear only after a refresh) and both renderers mark a spoken turn. The browser reads its own copy of the value with a drift guard, since client components here cannot import @pagespace/db/schema. Also: the right sidebar's page-context tab moved from panel-local state into the layout store, because a button in the header cannot reach a `useState` inside a panel that is unmounted while collapsed. Unrelated to this chunk, but the branch's knip gate was already red for it: two per-member request types in voice-bridge-contract.ts (from #2395) were exported and never imported. Removed — consumers narrow on the union's `kind`. Gate: `bun run typecheck` 17/17, `bun run lint` 15/15, `bun run knip:check` within baseline, web tests 17085 passed with only the 17 known Postgres-requiring integration files failing (no test DB in this worktree). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PyxC1FeYap8bU98cLzcXhH
feat(voice): the talk button, and voice as a mode on the chat surface (C-D)
The audio-native path is merged end to end — server call plane (#2393), server behaviour (#2395), browser + lifecycle (#2396), UI (#2397) — so the old pipeline is dead weight on the conversational path. This removes exactly that much and nothing else. WHAT WENT, and why each piece could not stay: - `useVoiceMode` + `/api/voice/transcribe`: the loop itself. Whisper existed to turn audio into text before inference; the realtime session hears the audio directly, so there is nothing left for it to do. Verified with a repo-wide search that no other caller reaches the route. - `useVoiceModeStore` and everything reading it — `VoiceCallPanel`, `VoiceModeSettings`, `VoiceModeBorder`, and the mic button in the chat box's footer. The store's only writer was that button. Left in place, the border would be UI that can never render and the button an affordance that toggles a mode nothing implements. The way into voice is the nav-bar trigger. - `selectVoiceStreamText`, `selectVoiceActivationBaseline` and `selectPostBaselineAssistantMessage`: three pure selectors whose only job was deciding which written reply the old path should speak. Spoken turns now arrive as ordinary messages, so nothing derives a "what to say out loud" from the message list any more. - The Whisper rate in `voice-pricing`, and `VOICE_HOLD_ESTIMATE_CENTS` — the flat hold that existed because STT could not know its own cost until the provider answered. Both had exactly one caller, the deleted route. WHAT DELIBERATELY STAYED. `/api/voice/synthesize`, the tts-1/tts-1-hd rates, `estimateVoiceHoldCents`, `VOICE_MAX_INFLIGHT` and `chunkForTts` all back Read Aloud, which is an open PR (#2173) and a genuinely different feature: an audio-native conversation does not replace "read this to me". `mic-errors` stays because the realtime path is now its only consumer. `chunkForTts` is kept despite having no in-tree caller on this branch — its last one went with `VoiceCallPanel` — because `useReadAloud` imports `flushForTts` from it on #2173. Deleting it would break work in flight. knip does not report it, so it needed no ignore. The one knip.json line added is for `@radix-ui/react-slider`: deleting VoiceModeSettings left `components/ui/slider.tsx` as its only importer, and `src/components/ui/**` is already ignored. TESTS DELETED WITH THEIR SUBJECTS, never to make the gate pass: `useVoiceModeStore.test.ts`, `transcribe/route.test.ts`, and the three stream selector tests. `voice-pricing.test.ts` loses its Whisper describe block; the unknown-model and 1¢-floor assertions are kept, retargeted off `whisper-1`. `whisper-1` survives as a fixture in the admin billing-coverage tests, where it stands for historical usage rows that still exist in the database. Gate: monorepo `bun run typecheck` 17/17, `bun run lint` 15/15, knip ratchet green with an unchanged baseline. Unit suites pass (lib 9175, web 17057); the only red files are the DB-backed integration tests, which need a Postgres this worktree has no access to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYjZgYpebAVgBq9f5oVt81
feat(voice): retire the conversational STT->LLM->TTS loop
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughThis PR adds audio-native realtime voice calls with WebRTC setup, server-side tool execution, transcript persistence, continuous metering, persistent browser controls, conversation rebinding, and spoken-turn metadata. It removes the previous transcription flow and voice-mode UI. ChangesRealtime voice calling
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91767b13e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| userId: input.userId, | ||
| status: response.status, | ||
| }); | ||
| return false; |
There was a problem hiding this comment.
When the attach endpoint returns 402 for insufficient credits or 429 for a concurrency limit, this path reduces the policy refusal to attached: false; the call route then still returns the already-created SDP answer, so the browser can continue using an unmetered OpenAI call and bypass both credit and concurrency gates. Distinguish admission refusals from an optional-service outage, terminate the just-created OpenAI call, and return a handshake failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in ba27f78.
handOff returned a bare boolean, so "the metering plane is unreachable" and "policy says no" both became attached: false and the route returned the answer SDP either way. It now returns a three-way HandoffOutcome:
- 402 / 429 →
refused: the handshake fails with a newadmission_refusedcode, andabandonCall()POSTs/v1/realtime/calls/{id}/hangupwith that call's own ephemeral secret before returning — refusing to attach does not end a call OpenAI already accepted. The route answers the upstream status (402/429) rather than 502, since this is a policy outcome, not a malfunction. - everything else →
degraded: unchanged best-effort behaviour. A 400 from our own wire or a 502 from the attach declined nothing, so the caller keeps the degraded call.
Tests in call-handshake.test.ts (runCallHandshake — admission) cover both refusal statuses, that no answerSdp/callId comes back on a refusal, that a failing hangup still reports the refusal rather than throwing, and that 400/502 still degrade without a hangup.
| request: RealtimeToolDispatchRequest, | ||
| model: string, | ||
| ): ToolExecutionContext => ({ | ||
| userId: request.userId, |
There was a problem hiding this comment.
For a voice call bound to a page agent whose ACL is narrower than the invoking user's, this context omits chatSource.agentPageId after the client has already discarded that target field. Consequently resolveActingAgentId() returns undefined and every centralized canActor* check falls back to the user's broader permissions, allowing the agent to read or modify resources its own memberships deny. Resolve the agent from the authorized conversation and populate the actor context before dispatching tools.
AGENTS.md reference: AGENTS.md:L119-L124
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in ba27f78 — this was the sharpest of the four, and they turned out to be one bug: nothing about the bound assistant was resolved server-side at all.
loadVoiceBinding (formerly seed-loader.ts, now binding-loader.ts) resolves the agent from the already-authorized conversation — a type: 'page' conversation's contextId IS its agent page — behind the same single access check the seed uses, and asserts the page is AI_CHAT so a document is never treated as an agent. It rides the attach payload as assistant, is echoed on each tool hop, and buildVoiceToolContext now sets chatSource: { type: 'page', agentPageId, agentTitle }, exactly as page-chat-turn.ts does for the same conversation.
Nothing the browser or the model says selects it — a request body naming its own agentPageId changes nothing, and there is a route test for that. Echoing it back on the bridge hop adds no trust assumption: that hop already carries userId under the same HMAC, so forging this would require the ability to already claim to be any user.
Mutation-checked: dropping the field turns the buildVoiceToolContext cases red.
| // branches (the code-execution kill switch) that are the caller's | ||
| // decision. The realtime server cannot build these itself — the | ||
| // registry lives in this app — so they ride the signed internal hop. | ||
| tools: buildRealtimeTools(buildPageSpaceTools()), |
There was a problem hiding this comment.
When voice is started on a page agent with a restricted enabledTools configuration, this advertises tools from the entire deployment registry instead of the agent-filtered set, while buildVoiceToolContext leaves enabledTools undefined and execute_tool interprets that as unrestricted. The model can therefore invoke write, delete, or other tools that the agent owner explicitly disabled; load the server-authoritative agent configuration and filter both advertised and executable tools with its allowlist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in ba27f78, on both lists — filtering only the advertised one would not have been a filter.
buildRealtimeTools/buildRealtimeToolSet take the bound agent's allowlist and apply filterToolsForAgentAllowlist before splitToolsForExposure and before tool_search is handed its catalog — the same order page-chat-turn.ts uses. Filtering afterwards would leave a blocked tool discoverable through tool_search and callable through execute_tool, which is every tool the owner switched off.
The executable set is built in a different process on a different request (the bridge, per tool call), so voiceToolDispatchDeps(allowlist) takes it too — otherwise the model is offered the narrow list and permitted to run the wide one. enabledTools is also carried on the execution context, because execute_tool re-checks against it and reads undefined as unrestricted.
null (unconfigured) and [] (every tool off) are kept distinct end to end; there are tests for both, plus one asserting tool_search cannot describe a blocked tool. Mutation-checked.
| Authorization: `Bearer ${deps.apiKey}`, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify(buildSessionConfig({ model: deps.model })), |
There was a problem hiding this comment.
Every voice session is minted with only the model, so neither the selected page agent's systemPrompt nor the Global Assistant's PageSpace instructions ever reach OpenAI. The UI and changelog present the call as talking to the assistant currently in view, but the resulting call actually uses the realtime model's generic defaults regardless of which agent was selected; resolve and authorize the bound assistant server-side and include its instructions in the session config.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in ba27f78.
The mint carries no instructions by design (same argument as the tools: a call that never gets a server attached should not advertise capability to a model with nobody listening), so they now ride the same session.update the tools do, from apps/realtime.
buildVoiceInstructions (instructions.ts) composes two parts: spoken-turn guidance that applies to every call — a listener cannot skim, markdown read aloud is noise, expect to be interrupted — and then the bound assistant: a page agent's own systemPrompt verbatim, or its name plus the PageSpace persona when its owner configured none.
Deliberately not copied in: the text pipeline's page tree, agent memory, skill catalog and tool-discovery prompt. Most of it is turn-volatile and a realtime session's instructions are set once at attach, so a copy would be stale for the rest of the call; the tool guidance is written for a surface where a silent tool loop is acceptable. The module header states this. Where the caller is standing still reaches the tools live, via locationContext.
instructions is omitted rather than sent empty when there is none — an empty instructions would replace the session's. There is a test for that.
| if (reason !== 'call_ended') { | ||
| await hangUp(callId, secret); | ||
| } |
There was a problem hiding this comment.
When the realtime server's attached WebSocket closes because of a network error, process shutdown, or its one-hour backstop, onClosed reports call_ended and this condition skips the hangup request. Closing that attached socket does not end the browser's independent WebRTC call, so the user can continue talking while metering and transcript supervision have already stopped. Only skip hangup for an event that proves the underlying call itself ended; unexpected attached-socket closure must explicitly hang up the browser call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in e664a3b — you are right that call_ended does not prove the call ended.
I went further than "only skip for an event that proves it", because no close code actually proves it: onClosed reports call_ended for a network drop, a process shutdown and the one-hour socket backstop alike. So the hangup now runs on every reason.
The reason it was conditional was log noise on the common path — the user hangs up in the browser, OpenAI tears the call down, and our teardown asks anyway. That is handled where it belongs: hangUpCall now reads 404 as success ("already ended") and returns true, instead of warning. Asking twice is free; asking zero times bills nobody for a live call.
The old test asserting no hangup on call_ended is inverted, and call-hangup.ts — which had no test file at all — now has one covering the 404, genuine-refusal, transport-failure and non-Error paths.
| * the only lever this process has: once a call is attached, its token spend is | ||
| * the model's to decide. | ||
| */ | ||
| export const DEFAULT_MAX_CONCURRENT_CALLS = 8; |
There was a problem hiding this comment.
The new REALTIME_MAX_GLOBAL_SESSIONS setting is parsed and documented as the deployment-wide tuning knob, but the production registry still hardcodes its default to 8 and never imports that setting. Any operator changing the environment variable therefore sees no change to admission capacity, which can either continue overloading a lower-limit OpenAI account or unnecessarily cap a higher-limit one.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in e664a3b. DEFAULT_MAX_CONCURRENT_CALLS is now REALTIME_MAX_GLOBAL_SESSIONS rather than a literal 8, so the documented knob is the one that decides admission.
There is a test asserting the two are the same value, specifically so a future literal cannot quietly outrank the env var again.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
apps/web/src/lib/ai/realtime/__tests__/session.test.ts-54-58 (1)
54-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe model-override cases cannot fail.
DEFAULT_REALTIME_MODELis'gpt-realtime-2.1'(Line 32). Every override case passes that same value:
- Line 55:
buildSessionConfig({ model: 'gpt-realtime-2.1' })- Line 97:
resolveRealtimeModel({ [REALTIME_MODEL_ENV_VAR]: 'gpt-realtime-2.1' })- Line 103: the trimmed variant
- Line 124: the round-trip variant
If
buildSessionConfigdropped themodeloption, orresolveRealtimeModelignored the environment variable, all four cases would still pass. The voice case at Line 61 uses'cedar'against a default of'marin'and does discriminate. Use a distinct sentinel model string here for the same reason.💚 Proposed fix
it('given a model override, should use it instead of the default', () => { - expect(buildSessionConfig({ model: 'gpt-realtime-2.1' }).session.model).toBe( - 'gpt-realtime-2.1', - ); + expect(buildSessionConfig({ model: 'gpt-realtime-next' }).session.model).toBe( + 'gpt-realtime-next', + ); });it('given an override, should use it — the env var is the path to a newer model', () => { expect( - resolveRealtimeModel({ [REALTIME_MODEL_ENV_VAR]: 'gpt-realtime-2.1' }), - ).toBe('gpt-realtime-2.1'); + resolveRealtimeModel({ [REALTIME_MODEL_ENV_VAR]: 'gpt-realtime-next' }), + ).toBe('gpt-realtime-next'); }); it('given an override with surrounding whitespace, should trim it', () => { expect( - resolveRealtimeModel({ [REALTIME_MODEL_ENV_VAR]: ' gpt-realtime-2.1 ' }), - ).toBe('gpt-realtime-2.1'); + resolveRealtimeModel({ [REALTIME_MODEL_ENV_VAR]: ' gpt-realtime-next ' }), + ).toBe('gpt-realtime-next'); });it('given a resolved model, should be usable as the session model', () => { const model = resolveRealtimeModel({ - [REALTIME_MODEL_ENV_VAR]: 'gpt-realtime-2.1', + [REALTIME_MODEL_ENV_VAR]: 'gpt-realtime-next', }); - expect(buildSessionConfig({ model }).session.model).toBe('gpt-realtime-2.1'); + expect(buildSessionConfig({ model }).session.model).toBe('gpt-realtime-next'); });Also applies to: 95-127
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/session.test.ts` around lines 54 - 58, Update the model-override tests around buildSessionConfig and resolveRealtimeModel to use a sentinel model value different from DEFAULT_REALTIME_MODEL, including the trimmed and round-trip environment-variable cases. Preserve the existing assertions while ensuring each test would fail if the override were ignored.apps/web/src/lib/voice/mic-errors.ts-72-75 (1)
72-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winScope the desktop message to macOS, or make it OS-neutral.
isDesktopShell()only reports that the app runs in the Electron shell. It does not report the operating system. On Windows and Linux the path "System Settings > Privacy & Security > Microphone" does not exist, so the user is sent to a control that is not there.Either read the platform before you pick the sentence, or use wording that fits every desktop OS.
🔤 Proposed OS-neutral wording
case 'denied': return isDesktop - ? 'Microphone access was blocked. Allow PageSpace in System Settings > Privacy & Security > Microphone, then try again.' + ? 'Microphone access was blocked. Allow PageSpace to use the microphone in your operating system privacy settings, then try again.' : 'Microphone access was blocked. Please allow microphone permissions in your browser settings and try again.';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/voice/mic-errors.ts` around lines 72 - 75, Update the denied microphone message selection in the mic error mapping so Electron desktop users are not directed to macOS-specific settings on Windows or Linux. Use an OS-neutral desktop message, or additionally check the runtime platform before selecting the macOS-specific wording; preserve the browser message for non-desktop users.apps/web/src/lib/ai/realtime/__tests__/seed.test.ts-265-285 (1)
265-285: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the truncation marker.
truncateToTokensretains the message head withtext.slice(0, maxChars), so(earlier part of this message omitted)is incorrect. Change the marker to describe the omitted later part. Replace the exact-string assertion with a prefix assertion plus the marker.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/seed.test.ts` around lines 265 - 285, The truncation tests use an incorrect marker describing omitted earlier content. Update the expected marker in the `truncateToTokens`-related assertions to indicate that the later part of the message was omitted, and replace the exact-string assertion in the word-boundary test with a prefix assertion plus a check for the corrected marker.apps/web/src/app/api/internal/voice/bridge/route.ts-42-46 (1)
42-46: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCheck the declared length before buffering the body.
await request.text()buffers the whole body first. The cap runs after that, so an unauthenticated caller can still force the process to buffer an arbitrary payload. Also,rawBody.lengthcounts UTF-16 code units, not bytes, so a multibyte payload passes the check above 256 KiB.Read
content-lengthfirst and reject early, then keep the parsed-size check as a backstop.🛡️ Proposed fix
export async function POST(request: Request) { try { + const declared = Number(request.headers.get('content-length') ?? ''); + if (Number.isFinite(declared) && declared > MAX_BODY_BYTES) { + return NextResponse.json({ ok: false, error: 'Payload too large' }, { status: 413 }); + } const rawBody = await request.text(); - if (rawBody.length > MAX_BODY_BYTES) { + if (Buffer.byteLength(rawBody, 'utf8') > MAX_BODY_BYTES) { return NextResponse.json({ ok: false, error: 'Payload too large' }, { status: 413 }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/internal/voice/bridge/route.ts` around lines 42 - 46, Update the request-body handling in the bridge route to inspect the Content-Length header and return the existing 413 response before calling request.text() when the declared byte length exceeds MAX_BODY_BYTES. Retain a post-read size check as a backstop, measuring the buffered body in bytes rather than UTF-16 code units.apps/web/src/lib/ai/realtime/connect.ts-214-242 (1)
214-242: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
createMediaStreamcan throw on the reuse path.Line 230 calls
createMediaStream(clones)outside anytry. If the constructor throws, the already-cloned tracks are never stopped andconnectVoiceCallrejects instead of returning aVoiceConnectFailed. Every other failure in this module returns a structured result.🛡️ Proposed fix
- return { ok: true, stream: createMediaStream(clones) }; + try { + return { ok: true, stream: createMediaStream(clones) }; + } catch (error) { + for (const clone of clones) clone.stop(); + return failure('mic-unknown', describeThrown(error), getMicPermissionErrorMessage(error)); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/connect.ts` around lines 214 - 242, Update acquireMicrophone to cover the reused-microphone branch, including createMediaStream(clones), with the same structured failure handling used for getUserMedia. If stream creation throws, stop all cloned tracks and return a classified mic failure via failure rather than allowing the exception to escape; preserve the existing no-audio-track result.apps/web/src/contexts/VoiceSessionContext.tsx-149-204 (1)
149-204: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRegister the voice session with
useEditingStore.
VoiceSessionProviderowns live transcript streaming, and the realtime runtime persists transcript rows. UsestartStreamingduringconnectingandendStreamingon connection failure, connection loss, rebind,stop, and unmount. This defers auth refresh;ai-streamingdoes not pause SWR by design.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/contexts/VoiceSessionContext.tsx` around lines 149 - 204, Register the session lifecycle with useEditingStore in VoiceSessionProvider: obtain startStreaming and endStreaming, call startStreaming when a call enters connecting, and call endStreaming on connection failure, connection loss, rebind, stop, and provider unmount. Ensure each streaming session is ended exactly once, including superseded or failed attempts, while preserving the existing call lifecycle.Source: Coding guidelines
apps/web/src/lib/ai/realtime/tools.ts-49-53 (1)
49-53: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHandle non-Zod AI SDK schemas in the realtime path.
Tool['inputSchema']also acceptsjsonSchema()schemas, but these casts assumez.ZodType. A non-Zod tool can make session construction throw, or make dispatch call a missingsafeParsebefore itstryblock. Narrow the realtime tool contract to Zod schemas or use the AI SDK validation contract. Apply the same change tocreateToolSearchToolandcreateExecuteTool.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/tools.ts` around lines 49 - 53, The realtime tool contract currently assumes every Tool['inputSchema'] is a Zod schema, causing invalid JSON-schema tools to fail during parameter conversion or dispatch validation. Update toRealtimeParameters and the realtime tool factories createToolSearchTool and createExecuteTool to require Zod-compatible schemas or consistently use the AI SDK validation contract; also update the dispatch path around tool-dispatch.ts lines 266-269 so validation does not call a missing safeParse before error handling.apps/web/src/components/layout/right-sidebar/index.tsx-67-68 (1)
67-68: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
setLocalActiveTabto both hook dependency arrays.Both hooks capture
setLocalActiveTab, but neither dependency array includes it. Add it to satisfy the globally enabledreact-hooks/exhaustive-depsrule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/layout/right-sidebar/index.tsx` around lines 67 - 68, Add setLocalActiveTab to the dependency arrays of both hooks in the right-sidebar component, alongside their existing dependencies, while leaving the hook behavior unchanged.apps/web/src/components/ai/shared/chat/MessageRenderer.tsx-275-275 (1)
275-275: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe microphone glyph repeats on multi-block messages. Both renderers gate
createdAtandeditedAtonisLastTextBlock, but passspokento every text block. A message that groups into more than one text block then rendersSpokenTurnGlyphonce per block, while the timestamp renders once.
apps/web/src/components/ai/shared/chat/MessageRenderer.tsx#L275-L275: change tospoken={isLastTextBlock && isSpokenTurn(message)}.apps/web/src/components/ai/shared/chat/CompactMessageRenderer.tsx#L270-L270: apply the same gate onisLastTextBlock.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/ai/shared/chat/MessageRenderer.tsx` at line 275, The spoken prop is passed to every text block, causing duplicate microphone glyphs. In apps/web/src/components/ai/shared/chat/MessageRenderer.tsx:275 and apps/web/src/components/ai/shared/chat/CompactMessageRenderer.tsx:270, gate isSpokenTurn(message) with isLastTextBlock so the glyph renders only on the final text block.apps/web/src/lib/ai/realtime/seed.ts-127-133 (1)
127-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the truncation notice: the code drops the tail, not the head.
truncateToTokenskeepstext.slice(0, maxChars), so the removed text is the LATER part of the message. The appended sentence states the earlier part was omitted. The model reads this notice as context, so an inverted statement misdescribes what is missing.🐛 Proposed fix for the truncation notice
const truncateToTokens = (text: string, maxTokens: number): string => { const maxChars = maxTokens * SEED_CHARS_PER_TOKEN; const window = text.slice(0, maxChars); const lastSpace = window.lastIndexOf(' '); const head = (lastSpace > 0 ? window.slice(0, lastSpace) : window).trimEnd(); - return `${head}… (earlier part of this message omitted)`; + return `${head}… (rest of this message omitted)`; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/seed.ts` around lines 127 - 133, Update the truncation notice returned by truncateToTokens to state that the later part of the message was omitted, matching the existing text.slice(0, maxChars) behavior; leave the truncation logic unchanged.apps/web/src/lib/ai/realtime/chain-schedule.ts-22-27 (1)
22-27: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
CHAIN_LEAD_MSis smaller than the worst-case handshake budget.The comment states the lead covers a whole handshake whose upstream timeout is 15s. The handshake makes two sequential upstream fetches, each with
OPENAI_TIMEOUT_MS = 15_000(apps/web/src/lib/ai/realtime/call-handshake.ts, the mint at line 263 and the SDP relay at line 294), plus the handoff withHANDOFF_TIMEOUT_MS = 5_000at line 223. The worst case is therefore about 35s, not 20s.If both upstream hops stall near their ceilings, the replacement call is not ready before the server hangs up the current call, and the user is cut off mid-sentence — the exact outcome this module exists to prevent.
Raise the lead above the sum of the three timeouts, or start the chain from a budget derived from those constants.
🐛 Proposed fix for the chain lead time
/** - * Headroom before the cap. It covers a whole handshake — mint, relay to OpenAI, - * the internal attach — whose own upstream timeout is 15s, plus the moment of - * swapping which stream feeds the speaker. + * Headroom before the cap. It covers a whole handshake in the WORST case: two + * sequential upstream fetches at 15s each (mint, then the SDP relay) plus the + * 5s internal attach, and then the moment of swapping which stream feeds the + * speaker. A lead shorter than that sum lets a slow handshake land after the + * server has already hung the call up. */ -export const CHAIN_LEAD_MS = 20_000; +export const CHAIN_LEAD_MS = 40_000;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/chain-schedule.ts` around lines 22 - 27, Update CHAIN_LEAD_MS to exceed the combined worst-case budget of the two sequential OPENAI_TIMEOUT_MS operations and HANDOFF_TIMEOUT_MS, totaling approximately 35 seconds. Prefer deriving the value from those timeout constants if they are safely available; otherwise set a fixed lead with sufficient margin so replacement setup completes before the current call is terminated.apps/realtime/src/voice/call-metering.ts-196-207 (1)
196-207: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
durationreports total call time on every settle, not the window.Line 202 computes
nowFn() - startedAtfor eachtrack()call. A five-minute call at a one-minute settle interval emits five usage rows with durations of 60s, 120s, 180s, 240s and 300s. Any aggregation that sumsdurationover these rows reports 900s for a 300s call.Every other field in the payload is per-window:
inputTokens,outputTokensandproviderCostDollarsall come fromusage, which is reset on Line 177.Track the last settle time and report the window.
🐛 Proposed fix
const startedAt = nowFn(); let lastActivityAt = startedAt; + let windowStartedAt = startedAt;+ const settledAt = nowFn(); try { await track({ userId, provider: 'openai_voice', model, source: 'voice', providerCostDollars: costDollars, - duration: nowFn() - startedAt, + duration: settledAt - windowStartedAt, success: true,Set
windowStartedAt = settledAt;after thetry/catchcompletes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/voice/call-metering.ts` around lines 196 - 207, Update the settle tracking around the usage reset and track call to maintain a window start timestamp, and calculate duration from the current settle time minus that timestamp instead of from startedAt. After each try/catch completes, assign windowStartedAt to settledAt so subsequent rows report only the elapsed interval while preserving the existing first-window and final-settlement behavior.
🧹 Nitpick comments (20)
apps/realtime/src/voice/__tests__/call-metering.test.ts (1)
235-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe interval test does not prove the interval settles.
The test fires the settle interval and then awaits
started.meter.settle()explicitly. If the interval callback did nothing, the explicitsettle()alone would still produce exactly onetrackcall. The assertion passes in both cases.Remove the explicit
settle()and let the un-awaited interval work drain instead.♻️ Proposed change to make the assertion meaningful
started.meter.record(usage()); h.fireSettle(); - await started.meter.settle(); + // Let the interval's un-awaited settle resolve. + await new Promise<void>((resolve) => setTimeout(resolve, 0)); expect(h.track).toHaveBeenCalledTimes(1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/voice/__tests__/call-metering.test.ts` around lines 235 - 245, Update the interval test around startCallMeter and h.fireSettle to remove the explicit started.meter.settle() call; after firing the interval, await the existing asynchronous drain mechanism so the interval callback performs settlement before asserting h.track was called once.apps/web/src/lib/ai/realtime/__tests__/tool-dispatch.test.ts (1)
97-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe title states one outcome, the body asserts two.
The title says the fragment should be refused. Line 98 asserts
ok === truefor'oops {"a":1} but also {', which is acceptance, not refusal. Only Line 99 asserts refusal. Split the case in two, or rename it to cover both recovery and refusal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/tool-dispatch.test.ts` around lines 97 - 100, Update the test description around parseToolArguments to accurately reflect both asserted outcomes: recovery and acceptance for the embedded object fragment, and refusal for the unrecoverable array fragment. Either split these assertions into separate tests or rename the existing test to explicitly cover both behaviors.apps/web/src/lib/ai/realtime/__tests__/connect.test.ts (2)
63-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore the
console.errorspy after each test.This file installs a
console.errorspy inbeforeEachbut never restores it. The sibling fileapps/web/src/contexts/__tests__/VoiceSessionContext.test.tsxrestores mocks inafterEach. Match that pattern so the spy cannot outlive the file.♻️ Proposed change
-import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';beforeEach(() => { vi.spyOn(console, 'error').mockImplementation(() => {}); }); + +afterEach(() => { + vi.restoreAllMocks(); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/connect.test.ts` around lines 63 - 65, Update the test setup around the existing beforeEach in connect.test.ts to add an afterEach hook that restores the console.error spy, matching the cleanup pattern used by VoiceSessionContext tests.
427-438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe test title does not match the input.
The title states "given no cap reported". The body sends
maxDurationMs: 'soon', which is a cap of the wrong type, not an absent cap. Rename the case to describe an invalid cap. Add a separate case that omitsmaxDurationMsentirely, because the chaining logic inVoiceSessionContextbranches on that value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/connect.test.ts` around lines 427 - 438, Rename the existing connectVoiceCall test to describe an invalid maxDurationMs value, preserving its assertion that the result leaves the cap undefined. Add a separate test with maxDurationMs omitted from the response body, covering the no-cap path used by VoiceSessionContext and asserting its expected undefined-cap behavior.apps/web/src/lib/ai/realtime/__tests__/voice-bridge-contract-drift.test.ts (1)
28-40: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winBuild the registry with code execution enabled, so the guard covers every tool.
Lines 29 and 39 call
buildPageSpaceTools()with no options.apps/web/src/lib/ai/realtime/__tests__/tools.test.tscallsbuildPageSpaceTools({ codeExecutionEnabled: true }). If the flag adds tools to the registry, those tools are never checked againstrealtimeToolSchemahere, and a drifting schema on a code-execution tool ships unnoticed.The stated purpose of this file is to run against the real registry. Use the widest registry it can produce.
♻️ Proposed change
it('given the real registry, every emitted tool should satisfy the shared schema', () => { - const tools = buildRealtimeTools(buildPageSpaceTools()); + const tools = buildRealtimeTools(buildPageSpaceTools({ codeExecutionEnabled: true })); expect(tools.length).toBeGreaterThan(0);it('should carry the whole registry-built tool set through the attach payload intact', () => { - const tools = buildRealtimeTools(buildPageSpaceTools()); + const tools = buildRealtimeTools(buildPageSpaceTools({ codeExecutionEnabled: true }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/voice-bridge-contract-drift.test.ts` around lines 28 - 40, Update both buildPageSpaceTools calls in the registry contract tests to enable code execution, matching the configuration used by tools.test.ts. Keep the existing schema-validation and attach-payload assertions unchanged so they cover the widest registry, including code-execution tools.apps/web/src/contexts/__tests__/VoiceSessionContext.test.tsx (1)
210-236: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnmount the second render explicitly, or capture state before the next mount.
This test mounts two providers.
probeis module state, so the last mountedStateProbewins. The test relies on that ordering. The assertion at Line 232 reads the second provider only because the first one was unmounted at Line 224.The behavior is correct today. It breaks silently if a future edit reorders the renders. Consider splitting the case into two tests, or capture
probe.state.statusnext to each message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/contexts/__tests__/VoiceSessionContext.test.tsx` around lines 210 - 236, Make the test’s provider state assertions independent of render ordering by capturing the first scenario’s status and error before unmounting, then capturing the second scenario’s status and error after its call. Update the assertions to use those captured values, or split the denied and missing scenarios into separate tests; keep the distinct-message assertion intact.apps/web/src/lib/ai/realtime/__tests__/tools.test.ts (2)
49-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis assertion pins a registry defect and fails when the defect is fixed.
CORE_TOOL_NAMESlistsget_page_details, but no tool module defines it, sobuildRealtimeToolscan never emit it. The assertion requires the gap to stay exactly['get_page_details']. If someone implements the tool, or removes the stale name fromstub-tools.ts, this test goes red for a correct change.Assert that the gap is a subset of a documented allowlist instead, so a fix passes and a new unimplemented name still fails.
Do you want me to open an issue to track the missing
get_page_detailsimplementation?♻️ Proposed change
- expect([...CORE_TOOL_NAMES].filter((n) => !registryCoreNames.includes(n))).toEqual([ - 'get_page_details', - ]); + // Names listed in CORE_TOOL_NAMES with no tool module behind them. + const KNOWN_UNIMPLEMENTED_CORE_NAMES = new Set(['get_page_details']); + for (const name of CORE_TOOL_NAMES) { + if (registryCoreNames.includes(name)) continue; + expect( + KNOWN_UNIMPLEMENTED_CORE_NAMES.has(name), + `"${name}" is a core tool name with no tool behind it`, + ).toBe(true); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/tools.test.ts` around lines 49 - 57, Update the assertion in the realtime tools test around CORE_TOOL_NAMES and registryCoreNames to verify that the missing-name gap is contained within a documented allowlist, rather than equaling exactly ['get_page_details']. Preserve the failure for any unimplemented name outside that allowlist, while allowing get_page_details to pass whether it is implemented or removed from the stub names.
93-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not match Zod’s internal error text.
Zod documents that unrepresentable schemas throw by default, but it does not document the exact
Date cannot be represented in JSON Schemamessage. Use.toThrow()alone, or assert a repository-owned wrapper message that includesread_page.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/tools.test.ts` around lines 93 - 99, Update the toRealtimeTool test for the unrepresentable z.date() schema to avoid matching Zod’s internal error text; assert only that the call throws, or assert a repository-owned error message that includes the read_page tool name.apps/web/src/lib/ai/realtime/__tests__/transcript-persistence.test.ts (1)
118-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the voice source on the page path too.
Line 72 asserts
source: MESSAGE_SOURCE_VOICEfor the global path. The page path has no equivalent assertion, so a regression that drops the marker on page conversations would pass.♻️ Proposed addition
expect(savePageMessage).toHaveBeenCalledWith( - expect.objectContaining({ pageId: 'page-1', role: 'user', userId: 'u1', sourceAgentId: null }), + expect.objectContaining({ + pageId: 'page-1', + role: 'user', + userId: 'u1', + sourceAgentId: null, + source: MESSAGE_SOURCE_VOICE, + }), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/transcript-persistence.test.ts` around lines 118 - 144, Extend the page-thread assertion in persistVoiceTranscript to verify the saved message includes source: MESSAGE_SOURCE_VOICE, alongside the existing pageId, role, userId, and sourceAgentId checks. Keep the assertion focused on the savePageMessage payload.apps/web/src/contexts/VoiceSessionContext.tsx (1)
198-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRef writes during render are unsafe under concurrent rendering.
Lines 199 and 359 assign to refs in the render body. React can discard a render, and the mutation still happens. Move both assignments into a layout effect, or read
depsandbeginAttemptthrough a smalluseEffectEvent-style wrapper.This is a hardening change, not a current defect: both values are overwritten on every render, so a discarded render is corrected by the committed one.
Also applies to: 359-359
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/contexts/VoiceSessionContext.tsx` around lines 198 - 203, Move the render-time assignments to depsRef.current and beginAttemptRef.current in VoiceSessionContext into a layout effect, or access them through a useEffectEvent-style wrapper. Ensure both refs update only after the render commits while preserving their latest committed deps and beginAttempt values.apps/web/src/components/ai/voice/realtime/VoiceNavTrigger.tsx (1)
141-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the
pressRefassignment into an effect.
pressRef.currentis written during render. Assign it fromuseEffectinstead.♻️ Proposed refactor
- pressRef.current = () => void handlePress(); + useEffect(() => { + pressRef.current = () => void handlePress(); + }, [handlePress]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/ai/voice/realtime/VoiceNavTrigger.tsx` at line 141, Move the pressRef.current assignment out of render and into a useEffect in the VoiceNavTrigger component, keeping it synchronized with handlePress and preserving the existing invocation behavior.apps/web/src/lib/ai/realtime/__tests__/bridge-handler.test.ts (1)
115-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the nested
experimental_contextvalues.
dispatchRealtimeToolCallpasses the context inexecute's second argument underexperimental_context. AssertconversationId,timezone, andlocationContextthere instead of checking only that the tool ran.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/bridge-handler.test.ts` around lines 115 - 133, Update the “should forward the call context to the dispatcher” test around handleVoiceBridgeRequest and execute to inspect execute’s second argument under experimental_context. Assert that conversationId, timezone, and locationContext match the values supplied by toolBody, replacing the broad toHaveBeenCalled assertion while preserving the successful response check.apps/web/src/components/ai/shared/chat/message-types.ts (1)
26-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow
ConversationMessage.sourceand fix the symbol reference.Current message writes use only
'voice'as a non-null source; other writes persistnull. DefineMessageSource = typeof VOICE_MESSAGE_SOURCE, usesource?: MessageSource | null, and referenceVOICE_MESSAGE_SOURCEfrom@/lib/ai/realtime/message-source. Update the voice-test fixture parameter to the narrowed type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/ai/shared/chat/message-types.ts` around lines 26 - 38, Update ConversationMessage.source to use a MessageSource alias defined as typeof VOICE_MESSAGE_SOURCE, importing VOICE_MESSAGE_SOURCE from `@/lib/ai/realtime/message-source`, and replace the outdated MESSAGE_SOURCE_VOICE reference. Also narrow the voice-test fixture parameter to MessageSource while preserving nullable and optional source behavior.apps/realtime/src/voice/voice-bridge-client.ts (1)
96-97: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate the bridge response before returning it as
VoiceBridgeResponse.Line 96 casts the parsed JSON directly to
VoiceBridgeResponse.packages/lib/src/realtime/voice-bridge-contract.tsdeclaresVoiceBridgeResponseas a TypeScript type only, with no matching zod schema, so no runtime check happens.A 200 response with an unexpected body then flows out typed as a success result. On the tool path, a caller that reads
.outputreceivesundefinedand can hand it tofunction_call_output.output. The same contract file states that a tool result must always be a string.The request direction is already validated by
voiceBridgeRequestSchema. Consider adding the matching response schema in the contract file and parsing here, so both directions of the hop are checked at the boundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/voice/voice-bridge-client.ts` around lines 96 - 97, Add a runtime zod schema for VoiceBridgeResponse in voice-bridge-contract.ts, including the required string output field, and use that schema to parse the JSON response in the voice bridge client instead of directly casting it. Preserve the existing successful return flow while rejecting malformed 200 responses before callers consume them.packages/lib/src/monitoring/voice-pricing.ts (1)
303-328: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the rate lookup against inherited
Object.prototypekeys.Line 307 indexes
REALTIME_RATESwith an arbitrarymodelstring.REALTIME_RATESis a plain object literal, so it inherits fromObject.prototype. Amodelvalue of'toString','valueOf','constructor', or'hasOwnProperty'resolves to an inherited member instead ofundefined. The!ratescheck at Line 308 then passes, and Line 323 dereferencesrates.cachedInput.texton a value that has nocachedInput, which throws aTypeError.This breaks the contract stated in the doc comment: "Returns 0 for an unknown model". It also matters downstream. In
apps/realtime/src/voice/call-metering.ts,calculateRealtimeCostDollars(model, usage)runs insideflush()but outside thetryblock that wrapstrack(...). A throw there rejects the chained settle promise, and the interval calls it asvoid enqueueFlush(), so the rejection is unhandled.
modelis env-derived today, so this is hardening rather than an active exploit. The fix is small.🛡️ Proposed own-property lookup
- const rates: RealtimeModelRates | undefined = REALTIME_RATES[model as RealtimeModel]; - if (!rates || !usage) return 0; + const rates: RealtimeModelRates | undefined = Object.hasOwn(REALTIME_RATES, model) + ? REALTIME_RATES[model as RealtimeModel] + : undefined; + if (!rates || !usage) return 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/src/monitoring/voice-pricing.ts` around lines 303 - 328, Guard the REALTIME_RATES lookup in calculateRealtimeCostDollars against inherited Object.prototype properties by accepting only own keys of REALTIME_RATES. Preserve the existing 0 return for unknown models, including values such as “toString”, before accessing rate fields.packages/lib/src/billing/credit-pricing.ts (1)
121-133: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEnforce the
CREDIT_HOLD_TTL_SECONDSinvariant instead of documenting it.The doc comment states that
REALTIME_MAX_SESSION_SECONDSmust stay belowCREDIT_HOLD_TTL_SECONDS, and that raising one requires raising the other. Both values are independently overridable through env. If an operator setsREALTIME_MAX_SESSION_SECONDS=1200and leaves the TTL at its 900s default, the reconcile cron can reclaim a live call's own hold mid-call. The test inpackages/lib/src/billing/__tests__/credit-pricing.test.tsasserts this invariant only for the default values, so an override is not covered.Consider clamping the value so the invariant holds for any env combination.
Note the ordering constraint:
CREDIT_HOLD_TTL_SECONDSis declared at Line 316, after this constant. A clamp that reads it here would hit the temporal dead zone. Either moveCREDIT_HOLD_TTL_SECONDSabove this declaration, or derive the clamped value after Line 316.♻️ Sketch of a clamped derivation
-export const REALTIME_MAX_SESSION_SECONDS = envInt('REALTIME_MAX_SESSION_SECONDS', 600); +const REALTIME_MAX_SESSION_SECONDS_RAW = envInt('REALTIME_MAX_SESSION_SECONDS', 600);Then, after
CREDIT_HOLD_TTL_SECONDSis declared:/** Clamped so a session can never outlive the hold that funds it. */ export const REALTIME_MAX_SESSION_SECONDS = Math.min( REALTIME_MAX_SESSION_SECONDS_RAW, Math.max(1, CREDIT_HOLD_TTL_SECONDS - 300), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/src/billing/credit-pricing.ts` around lines 121 - 133, Enforce the hold-TTL invariant for all environment overrides by separating the raw value from the exported value: rename the current envInt result near REALTIME_MAX_SESSION_SECONDS to a raw constant, then derive the exported REALTIME_MAX_SESSION_SECONDS after CREDIT_HOLD_TTL_SECONDS is declared, clamping it to at most CREDIT_HOLD_TTL_SECONDS minus the 300-second settle margin and ensuring the result is at least 1. Update the surrounding comment to describe the enforced clamp.apps/realtime/src/voice/usage-accumulator.ts (1)
82-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCompute
addCachedDetailsonce.The call is duplicated: once in the presence test and once for the value. Both calls allocate. A later edit that changes one argument list and not the other makes the condition disagree with the emitted value, and this is a billing path.
♻️ Proposed refactor to compute the cached details once
const inputA = a.input_token_details; const inputB = b.input_token_details; const outputA = a.output_token_details; const outputB = b.output_token_details; + const cachedDetails = addCachedDetails( + inputA?.cached_tokens_details, + inputB?.cached_tokens_details, + );cached_tokens: count(inputA?.cached_tokens) + count(inputB?.cached_tokens), - ...(addCachedDetails(inputA?.cached_tokens_details, inputB?.cached_tokens_details) - ? { - cached_tokens_details: addCachedDetails( - inputA?.cached_tokens_details, - inputB?.cached_tokens_details, - ), - } - : {}), + ...(cachedDetails === undefined ? {} : { cached_tokens_details: cachedDetails }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/voice/usage-accumulator.ts` around lines 82 - 89, In the usage accumulation logic, update the cached token details handling around addCachedDetails to compute its result once, store it in a local variable, and reuse that variable for both the presence check and cached_tokens_details value. Preserve the current omission behavior when the computed result is absent.apps/realtime/src/voice/call-metering.ts (1)
246-262: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
onLimitis invoked insideflush, and the callback re-enters the settle chain.Line 252 calls
onLimitwhile the currentflushis still executing. The registered callback isruntime.stop, which callsmeter.stop, which awaitsenqueueFlush()on Line 308. That new link chains ontosettling— the promise that is running thisflush.This works today only because
attach-handler.tsLine 168 usesvoid runtime?.stop(reason)and Line 253 returns immediately. If any caller awaitsonLimit, or if a futureflushdoes work afteronLimit, the chain waits on itself and the call hangs with its timers still armed.Defer the callback so the chain cannot be re-entered.
♻️ Proposed hardening
- onLimit('credit_exhausted', 'This call ran out of credits.'); + // Deferred: `onLimit` leads back into `meter.stop`, which enqueues onto + // the same settle chain this flush occupies. + queueMicrotask(() => onLimit('credit_exhausted', 'This call ran out of credits.')); return;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/voice/call-metering.ts` around lines 246 - 262, Defer the onLimit callback in the flush logic after detecting an exhausted credit window, rather than invoking it synchronously while flush is running. Preserve the existing credit-exhaustion reason and message, return from flush immediately, and ensure the deferred callback cannot enqueue work onto the currently executing settling promise.apps/realtime/src/__tests__/realtime-attach-handler.test.ts (1)
339-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a concurrent variant of the per-user cap test.
This test drives the two attaches sequentially, so the first call is registered before the second checks
getForUser. The deployment cap has a concurrent counterpart at Lines 193-219; the per-user cap does not.A concurrent test would fail against the current handler, because the per-user check reads only registered calls and in-flight attaches for the same user are invisible to it. See the related comment on
apps/realtime/src/voice/attach-handler.tsLines 137-153.🧪 Proposed test
+ it('given SIMULTANEOUS attaches from ONE user against a per-user cap of one, should admit exactly one', async () => { + const registry = new RealtimeCallRegistry(8); + let release: (() => void) | undefined; + const inFlight = new Promise<void>((resolve) => { + release = resolve; + }); + const attach = vi.fn(async (options: AttachOptions) => { + await inFlight; + return fakeSession(options); + }); + const d = deps({ registry, attach, maxCallsPerUser: 1 }); + + const both = Promise.all([ + handleRealtimeAttachRequest(d, JSON.stringify(VALID)), + handleRealtimeAttachRequest(d, JSON.stringify({ ...VALID, callId: 'rtc_second' })), + ]); + release?.(); + const [a, b] = await both; + + expect([a.status, b.status].sort()).toEqual([200, 429]); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/__tests__/realtime-attach-handler.test.ts` around lines 339 - 355, Add a concurrent per-user-cap test alongside the existing sequential test, starting two attach requests for the same user before awaiting either and asserting one succeeds, the other returns 429, and attach is invoked only once. Use the existing concurrency test pattern and symbols such as handleRealtimeAttachRequest, maxCallsPerUser, and attach.apps/realtime/src/voice/attach-handler.ts (1)
158-170: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConstrain
subscriptionTierat the contract boundary.
realtimeAttachPayloadSchemaaccepts arbitrary strings, andstartCallMeterpasses the value tocanConsumeAI. Usez.enum(TIERS).default('free'), remove the cast, and update the test that uses invalid tierplus.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/voice/attach-handler.ts` around lines 158 - 170, Update realtimeAttachPayloadSchema to validate subscriptionTier with z.enum(TIERS).default('free') before it reaches startCallMeter/canConsumeAI. Then remove the SubscriptionTier cast from the deps.startMeter call and update the affected test fixture to use a valid tier instead of invalid `plus`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 26e2a3cc-c395-42cf-83e0-8773ec4f5a2a
📒 Files selected for processing (133)
.env.exampleCHANGELOG.mdapps/realtime/src/__tests__/index.test.tsapps/realtime/src/__tests__/realtime-attach-handler.test.tsapps/realtime/src/__tests__/realtime-call-registry.test.tsapps/realtime/src/__tests__/realtime-call-session.test.tsapps/realtime/src/index.tsapps/realtime/src/voice/__tests__/call-metering.test.tsapps/realtime/src/voice/__tests__/usage-accumulator.test.tsapps/realtime/src/voice/__tests__/voice-bridge-client.test.tsapps/realtime/src/voice/__tests__/voice-call-runtime.test.tsapps/realtime/src/voice/attach-handler.tsapps/realtime/src/voice/call-hangup.tsapps/realtime/src/voice/call-metering.tsapps/realtime/src/voice/realtime-call-registry.tsapps/realtime/src/voice/realtime-call-session.tsapps/realtime/src/voice/usage-accumulator.tsapps/realtime/src/voice/voice-bridge-client.tsapps/realtime/src/voice/voice-call-runtime.tsapps/web/src/app/api/ai/chat/messages/[messageId]/__tests__/route.test.tsapps/web/src/app/api/ai/chat/messages/__tests__/route.test.tsapps/web/src/app/api/ai/global/[id]/messages/[messageId]/__tests__/route.test.tsapps/web/src/app/api/internal/voice/bridge/route.tsapps/web/src/app/api/v1/chat/completions/__tests__/route-backfill.test.tsapps/web/src/app/api/v1/chat/completions/__tests__/route.test.tsapps/web/src/app/api/v1/conversations/__tests__/route.test.tsapps/web/src/app/api/voice/realtime/call/__tests__/route.test.tsapps/web/src/app/api/voice/realtime/call/route.tsapps/web/src/app/api/voice/transcribe/__tests__/route.test.tsapps/web/src/app/api/voice/transcribe/route.tsapps/web/src/components/ai/chat/input/ChatInput.tsxapps/web/src/components/ai/shared/chat/CompactMessageRenderer.tsxapps/web/src/components/ai/shared/chat/MessageRenderer.tsxapps/web/src/components/ai/shared/chat/SpokenTurnGlyph.tsxapps/web/src/components/ai/shared/chat/__tests__/SpokenTurnGlyph.test.tsxapps/web/src/components/ai/shared/chat/message-types.tsapps/web/src/components/ai/voice/VoiceCallPanel.tsxapps/web/src/components/ai/voice/VoiceModeBorder.tsxapps/web/src/components/ai/voice/VoiceModeSettings.tsxapps/web/src/components/ai/voice/index.tsapps/web/src/components/ai/voice/realtime/VoiceCallBar.tsxapps/web/src/components/ai/voice/realtime/VoiceCallBarForConversation.tsxapps/web/src/components/ai/voice/realtime/VoiceNavTrigger.tsxapps/web/src/components/ai/voice/realtime/VoiceSessionBridge.tsxapps/web/src/components/ai/voice/realtime/__tests__/VoiceCallBar.test.tsxapps/web/src/components/ai/voice/realtime/__tests__/VoiceCallBarForConversation.test.tsxapps/web/src/components/ai/voice/realtime/__tests__/VoiceNavTrigger.test.tsxapps/web/src/components/ai/voice/realtime/__tests__/VoiceSessionBridge.test.tsxapps/web/src/components/ai/voice/realtime/index.tsapps/web/src/components/layout/Layout.tsxapps/web/src/components/layout/__tests__/Layout.voice-reveal.test.tsxapps/web/src/components/layout/__tests__/Layout.voice-session.test.tsxapps/web/src/components/layout/main-header/__tests__/TopBar.voice-trigger.test.tsxapps/web/src/components/layout/main-header/index.tsxapps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsxapps/web/src/components/layout/right-sidebar/__tests__/RightPanel.page-tab.test.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsxapps/web/src/components/layout/right-sidebar/index.tsxapps/web/src/components/ui/floating-input/InputFooter.tsxapps/web/src/contexts/VoiceSessionContext.tsxapps/web/src/contexts/__tests__/VoiceSessionContext.test.tsxapps/web/src/hooks/useVoiceMode.tsapps/web/src/hooks/voice/useAudioLevel.tsapps/web/src/hooks/voice/useVoiceBinding.tsapps/web/src/lib/ai/core/__tests__/message-utils.test.tsapps/web/src/lib/ai/core/message-utils.tsapps/web/src/lib/ai/realtime/__tests__/bridge-handler.test.tsapps/web/src/lib/ai/realtime/__tests__/call-chrome.test.tsapps/web/src/lib/ai/realtime/__tests__/call-handshake.test.tsapps/web/src/lib/ai/realtime/__tests__/chain-schedule.test.tsapps/web/src/lib/ai/realtime/__tests__/connect.test.tsapps/web/src/lib/ai/realtime/__tests__/seed-loader.test.tsapps/web/src/lib/ai/realtime/__tests__/seed.test.tsapps/web/src/lib/ai/realtime/__tests__/session-state.test.tsapps/web/src/lib/ai/realtime/__tests__/session.test.tsapps/web/src/lib/ai/realtime/__tests__/tool-dispatch.test.tsapps/web/src/lib/ai/realtime/__tests__/tools.test.tsapps/web/src/lib/ai/realtime/__tests__/transcript-persistence.test.tsapps/web/src/lib/ai/realtime/__tests__/voice-binding.test.tsapps/web/src/lib/ai/realtime/__tests__/voice-bridge-contract-drift.test.tsapps/web/src/lib/ai/realtime/__tests__/voice-location.test.tsapps/web/src/lib/ai/realtime/__tests__/voice-rebind.test.tsapps/web/src/lib/ai/realtime/__tests__/voice-reveal.test.tsapps/web/src/lib/ai/realtime/__tests__/voice-target.test.tsapps/web/src/lib/ai/realtime/__tests__/webrtc-fakes.tsapps/web/src/lib/ai/realtime/bridge-handler.tsapps/web/src/lib/ai/realtime/call-chrome.tsapps/web/src/lib/ai/realtime/call-handshake.tsapps/web/src/lib/ai/realtime/chain-schedule.tsapps/web/src/lib/ai/realtime/connect.tsapps/web/src/lib/ai/realtime/message-source.tsapps/web/src/lib/ai/realtime/seed-loader.tsapps/web/src/lib/ai/realtime/seed.tsapps/web/src/lib/ai/realtime/session-state.tsapps/web/src/lib/ai/realtime/session.tsapps/web/src/lib/ai/realtime/tool-dispatch.tsapps/web/src/lib/ai/realtime/tools.tsapps/web/src/lib/ai/realtime/transcript-persistence.tsapps/web/src/lib/ai/realtime/voice-binding.tsapps/web/src/lib/ai/realtime/voice-location.tsapps/web/src/lib/ai/realtime/voice-rebind.tsapps/web/src/lib/ai/realtime/voice-reveal.tsapps/web/src/lib/ai/realtime/voice-runtime-deps.tsapps/web/src/lib/ai/realtime/voice-target.tsapps/web/src/lib/ai/streams/__tests__/selectPostBaselineAssistantMessage.test.tsapps/web/src/lib/ai/streams/__tests__/selectVoiceActivationBaseline.test.tsapps/web/src/lib/ai/streams/__tests__/selectVoiceStreamText.test.tsapps/web/src/lib/ai/streams/selectPostBaselineAssistantMessage.tsapps/web/src/lib/ai/streams/selectVoiceActivationBaseline.tsapps/web/src/lib/ai/streams/selectVoiceStreamText.tsapps/web/src/lib/repositories/message-repository.tsapps/web/src/lib/repositories/unified-message-leg.tsapps/web/src/lib/voice/__tests__/mic-errors.test.tsapps/web/src/lib/voice/mic-errors.tsapps/web/src/stores/__tests__/useVoiceModeStore.test.tsapps/web/src/stores/useLayoutStore.tsapps/web/src/stores/useVoiceModeStore.tsapps/web/src/stores/useVoiceRebindStore.tsdocs/2.0-architecture/agent-sessions.mdknip.jsonpackages/db/drizzle/0258_thin_dormammu.sqlpackages/db/drizzle/meta/0258_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/conversations.tspackages/lib/package.jsonpackages/lib/src/billing/__tests__/credit-pricing.test.tspackages/lib/src/billing/credit-pricing.tspackages/lib/src/monitoring/__tests__/voice-pricing.test.tspackages/lib/src/monitoring/voice-pricing.tspackages/lib/src/realtime/__tests__/voice-events.test.tspackages/lib/src/realtime/voice-bridge-contract.tspackages/lib/src/realtime/voice-events.tspackages/lib/src/services/sandbox/tool-runners.ts
💤 Files with no reviewable changes (16)
- apps/web/src/lib/ai/streams/tests/selectVoiceActivationBaseline.test.ts
- apps/web/src/components/ai/voice/index.ts
- apps/web/src/lib/ai/streams/selectVoiceStreamText.ts
- apps/web/src/app/api/voice/transcribe/route.ts
- apps/web/src/components/ai/voice/VoiceModeSettings.tsx
- apps/web/src/lib/ai/streams/tests/selectVoiceStreamText.test.ts
- apps/web/src/lib/ai/streams/selectVoiceActivationBaseline.ts
- apps/web/src/app/api/voice/transcribe/tests/route.test.ts
- apps/web/src/components/ai/voice/VoiceModeBorder.tsx
- apps/web/src/stores/useVoiceModeStore.ts
- apps/web/src/hooks/useVoiceMode.ts
- apps/web/src/stores/tests/useVoiceModeStore.test.ts
- apps/web/src/components/ai/voice/VoiceCallPanel.tsx
- apps/web/src/components/ai/chat/input/ChatInput.tsx
- apps/web/src/lib/ai/streams/tests/selectPostBaselineAssistantMessage.test.ts
- apps/web/src/lib/ai/streams/selectPostBaselineAssistantMessage.ts
Review findingsThe patch permits unmetered realtime calls, can strand legacy personalization outside the migration path, and currently fails the tenant-export column coverage test. Additional concurrency and corroboration defects can violate documented limits and memory accuracy. P1 — Reject calls when the metering attachment fails
When the realtime server returns 402/429, is unreachable, or is unconfigured, this returns P1 — Preserve legacy personalization before provisioning pointers
For a legacy user with populated text columns and no page pointers, opening this GET provisions all pointers without copying the legacy content. P2 — Export the new message source column
The new P2 — Avoid recounting older evidence on every discovery run
When discovery re-cites evidence older than P2 — Honor the configured global realtime session limit
The newly introduced P2 — Reserve per-user call capacity atomically
When billing is disabled, two simultaneous attaches for the same user can both observe the same registered-call count before either reaches registration, because the existing pending reservation is global rather than keyed by user. Both requests then pass this check and exceed |
…ching Six review findings in apps/realtime, all of the same shape: a limit that did not limit, or a failure that escaped the path meant to contain it. - The per-user call cap counted only REGISTERED calls, and a call registers two awaits after admission — so simultaneous attaches from one person all saw zero and all passed. Claimed synchronously now, in the same reserveSlot() step as the deployment slot, with a race test that goes red against the old code. - REALTIME_MAX_GLOBAL_SESSIONS was parsed and documented but never read; the registry hardcoded 8. An operator tuning it saw nothing change. - 'call_ended' covers a network drop, a process shutdown and the one-hour socket backstop as well as a real hangup, so skipping the hangup for it left the browser talking to a model nobody was metering. We hang up on every reason now; a call that really ended answers 404, which is read as "already ended" rather than reported as a refusal. - Promise.all over the tool answers meant one throwing session.send skipped the response.create for the whole turn — the model holding a turn it never speaks. allSettled, with each rejection logged. - The fire-and-forget transcript write had no .catch, so a rejecting bridge raised an unhandled rejection inside a socket handler: process down, every other live call with it. - The timeout comments claimed the transcript got the longer budget while the values said the opposite. The values are right — a tool dispatch runs a real page read behind it, a transcript is one insert — so the reasoning is corrected to match, and the test that said "TIGHTER" now says what it asserts. Also restores the coverage gate CI was failing on (branches 97.01% -> 98.08%): call-hangup.ts had no test file at all, and call-metering.test.ts repeated the same unreachable `if (!started.ok) throw` narrowing twenty-two times, which is twenty-two half-taken branches. That is now one shared helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/realtime/src/voice/__tests__/voice-call-runtime.test.ts (1)
356-410: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove listener cleanup into
finally.
flushalready reaches theunhandledRejectionevent. Usefinallyto remove the listener if the test body fails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/voice/__tests__/voice-call-runtime.test.ts` around lines 356 - 410, Update both transcript rejection tests around the unhandledRejection listener so process.off is executed in a finally block. Keep the existing assertions and test setup in the protected try section, ensuring the listener is removed whether flush or an assertion fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/realtime/src/voice/__tests__/call-hangup.test.ts`:
- Around line 84-92: Update the test case around hangUpCall to guarantee
globalFetch cleanup even when the call or assertions fail. Place the await
expect and verification inside a try/finally block with
globalFetch.mockRestore() in the finally clause, or use the test suite’s
established afterEach mock cleanup if available.
In `@apps/realtime/src/voice/voice-call-runtime.ts`:
- Around line 258-269: Update the stop flow in the voice-call runtime so a
rejection from meter.stop(reason) is caught and does not abort execution or
propagate out of stop. Ensure hangUp(callId, secret) and session.end still run
after metering settlement fails, and preserve stop’s resolved completion for
callers such as attach-handler.
---
Nitpick comments:
In `@apps/realtime/src/voice/__tests__/voice-call-runtime.test.ts`:
- Around line 356-410: Update both transcript rejection tests around the
unhandledRejection listener so process.off is executed in a finally block. Keep
the existing assertions and test setup in the protected try section, ensuring
the listener is removed whether flush or an assertion fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d36af5e-d016-449e-b611-2a44d1c30f72
📒 Files selected for processing (12)
apps/realtime/src/__tests__/realtime-attach-handler.test.tsapps/realtime/src/__tests__/realtime-call-registry.test.tsapps/realtime/src/__tests__/realtime-call-session.test.tsapps/realtime/src/voice/__tests__/call-hangup.test.tsapps/realtime/src/voice/__tests__/call-metering.test.tsapps/realtime/src/voice/__tests__/voice-bridge-client.test.tsapps/realtime/src/voice/__tests__/voice-call-runtime.test.tsapps/realtime/src/voice/attach-handler.tsapps/realtime/src/voice/call-hangup.tsapps/realtime/src/voice/realtime-call-registry.tsapps/realtime/src/voice/voice-bridge-client.tsapps/realtime/src/voice/voice-call-runtime.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/realtime/src/voice/tests/voice-bridge-client.test.ts
- apps/realtime/src/tests/realtime-call-registry.test.ts
- apps/realtime/src/voice/attach-handler.ts
- apps/realtime/src/voice/voice-bridge-client.ts
- apps/realtime/src/tests/realtime-attach-handler.test.ts
- apps/realtime/src/tests/realtime-call-session.test.ts
- apps/realtime/src/voice/tests/call-metering.test.ts
The four P1 review findings were one bug wearing four hats: nothing about the bound assistant was ever resolved server-side, so a call presented as talking to a page agent got the realtime model's default persona, the whole deployment tool registry, and the CALLER's permissions rather than the agent's. Resolved once now, from the conversation the caller is already authorized for (a page conversation's contextId IS its agent page), and carried for the life of the call: - TOOL EXECUTION runs as the agent. `resolveActingAgentId` reads chatSource.agentPageId and every canActor* check silently fell through to the invoking user without it, so an agent whose memberships are narrower than the caller's borrowed the caller's reach. The text surface authorizes as the agent for the same conversation; voice now does too. - THE ALLOWLIST is honoured on BOTH lists. The advertised set and the executable set are built in different processes on different requests, and filtering only one of them is not filtering: `execute_tool` reaches whatever the exposure split deferred, so the allowlist is applied before the split and before tool_search is handed its catalog. - INSTRUCTIONS reach OpenAI. The mint carries none by design, so they ride the same session.update as the tools — spoken-turn guidance plus the agent's own systemPrompt verbatim. Omitted rather than sent empty when there is none: an empty `instructions` REPLACES the session's. - ADMISSION REFUSALS ARE REFUSALS. A 402 or 429 from the attach was reduced to `attached: false` and the browser kept a working, unmetered OpenAI call — a credit gate that was worse than absent. Those two statuses now fail the handshake and hang up the call OpenAI just made; 400/502 still degrade, because an outage declined nothing. Also from review, in the same surfaces: - The `upstream` passthrough is scoped to `realtime_call_rejected`. It exists to make a bad OPENAI_REALTIME_MODEL visible; a `mint_failed` body describes the managed API key request and can name the org, the quota or a masked key. - Voice transcript writes re-check `isActive` under `FOR UPDATE` inside the write transaction. A call is held open for minutes, so "checked before the bridge hop" is not a check. - The broadcast `source` is the PERSISTED one. It is insert-only, so a second write without it broadcast null over a row still saying 'voice' — the open thread dropped the mic glyph and grew it back on refresh. - connect.ts wraps peer setup in the try that releases the microphone; a throw from createPeerConnection/addTrack/createDataChannel left the device held and the recording indicator on, one cloned track per attempt on the reuse path. - useAudioLevel closes an AudioContext orphaned by a later setup failure — a stream with no live track is named as an expected cause, so it leaked a hardware audio thread in normal use. - The two browser voice surfaces catch their own rejections; the trigger also toasts, because a failure before `start` never reaches chrome.state 'error' and the press looked like it did nothing. seed-loader.ts is now binding-loader.ts: it answers one question — what is this call bound to — behind one read and one access check, rather than deciding access twice for the seed and the assistant separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
Self-review on the binding loader: `loadAgentPage` throwing escaped to the outer handler, which degrades the WHOLE binding to unbound — throwing away a seed that had already been fetched successfully alongside it, for a failure in a different read. Caught at the agent lookup instead. The call then runs as the Global Assistant acting as the authenticated user, which is what every voice call did before the binding existed and is bounded by that user's own ACL; the history survives, and the failure is logged, because a page conversation whose agent cannot be read is not the ordinary outcome a missing page row is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/lib/repositories/unified-message-leg.ts (1)
72-78: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPersist
sourcein every writer that acceptsUnifiedPageMessageRow.
sourceis now part ofUnifiedPageMessageRow, butmaterializeUnifiedPageMessageandinsertUnifiedPageMessagestill omit it from their insert values at Lines 154-168 and Lines 194-206. A caller that supplies voice metadata through either path can silently storeNULL. Addsource: row.source ?? nullto both insert paths, or use a narrower input type for functions that must not acceptsource.Proposed fix
sourceAgentId: row.sourceAgentId, + source: row.source ?? null,Apply the same addition to both
materializeUnifiedPageMessageandinsertUnifiedPageMessage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/repositories/unified-message-leg.ts` around lines 72 - 78, Update both materializeUnifiedPageMessage and insertUnifiedPageMessage to include source in their insert values, using row.source ?? null so supplied voice metadata is persisted while omitted values remain null.
🧹 Nitpick comments (2)
apps/web/src/lib/ai/realtime/binding-loader.ts (1)
68-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
maxTurnsandmaxTokensare declared but no caller supplies them.
apps/web/src/app/api/voice/realtime/call/route.tscallsloadVoiceBindingwithuserIdandconversationIdonly. The two seed-budget fields are therefore dead surface today. Keep them only if a second caller is planned; otherwise remove them and letbuildRealtimeSeedown its defaults.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/binding-loader.ts` around lines 68 - 73, Remove the unused maxTurns and maxTokens fields from BindingLoaderRequest, since loadVoiceBinding currently receives only userId and conversationId. Keep seed-budget defaults owned by buildRealtimeSeed and update any related request typing or destructuring to match.apps/web/src/lib/ai/realtime/__tests__/connect.test.ts (1)
390-405: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe clone-stop claim is not asserted.
The test asserts only that the caller's own track stays live. It does not assert that the clone taken by this attempt was stopped. A regression that never clones, or that leaks the clone, still passes. Capture the cloned stream from the fake and assert its track is stopped.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/realtime/__tests__/connect.test.ts` around lines 390 - 405, Update the reused-microphone test around fakeStream, asStream, and the connectVoiceCall call to capture the cloned stream created during the attempt, then assert that its track is stopped. Retain the existing assertion confirming the caller’s original track remains live.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/web/src/lib/repositories/unified-message-leg.ts`:
- Around line 72-78: Update both materializeUnifiedPageMessage and
insertUnifiedPageMessage to include source in their insert values, using
row.source ?? null so supplied voice metadata is persisted while omitted values
remain null.
---
Nitpick comments:
In `@apps/web/src/lib/ai/realtime/__tests__/connect.test.ts`:
- Around line 390-405: Update the reused-microphone test around fakeStream,
asStream, and the connectVoiceCall call to capture the cloned stream created
during the attempt, then assert that its track is stopped. Retain the existing
assertion confirming the caller’s original track remains live.
In `@apps/web/src/lib/ai/realtime/binding-loader.ts`:
- Around line 68-73: Remove the unused maxTurns and maxTokens fields from
BindingLoaderRequest, since loadVoiceBinding currently receives only userId and
conversationId. Keep seed-budget defaults owned by buildRealtimeSeed and update
any related request typing or destructuring to match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c75c933f-2f8a-4610-9ba1-400860052cf0
📒 Files selected for processing (29)
apps/realtime/src/__tests__/realtime-attach-handler.test.tsapps/realtime/src/__tests__/realtime-call-session.test.tsapps/realtime/src/voice/__tests__/voice-call-runtime.test.tsapps/realtime/src/voice/attach-handler.tsapps/realtime/src/voice/realtime-call-session.tsapps/realtime/src/voice/voice-call-runtime.tsapps/web/src/app/api/voice/realtime/call/__tests__/route.test.tsapps/web/src/app/api/voice/realtime/call/route.tsapps/web/src/components/ai/voice/realtime/VoiceNavTrigger.tsxapps/web/src/components/ai/voice/realtime/VoiceSessionBridge.tsxapps/web/src/hooks/voice/useAudioLevel.tsapps/web/src/lib/ai/realtime/__tests__/binding-loader.test.tsapps/web/src/lib/ai/realtime/__tests__/bridge-handler.test.tsapps/web/src/lib/ai/realtime/__tests__/call-handshake.test.tsapps/web/src/lib/ai/realtime/__tests__/connect.test.tsapps/web/src/lib/ai/realtime/__tests__/tool-dispatch.test.tsapps/web/src/lib/ai/realtime/__tests__/tools.test.tsapps/web/src/lib/ai/realtime/binding-loader.tsapps/web/src/lib/ai/realtime/bridge-handler.tsapps/web/src/lib/ai/realtime/call-handshake.tsapps/web/src/lib/ai/realtime/connect.tsapps/web/src/lib/ai/realtime/instructions.tsapps/web/src/lib/ai/realtime/tool-dispatch.tsapps/web/src/lib/ai/realtime/tools.tsapps/web/src/lib/ai/realtime/voice-runtime-deps.tsapps/web/src/lib/repositories/__tests__/message-repository-write-guards.test.tsapps/web/src/lib/repositories/message-repository.tsapps/web/src/lib/repositories/unified-message-leg.tspackages/lib/src/realtime/voice-bridge-contract.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/realtime/src/voice/attach-handler.ts
- apps/web/src/hooks/voice/useAudioLevel.ts
- apps/realtime/src/voice/voice-call-runtime.ts
- apps/realtime/src/tests/realtime-call-session.test.ts
- apps/web/src/components/ai/voice/realtime/VoiceNavTrigger.tsx
- apps/realtime/src/tests/realtime-attach-handler.test.ts
- apps/web/src/components/ai/voice/realtime/VoiceSessionBridge.tsx
- apps/realtime/src/voice/realtime-call-session.ts
- apps/web/src/lib/ai/realtime/tool-dispatch.ts
- apps/web/src/lib/repositories/message-repository.ts
- apps/web/src/lib/ai/realtime/connect.ts
- apps/web/src/lib/ai/realtime/bridge-handler.ts
- apps/web/src/lib/ai/realtime/call-handshake.ts
…cedes New review round, and it is the same failure class as the transcript write one directory over. `stop()` awaited `meter.stop(reason)` first — correct, the final window is usually the most expensive part of the call — but a rejection there skipped BOTH the hangup and `session.end`, leaving precisely the state that whole path exists to prevent: a live browser call with nobody metering it. The rejection then escaped into `attach-handler.ts`'s `void runtime?.stop(…)` calls, where an unhandled rejection takes the process down and every other live call with it. Caught and logged; teardown continues. An unbilled final window is a bad outcome, an unbilled ONGOING call is a worse one. Two tests, mutation-checked: one asserting hangup and session.end still run, one asserting nothing reaches the unhandledRejection handler. Also restores the global `fetch` spy in a `finally` in call-hangup.test.ts — a spy that survives a failing assertion is a spy every later test runs under. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
…nd that limits refuse it The unreleased voice entries described a call talking to 'whichever assistant you are already looking at' — true of the binding, but until this round the model never received that assistant's instructions or its tool allowlist. Both now hold, so the claim is worth stating plainly rather than leaving implied. The admission change is user-visible in its own right: out of credit, or at the concurrency limit, used to connect anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
New module with only indirect coverage through the binding loader. The cases that matter are an agent owner's own prompt arriving verbatim, the medium coming before the persona so nothing bolted on argues with it, a cleared field counting as no prompt rather than empty ones, and the builder never returning '' — an empty `instructions` on a session.update REPLACES the session's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
Follow-through on my own admission change: making the route answer 402/429 left the browser mapping both to `signaling-failed`, so "you are out of credit" and "too many calls right now" were presented as "Could not reach the voice service" — and both offered a retry. Named separately, because they want opposite advice: `no-credit` joins the two microphone cases in UNRETRYABLE (a retry cannot conjure credit), while `call-limit` stays retryable, since a concurrency cap clears on its own and trying again in a moment is exactly right. The route's own sentence still wins over these fallbacks — it is the one that knows the specifics — but the classification is what decides whether a Retry button appears, and that was wrong on both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
The guard the review asked for landed in `voice-runtime-deps.ts` — the module that exists precisely to be the untestable seam — so the mechanism itself had no test. Exported and pinned directly instead: allows a live conversation, refuses one deleted mid-call, refuses a missing row rather than assuming the best, and takes the row lock rather than merely reading it. That last case is the one that matters and is mutation-checked: without FOR UPDATE this is a second guess rather than a decision, because it would not serialise against the delete it is racing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
The migration this PR adds (`0258`, `messages.source`) never registered the column in `scripts/lib/tenant-export-columns.ts`, so the drift guard failed: an unlisted column is dropped silently by every tenant migration. Carried rather than excluded. `source` is how a turn is known to have been spoken rather than typed, and the thread renders a microphone from it — a tenant that migrated without it would keep every word and lose the account of how they were said, which is the same failure the column exists to prevent. This was invisible until now: the realtime coverage gate aborted the turbo run several tasks earlier, so the scripts suite never ran on this branch. Fixing that gate is what let CI reach this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H8anXTeDyT7VZLVN3Vp5z
1 participant
Replaces PageSpace's voice mode with an audio-native one on OpenAI's Realtime API (
gpt-realtime-2.1), built as nine reviewed PRs into this branch.Why
The old voice mode transcribed before inference:
MediaRecorder→ whisper → the text chat stream → tts → playback. The model never heard audio, so pace, hesitation, interruption and tone were destroyed before it saw your words, and you waited for a whole reply to be written before hearing any of it. That isn't a tuning problem — it's the wrong shape.The thesis every decision was framed against
Voice is not a feature of a surface. It is a second transport onto conversations PageSpace already has.
You speak into a conversation that already existed and still exists after you hang up — as text, in the thread, where the sidebar and history already render it. One trigger on every route, binding to whatever assistant is already in view. Any page agent someone already built is talkable, with no new config.
How it works
The browser owns only the mic and the speaker. It POSTs an SDP offer to our route; the server mints the ephemeral secret, relays to OpenAI, reads the
call_id, and hands off over an HMAC-signed internal call toapps/realtime— which attaches to that same session over WebSocket and holds it for the life of the call. Tools, permissions, persistence and metering all stay server-side; a tool schema never reaches the client.splitToolsForExposure, so voice exposes the identical core tools plustool_search/execute_toolas text — filtered by the bound agent's ownenabledTools, on both the advertised and the executable list. Permissions come for free because they're enforced inside the tools, against the agent the call is bound to.messagesrows viamessageRepository, bumpingconversations.revin-transaction and emitting the events every chat surface already subscribes to — so the sidebar updates live with zero new client wiring.usageperresponse.doneat real per-modality token rates, with hold/settle, idle timeout, duration cap and a concurrency cap against the account's 40k tok/min ceiling.What a call is bound to
Voice binds to a conversation that already exists, so almost everything about the call is a fact about that conversation — resolved once, server-side, behind that conversation's own access check (
loadVoiceBinding), and never read off the request body:type: 'page'conversation'scontextIdis its agent page, asserted to beAI_CHAT;systemPrompt, verbatim, pushed on the samesession.updateas the tools (the mint carries neither);tool_searchcannot describe andexecute_toolcannot reach a tool that was switched off.Tool execution therefore runs as the agent (
chatSource.agentPageId), exactly aspage-chat-turn.tsdoes for the same conversation — not as whoever pressed the mic.Verified against the live API before it was built
A spike proved the whole architecture end to end — real Chromium, real WebRTC call, server attached to the same session, registered a tool, received the
function_call, answered it, and the model spoke a value only our server could have supplied. It also found things the docs get wrong:client_secretsreturns 200 for a model/v1/realtime/callswill reject.404 call_id_not_found).session.createdever arrives.function_call.argumentsis a newline-laden string;usageis per-response, not per-call.Not verified
Nobody has spoken to this yet. Every test uses fakes — fake
RTCPeerConnection, fake WebSocket, fake mic. Coverage was checked by mutation (breaking the source and watching tests go red) which caught four gaps that green checkmarks hid, but real acceptance is pressing the mic and talking.One specific claim to confirm on a live call: the hangup credential is inferred, not measured — it uses the call's own ephemeral secret, which is what authenticates the attach, but that wasn't verified against a live hangup. It degrades safely (a refused hangup still tears our side down), and it now runs on two paths: every teardown reason, and an admission refusal abandoning a call OpenAI had already accepted.
Not retired
/api/voice/synthesizeand Read Aloud stay — on-demand "read this to me" is a different feature that an audio-native conversation doesn't replace, and open PR #2173 builds on that route. Only the conversational loop (useVoiceMode.ts,/api/voice/transcribe) is deleted.Migration
One nullable additive column:
ALTER TABLE messages ADD COLUMN source text(0258), marking voice-authored rows so the UI can show a mic glyph. No backfill, no lock risk. Renumbered from0256after syncing master.Gate
~1,800 web tests, 1,122 realtime tests, 826 lib tests, monorepo
tscandeslintclean, andapps/realtimeback over its 98% branch-coverage threshold (97.01% → 98.08%) — that gate was what CI was failing on. The only failures are the known Postgres-backed integration suites that need a local test DB.New tests were mutation-checked where they guard a security or correctness mechanism: the per-user admission race, the agent tool allowlist, the acting-agent context, the microphone release on a peer-setup throw, and the persisted-
sourcebroadcast were each verified to go red against the unfixed code.Summary by CodeRabbit
New Features
Bug Fixes
Changes