Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
18 changes: 14 additions & 4 deletions .takt/facets/instructions/analyze-coderabbit.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,21 @@ CodeRabbit sometimes raises findings that are not applicable to this project. Be
2. For each finding, check:
- **Platform scope**: This project targets Windows only. Findings about cross-platform compatibility (e.g., `.exe` hardcoding) are NOT applicable -- downgrade to `Info`
- **Intentional design**: Check if the finding contradicts an ADR decision. If so, mark as `not_applicable`
- **Scope mismatch**: If the finding targets a read-only zone (`.takt/`, `docs/adr/`, `templates/`), mark as `not_applicable`
- **Sensitive-file protection** (Edit-blocked): If the finding targets `.claude/` (Claude Code sensitive-file protected — Edit/Write tool will refuse), mark as `user_decision_path` (NOT `not_applicable` — the issue may be real, but auto-fix cannot apply it)
- **Scope mismatch**: If the finding targets a read-only zone (`.takt/`, `docs/adr/`, `templates/`) or a non-source path (`.git/`, `.jj/`, `node_modules/`, `target/`), mark as `not_applicable`
- **False positive**: If the finding misunderstands the code logic, mark as `not_applicable`

Mark each finding as:
- `applicable` -- genuine issue that should be addressed
- `user_decision_path` -- finding is real but auto-fix is blocked by sensitive-file protection (`.claude/`); user decides
- `not_applicable` -- does not apply to this project (with reason)

Comment thread
coderabbitai[bot] marked this conversation as resolved.
### Step 3: Severity classification
For `applicable` findings only, classify by severity:
For both `applicable` and `user_decision_path` findings, take the severity from CodeRabbit's `severity` field (do not reclassify):
- Critical > High > Major > Medium > Minor > Low > Info

The severity is preserved on `user_decision_path` findings so the user can prioritize their manual decision (a Critical `.claude/` finding still warrants attention even though auto-fix cannot apply it). For `not_applicable` findings, severity is irrelevant and may be omitted from the report.

### Step 4: Produce report and verdict

## Output Format
Expand All @@ -50,6 +54,11 @@ For `applicable` findings only, classify by severity:
|---|-------------|-------|---------------|
| 1 | path:line | ... | Platform scope: Windows only |

### User Decision Path (sensitive-file protected)
| # | File (Line) | Severity | Issue | Path Reason |
|---|-------------|----------|-------|-------------|
| 1 | .claude/... | Major | ... | sensitive-file protection — auto-fix blocked |

### Applicable Findings by Severity

#### Critical / High / Major
Expand All @@ -69,12 +78,13 @@ For `applicable` findings only, classify by severity:

- **approved**: No applicable findings, OR all applicable findings are Info/Low severity
- Output: `approved` condition
- **needs_fix**: Any applicable Critical, High, or Major finding exists
- **needs_fix**: Any applicable Critical, High, or Major finding exists (excluding `user_decision_path`)
- Output: `needs_fix` condition
- These will be automatically fixed in the next step
- **user_decision**: Only Medium or lower applicable findings exist (no Critical/High/Major)
- **user_decision**: Only Medium or lower applicable findings exist, OR all remaining findings are `user_decision_path` (sensitive-file protected) regardless of severity
- Output: `user_decision` condition
- These are reported but NOT auto-fixed; the user decides
- **Important**: A `.claude/` finding of any severity routes here to prevent fix loop pathology (auto-fix would attempt 4+ Edit calls all blocked by sensitive-file protection, wasting iterations)

## Important

Expand Down
17 changes: 17 additions & 0 deletions .takt/facets/instructions/review-security.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,20 @@ Before evaluating the change, **read the following project documents** using the
- Whether the change enables new privilege, data access, code execution, or prompt modification
3. For each detected issue, classify it as blocking or non-blocking
4. If there is even one blocking issue, judge as REJECT

## Docs-only changes: trust boundary criterion

For changes that touch **only documentation** (`docs/**`, ADRs, README, comments) and no executable code or configuration:

- **Pass criterion**: If the change does NOT alter a trust boundary, judge as APPROVE without further security analysis
- **Trust boundary unchanged** (APPROVE immediately):
- Policy explanations, terminology definitions, design rationale
- Workflow descriptions, ADR records of past decisions
- Reformatting, hierarchy reorganization, cross-reference updates
- **Trust boundary changed** (apply full security review):
- Documentation of new authentication / authorization policies
- Redefinition of permission scopes or privilege boundaries
- Changes to documented secret handling, credential storage, or trust assumptions
- Specifications that other systems will rely on (API contracts, security guarantees)

Rationale: documentation that does not redefine who-can-do-what cannot introduce security vulnerabilities by itself. Treating descriptive docs as security-relevant produces false-positive iterations and erodes review signal.
15 changes: 15 additions & 0 deletions .takt/facets/instructions/review-simplicity.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,21 @@ Review ONLY the lines changed in the diff. Do NOT explore cross-file dependencie
- **YAGNI violations**: Flag speculative abstractions, unused parameters, or over-engineered patterns that serve no current need
- **Naming clarity**: Flag ambiguous variable/function names that obscure intent

## Scope of DRY / YAGNI (do NOT raise findings outside this scope)

The DRY and YAGNI criteria above apply **only to executable code logic**.

- **DRY scope**: Flag duplicated *code logic* (copy-paste functions, repeated control flow, redundant computations). Do NOT flag:
- Documentation hierarchies that intentionally restate context (e.g., a summary table followed by detailed bullet points)
- Repetition between docs and code (docs explain, code executes — they serve different audiences)
- Test code mirroring production code structure (test independence > test DRY)
- **YAGNI scope**: Flag *speculative code abstractions* (unused parameters, premature interfaces, over-engineered patterns in production code). Do NOT flag:
- Planning documents listing "future candidates", "Phase 2 検討", or "out of scope but worth considering" sections — these capture design intent for shared understanding, not speculative implementation
- ADR alternatives sections describing rejected options — these document the decision rationale
- Comments documenting *known constraints or limitations* of the current implementation (these are not speculation; they are recorded reality)

If a finding cannot be tied to executable code logic, it is out of scope — do not raise it.

## Judgment procedure

1. Read the diff from `.takt/review-diff.txt`
Expand Down
38 changes: 38 additions & 0 deletions docs/adr/adr-030-deterministic-post-merge-feedback.md
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,44 @@ GitHub 上に観測可能な成果物 (PR / tag / commit description) は一切

`pnpm merge-pr` 自体は ADR-028 の対象 (PR マージは外部可視) だが、これは既存ゲートで管理済み。本 ADR で追加するのは merge **後** の post_steps のみ。

### post-pr-review fix loop の対象外パス

post-pr-review workflow の `analyze` step が CodeRabbit findings を分類する際、以下のパスに該当する finding は `user_decision_path` (severity に関わらず `user_decision` verdict 経路) に分類し、自動 fix loop に流さない。

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
#### 対象外条件

| カテゴリ | パスパターン | 理由 |
|---------|-------------|------|
| Claude Code sensitive-file protection | `.claude/**` | Edit/Write tool が refuse する。fix loop が回ると `fix.1` / `fix_supervisor.1-3` の 4 step が空費される pathological loop に陥る |
| VCS 内部 | `.git/**`, `.jj/**` | バージョン管理系の内部ファイルは手作業対象外 |
| 依存物 | `node_modules/**`, `target/**` | ビルド成果物 / 外部依存。リポジトリ内のソース変更で対応すべき |

#### 採用根拠

PR #91 で実証された pathological loop:

- CodeRabbit が `.claude/` 配下のファイルへの finding を生成
- `analyze` step が `needs_fix` 判定 → `fix` step 起動
- `fix` step の Edit tool 呼び出しが Claude Code の sensitive-file protection でブロック
- supervisor が「fix 失敗」と判定して再 fix トリガー、最大 4 iteration まで全失敗
- 結果: 8 step (analyze + fix×4 + supervisor×3) が空費され、rate-limit を浪費し review feedback が遅延

実装は `.takt/facets/instructions/analyze-coderabbit.md` の "Sensitive-file protection" / "Scope mismatch" filter として配置 (本 ADR は仕様のみ規定)。

#### Verdict ルールの整合

| Severity | 通常 path | `.claude/` etc. (対象外 path) |
|----------|----------|-------------------------------|
| Critical/High/Major | `needs_fix` (auto-fix) | `user_decision` (報告のみ、user に委ねる) |
| Medium 以下 | `user_decision` | `user_decision` (同左) |
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

`user_decision` 経路に流すことで、findings 自体は report に含まれユーザーが判断できる一方、fix loop は走らないため pathological loop が発生しない。findings の握りつぶしではなく **責任の所在を auto-fix から user に移す** 設計である。

#### 関連 ADR

- ADR-022 (責務分離) — Edit-blocked path を auto-fix 対象から除外することで、自動化と手動操作の境界が明確化
- ADR-018 (post-pr-monitor 移行) — post-pr-review workflow が `.takt/facets/` 配下の facet で挙動制御される設計の前提
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

## 実装タスク

詳細な実装手順は [`docs/todo.md`](../todo.md) の「マージ後フィードバック機構の決定論化」セクション Phase B-F を参照。本 ADR は仕様のみを規定する。
Expand Down
13 changes: 7 additions & 6 deletions docs/todo.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,15 @@
| 6 | 🚀 Tier 1 | ADR-032 PR-pre: GitHub Branch Protection 整備 | todo2.md | 設定のみ | なし (依存タスクは完了済) |
| 7 | 🚀 Tier 1 | **PowerShell custom-lint-rule の `(?i)` フラグ自動検証 (PR #91 T1-1)** | todo3.md | S | なし (PR #91 直接対策、code-review.md 追記も同 PR で land) |
| 8 | 🔧 Tier 2 | 週次レビュー (ADR-031) Phase B 実装 | todo.md | 中-高 | なし (順位 20 の compensating check 前提) |
| 9 | 🔧 Tier 2 | reviewer facet 改善 (review-simplicity / review-security の DRY/YAGNI/security 軸明文化) | todo2.md | S | なし |
| 10 | 🔧 Tier 2 | ADR-032 PR-broken-link: broken-link-check + 内部アンカー検査 統合 | todo2.md | Small-中 | なし (clean baseline 確立済) |
| 11 | 🔧 Tier 2 | `cli-pr-monitor` プロセス正常終了の integration test (PR #85 T2-2) | todo2.md | S | なし |
| 12 | 🔧 Tier 2 | **`cli-pr-monitor` ポーリング延長 + 重複起動ロック (PR #88 T2-4)** ★ rate-limit critical | todo3.md | Medium | なし (Polling anti-pattern 検出 (PR #86 T1-1, 完了済) と補完) |
| 13 | 🔧 Tier 2 | **post-pr-review に rate-limit 自動検出 + 再トリガー (PR #89 T2-1)** ★ rate-limit critical | todo3.md | Medium | なし (順位 12 と補完) |
| 14 | 🔧 Tier 2 | **post-pr-review fix loop の `.claude/` filter + ADR-030 制約明記 (PR #91 T2-1 + T3-2 Bundle)** ★ convergence | todo3.md | S + XS | なし (PR #91 直接対策、analyze facet + ADR 追記の同 PR Bundle) |
| 15 | 🔧 Tier 2 | **cli-pr-monitor 通知 Recovery 経路 (SessionStart hook 拡張)** ★ silent loss prevention | todo3.md | S/M | なし (ADR-030 L2 recovery パターンを cli-pr-monitor に適用) |
| 16 | 🔧 Tier 2 | **`vitest` を devDependencies に固定 (PR #88 T2-3)** | todo3.md | Small | なし |
| 17 | 🔧 Tier 2 | **`pnpm create-pr` 必須引数ヘルプ改善 (PR #88 T2-5)** | todo3.md | Small | なし |
| 18 | 🔧 Tier 2 | **`.failed` marker への recovery 手順自己文書化 (PR #90 T2-2)** | todo3.md | S | なし |
| 19 | 🔧 Tier 2 | **takt ハーネスの `REJECT-ESCALATE` terminal verdict 実装 (PR #91 T2-2)** | todo3.md | M | 順位 14 (path-based 解決) land 後推奨 |
| 19 | 🔧 Tier 2 | **takt ハーネスの `REJECT-ESCALATE` terminal verdict 実装 (PR #91 T2-2)** | todo3.md | M | post-pr-review fix loop の `.claude/` filter (Bundle T、完了済) land 後推奨 |
| 20 | 💎 Tier 3 | ADR-032 PR-β: 実装 (enabled=false default) | todo2.md | 中-高 | 6, 8, 10 |
| 21 | 💎 Tier 3 | ADR-032 PR-γ: enablement (1 行 flip) | todo2.md | XS | 順位 8 dogfood + 順位 20 |
| 22 | 💎 Tier 3 | ADR-032 PR-δ: dogfood + メトリクス検証 | todo2.md | (運用) | 順位 21 |
Expand All @@ -45,18 +43,21 @@
| 26 | 💎 Tier 3 | **post-pr-monitor polling 禁止のグローバル明文化 (PR #86 T3-2)** | todo2.md | XS | なし |
| 27 | 🧹 Tier 4 | ADR-030 Phase E/F: 旧機構廃止 + dogfood | todo.md | 中 | なし (cleanup) |
| 28 | ⏳ Tier 5 | (追って) ADR-030 の takt-test-vc 反映 | todo.md | 中 | 順位 27 Phase F |
| 29 | 🚀 Tier 1 | **非 docs ファイル `docs/todo` 参照検出 lint rule (PR #94 T1-1) ★ Bundle U** | todo3.md | S | なし (PR #94 直接対策、Cross-File Reference Lifecycle の決定論的防止層) |
| 30 | 💎 Tier 3 | **Cross-File Reference Lifecycle ルールに具体例追記 (PR #94 T3-2) ★ Bundle U** | todo3.md | XS | なし (Bundle U で 29 と並行 land 推奨) |

**戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。

**Bundle 1 完了 + post-merge-feedback 反映 (2026-04-29)**: PR #91 (Bundle 1: PowerShell + Markdown anchor lint rules) merge 後の post-merge-feedback で **4 件の新規 task を追加** (PowerShell `(?i)` 自動検証 / `.claude/` filter + ADR-030 制約 / cli-pr-monitor 通知 Recovery 経路 / takt REJECT-ESCALATE)。**前 2 件は本 PR で実証された「fix iteration の根因」に対する決定論的防止策で最優先候補**。**日付ベース見出しアンカーのグローバル明文化 task は決定論的防止 (no-mutable-anchor rule) との二重防衛として継続有効**。

**reviewer facet 改善 task は全 PR の review 精度を即時向上させ、Tier 2 内で 週次レビュー Phase B / ADR-032 PR-broken-link / cli-pr-monitor exit test と並列実施可能**
**reviewer facet 改善 (Bundle T で land 済)** + **post-pr-review fix loop の `.claude/` filter (Bundle T で land 済)** が完了し、reviewer 精度向上 + convergence cost 削減の二段構えが成立。残る Tier 2 では rate-limit critical 系 (cli-pr-monitor ポーリング延長 / post-pr-review rate-limit 自動検出) を最優先候補とする
**rate-limit 系の 2 タスク (cli-pr-monitor ポーリング延長 + 重複起動ロック / post-pr-review rate-limit 自動検出 + 再トリガー) は rate-limit 直撃のため Tier 2 内で最優先候補**。前者 = ポーリング頻度全体の削減、後者 = review 単位での自動再トリガー、Polling anti-pattern 検出 (PR #86 T1-1、完了済) を含む 3 層で rate-limit を抑制する設計。
**post-pr-review fix loop の `.claude/` filter + Recovery 経路 (SessionStart hook 拡張) は本 PR #91 の直接観測知見**。前者 = path-based filter で 8 step 空費の pathological loop を防止 / 後者 = SessionStart hook で再起動跨ぎの通知ロスト防止
**cli-pr-monitor 通知 Recovery 経路 (SessionStart hook 拡張) PR #91 の直接観測知見**。SessionStart hook で再起動跨ぎの通知ロスト防止。post-pr-review fix loop の `.claude/` filter (path-based 解決) は Bundle T で land 済
**Stop hook の `pnpm lint:md` 統合 task は Markdown linter hook 統合 (PR #88 で merged) の gap closure**。**AI 生成一時スクリプト pattern 検出は push 前 untracked `__*` hook (PR #85 T1-4) と関連** (実装前に擦り合わせ要)。
**`.failed` marker 自己文書化 task は ADR-030 soft-fail 機構の運用負荷削減** (PR #89 セッションで recovery が機能した実証から派生、Effort S)。
**takt REJECT-ESCALATE は post-pr-review fix loop の `.claude/` filter task の verdict-based 一般解**。path-based 解決の land 後に着手することで、補完関係になる
**takt REJECT-ESCALATE は post-pr-review fix loop の `.claude/` filter (Bundle T で land 済) の verdict-based 一般解**。path-based 解決が完了したので、本 task 着手で補完関係を完成させる
**T3 グローバルルール 4 件 (日付ベース見出しアンカー / jj conflict リカバリ / `__` prefix scratch / post-pr-monitor polling 禁止) は `~/.claude/` 配下への XS 追記なので並列実施推奨**。
**Bundle U (Cross-File Reference Lifecycle 強化) は PR #94 post-merge-feedback 直接対策**。非 docs ファイル `docs/todo` 参照検出 lint rule (Tier 1 = 決定論的防止) と Cross-File Reference Lifecycle ルール具体例追記 (Tier 3 = preventive guidance) を 1 PR で land 推奨。両者は同一テーマ (永続成果物→ephemeral 参照禁止の二層化) で補完関係、effort 合計 S+XS。

---

Expand Down
Loading