feat(#338): config dir 起動時不在でも祖先 watch で自動リカバリ#339
Conversation
ConfigWatchSession の directory tier を 2 状態化: config dir を watch できないときは最も近い実在祖先に park し、祖先イベント毎に再解決して パス出現に合わせて watch を下位へ張り替える。config dir 本体への昇格が 成立したときだけ file tier を arm して onChange（initial load）を発火 するため、祖先ノイズ（$HOME 直下の変化など）は syscall 数回で済み decode/reload は走らない。稼働中の config dir 削除も同機構で祖先へ 降格 park し、再作成で復帰する。 FakeWatchGateway は live gateway と同じく「ディレクトリが実在するときだけ arm 成功」とし、cancel 済み handler は発火しないよう live handler 管理に 変更。祖先探索は lazy.compactMap.first だと lazy collection の transform 二重評価で fd が重複 arm されるため、素直なループで実装。
#329 時点の「config dir が起動時に存在する場合に限る」制約記述を、 祖先 park → 段階的張り替え → 昇格時 initial load / 削除時降格の 新契約に更新。
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughConfig hot-reload watching now parks on existing ancestors when the config directory is absent, promotes or demotes directory watches as paths change, and re-arms directory and file tiers on every event. Tests cover inode replacement, lifecycle transitions, and live edits. Documentation and the version are updated. ChangesConfig watch recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ConfigWatchSession
participant AncestorDirectory
participant ConfigDirectory
participant FileWatchGateway
ConfigWatchSession->>FileWatchGateway: armDirectoryTierLocked()
FileWatchGateway-->>ConfigWatchSession: watch nearest ancestor
AncestorDirectory->>ConfigWatchSession: ancestorChanged()
ConfigWatchSession->>FileWatchGateway: resolve and watch config directory
ConfigWatchSession->>FileWatchGateway: re-arm file tier
ConfigWatchSession-->>ConfigDirectory: onChange()
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
This PR extends Lyra’s config hot-reload watch pipeline so that if the config directory (e.g. $XDG_CONFIG_HOME/lyra) is missing at daemon start, the watcher parks on the nearest existing ancestor directory and automatically “walks down” / promotes once the config directory appears (and demotes/re-recovers if it’s deleted while running).
Changes:
- Implement ancestor-parking + promotion/demotion logic in
ConfigWatchSession(including a puredirectoryAncestors(of:)helper). - Add unit + live-FS E2E tests covering ancestor park/promote/noise filtering and recovery after deletion.
- Update docs/contracts and bump version to
2.28.0.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/ConfigDataSourceTests/ConfigWatchTests.swift | Adds #338 regression tests and updates the fake watch gateway behavior used by tests. |
| Sources/ConfigDataSource/ConfigWatch.swift | Implements ancestor park/promote/demote watch-tier behavior and adds directoryAncestors(of:). |
| Sources/Domain/DataSource/ConfigDataSource.swift | Updates watchChanges contract docs to reflect ancestor parking behavior. |
| Sources/VersionHandler/Resources/version.txt | Minor version bump to 2.28.0. |
| docs/ARCHITECTURE.md | Updates hot-reload architecture docs to reflect new ancestor parking behavior. |
| AGENTS.md | Updates repository-level contract summary for config watch behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let handle = FileHandle(forWritingAtPath: lyraDir + "/config.toml") | ||
| try handle?.seekToEnd() | ||
| try handle?.write(contentsOf: Data("\n# edited\n".utf8)) | ||
| try handle?.close() |
There was a problem hiding this comment.
ba6f82f でしました。
try #require(FileHandle(forWritingAtPath:)) にし、open
は
提条件の
として
に顕在化するようにしています（期待値タイムアウトに化けて原因が読めなくなるのを
）。同パターンだった
liveGatewayObservesIncludeEditsAndSurvivesAtomicSave の 2 箇所も同時にしました。なお同
の Suggested changeset は、適用
を広げたローカル
で取り込んだため widget 側は
になっています。
| func watch(file: String, onChange: @escaping @Sendable () -> Void) -> (any ConfigWatchToken)? { | ||
| lock.withLock { | ||
| _watchedFiles.append(file) | ||
| return FakeToken { [weak self] in self?.recordStop() } | ||
| } |
There was a problem hiding this comment.
これは的な非
です。directory watch の成否は
ConfigWatchSession の制御フロー（祖先 park / 昇格の分岐）に直結するため、この で fake を実在ベースに変えました。一方 file watch の成否に session が分岐することはなく、
した arm は
compactMap で落ちるだけです。fake が「要求されたターゲット」を記録するのはがターゲット解決の契約 — 欠損 include も watch 要求され続けること（
keepsMissingIncludesInWatchTargets）— をするためで、実在ベースにするとこの契約自体が
できなくなります。実際の arm セマンティクス（missing file → nil）は実
FileWatchGateway を使う live
側でカバーしています。
を防ぐため、この
を
watch(file:) の doc コメントとして ba6f82f で明文化しました。
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…tch 契約をコメント明文化 Copilot 指摘: FileHandle(forWritingAtPath:) の optional chaining は open 失敗時に編集が黙って skip され、失敗が期待値タイムアウトとして現れて 原因が読めない。try #require で前提条件の失敗として即座に落とす （新規テスト 1 箇所 + 同パターンの既存 2 箇所も統一）。 FakeWatchGateway.watch(file:) が常に成功するのは意図的（session は file-arm 成否で分岐せず、テストはターゲット解決を検証する）である旨を doc コメントに明文化。
![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: 9554b95614
ℹ️ 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 ancestorPath == nil, (try? Folder(path: dataSource.configDir)) == nil else { return } | ||
| armDirectoryTierLocked() |
There was a problem hiding this comment.
When the watched config directory is removed and recreated before the dispatch handler runs, this existence check succeeds for the new directory and skips armDirectoryTierLocked(), leaving directoryToken attached to the deleted vnode. If the recreated directory is initially empty, rearmFilesLocked() also has no file tier to install, so a later config.toml creation in the new directory is never observed until some unrelated restart/re-arm occurs, which breaks the intended recovery for directories that come and go.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
実でした。b47d456 で
しています。指摘のシナリオを退行
として書いたところ、
前に赤・
後に緑になることを実測で
できました。
の方針は、
チェックを inode
へ精密化するのではなく、チェック自体の
です。
changed() で directory tier も file tier とく毎イベント無条件に re-arm します。
は 2 つあります。
チェックでは inode の
を
的に
できず、精密化しても「チェックが十分か」を考え続けることになること。そして再解決のコストが
open(2) 一回で、file tier が既に全 fd を張り直している以上ほぼ無視できることです。として「全 tier が毎イベント disk から再解決する」という一様な不変条件に収束し、
reparkDirectoryTierIfGoneLocked はになったので
しました。
再 arm と reload の（directory → files → onChange、いずれも disk を読み直す）により、fd 張り替えの隙間に落ちた作成もその
の disk 読みが拾うため、無条件化による取りこぼしはありません。
側も併せて直しました。
FakeWatchGateway が arm 時の inode を pin し、「された watch は最後の
.delete を配送したは二度と発火しない」という実 vnode の
を
します。これが無いと re-arm 漏れが
から
できず、まさにこの指摘を取りこぼしていました。
Codex レビュー指摘の実バグ。config dir が削除→即再作成されると、 ハンドラ実行時にはパスが再び存在するため存在チェックが通り、 reparkDirectoryTierIfGoneLocked が re-arm を skip していた。fd は削除済み vnode を掴んだままなので二度と発火せず、新ディレクトリに後から置かれる config.toml が永久に観測されない（#338 が意図した復帰が壊れる）。 修正は staleness チェックの精密化ではなく撤廃。changed() で directory tier も file tier と同じく毎イベント無条件に re-arm する。パス存在チェックでは inode 置換を原理的に検知できず、チェックこそが間違えどころ。再解決コストは open(2) 一回で、file tier が既に全 fd を張り直している以上ほぼ無視できる。 結果として「全 tier が毎イベント disk から再解決する」という一様な不変条件に 収束し、reparkDirectoryTierIfGoneLocked は不要になり削除。 テスト: FakeWatchGateway を実 vnode セマンティクスに寄せ、arm 時の inode を pin して「削除された watch は最後の .delete を配送した後は二度と発火しない」 を再現。これにより re-arm 漏れが観測可能になり、退行テストが修正前に赤・ 修正後に緑となることを実測で確認。stop カウンタは file/directory に分離。
![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
🤖 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 `@Sources/ConfigDataSource/ConfigWatch.swift`:
- Around line 158-161: Update armDirectoryTierLocked to preserve the existing
directoryToken and ancestorPath until a replacement watch is successfully
created; only stop and clear the active watch after securing the replacement. If
gateway.watch returns nil, retain the current token or schedule a retry, while
ensuring directory watch tiers are re-armed from disk after each event.
🪄 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: ae04bbdf-d342-4dd0-a546-4ec6e0a99e51
📒 Files selected for processing (6)
AGENTS.mdSources/ConfigDataSource/ConfigWatch.swiftSources/Domain/DataSource/ConfigDataSource.swiftSources/VersionHandler/Resources/version.txtTests/ConfigDataSourceTests/ConfigWatchTests.swiftdocs/ARCHITECTURE.md
CodeRabbit レビュー指摘。armDirectoryTierLocked が「stop してから open」の 順だったため、再 arm 時に open(2) が全滅すると（fd 枯渇 — 祖先チェーンが 行き着く / すら開けない状況）directoryToken が nil のまま残り、以後どの イベントも届かないので再試行の契機すら失われ、daemon 再起動まで恒久的に 無反応になる。#338 が消そうとした状態そのもの。 しかも直前の「毎イベント無条件 re-arm」化により、この窓は dir 消失時のみ から全 config 編集へ広がっていた。露出を持ち込んだのはこちらの変更なので 併せて塞ぐ。 freshDirectoryTierLocked() を state 非破壊の純粋な取得に切り出し、成功時 のみ stop → swap する stage-and-swap へ。全滅時は現行 token を維持する: 生きている可能性があり、少なくとも nil より悪くはならない。retry timer は 不要 — 生きた watch が残っていればイベント毎に arm が自然に再試行される。
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 機構に統合）:directoryAncestors（純関数）で得た祖先チェーン（例:~/.config→~→/）を近い順に試し、最初に arm できた祖先へ park祖先イベントの扱いが issue の検討事項「頻度が高いのでフィルタ必須」への回答:
mkdir -pの一括作成は一発で config dir まで到達）onChangeを発火（= initial load）。祖先ノイズ（$HOME直下の変化など）は syscall 数回で終わり、decode/reload は一切走らない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 管理へ（cancelledDispatchSourceはイベントを配送しないことのミラー）armed()が nil を返すのは祖先チェーン全滅時のみ（live では/が常に存在するため実質発生しない）テスト
directoryAncestors純関数(fake gateway 駆動)FileWatchGatewayで「config dir を後から作成 → 昇格 → onChange 発火 → in-place 編集も file tier で検知」の E2Emake 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
Summary by CodeRabbit
Bug Fixes
Documentation
Release