feat(voice): the browser holds the mic and the speaker, and nothing else - #2396
Conversation
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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4d0240ae8
ℹ️ 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".
| * the internal attach — whose own upstream timeout is 15s, plus the moment of | ||
| * swapping which stream feeds the speaker. | ||
| */ | ||
| export const CHAIN_LEAD_MS = 20_000; |
There was a problem hiding this comment.
Increase the lead to cover the complete handshake
When either OpenAI request is slow, 20 seconds is insufficient to complete a replacement before the old call is capped. runCallHandshake performs the mint and call requests sequentially, each with a 15-second timeout, followed by a handoff with another 5-second timeout; the old call therefore can close while the replacement is still pending, and onConnectionLost tears that pending attempt down. Base this lead on the full worst-case handshake or keep the pending replacement alive when the capped call closes.
Useful? React with 👍 / 👎.
| if (options.chained && previous?.connection) { | ||
| previous.connection.stop(); |
There was a problem hiding this comment.
Keep attached calls until the replacement is attached
When the realtime handoff is refused or unavailable, the route deliberately still returns HTTP 200 with attached: false, so connectVoiceCall reports ok: true. This branch consequently stops the healthy attached call and replaces it with an audio-only call that has no tools, transcript persistence, or usage metering; global/per-user saturation or a transient realtime outage can therefore leave a full session interval unmetered and unpersisted. Require an attached replacement when the outgoing call is attached, or retain it and retry.
Useful? React with 👍 / 👎.
| console.error( | ||
| `[voice] chain failed, staying on the current call: ${result.detail}`, | ||
| ); | ||
| return; |
There was a problem hiding this comment.
Retry a failed chain before the current call expires
When the single replacement request encounters a transient network error or 502, the chain timer has already fired and this return schedules no further attempt. The currently working call is still subject to its hard cap, so one brief failure guarantees that the user is disconnected roughly one lead interval later. Schedule another attempt within the remaining headroom rather than abandoning chaining permanently.
Useful? React with 👍 / 👎.
…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
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
C-C — the browser holds the mic and the speaker
The server side has been merged and working since #2393/#2395. Nothing had ever driven it from a browser. This is that half: the first real end-to-end exercise of the relay route from a page.
The page creates the peer connection, adds the mic track, opens the
oai-eventsdata channel, POSTs its SDP offer toPOST /api/voice/realtime/call, and applies the answer. It never contactsapi.openai.com, never holds an ephemeral secret, and is never sent a tool schema.What's here
lib/ai/realtime/connect.tslib/ai/realtime/voice-target.tslib/ai/realtime/chain-schedule.tslib/voice/mic-errors.tsuseVoiceMode, now shared by both paths.contexts/VoiceSessionContext.tsxWhy the provider is in
Layoutand not the sidebarRightPanelis unmounted outright when the sidebar closes (Layout.tsx:rightPanelVisible && …). A session owned by the panel would hang up every time somebody collapsed it. The sidebar is voice's home, not its owner.The test starts a call from inside the panel, then unmounts the panel, then remounts it and finds the same
callIdstill running — the property is asserted, not just arranged.Chaining past the cap
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 — the transcript is the durable layer, so the new session already knows what was said. The swap is make-before-break, so there is never a moment with no session. The microphone is handed over as an independent track clone: one permission prompt for the whole conversation, 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.1. The route now reports
maxDurationMs(I changed the contract).Chaining needs to know when the server will hang up.
REALTIME_MAX_SESSION_SECONDSis per-deployment env the browser cannot read, so the call route now returns it. The alternative was a client-side copy of a server env var — a copy that drifts, whose failure mode is a user cut off mid-sentence on the one deployment that tuned it. Additive; existing fields unchanged.Caveat in the code:
apps/realtime's own hour-long socket ceiling (MAX_CALL_DURATION_MS) is not importable from web (no dependency edge, deliberately), so the route reports the cap this tier owns. Correct unless a deployment setsREALTIME_MAX_SESSION_SECONDSabove 3600.2. There is no hop for updating a live call's
locationContext. ← the real gap"Navigating mid-call updates
locationContextinstead of rebinding" is a settled decision, butVOICE_BRIDGE_ROUTEShasattachand nothing else. The provider does its half — it tracks the latest location and carries it into every call it opens — but between chains, the tools answer "what's on this page?" with the page the call started on. The fix belongs on the server contract (an update route the realtime server applies to its held context), not in a client that would otherwise have to fake it by rebinding, which is exactly what the design forbids.3. There is no client-facing hangup. When the user hangs up or hard-refreshes, the browser closes the peer connection; nothing tells the realtime server, which holds its socket until
REALTIME_IDLE_TIMEOUT_SECONDS(120s) reaps it. No audio flows in that window so nothing bills, but the concurrency slot is held. Bounded, not free.The seam for C-D (voice UI) — build on exactly this
The UI chunk consumes two hooks and nothing else. Do not mount an
<audio>element and do not own a session per panel — that is the failure mode this provider exists to prevent.start({ conversationId, type, contextId?, agentPageId? }).start(…)with the new agent's target. It rebinds for free — do notstop()thenstart(), that discards the make-before-break path and flashes an idle state.setLocationContext(…). Never callstarton navigation.startwith the target already bound is a no-op, so a re-render handing back the same target cannot restart a call mid-sentence.attached: falseis a working audio call with no tools, no transcript, no metering. Say so in the UI rather than implying full capability.<audio>element and the microphone belong to the provider.Not in scope, deliberately
No UI chrome (that's C-D). No changelog entry — voice is not user-reachable until C-D lands a trigger, matching what #2387/#2388/#2391/#2395 did.
Gate
bun run typecheck(monorepo root) — 17/17bun run lint— 15/15Tests drive fakes for
RTCPeerConnection/getUserMedia/fetch. No live network, no real microphone.Base:
pu/gpt-realtime.🤖 Generated with Claude Code
https://claude.ai/code/session_01PhbBndG131JyacZCqXrF5w