Skip to content

feat(discord-voice): push bridge-delivered text into the active voice session - #1030

Closed
sonichi wants to merge 1 commit into
mainfrom
feat/discord-voice-text-push
Closed

sonichi wants to merge 1 commit into
mainfrom
feat/discord-voice-text-push

Conversation

@sonichi

@sonichi sonichi commented May 22, 2026

Copy link
Copy Markdown
Owner

The gap

The Discord bridge already pushes image attachments into a live discord-voice (Gemini Live) session — _push_vision_image / src/vision_push.py, called from the on-message attachment handler. It does not push text.

discord-voice-server.ts has no messageCreate handler — it never reads text posted in its voice channel. So any message the bridge delivers into a voice channel — including a results/ body routed there via the [channel: <id>] redirect marker — is never seen by the voice agent.

The change

Mirrors the image path exactly:

  • src/voice_text_push.py — new helper analogous to vision_push.py. urllib POST to a localhost control server: GET /voice/state for liveness + which channel is active, POST /voice/text to inject. Fail-soft on every error path (URLError / ConnectionRefused / non-200) — returns False, never raises. No-op when no session is active for the target channel (channel-match gate, on top of vision_push.py's is_voice_ready-equivalent check).

  • discord-voice-server.ts — adds a tiny localhost-only text control server (port 7848, env DISCORD_VOICE_CONTROL_PORT), mirroring vision-tools.ts's startVisionControlServer: localhost-bound, JSON, fail-soft on EADDRINUSE (another server owns the port). /voice/text injects via transport.sendContent([{role:'user', text}], false) — the same mechanism vision-tools.ts uses for screen-share context hints and the discord-voice resultQueue drain uses for task results. turnComplete=false → context-only, no forced generation. Started after the session is live, closed on shutdown.

    (The discord-voice-server runs its own VoiceSession; it does not — and the vision-tools control server on :7847 is owned by voice-agent.ts, not discord-voice-server — so a matching control server on the discord-voice side is the transport that lets the bridge reach a discord-voice session. transport.sendContent is the proven text-inject path the session already uses internally.)

  • discord-bridge.py — wires _push_voice_text into both outbound delivery paths (poll_results normal/redirected delivery; poll_dm_fallback channel-redirect) after the message is sent to the channel. The trigger case: a results/ body routed via [channel: <voice-channel-id>] to a channel where discord-voice is running — the text lands in the voice agent's session.

Fail-soft / additive

Purely additive — a channel without an active voice session is delivered exactly as before. A push failure (no session, WS error, server down, port unbound) returns False and is wrapped in try/except at the call site; it can never break message delivery or crash the bridge — exactly like the image path.

Verification

  • python3 -m py_compile src/discord-bridge.py src/voice_text_push.py — clean.
  • npm run typecheck — passes.
  • Helper fail-soft confirmed with no server listening (push_text → False, no exception).
  • End-to-end against a mock control server matching the /voice/state + /voice/text contract: matching-channel push delivers, non-matching-channel is a no-op, sessionReady=false is a no-op, int and str channel ids both work.
  • discord-bridge test suite passes (the one unrelated slack-bridge-allowlist failure is pre-existing — slack-bridge.py uses str | None on Python 3.9; confirmed identical on main).

🤖 Generated with Claude Code

… session

The Discord bridge already pushes image attachments into a live discord-voice
session (_push_vision_image / vision_push.py) but not text. discord-voice-server
has no messageCreate handler, so any message the bridge delivers into a voice
channel — including a results/ body routed there via [channel: <id>] — is never
seen by the voice agent.

This closes that gap, mirroring the image path:

- src/voice_text_push.py — new helper analogous to vision_push.py. urllib POST
  to a localhost control server, GET /voice/state for liveness + which channel
  is active, POST /voice/text to inject. Fail-soft on every error path
  (URLError / ConnectionRefused / non-200) — a push failure returns False, never
  raises. No-op when no session is active for the target channel.

- discord-voice-server.ts — adds a tiny localhost-only text control server
  (port 7848, env DISCORD_VOICE_CONTROL_PORT), mirroring vision-tools.ts's
  startVisionControlServer: localhost-bound, JSON, fail-soft on EADDRINUSE.
  /voice/text injects via transport.sendContent([{role:'user',text}], false) —
  the same mechanism vision-tools uses for context hints and the resultQueue
  drain uses for task results. turnComplete=false → context-only, no forced
  generation. Started after the session is live, closed on shutdown.

- discord-bridge.py — wires _push_voice_text into both outbound delivery paths
  (poll_results normal/redirected delivery, poll_dm_fallback channel-redirect)
  after the message is sent to the channel. The trigger case: a results/ body
  routed via [channel: <voice-channel-id>] to a channel running discord-voice.

Purely additive: channels without an active voice session are unchanged. A push
failure (no session, WS error, server down) never breaks message delivery.

Verified: py_compile clean on both .py files; npm run typecheck passes; helper
fail-soft confirmed with no server listening; end-to-end against a mock control
server confirms matching-channel push, non-matching-channel no-op, and
sessionReady=false no-op. discord-bridge test suite passes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@liususan091219 liususan091219 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.

Reviewed the bridge-text → voice-session push. Well-architected — it mirrors the proven image-push path rather than inventing a new mechanism, and it's genuinely fail-soft. Mergeable; two non-blocking points worth a look.

Things I like:

  1. It mirrors the vision_push.py / vision-tools control-server architecture exactly — same localhost-HTTP shape, same fail-soft contract. Reusing a proven pattern rather than a bespoke one keeps the discord-voice integration surface coherent.
  2. Fail-soft is airtight on every layer: voice_text_push.push_text returns False on URLError/ConnectionRefused/non-200, the bridge call sites in poll_results / poll_dm_fallback are wrapped in try/except, and injectVoiceText guards !s || s.closing, missing transport, non-function sendContent, and isConnected === false before a try/catch. A push failure can never break channel delivery — exactly the property the image path already has.
  3. turnComplete=false is the right call — pushed text lands as conversation context, not a forced generation. Matches how vision-tools injects screen-share hints and how the resultQueue drain injects task results.
  4. The channel-match gate in push_text (active != target → no-op) means text only injects into a session actually serving that channel — no cross-channel leakage.
  5. EADDRINUSE handling leaves voiceControlServer = null so shutdown never closes a port another process owns. That's the correct defensive choice and it's commented.
  6. Port choice is documented inline (7845 screen-capture / 7846 cred-proxy / 7847 vision-control / 7848 free) and env-overridable.

Non-blocking observations:

  1. No committed test. The PR body describes solid manual verification — fail-soft with no server, end-to-end against a mock control server, channel-match/non-match, sessionReady=false, int vs str channel ids. That's exactly the behavior a committed test should pin. The sibling security PRs in this batch (#1029, #1023, #1022) all ship tests; a small tests/voice-text-push.test.py codifying the /voice/state + /voice/text contract and the channel-match gate would protect this from a silent regression. Worth adding.
  2. Fixed port vs. multi-instance. VOICE_CONTROL_PORT defaults to 7848 and voice_text_push.py reads the same DISCORD_VOICE_CONTROL_PORT env. On main (single discord-voice process) that's correct. But if two discord-voice-server processes ever run on one host, the second hits EADDRINUSE and skips its server — and the bridge can only ever reach whichever instance grabbed 7848; the other's channel gets a silent no-op. Not a bug for main, but the multi-instance discord-voice work exists on a branch, so flagging the linkage: a per-instance port (and a way for the bridge to pick the right one per channel) would be needed there.

Other notes:

  • (s.voiceSession as any)?.transport reaches into bodhi internals via as any — but it's the same internal-access pattern vision-tools and the resultQueue drain already use, and the typeof sendContent !== 'function' guard degrades gracefully if bodhi ever renames the field. Acceptable.
  • The unauthenticated localhost-bound control server is the same trust model as the :7847 vision server and :7845 screen-capture — consistent, no new exposure.

Cross-PR: sits alongside #1026 (operational-log tee) and #1027 (health probe) — three PRs all closing the "discord-voice is under-integrated vs the other surfaces" gap. #1026 and this PR both insert a block into discord-voice-server.ts near the cleanup section; whichever merges second should expect a trivial textual rebase.

Solid, additive, correctly fail-soft. The test gap (obs 1) is the one thing I'd genuinely like closed before merge. COMMENT.
— Lucy (Mac Studio bot)

@sonichi

sonichi commented May 22, 2026

Copy link
Copy Markdown
Owner Author

Superseded — owner pivoted the za-warudo context handoff to a per-channel results/ design (filename-encode the channel; each surface, including discord-voice and phone, filters results/ by its own channel key). That gives discord-voice a native pull path, retiring this text-push approach. Branch feat/discord-voice-text-push left intact for reference.

@sonichi sonichi closed this May 22, 2026
bassilkhilo-ag2 added a commit to bassilkhilo-ag2/sutando that referenced this pull request May 26, 2026
…dirs

When the same skill directory name (e.g. screen-companion) exists in both
the repo skills/ and workspace skills/, both were appended into the tool
list, triggering assertUniqueToolNames and crashing the voice session at
startup. Fixes sonichi#1030.

Replace bare arrays with Maps keyed by dirName so later directories
(workspace, private) silently overwrite earlier ones (repo) — the same
last-write-wins semantics loadCoreDocumentedSkills already used.

5 tests in tests/skill-loader-dedup.test.ts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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