Fix: Context menu does not work after browser launch.

[ujiro99]

close #254

[claude]

PR レビュー: コンテキストメニュー初期化タイミングの修正

この PR は、ブラウザ再起動後にコンテキストメニューが動作しない問題を修正しています。変更内容を確認しましたので、フィードバックを提供します。

✅ 良い点

1. 根本原因の正確な特定: onInstalled イベントは拡張機能のインストールや更新時にのみ発火し、ブラウザ起動時には発火しません。onStartup に移動することで、この問題が適切に解決されています。

2. Chrome Extension ライフサイクルへの準拠: Chrome の公式ドキュメントによると:

   • chrome.runtime.onInstalled: 拡張機能が初めてインストールされた時、拡張機能が新しいバージョンに更新された時にのみ発火
   • chrome.runtime.onStartup: ブラウザが起動するたびに発火（プロファイルが最初に起動した時）

   コンテキストメニューは各ブラウザセッションで必要なため、onStartup が適切な場所です。

3. 最小限の変更: 必要最小限のコード変更で問題を解決しており、リグレッションのリスクが低いです。

🔍 コード品質

変更内容:

• packages/extension/src/background_script.ts:431-433: ContextMenu.init() を onInstalled から削除
• packages/extension/src/background_script.ts:453-454: onStartup に ContextMenu.init() を追加（適切なコメント付き）

コードスタイルは CLAUDE.md のガイドラインに準拠しています（英語コメント使用）。

⚠️ 潜在的な問題と改善提案

1. テストカバレッジの不足

現在、background_script.test.ts には onStartup および onInstalled イベントハンドラーのテストがありません。以下のテストケースを追加することを推奨します:

describe("Chrome Extension Lifecycle Events", () => {
  it("should initialize context menu on browser startup", async () => {
    // Test that ContextMenu.init() is called when onStartup fires
  })
  
  it("should not initialize context menu on extension install/update", async () => {
    // Test that ContextMenu.init() is NOT called during onInstalled
  })
  
  it("should handle both onInstalled and onStartup correctly", async () => {
    // Test that backup functions and other setup work correctly
  })
})

場所: packages/extension/src/background_script.test.ts

2. 初期インストール時のコンテキストメニュー

現在の実装では、拡張機能の初回インストール時にコンテキストメニューが初期化されません（onInstalled からコードを削除したため）。これは意図した動作でしょうか？

推奨される対応:

もし初回インストール時にもコンテキストメニューを表示したい場合は、両方のイベントで初期化が必要です:

chrome.runtime.onInstalled.addListener((details) => {
  chrome.storage.session.setAccessLevel({
    accessLevel: "TRUSTED_AND_UNTRUSTED_CONTEXTS",
  })

  if (
    details.reason === chrome.runtime.OnInstalledReason.INSTALL ||
    details.reason === chrome.runtime.OnInstalledReason.UPDATE
  ) {
    // Set uninstall survey URL
    chrome.runtime.setUninstallURL(`${HUB_URL}/uninstall`)
    
    // Initialize context menu on first install or update
    ContextMenu.init()
  }

  // ... backup checks ...
})

chrome.runtime.onStartup.addListener(() => {
  console.debug("Service worker started")

  // Initialize context menu on browser startup
  ContextMenu.init()

  // ... backup checks ...
})

ただし、ContextMenu.init() は既にデバウンス機能を持っているため（contextMenus.ts:45-69）、重複呼び出しは安全に処理されます。

3. エッジケースの考慮

• 拡張機能の更新時: 現在の実装では、拡張機能を更新した際にコンテキストメニューが初期化されません。ユーザーが次回ブラウザを再起動するまで待つ必要があります。
• Service Worker の再起動: Manifest V3 の service worker は非アクティブ後に自動的に停止・再起動します。onStartup は最初のブラウザ起動時のみ発火するため、service worker の再起動時には発火しません。ただし、ContextMenu.init() は設定変更時にも呼ばれるため、これは問題にならないと思われます。

🔒 セキュリティ

セキュリティ上の懸念はありません。変更は内部的なイベントハンドラーの再配置のみです。

⚡ パフォーマンス

パフォーマンスへの影響は最小限です。むしろ、onInstalled での不要な初期化を避けることで、わずかに改善されています。

📝 推奨事項

1. 優先度: 高 - 初回インストール時のコンテキストメニュー動作を確認し、必要に応じて onInstalled でも ContextMenu.init() を呼び出す
2. 優先度: 中 - onStartup と onInstalled イベントハンドラーのユニットテストを追加
3. 優先度: 低 - PR の説明に、なぜ onStartup が適切なのかの技術的理由を追加

✨ 総評

この修正は、報告された問題に対する適切な解決策です。Chrome Extension のライフサイクルを正しく理解し、最小限の変更で問題を解決しています。上記の推奨事項を考慮すれば、マージして問題ありません。

推奨アクション: 承認（上記の推奨事項を検討後）

[codecov]

Codecov Report

❌ Patch coverage is 28.00000% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.24%. Comparing base (b75d05c) to head (d4ea360).
⚠️ Report is 30 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/extension/src/services/contextMenus.ts	14.28%	24 Missing ⚠️
packages/extension/src/background_script.ts	45.45%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #256      +/-   ##
==========================================
- Coverage   17.26%   17.24%   -0.02%     
==========================================
  Files         301      301              
  Lines       27885    27917      +32     
  Branches     1133     1135       +2     
==========================================
  Hits         4815     4815              
- Misses      23070    23102      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

コードレビュー: PR #256

このPRは、ブラウザ起動後にコンテキストメニューが動作しなくなる問題(#254)を修正するもので、Manifest V3のサービスワーカーライフサイクルに適切に対応した優れた実装です。

---

✅ 良い点

1. 根本原因の正確な理解と修正

• サービスワーカー再起動時にイベントリスナーが失われる問題を正しく特定
• メニュー構造（永続的）とイベントリスナー（非永続的）のライフサイクルの違いを適切に分離
• background_script.ts:503-511でトップレベルでのリスナー登録により、サービスワーカーロード時に常にリスナーが利用可能に

2. 責務の明確な分離

• ContextMenu.init(): メニュー構造の初期化のみに専念
• ContextMenu.syncCommandIdObj(): コマンドIDマッピングの同期
• トップレベルコード: リスナー登録
• この設計により、各関数の目的が明確で保守性が向上

3. 優れたドキュメント

• docs/design/context-menu.mdに詳細な設計思想を文書化
• サービスワーカーライフサイクルの説明が非常にわかりやすい
• 各イベント（onInstalled、onStartup）の役割が明確に記述されている

4. 適切なテスト更新

• contextMenus.test.ts:294-295でリスナー関連の不要なアサーションを削除
• 新しいアーキテクチャに合わせてテストを更新

---

🔍 潜在的な懸念点

1. 競合状態の可能性 (軽微)

場所: background_script.ts:503-511

;(async () => {
  try {
    await ContextMenu.syncCommandIdObj()
    chrome.contextMenus.onClicked.addListener(ContextMenu.onClicked)
  } catch (error) {
    console.debug("Failed to initialize context menu listener:", error)
  }
})()

懸念点:

• syncCommandIdObj()が完了する前にユーザーがメニューをクリックした場合、commandIdObjが空の可能性
• 確率は低いですが、サービスワーカー起動直後に発生する可能性

提案:
リスナー内でcommandIdObjの存在確認を追加することを検討してください：

onClicked: async (info, tab) => {
  if (tab && tab.id) {
    const command = ContextMenu.commandIdObj[info.menuItemId]
    if (!command) {
      console.warn("Command not found:", info.menuItemId)
      return
    }
    // ...既存のコード
  }
}

2. addMenus()の非同期処理 (軽微)

場所: contextMenus.ts:72

addMenus: (settings: SettingsType) => {

観察:

• 元々はasync関数でしたが、同期関数に変更
• createMenu()内のchrome.contextMenus.create()は非同期的にコールバックを実行

確認ポイント:

• ContextMenu.init()内でaddMenus()を呼び出していますが、メニュー作成の完了を待たない
• 現在のコールバックベースの実装で問題ないか確認済みであれば問題なし

---

🎯 セキュリティとパフォーマンス

セキュリティ

• ✅ セキュリティ上の問題は見当たりません
• ✅ エラーハンドリングが適切に実装されている
• ✅ console.debug()でのエラー出力も適切

パフォーマンス

• ✅ トップレベルでのリスナー登録は1回のみで、パフォーマンス影響なし
• ✅ syncCommandIdObj()は設定読み込みのみで軽量
• ✅ 再帰的なbuildCommandIdObj()も効率的

---

📊 テストカバレッジ

現状:

• 既存テストの更新は適切
• リスナー登録の分離に伴う不要なアサーション削除

提案:
以下のテストケースを追加することを検討してください：

1. syncCommandIdObj()のテスト

   • 正しくcommandIdObjが構築されることを確認
   • 階層構造でも正しくマッピングされることを確認

2. エッジケースのテスト

   • commandIdObjが空の状態でonClickedが呼ばれた場合
   • 設定が存在しない場合のsyncCommandIdObj()の動作

---

📝 まとめ

総合評価: ⭐⭐⭐⭐⭐ (5/5)

このPRは非常に高品質で、以下の点で優れています：

1. ✅ 問題の根本原因を正確に理解し修正
2. ✅ Manifest V3のベストプラクティスに準拠
3. ✅ コードの責務分離が明確
4. ✅ 優れた設計ドキュメント
5. ✅ 後方互換性を維持

推奨アクション:

• 上記の「競合状態の可能性」について、実際の影響を確認
• 影響がある場合は、onClicked内でのガード句追加を検討
• それ以外は マージ可能 と判断します

素晴らしい仕事です！ この修正により、ユーザー体験が大幅に向上します。🎉