feat(#41): config ホットリロードを全ビジュアル要素で完結#337
Conversation
)       Part of #41 — config ホットリロードの **プレゼン層即時反映（PR2）**。**#324（PR1）にスタック**（base = `feat/41-config-hot-reload`、PR1 マージ後に main へ付け替え）。 ## 概要 PR1 で完成した `appStyleChanges` ping（config が有効更新された時に飛ぶ `Void` 通知）を **Header / Lyrics の Presenter が購読して視覚を即時に張り替える**。これでフォント・色・サイズ・artwork・歌詞スタイルが **daemon 再起動なし** で反映される。 過去の "push" モデル（各 Presenter に `restyle()` を生やし `stop(); start()` で購読ごと作り直す）は二重購読・`.share()` リプレイ喪失・monitor リークで頓挫していた。今回は **`start()` と `applyStyle()` を分離**する "pull" 設計で構造的に回避する。 ## 設計: `start()` / `applyStyle()` 分離 - **`start()`（起動時1回・購読の配線）**: 既存購読（`trackChange` / `artwork` / `playbackPosition`）に加え、`configInteractor.appStyleChanges` の sink を **1回だけ** 張る。長寿命の `DecodeEffectState` エンジンもここで1回構築。 - **`applyStyle()`（起動時＋変更の都度・冪等な反映）**: 所有 Interactor の **live computed prop**（`textLayout` / `artworkStyle` / `decodeEffectConfig` — これらは `configService.appStyle.*` を都度読むため reload 後は自動で新値）を `@Published` なスタイル状態へ流し込むだけ。 - **購読は絶対に張り替えない**。`appStyleChanges` が来たら `applyStyle()` を呼ぶのみ。これが旧 push モデルの三大障害（二重購読・リプレイ喪失・リーク）を構造的に消す核。 ## 何が即時反映されるか | Presenter | @published 化したスタイル | 反映されるもの | |---|---|---| | `HeaderPresenter` | `titleStyle` / `artistStyle` / `artworkSize` / `artworkOpacity` | タイトル・アーティストのフォント/色/サイズ、アートワークのサイズ/不透明度 | | `LyricsPresenter` | `lyricStyle` / `highlightStyle` | 歌詞・ハイライト行のフォント/色 | ## 意図的な2つの carve-out（`applyStyle()` に含めない） 1. **decode の *タイミング* config**: 長寿命の `DecodeEffectState`（Header の `titleEffect`/`artistEffect`）は `start()` で1回だけ構築し `applyStyle()` では作り直さない。進行中の scramble アニメを中断させないため、duration/charset の変更は **次回 reveal** から反映。（Lyrics の `decodeConfig` は reveal 都度 `DecodeEffectState` を新規構築する際に読まれる単なる config 値なので `applyStyle()` で毎回更新しても進行中アニメ非破壊。） 2. **AI 処理色の保持（#57）**: `HeaderPresenter` は private `isAIProcessing` フラグを持ち、AI 解決中に config 変更が来ても titleColor/artistColor を processing color のまま維持し、通常時のみ configured color を反映する。 ## テスト - `HeaderPresenterTests` / `LyricsPresenterTests` に **config hot reload suite** を追加: start 後に stub の `textLayout` を差し替え → `appStyleChanges` を発火 → `@Published` スタイルが新値へ更新されることを deadline ポーリングで検証（固定 sleep 不使用）。Header は「AI 処理中に config 変更が来ても processing color を維持」ケースも追加。 - stub は参照型 box（`StubTrackInteractorStyleBox` / `updateStyle`）で start 後の値差し替えを可能にし、既存テストは非破壊。 - 全 TDD（実装を外して red 確認 → 実装で green）。`swift test` 全 **1237 tests** green、`make lint` 指摘なし。 ## スコープと限界（PR2） Header / Lyrics のスタイル即時反映まで。**Wallpaper（ソース差し替え）と Screen（再選択）を同じ ping で張り替える処理は後続 PR（3〜4）**。#324（PR1）とともに Part of #41（Closes しない）。 ## Breaking changes なし。`@Published` 化と start/applyStyle 分離は Presenter 内部の再構成で、外部 API・既定挙動は後方互換。 ## バージョン `2.22.0 → 2.23.0`（feat → minor）。
     Part of #41 — config ホットリロードの **中核（PR1）**。 ## 概要 `config.toml` を daemon 再起動なしで反映するための **リアクティブな config 基盤** を導入する。過去の試みは各 Presenter に `restyle()` を生やし `stop(); start()` で購読を張り替える "push" モデルで、二重購読・`.share()` リプレイ喪失・monitor リーク・壁紙ブラックアウトといった複雑さで頓挫していた。今回は **config を単一の可変 source of truth に集約**し、購読の張り替えを一切せずに済む設計に組み直した。「設計を綺麗にすればロジックはシンプルになる」方針の実践。 ## 何が変わったか ### リアクティブ core - `ConfigUseCase`: `lazy` 凍結を `OSAllocatedUnfairLock<AppStyle>` の可変 store + `reload() -> ConfigReloadOutcome` に置換。**不正な config（decode 失敗 / 読取不能 / ファイルは在るのに既定へ落ちた）では store を更新せず前回の有効なスタイルを保持**し `.invalid` を返す。 - `ConfigWatchGateway`（Domain 契約）+ `FileWatchGateway`（新 Support モジュール）: `DispatchSource` で config の **親ディレクトリ** を監視。エディタの atomic-save は rename でファイル fd を失効させるため、ディレクトリを監視して保存を確実に検知する。`ProcessGateway`/`AudioTapGateway` と同じ「Domain 契約 + 専用 Support 実装 + `GatewayRegistration` の liveValue」定石に連なる。 - `ConfigInteractor`（新モジュール）: 監視イベントを 150ms debounce（注入 clock）して `reload()` を呼び、`.updated` は `appStyleChanges`（Void ping）、`.invalid` は `invalidConfig`（最新値 replay）として配信。 ### グラフィカルなエラー通知（stderr を使わない理由） - デーモン利用者はログを見られないため、不正 config は **画面で** 伝える。既存のローディング表示「金色の測地線球体」を再利用し、**琥珀色の "崩れた"（支柱が破断した）球体** `ConfigStatusOverlay` を右下に出す。破断は edge index を種にした決定論的手法（乱数不使用）。`ConfigStatusPresenter` が `invalidConfig` を購読して駆動。ゼロアイドルコスト（正常時は一切描画しない条件付き挿入、#252 パターン）。 ### lyrics のホットリロード - `CustomScriptLyricsDataSourceImpl` の `fallback_command`/`timeout_ms` の init 焼付を廃し、`get()` で毎回 config を再読込（`$LYRA_CONFIG_DIR`/`$LYRA_CACHE_DIR` 展開・timeout clamp 等の正規化はすべて保持）。 ### 配線 - `AppRouter` が `ConfigInteractor` と `ConfigStatusPresenter` を start/stop し、球体を `AppWindow → OverlayContentView` へ接続。 ## スコープと限界（PR1） PR1 で **ユーザーに見える即時反映** は「lyrics fallback コマンドの毎回反映」と「不正 config → 前回スタイル維持 ＋ 琥珀球体」。`appStyleChanges` の ping 配信基盤は完成しているが、**Header / Lyrics / Wallpaper / Screen の各 Presenter が config 変更でフォント・色・壁紙を張り替える処理（`start()` と `applyStyle()` の分離）は後続 PR（2〜4）** で入れる。球体の色・サイズ・破断度合いといった視覚の最終調整も後続で詰める。 ## テスト - **in-process パイプライン E2E**（`ConfigHotReloadTests`）: 実 `ConfigUseCase`＋`ConfigInteractor`＋`ConfigRepository`＋`ConfigDataSource` を temp config 上で結線し、fake gateway を手動 fire。①初期 A 反映 → ②B へ書換＋fire で `appStyleChanges` 発火＆`appStyle` が B → ③不正 TOML で `invalidConfig` 発火かつ `appStyle` は B のまま（前回値保持）を検証。 - 各層のユニットテスト（reload 分岐、DispatchSource 実 FS 発火、debounce 配信、Presenter 反映、崩れた球体レンダ、lyrics 毎回読み）。 - `swift test` 全 **1234 tests / 288 suites** green、`make lint` 指摘なし。 ## Breaking changes なし。既定値では従来挙動と後方互換（`ConfigReloadOutcome` 等は新規追加、`OverlayContentView`/`AppWindow` の presenter 引数は既定 nil）。 ## バージョン `2.21.3 → 2.22.0`（feat → minor）。
    Closes #330 — #324（config ホットリロード PR1）のレビューフォローアップ（Codex 指摘）。 ## 背景 起動時とホットリロードで optional セクション（`[ai]` / `[lyrics]`）の扱いに非対称があった: - **起動時**（`ConfigUseCaseImpl.init` → `loadAppStyle`）: `AppConfig.init` が `[ai]` / `[lyrics]` を `try?` で寛容にデコードし、不正でも当該セクションを `nil` に縮退して残りの視覚 config をロードする。 - **ホットリロード**（`reload()` → `validate()` → `tryDecode` → `strictDecodeOptionalSections`）: `[ai]` / `[lyrics]` のみ不正でも throw し、`validate()` が `.decodeError` → `reload()` が `.invalid` を返して **有効な text / wallpaper 編集まで捨てて** 前回スタイルを維持していた。 同じ config でも「起動時はロードできるがホットリロードでは拒否される」不整合が生じていた。 ## 変更 strictness を `tryDecode` / `validate` の `strictOptionalSections: Bool` フラグに集約: - `reload()` → `validate(strictOptionalSections: false)`（寛容）: optional セクション不正は起動時同様 `nil` 縮退させ、有効な部分を反映。**必須構造（text / wallpaper 本体）が不正な場合のみ** `.invalid`（琥珀色の崩れた測地線球体 + 前回維持）。 - `lyra healthcheck` → `validate(strictOptionalSections: true)`（strict）: malformed `[ai]` / `[lyrics]` を診断として surface し続ける（挙動不変）。 崩れた球体による error UX（#41 合意）は必須構造の不正に対してのみ発火するようになり、設計意図どおり。 ## テスト - `ConfigDataSourceImplTryDecodeTests`: malformed `[lyrics]` / `[ai]` が **strict では throw / lenient では throw しない** ことを検証。必須構造不正（`wallpaper = [`）は両モードで throw。well-formed は両モードで pass。 - `ConfigHotReloadPipelineTests`（実結線 E2E）: 有効な wallpaper 編集 + 不正 `[lyrics]` を書いて gateway fire → `.updated` で wallpaper 反映・`invalidConfig` emit なし（修正前は `.invalid` で前回維持だった挙動の回帰テスト）。 - `ConfigDataSource` / `ConfigRepository` の全 conformer（stub / mock 6 箇所）をシグネチャ更新。 ## 検証 - `swift build` ✅ / `swift test` 1253 件（`ScopedAPISessionTests` の既知 flaky を除き全通過、再実行でグリーン）/ `make lint` クリーン - docs 更新: `README.md`（Live reload）・`docs/ARCHITECTURE.md`（#41 Key Design Decision）・`AGENTS.md` を #330 挙動に正確化 - version: 2.24.0 → 2.24.1（patch） ## 設計判断 strictness を **labeled boolean**（`strictOptionalSections:`）で表現。呼び出し側 `validate(strictOptionalSections: false)` はラベルで意図が明瞭。2 値の内部区別のため enum 化はしていないが、レビューで enum が好まれれば差し替えは容易。 Base は stacked-PR の流儀に従い `feat/41-hot-reload/dev`。
    Closes #329. Part of #41 (config hot-reload) — フォローアップ。 ## 課題 デーモン起動時に config ファイルがまだ存在しない場合、`ConfigInteractorImpl.start()` は `existingConfigPath == nil` のため監視を張らずに return していた。そのため起動後に `lyra config init` や手動で `~/.config/lyra/config.toml` を作成しても、再起動するまで宣伝しているライブリロードが効かなかった。 ## 変更 - **`ConfigInteractorImpl.start()`**: early-return を廃止し、`ConfigUseCase.configDir`（ファイルの有無に関わらず設定ディレクトリを返す）を監視対象にする。`FileWatchGateway` は元々*親ディレクトリ*を監視する設計（atomic-save の rename 耐性）なので、ファイル未存在でも監視が armed になれば初回作成の create イベントを拾える。 - **`ConfigDataSourceImpl.configDir`**: ファイル未存在時のフォールバックを `home` から期待パス `$XDG_CONFIG_HOME/lyra` に変更。`expectedConfigDirectory` を抽出し `lyraConfigFolder()` と単一の真実源に統一（純粋なパス計算 — ディレクトリは作らない）。 - **配線**: `configDir` を `ConfigRepository` / `ConfigUseCase` へ委譲で通す。protocol 既定実装（空文字）を付与し、config 監視に無関係な既存スタブ 11 個は無変更のまま。 ## 残存エッジ（別スコープ） 設定**ディレクトリ自体**が起動時に存在しない場合は、`open()` が張れず従来通り再起動が必要。完全対応には親を遡って動的に再監視する仕組みが要り、監視対象の再ターゲットという別の複雑性を伴うため本 PR のスコープ外とした（issue #329 の想定挙動「親ディレクトリが存在する前提で監視を armed にする」に忠実）。 ## テスト - `ConfigInteractorImplTests`: config ファイルが無くても `configDir` を監視する新規テストを追加（`existingConfigPath == nil` → `watchCallCount == 1`、監視先ディレクトリを検証）。 - `ConfigDataSourceImplTests`: `configDir` のフォールバックが home ではなく期待パス（`.../lyra`）を指すことを検証するよう更新。 - 全 1254 テスト green、`make lint` clean。 ## Version 2.24.1 → 2.25.0 (minor)
     Part of #41（config ホットリロード）— 設計仕様 `docs/superpowers/specs/2026-07-12-config-hot-reload-design.md` の **PR3（トグル）**。**#334（#329）にスタック**しています（base: `feat/41-directory-watch`。#334 マージ後に dev へ自動リターゲット）。 ## 課題 ripple/spectrum は**見た目のパラメータ**（色・半径・shape・バー色・ノイズリダクション等）は既にライブ反映されるが、**`enabled` の on⇄off** は起動時スナップショットで固定されていた: - `AppRouter.frameHandlers` が起動時の `isEnabled` で ripple `idle()` / spectrum `tick()` を配列に含めるか決め、以後不変。 - `RipplePresenter.start()` が `RippleState` を1回生成し、`enabled` 時のみ mouse monitor を装着（トグルで着脱されない）。 - `SpectrumPresenter.start()` が `guard isEnabled` で capture を起動時のみ開始。 そのため config で `enabled` をトグルしても `lyra restart` まで反映されなかった。 ## 変更（Header/Lyrics と同じ `start()`/`applyStyle()` 分割を横展開） - **`RipplePresenter`**: `appStyleChanges` を `start()` で一度だけ購読 → `applyStyle()`。`applyStyle()` は ripple config を diff し、`enabled`/`idle`/`duration` 変化時のみ `RippleState` を再構築（色/半径/shape は `drawingContexts` で従来通りライブ読み、無関係な編集でライブ ripple を消さない）、`enabled` 変化時のみ global mouse monitor を着脱（冪等ガードで二重登録防止）。`idle()` に `enabled` early-return を追加。 - **`SpectrumPresenter`**: `isCapturing` を `start()` で一度だけ購読（subject は capture サイクルを跨いで永続するので張り替え不要）→ `appStyleChanges` 購読 → `applyStyle()` が `enabled` を diff して capture を start/stop。 - **`AppRouter.frameHandlers`**: ripple `idle()` / spectrum `tick()` を**常時含め**、`enabled` 判定を各ハンドラ内へ移動。`idle()` は disabled 時に early-return、`tick()` は capturing/残渣ガードで bail するので、無効時のゼロアイドルコスト（#252/#258）はガードで維持。 **購読は絶対に張り替えない**（仕様の中核原則）ので、`.share()` リプレイ喪失も monitor リークも構造的に起きない。 ## テスト - 共有 `FakeConfigInteractor`（`appStyleChanges` を任意発火）＋ `flushMainQueue()`（`.receive(on: .main)` の配送を FIFO で確定ドレイン、fixed `Task.sleep` 不使用で CI フレーク無し）を追加。 - **Ripple**: config ping で `enabled` false→true → `RippleState` 再構築を検証／色のみ変更 ping では同一 `RippleState` インスタンスが保たれる（ライブ ripple 生存）ことを検証。 - **Spectrum**: config ping で `enabled` false→true→false → capture の start/stop を検証。 - 全 **1257** テスト green（新規 3 件含む）、`make lint` clean。既存の frame-callback テスト（#278/#304）も回帰なし。 ## 残り（PR4・別 PR） Wallpaper ソース差し替え・Screen 再選択（重量級: AVPlayer 再アタッチのブラックアウト回避、画面再選択の駆動導線追加）。 ## Version 2.25.0 → 2.26.0 (minor)
      Part of #41（config ホットリロード）— 設計仕様 `docs/superpowers/specs/2026-07-12-config-hot-reload-design.md` の **PR4（重量級）**。**#335（PR3）にスタック**しています（base: `feat/41-ripple-spectrum-toggle`。#335 マージ後に dev へ自動リターゲット）。これで #41 の「視覚要素すべて」が揃い、**config ホットリロードは機能的に完了**します。 ## 課題 Header/Lyrics（PR2）・ripple/spectrum（PR3）はライブ反映されるが、**壁紙ソースの差し替え**と**画面の再選択**だけが起動時スナップショットで固定されていた: - `WallpaperPresenter.start()` が `resolvedWallpapers()` を1回だけ streaming して items を焼き付け。ソースを差し替えても `lyra restart` まで反映されない。素朴な `stop(); start()` は `controller.stop()` が AVPlayer を nil にし、`onPlayerAvailable` の `.first()` は一度きりなので**壁紙が永久に真っ黒**（過去 PR 破綻の主因の一つ）。 - `AppPresenter` は `[screen]` selector/debounce を起動時に評価するだけで、config 変更を `resolveLayout()` へ流す導線がゼロ。 ## 変更（Header/Lyrics と同じ `start()`/`applyStyle()` 分割を横展開） - **`WallpaperInteractor`**: `wallpaperSource`（`WallpaperStyle?` = items+mode）を追加。Presenter が ping ごとに diff する軽量な差分キー（`RipplePresenter` の `rippleConfig` diff と同じ発想）。 - **`WallpaperPresenter`**: `appStyleChanges` を `start()` で一度だけ購読 → `applyStyle()`。source が実際に変わった時だけ `resolvedWallpapers()` を張り直し、無関係な編集（例: header font）では**再生中の動画に一切触れない**。リロードは既存 AVPlayer 上で `replaceCurrentItem` するので、一度アタッチした `AVPlayerLayer` は有効なまま=**ブラックアウトしない**。全 items 削除時は新設の非破壊 `WallpaperPlaybackController.clearCurrentItem()`（映像だけ消して player は保持）で、後の再追加が同じ layered インスタンスに再アタッチできる。 - **`AppPresenter`**: `appStyleChanges` を layout パイプラインへ合流。ping で `resolveLayout()` を再実行（selector 変化で別ディスプレイを選択）し、`restartVacantPolling()` が live な selector/debounce から vacant ポーリングを張り直す（vacant 化・非 vacant 化・interval 変化を網羅）。 **購読は絶対に張り替えない**（仕様の中核原則）ので `.share()` リプレイ喪失も monitor リークも壁紙ブラックアウトも構造的に起きない。 ## テスト - `MutableWallpaperInteractor` + `HotReload` suite: - config ping でソース差替 → item がスワップされ、**AVPlayer インスタンスは同一**（ブラックアウト無し）を検証。 - 無変化 ping → `resolveCount` が増えない（再解決しない）ことを検証。 - 全 items 削除 ping → 映像はクリアされるが player は保持されることを検証。 - `AppPresenter`: config ping で `resolveLayout()` 再実行（画面再選択）／selector を vacant へ切替えるとポーリングが開始することを検証（`FakeConfigInteractor` + `flushMainQueue()`、fixed `Task.sleep` 不使用）。 - 全 **1262** テスト green（新規 5 件含む）、`make lint` clean。既存の Wallpaper/AVPlayer 回帰なし。 ## 実機スモーク（仕様 §12 ③ 番犬）— レビュー中に別途 `swift .claude/scripts/check-overlay.swift`（`memory > width×height` のブランク検出）で、壁紙 config 差替後も**非ブランク**であることを host/VM で確認予定（AVPlayer 再アタッチの唯一の難所ゆえ）。 ## Version 2.26.0 → 2.27.0 (minor)
📝 WalkthroughWalkthroughConfig hot reload now watches configuration files and includes, debounces reloads, preserves the last valid style after invalid edits, publishes reload outcomes, updates presenters without restart, displays invalid-config status, and handles wallpaper/player teardown and reattachment. ChangesConfiguration hot reload
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
config.toml の編集内容を daemon 再起動なしで 全ビジュアル要素へライブ反映させるためのホットリロード基盤〜適用（Presenter/Wallpaper/Screen）までを統合し、無効 config を UI 上で可視化できるようにする PR です。Clean Architecture/VIPER の依存方向を保ちつつ、ConfigInteractor の ping を各 Presenter の applyStyle() に接続して「購読は一度だけ」の運用に寄せています。
Changes:
ConfigInteractor+ConfigWatchGateway(FileWatchGateway)により、config ディレクトリ監視→debounce→ConfigUseCase.reload()→通知（updated/invalid）までのパイプラインを追加- Header/Lyrics/Ripple/Spectrum/Wallpaper/App(Screen layout) が
appStyleChangesを購読してapplyStyle()で差分適用し、再起動なし反映を実現（Wallpaper はブラックアウト回避も含む） - 無効 config を
ConfigStatusPresenter/ConfigStatusOverlay（琥珀色の崩れた測地線球体）で表示し、テスト・ドキュメント・バージョンを更新
Reviewed changes
Copilot reviewed 76 out of 76 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/WallpaperInteractorTests/WallpaperInteractorImplTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/ViewsTests/ViewRenderingTests.swift | Viewレンダリング待機の安定化＋ConfigStatusOverlayの描画回帰テスト追加 |
| Tests/ViewsTests/ConfigStatusGeometryTests.swift | ConfigStatusGeometry の決定性・投影・欠損・ソートのユニットテスト追加 |
| Tests/ViewsTests/AppWindowTests.swift | detachPlayerLayer による透明復帰のテスト追加 |
| Tests/TrackInteractorTests/TrackInteractorStyleTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/TrackInteractorTests/TrackInteractorRaceTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/TrackInteractorTests/TrackInteractorPlaybackPositionTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/TrackInteractorTests/TrackInteractorFallbackDisplayTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/TrackInteractorTests/TrackInteractorArtworkTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/TrackInteractorTests/TrackInteractorAIProcessingTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/SpectrumUseCaseTests/SpectrumUseCaseImplTests.swift | Analyzer の memo key 拡張（バンド設定変更で再構築）を検証 |
| Tests/SpectrumInteractorTests/SpectrumInteractorImplTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/ScreenInteractorTests/ScreenInteractorImplTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/PresentersTests/WallpaperPresenterTests.swift | wallpaperSource diff・player維持/破棄・onPlayerCleared の hot-reload テスト追加 |
| Tests/PresentersTests/TestSupport/WaitUntil.swift | CI負荷を考慮してデフォルト待機時間を 5s に拡張 |
| Tests/PresentersTests/SpectrumPresenterTests.swift | enabled トグルと styling ping の再描画（objectWillChange）をテスト追加 |
| Tests/PresentersTests/RippleScreenBoundsTests.swift | WallpaperInteractor 変更（wallpaperSource）に追従 |
| Tests/PresentersTests/RipplePresenterTests.swift | ripple の hot-reload toggle・state rebuild・hover reset のテスト追加 |
| Tests/PresentersTests/LyricsPresenterTests.swift | track/style stub を更新し appStyleChanges ping の applyStyle 反映をテスト追加 |
| Tests/PresentersTests/HeaderPresenterTests.swift | appStyleChanges ping の applyStyle、AI処理中色維持、decode effect境界再構築のテスト追加 |
| Tests/PresentersTests/FakeConfigInteractor.swift | presenterテスト共通の ConfigInteractor テストダブル＋flushMainQueue を追加 |
| Tests/PresentersTests/ConfigStatusPresenterTests.swift | invalidConfig の反映と stop() の購読解除テスト追加 |
| Tests/PresentersTests/AppPresenterTests.swift | config ping で layout 再解決＋vacant polling 再起動のテスト追加 |
| Tests/MetadataDataSourceTests/LLMMetadataDataSourceResolveTests.swift | ConfigDataSource.tryDecode シグネチャ変更に追従 |
| Tests/MetadataDataSourceTests/LLMMetadataDataSourceImplTests.swift | ConfigDataSource.tryDecode シグネチャ変更に追従 |
| Tests/LyricsDataSourceTests/CustomScriptLyricsDataSourceImplTests.swift | fallback_command/timeout_ms の毎回再読込（hot reload）をテストで保証 |
| Tests/FileWatchGatewayTests/FileWatchGatewayTests.swift | DispatchSource による directory watch の発火テスト追加 |
| Tests/ConfigUseCaseTests/ConfigUseCaseTests.swift | reload の outcomes（updated/invalid）と defaults 分岐をテスト追加 |
| Tests/ConfigRepositoryTests/ConfigRepositoryTests.swift | validate(strictOptionalSections:) の導入に追従 |
| Tests/ConfigInteractorTests/ConfigInteractorImplTests.swift | debounce→reload→publish（updated/invalid）と idempotent start、#329 をテスト追加 |
| Tests/ConfigHotReloadTests/ConfigHotReloadPipelineTests.swift | 実配線E2E（UseCase/Interactor/Repository/DataSource）でホットリロード成立を検証 |
| Tests/ConfigHandlerTests/ConfigHandlerImplTests.swift | ConfigUseCase stub に reload() を追加して新APIに追従 |
| Tests/ConfigDataSourceTests/TryDecodeTests.swift | setenv 排除＋ tryDecode(strictOptionalSections:) に更新 |
| Tests/ConfigDataSourceTests/ConfigDataSourceImplTests.swift | #329 configDir fallback の期待値＋#330 strict/lenient optional section をテスト |
| Tests/AppRouterTests/AppLaunchEnvironmentTests.swift | AppRouter が ConfigInteractor/ConfigStatusPresenter を start/stop することを検証 |
| Sources/WallpaperInteractor/WallpaperInteractorImpl.swift | wallpaperSource を追加し Presenter の diff キーを提供 |
| Sources/Views/Overlay/OverlayContentView.swift | ConfigStatusOverlay を最前面に追加できるよう拡張 |
| Sources/Views/Overlay/ConfigStatusRenderer.swift | ConfigStatusGeometry の結果を Canvas に描画する Renderer を追加 |
| Sources/Views/Overlay/ConfigStatusOverlay.swift | 無効 config を示すコーナーバッジUI（TimelineView+Canvas）を追加 |
| Sources/Views/Overlay/ConfigStatusGeometry.swift | 崩れた測地線球体の純粋ジオメトリ（決定的 jitter / 欠損）を追加 |
| Sources/Views/Overlay/AppWindow.swift | detachPlayerLayer を追加し、壁紙削除時に透明背景へ復帰 |
| Sources/VersionHandler/Resources/version.txt | バージョンを 2.22.0 → 2.27.0 に更新 |
| Sources/SpectrumUseCase/SpectrumUseCaseImpl.swift | Analyzer memo key を full spec 化して band 設定変更を即反映 |
| Sources/Presenters/Wallpaper/WallpaperPresenter.swift | appStyleChanges 購読＋wallpaperSource diff による再解決、player swap/clear seams を追加 |
| Sources/Presenters/Wallpaper/RipplePresenter.swift | appStyleChanges 購読＋RippleState/monitor を差分適用、disabled idle ガードへ |
| Sources/Presenters/Track/LyricsPresenter.swift | start()/applyStyle() 分離＋@published化＋appStyleChanges 購読を追加 |
| Sources/Presenters/Track/HeaderPresenter.swift | start()/applyStyle() 分離＋AI処理中色維持＋decode effect を境界で再構築 |
| Sources/Presenters/Spectrum/SpectrumPresenter.swift | appStyleChanges 購読＋enabled toggle で start/stop＋styling 変更で objectWillChange を送出 |
| Sources/Presenters/Config/ConfigStatusPresenter.swift | ConfigInteractor.invalidConfig を @Published に反映する Presenter を追加 |
| Sources/Presenters/App/AppPresenter.swift | config ping で resolveLayout 再実行＋vacant polling 再起動を追加 |
| Sources/LyricsDataSource/CustomScriptLyricsDataSourceImpl.swift | config を init で焼き付けず get() 毎に再読込（hot reload 対応） |
| Sources/FileWatchGateway/FileWatchGateway.swift | DispatchSource による directory watch 実装（ConfigWatchGateway live）を追加 |
| Sources/Entity/Style/SpectrumStyle.swift | SpectrumStyle を Equatable 化して差分判定に対応 |
| Sources/Entity/Config/ConfigReloadOutcome.swift | reload 結果（updated/invalid）を表す Entity を追加 |
| Sources/Entity/Config/ConfigReloadFailure.swift | invalid の理由（unreadable/decode）を表す Entity を追加 |
| Sources/Domain/UseCase/ConfigUseCase.swift | reload()/configDir を追加して hot-reload の契約を拡張 |
| Sources/Domain/Repository/ConfigRepository.swift | validate(strictOptionalSections:)/configDir を追加して strict/lenient を統一 |
| Sources/Domain/Misc/ConfigWatchGateway.swift | ConfigWatchGateway/Token の Domain 契約を追加 |
| Sources/Domain/Interactor/WallpaperInteractor.swift | wallpaperSource を追加して Presenter の差分判定キーを提供 |
| Sources/Domain/Interactor/ConfigInteractor.swift | ConfigInteractor の Domain 契約を追加 |
| Sources/Domain/DataSource/ConfigDataSource.swift | tryDecode(strictOptionalSections:) に変更し strict/lenient を明示化 |
| Sources/DependencyInjection/InteractorRegistration.swift | ConfigInteractor の liveValue 登録を追加 |
| Sources/DependencyInjection/GatewayRegistration.swift | ConfigWatchGateway の liveValue 登録（FileWatchGateway）を追加 |
| Sources/ConfigUseCase/ConfigUseCaseImpl.swift | OSAllocatedUnfairLock store＋reload outcome でホットリロード実装 |
| Sources/ConfigRepository/ConfigRepositoryImpl.swift | validate(strictOptionalSections:) と healthcheck strict を実装 |
| Sources/ConfigInteractor/ConfigInteractorImpl.swift | watch→debounce→reload→publish の interactor 実装を追加 |
| Sources/ConfigDataSource/ConfigDataSourceImpl.swift | configDir fallback を expected dir に変更＋optional strictness を導入 |
| Sources/AppRouter/OverlayWindow.swift | detachPlayerLayer を window API に追加 |
| Sources/AppRouter/AppRouter.swift | ConfigInteractor/ConfigStatusPresenter の lifecycle wire＋player clear で detach を配線 |
| Sources/AppRouter/AppDependencyBootstrap.swift | UITest 用 WallpaperInteractor を wallpaperSource 追加に追従 |
| README.md | Live reload の挙動と strict/lenient optional section の説明を追加 |
| Package.swift | ConfigInteractor/FileWatchGateway/関連テストターゲットを追加 |
| docs/ARCHITECTURE.md | #41 によるモジュール追加・依存関係・KDD を反映 |
| AGENTS.md | config hot-reload の repo ルール/実装概要を追記 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 9e2f59b18d
ℹ️ 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".
| guard source != appliedSource else { return } | ||
| loadWallpapers(source: source) |
There was a problem hiding this comment.
When a wallpaper hot-reload switches to a slow source (remote download/yt-dlp), appliedSource still contains the previous source until the async load succeeds. Any subsequent config ping for an unrelated edit sees the same new source as different here, cancels the in-flight loadTask, and starts resolution over again, so repeated edits can keep restarting the download instead of leaving the pending wallpaper load alone. Consider tracking a pending source separately from the last successfully applied source.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
489d655 で しました。ご提案どおり pending な source を
targetSource として applied とは別に追跡し、applyStyle() はこれと diff します。
- スロー解決中の無関係な編集ピンは pending と一致するため、in-flight のダウンロードを cancel→再起動しません
- 副次効果として、applied source への revert が pending と差分になるので、stale な pending swap を正しくキャンセルして再ロードします
- 空解決時は
targetSourceをappliedSourceにロールバックし、同一値の再保存でする従来のセマンティクス（F8）は維持
回帰テスト 2 本（ピン連打で resolveCount 不変 / revert で再ロード）を追加しています。
| let lyrics = configDataSource.load()?.config.lyrics | ||
| let fallbackCommand = lyrics?.fallbackCommand ?? [] |
There was a problem hiding this comment.
For a daemon that already has a valid [lyrics].fallback_command, saving a malformed required config (for example broken TOML or an invalid wallpaper section) is rejected by ConfigUseCase.reload() and the previous config remains in effect, but the next lyrics lookup re-reads the raw file here. In that rejected-edit state configDataSource.load() returns nil (or no lyrics), fallbackCommand becomes empty, and Tier C silently stops running even though the bad reload was not applied. The fallback settings need to come from the last accepted config or otherwise be retained across invalid reloads.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
230f482 で しました。
ConfigDataSourceImpl.load() に last-good 保持を追加し、ファイルが存在するのに必須構造の decode に失敗する間（rejected-edit 状態）は最後に成功した decode 結果を返します。
reload()の keep-previous-style 契約と per-call 読者の挙動が一致し、Tier C fallback は不正編集がディスク上にある間も直前のfallback_command/timeout_msで動き続けます（同じ per-call 読者である[ai]endpoint の同型問題も同時に解消）- config ファイル自体の削除は cache をクリアして defaults へ復帰（削除→壊れた内容で再作成でも旧値が漏れないことをテストで固定）
- 壊れた optional セクション単体は従来どおり lenient 降格（config ホットリロードのオプションセクション strict 判定を起動時の寛容判定と揃える (#41 フォローアップ) #330）で、reload が受理する内容と cache の内容は常に一致します
実ディスク backed の回帰テストを DataSource 層（4 本）と CustomScript 層（1 本）に追加しています。
| case .loaded: | ||
| let style = repository.loadAppStyle() | ||
| store.withLock { $0 = style } | ||
| return .updated(style) |
There was a problem hiding this comment.
When an existing config file is saved empty (or effectively contains no supported settings), tryDecode still succeeds and returns a path, so reload takes this .loaded branch and replaces the previous live style with defaults. That breaks the bad-edit guard for an existing file that resolves to defaults: a transient empty save or accidental truncation should keep the previous style instead of clearing wallpaper/text/screen settings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
と判断し、挙動を
します。空の（あるいは認識可能なキーを持たない）config は startup と同じく「全 defaults の正当な config」で、ファイルを空にするのは削除と並ぶ意図的なリセット手段です。defaults に解決される既存ファイルを reject すると、再起動なしの「空にしてリセット」が不可能になり、副作用の方が大きいと考えています。
ご懸念のトランジェントな空保存は三段で緩和されています:
- 同一保存内の truncate→write イベント連鎖は 150ms debounce が 1 回の reload に吸収
- 読み込み失敗（atomic-save の窓）は既存の
.defaults-for-existing-file ガードが.invalidとして前スタイルを保持 - 万一 defaults が一瞬適用されても、直後の write イベントの reload が復元する自己修復
恒久的に空になったファイルは「ユーザーが空 config を保存した」操作とこの層では区別できないため、正当な入力として扱います。
| /// Wireframe: creates Presenters, builds window, manages lifecycle. | ||
| @MainActor | ||
| public final class AppRouter { | ||
| @Dependency(\.configInteractor) private var configInteractor |
There was a problem hiding this comment.
In custom AppDependencyBootstrap contexts, this @Dependency is initialized when AppRouter is constructed, before withBootstrap applies overrides. The presenters created inside withBootstrap subscribe to the overridden configInteractor, but configInteractor.start()/stop() on this captured field target the default instance instead, so injected/test config watches are never started and their pings never reach the UI. Store the resolved interactor from inside the bootstrap scope instead of keeping it as an eagerly captured dependency field.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
です — レビュー対象 commit
9e2f59b の直後、05f4256 でまさにこの問題を解消しています。AppRouter は @Dependency(\.configInteractor) を保持しなくなり、lifecycle は withBootstrap {} スコープ内で構築される ConfigStatusPresenter へ移譲しました（SpectrumPresenter が SpectrumInteractor を持つのと同型）。Presenter の @Dependency は bootstrap スコープで解決されるため、custom AppDependencyBootstrap / テストの override が正しく start/stop されます。ConfigStatusPresenterTests に lifecycle 検証テスト、AppLaunchEnvironmentTests に配線検証テストがあります。
| items = [] | ||
| currentIndex = 0 | ||
| wallpaperScale = 1.0 |
There was a problem hiding this comment.
When switching to a new wallpaper source that is slow or fails to resolve, the old AVPlayer keeps showing the previous video, but these assignments immediately discard the old playlist and publish scale 1.0. That means the still-visible old wallpaper can suddenly change scale, and if the new source yields no items the old video remains stuck with an empty items list so it can no longer advance through its previous playlist. Keep the current state intact until a replacement item is actually ready or the source is explicitly removed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
489d655 で しました。旧 playlist / 公開中の
wallpaperScale のリセットを「置換の最初の item が到着するまで」遅延し、それまで公開状態には一切触れません。
- 再生中の旧壁紙の scale が 1.0 へ突然飛ぶことがなくなりました（scale は
activateCurrentItem()が新 item の値で上書きするときだけ変わる） - 新 source が空解決した場合は旧 playlist が丸ごと残るため、advance も従来どおり機能します
- 明示的な削除（nil source）のみ従来どおり即クリアし、player teardown → 透過復帰（F7）の経路は不変
回帰テスト 2 本（in-flight 中の状態維持＋playlist advance / 空解決での維持＋同一 source 再保存リトライ）を追加しています。
# Conflicts: # Sources/VersionHandler/Resources/version.txt # docs/ARCHITECTURE.md
…usPresenter へ移譲 AppRouter（Router=ワイヤーフレーム）が @dependency(\.configInteractor) を直接保持し configInteractor.start()/stop() を呼んでいたのを、front する ConfigStatusPresenter へ移譲。 他の overlay（SpectrumPresenter が SpectrumInteractor.start()/stop() を持つ）と同型に揃え、 Router が Interactor 層へ直接アクセスしないようにする。 ConfigStatusPresenter は bootstrap スコープ内で生成されるため @dependency がその スコープの interactor を捕捉する。これにより AppRouter.stop() の `withBootstrap { configInteractor.stop() }` という回避策も不要になり削除。 - ConfigStatusPresenter.start(): invalidConfig 購読の後に interactor.start()（全 appStyleChanges 購読が揃った後に watch を arm） - ConfigStatusPresenter.stop(): interactor.stop() で watch を disarm - ConfigStatusPresenterTests: lifecycle 駆動を検証するテストを追加 - AppRouterTests: spy の start/stop 回数アサーションは不変、コメントを実態に更新
| private func applyReload() { | ||
| switch configUseCase.reload() { | ||
| case .updated: | ||
| invalidSubject.send(nil) | ||
| appStyleSubject.send(()) | ||
| case .invalid(let failure): | ||
| invalidSubject.send(failure) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
6f1958d でしました。ご提案どおり armed チェック（
token != nil）を stop() が取るのと同じ NSLock の内側でゲートし、さらに Task.isCancelled チェックと reload + publish 自体も同じ critical section に入れています。これで「cancel チェックを通過済みのタスクが stop() 後に publish する」隙間が構造的になくなり、stop() がリターンした時点以降は何も emit されません。
TestClock で debounce を sleep 中に固定 → stop() → 起床、という順序を再現する回帰テスト stopSuppressesPendingReload を追加しています。
| @MainActor | ||
| private func waitUntil(timeout: Duration = .seconds(3), _ condition: @MainActor () -> Bool) async { | ||
| let deadline = ContinuousClock.now + timeout | ||
| while !condition(), ContinuousClock.now < deadline { | ||
| try? await Task.sleep(for: .milliseconds(10)) | ||
| } | ||
| } |
There was a problem hiding this comment.
です — このファイルローカル
waitUntil（3s）は b511bcc（autofix suggestion 受諾）で削除し、Tests/PresentersTests/TestSupport/WaitUntil.swift の共有 5s ヘルパーへ統合済みです。ホットリロード系のポジティブポーリングはすべて共有ヘルパー経由で 5s 上限になっています。
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- applyStyle() の diff 対象を appliedSource から targetSource（in-flight の pending source）へ変更。スロー解決（リモート DL / yt-dlp）中に無関係な config 編集がピンを打っても in-flight のダウンロードを cancel→再起動 しない。applied source への revert は pending と差分になるので正しく pending swap をキャンセルして再ロードする - loadWallpapers() の eager な items/scale リセットを「置換の最初の item が到着するまで」遅延。再生中の旧壁紙が scale 1.0 に飛んだり、新 source が空解決したときに旧 playlist を advance できなくなる問題を解消 - 空解決時は targetSource を appliedSource にロールバックし、同一値の 再保存でリトライする F8 セマンティクスを維持（回帰テストあり） #337 レビュー（Codex P2 ×2）対応
…読者を継続 - load() が最後に成功した decode 結果を lock 付き box（liveValue は単一 インスタンスなので値コピー間で共有）に保持し、ファイルが存在するのに 必須構造の decode に失敗する間はそれを返す。reload() の keep-previous-style 契約と per-call 読者（[lyrics] fallback_command、 [ai] endpoint）の挙動が一致し、不正編集がディスク上にある間も Tier C fallback や AI 処理が silent に無効化されない - config ファイル自体の削除は cache をクリアして defaults へ復帰 （削除→壊れた内容で再作成でも旧値が漏れないことをテストで固定） - 壊れた optional セクション単体は従来どおり lenient 降格（#330）で cache も上書きされる — doc comment に保護範囲を明記 #337 レビュー（Codex P2）対応
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
debounce タスクの armed/cancelled チェックと reload + publish を stop() が取る NSLock の内側へ移動。既に起床済みのタスクが stop() リターン後に spurious な appStyleChanges/invalidConfig を publish する teardown race を構造的に排除する。 TestClock で stop 後の発火を再現する回帰テストを追加。
Copilot の committable suggestion 受諾で reload() が column 0 に dedent され 波括弧が崩れていたのを復元。existingPath を一度だけ捕捉する意図のコメントも 維持（存在チェックと報告される失敗 path の不一致を atomic save 中でも防ぐ）。
- AppRouter: インラインだった default windowFactory を defaultFrameSchedulerFactory と同型の static メソッドへ抽出し、実 AppWindow 構築テストを追加（AppWindow.init も同時にカバー） - AppRouter: 壁紙削除リロードで onPlayerCleared → detachPlayerLayer が window へ届くことを検証する end-to-end テストを追加 - ConfigUseCase: validate の .unreadable 経路を検証 - WallpaperInteractor: wallpaperSource の委譲（有/無）を検証 - OverlayContentView: configStatusPresenter 非 nil 分岐のレンダリングを検証
…tch coverage を 100% に codecov の残余 0.39%（detachPlayerLayer の thin forwarding 3 行、#41 PR4 新設）を defaultWindowFactory テスト内で実 AppWindow に対し attach → detach を実行して解消。 static な layer plumbing 自体は既存の SpyOverlayWindowSurface テストでカバー済み。
CI で ObjectIdentifier 同士の比較が false failure（1 個目の URLSession 解放後、 2 個目の割当が同一ヒープアドレスを再利用）。両インスタンスを保持したまま !== で比較する形に変更し、アドレス再利用の影響を受けなくする。
ディレクトリ fd の DispatchSource はエントリ変化（rename/create/delete）でしか 発火せず、リネームせず保存するエディタ・cp・追記などの in-place 上書きでは リロードが走らなかった。ConfigWatchGateway に watch(file:) を追加した二段 watch とし、file fd は rename で無効化されるため ConfigInteractorImpl が リロード毎に再アームする。
ftruncate によるファイル縮小は NOTE_WRITE ではなく NOTE_ATTRIB のみ発火する ため、truncate ベースの in-place 保存がリロードされなかった（実機検証で発見）。 chmod/touch 等の偽イベントは debounce と冪等 reload が吸収する。
includes で取り込んだ TOML は directory watch（configDir 直下の atomic save） でしか検知されず、in-place 上書きや configDir 外の include は一切リロード されなかった。ConfigUseCase.includedConfigPaths（DataSource が decode の マージと同じ解決ヘルパーで算出）を追加し、ConfigInteractorImpl の再アーム 対象に include ファイルの file watch と configDir 外 include の親ディレクトリ watch を含める。
実機の lsof で configDir に fd が 2 本張られているのを発見。Files の folder path は末尾スラッシュ付きで NSString.deletingLastPathComponent の結果と 一致せず、subtraction をすり抜けていた。
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
r3596766789 / r3596766847 の suggested changeset はコメント修正と同時に interactor.start() と configStatusPresenter.start() のコード行まで削除して いた（watch が一切張られなくなる退行）。文面の修正は採用しつつ呼び出しを 復元。AppRouter 側コメントは行長制約に合わせ再整形。
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56fe6dd865
ℹ️ 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".
| return relativePath.hasPrefix("/") | ||
| ? try? File(path: relativePath) | ||
| : try? Folder(path: configDir).file(at: relativePath) |
There was a problem hiding this comment.
When a declared include is temporarily absent, especially outside configDir or in a subdirectory, this try? File/file(at:) drops it from includedConfigPaths. ConfigInteractor.armFileWatchLocked derives both file watches and foreign parent-directory watches only from that list, so after an included file is deleted or an editor save leaves a gap, the watcher stops observing its parent and recreating/editing that include no longer triggers hot reload until the main config changes. Keep declared include paths (or at least their parent directories) available for watching even when the file cannot be opened, while still skipping unreadable files during the merge.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ご指摘の通りでした。includes の watch 解決を
非依存に書き換え、欠損 include でも親ディレクトリを watch して
からの作成を
するようにしました。エントリのパースは decode 側の includeFiles と
しているため、watch
と実際にマージされる集合が乖離することはありません。bc1e9c2 で
、fake gateway での
も
済みです。
| let lyrics = configDataSource.load()?.config.lyrics | ||
| let fallbackCommand = lyrics?.fallbackCommand ?? [] | ||
| let timeoutMs = lyrics?.timeoutMs.value ?? 5000 | ||
| let configDir = configDataSource.configDir |
There was a problem hiding this comment.
When load() falls back to the last-good config after a broken higher-priority config appears, this line recomputes configDir from the current on-disk candidate instead of using the ConfigLoadResult that supplied fallbackCommand. A retained command such as $LYRA_CONFIG_DIR/script from the previous config can then expand against the broken new directory and fail the absolute executable check or run the wrong script. Read configDir from the same load result as lyrics whenever a last-good config is served.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
load() が last-good を供給している間に configDir だけ現ディスク候補から再計算される、ご指摘の通りです。
ConfigLoadResult の configDir を使うよう a5fa7d8 で
し、供給元と展開先が乖離するシナリオの
を
しました。
| .target( | ||
| name: "FileWatchGateway", |
There was a problem hiding this comment.
This adds a new Package target, but AGENTS.md's Change Checklist says module additions must update Package.swift, DependencyInjection, docs/ARCHITECTURE.md, .claude/CLAUDE.md, AGENTS.md, and README.md. The diff adds FileWatchGateway/ConfigInteractor but contains no .claude/CLAUDE.md change, leaving one of the repo's documented architecture entrypoints stale for future agents and maintainers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
したところ、AGENTS.md の Change Checklist が無条件に .claude/CLAUDE.md
を求める文言になっていましたが、canonical な規定（.claude/rules/module-checklist.md）では CLAUDE.md
は「Build & Test コマンド
時、またはレイヤー連鎖自体の
時」の条件付きです。今回の FileWatchGateway / ConfigInteractor
はどちらにも該当せず、アーキテクチャ文書は canonical な docs/ARCHITECTURE.md 側を
済みでした。stale だったチェックリスト側の文言を条件付き規定に整合させています（c90aff0）。
| // keeps the old wallpaper playing; rolling the target back | ||
| // means re-saving the same value retries resolution instead | ||
| // of being swallowed by the diff guard (#41 PR4 review, F8). | ||
| targetSource = appliedSource |
There was a problem hiding this comment.
If source B yields its first item and becomes visible, then is canceled by a later source C before B's stream finishes, appliedSource is still the old source A. When C resolves empty, this rollback sets targetSource to A even though B is what remains on screen; a subsequent user revert to A is then swallowed by guard source != targetSource and the wallpaper stays on B while the config says A. Commit the source that actually activated, or roll back to the currently visible source rather than the stale appliedSource.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
実在するでした。appliedSource のコミットが stream 終端にあったため、途中キャンセルで「B が可視なのに appliedSource=A」が成立し得ました。コミットを first item の活性化時へ移動し、applied 状態が常に画面と一致するようにしています。ご提示の ABC シナリオ（B 可視のまま C が空解決 → A の再
が diff guard に飲み込まれる）の
も
しました。6b92d1a です。
| currentIndex = 0 | ||
| wallpaperScale = 1.0 | ||
| targetSource = source | ||
| mode = interactor.playbackMode |
There was a problem hiding this comment.
When a hot reload points at a new wallpaper source whose resolution is slow or ultimately yields zero items, the old playlist intentionally remains visible, but this eagerly replaces its mode before any replacement item is activated. During that window, or forever after an empty resolve, end-of-item handling advances the old playlist using the failed/pending source's shuffle/cycle setting. Defer the mode assignment until the first new item is accepted, or restore the previous mode when the swap is not applied.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
mode の eager コミットも箇所で
しました。pendingMode として保持し first item 受理時にコミットするため、空解決時は旧 playlist が旧 mode のまま advance します。cycle と shuffle が判別できる 3 item + 固定 RandomSource の
付きです。6b92d1a です。
ConfigInteractorImpl が ConfigWatchGateway を直接消費していた段飛ばし (Interactor → OS 境界) を解消し、watchChanges(onChange:) を Interactor → UseCase → Repository → DataSource → Gateway の隣接連鎖に変更。 - ConfigDataSourceImpl.watchChanges + ConfigWatchSession が watch 対象解決 (config file / includes / 外部 include 親 dir) と再アームを所有。 AudioTapDataSource → AudioTapGateway と同じ消費層に揃う - 再アームはイベント毎にディスクの includes を再読するため reload 完了に 依存しない。Files 由来パス同士の比較になり末尾スラッシュ正規化ハックを削除 - ConfigInteractorImpl は debounce・reload 指揮・publish のみに縮退 - UseCase/Repository の configDir・includedConfigPaths 露出を削除 (watch のためだけの露出だった) - watch 対象解決テストを ConfigWatchTests (DataSource 層) へ移設し、 実 FS での include 編集 / atomic save 再アームの統合テストを追加
Gateway は DataSource 層で消費する規定（AudioTap/ConfigWatch 先例）と 隣接層のみの依存チェーンを enforceable ルールとして追記。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (2)
Tests/WallpaperInteractorTests/WallpaperInteractorImplTests.swift (1)
61-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify source freshness on the same interactor instance.
These assertions use two separately initialized interactors, so they still pass if
WallpaperInteractorImplsnapshots the wallpaper during initialization. Use a reference-backed mutableConfigUseCase, change its style, and assert that the existing interactor returns the new source. The presenter tests only validate this boundary through a stub.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/WallpaperInteractorTests/WallpaperInteractorImplTests.swift` around lines 61 - 79, Update wallpaperSourceReflectsConfig to use one existing WallpaperInteractorImpl backed by a mutable reference-based ConfigUseCase, then change the config’s wallpaper style after initialization and assert wallpaperSource returns the updated source; retain coverage for the absent-style case without creating a second interactor.Tests/ConfigUseCaseTests/ConfigUseCaseTests.swift (1)
73-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the cached style, not only outcomes and call counts.
Give the repository distinct initial/reloaded styles, then verify successful reload replaces
appStyleand every invalid outcome leaves it unchanged. The current assertions would miss an erroneous direct reset of the store.As per coding guidelines, last-good configuration retention is required during invalid reloads.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/ConfigUseCaseTests/ConfigUseCaseTests.swift` around lines 73 - 163, Update the reload tests around ConfigUseCaseImpl to configure CountingConfigRepository with distinct initial and reloaded app styles, then assert appStyle equals the reloaded style after successful reloadUpdatesFromDisk. In every invalid reload test—decodeError, existing-file defaults, and unreadable validation—assert appStyle remains the previously loaded style, while preserving the existing outcome, reason, path, and call-count assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-07-12-config-hot-reload-design.md`:
- Around line 146-149: Update the include-watching section of the configuration
hot-reload design to document that ConfigWatch.watchTargets monitors parent
directories for included files, including includes outside the main config
directory. Remove the statement that foreign-directory include tracking is
future work, while preserving the existing same-directory and atomic-save
behavior descriptions.
In `@Sources/ConfigDataSource/ConfigDataSourceImpl.swift`:
- Around line 244-251: Update include handling in includeFiles(of:configDir:) so
missing include paths are retained for directory watching instead of being
discarded by try?. Resolve each include’s watch path independently of whether
the file exists, while ensuring file loading still includes only existing files
and preserves the current relative versus absolute path behavior.
In `@Sources/ConfigDataSource/ConfigWatch.swift`:
- Around line 87-95: Update changed() and stop() to synchronize callback
execution with teardown so stop() cannot return while an onChange callback is in
flight or allow one to begin afterward. Preserve the stopped check under the
existing lock, and ensure callback completion is tracked or serialized before
stop() returns.
- Around line 75-84: The armed() watch setup must recover autonomously when
dataSource.configDir is absent: watch the nearest existing parent and re-arm the
directory watch when it appears, or retry failed arm attempts without requiring
another start() call. Update Sources/ConfigDataSource/ConfigWatch.swift:75-84
accordingly while preserving file-watch rearming. Add coverage in
Tests/ConfigInteractorTests/ConfigInteractorImplTests.swift:110-126 that creates
the directory later without calling start() again and counts failed arm attempts
to verify retry behavior.
In `@Sources/ConfigInteractor/ConfigInteractorImpl.swift`:
- Around line 71-79: Update the reload flow in ConfigInteractorImpl so
invalidSubject.send and appStyleSubject.send occur outside lock.withLock, since
subscribers may synchronously re-enter start() or stop(). Under synchronization,
compute and commit the reload result while preserving the existing token/epoch
and in-flight coordination; then publish the committed result after releasing
the lock, ensuring stop() still prevents any later emissions.
In `@Sources/ConfigUseCase/ConfigUseCaseImpl.swift`:
- Around line 27-35: Update the ConfigUseCaseImpl flow around
repository.validate and loadAppStyle to obtain and use a single validated
AppStyle snapshot from one repository read, eliminating the second loadAppStyle
read. Publish .updated only for that validated style, and preserve the existing
store value when required configuration validation fails.
In `@Sources/Presenters/Spectrum/SpectrumPresenter.swift`:
- Around line 66-114: The presenter lifecycle must be idempotent and
restartable. In Sources/Presenters/Spectrum/SpectrumPresenter.swift lines
66-114, guard start() against duplicate subscriptions and clear appliedStyle in
stop() so an unchanged enabled configuration starts capture again. Apply the
same changes in Sources/Presenters/Wallpaper/RipplePresenter.swift lines 91-183
using start(), stop(), and appliedRipple; retain the single appStyleChanges
subscription.
In `@Sources/Presenters/Wallpaper/WallpaperPresenter.swift`:
- Line 92: Update the playback-mode assignment in the replacement playlist flow
so interactor.playbackMode is committed to mode only after the replacement
becomes active. Keep the existing mode unchanged while the old playlist remains
active, including when the replacement is slow or empty.
In `@Tests/ConfigDataSourceTests/ConfigDataSourceImplTests.swift`:
- Line 141: Update the ConfigDataSource decoding and its tests so malformed
[developer] configuration always throws, regardless of strictOptionalSections;
add assertions for both true and false modes in ConfigDataSourceImplTests.
Preserve the existing behavior where malformed optional [ai] or [lyrics]
sections alone degrade to nil.
In `@Tests/ConfigHotReloadTests/ConfigHotReloadPipelineTests.swift`:
- Around line 62-68: Synchronize all test observation state accessed by Combine
callbacks and assertions, replacing unsynchronized `@unchecked` Sendable fields
with a lock or actor. Apply this to pinged and lastInvalid in
Tests/ConfigHotReloadTests/ConfigHotReloadPipelineTests.swift at lines 62-68 and
138-144, and the update, invalid-result, and teardown observation fields in
Tests/ConfigInteractorTests/ConfigInteractorImplTests.swift at lines 21-27,
52-58, and 140-144; ensure every read and write uses the same synchronization
mechanism.
In `@Tests/ViewsTests/AppWindowTests.swift`:
- Around line 138-169: Update detachPlayerRestoresTransparentBacking to capture
the original AVPlayerLayer before calling AppWindow.detachPlayer, then assert
that its player and superlayer are both nil after detachment instead of
searching the restored hosting view. In detachPlayerNoOpWithoutLayer, snapshot
the surface opacity and overlay background before detaching and assert both
values remain unchanged afterward.
In `@Tests/ViewsTests/ViewRenderingTests.swift`:
- Around line 540-631: Remove the pixel-based corner-placement assertion from
badgeIsCornerAnchoredNotCentered and retain only a rendering smoke test for
ConfigStatusOverlay. Move the badge position calculation into
ConfigStatusGeometry or SwiftUIResolver, then add focused tests for that
geometry/resolver logic while preserving the expected corner anchoring behavior.
---
Nitpick comments:
In `@Tests/ConfigUseCaseTests/ConfigUseCaseTests.swift`:
- Around line 73-163: Update the reload tests around ConfigUseCaseImpl to
configure CountingConfigRepository with distinct initial and reloaded app
styles, then assert appStyle equals the reloaded style after successful
reloadUpdatesFromDisk. In every invalid reload test—decodeError, existing-file
defaults, and unreadable validation—assert appStyle remains the previously
loaded style, while preserving the existing outcome, reason, path, and
call-count assertions.
In `@Tests/WallpaperInteractorTests/WallpaperInteractorImplTests.swift`:
- Around line 61-79: Update wallpaperSourceReflectsConfig to use one existing
WallpaperInteractorImpl backed by a mutable reference-based ConfigUseCase, then
change the config’s wallpaper style after initialization and assert
wallpaperSource returns the updated source; retain coverage for the absent-style
case without creating a second interactor.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36fa0e3e-750c-4ec0-a585-85e4472e0cf6
📒 Files selected for processing (80)
AGENTS.mdPackage.swiftREADME.mdSources/AppRouter/AppDependencyBootstrap.swiftSources/AppRouter/AppRouter.swiftSources/AppRouter/OverlayWindow.swiftSources/ConfigDataSource/ConfigDataSourceImpl.swiftSources/ConfigDataSource/ConfigWatch.swiftSources/ConfigInteractor/ConfigInteractorImpl.swiftSources/ConfigRepository/ConfigRepositoryImpl.swiftSources/ConfigUseCase/ConfigUseCaseImpl.swiftSources/DependencyInjection/GatewayRegistration.swiftSources/DependencyInjection/InteractorRegistration.swiftSources/Domain/DataSource/ConfigDataSource.swiftSources/Domain/Interactor/ConfigInteractor.swiftSources/Domain/Interactor/WallpaperInteractor.swiftSources/Domain/Misc/ConfigWatchGateway.swiftSources/Domain/Repository/ConfigRepository.swiftSources/Domain/UseCase/ConfigUseCase.swiftSources/Entity/Config/ConfigReloadFailure.swiftSources/Entity/Config/ConfigReloadOutcome.swiftSources/Entity/Style/SpectrumStyle.swiftSources/FileWatchGateway/FileWatchGateway.swiftSources/LyricsDataSource/CustomScriptLyricsDataSourceImpl.swiftSources/Presenters/App/AppPresenter.swiftSources/Presenters/Config/ConfigStatusPresenter.swiftSources/Presenters/Spectrum/SpectrumPresenter.swiftSources/Presenters/Track/HeaderPresenter.swiftSources/Presenters/Track/LyricsPresenter.swiftSources/Presenters/Wallpaper/RipplePresenter.swiftSources/Presenters/Wallpaper/WallpaperPresenter.swiftSources/SpectrumUseCase/SpectrumUseCaseImpl.swiftSources/VersionHandler/Resources/version.txtSources/Views/Overlay/AppWindow.swiftSources/Views/Overlay/ConfigStatusGeometry.swiftSources/Views/Overlay/ConfigStatusOverlay.swiftSources/Views/Overlay/ConfigStatusRenderer.swiftSources/Views/Overlay/OverlayContentView.swiftSources/WallpaperInteractor/WallpaperInteractorImpl.swiftTests/AppRouterTests/AppLaunchEnvironmentTests.swiftTests/ConfigDataSourceTests/ConfigDataSourceImplTests.swiftTests/ConfigDataSourceTests/ConfigWatchTests.swiftTests/ConfigDataSourceTests/IncludesTests.swiftTests/ConfigDataSourceTests/TryDecodeTests.swiftTests/ConfigHandlerTests/ConfigHandlerImplTests.swiftTests/ConfigHotReloadTests/ConfigHotReloadPipelineTests.swiftTests/ConfigInteractorTests/ConfigInteractorImplTests.swiftTests/ConfigRepositoryTests/ConfigRepositoryTests.swiftTests/ConfigUseCaseTests/ConfigUseCaseTests.swiftTests/FileWatchGatewayTests/FileWatchGatewayTests.swiftTests/LyricsDataSourceTests/CustomScriptLyricsDataSourceImplTests.swiftTests/MetadataDataSourceTests/LLMMetadataDataSourceImplTests.swiftTests/MetadataDataSourceTests/LLMMetadataDataSourceResolveTests.swiftTests/PresentersTests/AppPresenterTests.swiftTests/PresentersTests/ConfigStatusPresenterTests.swiftTests/PresentersTests/FakeConfigInteractor.swiftTests/PresentersTests/HeaderPresenterTests.swiftTests/PresentersTests/LyricsPresenterTests.swiftTests/PresentersTests/RipplePresenterTests.swiftTests/PresentersTests/RippleScreenBoundsTests.swiftTests/PresentersTests/SpectrumPresenterTests.swiftTests/PresentersTests/TestSupport/WaitUntil.swiftTests/PresentersTests/WallpaperPresenterTests.swiftTests/ScopedAPISessionTests/ScopedAPISessionTests.swiftTests/ScreenInteractorTests/ScreenInteractorImplTests.swiftTests/SpectrumInteractorTests/SpectrumInteractorImplTests.swiftTests/SpectrumUseCaseTests/SpectrumUseCaseImplTests.swiftTests/TrackInteractorTests/TrackInteractorAIProcessingTests.swiftTests/TrackInteractorTests/TrackInteractorArtworkTests.swiftTests/TrackInteractorTests/TrackInteractorFallbackDisplayTests.swiftTests/TrackInteractorTests/TrackInteractorPlaybackPositionTests.swiftTests/TrackInteractorTests/TrackInteractorRaceTests.swiftTests/TrackInteractorTests/TrackInteractorStyleTests.swiftTests/ViewsTests/AppWindowTests.swiftTests/ViewsTests/ConfigStatusGeometryTests.swiftTests/ViewsTests/ViewRenderingTests.swiftTests/WallpaperInteractorTests/WallpaperInteractorImplTests.swiftdocs/ARCHITECTURE.mddocs/superpowers/plans/2026-07-12-config-hot-reload-pr1-core.mddocs/superpowers/specs/2026-07-12-config-hot-reload-design.md
| lock.withLock { | ||
| guard token != nil, !Task.isCancelled else { return } | ||
| switch configUseCase.reload() { | ||
| case .updated: | ||
| invalidSubject.send(nil) | ||
| appStyleSubject.send(()) | ||
| case .invalid(let failure): | ||
| invalidSubject.send(failure) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Do not execute Combine subscribers while holding lock.
Subject.send invokes subscribers synchronously. Any subscriber that calls start() or stop() will re-enter this non-recursive NSLock and deadlock the reload task.
Compute and commit the lifecycle result under synchronization, then publish outside the lock while retaining an epoch/in-flight mechanism so stop() can still guarantee no later emissions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/ConfigInteractor/ConfigInteractorImpl.swift` around lines 71 - 79,
Update the reload flow in ConfigInteractorImpl so invalidSubject.send and
appStyleSubject.send occur outside lock.withLock, since subscribers may
synchronously re-enter start() or stop(). Under synchronization, compute and
commit the reload result while preserving the existing token/epoch and in-flight
coordination; then publish the committed result after releasing the lock,
ensuring stop() still prevents any later emissions.
There was a problem hiding this comment.
lock 内 publish は round 3 の指摘（stop() の spurious emission）への
と表裏の
的なトレードオフです。全購読者が receive(on: DispatchQueue.main) で hop してから処理するため、send での再入や lock の長時間保持は
しません。lock 外 send に変えると post-stop emission race が
します。この
を publish ブロックのコメントとして明文化しました（c90aff0）。
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline Sources/ConfigInteractor/ConfigInteractorImpl.swift --items all
sed -n '1,150p' Sources/ConfigInteractor/ConfigInteractorImpl.swift
printf '\n--- ConfigInteractor publisher consumers ---\n'
rg -n -U -P \
'(?s)\b(configInteractor\.(?:appStyleChanges|invalidConfig)|\.(?:appStyleChanges|invalidConfig)\b).*?(?:receive\(on:\s*DispatchQueue\.main\)|sink\s*\{|assign\s*\()' \
Sources TestsLength of output: 9784
The Comment images 3–8 show stylized Japanese glyphs: 後 (“after”), 修正 (“fix”), 意図 (“intent”), 発生 (“occur”), 再発 (“recur”), and 制約 (“constraint”). They emphasize the intentional concurrency trade-off described in the comment.
@GeneralD, thanks for documenting the post-stop() guarantee and the deliberate trade-off. I confirmed that every current production consumer applies .receive(on: DispatchQueue.main) before its sink.
However, I can’t withdraw the finding: the publisher APIs are public and do not enforce that convention. There are already direct synchronous test subscribers in Tests/ConfigInteractorTests/ConfigInteractorImplTests.swift and Tests/ConfigHotReloadTests/ConfigHotReloadPipelineTests.swift. They do not currently re-enter the interactor, but a synchronous present or future subscriber that calls start() or stop() would deadlock on this non-recursive NSLock.
The comment makes the invariant visible, which is valuable, but it remains convention-based rather than guaranteed by the API. Keeping publication outside the lock with a generation/epoch check remains the robust option if this interactor must be safe for arbitrary subscribers.
🐇💭
✏️ Learnings added
Learnt from: GeneralD
Repo: GeneralD/lyra PR: 337
File: Sources/ConfigInteractor/ConfigInteractorImpl.swift:78-86
Timestamp: 2026-07-17T11:06:30.983Z
Learning: For `Sources/ConfigInteractor/ConfigInteractorImpl.swift`, the hot-reload design intentionally prioritizes preventing post-`stop()` reload emissions: reload state checks and the current Combine subject sends are serialized under the lifecycle lock. Current production presenter subscribers use `receive(on: DispatchQueue.main)`, but this queue-hop convention is not enforced by the public `ConfigInteractor` publisher API.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
includes の watch 対象解決を存在非依存化。従来は try? File で欠損 include が落ち、外部ディレクトリの include を後から作成しても watch が張られず検知不能だった。エントリのパース（includePaths）を decode の includeFiles と共有して drift を防ぎ、親ディレクトリは存在時のみ Files 正規化して configDir と比較する。
壊れた編集がディスクにある間、load() は last-good config を供給するが、 configDir だけは現ディスク候補から再計算されており、$LYRA_CONFIG_DIR の展開先が供給中の config の出自と食い違い得た。同じ ConfigLoadResult の configDir を使い、常に供給された config と同じディレクトリを指す。
appliedSource のコミットが stream 終端にあったため、後続 load による 途中キャンセルで「B が可視なのに appliedSource=A」が成立し、空解決 ロールバックが stale な A を指して可視 source の再保存が diff guard に 飲み込まれた。mode も同様に eager コミットだったため、空解決時に旧 playlist が新 mode で advance してしまっていた。両方を first item の 可視化と同時にコミットし、状態が常に画面と一致するようにする。
…tus） stop() 後も applied sentinel が残るため、同一 config での再 start() が 「変化なし」と diff されて capture / mouse monitor が復帰しなかった。 stop() で sentinel をクリアし、start() には重複購読を防ぐ guard を追加。 ConfigStatusPresenter の二重 start() も購読と interactor.start() が重複 しないよう冪等化する。
sink コールバック（interactor のワーカーコンテキスト）から書き、テスト の polling ループから読む plain var は未定義動作。5 箇所の Observed を NSLock 保護のアクセサに置き換える。
- AppWindow detach: AVPlayerLayer を detach 前に捕捉し player 解放と superlayer 除去を直接検証（復元後 view の sublayers 探索は空振りで 自明に pass していた） - WallpaperInteractor: 同一インスタンス上で config 変更後の wallpaperSource live 追従を検証（init スナップショット化の退行を検知） - ConfigUseCase: reload 成功で store が新 style に置換され、invalid 系 では last-good が保持されることを style 値で直接 assert
- ConfigWatchToken: stop() は in-flight callback を同期待ちしない契約を doc 化（待つとロック順序が逆転しデッドロック。消費側が teardown 状態 で gate する — ConfigInteractorImpl.applyReload が実例) - ConfigInteractorImpl: lock 内 publish が安全なのは全購読者が main hop するためという制約コメントを追記 - spec: 別ディレクトリ include の「将来課題」記述を実装済みに更新 - AGENTS.md: Change Checklist の .claude/CLAUDE.md 更新を module-checklist.md の条件付き規定に整合
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/ConfigDataSourceTests/ConfigWatchTests.swift (1)
210-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
try#require`` to prevent opaque test timeouts if the file cannot be opened.If
FileHandle(forWritingAtPath:)returnsnil, the optional chaining silently skips the write. This causes the test to fail later with a generic timeout rather than an immediate, clear file-access failure.
Tests/ConfigDataSourceTests/ConfigWatchTests.swift#L210-L213: unwrapincludeHandlewithtry#require`` before using it.Tests/ConfigDataSourceTests/ConfigWatchTests.swift#L233-L236: unwrapconfigHandlewithtry#require`` before using it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/ConfigDataSourceTests/ConfigWatchTests.swift` around lines 210 - 213, Replace optional chaining on the file handles with required unwrapping via try `#require` before performing file operations. Apply this to includeHandle at Tests/ConfigDataSourceTests/ConfigWatchTests.swift:210-213 and configHandle at Tests/ConfigDataSourceTests/ConfigWatchTests.swift:233-236, preserving the existing seek, write, and close behavior after successful unwrapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Tests/ConfigDataSourceTests/ConfigWatchTests.swift`:
- Around line 210-213: Replace optional chaining on the file handles with
required unwrapping via try `#require` before performing file operations. Apply
this to includeHandle at
Tests/ConfigDataSourceTests/ConfigWatchTests.swift:210-213 and configHandle at
Tests/ConfigDataSourceTests/ConfigWatchTests.swift:233-236, preserving the
existing seek, write, and close behavior after successful unwrapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e8afcafe-cea2-47ca-a632-3dfd45ba9847
📒 Files selected for processing (24)
.claude/rules/architecture-boundaries.mdAGENTS.mdSources/ConfigDataSource/ConfigDataSourceImpl.swiftSources/ConfigDataSource/ConfigWatch.swiftSources/ConfigInteractor/ConfigInteractorImpl.swiftSources/Domain/Misc/ConfigWatchGateway.swiftSources/LyricsDataSource/CustomScriptLyricsDataSourceImpl.swiftSources/Presenters/Config/ConfigStatusPresenter.swiftSources/Presenters/Spectrum/SpectrumPresenter.swiftSources/Presenters/Wallpaper/RipplePresenter.swiftSources/Presenters/Wallpaper/WallpaperPresenter.swiftTests/ConfigDataSourceTests/ConfigWatchTests.swiftTests/ConfigDataSourceTests/IncludesTests.swiftTests/ConfigHotReloadTests/ConfigHotReloadPipelineTests.swiftTests/ConfigInteractorTests/ConfigInteractorImplTests.swiftTests/ConfigUseCaseTests/ConfigUseCaseTests.swiftTests/LyricsDataSourceTests/CustomScriptLyricsDataSourceImplTests.swiftTests/PresentersTests/ConfigStatusPresenterTests.swiftTests/PresentersTests/RipplePresenterTests.swiftTests/PresentersTests/SpectrumPresenterTests.swiftTests/PresentersTests/WallpaperPresenterTests.swiftTests/ViewsTests/AppWindowTests.swiftTests/WallpaperInteractorTests/WallpaperInteractorImplTests.swiftdocs/superpowers/specs/2026-07-12-config-hot-reload-design.md
🚧 Files skipped from review as they are similar to previous changes (16)
- Sources/Domain/Misc/ConfigWatchGateway.swift
- Tests/ConfigInteractorTests/ConfigInteractorImplTests.swift
- Sources/Presenters/Config/ConfigStatusPresenter.swift
- Tests/ConfigHotReloadTests/ConfigHotReloadPipelineTests.swift
- Tests/PresentersTests/ConfigStatusPresenterTests.swift
- Sources/ConfigInteractor/ConfigInteractorImpl.swift
- Sources/Presenters/Spectrum/SpectrumPresenter.swift
- Sources/LyricsDataSource/CustomScriptLyricsDataSourceImpl.swift
- Sources/ConfigDataSource/ConfigDataSourceImpl.swift
- Tests/ViewsTests/AppWindowTests.swift
- Tests/PresentersTests/WallpaperPresenterTests.swift
- Sources/ConfigDataSource/ConfigWatch.swift
- Tests/ConfigUseCaseTests/ConfigUseCaseTests.swift
- Sources/Presenters/Wallpaper/WallpaperPresenter.swift
- docs/superpowers/specs/2026-07-12-config-hot-reload-design.md
- Tests/LyricsDataSourceTests/CustomScriptLyricsDataSourceImplTests.swift
|
Round 4 レビューの nitpick 2 件（review body 内）も両方
63d2b5e です。actionable 13 件への |
     Closes #338 ## 概要 config ディレクトリ（`$XDG_CONFIG_HOME/lyra` など）が daemon 起動時に存在しない場合、これまでは watch を張る fd が無く `watchChanges` が nil を返すため、後からディレクトリと config を作成しても daemon の再起動が必要だった（#329 で明示的にスコープ外とした制約、#337 Round 4 レビューで再指摘）。 本 PR は「**祖先 watch + 段階的 re-arm**」でこの制約を撤廃する。 ## 設計 `ConfigWatchSession` の directory tier を 2 状態にした（新規 tier は追加せず、既存の re-arm 機構に統合）: - **target モード** — 従来どおり config dir を watch。イベント毎に file tier を re-arm - **ancestor park モード** — config dir を watch できないときは、`directoryAncestors`（純関数）で得た祖先チェーン（例: `~/.config` → `~` → `/`）を近い順に試し、最初に arm できた祖先へ park 祖先イベントの扱いが issue の検討事項「頻度が高いのでフィルタ必須」への回答: - 祖先イベント毎に directory tier を再解決し、**パスが出現した分だけ watch を下位へ張り替える**（`mkdir -p` の一括作成は一発で config dir まで到達） - **config dir 本体への昇格が成立したときだけ** file tier を arm して `onChange` を発火（= initial load）。祖先ノイズ（`$HOME` 直下の変化など）は syscall 数回で終わり、decode/reload は一切走らない - 昇格 ping 自体は上流 `ConfigInteractor` の 150ms debounce が他イベントと同様に吸収するため、この層に追加の debounce は不要 おまけの対称性として、**稼働中に config dir が削除された場合**も同機構で祖先へ降格 park する（従来は dead fd のまま沈黙し再起動が必要だった）。再作成されれば昇格して復帰する。 ## 実装メモ - 祖先探索は `lazy.compactMap { … }.first` だと **lazy collection の transform 二重評価**（`startIndex` 計算時と subscript 時）で watch が重複 arm され fd がリークするため、素直なループで実装（コメントに理由を明記） - `FakeWatchGateway` を live gateway と同じ「ディレクトリが実在するときだけ arm 成功」に変更し、cancel 済み handler は発火しない live handler 管理へ（cancelled `DispatchSource` はイベントを配送しないことのミラー） - `armed()` が nil を返すのは祖先チェーン全滅時のみ（live では `/` が常に存在するため実質発生しない） ## テスト - 祖先 park / 昇格 + initial load / ノイズフィルタ / 中間ディレクトリのみ出現時の walk-down / 稼働中削除→降格→復帰 / `directoryAncestors` 純関数(fake gateway 駆動) - 実 FS + 実 `FileWatchGateway` で「config dir を後から作成 → 昇格 → onChange 発火 → in-place 編集も file tier で検知」の E2E - 全 1355 テスト green、`make lint` クリーン ## ドキュメント - `docs/ARCHITECTURE.md` / `AGENTS.md` の「provided the config directory itself exists at start」制約記述を新契約に更新 - `ConfigWatchSession` / `ConfigDataSource.watchChanges` の doc コメント更新 ## バージョン feat のため minor bump: 2.27.2 → 2.28.0 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved configuration hot reload when configuration folders are missing, deleted, recreated, or replaced during saves. - Configuration changes are now detected more reliably after directory moves, renames, and atomic file updates. - Added recovery when configuration locations become available after application startup. - **Documentation** - Clarified configuration hot-reload behavior and recovery scenarios in the project documentation. - **Release** - Updated the application version to 2.28.0. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Closes #41
概要
config.tomlの編集を daemon 再起動なしで全ビジュアル要素へライブ反映する #41 の集大成。開発ブランチfeat/41-hot-reload/devに積み上げた PR1〜PR4（6 コミット）をmainへ統合します。これで config 編集にlyra restartは一切不要になります。変更内容
設計仕様
docs/superpowers/specs/2026-07-12-config-hot-reload-design.mdに沿ったスタック PR の集約です。ConfigUseCaseImplを lazy-frozen ロードから mutable なOSAllocatedUnfairLock<AppStyle>ストア＋reload() -> ConfigReloadOutcomeに置換（失敗時はストア不変で.invalidに理由を載せる）。FileWatchGateway（DispatchSource.makeFileSystemObjectSource、親ディレクトリ監視で atomic rename に強い）＋ConfigInteractorImpl（150ms デバウンス、appStyleChanges/invalidConfigを publish）。不正 config はConfigStatusOverlayが amber の崩れた測地線インジケータで可視化。lyrics の[lyrics] fallback_command/timeout_msも呼び出しごと再読込に。HeaderPresenter/LyricsPresenterをstart()（購読は一度だけ）と冪等なapplyStyle()（live config を@Publishedへ流す）に分割。購読を張り替えないので旧 push モデルの二重購読・リプレイ・monitor リークが構造的に消える。DecodeEffectStateで decode エフェクトを境界ごとに再構築。[ai]/[lyrics]セクション検証を起動時の寛容判定に揃える（不正な拡張セクションはnil降格で、正当な text/wallpaper 編集をブロックしない。healthcheckのみ厳格）。enabledトグルとスタイルをホットリロード。RipplePresenter.applyStyle()は ripple config を diff（rebuild 時に hover フラグをリセット、原点リップル湧出を防止）、SpectrumPresenter.applyStyle()はSpectrumStyle全体を diff しobjectWillChange再発行でSpectrumViewを再描画。バンド設定はresolvedAnalyzerのメモ化キーを全ビルド入力に拡張して即反映。DisplayLink のフレームハンドラは常時装着し、enabled ガードは各ハンドラ内へ移動（bug: パフォーマンス低下と電力消費・発熱・ファン回転の悪化 #252/観測タスク: lyra daemon (28% CPU) のホットパスを sample / Instruments で特定 #258 のゼロアイドルコストを維持）。WallpaperPresenterをstart()/applyStyle()に分割し、WallpaperInteractor.wallpaperSourceを diff して実変化時のみ再解決（無関係な編集で再生中の動画に触れない）。ソース差替は同一 AVPlayer 上でreplaceCurrentItem（ブラックアウトなし）、全削除時は player を破棄しonPlayerCleared→AppWindow.detachPlayerLayer()でAVPlayerLayerを外して透過背景へ復帰（黒画面残留を解消）。applied source は解決成功時のみ確定し、一時的な 0 件はリトライ可能に。superseded な load は空クリーンアップ前に bail。AppPresenterはappStyleChangesを layout パイプラインへ合流し、[screen]selector 変化でresolveLayout()再実行＋vacant ポーリングを張り直す。DeveloperLog/歌詞解決の判定トレースを残すデバッグログを config で追加 (#326 観測基盤) #331）とLyraKit.md更新を取り込み。衝突は加算的な docs（ARCHITECTURE の Mermaid ノード・Layer Summary・Key Design Decisions にFileWatchGateway/Watch config file changes and apply without restart #41 とDeveloperLog/歌詞解決の判定トレースを残すデバッグログを config で追加 (#326 観測基盤) #331 を併記）と version のみで、両者を結合して解消。AppRouter（Router=ワイヤーフレーム）が@Dependency(\.configInteractor)を直接保持しstart()/stop()を呼んでいたのを、front するConfigStatusPresenterへ移譲（SpectrumPresenterがSpectrumInteractorを持つのと同型）。Router が Interactor 層へ直接触れなくなり、AppRouter.stop()のwithBootstrap { configInteractor.stop() }回避策も不要になり削除。WallpaperPresenterに pending なtargetSourceを導入し、スロー解決中の無関係な編集ピンで in-flight ダウンロードを再起動しない＋applied source への revert は正しく pending swap をキャンセル。② 旧 playlist/scale のリセットを「置換の最初の item 到着まで」遅延し、空解決時も旧壁紙が scale 跳ね・advance 不能にならない。③ConfigDataSourceImpl.load()に last-good 保持を追加し、不正編集がディスク上にある間も[lyrics]fallback /[ai]endpoint が直前の承認済み config で動き続ける（ファイル削除は defaults 復帰）。④ 重複していたテスト待機ヘルパーは Copilot の suggested changeset を採用して共有waitUntilに統一。⑤ConfigInteractorImplの teardown race を解消 — debounce タスクの armed/cancelled チェックと publish をstop()と同一 lock 内で原子的に実行し、stop 後の spurious publish を構造的に排除。⑥reload()のexistingConfigPathを先頭で一度だけ捕捉し、atomic save 中でも存在判定と報告 path が乖離しない（suggested changeset 採用）。AppRouterのインライン default windowFactory をdefaultFrameSchedulerFactoryと同型の static メソッドへ抽出して実AppWindow構築を直接テスト、壁紙削除リロード→detachPlayerLayerの end-to-end 検証、ConfigUseCaseの.unreadable経路、WallpaperInteractor.wallpaperSource委譲、OverlayContentViewの configStatus 分岐レンダリングを追加。cp・追記・truncate などの in-place 上書きではリロードが一切走らなかった（手動テストで「構文が正しいのにエラー表示が残る」「live reload が効かない」として顕在化）。ConfigWatchGatewayにwatch(file:)を追加した二段 watch とし、file fd は rename で無効化されるためイベント毎に再アーム。さらに縮小ftruncateはNOTE_WRITEではなくNOTE_ATTRIBのみ発火するため eventMask に.attribを追加（chmod/touch 等の偽イベントは debounce＋冪等 reload が吸収）。加えてincludesで取り込んだファイルも watch 対象に: decode のマージと同じ解決ヘルパーで watch 対象を算出（乖離なし）し、configDir 外の include はその親ディレクトリも watch する（atomic save 対策）。実機の lsof で発見した configDir 二重 watch も解消。ConfigInteractorImplがConfigWatchGateway（OS 境界）を直接消費する段飛ばしになっていたため、watchChanges(onChange:)をConfigInteractor → ConfigUseCase → ConfigRepository → ConfigDataSource → ConfigWatchGatewayの隣接連鎖に再配置。watch 対象解決（config file / includes / 外部 include 親 dir）と再アームはConfigDataSourceImplのConfigWatchSessionが所有し（AudioTapDataSource → AudioTapGatewayと同じ消費層）、再アームはイベント毎にディスクのincludesを再読するため reload 完了に依存しない。Files 由来パス同士の比較になったことで末尾スラッシュ正規化ハックも消滅。ConfigInteractorImplは debounce・reload 指揮・publish のみに縮退し、watch のためだけに UseCase/Repository へ露出していたconfigDir/includedConfigPathsは契約から削除。watch 対象解決のテストはConfigWatchTests（DataSource 層、実 FS での include 編集・atomic save 再アーム統合テスト含む）へ移設。背景・動機
従来は config 編集のたびに
lyra restartが必要で、オーバーレイ表示のチューニング体験を大きく損なっていた。ホットリロードは #41 最難関の課題で、過去に「購読を push で張り替える」実装が二重購読・壁紙ブラックアウト・monitor リークで破綻していた。本統合は全 Presenter をstart()（購読は一度だけ）＋冪等なapplyStyle()（live config を diff） の単一パターンに統一し、その破綻要因を構造的に排除する。テスト計画
ConfigStatusPresenterの lifecycle 検証＋レビュー対応の回帰テスト＋patch coverage 100% 化のテスト群＋watch バグ修正の file watch/truncate テスト群＋DataSource 層移設後のConfigWatchTests＋Round 4 対応の回帰テスト群を含む）make lintclean>>）→ エラー検知・インジケータ表示（修正前は未検知）.attribで解消）備考
feat/41-hot-reload/devへ squash マージ済み。本 PR のマージで Watch config file changes and apply without restart #41 が default ブランチ上でクローズされます。Summary by CodeRabbit
New Features
Bug Fixes
[ai]/[lyrics]sections no longer block otherwise valid updates.Documentation