Merge PR #6421: fix(tts): play buffered audio when streaming backend exits nonzero (@roboomp)

packages/coding-agent/CHANGELOG.md

@@ -35,6 +35,7 @@
 - Fixed startup status messages (warnings, errors, extension/tool errors, status lines) keeping the dark-mode color after auto-theme detection later switched the active theme to light — e.g. `dark-catppuccin`/`light-catppuccin` warnings rendered in Mocha yellow on the Latte background. Transient status presenters now resolve their color lazily at render time so a theme swap re-shapes them ([#6337](https://github.com/can1357/oh-my-pi/issues/6337)).
 - Fixed MCP OAuth endpoint discovery and Smithery browser-login polling hanging indefinitely against an endpoint that accepts the TCP connection but never responds. `discoverOAuthEndpoints`/`fetchResourceMetadataScopes` now bound every metadata/well-known/authorization-server fetch with a per-request `AbortSignal.timeout`, and `pollSmitheryCliAuthSession` bounds each poll so the loop reaches its 5-minute deadline instead of stalling ([#4103](https://github.com/can1357/oh-my-pi/issues/4103)).
 - Fixed bash internal-URL expansion skipping unquoted `skill://` (and other supported schemes) inside a legacy backtick command substitution nested directly in double quotes (e.g. ``echo "`cat skill://valid-skill/SKILL.md`"``); `isInsideShellQuote` now treats `` ` `` as an expansion-context boundary like `$()`, including `$()`/backtick nesting in either order, while single-quoted and escaped-backtick text stay literal ([#5645](https://github.com/can1357/oh-my-pi/issues/5645)).
+- Fixed `omp say` playing no audio for a short single-segment clip on hosts where the first streaming backend (the bundled ffmpeg built without pulse/alsa output) spawns then exits nonzero: the pipe write succeeds before that death and `player.end()` has already closed the input, so neither the broken-pipe replay nor the early-exit handler advanced to `paplay`/`aplay`. `StreamingAudioPlayer` now retains the utterance PCM and, when the streaming backend exits nonzero, replays it through per-file playback so short clips still reach the speakers ([#5875](https://github.com/can1357/oh-my-pi/issues/5875)).
 - Fixed `error.notify` raising a "Stopped with error" toast for provider failures while an auto-retry or async-delivery continuation was pending; the toast now waits for the true terminal settle.
 - Fixed concurrent MCP config mutations losing updates and racing on a shared temp path: every `mcp.json` read-modify-write (add/update/remove server, disabled/force-enabled lists) is now serialized under a per-file lock, and each atomic write uses a unique temp file so overlapping writers no longer rename each other's `.tmp` out from under them (ENOENT or clobbered config) — reachable in-process via the fire-and-forget extensions-dashboard toggle and across processes on a shared `~/.omp/mcp.json` ([#4104](https://github.com/can1357/oh-my-pi/issues/4104)).
 - Fixed terminal `yield` results racing post-turn maintenance, which could trigger an unnecessary automatic handoff or compaction.

packages/coding-agent/src/tts/streaming-player.ts

@@ -89,6 +89,18 @@ export function streamingPlayerCommandsFor(
 	return commands;
 }
 
+/**
+ * Test seams for {@link StreamingAudioPlayer}: override backend discovery and
+ * the per-file fallback so playback logic can be exercised without a real audio
+ * device. Both default to the platform lookup and {@link playAudioFile}.
+ */
+export interface StreamingPlayerOptions {
+	/** Ordered backend commands for a sample rate; defaults to {@link streamingPlayerCommandsFor}. */
+	commandsFor?: (sampleRate: number) => PlayerCommand[];
+	/** Per-file fallback playback; defaults to {@link playAudioFile}. */
+	playAudio?: (wavPath: string, signal: AbortSignal) => Promise<void>;
+}
+
 /**
  * Single-session gapless player. Lifecycle: {@link start} once, {@link write}
  * chunks in order, then {@link end} to drain or {@link stop} to abort. Not
@@ -111,6 +123,15 @@ export class StreamingAudioPlayer {
 	#abortController = new AbortController();
 	#wake: (() => void) | null = null;
 	#drain: Promise<void> = Promise.resolve();
+	readonly #commandsFor: (sampleRate: number) => PlayerCommand[];
+	readonly #playAudio: (wavPath: string, signal: AbortSignal) => Promise<void>;
+	/** Streamed PCM retained for this utterance so a failed backend can be replayed via file playback. */
+	#played: Float32Array[] = [];
+
+	constructor(options: StreamingPlayerOptions = {}) {
+		this.#commandsFor = options.commandsFor ?? (rate => streamingPlayerCommandsFor(process.platform, rate));
+		this.#playAudio = options.playAudio ?? ((wavPath, signal) => playAudioFile(wavPath, { signal }));
+	}
 
 	/** Pick a backend and begin draining. Idempotent; the first call's rate wins. */
 	start(sampleRate: number): void {
@@ -146,6 +167,7 @@ export class StreamingAudioPlayer {
 		if (this.#stopped) return;
 		this.#stopped = true;
 		this.#queue.length = 0;
+		this.#played.length = 0;
 		this.#abortController.abort();
 		this.#signal();
 		try {
@@ -168,7 +190,7 @@ export class StreamingAudioPlayer {
 	 * in-flight chunk.
 	 */
 	#spawnStream(): boolean {
-		this.#candidates ??= streamingPlayerCommandsFor(process.platform, this.#sampleRate);
+		this.#candidates ??= this.#commandsFor(this.#sampleRate);
 		for (let command = this.#candidates.shift(); command; command = this.#candidates.shift()) {
 			const { cmd, args } = command;
 			try {
@@ -213,6 +235,7 @@ export class StreamingAudioPlayer {
 					continue;
 				}
 				if (this.#mode === "stream") {
+					this.#played.push(chunk);
 					// Pace writes so the player buffers ~LEAD_SECONDS, no more, keeping
 					// ducking and stop responsive instead of locked behind buffered audio.
 					const ahead = this.#writtenSec - (performance.now() - this.#startedAt) / 1000;
@@ -242,11 +265,27 @@ export class StreamingAudioPlayer {
 				try {
 					await this.#sink?.end();
 				} catch {}
-				if (this.#proc) {
+				const proc = this.#proc;
+				let exitCode: number | null = null;
+				if (proc) {
 					try {
-						await this.#proc.exited;
+						exitCode = await proc.exited;
 					} catch {}
 				}
+				// A streaming backend that exits nonzero never opened its audio
+				// device (e.g. the bundled ffmpeg built without pulse/alsa output).
+				// For a short single-segment clip the pipe write succeeds before
+				// that death and #inputClosed is already set, so neither the
+				// broken-pipe replay nor the early-exit handler advances backends.
+				// Replay the buffered utterance through per-file playback so it
+				// still reaches the speakers.
+				if (!this.#stopped && proc && exitCode !== 0) {
+					this.#mode = "file";
+					for (const chunk of this.#played) {
+						if (this.#stopped) break;
+						await this.#playFile(chunk);
+					}
+				}
 			}
 		} catch (error) {
 			logger.debug("tts: streaming player drain failed", {
@@ -291,7 +330,7 @@ export class StreamingAudioPlayer {
 		const wavPath = path.join(os.tmpdir(), `omp-speech-${Snowflake.next()}.wav`);
 		try {
 			await fs.writeFile(wavPath, encodeWav(this.#scaled(pcm), this.#sampleRate));
-			if (!this.#stopped) await playAudioFile(wavPath, { signal: this.#abortController.signal });
+			if (!this.#stopped) await this.#playAudio(wavPath, this.#abortController.signal);
 		} catch (error) {
 			logger.debug("tts: file playback failed", {
 				error: error instanceof Error ? error.message : String(error),

packages/coding-agent/test/tts/streaming-player.test.ts

@@ -0,0 +1,54 @@
+import { describe, expect, it } from "bun:test";
+import type { PlayerCommand } from "@oh-my-pi/pi-coding-agent/tts/player";
+import { StreamingAudioPlayer } from "@oh-my-pi/pi-coding-agent/tts/streaming-player";
+
+/** A one-segment ~0.5s clip at 24 kHz, the shape `omp say "hi"` produces. */
+const clip = (): Float32Array => new Float32Array(24_000 / 2).fill(0.1);
+
+describe("StreamingAudioPlayer nonzero-exit fallback", () => {
+	it("replays the buffered clip via file playback when the streaming backend exits nonzero", async () => {
+		// `cat` drains stdin so the pipe write succeeds, then the shell exits 1 —
+		// exactly the bundled-ffmpeg-without-outdev failure that used to silently
+		// drop `omp say`'s single short clip.
+		const played: string[] = [];
+		const player = new StreamingAudioPlayer({
+			commandsFor: (): PlayerCommand[] => [{ cmd: "sh", args: ["-c", "cat >/dev/null; exit 1"] }],
+			playAudio: async wavPath => {
+				played.push(wavPath);
+			},
+		});
+		player.start(24_000);
+		player.write(clip());
+		await player.end();
+		expect(played.length).toBe(1);
+	});
+
+	it("does not replay when the streaming backend exits cleanly", async () => {
+		const played: string[] = [];
+		const player = new StreamingAudioPlayer({
+			commandsFor: (): PlayerCommand[] => [{ cmd: "sh", args: ["-c", "cat >/dev/null"] }],
+			playAudio: async wavPath => {
+				played.push(wavPath);
+			},
+		});
+		player.start(24_000);
+		player.write(clip());
+		await player.end();
+		expect(played.length).toBe(0);
+	});
+
+	it("plays every chunk via the file fallback when no streaming backend exists", async () => {
+		const played: string[] = [];
+		const player = new StreamingAudioPlayer({
+			commandsFor: (): PlayerCommand[] => [],
+			playAudio: async wavPath => {
+				played.push(wavPath);
+			},
+		});
+		player.start(24_000);
+		player.write(clip());
+		player.write(clip());
+		await player.end();
+		expect(played.length).toBe(2);
+	});
+});