feat: screenshots モードで撮る story を絞り、撮らなかった story は baseline から引き継ぐ

[sousuke0422]

何をしたか

screenshots モードで「撮る story を絞る」を安全に成立させた。

当初は CLI に選択計画を出す vrt plan を足すだけの変更だった。
だがレビューを重ねるうちに、部分撮影が既存の比較契約と噛み合っていないことが分かり、
サーバー側の比較処理と成果物の検証にまで手を入れることになった。
本 PR は CLI から backend まで一続きの変更である。

なぜ CLI だけで終わらなかったか

screenshots モードはサーバーがレンダリングしない。
撮影は CI 側のテストランナーが行い、サーバーは送られてきた PNG を受け取るだけである。

この前提のもとで一部の story だけを撮って送ると、比較処理は撮らなかった story を
「消えた」と判定していた。
baseline が 100 件で今回撮ったのが 1 件なら、残り 99 件が removed として報告される。
それを承認すれば baseline から消える。

つまり部分撮影を成立させるには、次の三つが揃っている必要があった。

1. 撮らなかった story を baseline から引き継ぐこと
2. 「撮らなかった」と「撮り損ねた」を区別できること
3. 選択の根拠となった成果物が、本当にその commit で生成されたものだと確かめられること

3 が最も深く、vrt plan の入力である preview-stats.json と index.json が
通常 untracked であるため、commit と結び付いていないところまで遡った。

主な変更

撮る story を選ぶ（vrt plan）

依存グラフから影響のある story を割り出し、選択計画を JSON で出力する。
撮影は行わない。
CI ランナーがこの JSON を読み、plan が only のときだけ列挙された story を撮る。

入力が読めない、あるいは意味的に壊れている場合は全撮影へ倒す。
storybook モードの vrt upload --only-changed も同じ選択器を通るので、
選択のロジックは 1 本のままである。

撮らなかった story を引き継ぐ

比較処理に選択集合を渡し、集合に含まれない baseline entry は removed ではなく
引き継ぎとして扱う。
storybook モードが only_story_ids で行っている流用と同じ結果になる。

削除された story は引き継がない

引き継ぎは「撮らなかっただけ」の story に限る。
現在の index から消えた story は removed として扱い、baseline に残り続けないようにした。

選択集合をサーバー側に固定する

計画をビルドへ保存し、finalize と束縛する。
アップロード側の自己申告だけで検証すると、撮影が全滅したときに空集合どうしが一致して
通ってしまうためである。
計画が非空なのにアップロードが空という遷移は拒否する。

計画を添付した時点で baseline を固定する

計画を立てた時点の baseline と、比較を実行する時点の baseline がずれると、
選択の根拠と比較の相手が食い違う。
計画を添付する時に baseline を固定し、比較でもそれを使う。

固定はビルド作成時ではなく計画添付時である。
作成時に固定すると、作成から撮影開始までの間に進んだ baseline を取りこぼすためで、
利用者から見た保証は「撮影が始まる前に固定されている」ことになる。
計画を使わないビルドの意味は変えていない。

成果物を生成コミットに束縛する（vrt stamp）

vrt stamp -- <build command> の形でビルドコマンドを渡す。
vrt が開始前に HEAD と worktree の clean を観測して古い provenance と成果物を削除し、
ビルドを実行し、成功後に同じ HEAD と clean を再確認してから provenance を書く。

これで「そのコミットでビルドが走り、成果物を作り直した」ことを vrt 自身の計器で確認できる。
build を所有しない旧形式（version 1）の provenance は全撮影へ倒す。
provenance が無い、あるいは合わない成果物での絞り込みは拒否する。

競合と再試行

計画の添付、アップロードの登録、finalize を同じロック規約で直列化した。
引き継ぎのコピーは決定的なキーと upsert で冪等にし、一時的な失敗は再試行へ戻す。
途中で止まった場合の孤児オブジェクトは補償削除で回収する。

出力契約

{
  "version": 1,
  "plan": "only",
  "branch": "feat/x",
  "baseline_commit_sha": "<sha>",
  "head_commit_sha": "<sha>",
  "story_ids": ["button--primary"],
  "reason": null,
  "build_id": "<uuid>"
}

plan は only と capture_all の二値で、only かつ story_ids が空という状態も
正当な結果として存在する。
「撮る story が無い」と「判定できなかった」を取り違えないよう、CI レシピでは
空ファイルではなくセンチネルで区別している。

コミット SHA は CLI 側で 40 桁の OID へ正規化してから送る。
サーバーは長さの検証のみで、baseline との照合は完全一致である。
CLI を通さず API を直接叩いて短縮形を送ると、同じコミットでも一致しない。

何を保証し、何を保証しないか

保証するのは、計画の根拠となった成果物がその commit で生成されたことと、
撮らなかった story が黙って消えないことである。

保証しないものは README に列挙した。
主なものは、ビルドコマンドが古い内容を書き戻す場合、storybook のキャッシュが
古い stats を出す場合、provenance が無署名であることの三つである。
脅威モデルは事故であって、悪意ある CI ランナーへの防御ではない。

レビューの読み方

コミット数が多いので、次の順に読むと筋が追いやすい。

1. crates/cli/src/plan.rs と turbosnap.rs — 何を撮るかの判断
2. crates/cli/src/provenance.rs と main.rs の stamp — 成果物をコミットに束縛する仕組み
3. crates/job/src/compare_build.rs — 引き継ぎと削除の判定
4. crates/service/src/builds.rs と review_lock.rs — 計画の固定と直列化
5. README.md — CI レシピと保証の境界

テスト

回帰テストは、修正前のコードで実際に落ちることを確認したうえで入れている。
git を使うテストは一時ディレクトリに合成コミットを作って自給自足にしてあり、
CI の shallow checkout でも本体の検証まで到達する。

競合の再現は、実トランザクションで行ロックを保持したまま後続の要求を投げ、
ロック待ちに入っていることを確かめる形にしている。

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[yupix]

レビュー結果: APPROVE with Comments

実装は設計どおりで、select_stories 統一後も storybook モードの既存挙動(stats 不在 → 全撮影 / 破損 → exit 2)が維持されていることをコードとテストの両方で確認しました。base の main への retarget 済み・マージ済み PR #7 の stderr ログ / --json 変更との整合も確認済みです。コード本体にマージを止める欠陥は見つかりませんでした。

良い点(確認済み)

• 「空の only」と「capture_all」を型(Selection)・JSON 契約・テストの三層で区別し切っている
• fail-closed の分岐(CorruptInput)が enum 1 個に集約され、両モードの食い違いが構造的に起きない
• baseline / HEAD の commit SHA を計画へ焼き付けて CI 側で無効化検知させる設計は堅実
• plan が作る pending ビルドは baseline 選定に影響しない(baseline は承認時のみ昇格)ため、plan を 2 回実行しても baseline が汚れる事故は起きないことを確認した

指摘

1. 🟡 README の CI レシピが、本文で禁じている曖昧さを再生産している
   サンプルでは only かつ story_ids が空の場合と capture_all の場合のどちらも「空の stories.txt」になり、直後の本文で強調している両者の区別がレシピ上で消えます。CI 側が「空 = 撮らない」と解釈すると capture_all で 1 枚も撮らない危険側に倒れえます。plan の値をセンチネル(別ファイル/変数)で後段へ渡す形への書き換えを推奨します。あわせて、表で謳っている「未知の version なら全撮影へ倒す」チェックもレシピに含めてください。

2. 🟡 plan 由来の宙吊り pending ビルドに回収機構がない
   --baseline-commit なしの vrt plan は baseline SHA を得るためだけに screenshots ビルド(pending)を作成します。plan 後に CI が失敗して finalize されなかった場合、そのビルドは pending のまま永久に残ります(backend に expiry/reaper 相当の機構がないことを確認済み)。baseline 選定には影響しないため正確性は壊れませんが、運用ノイズとして蓄積します。本 PR では README への明記のみで可、根本対応(古い pending の自動 fail、または baseline 取得専用の read-only エンドポイント)は後続 PR が妥当と考えます。

3. 🔵 README の「build_id も null になる」は実装と不一致です(skip_serializing_if によりキー自体が省略される)。jq では実用上動きますが、厳密なスキーマ検証では食い違います。また story_ids = キー省略 / reason = null 常在 / build_id = キー省略と、フィールドごとに方針が分かれているため、契約として揃えられるとより良いです。

4. 🔵 機械可読出力の様式が subcommand 間で不揃いです(upload --json は 1 行 JSON、plan は pretty の複数行)。揃えるか、揃えない理由をコメントに残すことを推奨します。

5. 🔵 cli/src/main.rs で、機能と無関係に既存の説明コメント(--wait なし時の終了コード方針、--json の error キー契約、flush_logs が失敗を終了コードに波及させない理由など)が数カ所削られています。復元を推奨します。

6. 🔵 --baseline-commit に存在しない SHA を渡した場合、git diff 失敗として exit 0 の capture_all に倒れます。shallow clone と区別がつかないため fail-closed 自体は妥当ですが、設定ミスも黙って全撮影になります。SHA の存在確認(git cat-file -e 相当)だけを exit 2 に分離する余地があります。

テストカバレッジ

19 件は要所(空 only vs capture_all、story_ids 省略、SHA 焼き付け、round-trip、storybook モードの挙動維持)をすべて固定しており実効性は高いです。一方すべてライブラリ層(plan.rs)止まりで、run_plan 本体(stdout の JSON 純度、--output の書き出し、引数不足時の exit 2、ビルド作成経路)は未テストです。CLI レベルの smoke テストを 1〜2 本追加できると盤石です。

[yupix]

再レビュー結果: REQUEST CHANGES(修正 1 点のみ)

前回(2026-07-31)の指摘 6 件について、対応コミット(62e759c、dbbaaa5)を現物コードと突き合わせて検証しました。6 件すべて実体を伴って対応されています。 ただし、対応のために追加されたテストが PR head の CI(test チェック)を実際に落としており、この 1 点の解消までマージ不可と判断します。

前回指摘の対応状況(すべて検証済み)

#	指摘	状況
1	README レシピが only 空 / capture_all の区別を潰す	対応済み。PLAN_KIND センチネルファイル方式で区別が保存され、version ガードと未知値の capture_all への上書きも実装。シェルとしての正しさも確認
2	宙吊り pending ビルドの回収機構なし	対応済み(README に残留の明記と --baseline-commit での回避、根本対応は後続 PR。前回の合意どおり)
3	build_id の null 記述と実装の不一致	対応済み(README を「キーを省略」に修正、story_ids / reason の方針も明記)
4	機械出力の様式不揃い	対応済み(揃えない理由を doc comment に明記。前回提示した選択肢のひとつ)
5	PR #7 由来の説明コメントの削除	対応済み(復元を確認)
6	--baseline-commit の typo が exit 0 の capture_all に化ける	対応済み。明示指定時は即検証して exit 2、加えて OID 正規化と diff 終点の明示化まで踏み込んだ良い修正
-	CLI smoke テストの推奨	対応済み(cli_smoke.rs 5 本 + fixtures 5 本追加)

🔴 要修正: 新規テストが shallow clone の CI で落ちる

• 対象: apps/backend/crates/cli/src/git.rs の resolve_commit_accepts_head_tilde_notation / changed_files_diffs_between_two_explicit_commits、tests/cli_smoke.rs の plan_normalizes_baseline_ref_to_full_oid / plan_diff_uses_explicit_commit_not_worktree_head
• 原因: これらは HEAD~1 / HEAD~2 に依存しますが、.github/workflows/backend-test.yml の actions/checkout は fetch-depth 未指定(デフォルト 1)のため親コミットが存在せず、fatal: Needed a single revision で失敗します。PR head の CI ログで test result: FAILED. 17 passed; 2 failed を確認済みです。
• 修正案(いずれか):
  • (a) 推奨: テスト内で一時ディレクトリに git init して合成コミットを 2〜3 個作り、自給自足にする(実行環境に依存しない)
  • (b) 手軽: backend-test.yml に fetch-depth: 2 以上を指定する(ただしテストがランナーの checkout 履歴に依存する構造は残る)

🔵 Low: 未使用関数と doc comment への内部識別子の混入

apps/backend/crates/cli/src/git.rs:57 の commit_exists は、dbbaaa5 の clippy 対応で呼び出し元が resolve_commit 直呼びに変わり、定義だけが残っています(pub のため dead_code 警告が出ません)。また doc comment 内の「cmd_591」は読者に意味の取れない内部作業由来の識別子です。関数の削除、またはコメントの書き直しを推奨します。

総評

対応の中身は「直したつもり」ではなく全件が実効性を伴っており、特に指摘 6 は要求以上の踏み込みです。上記 🔴 の 1 点(修正は軽微)が解消され次第、APPROVE 相当と判断します。

[yupix]

レビュー

総評

CLI(vrt plan / vrt stamp)から比較ジョブ・ロック規約まで一続きで筋を通した大きな変更です。核心の安全性——空集合一致の偽 PASS 封鎖、carry-forward の冪等設計(UUIDv5 決定的キー + ON CONFLICT DO NOTHING + 不在確認つき補償削除)、「撮らなかった / 撮り損ねた / 削除された」の3値判定——は多層で正しく塞がっていることを確認しました。テストも positive control(修正前実装ならどう落ちるか)を20箇所で明記する規律があり、質は高いです。

ただし、高2件を含む要修正があります。

【高】マージ前に必須

1. pin_baseline_for_partial_render がロック外・状態チェック無しで、finalize 済みビルドの baseline_id を上書きできる — crates/service/src/builds.rs

トランザクションも review_lock::build も無く、baseline_id の update を finalize_storybook の遷移チェックより前に単独コミットしています。遷移チェックはハンドラが読んだ古いメモリ上のモデルで判定されるため、次の経路が成立します:

• ビルドが only_story_ids + SHA=S1 で finalize され rendering 中に、リトライ等で 2 度目の finalize が届く
• その間に別ビルドの承認で baseline が B2 に前進していると、2 度目のリクエスト(expected=S2)は SHA 照合を通過して baseline_id を B2 に上書きコミット
• 本体の finalize は 409 で弾かれるが、レンダリング・比較ジョブは以後 B2 を読む——固定で防いだはずの「計画の根拠と比較相手のズレ」がまさに起きる

finalize_screenshots と同様に、行ロック内で「pending 再確認 → SHA 照合 → pin → 遷移」を1トランザクションにまとめてください。review_lock.rs の新ドキュメントも screenshots 経路しか直列化を謳っておらず、規約上もこの経路だけ穴になっています。

2. README の storybook --only-changed レシピの vrt stamp が動かない

pnpm build-storybook --stats-json
vrt stamp --dir ./storybook-static

StampArgs.build_command は required = true なので、build コマンド無しの vrt stamp は clap の usage エラーで即死します(stamp_without_a_build_command_is_a_usage_error が固定しているとおり)。さらに「build 後の後追い stamp」という形自体が、本 README が2箇所で明示的に禁止した形であり、ドキュメント内で自己矛盾しています。コピペした CI は全撮影フォールバックですらなくジョブ失敗になります。

vrt stamp --dir ./storybook-static -- pnpm build-storybook --stats-json の1行に差し替えてください。

【中】PR 本文と実装の食い違い

• 「競合の再現は PostgreSQL のトリガ内 advisory lock」→ 実装に存在しません。 diff 全文に advisory lock もトリガも0件です。実際の作りは「実トランザクションで行ロックを保持 + 500ms sleep + !is_finished()」で、これ自体は妥当ですが、本文の説明が実装と一致していません。
• 「baseline をビルド作成時に固定」→ 実際は計画添付時です。 service/builds.rs の doc コメント自身が「作成時には常に NULL のまま」と明記しています。利用者向けの保証(撮影開始前に固定)は成立しているので、本文の記述の修正で足ります。
• 「SHA は常に40桁 OID へ正規化」は CLI 側のみです。 サーバーは長さ 1..=100 の検証だけで、baseline 照合は完全文字列一致。CLI(40桁)と直 API(短縮形)を混ぜたパイプラインでは同じコミットでも 409 になります。サーバー側でも形式検証か正規化を検討してください。

【中】ロジック・テストの穴

1. 新規 story の選択漏れが無言で消える: manifest ∖ baseline ∖ selected の集合はアップロードも引き継ぎ元も無く、比較結果のどこにも現れません。CI の選択ロジックにバグがあると、新規 story が added として報告されずレビュー可視性ゼロのまま PASS しえます。attach_capture_plan は baseline entries を既に読んでいるので、selected ⊇ manifest ∖ baseline の検査を足して 400 にできます。
2. provenance の verify→read に TOCTOU: provenance::verify がハッシュ照合した後、plan::select_stories が同じ stats/index を読み直しており、照合したバイト列と選別に使うバイト列の同一性がプロセス内で保証されません。verify が読んだバイト列を select に渡す read-once 化を推奨します。
3. 競合テストが本物の attach_capture_plan のロック取得を検証していない: uploads_are_serialized_with_plan_attachment_on_the_build_row は添付処理を手書き JSON + 自前トランザクションで再現しているため、将来 attach_capture_plan からロック取得が消えてもテストは緑のままです。手書き JSON は CapturePlan の serialize で生成し、attach 側のロック取得を別途固定すべきです。
4. CLI バイナリ経由の plan:"only" 成功経路の smoke が無い: plan_narrows_when_worktree_matches_the_recorded_commit は provenance 無しの fixture を使うため実際は capture_all になるはずで、テスト名と検証内容が乖離しています(assert が SHA 2つのみで plan の値を見ていないため通ってしまう)。stamp→plan の成功連鎖を smoke に追加してください。
5. 否定側テストの欠け(いずれも実装・openapi には宣言済み):
   • storybook finalize で only_story_ids あり・expected_baseline_commit_sha 省略 → 400(後方互換を破った当のガードが未テスト)
   • storybook モードへの captured_names → 400
   • 計画ありビルドで finalize の SHA 不一致 → 400
   • baseline が無いブランチへの plan 添付 → 409
6. 性能: materialize_carry_forward がエントリごと直列の download→upload→insert で、さらに比較フェーズが複製バイトを再ダウンロードしてピクセル比較します(コメント自身が「バイト列が同一なので unchanged と判定する」と述べています)。有界並列化と、reused == true のショットの diff スキップだけでも、大規模スイート + 小さい selected 集合という本 PR の主用途でジョブ時間を大きく削れます。

【低・微】

• migration: ベクタ順とタイムスタンプ名が逆転しています(m20260802000000_build_captured_names → m20260801000001_build_capture_plan)。captured_names は追加直後に次で drop される履歴遺物で、改名前の旧名で適用済みの開発 DB では IF NOT EXISTS 無しの再適用で落ちます。未出荷なら1本に潰すのが最善です。また、この lib.rs には PR feat: 承認時の危険操作に明示フラグと証跡を追加 #9 の m20260801000000_build_approval_evidence が無く、マージ時に lib.rs / entity が確実に衝突するため順序に注意してください。
• 「baseline が動いた(再計画で解消)」の応答が plan 添付=409 / finalize 系=400 で割れています。どちらも 409 に揃えることを推奨します。
• provenance::invalidate は --stats-json / --index-json に渡された任意パスを無検証で remove_file します。タイポで無関係ファイルが消えるため、少なくとも README に「指定したパスは build 前に削除される」ことの明記を。
• vrt upload に --index-json が無く、カスタム index で stamp した成果物はエラーメッセージ("its content changed after vrt stamp")から真因(stamp と upload で解決パスが違う)に辿れません。
• stamp の前後2点観測では build 中の A→B→A checkout 往復は検出できません。実装としては許容範囲ですが、README の「build 中の checkout はここで検出され」は過剰主張なので「保証しないこと」への1項追加を。
• openapi の plan エンドポイントの 400 記述に、命名ずれガード(ゼロ交差)・255バイト制限・前後空白規則が載っていません。
• 細かい点: --output の書き込みを stdout 出力より前に / reason に機械可読コードの検討(additive に足せるため低優先) / baseline 直読みヘルパのテスト間重複 / README の BUILD 変数が以後未使用 / 競合テストのコメント「ストレージは補償削除」に対応するストレージ側アサートが無い。

良かった点

• positive control を20箇所で明記する文化。テストが実装の写経になっていないことを自己証明しており、レビュー可能性が非常に高い
• 空集合一致の偽 PASS 封鎖が多層(添付条件・行ロック・finalize 検査・比較ジョブ再検証)で、撮影結果からの計画逆算の経路が実際に存在しない
• no-op build 問題を「invalidate → build → 存在検査」で解決し、失敗 stamp が旧 provenance を残さない性質が同じ1手から従属的に得られる設計
• ScreenshotName への検証一本化と、境界値(255/256バイト、空 selected / 空 manifest / ゼロ交差の三すくみ)の正負両側の押さえ方

判定

高2件(storybook pin の直列化、README レシピ)と PR 本文の記述修正(advisory lock / 作成時固定)が済むまではマージ不可と考えます。中の「新規 story の無言スキップ」と否定側テスト(expected_baseline_commit_sha 省略時の 400)も強く推奨します。それ以外は追撃 PR で問題ありません。

[sousuke0422]

詳細なレビューをありがとうございます。

PR 本文の食い違い（中）

三点とも本文の誤りでした。実装ではなく本文を直しました。

• 競合の再現方法：advisory lock とトリガは本 PR には入っていません。実装どおり「行ロックを保持したまま後続要求を投げ、ロック待ちを確かめる」に直しました
• baseline の固定時点：作成時ではなく計画添付時です。作成時に固定しない理由（作成から撮影開始までに進んだ baseline を取りこぼす）も添えました
• SHA の正規化範囲：CLI 側のみであることと、API を直接叩いて短縮形を送ると一致しないことを明記しました

サーバー側でも形式検証または正規化を行うかは、別途検討します。

高2件

いずれも対応します。

1 の pin_baseline_for_partial_render は、finalize_screenshots と同じ形で「pending 再確認 → SHA 照合 → pin → 遷移」を行ロック内の一トランザクションにまとめます。review_lock.rs の記述も storybook 経路を含む形に直します。

ご指摘のとおり、直列化を screenshots 経路にしか通していませんでした。

2 の README は vrt stamp --dir ./storybook-static -- pnpm build-storybook --stats-json に差し替えます。
本文中で二度禁じている形をレシピで再生産していました。

中・低

ロジックとテストの穴、および低・微の各項目も順に対応します。
とくに次の二点は重く見ています。

• 新規 story の選択漏れが無言で消える点は、attach_capture_plan が baseline entries を既に読んでいるので、ご提案どおり selected ⊇ manifest ∖ baseline の検査を足します
• 競合テストが実際の attach_capture_plan のロック取得を検証していない点は、テストが実装から乖離しても緑のままになる形なので、手書き JSON を CapturePlan の serialize に置き換えたうえで attach 側のロック取得を別途固定します

migration をまとめる件も、未出荷なので一本に潰す方向で見ます。

対応後に改めてご確認をお願いします。

[yupix]

レビュー(二周目): 前回指摘の訂正と追加の発見

前回のレビューコメントに対して敵対的な再検証と、一周目で手薄だった領域(認可境界・Unicode・git 出力パース・serde 互換)の追加レビューを行いました。まず前回コメントの訂正から。

前回指摘の訂正

訂正 1: 【高1】pin_baseline_for_partial_render — トリガーの記述が不正確でした(High → Medium)

「finalize リトライで baseline_id が上書きされる」は誤りです。素朴なリトライは初回と同じ expected_baseline_commit_sha を再送するため、baseline 不動なら同値書き込みの no-op + 409、baseline 移動後なら SHA 照合(builds.rs)で 400 になり、すり替えは起きません。

問題が成立するのは「baseline 昇格後に、新しい baseline の SHA を指定して同一ビルドへ finalize を再送した場合」に限られます。ただし構造上の指摘自体は事実のままです: pin_baseline_for_partial_render は status チェック・トランザクション・review_lock::build の外で baseline_id を単独コミットし、後続の finalize_storybook が 409 を返しても巻き戻りません。render / compare ジョブはジョブ実行時に baseline_id を読むため、rendering 中のビルドの比較 baseline を後から差し替えられる点は変わりません(screenshots 側 3 経路がすべて行ロック内なのに対し、この経路だけロック無しで浮いています)。

重大度を Medium に引き下げたうえで、修正推奨は前回どおり「pending 再確認 → SHA 照合 → pin → 遷移を finalize と同一トランザクション + build 行ロック内へ」です。

訂正 2: 【中】provenance の verify→read TOCTOU — セキュリティ問題ではありません(→ nit)

provenance は無署名のローカルファイルであり、verify〜select の間にファイルを差し替えられる主体は provenance 自体を再計算して偽造できるため、read-once 化しても攻撃耐性は変わりません(README の脅威モデル「事故のみ」とも整合)。残るのは「ビルドプロセスが並行して stats を書き換えた場合に、検証した内容と選別に使った内容がずれる」という堅牢性の nit です。verify がバイト列を返して select がそれを parse する read-once 化は安価なので、余裕があれば、という位置づけに格下げします。

再検証で維持された指摘

• 【高】README の vrt stamp --dir ./storybook-static(build コマンド無し)は required = true で usage エラー確定。README L267 の「build コマンド無しの形は提供しない」とも自己矛盾 — そのまま維持
• 【中】新規 story(manifest ∖ baseline ∖ selected)の無言スキップ — 全経路(upload 400 / carry-forward は baseline のみ走査 / join に不在 / 逆向き検査なし)を再確認して成立。storybook 側の decide_story_action が「baseline に無い新規 story は見逃さず撮影」する対称ガードを持つのに screenshots 側に等価物が無い非対称も裏付け — そのまま維持
• screenshots 3 経路(plan 添付 / upload / finalize)の行ロック設計は再監査でも崩れなし

追加の発見

1. 【中】ScreenshotName に Unicode 正規化が無く、NFC/NFD 差で carry-forward / removed 判定が狂う — crates/common/src/validation.rs

parse は空・前後空白・255 バイトのみを検査し、equality は全経路(plan / upload / finalize / carry-forward)でバイト一致です。baseline を Linux CI(NFC)で育てた後に macOS ランナー(APFS は NFD を返す)で plan を作ると、見た目同一の日本語名 story が「NFC 側は vanished → removed、NFD 側は added」に割れます。一部の名前だけなら attach_capture_plan の命名ずれガード(交差 1 件以上で通過)もすり抜けます。倒れる方向は「差分が見える側」なので偽 PASS ではありませんが、部分アップロードの意味論が壊れます。parse で NFC へ正規化して保持することを推奨します(既存 baseline は NFC 前提でほぼ無傷)。

2. 【低〜中】git diff --name-only が C-quoting 未対応 — 非 ASCII パスの変更で差分選別が永久に無効化 — crates/cli/src/git.rs

git は既定(core.quotepath=true)で非 ASCII / " / \ 入りパスを "\343\202\263..." 形式で出力しますが、changed_files は trim するだけで quote を解きません。この文字列は依存グラフのキーに一致せず、常に「グラフ外 → CaptureAll」フォールバックに落ちることを確認しました。日本語ファイル名のコンポーネントを 1 つでも変更するリポジトリでは、本 PR の主機能がそのリポジトリでは事実上効かず、reason には octal エスケープ列が出るため原因も分かりにくいです。git -c core.quotepath=false diff --name-only -z + NUL 区切りパースを推奨します(改行入りファイル名にも頑健になります)。

3. 【低】ScreenshotName が内部の制御文字(NUL・ESC・CR/LF)を許容する

検査が前後空白のみのため "a\nb" や "a\x1b[31mb"、"a\0b" が通ります。NUL 入り名は Postgres の text/jsonb が U+0000 を拒否するため、plan 添付や upload で 400 ではなく 500 になります。また名前はエラーメッセージ経由で build_logs に生で入り、CLI がログを端末へ流す経路があるため、ANSI エスケープ注入の芽になります(端末側の詳細は未確認)。parse で制御文字を拒否することを推奨します。なおストレージキーは UUID のみで構成され、名前中の / や .. がキーへ混入する経路は無いことを確認済みです。

4. 【低】CLI の Client::finalize が only_story_ids: None のとき expected_baseline_commit_sha を黙って捨てる — crates/cli/src/api.rs

ボディは only_story_ids が Some のときしか送られません。現行の呼び出しでは実害はありませんが、サーバーが実装している「screenshots モード + 計画あり + expected のみ」の照合にこのクライアントから構造的に到達できず、将来「照合したつもりで送られていない」footgun になります。only_story_ids.is_some() || expected.is_some() で送るようにするのが安全です。

5. 【低】安全照合フィールドの typo が unknown field として黙殺される

FinalizeBuildRequest / AttachCapturePlanRequest とも deny_unknown_fields が無く、expected_baseline_sha のような typo は黙って無視され、baseline 照合がサイレントに無効化されます(expected 省略は合法のため typo = 省略になる)。新設の AttachCapturePlanRequest には #[serde(deny_unknown_fields)] を付けて損がありません。FinalizeBuildRequest は新旧 CLI 混在の互換性と相談してください。

6. 【微】

• plan 添付後に pinned baseline の source build が(手動削除等で)消えると、finalize の 400 文言が「expected_baseline_commit_sha requires a capture plan」になり実態(計画も pin もある)と食い違う。文言の分岐を推奨
• 名前の trim → 拒否化(render_build / store_ci_screenshot)は意図的な破壊的変更としてコメントに明記済みだが、既存プロジェクトが前後空白付き story 名を持っているとデプロイ後の初回ビルドが突然落ちる。リリースノートへの記載を推奨
• store_ci_screenshot の plan 照合が線形走査(selected 最大 1 万件 × アップロード毎、行ロック保持中)。HashSet 化が安価

追加で確認して異常なしだった点

• テナント/プロジェクト境界: plan 添付・upload・finalize すべて load_build_with_role 経由で build → project → require_role(Member) を通し、NotFound は Forbidden にマスク。他テナントの build_id 指定は Forbidden
• CI トークン scope: attach_capture_plan は Scope::WriteBuild 要求で upload / finalize と同等
• metadata.reused はクライアントから注入不可(CI upload は metadata: Set(None) 固定)。reused と実アップロードの同名衝突は UNIQUE + existing スキップで実アップロード優先
• 計画外 captured は比較ジョブ段階でも uploaded(!reused) == selected の再検証で bail(多重防御)
• serde 後方互換: 新フィールドはすべて #[serde(default)] の Option で、旧 CLI のリクエストは 422 にならない
• git rev-parse は --verify + ^{commit} + --end-of-options + shell 非経由で、インジェクション経路なし
• pinned baseline の寿命: baselines は insert-only、prune_old_builds は全 baseline 行の source build を保護し、builds.baseline_id のぶら下がり経路なし

判定(更新)

マージ前必須は「README の stamp レシピ修正」「PR 本文の記述修正(advisory lock / 作成時固定)」に、「pin の直列化(Medium に降格)」と「新規 story の無言スキップ」「NFC 正規化」を強い推奨として加えた形に更新します。quotepath(#2)は主機能の実効性に関わるため、対象リポジトリに非 ASCII パスがあるなら実質必須です。

[yupix]

P1: public 配下の Markdown 変更を無条件に無視しないでください。

対象: apps/backend/crates/cli/src/turbosnap.rs:298-302

依存グラフ外の .md をすべて除外すると、story が public/copy.md のようなランタイム配信アセットを描画していても webpack stats には依存として現れず、そのファイルだけを変更した場合に seed が空になって Plan::Only([]) になります。結果としてサーバーが古い baseline を carry-forward し、見た目が変わっていても false PASS になります。

判定できない .md は CaptureAll に倒すか、除外対象を README 等の既知ドキュメントパスの allowlist に限定してください。public/*.md を変更した際に全撮影へ倒れる回帰テストもあると安全です。

注: 対象行がPR差分に含まれていないため、この指摘はインラインではなく会話欄へ投稿しています。