diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d876c4781..86bcafaa5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/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. diff --git a/packages/coding-agent/src/tts/streaming-player.ts b/packages/coding-agent/src/tts/streaming-player.ts index 219d55bdc..6dc458d72 100644 --- a/packages/coding-agent/src/tts/streaming-player.ts +++ b/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; +} + /** * 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 = Promise.resolve(); + readonly #commandsFor: (sampleRate: number) => PlayerCommand[]; + readonly #playAudio: (wavPath: string, signal: AbortSignal) => Promise; + /** 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), diff --git a/packages/coding-agent/test/tts/streaming-player.test.ts b/packages/coding-agent/test/tts/streaming-player.test.ts new file mode 100644 index 000000000..0b004cc35 --- /dev/null +++ b/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); + }); +});