Uh oh!
There was an error while loading. Please reload this page.
feat: add persistent OpenAI Realtime voice panel - #3997
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| scheduler: configScheduler, | ||
| concurrency: configConcurrency, | ||
| }), | ||
| setVoiceCredential: createEnvironmentRpcCommand(runtime, { |
There was a problem hiding this comment.
🟡 Mediumstate/server.ts:346
setVoiceCredential and removeVoiceCredential pass configConcurrency (serial mode keyed by environmentId) but omit the shared configScheduler. Because each command creates its own scheduler, the serial key does not serialize the two commands against each other — a rapid set-then-remove sequence can execute both RPCs concurrently, so the final stored credential depends on completion order rather than invocation order. Pass configScheduler to both commands, as updateSettings and the other config mutation commands do.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/server.ts around line 346:
`setVoiceCredential` and `removeVoiceCredential` pass `configConcurrency` (serial mode keyed by `environmentId`) but omit the shared `configScheduler`. Because each command creates its own scheduler, the serial key does not serialize the two commands against each other — a rapid set-then-remove sequence can execute both RPCs concurrently, so the final stored credential depends on completion order rather than invocation order. Pass `configScheduler` to both commands, as `updateSettings` and the other config mutation commands do.
| setAssistantTranscript(""); | ||
| }, []); | ||
| const start = useCallback(() => { |
There was a problem hiding this comment.
🟠 Highvoice/VoiceSession.tsx:355
If start() is called again after end() but before the pending createVoiceSession resolves, the first call's accessResult sees activeRef.current === true (set by the second call) and proceeds to create its own socket and VoiceAudioController, overwriting socketRef and audioRef with the stale session's instances. The second session's socket and audio controller are then orphaned — the socket stays open and the microphone keeps capturing audio — while either session's close handler can deactivate the other. Use a per-start generation token and discard results, socket handlers, and audio teardown that don't match the current generation.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/voice/VoiceSession.tsx around line 355:
If `start()` is called again after `end()` but before the pending `createVoiceSession` resolves, the first call's `accessResult` sees `activeRef.current === true` (set by the second call) and proceeds to create its own socket and `VoiceAudioController`, overwriting `socketRef` and `audioRef` with the stale session's instances. The second session's socket and audio controller are then orphaned — the socket stays open and the microphone keeps capturing audio — while either session's `close` handler can deactivate the other. Use a per-start generation token and discard results, socket handlers, and audio teardown that don't match the current generation.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| useEffect(() => { | ||
| if (queriedStatus) setConfigured(queriedStatus.configured); | ||
| }, [queriedStatus]); |
There was a problem hiding this comment.
🟡 Mediumsettings/VoiceSettingsPanel.tsx:54
When environmentId changes, configured keeps the old environment's value until the new environment's status finishes loading, so the Test and Remove buttons stay enabled for the new environment before its credential status is known. Clicking Remove deletes the new environment's credential based on stale state from a different environment. The useEffect only sets configured when queriedStatus is non-null, so a loading or failed query leaves the previous value in place. Consider resetting configured to false whenever environmentId changes, or deriving the controls directly from queriedStatus instead of mirroring it in local state.
- useEffect(() => {- if (queriedStatus) setConfigured(queriedStatus.configured);- }, [queriedStatus]);+ useEffect(() => {+ setConfigured(queriedStatus?.configured ?? false);+ }, [queriedStatus]);🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/VoiceSettingsPanel.tsx around lines 54-56:
When `environmentId` changes, `configured` keeps the old environment's value until the new environment's status finishes loading, so the Test and Remove buttons stay enabled for the new environment before its credential status is known. Clicking Remove deletes the new environment's credential based on stale state from a different environment. The `useEffect` only sets `configured` when `queriedStatus` is non-null, so a loading or failed query leaves the previous value in place. Consider resetting `configured` to `false` whenever `environmentId` changes, or deriving the controls directly from `queriedStatus` instead of mirroring it in local state.
| ), | ||
| })); | ||
| }, | ||
| clearHistory: () => set({ sessions: [], activeSessionId: null }), |
There was a problem hiding this comment.
🟡 Mediumvoice/voiceTraceStore.ts:153
clearHistory deletes the currently active session along with completed history. While a voice session is live, the settings page exposes this action, and clearing it discards the active traceSessionIdRef without ending the session. Every subsequent appendEntry and completeSession call silently finds no matching session, so the live timeline and the final trace are lost. Preserve the active session (or end/reset it consistently) when clearing history.
| clearHistory: ()=>set({sessions: [],activeSessionId: null}), | |
| clearHistory: ()=> | |
| set((state)=>({ | |
| sessions: state.activeSessionId | |
| ? state.sessions.filter((s)=>s.id===state.activeSessionId) | |
| : [], | |
| activeSessionId: state.activeSessionId, | |
| })), |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/voice/voiceTraceStore.ts around line 153:
`clearHistory` deletes the currently active session along with completed history. While a voice session is live, the settings page exposes this action, and clearing it discards the active `traceSessionIdRef` without ending the session. Every subsequent `appendEntry` and `completeSession` call silently finds no matching session, so the live timeline and the final trace are lost. Preserve the active session (or end/reset it consistently) when clearing history.
There was a problem hiding this comment.
Effect Service Conventions
One convention issue found in the new voice service error model: VoiceApiError cannot carry an underlying cause, so every wrapping site discards the real failure and its stack. This diverges from the repo-wide convention (SecretStoreError, and the filesystem/project/vcs/assets contracts) where service errors preserve the immediate failure as cause: Schema.Defect() and derive stable messages from structured attributes.
Posted via Macroscope — Effect Service Conventions
| export class VoiceApiError extends Schema.TaggedErrorClass<VoiceApiError>()("VoiceApiError", { | ||
| reason: VoiceApiErrorReason, | ||
| message: Schema.String, | ||
| }) {} |
There was a problem hiding this comment.
VoiceApiError has no cause field, so every translation boundary in VoiceSessionService (secretStoreFailure, the HTTP request/response mapErrors, and the schemaBodyJson decode mapError in createSession) throws away the underlying SecretStoreError / HTTP / decode failure and its stack. The convention is to preserve the immediate underlying error as cause alongside the structural fields, matching the rest of the contracts package (e.g. SecretStoreError, filesystem.ts, project.ts).
Add an optional cause (optional because the pure credential_invalid empty-key validation has no underlying failure):
| exportclassVoiceApiErrorextendsSchema.TaggedErrorClass<VoiceApiError>()("VoiceApiError",{ | |
| reason: VoiceApiErrorReason, | |
| message: Schema.String, | |
| }){} | |
| exportclassVoiceApiErrorextendsSchema.TaggedErrorClass<VoiceApiError>()("VoiceApiError",{ | |
| reason: VoiceApiErrorReason, | |
| message: Schema.String, | |
| cause: Schema.optional(Schema.Defect()), | |
| }){} |
Posted via Macroscope — Effect Service Conventions
| function secretStoreFailure(): VoiceApiError { | ||
| return new VoiceApiError({ | ||
| reason: "secret_store_failed", | ||
| message: "T3 Code could not access the saved xAI voice credential.", | ||
| }); | ||
| } |
There was a problem hiding this comment.
This mapper drops the underlying SecretStoreError when translating to VoiceApiError, losing the error chain. Once VoiceApiError accepts a cause, forward the incoming error (the existing Effect.mapError(secretStoreFailure) call sites already pass it):
| functionsecretStoreFailure(): VoiceApiError{ | |
| returnnewVoiceApiError({ | |
| reason: "secret_store_failed", | |
| message: "T3 Code could not access the saved xAI voice credential.", | |
| }); | |
| } | |
| functionsecretStoreFailure(cause: unknown): VoiceApiError{ | |
| returnnewVoiceApiError({ | |
| reason: "secret_store_failed", | |
| message: "T3 Code could not access the saved xAI voice credential.", | |
| cause, | |
| }); | |
| } |
Apply the same cause forwarding to the upstream_unavailable / credential_invalid mappers in createSession.
Posted via Macroscope — Effect Service Conventions
| const socket = socketRef.current; | ||
| if (!socket || socket.readyState !== WebSocket.OPEN) return; | ||
| const calls = toolQueueRef.current.splice(0); | ||
| for (const call of calls) { |
There was a problem hiding this comment.
🟡 Mediumvoice/VoiceSession.tsx:372
If the WebSocket closes during the 50 ms tool-call batching window, flushToolCalls bails early because the socket isn't open but never clears toolQueueRef.current. The socket close handler also doesn't drain the queue. When a new session starts on a fresh socket, the stale queued calls from the previous session are flushed and executed against the current composer, and their outputs are sent on the new socket. Clear the queue in flushToolCalls before the socket check, and in end and the socket close handler.
const flushToolCalls = useCallback(() => {
toolTimerRef.current = null;
- const socket = socketRef.current;- if (!socket || socket.readyState !== WebSocket.OPEN) return;
const calls = toolQueueRef.current.splice(0);
+ const socket = socketRef.current;+ if (!socket || socket.readyState !== WebSocket.OPEN) return;🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/voice/VoiceSession.tsx around lines 372-375:
If the WebSocket closes during the 50 ms tool-call batching window, `flushToolCalls` bails early because the socket isn't open but never clears `toolQueueRef.current`. The socket `close` handler also doesn't drain the queue. When a new session starts on a fresh socket, the stale queued calls from the previous session are flushed and executed against the current composer, and their outputs are sent on the new socket. Clear the queue in `flushToolCalls` before the socket check, and in `end` and the socket `close` handler.
Uh oh!
There was an error while loading. Please reload this page.
| http_status_code: Schema.optionalKey(Schema.Number), | ||
| content: Schema.String, |
There was a problem hiding this comment.
🟡 Mediumvoice/VoiceSessionService.ts:55
ParallelExtractResponse defines http_status_code as Schema.Number and content as Schema.String, but the Parallel /v1/extract API returns null for these fields when no HTTP status or body is available. A successful extract response containing such a per-URL error fails schema validation and is converted into web_tool_unavailable, discarding all successful results. Both fields should accept null (e.g. Schema.NullOr(Schema.Number) and Schema.NullOr(Schema.String)), and the content formatting at line 391 should handle null.
| http_status_code: Schema.optionalKey(Schema.Number), | |
| content: Schema.String, | |
| http_status_code: Schema.optionalKey(Schema.NullOr(Schema.Number)), | |
| content: Schema.NullOr(Schema.String), |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/voice/VoiceSessionService.ts around lines 55-56:
`ParallelExtractResponse` defines `http_status_code` as `Schema.Number` and `content` as `Schema.String`, but the Parallel `/v1/extract` API returns `null` for these fields when no HTTP status or body is available. A successful extract response containing such a per-URL error fails schema validation and is converted into `web_tool_unavailable`, discarding all successful results. Both fields should accept `null` (e.g. `Schema.NullOr(Schema.Number)` and `Schema.NullOr(Schema.String)`), and the `content` formatting at line 391 should handle `null`.
| storage: createJSONStorage(() => | ||
| resolveStorage(typeof window === "undefined" ? undefined : window.localStorage), | ||
| ), |
There was a problem hiding this comment.
🟡 Mediumvoice/voiceSettingsStore.ts:199
The storage factory passes window.localStorage directly into resolveStorage, so when the localStorage property access itself throws (hardened browsers / unavailable storage — the case this file explicitly intends to support), initializing useVoiceSettingsStore throws and can prevent the web UI from loading. resolveStorage only provides its memory fallback when it receives undefined, but it never gets that chance because the exception happens during argument evaluation. Wrap the property access in try/catch (or a safe accessor) so the factory returns undefined on failure.
+ storage: createJSONStorage(() => {+ try {+ return resolveStorage(typeof window === "undefined" ? undefined : window.localStorage);+ } catch {+ return resolveStorage(undefined);+ }+ }),Also found in 1 other location(s)
apps/web/src/components/voice/voiceTraceStore.ts:203
The storage factory evaluates
window.localStoragebeforeresolveStoragecan fall back to memory storage. If the browser exposeswindowbut throws when thelocalStorageproperty is accessed (for example in a hardened storage context), initialization ofuseVoiceTraceStorethrows and can prevent the voice UI from loading. The property access itself needs to be guarded.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/voice/voiceSettingsStore.ts around lines 199-201:
The storage factory passes `window.localStorage` directly into `resolveStorage`, so when the `localStorage` property access itself throws (hardened browsers / unavailable storage — the case this file explicitly intends to support), initializing `useVoiceSettingsStore` throws and can prevent the web UI from loading. `resolveStorage` only provides its memory fallback when it receives `undefined`, but it never gets that chance because the exception happens during argument evaluation. Wrap the property access in `try/catch` (or a safe accessor) so the factory returns `undefined` on failure.
Also found in 1 other location(s):
- apps/web/src/components/voice/voiceTraceStore.ts:203 -- The storage factory evaluates `window.localStorage` before `resolveStorage` can fall back to memory storage. If the browser exposes `window` but throws when the `localStorage` property is accessed (for example in a hardened storage context), initialization of `useVoiceTraceStore` throws and can prevent the voice UI from loading. The property access itself needs to be guarded.
| storage: createJSONStorage(() => | ||
| resolveStorage(typeof window === "undefined" ? undefined : window.localStorage), | ||
| ), | ||
| partialize: (state) => ({ sessions: state.sessions }), |
There was a problem hiding this comment.
🟡 Mediumvoice/voiceTraceStore.ts:205
After a page reload, persisted sessions with status: "active" remain stuck in that state forever. The partialize option persists sessions but not activeSessionId, and nothing on hydration finalizes sessions left active by a reload or crash, so completeSession is never called for them. They display a green active indicator indefinitely even though no voice connection exists. Consider marking previously-active sessions as error (or completed) during rehydration, for example in an onRehydrateStorage callback.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/voice/voiceTraceStore.ts around line 205:
After a page reload, persisted sessions with `status: "active"` remain stuck in that state forever. The `partialize` option persists sessions but not `activeSessionId`, and nothing on hydration finalizes sessions left active by a reload or crash, so `completeSession` is never called for them. They display a green active indicator indefinitely even though no voice connection exists. Consider marking previously-active sessions as `error` (or `completed`) during rehydration, for example in an `onRehydrateStorage` callback.
There was a problem hiding this comment.
Effect Service Conventions review: one error-modeling convention violation in the new voice contract/service code. See inline comment.
Posted via Macroscope — Effect Service Conventions
| export class VoiceApiError extends Schema.TaggedErrorClass<VoiceApiError>()("VoiceApiError", { | ||
| reason: VoiceApiErrorReason, | ||
| message: Schema.String, | ||
| }) {} |
There was a problem hiding this comment.
VoiceApiError stores a free-form message: Schema.String as its payload. Every other tagged error in this package derives message from structured attributes via an override get message() getter (e.g. SecretStore*Error, GitCommandError, ExternalLauncher*Error), so the user-facing string is a stable function of the error shape rather than hand-written at each new VoiceApiError({ reason, message }) call site.
Since reason already carries the distinction and each reason feeds a distinct message, derive the message from reason (plus any safe structural context) in a getter instead of storing it. If some reasons need genuinely different caller control flow or messages, split those into separate error classes, per the convention on modeling distinct failures.
Posted via Macroscope — Effect Service Conventions
t3dotgg
commented
Aug 23, 2026
Note 🤖 GPT-5.6 Sol responding on behalf of Theo Closing this PR after an automated pass over open pull requests. Duplicates the maintainer-owned voice implementation in #5213. |
Squashed from PR pingdotgg#3997 (9 commits; adds then replaces the Grok voice layer with OpenAI Realtime over WebRTC, gpt-realtime-2.1-mini default). - Server: authenticated voice.* RPCs (session, credential, Parallel web tools) implemented by VoiceSessionService with server-side key storage and short-lived OpenAI client secrets. - Web: persistent VoiceSession panel + trace timeline, composer tools (voice can draft but never send), /settings/voice settings page. - Desktop: 'T3 Code Voice' packaged variant with isolated state dir. Fork merge notes: adopted fork's rewrites (settingsSearch-driven nav, DesktopStatePaths helpers, single RPC_REQUIRED_SCOPES map with voice scopes registered there instead of the PR's dead duplicate map).
Summary
gpt-realtime-2.1-minias the default andgpt-realtime-2.1as the stronger selectable modelRealtime behavior
search_webwaits for Parallel Search;extract_web_pagesis available for selected URLs when ranked excerpts are insufficientSecurity and migration
Validation
vp checkpasses with 9 pre-existing React warnings in unrelated filesvp run typecheckpasses/Applications/T3 Code Voice.appis ad-hoc signed and passes strict deep signature verificationReferences