Skip to content

fix(voice): defer OpenAI Keychain access - #292

Merged
johnmatthewtennant merged 6 commits into
mainfrom
jt/openai-voice-keychain-gating
Sep 3, 2026
Merged

fix(voice): defer OpenAI Keychain access#292
johnmatthewtennant merged 6 commits into
mainfrom
jt/openai-voice-keychain-gating

Conversation

@johnmatthewtennant

Copy link
Copy Markdown
Contributor

Summary

  • Read the shared OpenAI voice credential only when the selected Voice settings need it, while keeping OpenAI TTS selectable before that read.
  • Coalesce concurrent mount-time status requests and force fresh status reads after credential changes.
  • Use only Berd’s shared berd-openai-voice / api-key credential, without legacy account lookup or migration.

Reviewer-reproducible examples

  • Open Voice settings with the default on-device chained configuration. Berd does not inspect the OpenAI Keychain credential.
  • Select OpenAI transcription, OpenAI playback, or OpenAI Realtime. The corresponding settings load the shared credential status.
  • Save or clear the shared key. Every mounted OpenAI voice surface refreshes to the new status, while simultaneous mount reads share one in-flight request.

@johnmatthewtennant
johnmatthewtennant marked this pull request as ready for review September 3, 2026 19:30
@johnmatthewtennant
johnmatthewtennant requested a review from a team September 3, 2026 19:30

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

Engineering found one blocking credential-refresh race. The supplied GitHub evidence is structurally valid: Semgrep, zizmor, DCO, clean-room install, and Linux Rust checks passed, while frontend and macOS/Windows Rust checks were in progress when captured; required checks still govern merge readiness.

Deterministic publication result: 1 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

Comment thread src/features/voice-conversation/hooks/useOpenAiVoiceSetup.ts

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

Engineering completed a fresh static review of the full exact PR comparison and found no publishable issues. The previously reported credential-refresh race is fixed at this head and its resolved thread has a substantive human reply. Supplied GitHub evidence is structurally valid, and all captured check runs completed successfully; required checks still govern merge readiness.

Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

Pending checks: 1 check(s) are not complete.

This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.

@johnmatthewtennant
johnmatthewtennant merged commit b5d8f2f into main Sep 3, 2026
10 checks passed
@johnmatthewtennant
johnmatthewtennant deleted the jt/openai-voice-keychain-gating branch September 3, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants