Skip to content

feat(cli-merge-pipeline): ai ステップで pending file 書き込み (task 1-B) #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aloekun merged 4 commits into from
Apr 23, 2026
Merged

feat(cli-merge-pipeline): ai ステップで pending file 書き込み (task 1-B) #70

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2bd6290
feat(cli-merge-pipeline): post-merge ai ステップで pending file を書き込む (tas…
aloekun Apr 23, 2026
878a8c7
fix(review): apply CodeRabbit fixes for PR #70
aloekun Apr 23, 2026
ac7270b
fix(review): apply CodeRabbit round 2 fixes for PR #70
aloekun Apr 23, 2026
0441866
fix(review): apply CodeRabbit round 3 fix for PR #70
aloekun Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions docs/adr/adr-029-post-merge-feedback-auto-trigger.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ Claude がメイン会話内で /post-merge-feedback を起動
"status": "pending",
"created_at": "2026-04-23T10:00:00Z",
"dispatched_at": null,
"consumed_at": null
"consumed_at": null,
"producer": "cli-merge-pipeline@pid-1234@2026-04-23T10:00:00Z"
}
```

Expand All @@ -103,6 +104,7 @@ Claude がメイン会話内で /post-merge-feedback を起動
| `created_at` | ISO 8601 UTC string | yes | cli-merge-pipeline が書き込んだ時刻 |
| `dispatched_at` | ISO 8601 UTC string | nullable | hooks-stop-feedback-dispatch が additionalContext を出した時刻 |
| `consumed_at` | ISO 8601 UTC string | nullable | skill が完了処理を行った時刻 |
| `producer` | string | optional | 書き込み元の識別子 (`cli-merge-pipeline@pid-{pid}@{iso8601}` 形式)。取りこぼし発生時の追跡用。schema v1 互換 (既存 reader は欠損 / 未知値を無視) |

### 状態遷移

Expand Down Expand Up @@ -130,12 +132,24 @@ cli-merge-pipeline が pending file を書き込もうとしたときの既存

| 既存 status | 挙動 |
|---|---|
| 不在 | 新規書き込み (通常経路) |
| `consumed` (削除忘れ) | 上書き |
| 不在 | 新規書き込み (通常経路; `create_new` で atomic 排他作成) |
| `consumed` (削除忘れ) | 削除後に上書き |
| `pending` / `dispatched` | **書き込み skip + WARN** (ステップ自体は PASS 扱いで merge-pr を中断しない) |
| 破損 (size 0 / JSON parse 失敗 / schema_version 不一致) | 削除してから書き込み |
| 破損 (size 0 / JSON parse 失敗 / schema_version 不一致 / 未知 status) | 削除後に上書き |

同一セッション内で短時間に複数 PR をマージした場合 (現実には稀)、最初の pending が consume されるまで後続は取りこぼす。取りこぼしは WARN ログで可観測性を残すことで後追い対応可能とする。
**排他性保証 (TOCTOU 対策)**:

新規書き込み経路では `OpenOptions::new().write(true).create_new(true).open(path)` (O_EXCL 相当) で最終ファイルを直接 atomic 排他作成する。`read_existing` と書き込みが分離していても、2 プロセスが同時に `None` を観測した場合は OS 層で一方のみ成功、他方は `AlreadyExists` で弾かれて **WARN ログを残す**。これにより「取りこぼしは可視」という本 ADR の保証が実装レベルで満たされる。

**tmp → rename は新規作成経路では使わない** — `rename` は既存を無条件上書きするため、placeholder 方式 (`create_new` で空ファイル予約 → `rename` で置換) だと排他予約が自壊する (他プロセスが後から rename で上書き可能)。PR #70 のレビューで指摘・修正済。

上書き経路 (既存 `Consumed` / 破損 を削除後の書き込み) の read→write 間 race は許容する。`Consumed` は skill バグ時、破損は schema drift 時のみの稀経路で、発生しても破損ポリシー (size=0 / parse 失敗 → 削除 → silent exit) で自己回復する。

**中間状態の可観測性**: 新規作成経路は `create_new` で直接書くため、`write_all` 完了前の reader が size=0 or 部分書き込みを観測し得る。hooks-stop-feedback-dispatch の破損ポリシーが吸収する設計判断 (**完全性より排他性を優先**)。

**producer 追跡**: pending file には `producer: Option<String>` (`cli-merge-pipeline@pid-{pid}@{iso8601}` 形式) を含む。取りこぼし発生時に「誰が書いた pending が消えたか」を後追いできる観測性補助。schema_version は v1 のまま (未知フィールドは既存 reader が無視するため bump 不要)。

同一セッション内で短時間に複数 PR をマージした場合 (現実には稀)、最初の pending が consume されるまで後続は取りこぼす。取りこぼしは `create_new AlreadyExists` か `Active → skip` の WARN ログで観測可能。

**将来拡張**: 取りこぼしが問題化したらディレクトリベースのキュー (`.claude/post-merge-feedback/<pr>.json`) への移行を検討。現段階では YAGNI で単一ファイルを採用する。

Expand All @@ -155,14 +169,21 @@ hooks-stop-feedback-dispatch が pending file を読み取る際の分岐表:
| `status == "dispatched"` | silent exit (二重通知しない) |
| `status == "consumed"` | ファイル削除 + silent exit |

**書き込み方式**: 「一時ファイルに write → `fs::rename` で atomic rename」の 2 段階を常に使う。ロックファイルは不要。
**書き込み方式** (2026-04-23 改訂):

| 経路 | 方式 | 排他保証 |
|---|---|---|
| 新規作成 (既存不在) | `OpenOptions::new().write(true).create_new(true).open(path)` で直接書き込み + `sync_all` | OS の O_EXCL / CREATE_NEW による atomic 排他作成 |
| 上書き (Consumed / 破損 削除後) | 一時ファイルに write → `fs::rename` で atomic overwrite | fs::rename の atomic overwrite |

**ロックファイルは不要** (create_new が OS 層で atomic 排他を保証するため)。placeholder + rename 方式は採用しない (rename の無条件上書きで排他予約が自壊するため)。

ただし atomic 保証はプラットフォームとファイルシステムに依存する:

| 環境 | `std::fs::rename` の atomicity |
|---|---|
| POSIX (Linux / macOS 等) | `rename(2)` により atomic overwrite (同一ファイルシステム内) |
| Windows 10 1607+ / NTFS or ReFS | `FileRenameInfoEx` + `FILE_RENAME_FLAG_POSIX_SEMANTICS` 経路が成功すれば atomic overwrite。本プロジェクトのターゲット (Windows 11 + NTFS) はこの範囲 |
| POSIX (Linux / macOS 等) | `open(O_EXCL)` / `rename(2)` により atomic (同一ファイルシステム内) |
| Windows 10 1607+ / NTFS or ReFS | `CREATE_NEW` (O_EXCL 相当) / `FileRenameInfoEx` + `FILE_RENAME_FLAG_POSIX_SEMANTICS` 経路が成功すれば atomic。本プロジェクトのターゲット (Windows 11 + NTFS) はこの範囲 |
| 旧 Windows / 非対応 FS | `FileRenameInfo` に fallback し **atomic 保証なし**。他プロセスが中間状態を観測する可能性がある |

Rust 側の実装順序は rust-lang/rust の [#131072](https://github.com/rust-lang/rust/pull/131072) / [#138133](https://github.com/rust-lang/rust/pull/138133) で 2024-2025 に変更されており、信頼性のため non-atomic を先に試行、失敗時のみ POSIX semantics 版へ fallback する挙動になっている点にも留意。
Expand Down
Loading