Feat/download button

[ujiro99]

新しいSelection Commnad Hubのダウンロードボタンに合わせたダウンロードボタン押下時の処理を実装する

[claude]

コードレビュー

概要

このPRは、Selection Command Hubとの通信方式を刷新するものです。従来の DownloadButton.tsx・StarButton.tsx コンポーネントおよび useDetectUrlChanged.ts フックを削除し、新しい useCommandHubBridge.ts フックに統合しています。Hub側からの postMessage を受信してコマンドの追加・削除を行い、Ack メッセージで応答する双方向通信に切り替えています。

---

問題点・改善提案

🔴 セキュリティ上の懸念

window.postMessage のターゲットオリジンに "*" を使用している

packages/extension/src/hooks/useCommandHubBridge.ts の2箇所で、window.postMessage(..., "*") を使用しています。

• L731–737 (コマンド変更時の proactive push):

window.postMessage(
  { action: "SyncInstalledCommand", installedIds: commands.map((c) => c.id) },
  "*",
)

• L785–787 (RequestInstalledCommand 応答):

window.postMessage(
  { action: "SyncInstalledCommand", installedIds: ids },
  "*",
)

"*" を指定すると、同一ウィンドウ内に読み込まれている任意の iframe やスクリプトがこのメッセージを受信できます。インストール済みコマンドのIDリストが外部ページに漏洩する可能性があります。

RequestInstalledCommand への応答については、event.source と event.origin が既に取得できているため、以下のように修正することを推奨します:

} else if (action === "RequestInstalledCommand") {
  const ids = commandsRef.current?.map((c) => c.id) ?? []
  ;(event.source as WindowProxy)?.postMessage(
    { action: "SyncInstalledCommand", installedIds: ids },
    { targetOrigin: event.origin },  // hubOrigin を指定
  )
}

proactive push（L731）については Hub ウィンドウの参照がないため "*" を避けるのが難しいですが、コメントで意図を明記することを推奨します。

---

🟡 getOrCreateClientId() の重複呼び出し

packages/extension/src/hooks/useCommandHubBridge.ts の AddCommand ハンドラ内で、getOrCreateClientId() が .then() と .catch() の両方で呼ばれています（L749, L761）。

.then(async (res) => {
  const install_id = await getOrCreateClientId()  // L749
  // ...
})
.catch(async () => {
  const install_id = await getOrCreateClientId()  // L761
  // ...
})

getOrCreateClientId() は非同期処理でストレージを読み書きするため、事前に一度だけ取得してから分岐させる方が効率的です:

if (action === "AddCommand") {
  if (typeof command !== "string") return
  const install_id = await getOrCreateClientId()
  Ipc.send(BgCommand.addCommand, { command })
    .then(async (res) => {
      ;(event.source as WindowProxy)?.postMessage(
        { action: "AddCommandAck", result: !!res, install_id },
        { targetOrigin: event.origin },
      )
      // ...
    })
    .catch(async () => {
      // ...
    })
}

ただし handleMessage 自体が非同期関数ではないため、await を使う場合は関数を async にする必要があります。

---

🟡 analytics イベントの id が未定義の可能性

packages/extension/src/hooks/useCommandHubBridge.ts L744 のデストラクチャリング:

const { action, command, id } = event.data ?? {}

AddCommand メッセージの仕様（L630付近のJSDoc）では id フィールドは定義されていませんが、L756 で:

await sendEvent(ANALYTICS_EVENTS.COMMAND_HUB_ADD, { id }, SCREEN.COMMAND_HUB)

と使用されています。id が undefined になるケースが想定されており、分析イベントのデータが欠落する可能性があります。command を JSON.parse して id を取得するか、送信側のインターフェースで id を明示する必要があります。

---

🟡 未使用の i18n キー

commandHub_add_success / commandHub_add_error は今回削除されましたが、旧 DownloadButton.tsx で使用されていた commandHub_delete_success / commandHub_delete_error は全ロケールに残っています（例: packages/extension/public/_locales/ja/messages.json）。

新しい実装ではトースト通知を Hub 側で管理するため、これらのキーも不要になっているはずです。不要であれば削除することを検討してください。

---

🟢 良い点

• commandsRef パターン (useCommandHubBridge.ts L723–726): commands の変更のたびにメッセージリスナーを再登録せずに最新値を参照できる、適切なパターンです。
• proactive push (useCommandHubBridge.ts L729–738): Hub がインストール済みIDを常に把握できるよう、コマンド変更時に自動で同期する設計は合理的です。
• JSDoc コメント: APIの仕様が詳細に文書化されており、外部との連携インターフェースが明確です。
• コンポーネントの簡素化: CommandHub.tsx が副作用専用フックのみを呼ぶ形に整理され、責務が明確になっています。

---

テストカバレッジ

新規の useCommandHubBridge.ts フックに対するユニットテストが追加されていません。特に以下のケースはテストがあると安心です:

• 不正オリジンからのメッセージが無視されること
• AddCommand / DeleteCommand / RequestInstalledCommand のそれぞれの正常系・異常系
• commands 変更時に SyncInstalledCommand が送信されること

[codecov]

Codecov Report

❌ Patch coverage is 0% with 88 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.77%. Comparing base (c0ef39e) to head (c499552).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...ackages/extension/src/hooks/useCommandHubBridge.ts	0.00%	84 Missing ⚠️
...extension/src/components/commandHub/CommandHub.tsx	0.00%	3 Missing ⚠️
packages/extension/src/services/analytics.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
+ Coverage   26.61%   26.77%   +0.15%     
==========================================
  Files         314      312       -2     
  Lines       31929    31742     -187     
  Branches     1684     1682       -2     
==========================================
  Hits         8498     8498              
+ Misses      23431    23244     -187     

☔ 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.

[Copilot]

Pull request overview

新しい（Hub 側UI/ボタン挙動に合わせた）Command Hub 連携として、従来の DOM 直付けクリック監視・URL変更監視を廃止し、Hub からの postMessage を受けてコマンド追加/削除を行うブリッジに置き換えるPRです。

Changes:

• Hub との連携を useCommandHubBridge（postMessage ベース）に集約し、CommandHub はブリッジ起動のみ行う構成へ変更
• 旧実装（DownloadButton/StarButton、URL変更監視 hook）を削除
• install_id 返却のため getOrCreateClientId を export 化、不要になった i18n キーを削除

概要

• Hub ↔ 拡張間の通信を postMessage に寄せたことで、Hub 側の新しいダウンロードボタン挙動に追従しやすい構成になっています。

懸念点

• SyncInstalledCommand を postMessage(..., "*") で送っており、インストール済みID一覧の露出が大きい点（セキュリティ）
• 追加失敗時でも COMMAND_HUB_ADD が計測されうる点（計測の正確性）
• PR説明（新Hub）に対して origin 判定が HUB_URL 前提になっている点（意図どおり新Hubで動くか）

改善提案

• postMessage の送信先 origin を hubOrigin に限定（必要なら要求元 event.source へ返信）
• analytics は成功時のみ送信
• Hub 側が送る payload 仕様（id の有無など）と実装/コメントを一致させる

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
packages/extension/src/services/analytics.ts	getOrCreateClientId を外部利用可能に変更
packages/extension/src/hooks/useDetectUrlChanged.ts	URL変更監視 hook を削除（旧Hub DOM監視ベース）
packages/extension/src/hooks/useCommandHubBridge.ts	Hub からの postMessage を受けて add/remove/sync を行う新ブリッジを追加
packages/extension/src/components/commandHub/StarButton.tsx	旧スター連携（DOM直付け）を削除
packages/extension/src/components/commandHub/DownloadButton.tsx	旧ダウンロード連携（DOM直付け + toast）を削除
packages/extension/src/components/commandHub/CommandHub.tsx	Button コンポーネントを廃止し、ブリッジ hook 呼び出しへ変更
packages/extension/public/_locales/zh_CN/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/ru/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/pt_PT/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/pt_BR/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/ms/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/ko/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/ja/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/it/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/id/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/hi/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/fr/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/es/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/en/messages.json	追加成功/失敗の文言キーを削除
packages/extension/public/_locales/de/messages.json	追加成功/失敗の文言キーを削除

[claude]

コードレビュー

概要

DownloadButton.tsx・StarButton.tsx・useDetectUrlChanged.ts を削除し、新しい useCommandHubBridge.ts フックへ統合した大きなリファクタリングです。postMessage ベースの双方向通信（AddCommandAck / DeleteCommandAck / SyncInstalledCommand）を追加し、Hub 側がインストール状態を把握できるようになっています。全体的な方向性は良いですが、いくつか指摘点があります。

---

🔴 重大な問題

1. オリジン検証が旧 Hub URL を参照している可能性

packages/extension/src/hooks/useCommandHubBridge.ts:125

const hubOrigin = new URL(HUB_URL).origin

HUB_URL は旧ハブ (ujiro99.github.io/selection-command) を指しています。PR 説明に「新しい Selection Command Hub のダウンロードボタン」とあるにもかかわらず、新ハブ (NEW_HUB_URL = selection-command-hub.pages.dev) のオリジンと一致しません。

もし新ハブからの postMessage を受け取る想定であれば、オリジン検証が常に失敗し、AddCommand / DeleteCommand が一切機能しない可能性があります。hubShare.ts:107 では NEW_HUB_URL でオリジンチェックしている点と整合性を確認してください。

---

🟠 バグ

2. AddCommand 失敗時にもアナリティクスが送信される

packages/extension/src/hooks/useCommandHubBridge.ts:138-148

Ipc.send(BgCommand.addCommand, { command })
  .then(async (res) => {
    const install_id = await getOrCreateClientId()
    ;(event.source as WindowProxy)?.postMessage(
      { action: "AddCommandAck", result: !!res, install_id },
      ...
    )
    await sendEvent(   // ← res が false でも呼ばれる
      ANALYTICS_EVENTS.COMMAND_HUB_ADD,
      { id },
      SCREEN.COMMAND_HUB,
    )
  })

sendEvent は res の真偽にかかわらず .then() ブロック内で常に呼ばれています。旧コードでは if (res) { sendEvent(...) } と成功時のみ送信していました。失敗時もアナリティクスを送信したい意図であれば明示的なコメントが必要です。そうでなければ条件分岐が必要です。

3. id が undefined になる可能性

packages/extension/src/hooks/useCommandHubBridge.ts:127-130

const { action, command, id } = event.data ?? {}

AddCommand メッセージの仕様には action と command のみ定義されており、id フィールドは含まれていません。そのため sendEvent に渡す { id } は常に undefined になる可能性があります。Hub 側で id を明示的に送信する仕様であれば、JSDoc のメッセージ形式の定義にも追記してください。

---

🟡 セキュリティ

4. window.postMessage に "*" を使用している

packages/extension/src/hooks/useCommandHubBridge.ts:110-117

window.postMessage(
  {
    action: "SyncInstalledCommand",
    installedIds: commands.map((c) => c.id),
  },
  "*",   // ← 任意のオリジンに配信される
)

コンテントスクリプトから Hub ページへプロアクティブに送信する際、"*" を使うとページ上の任意のリスナー（悪意ある拡張機能やスクリプトなど）も受信できます。Hub URL をターゲットに指定する方がより安全です:

window.postMessage(payload, new URL(HUB_URL).origin)

一方、ACK 応答 (event.source.postMessage(..., { targetOrigin: event.origin })) は正しく実装されており問題ありません。

---

🟡 コード品質

5. ロケールキーの孤立（未使用文字列）

commandHub_delete_success / commandHub_delete_error が全言語の messages.json に残っていますが、ソースコード内で使用されているファイルが見当たりません（grep 結果がゼロ件）。新実装では toast 通知の代わりに postMessage ACK を返すため、これらキーは不要と思われます。

影響ファイル（16 言語すべて）:

• packages/extension/public/_locales/*/messages.json

6. StarButton 機能の削除について

StarButton.tsx が完全に削除されており、スターボタンのクリック処理・カウント更新ロジックも失われています。新ハブにスター機能がない場合は問題ありませんが、旧ハブでまだスター機能を提供している場合はリグレッションになります。意図的な削除であれば PR 説明への明記を推奨します。

7. CommandHub コンポーネントが空の Fragment を返す

packages/extension/src/components/commandHub/CommandHub.tsx

export const CommandHub = (): JSX.Element => {
  useCommandHubBridge()
  return <></>
}

フックのみを実行して空 Fragment を返すコンポーネントは機能的には問題ありませんが、useCommandHubBridge() を呼び出し元で直接使用することでこのコンポーネント自体を削除できる可能性があります。小さな設計上のポイントです。

---

✅ 良い点

• commandsRef パターンを使ってイベントリスナーを再生成せずに最新の commands を参照している設計は正しいです (useCommandHubBridge.ts:103-106)
• ACK 応答に targetOrigin: event.origin を指定しており、セキュアな postMessage 実装です
• SyncInstalledCommand のプロアクティブ push により、Hub 側がリクエストせずともインストール状態を把握できる設計は良いです
• getOrCreateClientId の export 化によりフック側から再利用できる点も適切です
• 旧来の deprecated なボタンクリックリスナー実装が整理されており、コードベースがシンプルになっています

---

最優先で確認すべき点: HUB_URL vs NEW_HUB_URL のオリジン検証の整合性（項目1）と、アナリティクス送信条件（項目2）です。