Skip to content

fix(tui): skip audio init when no playback device - #51854

Closed
aydemir wants to merge 3 commits into
anomalyco:devfrom
aydemir:alsa-device-guard-v2
Closed

aydemir wants to merge 3 commits into
anomalyco:devfrom
aydemir:alsa-device-guard-v2

Conversation

@aydemir

@aydemir aydemir commented Sep 28, 2026 •

Copy link
Copy Markdown

Issue for this PR

Closes #41763

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

On Linux hosts without an audio playback device (Termux/Android, soundless servers), any attention sound makes the native backend print ALSA diagnostics to stderr, overwriting the TUI.

I added a probe (new packages/tui/src/audio-device.ts) that looks for a PCM playback node under /dev/snd before Audio.create runs. If none exists, audio is skipped with a debug line and nothing touches the native backend. Non-Linux behavior is unchanged, and nothing changes when a device is present. The notification leg of attention.notify is untouched — only the sound leg can now come back false early.

How did you verify your code works?

  • New packages/tui/test/audio-device.test.ts: playback detect, capture/mixer-only reject, unreadable /dev/snd, non-Linux skip — 5/5 standalone.
  • End to end on an audio-less aarch64 host playing a real bundled mp3 through the TUI audio path: before 36 ALSA lines (same signature as tui: ALSA errors flood and corrupt the terminal during interaction #41763), after 0.
  • Live decision table with a stub renderer and the real audio module: blurred gives notification:true/sound:false, focused gives notification:false (by design)/sound:false, both with 0 ALSA lines.
  • Field check on Termux (Android 16) with the patched binary and sounds on: no ghost text across tool calls.

Screenshots / recordings

Not a UI change — the fix removes stray stderr text. No screenshot applies.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions github-actions Bot added needs:compliance This means the issue will auto-close after 2 hours. needs:issue labels Sep 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@aydemir

aydemir commented Sep 28, 2026

Copy link
Copy Markdown
Author

Field confirmation on Termux (Android 16), patched v2 binary, sounds ON: tool calls complete with no ALSA ghost text. Note: no toast either, which matches the notify path (done-sound is when:always, visual notification is when:blurred, TUI was focused) — so silence here is the correct outcome, not a regression.

@aydemir

aydemir commented Sep 28, 2026

Copy link
Copy Markdown
Author

Live decision-table check on an audio-less host (stub renderer, real audio module): blurred -> {notification: true, sound: false}, 0 ALSA lines; focused -> {notification: false (by design), sound: false}, 0 ALSA lines. So the patch only ever removes the sound leg; the toast leg is untouched.

@github-actions github-actions Bot removed the needs:compliance This means the issue will auto-close after 2 hours. label Sep 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@aydemir

aydemir commented Sep 29, 2026

Copy link
Copy Markdown
Author

Quick note on the needs:issue label, since it looks like this PR failed the issue-first check. It didn't.

This PR intentionally targets v2, which is where most merged PRs go — 24 of the last 30 merged PRs target v2, vs 3 for dev. v2 is the active line, and this bug affects current users.

The label comes from check-standards, which resolves the issue link through the GraphQL closingIssuesReferences field. That field only resolves when a PR targets the repository's default branch (dev). On a v2-targeted PR it returns 0 even when the body contains Closes #41763 — the link is real (it appears as a cross-reference on the issue), the query just can't see it.

So on v2, the fix/chore/test/docs issue check can't be satisfied structurally. Verified against open PRs: 52037 (dev) returns 1 reference, while 51854, 52038, 52012 and 52040 (all v2) return 0.

Flagging this so the label isn't read as a problem with the PR itself. needs:issue doesn't block merge either — only needs:compliance does, and that one is already cleared.

On the change: it's self-contained — a /dev/snd probe for a PCM playback node before Audio.create runs, so the native backend is never touched on audio-less hosts. Non-Linux and device-present paths are unchanged, and only the sound leg of attention.notify is affected; the notification leg is untouched.

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.

tui: ALSA errors flood and corrupt the terminal during interaction

1 participant