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
124 changes: 117 additions & 7 deletions docs/push-pipeline-fix-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ takt / push) を使う。書式の定義元は `src/cli-push-runner/src/log.rs`
| T5 | push 拒否検知の truncate 依存修正 | 不具合 | silent-failure push の防止 | S | なし |
| T6 | diff stage の timeout 追加 | 不具合 | 無限ハング防止 | XS | なし |
| T7 | Stop hook file-length step の cwd 依存修正 | 不具合 | quality gate 誤失敗の防止 | S | なし |
| T8 | bookmark_check の空 `@` 誤誘導修正 | 不具合 | exit 7 誤案内の防止 | S | なし |
| T8 | bookmark_check の空 `@` 誤誘導修正 | 不具合 | exit 7 誤案内の防止 (**再現確認済**) | S | なし |
| T3 | `pnpm build` 形骸ゲートの実体化 or 削除 | 不具合 | 見せかけゲートの解消 | XS | なし |
| T2 | 旧 cli-push-pipeline の workspace 除去 | 改善 | clippy/test 対象の純減 | S (大量削除) | なし |
| T10 | takt builtin review policy の shadow | 改善 | **-1.5〜3 分/iter + 無駄 fix 削減** | M | T0 |
Expand Down Expand Up @@ -143,17 +143,123 @@ T1 を最優先とする理由: 以降の全 PR の dogfood push が速くなり
(`main.rs:306` の `current_dir()` 使用箇所) に影響しないか確認が必要。
正規化は「ステップ実行の子プロセス」にのみ適用し、判定ロジックは元 cwd を使う形が安全。

### T8: 空 `@` 時の bookmark_check 誤誘導 (要再現確認)
### T8: 空 `@` 時の bookmark_check 誤誘導 (**再現確認済み** 2026-07-16)

- **現状** (コード監査の指摘、実装前に再現テストで確認すること):
- **現状** (当初はコード監査による推測。2026-07-16 に実機で再現し確定 — 下記「再現記録」):
`advance_jj_bookmarks` は `@` が空なら bookmark を `@-` へ前進させる
(`stages/push_jj_bookmark.rs:82-95` 付近) のに、`stages/bookmark_check.rs` (L44, L117-146 付近) は
`jj bookmark list -r @` の厳密一致で検査するため、`jj new` 直後の正常な再 push 状態でも
exit 7「bookmark を作成して再実行してください」で中断し、従うと bookmark を壊す方向に誘導する。
- **方針**: まず再現テストを書く (`@` 空 + bookmark が `@-` にある状態)。再現したら、
検査を `@` 空時は `@-` を対象にする (advance と同じ規則) よう揃え、メッセージを
「push すべき新変更がない」旨に修正。再現しなければ本タスクは却下として §8 の判定記録に残す。
- **再現記録 (2026-07-16、T1 = PR #279 の dogfood push で実際に発火)**:
「要再現確認」だった本タスクは **in the wild で再現した**。よって却下条件
(「再現しなければ却下」) は解消し、実施対象として確定。

再現状態 (T1 セッションで自然発生したもの。作為的に作った状態ではない):

```text
@ zxxkpomz (empty) "WIP: next work" ← 空の working copy
@- nvmysvqk perf/lint-screen-evals-opt-in ← bookmark はここ
```

`pnpm push` の実際の出力 (抜粋):

```text
[push-runner] [push] bookmark 'perf/lint-screen-evals-opt-in' を @- に自動更新
[push-runner] [bookmark] ローカル bookmark (非 trunk) が見つかりません
[push-runner] push 不可: `jj git push` は bookmark が必要です。
対処: `jj bookmark create <name> -r @` で bookmark を作成して再実行してください
[push-runner] パイプライン中断: 非 trunk bookmark が見つかりません。
[push-runner] stage=pre_checks elapsed=0.5s
```

観測できた事実:
1. **同一 run 内で 2 つの stage が矛盾している**: `advance_jj_bookmarks` は
「bookmark を `@-` に自動更新」と報告済み (= `@` が空である前提を正しく扱っている)
のに、直後の `bookmark_check` が「bookmark が見つからない」と報告する。
矛盾する 2 行が連続して出るため、ログだけでも異常と判る。
2. **誤誘導が確定**: 案内される `jj bookmark create <name> -r @` に従うと、
**空の WIP コミットに bookmark が付く**。計画時の推測どおり「bookmark を壊す方向」。
3. **exit code は 7** (計画の推測どおり)。stage は `pre_checks` で中断するため、
quality_gate 以降は一切走らない。
4. **回避策** (T1 セッションで実際に採った手段): 誤誘導に従わず、
`jj edit @-` で `@` を bookmark のあるコミットへ移動 + 空 WIP コミットを abandon。
これで `bookmark_check` を通過し push 成功。
5. **前段の別症状**: bookmark が 1 つも無い状態でも同じ exit 7 + 同じ文面が出る
(「ローカル bookmark が見つかりません (新規ブランチ等)」)。こちらは案内が正しい
(実際に作成が必要) ため、**修正時に 2 ケースを取り違えないこと** —
「bookmark が皆無」と「bookmark が `@-` にあるが `@` が空」を区別してメッセージを
出し分ける必要がある。現状は両者が同じ文面に潰れており、これが誤誘導の実体。
- **方針** (再現済みのため却下判定は不要。当初の「再現しなければ却下」条件は解消):
1. 再現テストを書く (`@` 空 + bookmark が `@-` にある状態)。上記「再現状態」がそのまま
fixture の仕様になる。incident 由来なので ADR-049 の流儀に従う。
2. 検査を `@` 空時は `@-` を対象にする (advance と同じ規則) よう揃える。
3. メッセージを分岐させる: 「bookmark が `@-` にあるが `@` が空」= 正常な再 push 状態なので
「push すべき新変更がない」旨に修正。「bookmark が皆無」= 既存の案内が正しいので維持
(上記 5.)。**現状は両者が同じ文面に潰れており、これが誤誘導の実体**。
- **リスク**: 中。jj 変更検出は ADR-021 の設計原則に従うこと (revset 合成の流儀)。
- **実施結果 (2026-07-17, 実装済み / PR #280)**:
- **方針 2・3 の矛盾を実施前に解消**: 着手時に方針 2 (「検査を `@` 空時は `@-` を対象にする」) と
方針 3 (「`@` 空 + bookmark が `@-` は正常な再 push 状態なので "push すべき新変更がない" 旨に修正」)、
および再現記録の事実 4 (T1 セッションは `jj edit @-` 後に **push 成功** = push すべき変更はあった)
の 3 者が矛盾していることが判明した。さらに方針 2 を文字通り実装すると
**AI レビューを無言でバイパスする**ことが判明 — `[diff] command = "jj diff -r @"`
(`push-runner-config.toml`) のため `@` が空のまま続行すると diff が空になり、
`main.rs` の「diff が空のためレビューをスキップして push に進みます」経路で
takt が skip されたまま `@-` の変更が push される。誤誘導バグを
レビューバイパスに置き換えることになるため、方針 2 の文字通りの実装は採らない。
- **採用した方針 (ユーザー承認済み)**: **exit 7 による中断は維持し、案内文のみ正す**。
`@` に bookmark が無い状態を 2 ケースに切り分け、T8 の状態には
T1 セッションで実証済みの回避策 (`jj edit @-` + 空 WIP の abandon) を案内する。
レビュー範囲は無傷、変更は bookmark_check に閉じる。
- **実装**:
- `push_jj_bookmark.rs` の `determine_target_revision()` から `working_copy_is_empty()` を
切り出して `bookmark_check` と共有した。「`@` が空なら `@-`」の規則を
advance と検査で**二重定義しない**ことが再発防止の核心 (矛盾の実体がこれだった)。
- `bookmark_check.rs` に判定 enum `BookmarkCheckOutcome`
(`Proceed` / `EmptyWorkingCopy` / `NoBookmarks`) と pure function
`decide_bookmark_check(bookmarks_at_head, head_is_empty, parent_bookmarks_fn)` を追加。
jj 呼び出しは closure 注入で外に出した (**ADR-021 原則 3** に準拠。既存の
`dispatch_bookmark_advance` と同じ流儀で、本 repo には実 jj repo を張る
test 前例が無いためこの形を踏襲)。
- 判定順は **`@` の空判定を最優先**する。当初は「`@` に bookmark があれば従来どおり続行」を
先に置き、bookmark が空の `@` にある既存経路を温存していたが、**PR #280 の CodeRabbit
Major 指摘で反転**した: その経路は `jj diff -r @` が空になり、祖先の未 push 変更が
AI レビューを経ずに push される (本タスクが方針 2 を却下した理由と同じ穴が、
bookmark の位置違いで残っていた)。`advance_jj_bookmarks` は非 trunk bookmark が
2 つ以上あると fallback 更新を skip するため、この状態は実在する。
「レビュー範囲 = `@` だから `@` は非空でなければならない」という本タスクの不変条件に
判定順を揃えた (§8 判定記録の post-PR 修正欄)。
- `main.rs:76-79` にも同じ `jj bookmark create -r @` 案内が**重複**しており、
これも誤誘導の出力元だったため撤去し、ケース別案内を出す bookmark_check に一本化した。
- **ADR-021 原則 5 との関係**: 原則 5 の標準 (`@`/`@-`/`@--` の優先度付き revset) に対し、
bookmark_check は PR #271 (CodeRabbit Major: 他 workspace の bookmark 混入) の対策として
意図的に `@` 厳密一致へ狭めた経緯がある (`OWN_WORKSPACE_BOOKMARKS_REVSET` の doc)。
本修正の `@-` 照会は**案内文の出し分け (診断) 専用**で、push 対象
(`-b <name>` の組み立て) は `@` のままとし、PR #271 の対策を維持する。
- **回帰テスト (ADR-049 の流儀)**: `mod t8_empty_head_misdirection` に 12 本追加
(cli-push-runner 全体 186 → 198 passed。初版 7 本 + post-PR 修正で 5 本)。
由来 incident (PR #279 の dogfood push) と
再現状態を module doc に明記し、追跡鎖 incident → 修正 → test を残した。
bad = 「`@` 空 + bookmark が `@-`」が `NoBookmarks` に潰れないこと、
good = 「bookmark 皆無かつ `@` が非空」が `NoBookmarks` のままであること
(**記録 5. の取り違え防止をテストで固定**)、および `@` に bookmark がある場合に
`@-` を照会しないこと (panic で固定)。
- **サンドボックス実機検証 (before/after)**: 記録と同型の状態
(`@` = 空 WIP / `@-` = `perf/lint-screen-evals-opt-in`) を張った jj repo で
配布 exe と修正後 exe を実行して比較した。

| | before (現行配布 exe) | after (修正後) |
|---|---|---|
| bookmark 所在の報告 | 「ローカル bookmark (非 trunk) が見つかりません」(矛盾) | 「`@` が空です (bookmark は @- にあります: perf/...)」 |
| 案内 | `jj bookmark create <name> -r @` (**空コミットに bookmark = 破壊的**) | `jj edit @-` + 空 WIP の `jj abandon` |
| exit code | 7 | 7 (維持) |

before が記録の出力を**逐語で再現**することを確認した上で修正を当てている。
あわせて (a) 案内どおり `jj edit @-` + abandon した後の再実行で bookmark_check を
通過し scratch/quality_gate へ進むこと、(b)「`@` 非空 + bookmark 皆無」では
従来の作成案内が**そのまま出る**ことも実機で確認した。
Windows 注意: jj の index segment 名が長く `MAX_PATH` に掛かるため、
サンドボックスは深い scratchpad 配下ではなく短いパスに作る必要がある。

### T3: `pnpm build` の形骸化

Expand Down Expand Up @@ -207,7 +313,10 @@ T1 を最優先とする理由: 以降の全 PR の dogfood push が速くなり
- **現状**: `push-runner-config.toml` の rust-lint-test group 3 本目
`cargo test -- --ignored --test-threads=1` が、
`src/cli-finding-classifier/tests/lint_screen_evals.rs` の
`run_lint_screen_against_all_fixtures` (L746-769) を巻き込む。このテストは
`run_lint_screen_against_all_fixtures` (L746-769) を巻き込む
(**パスと行番号は T1 着手前の記述。T1 で当該ファイルは
`tests/lint_screen_evals/{main.rs,e2e.rs}` に分割済みで、この関数は現在
`e2e.rs` にある** — 下記「実施結果」参照)。このテストは
`#[ignore]` 付き・**assert ゼロ** (`report_summary` は println のみ)・mistral:7b を
15 fixture 分実呼出する計測専用テストで、doc コメント自体が名前フィルタ付き
手動起動を想定している。269s の実測記録あり (GPU 更新後は短縮の可能性)。
Expand Down Expand Up @@ -450,3 +559,4 @@ T1 を最優先とする理由: 以降の全 PR の dogfood push が速くなり
|--------|------|------|--------------------------|
| T0 | 実装・マージ済 (PR #278) | 2026-07-16 | stage 別ログ `stage=<name> elapsed=<秒>s` を追加 (§5 T0 実施結果)。before 値は §1 表を使用。初回実測で T1 の前提に疑義 → §5 T1 の申し送り参照 |
| T1 | 実装済 (PR #279) | 2026-07-16 | `LINT_SCREEN_EVALS` env opt-in で eval を gate から除外。`--ignored` 63s → 21s。step_timeout 600 → 300 (実測 right-size)。**判断根拠**: 着手前実測で (b) 41.3s が (a) 63s の 65% を占め、申し送りの判定基準「前提は生きている」に該当したため実施。ただし絶対値が 269s の約 1/4 だったため期待効果を -2〜4.5 分 → -42s に下方修正し、§1 結論の (1)「主犯」認定も修正した。実行の本丸は (2)(3) = T10/T12 |
| T8 | 実装済 (PR #280) | 2026-07-17 | 「`@` 空 + bookmark が `@-`」を `NoBookmarks` から切り分け、`jj edit @-` を案内するよう修正。`working_copy_is_empty()` を advance と共有して規則の二重定義を解消。`main.rs` 側の重複案内も撤去。回帰テスト 12 本 + サンドボックス実機で before/after 比較 (§4 T8 実施結果)。**post-PR 修正 (CodeRabbit Major 2 件 + simplicity 警告 2 件を採用)**: (a) 判定順を反転し、bookmark が空の `@` にある場合も中断する — 続行すると `jj diff -r @` が空になり祖先の未 push 変更が AI レビューを経ずに push される (方針 2 を却下した理由と同じ穴が bookmark の位置違いで残っていた)。(b) `@-` 照会の失敗を `unwrap_or_default()` で「親はあるが bookmark 無し」に潰していたのを `ParentState::Unavailable` として保持し、親を確認できない場合は実行不能な `jj edit @-` を案内しない (T8 が直したはずの誤誘導の再生産だった)。あわせて simplicity-review の非ブロッキング警告 2 件も採用: (c) `query_parent_state()` の jj 失敗を log する (他の jj 失敗処理の慣習と揃える)。(d) `@-` に bookmark が無い場合は `jj edit @-` だけでは次に `NoBookmarks` で止まるため、bookmark 作成まで含めて案内する。Minor 1 件のうち日付指摘は CodeRabbit が UTC 基準のため不採用 (本 repo の記録は JST 基準で 2026-07-17 が正)。**dogfood 実証**: 本修正の push 作業中に、私自身が `jj new` で空コミットを作ってしまい T8 の incident 状態を再現したが、修正後の bookmark_check が破壊的な `jj bookmark create -r @` ではなく正しい `jj edit @-` を案内し、案内どおりの操作で復旧できた (修正が in the wild で機能することの実証)。**方針変更**: 方針 2 (`@-` を検査対象にして続行) は `[diff] command = "jj diff -r @"` により **takt レビューを無言 skip して push する**ことが判明したため不採用。方針 3 の「push すべき新変更がない」も再現記録の事実 4 (実際に push 成功 = 変更はあった) と矛盾するため不採用。exit 7 は維持し**案内文のみ正す**方針をユーザー承認のうえ採用した。**実施順**: T4-T7 を飛ばして T1 の次に実施 (T1 の dogfood push で再現が取れたタイミングを優先。T8 は他タスクと独立のため順序入替は無害) |
5 changes: 1 addition & 4 deletions src/cli-push-runner/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,7 @@ fn run_diff_and_lint_screen(config: &config::Config) -> Result<bool, i32> {
/// 失敗時は exit code を Err で返して pipeline を中断する。
fn run_pre_checks(config: &config::Config) -> Result<Vec<String>, i32> {
let Some(detected_bookmarks) = run_bookmark_check() else {
log_info(
"パイプライン中断: 非 trunk bookmark が見つかりません。\
`jj bookmark create <name> -r @` で bookmark を作成して再実行してください。",
);
log_info("パイプライン中断: push 可能な bookmark がありません (対処は直前のログを参照)。");
return Err(EXIT_BOOKMARK_MISSING);
};
if !run_scratch_file_warning(config.scratch_file_warning.as_ref()) {
Expand Down
Loading