fix: インポート/バックアップ復元と sync ストレージ書き込みの不具合修正

[ujiro99]

概要

設定のインポートがうまく動作しない不具合の調査に伴う、リファクタリングと関連不具合の修正です。

変更内容

refactor: ImportExport を service / hook / view に分割

• React に依存しないインポート・エクスポート・バックアップ処理を services/settings/importExport.ts へ移動
• 状態保持とサービス呼び出しのみを行う hooks/option/useImportExport.ts を追加
• components/option/ImportExport.tsx はレンダリングのみを担当
• importExport.ts のユニットテストを追加

fix: コマンドが0件のバックアップを復元候補から除外

• commands: [] のバックアップが選択可能なのに、復元すると必ず失敗していた
• コマンドが1件以上あるバックアップのみを「利用可能」と判定するよう変更

fix: sync ストレージの書き込み中に追加されたデータが失われる問題

• debouncedSyncSet は書き込み完了時にバッファを丸ごとクリアし、待機中の Promise をすべて resolve していたため、chrome.storage.sync.set の実行中に追加されたデータが書き込まれずに消え、呼び出し元にも成功が返っていた
• 書き込み開始時にバッファと resolve を切り離し、書き込み中の追加分は次のバッチで保存するよう修正
• 書き込みを直列化し、後のバッチが先のバッチより先に反映されないように変更
• 書き込み中の追加を再現するテスト（DS-10〜12）を追加

その他

• 110f1583 fix: fall back to default icon colors when resolveIconColors fails も未マージのため本 PR に含まれています

テスト

• yarn test（extension）: 全件パス
• tsc --noEmit: エラーなし

🤖 Generated with Claude Code

[claude]

レビュー結果

全体として、ImportExport の service/hook/view 分割はテスト容易性の観点で非常に良い設計です。特に services/settings/importExport.ts の各関数が純粋なロジックとして切り出され、importExport.test.ts（540行、IE-01〜IE-30）で網羅的にテストされている点、storage/index.ts の同期書き込み修正に対して並行書き込みのレースコンディションを再現するテスト（DS-10〜DS-12）が追加されている点は高く評価できます。AGENTS.md の規約（コメント英語、camelCase等）にも準拠しています。

気になった点

1. JSONパースエラー時のフィードバックが欠落（提案）
   packages/extension/src/hooks/option/useImportExport.ts:43-45

   const selectImportFile = async (file: File) => {
     setImportJson(await readSettingsFile(file))
   }

   不正なJSONファイルを選択した場合、readSettingsFile（importExport.ts:125-139）が reject し、呼び出し元の handleImportFileChange（ImportExport.tsx:51-54）も catch していないため、unhandled promise rejection となります。ユーザーには何のフィードバックもなく「インポートボタンが押せないまま」になってしまいます。importExport.test.ts の IE-12 で readSettingsFile 自体のreject挙動はテストされていますが、呼び出し側でのエラーハンドリングが抜けています。restore()（同ファイル60-64行目）と同様に try/catch であるいはエラーメッセージ表示を追加すると良いと思います。
   ※ この挙動自体は今回の変更前（旧 handleImport内の JSON.parse）から存在していた問題ですが、せっかく importExport.ts を切り出してテスト可能にしたので、このタイミングで直すと良さそうです。

2. エラーメッセージの i18n 未対応（軽微・既存踏襲）
   packages/extension/src/hooks/option/useImportExport.ts:60-63

   } catch (error) {
     console.error("Failed to restore from backup:", error)
     alert("Failed to restore from backup.")
   }

   同じ関数内の失敗パス（58行目）は t("Option_RestoreFromBackup_failed") を使っているのに対し、こちらはハードコードされた英語文字列です。旧コードからの踏襲ではありますが、i18n の一貫性の観点で t() 化を検討してもよさそうです。

3. sync書き込み失敗時もPromiseがresolveされてしまう（既存挙動・情報共有）
   packages/extension/src/services/storage/index.ts:37-44

   chrome.storage.sync.set(dataToSet, () => {
     if (chrome.runtime.lastError != null) {
       console.error(chrome.runtime.lastError)
     }
     resolves.forEach((res) => res())
     done()
   })

   chrome.runtime.lastError（QUOTA_BYTES超過など）が発生してもconsole.errorのみで、呼び出し元には成功として resolve されます。今回のPRの主目的（in-flightな書き込み中のデータ消失の修正）とは別軸の問題であり、旧実装から変わっていない挙動ですが、せっかくこの関数を書き直したので、書き込み失敗を呼び出し元に伝播させる（reject する）改善も今後検討の余地があるかもしれません（バックアップ機能全体の信頼性に関わる箇所のため）。

良かった点

• packages/extension/src/services/storage/index.ts:25-46 の flushSyncSet：バッチを即座にdetachし、syncSetLastWrite で書き込みを直列化する設計は、元のバグ（書き込み完了時に丸ごとクリア→書き込み中に追加されたデータの消失＆誤ったresolve）を的確に修正しています。DS-10〜12のテストがこの修正の意図を正確に検証できています。
• packages/extension/src/services/settings/importExport.ts:164-177 の toBackupEntry：commands.length > 0 によるフィルタで「復元できないのに選択可能」というバグを解消。IE-19でエッジケース（空配列バックアップ）もカバー。
• useSettingsWithImageCache.ts の resolveIconColors が null を返すケースへのフォールバック（74-134行目）も、Array.isArray チェックとテスト（US-30-a）が追加されており堅牢です。

テストカバレッジ

• yarn test は全件パス、tsc --noEmit もエラーなしとのことで問題なさそうです。
• パフォーマンス・セキュリティ面で目立った懸念はありませんでした（debouncedSyncSet はローカル拡張機能内のみの処理で外部入力を扱わないため）。

---

🤖 Generated with Claude Code

[claude]

コードレビュー結果

全体として、ImportExport.tsx の service/hook/view への分割、および debouncedSyncSet のデータロスト修正は、いずれも設計・実装ともに丁寧で、テストも充実しています。以下、気づいた点を記載します。

良い点

• packages/extension/src/services/storage/index.ts:35-48（flushSyncSet）: 書き込み開始時に syncSetData / syncSetResolves を新しいインスタンスに差し替えてから非同期書き込みに入る設計により、書き込み中に追加されたデータが失われる不具合を正しく解消しています。syncSetLastWrite によるチェーンで直列化しているため、後続バッチが先行バッチより先に反映される心配もありません。DS-10〜12 のテスト（packages/extension/src/services/storage/index.test.ts の該当箇所）で in-flight 書き込み中の追加ケースがきちんとカバーされている点も良いです。
• packages/extension/src/services/settings/importExport.ts:164-179（toBackupEntry）: backup.commands.length > 0 のチェックを追加したことで、「コマンド0件のバックアップを復元候補にしてしまう」不具合が解消されています。checkBackupStatus（182行目〜）でも legacy/daily/weekly に同じロジックを共通適用できており、リファクタ前に3箇所あった重複コードが解消されています。
• packages/extension/src/hooks/useSettingsWithImageCache.ts:82-134: resolveIconColors が配列以外（例: null）を返した場合に例外を投げるガード（99-101行目）と、失敗時に false へフォールバックする仕組み（117行目, 133行目）が明確なコメント付きで実装されており、意図が読み取りやすいです。queries が useMemo で安定した参照になっているため、failedQueries !== queries の参照比較（130行目）も正しく機能します。

気になった点（軽微）

1. テストカバレッジのギャップ: packages/extension/src/hooks/option/useImportExport.ts には reset（40-43行目）、runImport（55-58行目）、restore（60-70行目）という重要な関数がありますが、useImportExport.test.ts では selectImportFile のケース（UIE-01〜03）しかテストされていません。特に restore() の try/catch による alert(t("Option_RestoreFromBackup_failed")) 分岐や失敗時の console.error 呼び出し（useImportExport.ts:61-69）は、サービス層の restoreFromBackup 自体はテスト済み（importExport.test.ts IE-27〜30）でも、フック側のエラーハンドリング経路（location.reload() を呼ぶかどうかの分岐など）は未検証です。可能であれば useImportExport.test.ts に reset/runImport/restore のテストケースを追加することをおすすめします。

2. useImportExport.ts:26-32（useEffect 内の checkBackupStatus().then(...)）: コンポーネントがアンマウントされた後に Promise が解決した場合でも setBackupStatus / setSelectedBackupType を呼び出してしまいます。React 18 では警告は出ませんが、useSettingsWithImageCache.ts:89-127 で使われている active フラグのパターンと揃えて cleanup を入れると、より堅牢になるかもしれません。ただし、これはリファクタ前から存在していた挙動で、今回新たに持ち込まれた問題ではありません。

3. packages/extension/src/services/storage/index.ts:24-33（writeSyncBatch）: chrome.storage.sync.set が Promise を返すことを前提にした実装に変更されていますが、これはリポジトリ内の他の呼び出し箇所（例: commandStorage.ts:415, hub/background.ts:35 など）と同じ Promise ベースの API 呼び出しパターンに揃っているため、特に問題はないと判断しました（念のため確認した点として記載します）。

まとめ

debouncedSyncSet のデータロスト修正は本PRの中核となる不具合修正で、実装・テストともに妥当性が高いと判断します。上記の指摘は「必須の修正」ではなく、追加のフックレベルテストによる安心材料の積み増しを推奨する軽微な提案です。マージにあたって大きな懸念はありません。

---

🤖 Generated with Claude Code

[claude]

コードレビュー結果

fix/import-error の変更（ImportExport の service/hook/view 分割、debouncedSyncSet の書き込み直列化、useSettingsWithImageCache のフォールバック処理）を確認しました。

気になった点

1. packages/extension/src/hooks/useSettingsWithImageCache.ts:129
usePreservedIconColors 内で failedQueries !== queries と参照比較をしていますが、アイコン色解決のIPCが一度失敗した後、queries が内容は同じでも新しい配列参照として再生成されるケース（例: コマンドの編集・並び替えでメニューがマウントされたまま commands の参照が変わる場合）では、フォールバック済みの判定が無効化され null が返ります。Menu.tsx 側では loading を明示的に見ておらず、useSettingsWithImageCache の resolved は loading === true の間 commands/folders を [] にするため、一度フォールバックが機能していたのに再度IPC応答待ちでメニュー項目が一時的に消えるリグレッションが起こり得ます。内容ベースの比較（例: クエリ文字列の配列比較）にするか、failedQueries を安定した参照として保持する方法を検討すると良さそうです。

2. packages/extension/src/services/settings/importExport.ts:129 付近（readSettingsFile）
   FileReader.onload 内で e.target == null の場合に return するだけで resolve/reject のどちらも呼ばれていません。通常のブラウザでは発生しないケースですが、もし e.target が null になる環境（一部のテストハーネスやポリフィルなど）があると、呼び出し元の await readSettingsFile(file)（useImportExport.ts の selectImportFile）が永久にpendingのままになり、インポートダイアログが固まってしまいます。防御的にせよ、reject(new Error(...)) を呼ぶ方が安全です。

3. packages/extension/src/services/settings/importExport.ts:239 付近（createBackupManager の switch）
   BackupType に対する switch に default ケースがなく、TypeScript の tsconfig でも noImplicitReturns が有効になっていないため、将来 BACKUP_TYPES に値が追加された場合にコンパイルエラーにならず、実行時に undefined が返り restoreFromBackup(type).restoreFromBackup() で TypeError になります。エラーは useImportExport.ts の restore() で汎用的な alert("Failed to restore from backup.") に丸められるため、原因の切り分けが難しくなります。default: throw new Error(...) などで早期に検出できるようにすると安全です。

良かった点（参考）

• packages/extension/src/services/storage/index.ts の flushSyncSet：書き込み中に syncSetData/syncSetResolves を新しいインスタンスへ差し替え、syncSetLastWrite で直列化することで、元々あった「書き込み完了時に丸ごとクリアしてしまい、書き込み中に追加されたデータが失われる」バグを的確に修正しています。DS-10〜13のテストでこの意図がきちんと検証されています。
• toBackupEntry（importExport.ts）で commands.length > 0 のチェックを追加し、コマンド0件のバックアップを復元候補として選べてしまう不具合を解消している点も良い修正です。

テストカバレッジ

• コアの debouncedSyncSet 直列化修正と toBackupEntry の変更にはそれぞれ対応するテスト（storage/index.test.ts, importExport.test.ts）が追加されており、意図を正しく検証できています。
• 上記1〜3で指摘した異常系（IPC失敗後の再フェッチ、FileReader の異常系、BackupType 追加時の網羅性）は現状テストが薄いため、余裕があれば追加を検討してください。

---

🤖 Generated with Claude Code