Skip to content

refactor(csp): #176 B 案 PR 6 (scope 縮小) — styles.ts 削除 + migration tracker glob 化、Astro inline 残存は #289 へ委譲 - #290

Merged
fumtas1k merged 4 commits into
developfrom
feature/issue-176-b6-csp-flip-and-cleanup
May 7, 2026
Merged

refactor(csp): #176 B 案 PR 6 (scope 縮小) — styles.ts 削除 + migration tracker glob 化、Astro inline 残存は #289 へ委譲#290
fumtas1k merged 4 commits into
developfrom
feature/issue-176-b6-csp-flip-and-cleanup

Conversation

@fumtas1k

@fumtas1k fumtas1k commented May 7, 2026

Copy link
Copy Markdown
Owner

概要

#176 B 案 PR 6 を scope 縮小 で実施。当初は「public/_headers から style-src 'unsafe-inline' 撤廃 + cleanup」を予定していたが、実装中の npm run test:e2eAstro <element style="..."> 属性 65 件 / 15 ファイル が未移行で残存していたことが発覚 (CSP 違反 12 spec fail)。元の B 案 PR 1〜5b は React style={{...}} のみが対象で、Astro 側 inline 属性は scope 外だった。

→ 本 PR では styles.ts 削除 + migration tracker glob 化のみ に scope 縮小し、CSP flip / stripMetaStyleSrc 撤去 / decisions.md [067] は後続 PR に委譲する。残存 65 件の対応は新規 issue #289 で追跡。

詳細経緯: docs/superpowers/specs/2026-05-07-issue-176-b6-csp-flip-and-cleanup-design.md の冒頭 ⚠️ post-mortem section 参照。

変更内容

cleanup (本 PR の実施スコープ)

  • src/utils/styles.ts: 削除 (PR 1〜5b で全 import 元が CSS class 参照に置換完了済の orphan file)
  • src/utils/__tests__/inline-style-migration.test.ts: MIGRATED_FILES array (31 件) を await glob('src/components/**/*.tsx') に置換し全件カバー化
    • 件数 > 0 を陽性 assert で silent skip 防止
    • 陽性対照 (migration detector の陽性対照) は完全維持
    • 結果: glob で 50 件カバー (旧 array 31 件から拡張、新規 .tsx も自動で網に乗る)

設計記録 (post-mortem)

  • spec ヘッダに ⚠️ post-mortem section を追加し、Astro inline 残存発覚 → scope 縮小の経緯を historical record として明記
  • plan ファイルも descope note 付きで docs/superpowers/plans/ に保存

見送り (後続 PR に委譲)

  • public/_headersstyle-src 'self' 反転 (元 spec §7.1)
  • src/utils/csp.ts:PRODUCTION_CSP 同期 (元 spec §7.2)
  • astro.config.mjsstripMetaStyleSrc 撤去 (元 spec §7.3)
  • headers.test.ts / meta-csp.test.ts / astro-config-csp.test.ts の strict 化 (元 spec §7.6 / §7.7 / §7.8)
  • docs/decisions.md [067] 追加 (元 spec §7.9) — B 案完了時に書く

→ 全て #289 (Astro inline migration) 完了後の最終 flip PR で実施。

関連 PR / issue

  • 前提: B 案 PR 1〜5b 全 merged (#256#261#272#275#277#283#286)
  • 後続 (本 PR で起票): #289 Astro inline <element style="..."> 属性 65 件の CSS class 移行
  • 後続 (最終 flip): #289 完了後に別 PR で 'unsafe-inline' 完全撤廃 → #176 close

検証

  • npm run build && npm run test (vitest) green (750 tests)
  • npx astro check green (0 errors / 0 warnings)
  • npm run test:e2e (build + preview) green (146 passed / 1 skipped)
  • grep -rn --include='*.tsx' 'style={{' src/ = 0 件 (PR 5b 時点から維持)
  • grep -rn '@/utils/styles' src/ tests/ = 0 件
  • migration tracker glob で src/components/**/*.tsx 50 件カバー
  • CI: 全 required check + VRT green

ロールバック計画

cleanup commit (985f0c3) のみが本 PR のコード変更。revert すれば PR 5b マージ済の状態に戻る。styles.ts は git history に保存されているため復元可能。

メモリ参照 (進捗 doc 更新)

post-merge で docs/projects/issue-176-b-plan-progress.md の PR 6 列を「✅ scope 縮小 merge」更新する別 chore PR を予定 (feedback_followup_routing.md)。

fumtas1k added 3 commits May 7, 2026 22:33
`style-src 'unsafe-inline'` 撤廃 + 暫定 strip integration / styles.ts /
migration tracker 削除を含む B 案最終 PR の設計書。9 ファイル変更内容を
diff 形式で詳述、decisions.md [067] エントリの確定文面を draft、
リスク 6 件 + ロールバック計画 + PR description テンプレートを含む。

実装は本セッションでは行わず、次セッションで writing-plans skill 経由で
plan を起草してから着手する。
- src/utils/styles.ts: 削除 (PR 1〜5b で全 import 元が CSS class 参照に置換完了)
- src/utils/__tests__/inline-style-migration.test.ts:
  - MIGRATED_FILES array (31 件) を await glob('src/components/**/*.tsx') に置換し全件カバー化
  - 件数 > 0 を陽性 assert で silent skip 防止
  - 陽性対照 (migration detector の陽性対照) は完全維持
PR 6 実施中の npm run test:e2e で Astro <element style="..."> 属性 65 件 /
15 ファイルが未移行で残存していたことが判明 (CSP style-src 'self' 違反 12 spec)。
B 案 PR 1〜5b は React style={{...}} のみが対象で Astro 側 inline 属性は
scope 外だった (Astro <style> block の scoped CSS とは別物)。

本 PR 6 は scope を styles.ts 削除 + migration tracker glob 化のみに縮小し、
CSP flip / stripMetaStyleSrc 撤去 / decisions.md [067] は後続 PR に委譲する。
spec ヘッダに ⚠️ post-mortem section を追加、plan も historical record として
descope note 付きで保存。
@fumtas1k

fumtas1k commented May 7, 2026

Copy link
Copy Markdown
Owner Author

レビュー(多角的評価)

scope 縮小判断(flip 見送り)と post-mortem の記録は妥当だと思います。実コード変更は styles.ts 削除と test の glob 化のみで小さく、reviewability も高い。以下は確認結果と気になった点。

✅ 確認できた点

  • styles.ts の orphan 性: grep -rn "@/utils/styles" src/ tests/ の本体 import = 0 件で削除安全(参照は docs と global.css のコメントのみ)。
  • node:fs/promisesglob: Node 22 で stable 化された API。package.jsonengines: ">=22.12.0" と整合。手元 (v22.18.0) でも typeof glob === 'function' を確認。
  • top-level await: vitest は vite-node 経由 ESM 実行のため module-level の for await は動作する。コメントの注釈通り。
  • silent skip 防止: expect(TARGET_FILES.length).toBeGreaterThan(0) の陽性 assert と describe.skipIf の併設は feedback_positive_control_for_gates ガイドに沿っており、glob が 0 件返した場合の偽陰性を防げる。陽性対照 (migration detector の陽性対照) も完全維持。
  • scope 一致: 旧 array は全件 src/components/** 配下、新 glob src/components/**/*.tsx で同等以上をカバー(決定は scope 拡張、後述)。
  • CSP 攻撃面: 本 PR は _headerscsp.ts:PRODUCTION_CSP も触らず、'unsafe-inline' は維持。security delta は dead code 削減(微減) + 回帰防止網強化(preparatory) のみで、新規リスク導入なし。Astro inline 65 件残存 → #176 B 案 follow-up: Astro <element style="..."> 属性 65 件の CSS class 移行 #289 で追跡という後続計画も明示されている。

🟡 マージ前に直したい点

1. docs/shared-agent-rules.md:143 が削除済 file を参照したまま残る

- React (`.tsx`): `src/utils/styles.ts` の `colors.*` をインラインスタイルで使用

本 PR で src/utils/styles.ts を削除する以上、この行は merge 直後から 「存在しない file へ import せよ」 という contradictory な指示になります。shared-agent-rules.md は新規 agent / contributor が真っ先に読む正本(CLAUDE.md 冒頭で参照必須と明示)なので、ここが嘘の状態で merge されると、

  • 新規 React tool 追加時に「@/utils/styles から colors を import」する旧 pattern が再導入される
  • migration tracker (本 PR で強化された glob) は style={{ の有無しか見ないため、<div className={...} style={{ color: colors.primary }}> 風の hybrid が混入しても検知できない

の二重失敗を招きます。本 PR scope 内で 7 章 §139–144 を CSS class 参照(text-primary / bg-subtle 等)に書き換えるのを推奨。flip まで描かないなら「PR 1〜5b で migration 完了、新規 React component は text-primary 等 utility class を使う」という現状ベースの記述で十分。

2. test ファイル冒頭コメントの decisions.md [067] 参照が pre-mature

 * 参照: docs/decisions.md [067] (B 案完了の記録)

[067] は本 PR では descope されており、最終 flip PR で追加される予定(PR 本文の §7.9 委譲リスト通り)。merge 直後は 存在しないエントリへの参照 になります。参照: docs/decisions.md [067] (B 案完了時に追加予定) 等に弱める、または最終 flip PR まで参照行を入れない方が、historical record として一貫します。

🟢 nit(任意)

  • PR 本文の件数: 「glob で 50 件カバー」とありますが、pr-290 ブランチで git ls-tree -r pr-290 --name-only | grep '^src/components/.*\.tsx$' | wc -l = 40 件(うち __tests__/*.test.tsx 8 件)。実害は無いですが、CI のログ assertion にも乗る数字なので post-merge で進捗 doc 更新する際は 40 で記録した方が無難。
  • scope 拡張の副作用: 旧 array は production component のみだったのに対し、新 glob は __tests__/*.test.tsx 8 件を含みます。現状どの test も style={{ を含まないので問題なし。ただし将来「component の inline style 挙動を test するため意図的に style={{}} を含む test fixture を書きたい」ケースで false positive を踏みます。記録のみ(修正不要)。
  • src/styles/global.css:186legacy bodyEmphasis / caption from src/utils/styles.ts コメントも dead reference になりますが breadcrumb として残しても害は薄い。気になれば一緒に落とせる範囲。

まとめ

scope 縮小判断と post-mortem の透明性は良好で、test の glob 化は migration 完了後の保守性として正しい方向性です。🟡 #1shared-agent-rules.md は merge 前に直したい (新規 contributor の地雷化を防ぐため)。#2 は弱い指摘で、scope 内・後続 PR どちらでも吸収可能。

マージ条件としては shared-agent-rules.md の 1 行修正だけで十分(再 e2e は不要、docs のみ)。


🤖 Generated with Claude Code

….ts 参照を意味クラス案内に更新

PR #290 self-review 🟡 #1 / #2 対応:

- docs/shared-agent-rules.md §7: `src/utils/styles.ts` の `colors.*` インラインスタイル指示
  を、@layer components 意味クラス使用に書き換え。本 PR で styles.ts 削除済のため、削除前の指示
  は dead reference になり、新規 contributor が `@/utils/styles` を再導入する地雷化を防ぐ。
  Astro 側は #289 で migration 進行中の旨を併記
- src/utils/__tests__/inline-style-migration.test.ts JSDoc: `[067]` (本 PR で descope 済) への
  参照を「B 案完了時に追加予定」に弱め、historical 一貫性を確保

global.css:186 のコメント参照は reviewer 判断「breadcrumb として残しても害は薄い」に従い
据え置き。
@fumtas1k

fumtas1k commented May 7, 2026

Copy link
Copy Markdown
Owner Author

レビューありがとうございます。指摘対応を 7006856 で push しました。

対応

据え置き

  • 🟢 ロットの文字数が多いと、表記が見切れる #3 (src/styles/global.css:186legacy bodyEmphasis / caption from src/utils/styles.ts コメント): 「breadcrumb として残しても害は薄い」との指摘どおり据え置きました。
  • 🟢 PR 本文の「50 件カバー」: 実態 40 件(test 8 件含む)。コード変更ではなく PR description の記述ずれなので post-merge の docs/projects/issue-176-b-plan-progress.md 更新時に 40 で記録します。
  • 🟢 scope 拡張で __tests__/*.test.tsx 8 件が glob 対象に: 現状全 test が style={{ を含まないため pass。将来 fixture で意図的に style={{}} を含めたい場合は false positive を踏むため、その時点で glob exclusion か別 path 切替を検討します。

CI

merge 判断は reviewer 側にお返しします。追加指摘あればお知らせください。

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

🖼️ Visual Regression Test 結果

  • Status: ✅ 全 36 件 pass
  • Workflow run: 25502104320
  • Artifact (diff 画像 / playwright-report): 上記 workflow run の Artifacts セクションから download

diff が 意図的な visual 変更の場合: Update Visual Regression Baseline workflow を本 PR ブランチで workflow_dispatch trigger して baseline を更新。
diff が 意図しない regression の場合: 該当変更を fix。
本 check は required ではないため fail のままでも merge は可能(reviewer 判断)。

@fumtas1k

fumtas1k commented May 7, 2026

Copy link
Copy Markdown
Owner Author

再レビュー (commit 7006856 確認)

両指摘とも反映されているのを確認しました。LGTM です。

✅ 修正確認

🟡 #1: docs/shared-agent-rules.md:143-144

-- React (`.tsx`): `src/utils/styles.ts` の `colors.*` をインラインスタイルで使用
-- Astro (`.astro`): `var(--color-*)` を `style` 属性または `<style>` ブロックで使用
+- React (`.tsx`): `src/styles/global.css` の `@layer components` で定義された意味クラス
+  (`text-primary` / `bg-subtle` / `alert-success` 等) を `className` で使用 …
+  (`#176` B 案で `style={{ color: colors.primary }}` 形式は全廃済、`@/utils/styles` import も無効)
+- Astro (`.astro`): 現状 `var(--color-*)` を `style` 属性で書く箇所が残存
+  ([#289] で CSS class 化を進行中) …

検証:

  • text-primary / bg-subtle / alert-success の 3 例とも src/styles/global.css に実在 (grep -nE '^\s*\.(text-primary\|bg-subtle\|alert-success)\b' で line 213 / 383 / 388 を確認)
  • @/utils/styles import も無効」の明記で再導入の地雷化を予防
  • Astro 側は #176 B 案 follow-up: Astro <element style="..."> 属性 65 件の CSS class 移行 #289 進行中である現状を正確に記述、新規は React と同じ意味クラス推奨と方向性も一致

参考までに 1 点だけ awareness — feedback_tailwind_v4_layer_variant.md にある通り、@layer components 内手書きクラスは hover: / focus: variant を silent で無視します。新規意味クラス追加時に hover が必要なら :hover 擬似クラスごと layer 内で書く必要があります。既存 text-link:hover (line 178-181 の通常 layer) や .bg-subtle の hover 状態の運用パターンに沿えば OK で、本 PR の責務外。

🟡 #2: inline-style-migration.test.ts JSDoc

- * 参照: docs/decisions.md [067] (B 案完了の記録)
+ * 参照: docs/decisions.md (B 案完了時に [067] エントリ追加予定。
+   flip + 完了記録は #289 Astro inline migration 完了後の最終 PR で実施)

descope された [067] への pre-mature 参照が「追加予定」に弱められ、historical 一貫性が確保された。

global.css:186 のコメント据え置きについて

legacy bodyEmphasis / caption from src/utils/styles.ts のコメント参照は reviewer 判断「breadcrumb として残しても害は薄い」に従い据え置き。

合意。class 名 (.body-emphasis / .caption) と元の bodyEmphasis / caption 識別子の対応を辿るための breadcrumb として正しく機能しており、削除する利益が薄い。

結論

✅ 本 PR (scope 縮小版) は merge OK の状態と判断します。CI green を待って merge へ進めて問題ないと思います。

次の最終 flip PR (#289 完了後) では、本 PR で追加された glob ベース回帰防止網が _headers'unsafe-inline' 削除前の最後のセーフティネットとして機能するはずなので、その PR でも inline-style-migration.test.ts の実行が green であることを必須ゲートに含めるのを推奨します。


🤖 Generated with Claude Code

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.

1 participant