Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .takt/facets/instructions/fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ If you catch yourself about to run a Bash command that writes into a read-only z
- Use the latest review reports in the Report Directory as primary evidence.
- Past iteration reports are saved as `{filename}.{timestamp}` in the same directory. For each report, run Glob with a `{report-name}.*` pattern, read up to 2 files in descending timestamp order, and understand persists / reopened trends before starting fixes.

### Optional: refutation-report.md filter (pre-push-review-refute only)
### Optional: refutation-report.md filter (applies whenever refutation-report.md is present)

If the Report Directory contains `refutation-report.md`, a verify (refute) step has already adversarially filtered the reviewer findings. In that case:

Expand Down
2 changes: 1 addition & 1 deletion .takt/facets/instructions/supervise.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ You are the supervisor. The review-fix cycle has either completed or been escala
3. Check that fixes did not introduce new issues
4. Verify read-only zone compliance (no writes to .takt/, docs/adr/, templates/, .claude/hooks-config.toml)

## Optional: refutation-report.md (pre-push-review-refute only)
## Optional: refutation-report.md (applies whenever refutation-report.md is present)

If the Report Directory contains `refutation-report.md` with Verdict `ALL_REFUTED` and there is no fix report, the verify (refute) step rejected every reviewer finding as a false positive, so there is intentionally no code change. Confirm the rejection reasons are sound (each should cite concrete code-level evidence). If they are, this is **ready to push** -- any wrongly-rejected finding is recaught by the post-pr CodeRabbit layer. If a rejection reason looks unsound, treat it as **issues detected**.

Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
- [ADR-047: pre-push review の反証(refute)facet](docs/adr/adr-047-prepush-refute-facet.md) *(試験運用)*
- [ADR-048: reviewers→fix findings handoff の output-contract 標準化(markdown 統一・JSON 却下)](docs/adr/adr-048-facet-findings-handoff-markdown-contract.md) *(試験運用)*
- [ADR-049: incident→eval 回帰スイート(カスタムルールの由来 incident 再現テスト)](docs/adr/adr-049-incident-eval-regression-suite.md) *(試験運用)*
- [ADR-050: multi-iteration workflow の decision criteria scope 明示](docs/adr/adr-050-iteration-aware-decision-criteria.md) *(試験運用)*

## 開発 convention / チェックリスト

Expand Down
6 changes: 6 additions & 0 deletions docs/adr/adr-043-security-gates-fail-closed.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ fn is_stale(behind: Option<usize>) -> bool {

一般原則として、model/heuristic の評価では **accuracy と安全軸 (安全側デフォルトを破らないこと) を独立指標として測り、両者が trade-off するときは安全軸を優先**する。accuracy 改善が安全後退を隠しうる (WP-04 の qwen3-coder) 点に注意する。gate 関数における「判定不能 → block」と、助言層における「不確実 → 保守側 (human_review)」は同一の設計思想の別レイヤーへの適用である。

### 原則 6: read-only zone への reviewer 提案は silently skip せず escalate (2026-07-07 追記、WP-06/07 feedback)

レビュアー (CodeRabbit 等) の提案が read-only zone (`.takt/facets/**` / `docs/adr/**` / `templates/**` 等、fix step が編集不可の領域) を対象とする場合、fix step はそれを適用できない。このとき **silently skip せず、misdirected として明示 escalate** する (fix report の `### Misdirected finding` 記載 + analyze step での `not_applicable` 明示分類)。silently drop は valid finding の取りこぼし = 「自動対応不能 → 楽観的にスルー」であり、原則 5 の逆で fail-open 的リスクだからである。read-only zone の対応は human (project owner) が直接編集するか、analyze が理由付きで `not_applicable` に分類して監査可能な形で残す。

**観測事例**: PR #250 (supervise.md への CodeRabbit Major) / PR #252 (simplicity-review.md への CodeRabbit Major) の 2 件で、`.takt/facets/**` への提案が fix step で read-only zone として misdirected 分類された。両件とも post-pr-review pipeline が misdirected → not_applicable → approved と正しく escalate/分類し silently drop しなかった。この既存の望ましい挙動を原則として明文化する。

## 反例の判別ヒント

関数が gate 関数か non-gate 関数かは、以下の質問で判別する:
Expand Down
7 changes: 7 additions & 0 deletions docs/adr/adr-048-facet-findings-handoff-markdown-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ takt に同梱の公式スタイルガイド・スキーマ・builtins を精査
4. 契約は公式スタイルガイド準拠 (` ```markdown ` ブロック + 認知負荷軽減ルール + `finding_id` 必須の
Rejection Gate)。

### output-contract 設計原則 (次回 contract 追加・編集時の参照基準、2026-07-07 追記 / WP-07 feedback)

新規 output-contract を追加・編集する際は以下の 2 原則を守る (WP-07 で列不整合が CodeRabbit Major 指摘となった再発防止):

1. **全 finding セクションで同一列セット**: 1 contract 内の Current Iteration / Carry-over / Reopened 等の finding テーブルは同じ finding 列 (`finding_id` / `family_tag` / ...) を共有する。セクション間で列が漂流すると下流の fix / refute facet の parse を壊す。
2. **builtin `security-review` を mirror**: reviewer contract は takt builtin `security-review` の列構造・casing を踏襲する。casing の混在 (snake_case の field id `finding_id`/`family_tag` + Title Case の表示ラベル `Severity`/`Type`/...) は builtin 由来の**意図的な区別**であり「不整合」ではない (PR #252 で CodeRabbit が誤指摘)。意図的に mirror しない contract を作る場合は、その設計意図を contract 冒頭コメントに明記する。

適用範囲は **pre-push review の reviewers** (通常 `pre-push-review.yaml` と refute variant
`pre-push-review-refute.yaml` の両方が同じ `format:` 名を共有するため自動的に裨益)。post-pr-review は
simplicity/security reviewer を持たず (CodeRabbit findings 駆動の analyze/fix 構成) 対象外。
Expand Down
59 changes: 59 additions & 0 deletions docs/adr/adr-050-iteration-aware-decision-criteria.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# ADR-050: multi-iteration workflow の decision criteria scope 明示

## ステータス

試験運用 (2026-07-07)

## コンテキスト

takt の review-fix workflow は `Iteration N/max` の反復構造を持ち、各 step (analyze / fix / supervise 等) が
Report Directory に**複数 iteration 分の report** を蓄積する (最新は `{filename}`、過去は `{filename}.{timestamp}`)。

PR #250 (WP-06) で `supervise.md` の Decision Criteria が「all blocking findings が解決したら push」とだけ書かれ、
その「all blocking findings」が **全 iteration の累積か、現 iteration のみか** が曖昧だった。この scope 曖昧性は
「前 iteration で解決済みの finding を再判定して不要な reject」または「現 iteration の未解決を見落として過剰 approve」
という判定ミスに直結する。CodeRabbit Major 指摘となり、supervise.md に scope 宣言を追記して解消した。

この曖昧性は supervise 固有ではなく、**Report Directory を読む全ての multi-iteration step に共通の systemic pattern**
であり、今後の複数 iteration workflow 実装で再発しうる。

## 決定

multi-iteration workflow の各判定 step の decision criteria は、**評価 scope を明示**する。

### scope の 3 分類

| scope | 意味 | 典型例 |
|---|---|---|
| `current-iteration-only` | 現 iteration の report のみを判定対象とする (既定) | fix / supervise の per-iteration 判定 |
| `cumulative` | 全 iteration の累積状態を判定対象とする | convergence 全体の最終判定 |
| `sliding-window` | 直近 N iteration を対象とする | 膠着 (stall) 検出 |

### 原則

1. **既定は `current-iteration-only`** — 特に明示しない限り、判定は現 iteration の report (`{filename}` suffix なし、
最新) のみを対象とする。過去 iteration の archived report (`{filename}.{timestamp}`) は参照しない
([dev-conventions.md](../dev-conventions.md) の Report Directory アクセスパターンと整合)。
2. **scope 宣言を instruction に明記** — decision criteria セクション冒頭で、どの scope で評価するかを 1-2 行の
negative/positive specification で宣言する (例: supervise.md「本セクションは current iteration の report のみを
判定対象とし、前 iteration の convergence_verdict とは比較しない」)。
3. **cumulative / sliding-window を使う場合は理由を明記** — 既定から外れる scope を採る step は、なぜ全履歴 / 窓が
必要かを instruction に書く (膠着検出は反復の history が本質的に必要、等)。

## 帰結

### 利点

- multi-iteration step の判定 scope が暗黙にならず、scope 曖昧性由来の判定ミスを設計段階で排除。
- 新規 iteration workflow / facet 追加時の設計チェック項目として再利用可能。

### 留意点

- takt は scope を機械強制しない (instruction prompt レベルの規約)。宣言忘れの検出は reviewer / dogfood に依存する。

### 関連 ADR

- [ADR-020](adr-020-takt-facets-sharing.md) — facet 共有 (fix/supervise は複数 workflow で共有、scope 宣言も共有される)
- [ADR-030](adr-030-deterministic-post-merge-feedback.md) — 決定論的 post-merge feedback (iteration 構造の文脈)
- [ADR-037](adr-037-takt-fix-trust-shortcut.md) — fix-trust shortcut (convergence_verdict による iteration 短絡)
- [ADR-047](adr-047-prepush-refute-facet.md) — refute facet (loop_monitor の cycle = sliding-window scope の例)
27 changes: 27 additions & 0 deletions docs/dev-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,30 @@ spike・実験タスクを見送る (採用しない) と判断したときは
5. **public リポ特典の適用範囲** — public リポで無償化される「機能」と、緩和されない「rate limit」を区別する。

**由来** (WP-03、[ADR-019](adr/adr-019-coderabbit-review-hybrid-policy.md) § CodeRabbit クォータ設計): CodeRabbit の「public リポ向け Pro 機能無償提供」を「rate limit 撤廃」と誤解しかけたが、実際には月間上限と時間単位 rate limit は別次元で、時間単位上限 (3〜4 回 / 時) は残存していた (2026-07-04 ユーザー確認)。この誤解は LLM API・CI 等の他 SaaS 統合でも再発しうる汎用パターン。

## facet の Report Directory アクセスパターン (WP-06 feedback)

takt facet が Report Directory から report を読む際は、**現 iteration の report のみを対象**とする:

1. **archived timestamped ファイルを除外** — 過去 iteration の report は `{filename}.{timestamp}` として同ディレクトリに残る。読み取り対象は suffix なしの `{filename}` (最新) に限る。
2. **既存パターンの踏襲** — `fix.md` は「`{report-name}.*` を Glob し descending timestamp 順で最新 2 件のみ読む」パターンを確立済み。新規 facet が Report Directory を読む場合はこれに揃える。

**由来** (PR #250 / WP-06): `supervise.md` が Report Directory のフィルタを持たず全履歴を読みうる曖昧性が CodeRabbit Major 指摘となった。facet が「全履歴」か「current-iteration-only」かを暗黙にせず明示する (scope 曖昧さは判定ミスに直結)。

## 見出し ⇔ 実装スコープの整合 (WP-06/07 feedback)

見出し (section heading / WP heading) は実装の条件スコープと 1:1 対応させ、実装変更時は見出しも追随させる:

1. **takt instruction / output-contract の section 見出し** — `if` ガードや file-existence check の条件を反映する。「X-variant only」ではなく「Applies when \<condition\>」形式で実際の適用条件を表す (例: 「pre-push-review-refute only」→「applies whenever refutation-report.md is present」)。
2. **計画文書の WP 見出し** — `docs/harness-improvement-plan.md` 等の WP status を「実装済」に更新する際は、WP 見出しが実装内容を正確に反映しているか確認する。方針転換した場合は見出しも更新する。

**由来** (PR #252 / WP-07): WP 見出し「JSON 化」が markdown 契約標準化への方針転換後も未更新で CodeRabbit 指摘。同 PR で `fix.md` の section 見出しが本文の適用条件より狭い (「refute only」だが実際は file 存在時) ことも simplicity review で観測。

## 外部 exe を spawn する integration test の bounded wait (WP-08 feedback)

integration test で外部バイナリを spawn する場合、**無期限 wait を避け bounded duration の wait を必須**とする:

1. **timeout 付き wait** — `child.wait_with_output()` / `child.wait()` は子プロセスが hang すると CI を無期限ブロックする。代わりに `lib-subprocess::wait_with_timeout_safe(label, &mut child, 30)` 等の timeout 付き wait を使い、超過時は kill + test 失敗させる。
2. **出力捕捉との両立** — 出力が必要なら stdout/stderr を `lib-subprocess::drain_pipe_unlimited` で別スレッド drain してから timeout wait する (pipe バッファ充填による deadlock 回避)。

**由来** (PR #254 / WP-08、[ADR-049](adr/adr-049-incident-eval-regression-suite.md)): codebase 初の exe-spawn E2E テスト (`incident_eval.rs`) パターンを確立したが timeout 境界が欠落し CodeRabbit nitpick。WP-16 CI smoke test 等で同パターン流用が見込まれるため convention 化する。
3 changes: 3 additions & 0 deletions src/hooks-post-tool-linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,8 @@ tempfile = "3"
# stdout HookOutput/LintViolation JSON. serde_json is also a [dependencies] entry but
# integration tests only see [dev-dependencies], so it is re-declared here.
serde_json = "1.0"
# lib-subprocess: bounded-wait + pipe drain for the exe-spawn E2E test (dev-conventions.md
# § bounded wait). Re-declared here because integration tests only see [dev-dependencies].
lib-subprocess = { path = "../lib-subprocess" }

# [profile.release] は workspace root (Cargo.toml) に集約 (ADR-026)
45 changes: 43 additions & 2 deletions src/hooks-post-tool-linter/tests/incident_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,15 @@
//! data reproducing the real incident each rule was created for (see the fixture
//! headers and `[rules.incident]` in `.claude/custom-lint-rules.toml`).

use lib_subprocess::{drain_pipe_unlimited, wait_with_timeout_safe};
use std::io::Write;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};

/// Bounded wait for the spawned linter exe (dev-conventions.md § bounded wait): a hung
/// child is killed and the test fails rather than blocking CI indefinitely.
const LINTER_TIMEOUT_SECS: u64 = 30;

/// One incident-derived rule's E2E expectation.
struct Case {
/// Expected `LintViolation.type` (rule id upper-cased, hyphens -> underscores).
Expand Down Expand Up @@ -119,14 +124,22 @@ fn run_linter(cwd: &Path, invoke_path: &str) -> Vec<serde_json::Value> {
.stderr(Stdio::piped())
.spawn()
.expect("spawn hooks-post-tool-linter");
let stdout_drain = drain_pipe_unlimited(child.stdout.take().expect("child stdout"));
let stderr_drain = drain_pipe_unlimited(child.stderr.take().expect("child stderr"));
child
.stdin
.take()
.expect("child stdin")
.write_all(payload.as_bytes())
.expect("write stdin payload");
let out = child.wait_with_output().expect("wait for linter exe");
let stdout = String::from_utf8_lossy(&out.stdout);
let status = wait_with_timeout_safe("hooks-post-tool-linter", &mut child, LINTER_TIMEOUT_SECS)
.expect("wait_with_timeout_safe errored");
let stdout = stdout_drain.join().expect("stdout drain thread panicked");
let _stderr = stderr_drain.join().expect("stderr drain thread panicked");
assert!(
status.is_some(),
"hooks-post-tool-linter hung > {LINTER_TIMEOUT_SECS}s on {invoke_path} (killed) — investigate",
);
parse_custom_lint_violations(&stdout)
}

Expand Down Expand Up @@ -222,3 +235,31 @@ fn incident_eval_all_incident_rules() {
assert_good_fixture_clean(case);
}
}

/// fail-closed: `CASES` must have one entry per incident-derived rule. This closes the
/// asymmetry where `incident_fixture_coverage_check` gates the *fixture* dimension
/// dynamically but the E2E `CASES` array was manually synced — a new `[rules.incident]`
/// rule added without a CASES entry would silently skip E2E coverage (ADR-043 / ADR-049).
#[test]
fn cases_cover_every_incident_rule() {
let toml = std::fs::read_to_string(
repo_root().join(".claude").join("custom-lint-rules.toml"),
)
.expect("read deployed custom-lint-rules.toml");
let incident_rule_count = toml
.lines()
.filter(|l| l.trim() == "[rules.incident]")
.count();
assert!(
incident_rule_count > 0,
"false-green guard: no `[rules.incident]` sections found in deployed toml"
);
assert_eq!(
CASES.len(),
incident_rule_count,
"CASES ({}) must have one entry per `[rules.incident]` rule ({}) — a new incident \
rule was added without an E2E case (fail-closed)",
CASES.len(),
incident_rule_count
);
}