Skip to content

fix(render): 撮影直前にキャレットとアニメーションを決定的に静止させる - #19

Merged
yupix merged 15 commits into
mainfrom
feat/render-caret-anim-freeze
Aug 10, 2026
Merged

fix(render): 撮影直前にキャレットとアニメーションを決定的に静止させる#19
yupix merged 15 commits into
mainfrom
feat/render-caret-anim-freeze

Conversation

@sousuke0422

@sousuke0422 sousuke0422 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

何をしたか

storybook モードのレンダラが、撮影の直前にページを静止させてから撮るようにした。

  • 入力キャレットを不可視にする
  • 有限のアニメーションと transition は終端へシークして止める
  • 無限のアニメーション(スピナー等)は先頭へ巻き戻して止める

利用者の Storybook や preview には何も足さない。

なぜ必要か

差分レビューに、キャレット一本とスピナーの弧しか写っていない回があった。コードは何も変わっていないのに、撮った瞬間の位相が違うだけで差分になる。

こうしたものが毎回出ると、やがて中身を確かめずに承認するようになる。それが一番危ない。

Chromatic や Playwright でこの種の差分が出にくいのは、差分を許しているからではなく、撮る前に止めているからである。同じことを我々のレンダラでもやる。

なぜ「止める」だけでは足りないか

animation-play-state: paused にするだけだと、止まる位置が撮影タイミングに依存するので、flaky はそのまま残る。

そこで位置を一意に決める。

// 有限は終端へ、無限は初期(タイムライン座標 0)へ。どちらも
// 壁時計に依存しない一意な座標なので、2 回撮っても同じ絵になる。
anim.currentTime = Number.isFinite(end) ? end : 0;
anim.pause();

静止用の CSS は document だけでなく open shadow root ごとにも直接注入する。継承に頼らないのは理由が二つあるためで、それぞれ別である。

  • caret-color は継承プロパティだが、shadow 内に明示宣言があればそちらが勝つ。継承はカスケードで最も弱く、外側の !important も継承経由では効かない
  • transition-duration はそもそも継承されない

document.getAnimations() が shadow tree を返さない実装があるため、open shadow root は自前で辿って合流させている。iframe の探索も各 root の中で行い、同一オリジンなら再帰する。

スタイル適用やシーク自体をきっかけに始まるアニメーションがあるので、2 巡かけて同じ座標へ落とす。

しきい値は触っていない

diff_ratio_fail を上げれば flaky は消えるが、小さいが本物の変化も一緒に見逃す。

雑音を発生源で消すなら検出力は落ちない。こちらを採った。SETTLE_DELAY も変えていない。

検証

同じ story を 2 回撮って PNG のバイト列が一致することを固定した。一致すれば flaky がないことの直接の証明になる。

  • frozen_captures_are_byte_identical_across_runs — 静止させれば一致する
  • unfrozen_captures_differ_from_frozen_ones — 静止させなければ異なる

両方向を押さえてあるので、テストが素通りしていないことも分かる。

移行について

この版へ更新した直後の最初のビルドでは、アニメーションやキャレットを含む story が changes_detected になりうる。baseline が静止前の絵のままだからである。

一度きりの差分で、レビューして承認すれば以降は安定する。README に手順を書いた。

静止できなかったときは撮らない

静止処理は最後に、running のまま残っている animation が無いことを検証する。残っていれば失敗を返し、その story はエラーになる。撮らない。

ok が真であると確かめられた場合にだけ撮影へ進む。結果を解析できなかった場合(JSON でない、ok が無い、型が違う)も失敗である。「判定できなかった」を成功へ倒さない。

検証そのものが失敗した場合も同じである。getComputedStyle が投げるなど、適用を確かめられなかったときは成功へ倒さない。集めた診断は判定の入力として使い、一件でもあれば失敗とする。

失敗の理由はメッセージで区別する。「静止に失敗した(何が running のまま残ったか)」「静止結果を解析できなかった」「適用を検証できなかった」は別の事象で、利用者が原因に辿り着けるようにするためである。

静止に失敗した story は、その story だけをエラーとして記録し、残りは撮り続ける。ビルド全体は落とさない。以前は失敗が伝播してビルドごと落ちていたので、静止できない story が一つあるだけで全 story のスクリーンショットが得られなくなっていた。

ただしビルドの結果自体は失敗のままにする。欠けたまま比較へ進めば、撮れていない story が「変化なし」として通ってしまうためである。

静止処理そのものにも上限がある。FREEZE_SCRIPT は非同期で、解決がページ側の requestAnimationFrame に依存する。rAF が発火しないページでは永久に返らないので、readiness ループと同じ story_timeout に載せた。時間内に終わらなければ失敗である。

この PR では同じ形の見落としを三度やった。静止の失敗、結果の解析の失敗、そして検証自体の失敗。層を足すたびに、その層自身の失敗経路が素通りとして生まれる。同じことを繰り返さないよう、各層について「失敗を検知できるか」「検知したらどう倒れるか」「検知できないならなぜか」を表にして、browser.rs のモジュール doc へ残した。

あわせて、層をまたぐ手当ての漏れも起きた。先に入れた per-element の収集が、後から足した検証層に移っていなかった。縦(層ごとの失敗経路)だけでは見えないので、横(どの層がどの手当てを持っているか)の表も並べてある。

動かないはずのものが動いたまま撮られて「なぜか毎回差分が出る」となるより、「この story は静止できなかった」と出た方がよい。

収束させる

sweep 自体が animationend を発火させ、その handler が次の animation を開始することがある。2 巡固定では取り切れないので、running が無くなるまで反復する(上限 10 回)。

途中で「進捗が止まった」と見て早期に打ち切る仕組みは入れていない。残っている animation の集合が同じかどうかを判定しようとしたが、tagName のような代理では同名・同タグの別要素を区別できず、正常に進んでいる連鎖を誤って打ち切った。identity を確実に持つ手立てを別途保証する必要が出るので、依存する側をやめた。

上限内に running が無くならなければ失敗とする。

新しい animation を継続的に抑止する案も検討したが、animation: none に倒すと要素が非アニメーション時の見た目へ戻り、撮れる絵そのものが変わる。反復する方を採った。

CSS が効いたことを確かめる

静止用の CSS は <style> の追加ではなく、constructed stylesheet(new CSSStyleSheet() + adoptedStyleSheets)で当てる。CSSOM 操作は CSP の style-src の管轄外なので、CSP が原因で注入が拒否されるという状態そのものが起きない

以前は <style> を追加していて、CSP に拒否されても例外が出ないため、静止できていない絵を成功として撮っていた。検証を足して落とす形も試したが、そもそも起きなくする方が強い。

ok を返す前に、専用のプローブで CSS が実際に効いていることを確かめる点は変わらない。利用者側が !important で指定した caret-colortransition には勝てないので、そこは best-effort である(README に明記)。

CSP を迂回する手段(Page.setBypassCSP)は使っていない。ページ全体の CSP が外れると script-src も無効になり、本来実行されない inline script が動いて DOM が変わる。撮影対象が通常のブラウザと違う状態になるので、この PR の前提に反する。

progress-based timeline

animation-timeline: scroll() などでは getComputedTiming().endTime が数値ではなく CSSUnitValue になる。数値かどうかで判定すると「無限」と誤認し、数値の 0 を代入して TypeError になる。

型を保ったまま有限の終端へ seek する。

transition イベントについて

静止よりあとに始まる transition は、そもそも生成されない。CSS Transitions の仕様では duration と delay の合計が 0s より大きいときにだけ transition が開始されるためで、プロパティ値だけが即座に終値へ変わる。生成されない以上 transitionrun / transitionstart / transitionend はどれも発火しない。

静止の時点ですでに走っていた transition は別で、終端へシークされて完了し、その transitionend は発火する。どちらの向きもテストで実測して固定してある。

したがって、静止よりあとに始まる transition の完了を合図に見た目を変えるコンポーネントは、その更新が起きる前の絵で撮られる

これは意図した割り切りである。理由は二つ。

  1. 目的は決定的な静止画であり、ページは静止の直後に撮られてそれ以降の相互作用が無い
  2. 仮に transition を生成させて transitionend を発火させると、それに依存する状態更新がいつ描画へ反映されるかが撮影タイミングに依存し、「時刻に依存しない絵を撮る」という目的そのものに反する

イベント互換性より撮影の決定性を優先している。詳細は README に書いた。

届かない範囲

README に明記した。実装で塞げていないものだけを書いてある。

  • closed shadow root とクロスオリジン iframe の中
  • canvas や requestAnimationFrame で毎フレーム描き直す JS アニメーション
  • 利用者側スタイルが !important で明示した caret-color / transition(注入 CSS も !important だが * セレクタなので、!important 同士の比較では利用者側が勝つ)
  • 静止処理より後から JS で生成される shadow root や iframe

JS アニメーションを止められないのは、次に何が描かれるかが実行するまで決まらないためで、「終端」も「初期」も定義できない。そうした story は動きを止めた状態を Storybook 側で用意することになる。

またこれが効くのは storybook モードだけである。screenshots モードでは撮るのは CI 側なので、キャレット隠蔽とアニメーション静止も撮影側で行う。

storybook モードのレンダラが READY 検出 → settle のあと素通りで撮影して
いたため、キャレットの明滅とアニメーションの位相がそのまま flaky な差分に
なっていた。しきい値を緩めるのではなく、撮影の入力から時刻依存を消す。

- キャレットは caret-color: transparent で不可視化
  (Playwright の caret: 'hide' と同型。caret-color は継承プロパティなので
  open shadow DOM へも波及する)
- 有限アニメーション・transition は currentTime = endTime へシークして pause。
  終端は仕様上ただ一つに定まる状態で、壁時計に依存しない
- 無限アニメーションは currentTime = 0 へ巻き戻して pause。
  paused だけでは止まる位置がタイミング依存のまま残るため、
  タイムライン座標を明示的に固定する
- open shadow DOM・同一オリジン iframe は再帰的に辿る。closed shadow root・
  クロスオリジン iframe・canvas / rAF 駆動の JS アニメには届かない
  (README に明記)

positive control として、スピナー(無限)・フォーカス入力欄(キャレット)・
有限アニメの 3 story を同一 story 二回撮りで PNG バイト一致させる統合テストと、
freeze を切ると決定的な絵から乖離する(= 修正前は一致しない)ことを固定する
対照テストを追加。注入は全てレンダラ側で、利用者 repo には一切手を入れない。

Assisted-by: multi-agent-shogun-aki-tweak
@sousuke0422

This comment was marked as resolved.

document.head への注入だけでは open shadow root の中に静止が届かない:
caret-color の継承値は shadow 内で明示された宣言に負け(継承はカスケード最弱)、
transition-duration はそもそも継承せず、querySelectorAll('iframe') は
shadow 境界を越えない。README の「未対応は closed shadow」という主張は
open shadow に対して保証できていなかった。

- freezeDoc を freezeRoot(root) に共通化し、Document と各 open ShadowRoot を
  同じ手順で処理する。root ごとに <style data-vrt-freeze> を注入し、
  root 内の iframe(shadow 内のものを含む)へ同一オリジンの範囲で再帰する
- shadow DOM 版のテストを追加。二回撮り byte 一致(caret / transition /
  shadow 内 iframe)、caret-color が本当に効いた直接証拠
  (明示 transparent の対照 story との絵一致)、computed transition-duration の
  色表示による 0s 強制の画素検証、freeze なしでは一致しない対照群。
  旧実装ではこの 3 テストはすべて落ちることを実測済み
- README の「届かない範囲」を実装どおりに更新(!important 明示宣言・
  静止後に生成される root は届かないことを明記)

Assisted-by: multi-agent-shogun-aki-tweak
@sousuke0422

This comment was marked as resolved.

browser.rs の「transition-duration: 0s でも transitionend は発火する」は誤り。
CSS Transitions Level 1 §3 は combined duration(= max(duration, 0s) + delay)
が 0s より大きいときだけ transition を開始すると定めており、duration と
delay をともに 0s にする静止 CSS の下では transition が生成されず
transitionrun / transitionstart / transitionend はいずれも発火しない。

- browser.rs: 誤った断定を削り、仕様参照つきの正しい記述へ差し替え
- 新テスト transitions_under_freeze_fire_no_events で三方向を固定:
  1) freeze なしでは transitionend が判定窓内に届く(positive control。
     「無いことの確認」の判定窓 2s = duration の 20 倍を同一環境の実測で裏づける)
  2) freeze 後に起こした transition はイベントを一切出さず、値だけ即座に終値へ変わる
  3) freeze 時点で走っていた transition は終端へシークされて完了し、
     transitionend を発火する(Chromium 実測)
- README「届かない範囲」: 静止後に始まる transition の transitionend を合図に
  見た目を変えるコンポーネントは更新前の絵で撮られるという代償と、
  イベント互換性より撮影の決定性を採る理由を明記
- Playwright caret:'hide' との対応関係の記述を実装(インライン指定)に合わせ精密化

Assisted-by: multi-agent-shogun-aki-tweak
軍師QC指摘: transitions_under_freeze_fire_no_events の手順3見出しが
「走行中の transition も transitionend を出さない」という当初の思い込みの
まま残り、直後の assert(Some(1) で発火)と矛盾していた。実測どおり
「終端へシークされ transitionend を出す」側へ訂正。

同型の直しかけ痕跡を grep で点検し、fixture doc(#slow の説明)と
テスト doc 見出しの過度に不発火側へ寄った記述も同時に揃えた。
コード・assert・FREEZE_SCRIPT は不変更。

Assisted-by: multi-agent-shogun-aki-tweak
@sousuke0422

This comment was marked as resolved.

@sousuke0422
sousuke0422 marked this pull request as ready for review August 8, 2026 17:17

@yupix yupix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

通常の time-based animation、caret、open shadow root、同一オリジン iframe、transition event のテストは手元でも 14 件すべて通過し、CI 5 workflow も成功していることを確認しました。

一方で、撮影直前に animation が実際に running のまま残る境界ケースを 2 件、PR head の Chromium で再現しました。どちらもこの変更の中心である「時刻に依存しない静止画」という保証を崩すため、修正と回帰テストをお願いします。詳細はインラインに記載しました。

Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
静止処理を fail-closed へ倒す。

- FREEZE_SCRIPT が無条件に true を返し catch(e){} が全失敗を握りつぶしていた
  構造を廃止。最終 sweep 後に running な animation が残っていれば失敗の JSON
  を返し、Rust 側で RenderError::Story へ伝播する。利用者は「なぜか差分が
  出続ける」ではなく「この story は静止できなかった」と原因に辿り着ける
- endTime が CSSNumericValue(progress-based timeline の percent 等)の場合、
  型を保って currentTime へ渡す。Number.isFinite による数値判定では percent を
  「無限」と誤認し、数値 0 を代入して TypeError → catch 握りつぶし → running
  のまま成功扱いになっていた
- 2 巡固定の sweep を安定状態まで反復する収束ループへ変更(上限 10 巡)。
  animationend ハンドラが次を開始する連鎖でも、収束すれば成功する。
  上限内に収束しなければ失敗を返す
- catch(e){} の洗い出し: クロスオリジン iframe(原理的に触れない)のみ
  握りつぶしを許容し、それ以外は errors 配列に記録して返却する
- テスト追加: scroll timeline 二回撮り一致、animationend 連鎖(p1→p2→p3)
  二回撮り一致、凍結不能ページの失敗検証(positive control 両方向)
- README: 静止失敗時の挙動・エラー形式・progress-based timeline 対応を追記

Assisted-by: multi-agent-shogun-aki-tweak
FREEZE_SCRIPT の返り値解析が「ok が明示的に false のときだけ失敗」だったため、
値が文字列でない・JSON でない・ok キー欠落・非 boolean のいずれの場合も
暗黙に成功として撮影へ進んでいた。freeze_verdict() へ切り出し、ok が true と
確かめられた場合にだけ撮影する fail-closed へ改める。

- 「静止に失敗した (freeze failed: ... still running)」と
  「静止結果を解析できなかった (freeze result was unparseable: ...)」を
  メッセージで区別し、利用者が原因へ辿り着ける形にする
- 単体テストで解析不能応答 6 形(値なし・非文字列・非 JSON・ok 欠落・
  非 boolean・null)の失敗を固定。修正前の判定では成功として通ることも実測
- E2E positive control: ページ側で JSON.stringify を ok キー持ちオブジェクト
  に限って壊し、READY_PROBE を無傷のまま freeze 応答だけ解析不能にする
  bundle を追加。修正前は撮影成功として通っていた

Assisted-by: multi-agent-shogun-aki-tweak
@sousuke0422

Copy link
Copy Markdown
Contributor Author

再現つきのご指摘、ありがとうございます。両方とも直しました。head は 72ce40c6 です。

指摘の根にあったもの

2 件を追っていくと、同じところに行き着きました。静止に失敗しても成功として返していたことです。

FREEZE_SCRIPT は無条件に true を返し、catch (e) {} が全ての失敗を握りつぶしていました。progress-based timeline の TypeError が飲まれていたのも、連鎖で残った animation が見過ごされていたのも、そのためです。

個別に潰すのではなく、そこから直しました。

静止できなかったときは撮らない

最後に running のまま残っている animation が無いことを検証し、残っていれば失敗を返します。その story はエラーになり、撮りません。

失敗の理由は区別します。「静止に失敗した(何が running のまま残ったか)」と「静止結果を解析できなかった」は別の事象なので、メッセージを分けました。

動かないはずのものが動いたまま撮られて「なぜか毎回差分が出る」となるより、「この story は静止できなかった」と出た方がよいと考えています。

① progress-based timeline

型を保ったまま有限の終端へ seek するようにしました。animation-timeline: scroll() の回帰テストを追加しています。

② animationend 連鎖

running が無くなるまで反復します(最低 2 回、上限 10 回、進捗が無くなった時点で打ち切り)。上限内に収束しなければ失敗です。p1 → p2 → p3 と連鎖するページの回帰テストを追加しました。

新しい animation を継続的に抑止する案も検討しましたが、animation: none に倒すと要素が非アニメーション時の見た目へ戻り、撮れる絵そのものが変わります。反復する方を採りました。

証明

「わざと凍らせられないページで、レンダラが失敗を返すこと」を固定しました。修正前のコードでは成功として通っていたことも確認しています。個別の 2 件が直ったことより、こちらの方が重いと考えています。

再現していただいた 2 件についても、それぞれ二回撮ってバイト列が一致することを固定しました。

内部レビューで見つかった分も塞ぎました

結果を受け取る Rust 側に、JSON の解析に失敗したときや ok が無いときに暗黙で成功へ落ちる経路が残っていました。同じ病が一段上に移っただけなので、ok === true と確かめられた場合にだけ撮影へ進むようにしました。

READY_PROBE の判定も同種でないか確認しましたが、こちらは既に Pending から Timeout へ倒れる形で fail-closed でした。

現在 23 件のブラウザテストが通り、CI も 5 workflow すべて緑です。改めてご確認をお願いします。

@yupix yupix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正後 head を再確認しました。既存の browser テスト 23 件は Chromium で全件通過していますが、決定的静止/fail-closed の保証を崩すケースを 2 件再現しました。どちらもインラインに再現条件を記載しています。

Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
The sweep convergence check compared running animation counts, which
misidentified progress in chains like p1→p2→p3→p4 where the count
stayed at 1 but the identity changed each sweep. Replace with set
identity comparison (animation name + target element) so chains are
correctly recognized as making progress.

Add CSS application verification via getComputedStyle before returning
ok:true. A successful appendChild does not prove CSS was applied — CSP
style-src 'self' silently blocks inline styles without throwing. The
renderer now calls Page.setBypassCSP before injection and verifies
caret-color and transition-duration are actually applied. Both layers
ensure fail-closed even if one is unavailable.

Assisted-by: multi-agent-shogun-aki-tweak
getComputedStyle returns 'rgba(0, 0, 0, 0)' instead of 'transparent'
in Chromium. Accept both representations in the CSS verification check.
Also apply cargo fmt.

Assisted-by: multi-agent-shogun-aki-tweak
Page.setBypassCSP must be called before the page loads so the CSP
policy from <meta> tags is bypassed when inline styles are injected.
Calling it after navigation leaves the meta CSP already in effect,
causing the injected <style> to be silently blocked.

Move the call from render_on_page (post-load) to render_story
(pre-navigation, on about:blank).

Assisted-by: multi-agent-shogun-aki-tweak

@yupix yupix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最新 head を再確認しました。既存の browser テスト 27 件と GitHub checks 5 件はすべて成功していますが、前回指摘の修正境界で 2 件を Chromium 再現しました。詳細はインラインに記載しています。

Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
P1-①: FREEZE_SCRIPT の収束ループから identity proxy(animation 名 +
tagName)による早期停止を除去。running な animation がゼロになるまで
MAX_SWEEPS(10 巡)反復する。proxy では要素の同一性を表せず、同一
keyframes が異なる要素へ順移動する連鎖(el1→el2→el3→el4)を
「停滞」と誤判定していた。

P1-②: Page.setBypassCSP(true) を撤回。本番と異なる CSP 条件で撮影すると
実際には CSP に拒否される inline style が通り、本番と異なる絵を撮る。
CSP が静止 CSS を拒否するページでは getComputedStyle 検証が fail-closed
で失敗し、利用者に原因が伝わる。script-src 'none' のページでは inline
script が実行されないことをテストで固定。

テスト追加:
- roaming_keyframes_across_elements_converge (同一 keyframes × 4 要素)
- waapi_chain_without_ids_converges (WAAPI id 無し連鎖)
- strict_csp_page_fails_closed_without_bypass (CSP fail-closed)
- script_csp_blocks_inline_scripts_with_freeze_enabled (script-src 'none')
- inline_script_runs_without_csp (positive control)

Assisted-by: multi-agent-shogun-aki-tweak
script_csp_blocks_inline_scripts_with_freeze_enabled は new_page で
直接開くだけで freeze も render_story も通っておらず、名が存在しない
担保を主張していた。

- render_story_with_freeze_keeps_script_csp_enforced を追加:
  script-src 'none' のみ(style-src 制約なし)の fixture を本番経路
  (render_story → FREEZE_SCRIPT → freeze_verdict → 撮影)で撮り、
  inline script が実行されない(絵が緑のまま)ことをピクセルで検証
- render_story_executes_inline_scripts_without_csp を追加:
  CSP を外した同一 fixture が同経路で赤になる positive control
- 旧テストは script_csp_blocks_inline_scripts_on_a_directly_opened_page
  へ改名し、freeze を通っていない旨を doc に明記
- freeze 関連の全テストに「何を証明し/何を証明していないか」を明記
  (new_page 系は freeze/render_story 経路を通っていないと明示)

Assisted-by: multi-agent-shogun-aki-tweak
@sousuke0422

Copy link
Copy Markdown
Contributor Author

両方とも直しました。head は 4764db21 です。

① identity への依存をやめました

tagName は要素の代理でしかなく、同名・同タグの別要素を区別できませんでした。ご指摘のとおりです。

WeakMap で安定 ID を付ける道も考えましたが、その ID が本当に identity を表せているかを別途保証する必要が出て、そこがまた代理になります。そこで早期停止そのものをやめました。running が無くなるまで反復し、上限内に無くならなければ失敗とします。

同じ @keyframes が 4 つの別要素へ順に移る連鎖と、id が空の generic WAAPI の連鎖を回帰テストに入れました。旧ロジックが 3 巡目で誤判定することも確認しています。

コメントも実装に合わせました。「対象要素の実体で比較する」は事実ではありませんでした。

Page.setBypassCSP を撤去しました

これは私の判断が誤っていました。前回「注入を通すために bypass しつつ、適用も検証する」という方針を出したのですが、bypass の副作用を数えていませんでした。

ご指摘のとおり script-src も外れ、本来実行されない inline script が動いて DOM が変わります。撮影対象が通常のブラウザと違う状態になるので、この PR の前提そのものに反していました。

撤去し、strict-CSP で静止 CSS が効かない場合は computed-style 検証で失敗として倒れるようにしました。script-src 'none' が freeze 有効時にも維持されることは、render_story を通す回帰テストで確認しています。

テスト名が担保より強かった点も直しました

内部レビューで、script-src 'none' のテストの一つが render_story を通らず new_page で直接ページを開いていることが分かりました。名前は「freeze 有効時」を含意していたので、ご要望の担保が実際には存在していませんでした。

render_story を通すテストを追加し、既存のものは実体に合わせて改名しました。あわせて、各テストが何を証明していて何を証明していないかを一行ずつ注記しています。とくに new_page で開くものには「freeze は通っていない」と明記しました。

現在 33 件のブラウザテストが通り、CI も 5 workflow すべて緑です。改めてご確認をお願いします。

@yupix yupix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最新HEADを再レビューしました。前回の identity-based early stop と CSP bypass の修正、および追加テストを確認しました。既存のブラウザテスト33件とGitHub Actions 5件はすべて成功していますが、freeze検証が例外になった場合の fail-open を実ブラウザで再現したため、1点コメントします。

Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
検証層(CSS 適用検証の getComputedStyle 等)の例外を errors に積むだけで
ok: true へ到達していた fail-open を塞ぐ。errors は診断の置き場ではなく
判定の入力——1 件でも積まれていれば撮影せず失敗を返す。collectRunning の
握りつぶしも同様に errors へ反映し、切り離された root は検証対象から外す。
freeze_verdict は errors をメッセージへ載せ、原因ごと利用者へ届ける。

preview の iframe 内で window.getComputedStyle を throw させる回帰テストを
追加(修正前は PNG 取得まで成功することを positive control として実測済み)。
層ごとの失敗経路表をモジュールコメントへ、検証層 fail-closed と脅威モデル
(事故であってセキュリティ境界ではない)の整合を README へ記した。

Assisted-by: multi-agent-shogun-aki-tweak
@sousuke0422

Copy link
Copy Markdown
Contributor Author

直しました。head は 40ac4a81 です。

検証の失敗も失敗として返します

ご指摘のとおり、errors に積むだけで ok: true へ到達していました。集めておいて判定に使っていない状態です。

集めた診断を判定の入力にし、一件でもあれば失敗とするようにしました。getComputedStyle を throw する関数へ差し替えたページ(iframe 内)で render_story が失敗することを回帰テストで固定しています。修正前は PNG 取得まで成功していたことも確認しました。

三度目だったので、表を作りました

この PR で同じ形の見落としを三度やっています。

  • 静止の失敗を握りつぶしていた
  • 結果の解析の失敗を握りつぶしていた
  • 検証自体の失敗を握りつぶしていた(今回)

いずれも「前の層の失敗を塞げ」と言って足した層で、その層自身の失敗経路を数えていませんでした。層を足すたびに、その層自身の失敗経路が素通りとして生まれるという形です。

個別に潰しても四度目が別の場所から出るので、各層について「失敗を検知できるか」「検知したらどう倒れるか」「検知できないならなぜか」を表にして、browser.rs のモジュール doc へ残しました。次に層を足す人が読める場所に置いてあります。

脅威モデルについて

ご指摘の「JSON.stringify の上書きを検証対象としている以上、同じ main world の組み込み関数上書きも除外できない」は筋が通っていると考えます。

加えて、悪意を想定しなくても detached な document やクロスオリジンで getComputedStyle が投げる経路はあるので、事故モデルのままでも塞ぐ理由が立ちます。README の該当箇所を整理しました。

正当に投げうる場面で、静止できているのに失敗と判じないかも確かめています。

現在 34 件のブラウザテストが通り、CI も 5 workflow すべて緑です。改めてご確認をお願いします。

@yupix yupix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

レビュー結果: 5 件(重大度順)。fail-open を潰す設計意図・テストの網羅は十分に見えるので、指摘は境界条件と一貫性が中心。詳細は各インラインコメント参照。

Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
…nimation collection

- FREEZE_SCRIPT の evaluate を story_timeout の tokio::time::timeout に載せる。
  rAF を発火させないページ(コールバックを保持して呼ばない差し替え)では
  promise が解決せず evaluate が返らない——時間内に静止が終わらねば
  fail-closed で失敗を返す(READY 待ちの deadline と同じ手当てを後から
  足した層へ移す)
- freezeRoot / collectRunning の収集を共通の collectAnimations へ。root 側
  getAnimations API の欠落は黙って [] へ倒さず errors に積んで判定へ反映
  (shadow 内擬似要素の running 見逃しの fail-open を閉じる)
- 無限アニメの 0 巻き戻しも型を保つ: progress-based timeline では
  CSSUnitValue(0, unit) を渡し、infinite + scroll() の正当な story を
  TypeError で落とさない
- sweep 数の off-by-one を while 構造で解消(上限脱出時に 11 と報告していた)
- ループ後の二重 collectRunning を still の持ち上げで省略(最終巡から判定まで
  await 無し・タイムライン時刻はタスク内固定ゆえ同値)
- 層ごとの失敗経路表を実態へ更新し、「層ごとの手当て(横並び)」表を新設。
  README に停止性・infinite progress-based・収集 API 欠落の記述を追記

Assisted-by: multi-agent-shogun-aki-tweak
@sousuke0422

Copy link
Copy Markdown
Contributor Author

5 件とも直しました。head は 39b2de4 です。

① evaluate に上限を与えました

story_timeout に載せ、tokio::time::timeout で包みました。rAF が発火しないページでも時間内に失敗として倒れます。

このご指摘は、失敗経路表に「停止性は上位の CI ジョブタイムアウト頼み」と書いたのを読んで拾っていただいたものだと思います。表を残しておいてよかったです。表の該当箇所も実態へ更新しました。

なお回帰テストを書く過程で、rAF の stub でコールバックを捨てると V8 が promise を GC 回収してしまい、ハングを再現したつもりが偶発的に返ることが分かりました。fixture はコールバックの参照を保持する形へ直しています。

② 収集ロジックを共通化しました

freezeRoot 側にだけ per-element の fallback があり、後から足した collectRunning に移っていませんでした。同時に忘れたのではなく、時間差で足した層に先の手当てが移らなかった形です。

collectAnimations として共通化し、両層で使うようにしました。getAnimations が無い場合も errors へ積んで判定に反映します。観測できる失敗を捨てないという、この PR で決めた方針と揃えました。

縦(層ごとの失敗経路)の表だけでは層をまたぐ漏れが見えないので、横(どの層がどの手当てを持っているか)の表も並べています。

③ 過剰に落としていた分を直しました

animation-timeline: scroll() かつ無限回のアニメーションで、型を保たない巻き戻しをしていたため常に失敗していました。CSSUnitValue を渡す形へ直しています。

fail-closed へ倒す方向ばかり見ていて、正当なものを落とす側を拾えていませんでした。

④⑤

sweeps の off-by-one は構造から直しました。失敗メッセージは後から証拠として読まれるので、実際の巡回数と一致させています。

二重の collectRunningstill を持ち上げて省きました。「await が無いので playState は変わらない」という前提が将来崩れないよう、根拠をコメントに残しています。

現在 38 件のブラウザテストが通り、CI も 5 workflow すべて緑です。改めてご確認をお願いします。

@yupix yupix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multi-agent review(8 角度 + 相互検証)の結果です。インラインに 7 件。

重要度高

  • freeze evaluate の timeout が story_timeout をフル予算で取り直す二重取り(browser.rs:895)
  • CSS 適用検証が root ごと先頭 1 要素のみで fail-open / 誤 fail-closed の両方向あり(browser.rs:501)
  • 新しい freeze 失敗クラスがビルド全落ちになる点が移行ノート未記載(README)

提案・nit: 最低 2 巡コメントの根拠 / per-element getAnimations の冗長 / テスト重複 2 系統 / prefers-reduced-motion エミュレーション

参考: 検討のうえ棄却した候補 — finiteEnd の CSSMathValue 誤分類(Blink は endTime に CSSUnitValue しか構築しない)、paused アニメの 0 巻き戻し(README / module doc に文書化済みの意図的挙動)、inactive scroll timeline での number 代入 TypeError(endTime 側の unit で救済される)。より大掛かりな設計代替(isolated world での実行、CDP Animation domain、virtual time)は今回の設計判断の範囲内と見て個別指摘にはしていません。

Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs
Comment thread README.md
Comment thread apps/backend/crates/service/src/render/browser.rs
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/service/src/render/browser.rs
…d stylesheets, share the story deadline

- render_all: story 固有の失敗(RenderError::Story / Timeout)はその story
  だけをエラーにし、残りの story は撮り続ける。全 story 処理後に失敗を
  列挙してビルドを failed へ(fail-closed は維持——欠けた story のまま
  比較へ進めると差分ゼロに見える偽 PASS になる)。環境側の失敗
  (Cdp / Launch / Server・名前規則違反)は従来どおり即中断
- 静止 CSS の注入を <style> appendChild から constructed stylesheet
  (new CSSStyleSheet + replaceSync + adoptedStyleSheets)へ。CSSOM は
  CSP style-src の管轄外なので、CSP 起因の適用失敗が構造的に消える。
  README の unsafe-inline 案内も撤去。構築 API の欠落・throw は
  errors → ok:false(fail-closed)
- CSS 適用検証を --vrt-frozen プローブへ。旧・先頭 1 要素の
  caret/transition 値検査は両方向に破れていた——既定値 '0s' との偶然一致で
  素通し(fail-open)し、先頭要素の利用者 !important でハード失敗
  (README の best-effort 契約と食い違う誤 fail-closed)。プローブは
  既定値を持たず、利用者の上書きにも反応しない
- FREEZE evaluate の時間上限をフル予算から started + story_timeout の
  残余へ(READY 待ちと共有の deadline)。時間切れの分類も READY 側と同じ
  RenderError::Timeout へ統一(phase フィールドで段を区別)
- collectAnimations: root 側 getAnimations 成功時は per-element 走査を
  省略(Chromium では subtree 全体を返すため全要素ぶんの no-op だった)。
  欠落・throw 時だけフォールバック(fail-closed は維持)
- 2 巡目の根拠コメントを実態へ(注入 CSS は animation を誘発しない。
  価値は seek 由来の animationend/transitionend 連鎖と遅延開始の捕捉——
  0 件時スキップという誤最適化の芽を断つ)
- テスト: readiness ポーリング 4 箇所を wait_until へ抽出(timeout 時は
  break でなく panic)。fake channel スキャフォールドを write_story_html
  へ共通化(プロトコルを壊す CSP/garbled/throwing/rAF 系は手書き維持)。
  strict-CSP は「fail-closed で落ちる」から「CSP 下でも静止が効く」検証へ
  反転し、透明キャレット対照との一致で効果を実測。CSSStyleSheet 欠落の
  fail-closed と deadline 共有の実測テストを追加

Assisted-by: multi-agent-shogun-aki-tweak
@sousuke0422

Copy link
Copy Markdown
Contributor Author

7 件とも対応しました。head は 6549c393 です。

ビルド全落ちを断ちました(ご指摘のうち最も実害が大きい点)

render_all? 伝播を切り、story 固有の失敗(RenderError::Story / RenderError::Timeout)は build_logs へ記録して次の story へ進むようにしました。静止できない story が一つあっても、残りのスクリーンショットは撮れます。

環境側の失敗は従来どおりビルドを中断します。隔離しても同じ理由で全滅するだけなので。

ビルドの結果自体は失敗のままにしています。欠けたまま比較へ進めると、撮れていない story が「変化なし」として通ってしまうためです。この残る巻き添えは README とコードコメントの両方に書きました。

per-story の opt-out は設けませんでした。「この story は静止しなくてよい」と言える口は、そのまま差分を見逃す口になります。

「何が落ちるか」は数えていましたが、「一つ落ちると何が巻き添えになるか」を数えていませんでした。

constructed stylesheet へ移しました

ご提案どおりです。CSSOM 操作は style-src の管轄外なので、CSP が原因で注入が拒否される状態そのものが起きなくなりました。README の unsafe-inline の案内も落としています。

検証して落とすより、起きなくする方が強いと判断しました。

先頭 1 要素だけを見ていた検証は、--vrt-frozen のプローブに置き換えて両方向の破れを直しています。transition-duration が既定で '0s' に算出されるため素通ししていた件も、先頭要素にだけ利用者の !important があるとハード失敗していた件も、これで解消です。!important に勝てない点は変わらないので、README の best-effort という位置づけとも揃いました。

timeout を共有 deadline の残余にしました

フル予算を取り直していたので、最悪 2×story_timeout になっていました。started + story_timeout の残余を渡す形に直し、コメントの「READY 待ちと同じ手当て」が実態と一致するようにしています。

エラー分類も RenderError::Timeout へ揃えました。時間切れが Story として報告されるのは非対称でした。

残りの 4 件

2 巡目の根拠コメントは、animationend / transitionend 由来の連鎖と double-rAF の間の遅延開始を捕捉する点へ寄せました。「誘発しないなら 2 巡目は不要では」という誤読を招く書き方でした。

getAnimations() の per-element ループは、root 側が欠落・throw した場合だけのフォールバックにしました。

readiness ポーリングは wait_until に抽出し、タイムアウト時に無言で break していた 3 箇所も揃えています。fixture の共通スキャフォールドもフォーマッタに寄せました。プロトコルを意図的に壊す 5 件は手書きのまま残しています。

触れていないもの

prefers-reduced-motion のエミュレーションは、撮る絵そのものが変わる判断になるので保留しています。行儀の良いコンポーネントで母集団が減る利点と、rAF / canvas に届く点は理解しているので、別途検討します。

検討のうえ棄却されたとお書きいただいた 3 件にも触れていません。

現在 40 件のブラウザテストが通り、CI も 5 workflow すべて緑です。改めてご確認をお願いします。

@yupix yupix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

レビュー結果: 重大なバグは無し。隔離設計の趣旨と食い違う分類が 2 点、README の契約の穴が 1 点。詳細はインラインコメント参照。

Comment thread apps/backend/crates/service/src/render/browser.rs Outdated
Comment thread apps/backend/crates/job/src/render_build.rs Outdated
Comment thread README.md
The freeze evaluate mapped CDP errors straight to RenderError::Cdp, which
render_build classifies as infrastructure and aborts the whole build — while
the READY probe retries the same class of error until the shared deadline.
Two story-caused paths hit this asymmetry (both measured): a story that
navigates/reloads during the freeze ("Inspected target navigated or closed")
and a story that discards rAF callbacks so the pending promise is garbage-
collected ("Promise was collected", the very behavior recorded in the
raf-suppressed fixture note). Retry the freeze evaluate like the READY
probe and fall to the same story-scoped Timeout at the deadline.

Screenshot-name rule violations are story content too (title/name), so
collect them as story failures instead of aborting at the first one —
several violating stories are now all reported in a single build.

Add the third module table classifying every failure path as story-scoped
vs environment, and extend the README "out of reach" contract with the
time-varying mechanisms getAnimations() cannot see (verified against
Chromium): animated images, media playback, SVG SMIL, in-flight smooth
scroll, marquee, UA-shadow built-ins, and one-shot load-time changes
(lazy images, font swap, content-visibility). @Property animations and
same-document view transitions were verified to be enumerable, and are
called out as reachable.

Assisted-by: multi-agent-shogun-aki-tweak
@yupix
yupix merged commit aa78ad2 into main Aug 10, 2026
5 checks passed
@yupix
yupix deleted the feat/render-caret-anim-freeze branch August 10, 2026 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants