feat(desktop): 通知音と Aivis 音声の重なりを防ぐ AudioScheduler 導入#405
Conversation
通知音と Aivis TTS が同時イベントで重なる問題を解消。
- 通知音: ringtone / aivis いずれかが再生中なら 2 件目を破棄
- Aivis: FIFO キューで直列再生、PermissionRequest は Stop より優先
- Aivis API エラーをステータスごとに分類:
- 429 / 5xx / network: 指数バックオフで最大 3 回リトライ。429 は
X-Aivis-RateLimit-Requests-Reset ヘッダーを尊重
- 401 / 402 / 404: キューを破棄して一時停止し、OS 通知で理由を表示
- 422: そのアイテムのみスキップ
- 成功レスポンスの RateLimit ヘッダーを記録し、Remaining 0 の時は
Reset + 0.5s 待機する proactive 制御
- 30s のリクエストタイムアウトを追加
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 34 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAudioSchedulerを導入して通知音声再生を調整し、リングトーンとAivis TTSの重複を防止。Aivis TTSタスクを優先度付きキューで処理し、レート制限をヘッダーから取得して管理。aivis-ttsモジュールを合成と再生の関心分離に再設計。 Changes
Sequence DiagramsequenceDiagram
participant User
participant NotificationManager
participant AudioScheduler
participant AivisAPI
participant Ringtone
participant Speaker
User->>NotificationManager: Lifecycle Event (Stop/PermissionRequest)
NotificationManager->>AudioScheduler: playRingtone()
alt Ringtone Idle
AudioScheduler->>Ringtone: Play
Ringtone->>Speaker: Audio Output
Note over Ringtone: Playing...
end
NotificationManager->>NotificationManager: buildAivisRunner(event)
NotificationManager->>AudioScheduler: enqueueAivis(runner, priority)
Note over AudioScheduler: Task queued (FIFO/Priority)
loop Rate Limit Check
AudioScheduler->>AudioScheduler: Check remaining quota
alt Quota Available
AudioScheduler->>AudioScheduler: Proceed
else Quota Exhausted
AudioScheduler->>AudioScheduler: Await resetSeconds + margin
end
end
Note over Ringtone: Completion
AudioScheduler->>AudioScheduler: Mark ringtone idle
par Aivis Processing
AudioScheduler->>AivisAPI: synthesizeAivisAudio()
AivisAPI-->>AudioScheduler: Audio Buffer + Rate-Limit Headers
Note over AudioScheduler: Capture X-Aivis-RateLimit-*
AudioScheduler->>AudioScheduler: playAivisAudio(buffer)
AudioScheduler->>Speaker: Play Aivis Audio
end
alt Error (429/5xx)
AudioScheduler->>AudioScheduler: Exponential backoff retry
else Error (401/402/404)
AudioScheduler->>AudioScheduler: Pause queue
AudioScheduler->>User: Show "Aivis paused" notification
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a8629aee7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.aivisBusy) return; | ||
| if (this.disposed || this.paused) return; |
There was a problem hiding this comment.
The scheduler starts Aivis work as soon as an item is enqueued, but this guard only checks aivisBusy/paused and ignores ringtoneBusy. Since NotificationManager now calls playRingtone() and enqueueAivis() back-to-back for the same event, pump() can begin synthesis/play before the ringtone completion callback fires, so notification audio can overlap again on fast Aivis responses. This contradicts the stated non-overlap contract and is user-visible whenever ringtone duration exceeds synth latency.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
指摘の通りでした。pump() は aivisBusy しか見ておらず、ringtone 再生中でも Aivis の合成→再生が即座に始まる競合がありました。
修正: 合成はネットワーク呼び出しで音を出さないのでそのまま並行実行できるが、runner.play() の直前で waitForRingtoneIdle() を挟んで ringtone の onComplete を待つようにしました。ringtone が終わったら pending 待機者を全部 resolve します。dispose 時も待機者を flush して runOne が hang しないようにしています。
テスト2件追加 (ringtone 完了前は aivis play が呼ばれない / dispose で待機中の aivis が解放される)。
Codex レビュー (PR #405) で指摘されたバグ。pump() は aivisBusy のみを チェックしていたので、playRingtone() 直後に enqueueAivis() を呼ぶと、 ringtone 再生中に Aivis の合成と再生が始まって音声が重なっていた。 修正: 合成はそのまま並行実行できる（ネットワーク呼び出しなので安全） が、re-sampled した音声の再生 (runner.play) は waitForRingtoneIdle() で ringtone 完了を待ってから実行するようにする。ringtone の onComplete が pending 待機者を解放する。dispose 時も待機者を flush して runOne が hang しないようにする。 テスト2件追加: - ringtone 完了前は aivis play が呼ばれない - dispose で待機中の aivis が解放される
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4dfc1dc75
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private drainAndPause(reason: string): void { | ||
| const dropped = this.queue.length; | ||
| this.queue = []; | ||
| this.paused = true; |
There was a problem hiding this comment.
drainAndPause permanently flips paused to true, and enqueueAivis drops all future tasks while paused, but this change set never wires any call to resume() from the main notification flow. As a result, a single 401/402/404 response disables Aivis for the rest of the running app session, even after the user fixes API key/credits/model settings, so subsequent voice notifications are silently lost until restart.
Useful? React with 👍 / 👎.
| err, | ||
| ); | ||
| } | ||
| clearTimeout(timeoutId); |
There was a problem hiding this comment.
The 30s timeout is cleared right after fetch() returns, before res.arrayBuffer() runs. Because fetch can resolve as soon as headers arrive, a slow or stalled response body can still hang this path well beyond 30s, leaving the scheduler busy and delaying later queued utterances despite the new timeout policy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ご指摘ありがとうございます。42b5276d で修正しました。
clearTimeout(timeoutId) を fetch() 直後ではなく try/finally の finally に移し、res.text() / res.arrayBuffer() の本体読み取りが完了 (or 例外) するまでタイマーを armed のまま保持するようにしました。本体読み取り中に AbortSignal で abort した場合も wrapFetchError で AivisError(retryable) に変換し、scheduler の retry ポリシーに乗せます。
Codex レビュー (PR #405) で指摘された P2 問題の修正。 fetch() はヘッダー到着時点で resolve するので、直後に clearTimeout していると、サーバが body のストリームで stall した場合にタイムアウ トが効かず、scheduler が 30s を大きく超えて busy のままになる恐れが あった。 修正: timeoutId のクリアを try/finally の finally に移し、レスポンス ボディ (res.text / res.arrayBuffer) の読み取り完了後に初めてタイマー を止めるようにする。body 読み取り中の AbortError も wrapFetchError 経由で AivisError(retryable) に変換し、scheduler の retry ポリシー に乗せる。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/desktop/src/main/lib/notifications/aivis-tts.ts (1)
243-247:rmサブプロセスでの削除はfs/promisesのunlinkに置き換えるのが望ましい
execFile("rm", ...)は Windows でrmが存在しないため失敗し、コールバックでエラーを握りつぶしているのでテンポラリファイルがリークします。また本ファイルは既にnode:fs/promisesからwriteFileを import しているので、サブプロセスを起動する必要はありません。♻️ 提案パッチ
-import { writeFile } from "node:fs/promises"; +import { unlink, writeFile } from "node:fs/promises";-function removeFile(path: string): void { - execFile("rm", ["-f", path], () => { - /* ignore */ - }); -} +function removeFile(path: string): void { + void unlink(path).catch(() => { + /* ignore — temp file may already be gone */ + }); +}
execFile/child_processが他に使われていなければ import も整理できます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/notifications/aivis-tts.ts` around lines 243 - 247, removeFile currently spawns rm via execFile which fails on Windows and swallows errors; replace its implementation to use node:fs/promises.unlink (make removeFile async or return the Promise) and properly handle ENOENT (ignore if file already missing) while rethrowing other errors; update references to await or handle the returned Promise, remove the child_process/execFile import if no longer used, and keep existing writeFile import intact.apps/desktop/src/main/lib/notifications/audio-scheduler.ts (2)
85-86:DEFAULT_BACKOFF_MSの 3 番目の要素は到達しない
MAX_RETRY_ATTEMPTS = 3かつ Line 238 でattempt >= 3のとき break するため、sleep()に渡されるのはattempt = 1, 2の時のみ、すなわちDEFAULT_BACKOFF_MS[0]と[1]だけです。3 番目の4000は常にデッドコードになっています。意図的に将来の
MAX_RETRY_ATTEMPTS引き上げに備えた配列であればコメントで明示、そうでなければ要素を 2 個に削減するか、配列を[1000, 2000]にしてat(-1)フォールバック（Line 255）に一本化すると読み手の混乱を減らせます。Also applies to: 238-240, 251-256
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/notifications/audio-scheduler.ts` around lines 85 - 86, DEFAULT_BACKOFF_MS contains three values but MAX_RETRY_ATTEMPTS is 3, so the third value (4000) is never used; update the retry/backoff logic by either reducing DEFAULT_BACKOFF_MS to two entries ([1000, 2000]) or keep the array but rely consistently on the fallback (e.g., using DEFAULT_BACKOFF_MS.at(-1)) where backoff is selected in the retry loop (refer to MAX_RETRY_ATTEMPTS, DEFAULT_BACKOFF_MS, and the sleep/backoff usage around the retry logic) and add a short comment explaining the chosen approach so the dead-code confusion is removed.
33-46:Error.causeはsuperのオプションで渡すのが簡潔プロジェクトの TypeScript ターゲットは
esnextに設定されているため、ES2022 のErrorコンストラクタでcauseオプションが利用可能です。手動キャストを避けて次のように書けます。♻️ 提案パッチ
export class AivisError extends Error { constructor( readonly kind: AivisErrorKind, readonly reason: string, readonly status?: number, /** For 429: seconds to wait before retrying (from header). */ readonly rateLimitReset?: number, cause?: unknown, ) { - super(reason); + super(reason, cause !== undefined ? { cause } : undefined); this.name = "AivisError"; - if (cause !== undefined) (this as { cause?: unknown }).cause = cause; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/notifications/audio-scheduler.ts` around lines 33 - 46, The AivisError constructor currently assigns the cause via a manual cast after calling super; instead pass the cause into Error's constructor options to leverage ES2022's Error.cause support: in the AivisError constructor (class AivisError) call super(reason, { cause }) when cause is provided (or unconditionally pass the cause value), and remove the manual cast and assignment of (this as { cause?: unknown }).cause = cause.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/lib/notifications/audio-scheduler.ts`:
- Around line 109-121: playRingtone currently relies on deps.playRingtone to
always call the provided callback, but if that contract is violated ringtoneBusy
may remain true and waitForRingtoneIdle will hang; update playRingtone and/or
waitForRingtoneIdle to defend against this by adding a bounded timeout and
explicit logging: wrap the deps.playRingtone invocation so that if
onRingtoneComplete is not invoked within N ms you force-clear ringtoneBusy via
onRingtoneComplete (or a shared cleanup path), emit a warning/error log that the
ringtone callback timed out, and ensure any finally-style cleanup always calls
onRingtoneComplete; reference the playRingtone, deps.playRingtone,
onRingtoneComplete, ringtoneBusy, and waitForRingtoneIdle symbols when making
the change.
---
Nitpick comments:
In `@apps/desktop/src/main/lib/notifications/aivis-tts.ts`:
- Around line 243-247: removeFile currently spawns rm via execFile which fails
on Windows and swallows errors; replace its implementation to use
node:fs/promises.unlink (make removeFile async or return the Promise) and
properly handle ENOENT (ignore if file already missing) while rethrowing other
errors; update references to await or handle the returned Promise, remove the
child_process/execFile import if no longer used, and keep existing writeFile
import intact.
In `@apps/desktop/src/main/lib/notifications/audio-scheduler.ts`:
- Around line 85-86: DEFAULT_BACKOFF_MS contains three values but
MAX_RETRY_ATTEMPTS is 3, so the third value (4000) is never used; update the
retry/backoff logic by either reducing DEFAULT_BACKOFF_MS to two entries ([1000,
2000]) or keep the array but rely consistently on the fallback (e.g., using
DEFAULT_BACKOFF_MS.at(-1)) where backoff is selected in the retry loop (refer to
MAX_RETRY_ATTEMPTS, DEFAULT_BACKOFF_MS, and the sleep/backoff usage around the
retry logic) and add a short comment explaining the chosen approach so the
dead-code confusion is removed.
- Around line 33-46: The AivisError constructor currently assigns the cause via
a manual cast after calling super; instead pass the cause into Error's
constructor options to leverage ES2022's Error.cause support: in the AivisError
constructor (class AivisError) call super(reason, { cause }) when cause is
provided (or unconditionally pass the cause value), and remove the manual cast
and assignment of (this as { cause?: unknown }).cause = cause.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 271a8c5b-ecf8-4c17-bda7-42b694928abc
📒 Files selected for processing (7)
README.mdapps/desktop/src/main/lib/notifications/aivis-tts.tsapps/desktop/src/main/lib/notifications/audio-scheduler.test.tsapps/desktop/src/main/lib/notifications/audio-scheduler.tsapps/desktop/src/main/lib/notifications/notification-manager.test.tsapps/desktop/src/main/lib/notifications/notification-manager.tsapps/desktop/src/main/windows/main.ts
PR #405 の CodeRabbit レビュー 4 件まとめて対応。 1. playRingtone に safety timeout (30s) を追加 deps.playRingtone の onComplete contract が将来壊れても scheduler が無限 hang しないよう、deps 経由で注入可能な safety timer を追加。発火時は警告ログを残して busy フラグを強制解放し、 pending waiters を全部 resolve する。テスト1件追加。 2. aivis-tts.removeFile を execFile("rm") から fs.unlink へ Windows には rm が無くテンポラリファイルがリークしていた。 既に node:fs/promises から writeFile を import していたので unlink を同時に取り込み child_process 依存を削除。 3. DEFAULT_BACKOFF_MS の 3 番目 (4000) はデッドコードだった MAX_RETRY_ATTEMPTS=3 なので sleep は attempt 1→2, 2→3 の 2 回 だけ。配列を [1000, 2000] に縮め、なぜ 2 個なのかコメントで 説明。at(-1) フォールバックはそのまま残す。 4. AivisError の cause を super のオプション引数経由に ES2022 の Error.cause を使って手動キャストを除去。
|
@coderabbitai レビューありがとうございます！4 件すべて妥当な指摘でしたので
再確認お願いします。 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 33227
コードの品質が全体的に高く、設計も堅牢です。このままマージいただいて問題ありません。 If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
1 participant
Summary
通知音（ringtone）と Aivis 音声通知が同時イベントで重なって再生されてしまう問題を解消します。
変更の要点
Stopイベントより先にキューの前に割り込ませる。429/5xx/ network / timeout）: 指数バックオフで最大 3 回リトライ。429はX-Aivis-RateLimit-Requests-Resetヘッダーの秒数 + 0.5s を優先。401/402/404）: キューを破棄して一時停止。原因を OS 通知で 1 回だけ表示（1 分クールダウン）。ユーザー対応待ちなので自動再開はしない。422）: そのアイテムだけスキップしてキュー継続。X-Aivis-RateLimit-Requests-Remaining/-Resetを記録し、Remaining=0なら次の送信をReset + 0.5s待機して先回りで回避。/v1/tts/synthesizeに 30s のAbortSignalを追加（通常 1 秒未満で返る API なのに永久 hang しないように）。新規ファイル
apps/desktop/src/main/lib/notifications/audio-scheduler.ts— ドロップ / キュー / 再試行 / 停止ロジックを集約apps/desktop/src/main/lib/notifications/audio-scheduler.test.ts— スケジューラー単体テスト（13 ケース）既存ファイルの変更
aivis-tts.ts:synthesizeAivisAudio/playAivisAudio/buildAivisTaskRunnerに分離。AivisErrorを throw するよう再実装。設定画面の「テスト発声」用playAivisTtsはそのまま維持（スケジューラーをバイパスする一発実行）。notification-manager.ts:playSound/playAivisの依存をplayRingtone/buildAivisRunner/enqueueAivisに置き換え。main.ts:AudioSchedulerを初期化してNotificationManagerに配線。致命的エラー時の OS 通知ハンドラshowAivisPausedNotificationを追加。設計の補足
dispose()でキューを破棄するので、ウィンドウ閉鎖時に未処理タスクが残ることはありません。Aivis pausedOS 通知は 1 分クールダウン。致命的エラーが連発しても OS 通知を爆撃しません。Test plan
bun test src/main/lib/notifications/— 87 pass / 0 fail（うち audio-scheduler 単独で 13 ケース）bun run typecheck— 通過bun run lint— 通過Summary by CodeRabbit
リリースノート
新機能
テスト