diff --git a/.gitignore b/.gitignore index a463985..5afdd85 100644 --- a/.gitignore +++ b/.gitignore @@ -32,7 +32,15 @@ src/*/Cargo.lock .claude/pr-monitor-state.json.tmp .claude/scheduled_tasks.lock # ADR-029: post-merge-feedback の pending file (cli-merge-pipeline 生成、skill が consume する一時 artifact) +# 旧経路。Phase E で機構ごと廃止予定 (ADR-030 supersede) .claude/post-merge-feedback-pending.json +# ADR-030: post-merge-feedback の出力レポート + 失敗マーカー (内部 artifact) +.claude/feedback-reports/ + +# ADR-030: takt workflow への入力 (cli-merge-pipeline が生成、workflow が読む) +.takt/post-merge-feedback-context.json +.takt/post-merge-feedback-transcript.jsonl + # Temp PR body file (generated by `pnpm prepare-pr-body`, removed by `pnpm prepare-pr-body:cleanup`) .tmp-pr-body.md diff --git a/.takt/facets/instructions/aggregate-feedback.md b/.takt/facets/instructions/aggregate-feedback.md new file mode 100644 index 0000000..704e35a --- /dev/null +++ b/.takt/facets/instructions/aggregate-feedback.md @@ -0,0 +1,112 @@ +# Aggregate Feedback + +3 つの分析レポート (PR 知見・セッション知見・pre-push レポート知見) を Plankton 優先度で統合し、最終的な再発防止策レポートを生成する。 + +旧 `/post-merge-feedback` skill (`E:\work\claude-code-skills\post-merge-feedback\SKILL.md`) の Phase 4 ロジックから port。 + +**重要な原則:** +- 読み取り専用。コードの修正は一切行わない (実装は L2 recovery / ユーザー判断で行う) +- 知見がない場合は「提案なし」で正常終了する。無理に提案を捻出しない +- 重複する提案はマージし、根拠 (rationale) を統合する +- Tier 1 を最優先で提案する。Tier 3 のみの提案は価値が低い + +--- + +## Input + +### Report Directory (takt が提供) + +本 step (`pass_previous_response: false`) は前 step の response を受け取らない代わりに、**Report Directory** に保存された 3 つの先行レポートを Read で読み取る: + +- `pr-analysis.md` — analyze-pr facet の出力 +- `session-analysis.md` — analyze-session facet の出力 +- `prepush-analysis.md` — analyze-prepush-reports facet の出力 + +### Context file + +`.takt/post-merge-feedback-context.json` も Read で読み、PR 番号・タイトル等のメタデータを取得する: + +```json +{ + "pr_number": 123, + "owner_repo": "aloekun/claude-code-hook-test", + "merged_at": "2026-04-25T10:00:00Z" +} +``` + +PR タイトルが context に含まれていない場合は、レポート内の `### PR: ...` ヘッダから抽出する。 + +## Phase 1: 3 レポートの統合 + +各レポートの提案リスト (Tier 1 / 2 / 3 の表) を抽出し、以下のルールで統合する: + +1. **重複検出**: 同じ `Target` + 似た `Description` の提案はマージする +2. **根拠統合**: マージした提案の `Rationale` カラムには複数ソース (PR diff / session / prepush) を併記する +3. **Tier 並び**: 最終リストは Tier 1 → Tier 2 → Tier 3 の順 +4. **品質フィルタ**: 以下の提案は除外する + - 一般的なベストプラクティスの押し付け (具体的根拠がない) + - すでに hooks-config.toml / custom-lint-rules.toml に存在するルール (Read で確認可能) + - 対象ファイルが read-only zone (`.takt/`, `docs/adr/`, `templates/`) のみで具体的な編集箇所が示せないもの + +## Phase 2: 最終レポート生成 + +以下の Required output 形式で `feedback-report.md` を出力する。 + +### Source 表記の凡例 + +`Rationale` に書く `Source` の表記: +- `PR diff` — PR の差分から抽出 +- `Review comment` — PR レビューコメントから抽出 +- `Session` — セッション transcript から抽出 +- `Prepush:simplicity` / `Prepush:security` — pre-push-review の各レポートから抽出 +- 複数ソースは `;` 区切り (例: `PR diff; Session`) + +--- + +## Required output + +```markdown +## Post-Merge Feedback Report + +### PR: # +- マージ日時: <merged_at> +- 分析ソース: PR data, Session transcript, Pre-push reports + +### 統合された再発防止策 + +#### Tier 1: Hooks/Linter 改善 (決定論的防止) + +| # | Type | Description | Target | Effort | Rationale (Source) | +|---|------|-------------|--------|--------|--------------------| +| 1 | custom_lint_rule | ... | .claude/custom-lint-rules.toml | Low | PR diff; Session | + +#### Tier 2: テスト/自動化 + +| # | Type | Description | Target | Effort | Rationale (Source) | +|---|------|-------------|--------|--------|--------------------| + +#### Tier 3: ドキュメント/ルール + +| # | Type | Description | Target | Effort | Rationale (Source) | +|---|------|-------------|--------|--------|--------------------| + +### 次のアクション + +- ユーザーがレポートを確認後、UserPromptSubmit hook (L2 recovery) または直接的な指示で実装へ進む +- このレポートは `.claude/feedback-reports/<pr_number>.md` に保存される (`.gitignore` 除外、内部 artifact) +``` + +提案がない Tier はセクションごと省略する。 + +提案がゼロの場合は以下: + +```markdown +## Post-Merge Feedback Report + +### PR: <owner/repo>#<number> <title> + +この PR から特筆すべき再発防止策は見つかりませんでした。 +3 レポート (PR / Session / Pre-push) のいずれにも、決定論的な防止策に値する事象が記録されていませんでした。 +``` + +最後に `aggregation complete` で終了する。 diff --git a/.takt/facets/instructions/analyze-pr.md b/.takt/facets/instructions/analyze-pr.md new file mode 100644 index 0000000..fedd3e4 --- /dev/null +++ b/.takt/facets/instructions/analyze-pr.md @@ -0,0 +1,143 @@ +# Analyze PR (post-merge-feedback workflow) + +マージ済み PR のコード差分・レビューコメントを分析し、再発防止に役立つ知見を構造化レポートで出力する。 + +旧 `/analyze-pr` skill (`E:\work\claude-code-skills\analyze-pr\SKILL.md`) から port。 + +**重要な原則:** +- 読み取り専用。コードの修正は一切行わない +- 知見がない場合は「提案なし」で正常終了する。無理に提案を捻出しない +- セッション知見は扱わない (`analyze-session` facet が別途処理) + +--- + +## Input + +`.takt/post-merge-feedback-context.json` を Read で読み、PR メタデータを取得する。 + +```json +{ + "pr_number": 123, + "owner_repo": "aloekun/claude-code-hook-test", + "merged_at": "2026-04-25T10:00:00Z", + "first_commit_time": "2026-04-25T08:00:00Z", + "transcript_path": ".takt/post-merge-feedback-transcript.jsonl", + "prepush_reports_dir": ".takt/runs/<latest>-pre-push-review/reports" +} +``` + +context file が存在しない / parse 失敗の場合は `## PR Analysis Report` セクションに「context unavailable」と書き、analysis complete で次へ進める。 + +## Phase 1: PR データ取得 + +`pr_number` と `owner_repo` を使い、Bash で並列にデータを取得する: + +```bash +# コード差分 +gh pr diff <pr_number> --repo <owner_repo> + +# レビューコメント (インライン) +gh api repos/<owner_repo>/pulls/<pr_number>/comments \ + --jq '.[] | {user: .user.login, body: .body, path: .path, line: .line}' + +# レビュー判定 +gh api repos/<owner_repo>/pulls/<pr_number>/reviews \ + --jq '.[] | {user: .user.login, state: .state, body: .body}' + +# PR メタデータ +gh pr view <pr_number> --repo <owner_repo> \ + --json title,body,labels,mergedAt,state +``` + +### エラーハンドリング + +| エラー | 対応 | +|--------|------| +| PR が存在しない | エラーセクションを書き、analysis complete で次へ | +| diff 取得失敗 | エラーを記録し、レビューコメントのみで続行 | +| コメント / レビュー取得失敗 | 警告を出して diff のみで続行 | + +## Phase 2: 分析 & 知見抽出 + +PR diff + レビューコメントを分析し、再発防止に役立つ知見を抽出する。 + +### 着眼点 + +- **繰り返しパターン**: diff 内で同じ種類の修正が複数箇所にあるか → リンタールールで防止可能か +- **レビュー指摘の傾向**: 同じカテゴリの指摘が複数あるか → hooks で自動検出可能か +- **危険な操作**: セキュリティリスク、破壊的操作が含まれていたか → block_pattern で防止可能か +- **設計上の課題**: アーキテクチャ的な問題が指摘されていたか → ドキュメントやテストで防止可能か + +### Plankton 優先度 (Tier 分類) + +各提案を以下の Tier に分類する。**Tier 1 を最優先で検討**。 + +#### Tier 1: Hooks/Linter 改善 (決定論的防止 — 最も強力) + +| Type | 対象ファイル | 説明 | +|------|------------|------| +| `block_pattern` | `.claude/hooks-config.toml` | PreToolUse のコマンド実行ブロック正規表現 | +| `custom_lint_rule` | `.claude/custom-lint-rules.toml` | PostToolUse のリテラル検出ルール | +| `linter_pipeline` | `.claude/hooks-config.toml` | リンターパイプラインへのステップ追加 | + +#### Tier 2: テスト/自動化 (半決定論的) + +| Type | 説明 | +|------|------| +| `test_addition` | 再発検出のためのテストケース追加 | +| `ci_step` | CI パイプラインへのステップ追加 | + +#### Tier 3: ドキュメント/ルール (非決定論的 — 最も弱い) + +| Type | 対象ファイル | 説明 | +|------|------------|------| +| `claude_md_rule` | `CLAUDE.md` | プロジェクトルールの追加 | +| `adr` | `docs/adr/` | 設計判断の記録 | + +### 提案の品質基準 + +- **具体的であること**: 「セキュリティに注意」ではなく「危険な API 使用を `custom_lint_rule` で検出」のように具体的なルール提案 +- **実装可能であること**: 対象ファイルと具体的な変更内容を含む +- **根拠があること**: PR diff またはレビューコメントの具体的な事例に基づく +- **過剰提案しないこと**: 本当に再発リスクがある問題のみ。一般的なベストプラクティスの押し付けはしない + +--- + +## Required output + +```markdown +## PR Analysis Report + +### PR: <owner/repo>#<number> <title> +- 状態: Merged (<mergedAt>) +- 分析ソース: PR diff, レビューコメント + +#### Tier 1: Hooks/Linter 改善 + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| + +#### Tier 2: テスト/自動化 + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| + +#### Tier 3: ドキュメント/ルール + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| +``` + +提案がない Tier はセクションごと省略する。 + +提案がゼロなら以下: + +```markdown +## PR Analysis Report + +### PR: <owner/repo>#<number> <title> + +この PR から特筆すべき再発防止策は見つかりませんでした。 +``` + +最後に `analysis complete` で終了する。 diff --git a/.takt/facets/instructions/analyze-prepush-reports.md b/.takt/facets/instructions/analyze-prepush-reports.md new file mode 100644 index 0000000..a62a626 --- /dev/null +++ b/.takt/facets/instructions/analyze-prepush-reports.md @@ -0,0 +1,110 @@ +# Analyze Pre-Push Reports + +PR がマージされる前の最終 push 時に生成された pre-push-review レポート (simplicity / security) を集約し、再発防止に値する指摘をまとめる。 + +**重要な原則:** +- 読み取り専用。コードの修正は一切行わない +- pre-push-review レポートが見つからない / 空の場合は「対象データなし」で正常終了する +- 既に push 時に APPROVED されている指摘でも、「再発防止策に転用できそうな知見」がある場合は抽出する + +--- + +## Input + +`.takt/post-merge-feedback-context.json` を Read で読み、`prepush_reports_dir` を確認する: + +```json +{ + "pr_number": 123, + "prepush_reports_dir": ".takt/runs/20260425-094925-pre-push-review/reports" +} +``` + +`prepush_reports_dir` が空 / dir が存在しない場合は: + +```markdown +## Pre-Push Reports Analysis + +### Status + +pre-push-review の reports が見つかりませんでした。 +``` + +を出力し `analysis complete` で次へ進める。 + +## Phase 1: レポートの収集 + +Glob で `<prepush_reports_dir>/*.md` を列挙し、それぞれ Read で内容を取得する。 + +典型的なレポート: +- `simplicity-review.md` — 簡潔性レビューの指摘 +- `security-review.md` — セキュリティレビューの指摘 +- `supervisor-validation.md` — supervisor 判定 (任意) +- `summary.md` — 統合サマリ (任意) + +各レポートは markdown 形式で、findings / verdict / recommendations が含まれる想定。 + +## Phase 2: 集約・整理 + +以下の観点で要約する: + +1. **明示された finding**: 各レビューで `REJECT` / `needs_fix` 判定だった指摘 +2. **修正完了済の事象**: takt の fix loop で APPROVE に至った修正の系統 +3. **supervise 判定の警告**: supervisor の警告 / コメント + +各 finding に対して、**Plankton 優先度 (Tier 1〜3)** で再発防止策を提案する。 + +注意点: +- supervisor が `ready to push` で APPROVE した場合、**コードレベルの修正は不要** だが、それでも「同じパターンを次回検出するための仕組み」を Tier 1 候補として検討する +- 個別の review コメントは要約のみ (原文引用は最小限) + +--- + +## Required output + +```markdown +## Pre-Push Reports Analysis + +### 集約サマリ + +- 対象 reports: {ファイル名のリスト} +- simplicity verdict: {APPROVE / REJECT / N/A} +- security verdict: {APPROVE / REJECT / N/A} +- supervisor verdict: {APPROVE / REJECT / N/A} + +### 主要 findings (要約) + +1. {要約} (出典: {report ファイル名}) + - 防止策: Tier {N} - {具体的な提案} + +### 再発防止候補 (Plankton 分類) + +#### Tier 1: Hooks/Linter 改善 + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| + +#### Tier 2: テスト/自動化 + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| + +#### Tier 3: ドキュメント/ルール + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| +``` + +提案がない Tier はセクションごと省略する。 + +該当なしの場合は以下: + +```markdown +## Pre-Push Reports Analysis + +### Status + +pre-push reports は読み込めましたが、再発防止に値する findings は見つかりませんでした。 +``` + +最後に `analysis complete` で終了する。 diff --git a/.takt/facets/instructions/analyze-session.md b/.takt/facets/instructions/analyze-session.md new file mode 100644 index 0000000..f950917 --- /dev/null +++ b/.takt/facets/instructions/analyze-session.md @@ -0,0 +1,134 @@ +# Analyze Session Transcript + +PR の commit 期間に該当するセッション transcript を分析し、実装時の学び・トラブル・ユーザー指示を抽出する。 + +ADR-030 §transcript 抽出戦略に基づく Phase 0 で確認済の方針: +- transcript ファイルは Rust 側 (cli-merge-pipeline) で時刻 range filter 済 (時刻 range は PR の `first_commit_time` 〜 `merged_at`) +- 本 facet は filter 済 jsonl を読むだけ。生 file を直接 grep しない + +**重要な原則:** +- secrets / PII は要約から除外する (トークン、API キー、パスワード、個人情報、長文の生ログ全文) +- 生ログ全文は出力しない (要約のみ) +- 不確実な値は除外する + +--- + +## Input + +`.takt/post-merge-feedback-context.json` を Read で読み、`transcript_path` を確認する: + +```json +{ + "pr_number": 123, + "transcript_path": ".takt/post-merge-feedback-transcript.jsonl", + "first_commit_time": "2026-04-25T08:00:00Z", + "merged_at": "2026-04-25T10:00:00Z" +} +``` + +`transcript_path` が空 / file が存在しない / file が空の場合は: + +```markdown +## Session Analysis Report + +### Status + +セッション transcript が見つかりませんでした (該当期間のデータなし)。 +``` + +を出力し `analysis complete` で次へ進める。 + +## Phase 1: Transcript の読み取り + +`transcript_path` を Read で読む。**JSONL** 形式 (1 行 1 entry)。 + +各 entry のスキーマ (Phase 0 確認済): + +```json +{ + "type": "user" | "assistant", + "timestamp": "2026-04-25T05:44:35.040Z", + "sessionId": "<uuid>", + "message": { + "role": "user" | "assistant", + "content": [ + { "type": "text", "text": "..." }, + { "type": "thinking", "thinking": "", "signature": "<encrypted>" }, + { "type": "tool_use", "name": "Bash", "input": {...} } + ] + } +} +``` + +注意: +- `thinking` の content は encrypted (`thinking` field は空)。chain-of-thought は抽出不可 +- `type: queue-operation` / `type: attachment` は Rust 側で除外済の想定だが、出現したら無視する +- 1.7 MB / 数百行になり得る。重要な箇所だけ要約する + +## Phase 2: 知見抽出 + +以下の観点で抽出する。各観点は **要約のみ**、原文引用は最小限。 + +### 抽出観点 + +1. **実装の困難**: 何度も試行錯誤した箇所、アプローチを変更した箇所 + - 例: 「lib-pending-file の atomic write で 3 回 retry した」 +2. **ユーザー修正指示**: ユーザーから「そうじゃない」「こうして」と指摘された箇所 + - 例: 「DRY を試みたがユーザーから却下された (テスト独立性優先)」 +3. **バグ発見**: 開発中に発見・修正したバグ + - 例: 「percent encode が `?` `#` を素通りしていた」 +4. **ワークアラウンド**: 本来の方法ではなく回避策を適用した箇所 +5. **混乱を招いたパターン**: コードの読み間違いや誤った前提 + +### Plankton 優先度による分類 + +各知見に対して、再発防止策を以下の Tier で分類する (analyze-pr facet と同じ Tier 体系): + +- **Tier 1**: hooks/linter 改善 (`block_pattern`, `custom_lint_rule`, `linter_pipeline`) +- **Tier 2**: テスト/自動化 (`test_addition`, `ci_step`) +- **Tier 3**: ドキュメント/ルール (`claude_md_rule`, `adr`) + +--- + +## Required output + +```markdown +## Session Analysis Report + +### 実装の学び (要約) + +1. {観点 (実装の困難 / ユーザー修正指示 / バグ発見 / ワークアラウンド / 混乱)}: {要約} + - 推定原因: {原因の推定} + - 防止策: Tier {N} - {具体的な提案} + +### 再発防止候補 (Plankton 分類) + +#### Tier 1: Hooks/Linter 改善 + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| + +#### Tier 2: テスト/自動化 + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| + +#### Tier 3: ドキュメント/ルール + +| # | Type | Description | Target | Effort | Rationale | +|---|------|-------------|--------|--------|-----------| +``` + +提案がない Tier はセクションごと省略する。 + +知見がない場合は以下: + +```markdown +## Session Analysis Report + +### Status + +セッションから特筆すべき知見は抽出できませんでした (transcript は読み取り済みだが、再発防止に値する事象なし)。 +``` + +最後に `analysis complete` で終了する。 diff --git a/.takt/workflows/post-merge-feedback.yaml b/.takt/workflows/post-merge-feedback.yaml new file mode 100644 index 0000000..10ff312 --- /dev/null +++ b/.takt/workflows/post-merge-feedback.yaml @@ -0,0 +1,126 @@ +name: post-merge-feedback +description: > + マージ済み PR のフィードバックレポートを生成する決定論的 workflow (ADR-030)。 + 入力: cli-merge-pipeline が事前に書き出すコンテキストファイル群 + - .takt/post-merge-feedback-context.json (PR メタデータ + 時刻 range) + - .takt/post-merge-feedback-transcript.jsonl (filter 済みセッション履歴) + facets を順次 chain し、aggregate-feedback で最終レポートを生成する。 + fix loop なし: コードではなくレポートを生成するため reviewers/fix の構造は不要。 + 出力: feedback-report.md (Report Directory に保存) + cli-merge-pipeline が takt 完了後に .claude/feedback-reports/<pr>.md にコピーする。 + +workflow_config: + provider_options: + codex: + network_access: true + opencode: + network_access: true + +max_steps: 10 +initial_step: analyze-pr + +steps: + # --------------------------------------------------------------------------- + # Step 1: analyze-pr + # PR diff + reviews を分析し、再発防止に役立つ知見を抽出する。 + # 旧 /analyze-pr skill から port (E:\work\claude-code-skills\analyze-pr\SKILL.md)。 + # --------------------------------------------------------------------------- + - name: analyze-pr + edit: false + persona: code-reviewer + model: sonnet + policy: review + knowledge: architecture + provider_options: + claude: + allowed_tools: + - Read + - Glob + - Grep + - Bash + instruction: analyze-pr + output_contracts: + report: + - name: pr-analysis.md + format: pr-analysis + rules: + - condition: analysis complete + next: analyze-session + + # --------------------------------------------------------------------------- + # Step 2: analyze-session + # filter 済みセッション transcript から実装時の学び・トラブル・ユーザー指示を抽出。 + # ADR-030 §transcript 抽出戦略に従い、Rust 側で時刻 range filter 済の入力を読む。 + # --------------------------------------------------------------------------- + - name: analyze-session + edit: false + persona: code-reviewer + model: sonnet + policy: review + provider_options: + claude: + allowed_tools: + - Read + - Glob + - Grep + instruction: analyze-session + pass_previous_response: false + output_contracts: + report: + - name: session-analysis.md + format: session-analysis + rules: + - condition: analysis complete + next: analyze-prepush-reports + + # --------------------------------------------------------------------------- + # Step 3: analyze-prepush-reports + # pre-push-review の reports (.takt/runs/<latest>-pre-push-review/reports/) を集約。 + # --------------------------------------------------------------------------- + - name: analyze-prepush-reports + edit: false + persona: code-reviewer + model: sonnet + policy: review + provider_options: + claude: + allowed_tools: + - Read + - Glob + - Grep + instruction: analyze-prepush-reports + pass_previous_response: false + output_contracts: + report: + - name: prepush-analysis.md + format: prepush-analysis + rules: + - condition: analysis complete + next: aggregate-feedback + + # --------------------------------------------------------------------------- + # Step 4: aggregate-feedback + # 3 つの分析レポートを Plankton 優先度で統合し、最終レポートを生成する。 + # 旧 post-merge-feedback skill の Phase 4 ロジックから port。 + # pass_previous_response: false で、Report Directory の各 *-analysis.md を直接読む。 + # --------------------------------------------------------------------------- + - name: aggregate-feedback + edit: false + persona: supervisor + model: sonnet + policy: review + provider_options: + claude: + allowed_tools: + - Read + - Glob + - Grep + instruction: aggregate-feedback + pass_previous_response: false + output_contracts: + report: + - name: feedback-report.md + format: feedback-report + rules: + - condition: aggregation complete + next: COMPLETE diff --git a/docs/todo.md b/docs/todo.md index 945d1d2..c7f3465 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -103,24 +103,6 @@ dogfood では PR #74 マージ後、pending file が `dispatched` で stuck し #### 作業計画 -##### Phase B: takt workflow + 4 facets — L1 Floor (PR 2) - -- [ ] `.takt/workflows/post-merge-feedback.yaml` 新規作成 - - 入力: PR 番号 (cli-merge-pipeline から渡される) - - facets を順次 chain -- [ ] facets を新設 (`.takt/facets/` 下): - - **`analyze-pr.md`**: PR diff + reviews を分析 (既存 `analyze-pr` skill から port。`E:\work\claude-code-skills\analyze-pr\SKILL.md` を参照) - - **`analyze-session.md`** (新規): transcript range filter で抽出した user/assistant 履歴から 実装時の学び・トラブル修正・ユーザー指示 を抽出。**Phase 0 調査結果の transcript 抽出戦略を参照** - - **`analyze-prepush-reports.md`** (新規): `.takt/runs/<latest>/reports/*.md` (pre-push-review の simplicity / security レポート) を集約 - - **`aggregate-feedback.md`** (新規): 上記 3 facets の出力 + Plankton 優先度で統合 → ADR 提案 / 仕組み改善案を生成 (旧 post-merge-feedback skill の Phase 4 統合フィードバックロジックを port、`E:\work\claude-code-skills\post-merge-feedback\SKILL.md` を参照) -- [ ] `src/cli-merge-pipeline/` の post_steps `type = "ai"` 分岐を変更: - - 旧: pending file 書き込み (ADR-029) - - 新: takt workflow を spawn して同期実行 (push-runner / cli-pr-monitor の takt 起動方法に倣う) - - 出力 `.claude/feedback-reports/<pr>.md` を生成 - - 失敗時 `<pr>.md.failed` marker 書き込み (soft fail、merge は成功扱い) -- [ ] `.gitignore` に `.claude/feedback-reports/` を追加 (artifact、コミット対象外) -- [ ] テスト追加 (workflow 成功/失敗ケース、transcript 抽出正常性、cli-merge-pipeline 統合) - ##### Phase C: UserPromptSubmit hook — L2 Recovery (PR 3) - [ ] `src/hooks-user-prompt-feedback-recovery/` 新規 crate @@ -207,7 +189,7 @@ dogfood では PR #74 マージ後、pending file が `dispatched` で stuck し 4. `docs/adr/adr-014-post-merge-feedback.md` を読む (skill 自体の元設計) 5. `docs/adr/adr-015-push-runner-takt-migration.md` / `docs/adr/adr-018-pr-monitor-takt-migration.md` を読む (takt 移行の先行事例として参考) 6. `docs/adr/adr-020-takt-facets-sharing.md` を読む (facets 共通化方針の根拠) -7. Phase A から着手: `docs/adr/adr-030-deterministic-post-merge-feedback.md` の起案 +7. Phase C から着手 (Phase A: ADR 起案 / Phase B: takt workflow + facets + cli-merge-pipeline 統合 はマージ済) #### 完了基準 diff --git a/src/cli-merge-pipeline/src/feedback.rs b/src/cli-merge-pipeline/src/feedback.rs new file mode 100644 index 0000000..23f3b4c --- /dev/null +++ b/src/cli-merge-pipeline/src/feedback.rs @@ -0,0 +1,710 @@ +//! Post-merge feedback workflow runner (ADR-030 Phase B). +//! +//! 旧 pending file 機構 (ADR-029) を置き換え、takt workflow `post-merge-feedback` +//! を同期実行する決定論的経路を提供する。 +//! +//! 入力: +//! +//! - PipelineContext (pr_number, owner_repo) +//! +//! 副作用: +//! +//! - `.takt/post-merge-feedback-context.json` — workflow が読む PR メタデータ +//! - `.takt/post-merge-feedback-transcript.jsonl` — 時刻 range filter 済セッション履歴 +//! - takt workflow を spawn (`pnpm exec takt -w post-merge-feedback ...`) +//! - 成功時: `.claude/feedback-reports/<pr>.md` を生成 (takt 出力をコピー) +//! - 失敗時: `.claude/feedback-reports/<pr>.md.failed` marker を残す (soft fail) +//! +//! 失敗時も exit code は変えない (merge は完了済み)。L2 recovery で後続ターンに +//! UserPromptSubmit hook が拾う想定。 + +use serde::Serialize; +use std::fs; +use std::io::{BufRead, BufReader, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::time::Duration; + +/// takt workflow 名 / task ラベル +const TAKT_WORKFLOW: &str = "post-merge-feedback"; +const TAKT_TASK_PREFIX: &str = "post-merge feedback for #"; + +/// takt 実行のデフォルトタイムアウト (10 分) +pub const TAKT_TIMEOUT_SECS: u64 = 600; + +/// run_takt_workflow のポーリング間隔 (ms) +const POLL_INTERVAL_MS: u64 = 500; + +/// 出力ファイルの相対パス (リポジトリルートからの相対) +pub const FEEDBACK_DIR: &str = ".claude/feedback-reports"; +pub const CONTEXT_PATH: &str = ".takt/post-merge-feedback-context.json"; +pub const TRANSCRIPT_PATH: &str = ".takt/post-merge-feedback-transcript.jsonl"; + +/// post-merge-feedback workflow の入力。 +pub struct FeedbackInput<'a> { + pub pr_number: u64, + pub owner_repo: &'a str, + /// リポジトリルート (`.takt/`, `.claude/` の親)。通常は `std::env::current_dir()`。 + pub repo_root: PathBuf, + /// transcript ファイルが置かれるディレクトリ (`~/.claude/projects/<project-id>/`)。 + /// `None` なら transcript filter をスキップする (空 jsonl を出力)。 + pub transcript_source_dir: Option<PathBuf>, +} + +/// PR の時刻 range (gh api の出力から取得) +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PrTimeRange { + pub first_commit_time: String, + pub merged_at: String, +} + +/// takt workflow に渡す JSON コンテキスト。 +#[derive(Serialize)] +struct WorkflowContext<'a> { + pr_number: u64, + owner_repo: &'a str, + merged_at: &'a str, + first_commit_time: &'a str, + transcript_path: &'a str, + prepush_reports_dir: &'a str, +} + +/// `cwd` パス → `~/.claude/projects/` の project ID 形式へ変換する。 +/// +/// Windows: `E:\work\claude-code-hook-test` → `e--work-claude-code-hook-test` +/// (lowercase、`:` `\` `/` をすべて `-` に置換)。 +pub fn cwd_to_project_id(cwd: &Path) -> String { + cwd.to_string_lossy() + .to_lowercase() + .replace([':', '\\', '/'], "-") +} + +/// `~/.claude/projects/<project-id>/` を返す。`USERPROFILE` 未設定なら `None`。 +pub fn project_transcript_dir(cwd: &Path) -> Option<PathBuf> { + let home = std::env::var_os("USERPROFILE").or_else(|| std::env::var_os("HOME"))?; + let project_id = cwd_to_project_id(cwd); + let dir = PathBuf::from(home) + .join(".claude") + .join("projects") + .join(project_id); + if dir.is_dir() { + Some(dir) + } else { + None + } +} + +/// gh api から PR の `first_commit_time` (oldest commit authoredDate) と `mergedAt` を取得する。 +/// +/// 失敗時は `Err` を返す (caller 側で fallback)。 +pub fn fetch_pr_time_range(pr_number: u64, owner_repo: &str) -> Result<PrTimeRange, String> { + let pr_str = pr_number.to_string(); + let output = Command::new("gh") + .args([ + "pr", + "view", + &pr_str, + "--repo", + owner_repo, + "--json", + "commits,mergedAt", + ]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .map_err(|e| format!("gh コマンド起動失敗: {}", e))?; + + if !output.status.success() { + return Err(format!( + "gh pr view 失敗: {}", + String::from_utf8_lossy(&output.stderr).trim() + )); + } + + let json: serde_json::Value = serde_json::from_slice(&output.stdout) + .map_err(|e| format!("gh 出力 JSON パース失敗: {}", e))?; + + let merged_at = json + .get("mergedAt") + .and_then(|v| v.as_str()) + .ok_or("mergedAt が応答に含まれていません")? + .to_string(); + + let commits = json + .get("commits") + .and_then(|v| v.as_array()) + .ok_or("commits が応答に含まれていません")?; + + let first_commit_time = commits + .iter() + .filter_map(|c| { + c.get("authoredDate") + .or_else(|| c.get("committedDate")) + .and_then(|v| v.as_str()) + }) + .min() + .ok_or("commits 配列が空です")? + .to_string(); + + Ok(PrTimeRange { + first_commit_time, + merged_at, + }) +} + +/// transcript jsonl をフィルタして書き出す。 +/// +/// 入力: `source_dir` 配下の `*.jsonl` +/// 出力: `out_path` に [first_commit_time, merged_at] かつ type が user/assistant の行のみ +/// 戻り値: 書き込んだ行数 +pub fn filter_transcripts( + source_dir: &Path, + range: &PrTimeRange, + out_path: &Path, +) -> Result<usize, String> { + if let Some(parent) = out_path.parent() { + fs::create_dir_all(parent) + .map_err(|e| format!("出力ディレクトリ作成失敗 {}: {}", parent.display(), e))?; + } + + let mut writer = fs::File::create(out_path) + .map(std::io::BufWriter::new) + .map_err(|e| format!("出力ファイル作成失敗 {}: {}", out_path.display(), e))?; + + let mut written = 0usize; + let entries = fs::read_dir(source_dir) + .map_err(|e| format!("transcript dir 読込失敗 {}: {}", source_dir.display(), e))?; + + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().and_then(|s| s.to_str()) != Some("jsonl") { + continue; + } + let file = match fs::File::open(&path) { + Ok(f) => f, + Err(_) => continue, // best-effort + }; + let reader = BufReader::new(file); + for line in reader.lines().map_while(Result::ok) { + if line.trim().is_empty() { + continue; + } + if entry_matches_filter(&line, range) { + writeln!(writer, "{}", line).map_err(|e| format!("出力書込失敗: {}", e))?; + written += 1; + } + } + } + + writer.flush().map_err(|e| format!("flush 失敗: {}", e))?; + Ok(written) +} + +/// ISO 8601 UTC タイムスタンプを lexicographic 比較用に正規化する。 +/// +/// `gh api` は秒精度 (`…:SSZ`) を返し、Claude transcript は ms 精度 (`…:SS.fffZ`) を返す。 +/// `'.'` (0x2E) < `'Z'` (0x5A) のため、精度が混在すると境界判定が狂う。 +/// `Z` 末尾かつ小数部なしの文字列を `…:SS.000Z` に揃えることで同一精度での比較を保証する。 +/// +/// 入力契約: タイムスタンプは UTC (`Z` 末尾) であること。`+09:00` 等のオフセット形式は +/// このシステムでは現れない前提。 +fn normalize_timestamp_for_comparison(ts: &str) -> String { + if ts.ends_with('Z') && !ts.contains('.') { + format!("{}.000Z", &ts[..ts.len() - 1]) + } else { + ts.to_string() + } +} + +/// transcript の 1 行が時刻 range + type filter に該当するかを判定する。 +fn entry_matches_filter(line: &str, range: &PrTimeRange) -> bool { + let value: serde_json::Value = match serde_json::from_str(line) { + Ok(v) => v, + Err(_) => return false, + }; + + let entry_type = value.get("type").and_then(|v| v.as_str()).unwrap_or(""); + if !matches!(entry_type, "user" | "assistant") { + return false; + } + + let timestamp = match value.get("timestamp").and_then(|v| v.as_str()) { + Some(t) => t, + None => return false, + }; + + let ts = normalize_timestamp_for_comparison(timestamp); + let lower = normalize_timestamp_for_comparison(range.first_commit_time.as_str()); + let upper = normalize_timestamp_for_comparison(range.merged_at.as_str()); + ts >= lower && ts <= upper +} + +/// `.takt/runs/` 配下で `suffix` に一致する最新ディレクトリ (lex-sort 末尾) を返す。 +fn find_latest_run_dir(runs_dir: &Path, suffix: &str) -> Option<PathBuf> { + let mut candidates: Vec<PathBuf> = fs::read_dir(runs_dir) + .ok()? + .flatten() + .filter_map(|e| { + let path = e.path(); + let name = path.file_name()?.to_string_lossy().into_owned(); + if name.ends_with(suffix) { + Some(path) + } else { + None + } + }) + .collect(); + candidates.sort(); + candidates.into_iter().next_back() +} + +/// `.takt/runs/*-pre-push-review/reports/` のうち最新 (lex-sort 末尾) を返す。 +pub fn find_latest_prepush_reports_dir(repo_root: &Path) -> Option<PathBuf> { + let runs_dir = repo_root.join(".takt").join("runs"); + let latest = find_latest_run_dir(&runs_dir, "-pre-push-review")?; + let reports = latest.join("reports"); + if reports.is_dir() { + Some(reports) + } else { + None + } +} + +/// context file (workflow が Read で読む) を書き出す。 +pub fn write_context_file( + out_path: &Path, + pr_number: u64, + owner_repo: &str, + range: &PrTimeRange, + transcript_relpath: &str, + prepush_reports_dir: &str, +) -> Result<(), String> { + if let Some(parent) = out_path.parent() { + fs::create_dir_all(parent) + .map_err(|e| format!("context dir 作成失敗 {}: {}", parent.display(), e))?; + } + let ctx = WorkflowContext { + pr_number, + owner_repo, + merged_at: &range.merged_at, + first_commit_time: &range.first_commit_time, + transcript_path: transcript_relpath, + prepush_reports_dir, + }; + let json = serde_json::to_string_pretty(&ctx) + .map_err(|e| format!("context JSON serialize 失敗: {}", e))?; + fs::write(out_path, json).map_err(|e| format!("context 書込失敗: {}", e)) +} + +/// takt workflow を spawn し、終了まで待つ。 +/// +/// stdio は inherit (push-runner / pr-monitor と同じパターン)。 +/// timeout 経過時は kill して false を返す。 +pub fn run_takt_workflow(repo_root: &Path, pr_number: u64, timeout_secs: u64) -> bool { + let task_label = format!("{}{}", TAKT_TASK_PREFIX, pr_number); + let mut child = match Command::new("pnpm") + .args(["exec", "takt", "-w", TAKT_WORKFLOW, "-t", &task_label]) + .current_dir(repo_root) + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .spawn() + { + Ok(c) => c, + Err(_) => return false, + }; + + let deadline = std::time::Instant::now() + Duration::from_secs(timeout_secs); + let exited_success = loop { + match child.try_wait() { + Ok(Some(status)) => break Some(status.success()), + Ok(None) if std::time::Instant::now() >= deadline => break None, + Err(_) => break None, + Ok(None) => std::thread::sleep(Duration::from_millis(POLL_INTERVAL_MS)), + } + }; + + match exited_success { + Some(success) => success, + None => { + let _ = child.kill(); + let _ = child.wait(); + false + } + } +} + +/// takt 完了後、最新 run dir の `feedback-report.md` を `.claude/feedback-reports/<pr>.md` にコピーする。 +pub fn copy_feedback_report(repo_root: &Path, pr_number: u64) -> Result<PathBuf, String> { + let runs_dir = repo_root.join(".takt").join("runs"); + let latest = find_latest_run_dir(&runs_dir, &format!("-{}", TAKT_WORKFLOW)) + .ok_or("post-merge-feedback の run dir が見つかりません")?; + + let source = latest.join("reports").join("feedback-report.md"); + if !source.is_file() { + return Err(format!( + "feedback-report.md が見つかりません: {}", + source.display() + )); + } + + let target_dir = repo_root.join(FEEDBACK_DIR); + fs::create_dir_all(&target_dir) + .map_err(|e| format!("feedback dir 作成失敗 {}: {}", target_dir.display(), e))?; + let target = target_dir.join(format!("{}.md", pr_number)); + fs::copy(&source, &target).map_err(|e| { + format!( + "コピー失敗 {} → {}: {}", + source.display(), + target.display(), + e + ) + })?; + Ok(target) +} + +/// `.failed` marker を書き出す (L2 recovery が拾う前提)。 +pub fn write_failed_marker( + repo_root: &Path, + pr_number: u64, + reason: &str, +) -> Result<PathBuf, String> { + let dir = repo_root.join(FEEDBACK_DIR); + fs::create_dir_all(&dir) + .map_err(|e| format!("feedback dir 作成失敗 {}: {}", dir.display(), e))?; + let path = dir.join(format!("{}.md.failed", pr_number)); + let body = format!( + "# post-merge-feedback failed (PR #{})\n\n\ + takt workflow `{}` の同期実行が失敗しました。\n\n\ + ## 失敗理由\n\n{}\n\n\ + ## 復旧手順\n\n\ + 1. このマーカー (`{}`) を残したまま、Claude Code セッションで何か入力する\n\ + 2. UserPromptSubmit hook (`hooks-user-prompt-feedback-recovery`) が検出し、再実行を促す\n\ + 3. または直接: `pnpm feedback-retry {}` (Phase C 以降で実装)\n", + pr_number, + TAKT_WORKFLOW, + reason, + path.display(), + pr_number, + ); + fs::write(&path, body).map_err(|e| format!("failed marker 書込失敗: {}", e))?; + Ok(path) +} + +/// 成功時に `.failed` marker が残っていたら削除する。 +fn cleanup_failed_marker(repo_root: &Path, pr_number: u64) { + let path = repo_root + .join(FEEDBACK_DIR) + .join(format!("{}.md.failed", pr_number)); + let _ = fs::remove_file(path); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn project_id_windows_drive() { + let p = Path::new("E:\\work\\claude-code-hook-test"); + assert_eq!(cwd_to_project_id(p), "e--work-claude-code-hook-test"); + } + + #[test] + fn project_id_unix_path() { + let p = Path::new("/home/user/project"); + assert_eq!(cwd_to_project_id(p), "-home-user-project"); + } + + #[test] + fn entry_matches_user_in_range() { + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00.000Z".into(), + merged_at: "2026-04-25T10:00:00.000Z".into(), + }; + let line = r#"{"type":"user","timestamp":"2026-04-25T09:00:00.000Z"}"#; + assert!(entry_matches_filter(line, &range)); + } + + #[test] + fn entry_skips_assistant_outside_range() { + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00.000Z".into(), + merged_at: "2026-04-25T10:00:00.000Z".into(), + }; + let line = r#"{"type":"assistant","timestamp":"2026-04-25T11:00:00.000Z"}"#; + assert!(!entry_matches_filter(line, &range)); + } + + #[test] + fn entry_skips_queue_operation() { + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00.000Z".into(), + merged_at: "2026-04-25T10:00:00.000Z".into(), + }; + let line = r#"{"type":"queue-operation","timestamp":"2026-04-25T09:00:00.000Z"}"#; + assert!(!entry_matches_filter(line, &range)); + } + + #[test] + fn entry_skips_attachment() { + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00.000Z".into(), + merged_at: "2026-04-25T10:00:00.000Z".into(), + }; + let line = r#"{"type":"attachment","timestamp":"2026-04-25T09:00:00.000Z"}"#; + assert!(!entry_matches_filter(line, &range)); + } + + #[test] + fn entry_skips_invalid_json() { + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00.000Z".into(), + merged_at: "2026-04-25T10:00:00.000Z".into(), + }; + assert!(!entry_matches_filter("not-json", &range)); + } + + #[test] + fn entry_includes_boundary_timestamps() { + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00.000Z".into(), + merged_at: "2026-04-25T10:00:00.000Z".into(), + }; + let lower = r#"{"type":"user","timestamp":"2026-04-25T08:00:00.000Z"}"#; + let upper = r#"{"type":"user","timestamp":"2026-04-25T10:00:00.000Z"}"#; + assert!(entry_matches_filter(lower, &range)); + assert!(entry_matches_filter(upper, &range)); + } + + // gh api は秒精度 (`Z`), transcript は ms 精度 (`.000Z`) を返すため + // 精度が混在しても境界判定が正しく動くことを保証するリグレッションテスト。 + #[test] + fn entry_includes_lower_boundary_with_mixed_precision() { + // first_commit_time が秒精度 (Z 末尾), entry が ms 精度 (.000Z) + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00Z".into(), + merged_at: "2026-04-25T10:00:00Z".into(), + }; + // 下限境界: entry timestamp == first_commit_time (ms = 0) → 含まれるべき + let at_lower = r#"{"type":"user","timestamp":"2026-04-25T08:00:00.000Z"}"#; + assert!(entry_matches_filter(at_lower, &range)); + } + + #[test] + fn entry_excludes_past_upper_boundary_with_mixed_precision() { + // merged_at が秒精度 (Z 末尾), entry が ms 精度 (.000Z) + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00Z".into(), + merged_at: "2026-04-25T10:00:00Z".into(), + }; + // 上限超過: entry timestamp > merged_at (500ms 後) → 含まれないべき + let past_upper = r#"{"type":"user","timestamp":"2026-04-25T10:00:00.500Z"}"#; + assert!(!entry_matches_filter(past_upper, &range)); + } + + #[test] + fn filter_transcripts_writes_only_in_range() { + let dir = std::env::temp_dir().join(format!( + "feedback-filter-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0), + )); + fs::create_dir_all(&dir).unwrap(); + + let session_path = dir.join("session-a.jsonl"); + let mut content = String::new(); + content.push_str(r#"{"type":"user","timestamp":"2026-04-25T07:00:00.000Z"}"#); + content.push('\n'); + content.push_str(r#"{"type":"user","timestamp":"2026-04-25T09:00:00.000Z"}"#); + content.push('\n'); + content.push_str(r#"{"type":"assistant","timestamp":"2026-04-25T09:30:00.000Z"}"#); + content.push('\n'); + content.push_str(r#"{"type":"queue-operation","timestamp":"2026-04-25T09:00:00.000Z"}"#); + content.push('\n'); + content.push_str(r#"{"type":"user","timestamp":"2026-04-25T11:00:00.000Z"}"#); + content.push('\n'); + fs::write(&session_path, content).unwrap(); + + let out_path = dir.join("filtered.jsonl"); + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00.000Z".into(), + merged_at: "2026-04-25T10:00:00.000Z".into(), + }; + let written = filter_transcripts(&dir, &range, &out_path).unwrap(); + assert_eq!(written, 2); + + let out = fs::read_to_string(&out_path).unwrap(); + assert!(out.contains("09:00:00")); + assert!(out.contains("09:30:00")); + assert!(!out.contains("07:00:00")); + assert!(!out.contains("11:00:00")); + assert!(!out.contains("queue-operation")); + + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn write_context_file_serializes_fields() { + let dir = std::env::temp_dir().join(format!( + "feedback-ctx-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0), + )); + fs::create_dir_all(&dir).unwrap(); + let out = dir.join("context.json"); + let range = PrTimeRange { + first_commit_time: "2026-04-25T08:00:00.000Z".into(), + merged_at: "2026-04-25T10:00:00.000Z".into(), + }; + write_context_file( + &out, + 42, + "owner/repo", + &range, + ".takt/transcript.jsonl", + ".takt/runs/foo/reports", + ) + .unwrap(); + + let raw = fs::read_to_string(&out).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&raw).unwrap(); + assert_eq!(parsed.get("pr_number").and_then(|v| v.as_u64()), Some(42)); + assert_eq!( + parsed.get("owner_repo").and_then(|v| v.as_str()), + Some("owner/repo") + ); + assert_eq!( + parsed.get("merged_at").and_then(|v| v.as_str()), + Some("2026-04-25T10:00:00.000Z") + ); + assert_eq!( + parsed.get("transcript_path").and_then(|v| v.as_str()), + Some(".takt/transcript.jsonl") + ); + + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn write_failed_marker_creates_file() { + let root = std::env::temp_dir().join(format!( + "feedback-marker-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0), + )); + fs::create_dir_all(&root).unwrap(); + let path = write_failed_marker(&root, 7, "takt timeout (10 minutes)").unwrap(); + assert!(path.exists()); + let body = fs::read_to_string(&path).unwrap(); + assert!(body.contains("PR #7")); + assert!(body.contains("takt timeout (10 minutes)")); + + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn cleanup_failed_marker_removes_existing() { + let root = std::env::temp_dir().join(format!( + "feedback-cleanup-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0), + )); + fs::create_dir_all(root.join(FEEDBACK_DIR)).unwrap(); + let marker = root.join(FEEDBACK_DIR).join("5.md.failed"); + fs::write(&marker, "old failure").unwrap(); + assert!(marker.exists()); + cleanup_failed_marker(&root, 5); + assert!(!marker.exists()); + + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn find_latest_prepush_picks_lexicographic_max() { + let root = std::env::temp_dir().join(format!( + "feedback-prepush-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.subsec_nanos()) + .unwrap_or(0), + )); + let runs = root.join(".takt").join("runs"); + fs::create_dir_all(runs.join("20260425-000000-pre-push-review").join("reports")).unwrap(); + fs::create_dir_all(runs.join("20260425-094925-pre-push-review").join("reports")).unwrap(); + fs::create_dir_all(runs.join("20260425-100000-other-workflow").join("reports")).unwrap(); + + let latest = find_latest_prepush_reports_dir(&root).unwrap(); + assert!(latest + .to_string_lossy() + .contains("20260425-094925-pre-push-review")); + + let _ = fs::remove_dir_all(&root); + } +} + +/// 全工程を実行する高水準エントリポイント。 +/// +/// 失敗時は `Err(reason)` を返す。caller は `write_failed_marker` で marker を残す前提。 +/// 成功時は `Ok(report_path)` (生成された feedback report の絶対パス)。 +pub fn run(input: &FeedbackInput) -> Result<PathBuf, String> { + let range = fetch_pr_time_range(input.pr_number, input.owner_repo) + .map_err(|e| format!("PR 時刻 range 取得失敗: {}", e))?; + + let context_path = input.repo_root.join(CONTEXT_PATH); + let transcript_path = input.repo_root.join(TRANSCRIPT_PATH); + + let written = match input.transcript_source_dir.as_ref() { + Some(dir) => filter_transcripts(dir, &range, &transcript_path) + .map_err(|e| format!("transcript filter 失敗: {}", e))?, + None => { + // ソース dir 不明: 空 jsonl を出力 (facet が「データなし」分岐に進む) + if let Some(parent) = transcript_path.parent() { + let _ = fs::create_dir_all(parent); + } + let _ = fs::write(&transcript_path, ""); + 0 + } + }; + eprintln!( + "[merge-pipeline] [feedback] transcript filter 完了 ({} entries → {})", + written, + transcript_path.display() + ); + + let prepush_dir = find_latest_prepush_reports_dir(&input.repo_root) + .map(|p| p.to_string_lossy().to_string()) + .unwrap_or_default(); + + write_context_file( + &context_path, + input.pr_number, + input.owner_repo, + &range, + TRANSCRIPT_PATH, + &prepush_dir, + )?; + + if !run_takt_workflow(&input.repo_root, input.pr_number, TAKT_TIMEOUT_SECS) { + return Err(format!( + "takt workflow `{}` が失敗または timeout しました", + TAKT_WORKFLOW + )); + } + + let report = copy_feedback_report(&input.repo_root, input.pr_number)?; + cleanup_failed_marker(&input.repo_root, input.pr_number); + Ok(report) +} diff --git a/src/cli-merge-pipeline/src/main.rs b/src/cli-merge-pipeline/src/main.rs index e946f55..c02fbb3 100644 --- a/src/cli-merge-pipeline/src/main.rs +++ b/src/cli-merge-pipeline/src/main.rs @@ -15,10 +15,9 @@ //! 1 - マージ失敗 / PR 検出失敗 //! 2 - 設定エラー -mod pending_file; +mod feedback; use lib_jj_helpers::{get_jj_bookmarks as lib_get_jj_bookmarks, StderrMode}; -use pending_file::{ExistingPending, PendingFile}; use serde::Deserialize; use std::path::{Path, PathBuf}; use std::process::Command; @@ -48,6 +47,9 @@ struct PipelineStepConfig { #[serde(rename = "type")] step_type: String, cmd: Option<String>, + /// 旧 ADR-029 で参照されていた hint。ADR-030 では takt workflow が固定なので未使用だが、 + /// hooks-config.toml の既存エントリと互換を保つため deserialize 対象として残す。 + #[allow(dead_code)] prompt: Option<String>, } @@ -412,10 +414,9 @@ fn run_steps( } } "ai" => { - // ADR-029: pending file を書き込んで Stop hook 経由で skill を起動する。 + // ADR-030: takt workflow `post-merge-feedback` を同期実行する (L1 Floor)。 // 失敗しても WARN + PASS 扱い (merge 本体は完了済みなので pipeline を止めない)。 - let pending_path = pending_file::default_path(&config_dir()); - run_ai_step(&label, step, ctx, &pending_path); + run_ai_step(&label, ctx); } unknown => { log_step( @@ -430,108 +431,9 @@ fn run_steps( Ok(()) } -/// 書き込み経路の種別 (ADR-029 §競合ポリシー)。 -/// -/// - `NewExclusive`: 既存 pending 不在 → `create_new` で atomic 排他作成 (race 検出可能) -/// - `Overwrite`: 既存 Consumed/Corrupt を削除後の上書き (tmp → rename、race は許容) -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum WriteMode { - NewExclusive, - Overwrite, -} - -impl WriteMode { - fn label(self) -> &'static str { - match self { - WriteMode::NewExclusive => "NewExclusive", - WriteMode::Overwrite => "Overwrite", - } - } -} - -/// 既存の pending file の状態から書き込み経路を決定する (ADR-029 §競合ポリシー)。 -/// -/// Ok(mode) → 書き込みを続行する (mode に応じて write_new_exclusive / write_overwrite)。 -/// Err(()) → スキップ (ログ済み)。 -fn determine_write_mode(label: &str, pending_path: &Path) -> Result<WriteMode, ()> { - match pending_file::read_existing(pending_path) { - ExistingPending::None => Ok(WriteMode::NewExclusive), - ExistingPending::Consumed => { - log_info(&format!( - "[{}] 既存 pending は status='consumed' — 削除して上書き", - label - )); - remove_existing_pending(label, pending_path).map(|()| WriteMode::Overwrite) - } - ExistingPending::Corrupt(reason) => { - // reason は pending file 由来で制御文字を含み得るため {:?} でエスケープ (CodeRabbit Minor) - log_info(&format!( - "[{}] 既存 pending が破損 ({:?}) — 削除して上書き", - label, reason - )); - remove_existing_pending(label, pending_path).map(|()| WriteMode::Overwrite) - } - ExistingPending::Active(status) => { - log_step( - label, - "WARN", - &format!( - "既存 pending が status={:?} — 新規書き込みをスキップ (取りこぼしは ADR-029 将来拡張で対応)", - status - ), - ); - Err(()) - } - } -} - -fn remove_existing_pending(label: &str, path: &Path) -> Result<(), ()> { - std::fs::remove_file(path).map_err(|e| { - log_step( - label, - "WARN", - &format!("既存 pending の削除失敗: {} — 続行不可のためスキップ", e), - ); - }) -} - -fn write_pending_and_log(label: &str, pending: &PendingFile, pending_path: &Path, mode: WriteMode) { - let result = match mode { - WriteMode::NewExclusive => pending_file::write_new_exclusive(pending_path, pending), - WriteMode::Overwrite => pending_file::write_overwrite(pending_path, pending), - }; - match result { - Ok(()) => log_step( - label, - "PASS", - &format!( - "pending file 書き込み完了: {} (PR #{}, prompt={}, mode={})", - pending_path.display(), - pending.pr_number, - pending.prompt, - mode.label() - ), - ), - Err(pending_file::WriteError::AlreadyExists) => log_step( - label, - "WARN", - "別プロセスが同時に pending を書き込みました (create_new AlreadyExists) — 本プロセスの書き込みをスキップ (取りこぼしを WARN で観測)", - ), - // merge 本体は完了済みなので WARN にとどめ、次のマージで復帰可能とする (ADR-029 §破損耐性) - Err(e) => log_step( - label, - "WARN", - &format!( - "pending file 書き込み失敗: {} — merge 完了済みのため続行", - e - ), - ), - } -} - /// `run_ai_step` の入力ガード: PipelineContext の存在・owner_repo の存在・形式を確認する。 /// -/// Ok((pr_number, owner_repo)) → 書き込みを続行できる。 +/// Ok((pr_number, owner_repo)) → workflow を続行できる。 /// Err(()) → スキップ (ログ済み)。 fn validate_ai_step_context<'a>( label: &str, @@ -550,17 +452,17 @@ fn validate_ai_step_context<'a>( log_step( label, "WARN", - "owner_repo を取得できませんでした (gh repo view 失敗?) — pending file を書き込まずスキップ", + "owner_repo を取得できませんでした (gh repo view 失敗?) — feedback workflow をスキップ", ); return Err(()); }; - if !pending_file::is_valid_owner_repo(owner_repo) { + if !lib_pending_file::is_valid_owner_repo(owner_repo) { log_step( label, "WARN", &format!( - "owner_repo {:?} の形式が不正 — pending file を書き込まずスキップ", + "owner_repo {:?} の形式が不正 — feedback workflow をスキップ", owner_repo ), ); @@ -570,44 +472,84 @@ fn validate_ai_step_context<'a>( Ok((ctx.pr_number, owner_repo)) } -/// post-merge の `type = "ai"` ステップを実行する (ADR-029)。 +/// post-merge の `type = "ai"` ステップを実行する (ADR-030 L1 Floor)。 /// /// 戻り値はなし: どの分岐も PASS 扱いでステップを継続させる (pipeline を止めない)。 -/// 具体的な挙動は ADR-029 §競合ポリシー / §破損耐性 に従う。 -fn run_ai_step( - label: &str, - step: &PipelineStepConfig, - ctx: Option<&PipelineContext>, - pending_path: &Path, -) { - let prompt = step - .prompt - .as_deref() - .map(str::trim) - .filter(|s| !s.is_empty()) - .unwrap_or("post-merge-feedback"); - +/// 失敗時は `.failed` marker を残し、L2 recovery (UserPromptSubmit hook, Phase C で実装) +/// が後続 prompt 入力時に再実行を促す。 +fn run_ai_step(label: &str, ctx: Option<&PipelineContext>) { let Ok((pr_number, owner_repo)) = validate_ai_step_context(label, ctx) else { return; }; - let Ok(mode) = determine_write_mode(label, pending_path) else { - return; + let repo_root = match std::env::current_dir() { + Ok(p) => p, + Err(e) => { + log_step( + label, + "WARN", + &format!("current_dir 取得失敗: {} — feedback workflow をスキップ", e), + ); + return; + } }; - let pending = PendingFile { - schema_version: pending_file::SCHEMA_VERSION, + let transcript_source_dir = feedback::project_transcript_dir(&repo_root); + if transcript_source_dir.is_none() { + log_step( + label, + "INFO", + "transcript dir が見つかりません (USERPROFILE 未設定 or session 未生成) — 空 transcript で続行", + ); + } + + let input = feedback::FeedbackInput { pr_number, - owner_repo: owner_repo.to_string(), - prompt: prompt.to_string(), - status: pending_file::STATUS_PENDING.to_string(), - created_at: pending_file::utc_now_iso8601(), - dispatched_at: None, - consumed_at: None, - producer: Some(pending_file::producer_string()), + owner_repo, + repo_root: repo_root.clone(), + transcript_source_dir, }; - write_pending_and_log(label, &pending, pending_path, mode); + log_step( + label, + "RUN", + &format!( + "takt workflow `post-merge-feedback` を同期実行 (PR #{})", + pr_number + ), + ); + + match feedback::run(&input) { + Ok(report) => { + log_step( + label, + "PASS", + &format!("feedback report 生成: {}", report.display()), + ); + } + Err(reason) => { + // soft fail: merge は成功扱いで続行。marker を残して L2 recovery に委ねる。 + match feedback::write_failed_marker(&repo_root, pr_number, &reason) { + Ok(marker) => log_step( + label, + "WARN", + &format!( + "feedback workflow 失敗: {} — marker: {} (L2 recovery が拾います)", + reason, + marker.display() + ), + ), + Err(marker_err) => log_step( + label, + "WARN", + &format!( + "feedback workflow 失敗: {} — marker 書込も失敗: {}", + reason, marker_err + ), + ), + } + } + } } fn delete_remote_branch(branch_name: &str) { @@ -995,307 +937,43 @@ step_timeout = 60 // lib-jj-helpers/src/lib.rs#tests に集約 (ADR-024 本採用、PR-C で移設)。 // cli-merge-pipeline 側からは lib_jj_helpers の公開 API 経由でのみ使用する。 - // ─── AI step (ADR-029) ─── - - fn unique_tmp_pending(label: &str) -> PathBuf { - std::env::temp_dir().join(format!( - "cli-merge-ai-{}-{}-{}.json", - label, - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.subsec_nanos()) - .unwrap_or(0), - )) - } - - fn ai_step(prompt: Option<&str>) -> PipelineStepConfig { - PipelineStepConfig { - name: "post_merge_feedback".to_string(), - step_type: "ai".to_string(), - cmd: None, - prompt: prompt.map(str::to_string), - } - } - - fn read_pending(path: &Path) -> PendingFile { - let content = std::fs::read_to_string(path).expect("pending file should exist"); - serde_json::from_str(&content).expect("pending file should parse") - } - - #[test] - fn ai_step_writes_pending_when_ctx_present() { - let path = unique_tmp_pending("writes-pending"); - let ctx = PipelineContext { - pr_number: 123, - owner_repo: Some("aloekun/claude-code-hook-test".to_string()), - }; - let step = ai_step(Some("post-merge-feedback")); - - run_ai_step("test", &step, Some(&ctx), &path); - - let loaded = read_pending(&path); - assert_eq!(loaded.schema_version, pending_file::SCHEMA_VERSION); - assert_eq!(loaded.pr_number, 123); - assert_eq!(loaded.owner_repo, "aloekun/claude-code-hook-test"); - assert_eq!(loaded.prompt, "post-merge-feedback"); - assert_eq!(loaded.status, pending_file::STATUS_PENDING); - assert!(loaded.dispatched_at.is_none()); - assert!(loaded.consumed_at.is_none()); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn ai_step_uses_default_prompt_when_not_set() { - let path = unique_tmp_pending("default-prompt"); - let ctx = PipelineContext { - pr_number: 1, - owner_repo: Some("o/r".to_string()), - }; - let step = ai_step(None); - - run_ai_step("test", &step, Some(&ctx), &path); - - assert_eq!(read_pending(&path).prompt, "post-merge-feedback"); - - let _ = std::fs::remove_file(&path); - } + // ─── AI step input gate (ADR-030) ─── + // + // 旧 pending file ベースの ai_step_* tests は ADR-030 で takt workflow に置き換わったため削除。 + // run_ai_step() の成功パスは feedback module の単体テスト + 実マージ dogfood で検証する。 + // ここでは入力ガード (validate_ai_step_context) の skip 経路のみカバーする。 #[test] - fn ai_step_skips_when_ctx_none() { - let path = unique_tmp_pending("ctx-none"); - let step = ai_step(Some("post-merge-feedback")); - - run_ai_step("test", &step, None, &path); - - assert!(!path.exists(), "pending file should not be created"); + fn validate_ai_step_skips_when_ctx_none() { + assert!(validate_ai_step_context("test", None).is_err()); } #[test] - fn ai_step_skips_when_owner_repo_none() { - let path = unique_tmp_pending("owner-repo-none"); + fn validate_ai_step_skips_when_owner_repo_none() { let ctx = PipelineContext { pr_number: 42, owner_repo: None, }; - let step = ai_step(Some("post-merge-feedback")); - - run_ai_step("test", &step, Some(&ctx), &path); - - assert!(!path.exists()); + assert!(validate_ai_step_context("test", Some(&ctx)).is_err()); } #[test] - fn ai_step_skips_when_owner_repo_invalid() { - let path = unique_tmp_pending("owner-repo-invalid"); + fn validate_ai_step_skips_when_owner_repo_invalid() { let ctx = PipelineContext { pr_number: 42, owner_repo: Some("has space/repo".to_string()), }; - let step = ai_step(Some("post-merge-feedback")); - - run_ai_step("test", &step, Some(&ctx), &path); - - assert!(!path.exists()); - } - - #[test] - fn ai_step_overwrites_consumed_pending() { - let path = unique_tmp_pending("overwrite-consumed"); - let consumed = PendingFile { - schema_version: pending_file::SCHEMA_VERSION, - pr_number: 999, - owner_repo: "old/repo".to_string(), - prompt: "post-merge-feedback".to_string(), - status: pending_file::STATUS_CONSUMED.to_string(), - created_at: "2026-04-01T00:00:00Z".to_string(), - dispatched_at: Some("2026-04-01T00:01:00Z".to_string()), - consumed_at: Some("2026-04-01T00:02:00Z".to_string()), - producer: None, - }; - pending_file::write_new_exclusive(&path, &consumed).unwrap(); - - let ctx = PipelineContext { - pr_number: 555, - owner_repo: Some("new/repo".to_string()), - }; - run_ai_step("test", &ai_step(None), Some(&ctx), &path); - - let loaded = read_pending(&path); - assert_eq!(loaded.pr_number, 555); - assert_eq!(loaded.owner_repo, "new/repo"); - assert_eq!(loaded.status, pending_file::STATUS_PENDING); - assert!(loaded.dispatched_at.is_none()); - assert!(loaded.consumed_at.is_none()); - - let _ = std::fs::remove_file(&path); + assert!(validate_ai_step_context("test", Some(&ctx)).is_err()); } #[test] - fn ai_step_skips_when_existing_pending_is_active() { - let path = unique_tmp_pending("existing-active"); - let existing = PendingFile { - schema_version: pending_file::SCHEMA_VERSION, - pr_number: 100, - owner_repo: "a/b".to_string(), - prompt: "post-merge-feedback".to_string(), - status: pending_file::STATUS_PENDING.to_string(), - created_at: "2026-04-22T00:00:00Z".to_string(), - dispatched_at: None, - consumed_at: None, - producer: None, - }; - pending_file::write_new_exclusive(&path, &existing).unwrap(); - - let ctx = PipelineContext { - pr_number: 200, - owner_repo: Some("c/d".to_string()), - }; - run_ai_step("test", &ai_step(None), Some(&ctx), &path); - - // 既存が保持される - let loaded = read_pending(&path); - assert_eq!(loaded.pr_number, 100); - assert_eq!(loaded.owner_repo, "a/b"); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn ai_step_skips_when_existing_pending_is_dispatched() { - let path = unique_tmp_pending("existing-dispatched"); - let existing = PendingFile { - schema_version: pending_file::SCHEMA_VERSION, - pr_number: 100, - owner_repo: "a/b".to_string(), - prompt: "post-merge-feedback".to_string(), - status: pending_file::STATUS_DISPATCHED.to_string(), - created_at: "2026-04-22T00:00:00Z".to_string(), - dispatched_at: Some("2026-04-22T00:01:00Z".to_string()), - consumed_at: None, - producer: None, - }; - pending_file::write_new_exclusive(&path, &existing).unwrap(); - + fn validate_ai_step_passes_with_valid_owner_repo() { let ctx = PipelineContext { - pr_number: 200, - owner_repo: Some("c/d".to_string()), - }; - run_ai_step("test", &ai_step(None), Some(&ctx), &path); - - let loaded = read_pending(&path); - assert_eq!(loaded.pr_number, 100); - assert_eq!(loaded.status, pending_file::STATUS_DISPATCHED); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn ai_step_overwrites_corrupt_pending() { - let path = unique_tmp_pending("corrupt-parse"); - std::fs::write(&path, "this is not valid json").unwrap(); - - let ctx = PipelineContext { - pr_number: 777, - owner_repo: Some("x/y".to_string()), - }; - run_ai_step("test", &ai_step(None), Some(&ctx), &path); - - let loaded = read_pending(&path); - assert_eq!(loaded.pr_number, 777); - assert_eq!(loaded.status, pending_file::STATUS_PENDING); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn ai_step_overwrites_empty_pending() { - let path = unique_tmp_pending("corrupt-empty"); - std::fs::write(&path, "").unwrap(); - - let ctx = PipelineContext { - pr_number: 888, - owner_repo: Some("x/y".to_string()), - }; - run_ai_step("test", &ai_step(None), Some(&ctx), &path); - - let loaded = read_pending(&path); - assert_eq!(loaded.pr_number, 888); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn ai_step_leaves_no_tmp_residue_on_success() { - let path = unique_tmp_pending("no-tmp-residue"); - let ctx = PipelineContext { - pr_number: 1, - owner_repo: Some("o/r".to_string()), - }; - run_ai_step("test", &ai_step(None), Some(&ctx), &path); - - // 新形式: tmp file 名は "{basename}.tmp.{pid}.{counter}" - let dir = path.parent().unwrap_or(std::path::Path::new(".")); - let basename = path.file_name().unwrap().to_string_lossy().into_owned(); - let tmp_prefix = format!("{}.tmp.", basename); - let residues: Vec<_> = std::fs::read_dir(dir) - .unwrap() - .flatten() - .filter_map(|e| { - let name = e.file_name().to_string_lossy().into_owned(); - if name.starts_with(&tmp_prefix) { - Some(e.path()) - } else { - None - } - }) - .collect(); - assert!(residues.is_empty(), "tmp residue: {:?}", residues); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn ai_step_sets_producer_field() { - let path = unique_tmp_pending("producer-set"); - let ctx = PipelineContext { - pr_number: 1, - owner_repo: Some("o/r".to_string()), - }; - run_ai_step("test", &ai_step(None), Some(&ctx), &path); - - let loaded = read_pending(&path); - let producer = loaded.producer.expect("producer should be set"); - assert!(producer.starts_with("cli-merge-pipeline@pid-")); - assert!(producer.ends_with('Z')); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn ai_step_warns_when_concurrent_writer_wins_new_exclusive() { - // 同じ path に 2 度 run_ai_step を呼ぶ: 1 度目は成功、2 度目は既存 Active で skip - // (read_existing が Active を返す経路なので直接の AlreadyExists ではないが、 - // 不可視ロストが起きないことを担保する代表シナリオ) - let path = unique_tmp_pending("concurrent-win"); - let ctx = PipelineContext { - pr_number: 1, - owner_repo: Some("o/r".to_string()), - }; - run_ai_step("test", &ai_step(None), Some(&ctx), &path); - let first = read_pending(&path); - assert_eq!(first.pr_number, 1); - - // 2 度目: 既存 pending (Active) があるため skip (上書きされない) - let ctx2 = PipelineContext { - pr_number: 2, - owner_repo: Some("o/r".to_string()), + pr_number: 7, + owner_repo: Some("aloekun/claude-code-hook-test".to_string()), }; - run_ai_step("test", &ai_step(None), Some(&ctx2), &path); - let second = read_pending(&path); - assert_eq!(second.pr_number, 1, "既存 pending が上書きされてはならない"); - - let _ = std::fs::remove_file(&path); + let (pr, repo) = validate_ai_step_context("test", Some(&ctx)).unwrap(); + assert_eq!(pr, 7); + assert_eq!(repo, "aloekun/claude-code-hook-test"); } } diff --git a/src/cli-merge-pipeline/src/pending_file.rs b/src/cli-merge-pipeline/src/pending_file.rs deleted file mode 100644 index 013964d..0000000 --- a/src/cli-merge-pipeline/src/pending_file.rs +++ /dev/null @@ -1,511 +0,0 @@ -//! post-merge-feedback pending file の読み書きと入力検証 (ADR-029) -//! -//! pending file は `.claude/post-merge-feedback-pending.json` に配置され、 -//! cli-merge-pipeline が post-merge ステップ (`type = "ai"`) で書き込み、 -//! hooks-stop-feedback-dispatch が Stop 時に検出して Claude に skill 起動を指示する。 -//! -//! 共有スキーマ・定数・UTC ヘルパーは `lib-pending-file` に集約。 -//! 本モジュールは cli-merge-pipeline 固有の書き込みロジックと I/O を担う。 -//! -//! 書き込み経路は 2 種類 (ADR-029 §破損耐性): -//! - 新規作成: `OpenOptions::new().write(true).create_new(true).open(path)` で -//! 最終ファイルを直接 atomic 排他作成 (O_EXCL 相当)。rename は使わない。 -//! - 上書き (既存 Consumed/Corrupt 削除後): tmp file → `fs::rename` の 2 段階。 -//! -//! 排他性保証: -//! - 新規作成経路: `create_new` の `AlreadyExists` で TOCTOU race を atomic に検出 -//! (ADR-029 §競合ポリシーの「skip + WARN で取りこぼしを観測可能」を実装レベルで保証) -//! - 上書き経路: read→write 間の race は許容 (Consumed/Corrupt は稀経路、 -//! 破損ポリシーで自己回復) - -use std::fs::OpenOptions; -use std::io::{ErrorKind, Write}; -use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicU64, Ordering}; - -// ─── Re-exports from lib-pending-file ─── - -pub(crate) use lib_pending_file::PendingFile; -pub(crate) use lib_pending_file::{ - is_valid_owner_repo, utc_now_iso8601, FILE_NAME, SCHEMA_VERSION, STATUS_CONSUMED, - STATUS_DISPATCHED, STATUS_PENDING, -}; - -// ─── cli-merge-pipeline-local items ─── - -/// プロセス内で一意な tmp ファイル名を生成するためのカウンタ。 -/// 複数の writer が同時に `write_overwrite` を呼んでも tmp パスが衝突しない。 -/// `write_new_exclusive` は tmp path を使わないため本カウンタを参照しない。 -static TMP_COUNTER: AtomicU64 = AtomicU64::new(0); - -/// producer 文字列を生成する (`cli-merge-pipeline@pid-{pid}@{iso8601}`)。 -/// -/// PID は再利用されるため timestamp を併記して時系列追跡可能にする。hostname は YAGNI で省略。 -pub(crate) fn producer_string() -> String { - format!( - "cli-merge-pipeline@pid-{}@{}", - std::process::id(), - utc_now_iso8601() - ) -} - -/// 既存 pending file の読み取り結果。 -/// -/// status 未知値は Corrupt 扱いとする (ADR-029 は `pending`/`dispatched`/`consumed` -/// のみを schema_version=1 で enumerate しているため、それ以外は schema drift とみなす)。 -#[derive(Debug)] -pub(crate) enum ExistingPending { - /// ファイル不在。通常経路。 - None, - /// size 0 / parse 失敗 / schema_version 不一致 / 未知 status。削除して再書き込みする。 - Corrupt(String), - /// status = "consumed"。上書き OK。 - Consumed, - /// status = "pending" または "dispatched"。書き込みをスキップする (WARN)。 - Active(String), -} - -/// 既存 pending file の状態を判定する。 -pub(crate) fn read_existing(path: &Path) -> ExistingPending { - let meta = match std::fs::metadata(path) { - Ok(m) => m, - Err(_) => return ExistingPending::None, - }; - if meta.len() == 0 { - return ExistingPending::Corrupt("size=0".to_string()); - } - let content = match std::fs::read_to_string(path) { - Ok(c) => c, - Err(e) => return ExistingPending::Corrupt(format!("read error: {}", e)), - }; - let pending: PendingFile = match serde_json::from_str(&content) { - Ok(p) => p, - Err(e) => return ExistingPending::Corrupt(format!("parse error: {}", e)), - }; - if pending.schema_version != SCHEMA_VERSION { - return ExistingPending::Corrupt(format!( - "schema_version mismatch (got {}, want {})", - pending.schema_version, SCHEMA_VERSION - )); - } - match pending.status.as_str() { - STATUS_CONSUMED => ExistingPending::Consumed, - STATUS_PENDING | STATUS_DISPATCHED => ExistingPending::Active(pending.status), - other => ExistingPending::Corrupt(format!("unknown status '{}'", other)), - } -} - -/// pending file 書き込みの失敗種別。 -/// -/// `AlreadyExists` は新規作成経路で TOCTOU race を atomic に検出した場合に返る -/// (ADR-029 §競合ポリシー)。呼び出し側は `WARN + skip` でログに残すことで -/// 「取りこぼしの可視化」を保証する。 -#[derive(Debug)] -pub(crate) enum WriteError { - /// 新規作成時に既に pending file が存在した (他プロセスが先に書き込んだ) - AlreadyExists, - /// I/O エラー (書き込み / sync / rename 失敗) - Io(String), - /// serde_json シリアライズ失敗 - Serialize(String), -} - -impl std::fmt::Display for WriteError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - WriteError::AlreadyExists => write!(f, "pending file already exists"), - WriteError::Io(msg) => write!(f, "I/O error: {}", msg), - WriteError::Serialize(msg) => write!(f, "serialize error: {}", msg), - } - } -} - -/// pending file を **新規排他作成** する (ADR-029 §競合ポリシー)。 -/// -/// `OpenOptions::create_new(true)` (O_EXCL 相当) で最終ファイルを直接 atomic 排他作成する。 -/// 他プロセスが先に作成済みなら `WriteError::AlreadyExists` を返し、呼び出し側は -/// WARN + skip で取りこぼしを観測可能にする。 -/// -/// **rename は使わない** — `rename` は既存を無条件上書きするため、placeholder 方式だと -/// 排他予約が自壊する (CodeRabbit PR #70 Major 指摘の本質)。 -pub(crate) fn write_new_exclusive(path: &Path, pending: &PendingFile) -> Result<(), WriteError> { - let json = - serde_json::to_string_pretty(pending).map_err(|e| WriteError::Serialize(e.to_string()))?; - - match OpenOptions::new().write(true).create_new(true).open(path) { - Ok(mut file) => { - if let Err(e) = file.write_all(json.as_bytes()) { - return Err(WriteError::Io(format!("write_all 失敗: {}", e))); - } - if let Err(e) = file.sync_all() { - return Err(WriteError::Io(format!("sync_all 失敗: {}", e))); - } - Ok(()) - } - Err(e) if e.kind() == ErrorKind::AlreadyExists => Err(WriteError::AlreadyExists), - Err(e) => Err(WriteError::Io(format!( - "create_new 失敗 ({}): {}", - path.display(), - e - ))), - } -} - -/// pending file を **上書き書き込み** する (tmp → rename 2 段階)。 -/// -/// 呼び出し元は事前に `read_existing` で `Consumed` / `Corrupt` を判定し、 -/// ファイルを削除してから本関数を呼ぶことを想定。稀経路なので read→write 間の -/// race は許容 (ADR-029 §競合ポリシー)。 -pub(crate) fn write_overwrite(path: &Path, pending: &PendingFile) -> Result<(), WriteError> { - let json = - serde_json::to_string_pretty(pending).map_err(|e| WriteError::Serialize(e.to_string()))?; - let counter = TMP_COUNTER.fetch_add(1, Ordering::Relaxed); - let file_name = path - .file_name() - .map(|n| n.to_string_lossy().into_owned()) - .unwrap_or_else(|| "pending".to_string()); - let tmp_name = format!("{}.tmp.{}.{}", file_name, std::process::id(), counter); - let tmp_path = path.with_file_name(tmp_name); - if let Err(e) = std::fs::write(&tmp_path, &json) { - let _ = std::fs::remove_file(&tmp_path); - return Err(WriteError::Io(format!( - "tmp 書き込み失敗 ({}): {}", - tmp_path.display(), - e - ))); - } - if let Err(e) = std::fs::rename(&tmp_path, path) { - let _ = std::fs::remove_file(&tmp_path); - return Err(WriteError::Io(format!( - "rename 失敗 ({} → {}): {}", - tmp_path.display(), - path.display(), - e - ))); - } - Ok(()) -} - -/// pending file のデフォルト配置先 (exe と同じディレクトリ = `.claude/`)。 -/// -/// 本プロジェクトは `pnpm deploy:hooks` で exe を `.claude/` に配置するため、 -/// exe の親ディレクトリが pending file の正しい置き場になる。派生プロジェクトも同様。 -pub(crate) fn default_path(config_dir: &Path) -> PathBuf { - config_dir.join(FILE_NAME) -} - -#[cfg(test)] -mod tests { - use super::*; - - fn sample_pending(status: &str) -> PendingFile { - PendingFile { - schema_version: SCHEMA_VERSION, - pr_number: 123, - owner_repo: "aloekun/claude-code-hook-test".to_string(), - prompt: "post-merge-feedback".to_string(), - status: status.to_string(), - created_at: "2026-04-23T10:00:00Z".to_string(), - dispatched_at: None, - consumed_at: None, - producer: None, - } - } - - fn unique_tmp(label: &str) -> PathBuf { - std::env::temp_dir().join(format!( - "pending-{}-{}-{}.json", - label, - std::process::id(), - // nanosecond で同一 pid 内の衝突も回避 - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.subsec_nanos()) - .unwrap_or(0), - )) - } - - #[test] - fn valid_owner_repo_accepts_typical_slugs() { - assert!(is_valid_owner_repo("aloekun/claude-code-hook-test")); - assert!(is_valid_owner_repo("octo-org/my.repo")); - assert!(is_valid_owner_repo("a/b")); - assert!(is_valid_owner_repo("Ab_12/X.y-z")); - } - - #[test] - fn valid_owner_repo_rejects_malformed() { - assert!(!is_valid_owner_repo("")); - assert!(!is_valid_owner_repo("noslash")); - assert!(!is_valid_owner_repo("/missing-owner")); - assert!(!is_valid_owner_repo("missing-repo/")); - assert!(!is_valid_owner_repo("a/b/c")); // 複数スラッシュ - assert!(!is_valid_owner_repo("has space/repo")); - assert!(!is_valid_owner_repo("owner/repo\nfoo")); // newline injection - assert!(!is_valid_owner_repo("owner/repo\r")); - assert!(!is_valid_owner_repo("owner/repo\t")); - assert!(!is_valid_owner_repo("owner!/repo")); - } - - #[test] - fn write_new_exclusive_creates_file() { - let path = unique_tmp("write-new-exclusive"); - let pending = sample_pending(STATUS_PENDING); - - write_new_exclusive(&path, &pending).unwrap(); - - let loaded: PendingFile = - serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); - assert_eq!(loaded, pending); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn write_new_exclusive_returns_already_exists_when_target_present() { - let path = unique_tmp("already-exists"); - let pending = sample_pending(STATUS_PENDING); - - // 1 回目は成功 - write_new_exclusive(&path, &pending).unwrap(); - // 2 回目は AlreadyExists - match write_new_exclusive(&path, &pending) { - Err(WriteError::AlreadyExists) => {} - other => panic!("expected AlreadyExists, got {:?}", other), - } - // 既存内容は上書きされていない - let loaded: PendingFile = - serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); - assert_eq!(loaded, pending); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn write_new_exclusive_atomic_under_concurrent_writers() { - use std::sync::atomic::AtomicUsize; - use std::sync::atomic::Ordering as AtomicOrdering; - use std::sync::Arc; - - let path = Arc::new(unique_tmp("concurrent-new-exclusive")); - let success_count = Arc::new(AtomicUsize::new(0)); - let already_exists_count = Arc::new(AtomicUsize::new(0)); - - let handles: Vec<_> = (0..8) - .map(|i| { - let p = Arc::clone(&path); - let ok = Arc::clone(&success_count); - let ae = Arc::clone(&already_exists_count); - std::thread::spawn(move || { - let mut pf = sample_pending(STATUS_PENDING); - pf.pr_number = i + 1; - match write_new_exclusive(&p, &pf) { - Ok(()) => { - ok.fetch_add(1, AtomicOrdering::Relaxed); - } - Err(WriteError::AlreadyExists) => { - ae.fetch_add(1, AtomicOrdering::Relaxed); - } - Err(other) => panic!("unexpected write error: {:?}", other), - } - }) - }) - .collect(); - - for h in handles { - h.join().unwrap(); - } - - // atomic 排他予約が効いているなら、成功は 1 スレッドのみ・残りは AlreadyExists - assert_eq!(success_count.load(AtomicOrdering::Relaxed), 1); - assert_eq!(already_exists_count.load(AtomicOrdering::Relaxed), 7); - - // 最終ファイルは有効な JSON - let content = std::fs::read_to_string(path.as_ref()).unwrap(); - let _: PendingFile = serde_json::from_str(&content).unwrap(); - - let _ = std::fs::remove_file(path.as_ref()); - } - - #[test] - fn write_overwrite_replaces_existing_file() { - let path = unique_tmp("overwrite"); - let first = sample_pending(STATUS_CONSUMED); - let mut second = sample_pending(STATUS_PENDING); - second.pr_number = 999; - - write_new_exclusive(&path, &first).unwrap(); - write_overwrite(&path, &second).unwrap(); - - let loaded: PendingFile = - serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); - assert_eq!(loaded, second); - - // tmp 残骸 (`{basename}.tmp.{pid}.{counter}`) が残っていないこと - let dir = path.parent().unwrap_or(std::path::Path::new(".")); - let basename = path.file_name().unwrap().to_string_lossy().into_owned(); - let tmp_prefix = format!("{}.tmp.", basename); - let residues: Vec<_> = std::fs::read_dir(dir) - .unwrap() - .flatten() - .filter(|e| e.file_name().to_string_lossy().starts_with(&tmp_prefix)) - .collect(); - assert!( - residues.is_empty(), - "tmp residue: {} entries", - residues.len() - ); - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn producer_string_contains_pid_and_timestamp() { - let s = producer_string(); - assert!(s.starts_with("cli-merge-pipeline@pid-")); - // @{iso8601} が含まれることを "Z" で軽く確認 - assert!(s.ends_with('Z'), "expected iso8601 suffix: {}", s); - // pid 部分が数値 - let pid_part = s - .strip_prefix("cli-merge-pipeline@pid-") - .and_then(|rest| rest.split('@').next()) - .unwrap(); - assert!(pid_part.chars().all(|c| c.is_ascii_digit())); - } - - #[test] - fn pending_file_roundtrip_with_producer() { - let mut pending = sample_pending(STATUS_PENDING); - pending.producer = Some("cli-merge-pipeline@pid-1234@2026-04-23T12:34:56Z".to_string()); - - let json = serde_json::to_string(&pending).unwrap(); - let loaded: PendingFile = serde_json::from_str(&json).unwrap(); - assert_eq!(loaded, pending); - } - - #[test] - fn pending_file_without_producer_field_deserializes() { - // producer フィールド不在の JSON (schema v1 既存ファイル) が正しく読めること - let json = r#"{ - "schema_version": 1, - "pr_number": 42, - "owner_repo": "o/r", - "prompt": "post-merge-feedback", - "status": "pending", - "created_at": "2026-04-23T10:00:00Z", - "dispatched_at": null, - "consumed_at": null - }"#; - let loaded: PendingFile = serde_json::from_str(json).unwrap(); - assert_eq!(loaded.producer, None); - assert_eq!(loaded.pr_number, 42); - } - - #[test] - fn read_existing_returns_none_when_absent() { - let path = unique_tmp("absent"); - assert!(matches!(read_existing(&path), ExistingPending::None)); - } - - #[test] - fn read_existing_returns_corrupt_for_empty_file() { - let path = unique_tmp("empty"); - std::fs::write(&path, "").unwrap(); - - match read_existing(&path) { - ExistingPending::Corrupt(reason) => assert!(reason.contains("size=0")), - other => panic!("expected Corrupt, got {:?}", other), - } - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn read_existing_returns_corrupt_for_invalid_json() { - let path = unique_tmp("bad-json"); - std::fs::write(&path, "not a json").unwrap(); - - match read_existing(&path) { - ExistingPending::Corrupt(reason) => assert!(reason.contains("parse")), - other => panic!("expected Corrupt, got {:?}", other), - } - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn read_existing_returns_corrupt_for_schema_mismatch() { - let path = unique_tmp("bad-schema"); - let mut pending = sample_pending(STATUS_PENDING); - pending.schema_version = 99; - write_new_exclusive(&path, &pending).unwrap(); - - match read_existing(&path) { - ExistingPending::Corrupt(reason) => { - assert!(reason.contains("schema_version")); - } - other => panic!("expected Corrupt, got {:?}", other), - } - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn read_existing_returns_corrupt_for_unknown_status() { - let path = unique_tmp("bad-status"); - let pending = sample_pending("garbage"); - write_new_exclusive(&path, &pending).unwrap(); - - match read_existing(&path) { - ExistingPending::Corrupt(reason) => assert!(reason.contains("unknown status")), - other => panic!("expected Corrupt, got {:?}", other), - } - - let _ = std::fs::remove_file(&path); - } - - #[test] - fn read_existing_returns_active_for_pending_and_dispatched() { - let path_p = unique_tmp("active-pending"); - write_new_exclusive(&path_p, &sample_pending(STATUS_PENDING)).unwrap(); - match read_existing(&path_p) { - ExistingPending::Active(s) => assert_eq!(s, STATUS_PENDING), - other => panic!("expected Active(pending), got {:?}", other), - } - let _ = std::fs::remove_file(&path_p); - - let path_d = unique_tmp("active-dispatched"); - write_new_exclusive(&path_d, &sample_pending(STATUS_DISPATCHED)).unwrap(); - match read_existing(&path_d) { - ExistingPending::Active(s) => assert_eq!(s, STATUS_DISPATCHED), - other => panic!("expected Active(dispatched), got {:?}", other), - } - let _ = std::fs::remove_file(&path_d); - } - - #[test] - fn read_existing_returns_consumed_for_consumed_status() { - let path = unique_tmp("consumed"); - write_new_exclusive(&path, &sample_pending(STATUS_CONSUMED)).unwrap(); - assert!(matches!(read_existing(&path), ExistingPending::Consumed)); - let _ = std::fs::remove_file(&path); - } - - #[test] - fn utc_now_iso8601_matches_expected_format() { - let s = utc_now_iso8601(); - // YYYY-MM-DDTHH:MM:SSZ = 20 chars - assert_eq!( - s.len(), - "1970-01-01T00:00:00Z".len(), - "unexpected length: {}", - s - ); - assert!(s.ends_with('Z')); - assert_eq!(s.chars().nth(4), Some('-')); - assert_eq!(s.chars().nth(7), Some('-')); - assert_eq!(s.chars().nth(10), Some('T')); - assert_eq!(s.chars().nth(13), Some(':')); - assert_eq!(s.chars().nth(16), Some(':')); - } -}