Feat/add page url

[ujiro99]

close #429

[claude]

コードレビュー: #430 Feat/add page url

現在選択中のページURL(%pageUrl)を各種アクションのURLテンプレートに埋め込めるようにする機能追加です。全体的な設計・テストの充実度は良いですが、実装とテストの不整合と思われる箇所、およびコンテキストによって location.href の意味が変わってしまうエッジケースを見つけたので報告します。

🔴 重要な指摘

1. バックグラウンド(Service Worker)経由の実行パスで location.href が意図しない値になる

• packages/extension/src/action/tab.ts:24、popup.ts、window.ts、backgroundTab.ts、pageAction.ts に pageUrl: location.href が追加されています。これらのモジュールは2つの異なるコンテキストから呼ばれます。
  • packages/extension/src/action/index.ts 経由(useCommandExecutor.ts → content script内)では location.href は正しく閲覧中のページURLになります。
  • しかし packages/extension/src/action/background.ts の actionsForBackground(Popup/Window/Tab/BackgroundTab/PageAction を含む)は background_script.ts:582 の execute(...) からService Worker上で直接呼び出されます。これはショートカットキー実行時、enableSendTab が false(対象タブが chrome:// やウェブストアページ、または content script へのIPC送信が失敗した場合)にフォールバックするパスです(background_script.ts:560-589)。
  • Service Worker内での location.href はページのURLではなく、Service Worker自身のスクリプトURL(chrome-extension://<id>/...)を指してしまいます。このフォールバックパスを通ると pageUrl が全く見当違いの値になります。
• 対応案: pageUrl を呼び出し元(content script側)で明示的に取得して ExecuteCommandParams に含めて渡すか、バックグラウンド実行時は chrome.tabs.query で取得した tab.url を使うようにすべきです。

2. defaultSettings.test.ts の DS-24 テストが実装と矛盾している

• packages/extension/src/services/option/defaultSettings.ts:402 では openMode: OPEN_MODE.POPUP(値は "popup")を設定しています。
• しかし追加されたテスト packages/extension/src/services/option/defaultSettings.test.ts:295 では expect((cmd as any).openMode).toBe("sidePanel") を期待しています。
• OPEN_MODE.POPUP = "popup"、OPEN_MODE.SIDE_PANEL = "sidePanel"(packages/shared/src/constants/open-mode.ts:5,9)なので、このテストは実装と一致せず失敗するはずです。「Search Commands on Hub」をポップアップで開く仕様なのか、サイドパネルで開く仕様なのか、意図を確認して実装かテストのどちらかを修正してください。

3. settings.test.ts の ST-34 / ST-34-a がマイグレーション処理を実際には実行していない

• packages/extension/src/services/settings/settings.test.ts:677 と :696 で settingVersion: "1.1.0" を設定していますが、本PRで package.json のバージョンも 1.1.0 に上げているため(package.json:3)、テスト実行時の VERSION 定数も "1.1.0" になります。
• settings.ts:266 の if (versionDiff(currentVersion, "1.1.0") === VersionDiff.Old) は「現在のバージョンが1.1.0より古い」場合のみtrueになりますが、versionDiff("1.1.0", "1.1.0") は VersionDiff.Same を返すため条件がfalseとなり、migrate1_1_0 が実行されません。
• 同ファイルの他のマイグレーションテスト(例: 0.11.5 用テストは settingVersion: "0.11.3" を使用、0.15.1 用テストは "0.14.2" を使用)は対象バージョンより明確に古い値を使っており、本テストだけこの慣習から外れています。
• 結果として ST-34 は cmd が undefined になり expect(cmd).toBeDefined() で失敗する可能性が高く、ST-34-a はマイグレーションを実質検証しないまま通ってしまいます。settingVersion は "1.0.0" など明確に古い値にすべきです。

🟡 確認したい点

4. デフォルトコマンド「Search Commands on Hub」の searchUrl が %pageUrl を使っている

• packages/extension/src/services/option/defaultSettings.ts:404: searchUrl: `https://selection-command.com/${hubLocale}?q=%pageUrl` となっており、選択テキスト(%s)ではなく現在のページURLを q パラメータに渡しています。
• Hub側(packages/hub/src/app/[lang]/page.tsx や CommandList コンポーネント)を確認しましたが、現状 q クエリパラメータを読み取る実装は見当たりません(コマンド一覧は静的JSONと tagName のみでフィルタされています)。
• コマンド名が「Search Commands on Hub」(Hubでコマンドを検索)であることを踏まえると、本来は選択テキストを検索クエリとして渡す %s を意図していたのに %pageUrl に誤って置き換えてしまった可能性があります。意図的であれば問題ありませんが、念のため確認をお願いします。

🟢 軽微な指摘

5. 変数名の衝突による可読性の低下

• packages/extension/src/services/option/defaultSettings.ts:85 でエクスポートされている固定ID定数 COMMAND_SEARCH_ID と、インドネシア語("id"ロケール)用コマンド変数 CMD_COMMAND_SEARCH_ID(同ファイル453行目)の名前が非常に紛らわしいです。将来的な可読性のため、ロケール変数側の命名を見直すことを検討してください。

6. toUrl のプレースホルダー置換が単一置換のまま

• packages/extension/src/lib/utils.ts:87-89 で %s と %pageUrl の置換に .replace()(先頭一致のみ)を使用しています。既存の %s 置換も同様の実装なので一貫はしていますが、テンプレート内に同じプレースホルダーが複数回登場するケースには対応できません。将来的に replaceAll へ統一することを検討してもよいかもしれません。
• また同関数は戻り値の型が string ですが、searchUrl が undefined の場合は undefined を返し得ます(既存の問題ですが、今回この行に手を入れているのでついでに直すのもありです)。

✅ 良い点

• pageUrl は encodeURIComponent でエンコードしてからテンプレートに埋め込んでいるため、URLインジェクションなどのセキュリティ上の問題はありません。
• migrate1_1_0(settings.ts:388-402)は既存コマンドとのID重複チェックを行っており、二重追加を防ぐロジック自体は適切です。
• toUrl の新しい分岐(TU-14〜TU-17, utils.test.ts:202-244)や DS-24、ST-34 など、新機能に対するユニットテストがしっかり追加されている点は良い取り組みです(内容の不整合は上記参照)。

---

このレビューはPRの差分とリポジトリの現在のコードを確認した上でClaudeが作成しました。

[codecov]

Codecov Report

❌ Patch coverage is 63.63636% with 64 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.11%. Comparing base (09eb031) to head (a1653df).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...sion/src/components/option/editor/ShortcutList.tsx	0.00%	47 Missing ⚠️
packages/extension/src/action/pageAction.ts	0.00%	3 Missing ⚠️
packages/extension/src/action/background.ts	0.00%	2 Missing ⚠️
packages/extension/src/action/backgroundTab.ts	0.00%	2 Missing ⚠️
packages/extension/src/action/popup.ts	0.00%	2 Missing ⚠️
packages/extension/src/action/sidePanel.ts	0.00%	2 Missing ⚠️
packages/extension/src/action/tab.ts	0.00%	2 Missing ⚠️
packages/extension/src/action/window.ts	0.00%	2 Missing ⚠️
packages/extension/src/action/index.ts	0.00%	1 Missing ⚠️
...s/extension/src/services/option/defaultSettings.ts	98.83%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #430      +/-   ##
==========================================
+ Coverage   29.93%   30.11%   +0.17%     
==========================================
  Files         322      322              
  Lines       33274    33435     +161     
  Branches     1943     1955      +12     
==========================================
+ Hits         9962    10070     +108     
- Misses      23312    23365      +53     

☔ View full report in Codecov by Harness.
📢 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]

レビュー結果

pageUrl を content script/background の各実行経路に伝播させ、%pageUrl プレースホルダーと「Search Commands on Hub」デフォルトコマンドを追加する変更、全体として意図が明確で、toUrl・defaultSettings・settings のマイグレーションにテストも追加されており良い対応だと思います。いくつか気になった点を挙げます。

1. pageUrl がバックグラウンド直接実行パスで欠落する（バグの疑い）

packages/extension/src/background_script.ts:582-589 の chrome.commands.onCommand ハンドラでは、タブへの送信が使えない場合（chrome:// ページ等）に execute() を直接呼び出し、pageUrl: tab?.url ?? "" を渡しています。

しかし呼び出し先の packages/extension/src/action/background.ts:22-41 の execute は、分割代入のパラメータに pageUrl を含めておらず、executeAction() にも転送していません。

export async function execute({
  command,
  position,
  selectionText,
  target,
  useSecondary = false,
  useClipboard = false,
  changeState,
  // pageUrl が欠落している
}: ExecuteCommandParams) {
  return executeAction({
    ...
    // pageUrl が渡されていない
    actions: actionsForBackground,
  })
}

このため、この経路（タブへのメッセージ送信が無効/失敗した場合のフォールバック実行）で実行されたコマンドは pageUrl が常に空文字になり、%pageUrl や srcUrl が本来の値にならないと思われます。この PR がまさに解決しようとしている問題が、このパスだけ再発している状態です。

なお packages/extension/src/background_script.test.ts では @/action/background を vi.mock しているため（14行目）、execute() に pageUrl が渡されていることは確認できても、実装側で握りつぶされていることまではテストで検出できていません。action/background.ts に対する単体テストも見当たりませんでした。

ExecuteCommandParams の分割代入に pageUrl を追加し、executeAction に渡すよう修正が必要そうです。

2. SidePanel.execute だけ pageUrl を使わず location.href を直接参照している

packages/extension/src/action/sidePanel.ts:9-13, 42 は、他の Tab/Window/Popup/BackgroundTab/PageAction/AiPrompt と異なり pageUrl パラメータを受け取らず、location.href を直に使っています。

現状は SIDE_PANEL が action/index.ts（content script 側のマップ）からしか呼ばれないため実害はなさそうですが、他のアクションと実装パターンが揃っていないのは今後のバグの温床になりえます（1点目のような、背景コンテキストから直接呼ばれるケースが将来追加された場合に同様の問題が起きえます）。一貫性のため pageUrl を受け取る形に揃えることをお勧めします。

3. Hub 側が q クエリパラメータを読んでいない可能性

新規デフォルトコマンド「Search Commands on Hub」(packages/extension/src/services/option/defaultSettings.ts:602-620) は https://selection-command.com/{locale}?q=%pageUrl を開きますが、packages/hub/src/app/[lang]/page.tsx は searchParams を受け取っておらず、リポジトリ内を検索した限り hub パッケージ側で q パラメータを読んでいる箇所が見つかりませんでした。

意図的に hub 側の対応を別 PR に分けているのであれば問題ありませんが、そうでない場合はこのコマンドをクリックしても Hub のトップページが開くだけで検索は実行されない状態だと思います。ご確認いただけると安心です。

4. ShortcutList.tsx の willUseClipboard / referencesSelection がほぼ重複

packages/extension/src/components/option/editor/ShortcutList.tsx:41-58（willUseClipboard）と :89-106（referencesSelection）は、プレースホルダー判定用のヘルパー（hasSelectionOrClipboardPlaceholder / hasSelectionPlaceholder）が違うだけで分岐構造が完全に同一です。判定関数を引数化して1つの共通ヘルパーにまとめられそうです。

5. createNameRender のインデントが崩れている（フォーマット）

packages/extension/src/components/option/editor/ShortcutList.tsx:108-124 の三項演算子内の JSX ブロックのインデントが、周辺のコードや元のスタイルと比べて1段浅くなっています（diff で確認できます）。Prettier/ESLint 実行後の差分ではない可能性があるので、yarn lint を通してフォーマットを揃えておくと良さそうです。

6. （軽微）toUrl の %pageUrl 置換順序

packages/extension/src/lib/utils.ts:84-88

let url = searchUrl?.replace("%s", textEncoded)
url = url?.replace("%pageUrl", encodeURIComponent(param.pageUrl ?? ""))

%s の置換を先に行っているため、選択テキスト自体に文字列 "%pageUrl" が含まれていた場合、意図せず pageUrl の値で置換されてしまう可能性があります。かなりレアなケースですが、念のため共有します。

テストについて

toUrl（TU-14〜17）、defaultSettings（DS-24）、settings migration（ST-34, ST-34-a）へのテスト追加は良い対応だと思います。一方で上記1点目のとおり、action/background.ts 単体・および pageUrl がバックグラウンド直接実行パスまで実際に伝播することを検証するテストは無いようなので、追加を検討いただけると良さそうです。

---

以上、よろしくお願いいたします 🙏

[claude]

Test comment from review tool - checking gh pr comment with stdin

[claude]

レビュー概要

このPRは「Hub上でコマンドを検索する」デフォルトコマンドを追加し、そのための %pageUrl プレースホルダーをURLテンプレートに導入するものです。あわせて location.href への直接依存を排除して pageUrl を ExecuteCommandParams 経由で明示的に受け渡す設計に変更し、ShortcutList.tsx の「テキスト選択必須」判定ロジックをより正確なもの（openModeごとの分岐、プレースホルダー参照有無の判定）にリファクタリングしています。設定マイグレーション（migrate1_1_0）・toUrl() の %pageUrl 置換・デフォルトコマンド生成には、それぞれ対応するテスト（ST-34系、TU-14〜17、DS-24）が追加されており、全体として丁寧に作り込まれています。

以下、気になった点を挙げます。

潜在的な不具合

1. Api（OPEN_MODE.API）コマンドで %pageUrl が空文字に置換されてしまう

   • packages/extension/src/action/api.ts:12-16
   • packages/extension/src/lib/utils.ts:84-89

   Popup/Tab/Window/BackgroundTab/SidePanel/PageAction/AiPrompt の各アクションモジュールは今回すべて pageUrl を受け取り toUrl() に渡すよう修正されていますが、Api.execute（api.ts:11）だけは ExecuteCommandParams から pageUrl を分割代入しておらず、searchUrl を組み立てる toUrl() 呼び出し（api.ts:44付近）にも pageUrl を渡していません。

   toUrl()（utils.ts:88）は param.pageUrl が undefined でも無条件に %pageUrl を空文字へ置換するため、API種別のコマンドで searchUrl に %pageUrl を使った場合、静かに空文字へ置き換わってしまいます（エラーにはならず気づきにくい）。Api.execute 内では別途 chrome.tabs.query で取得した pageUrl を IPC の pageUrl フィールド（${pageUrl} テンプレート用、helper.ts:274-287）としては使っていますが、toUrl() には渡していません。一貫性の観点から、Api.execute にも pageUrl を渡す（既存の chrome.tabs.query の結果を使う）ことを検討してください。

テストカバレッジ

2. ShortcutList.tsx の新規ロジックに対する単体テストが見当たらない

   • packages/extension/src/components/option/editor/ShortcutList.tsx:33-72

   referencesPlaceholder / willUseClipboard / isTextSelectionOnly / referencesSelection という、openMode とコマンド種別（search / pageAction / aiPrompt / その他）で分岐する非自明なロジックが追加・拡張されていますが、ShortcutList.test.tsx に相当するテストファイルが存在しないようです。特に isPageActionType 分岐（steps の paramToStr を走査する部分、ShortcutList.tsx:410-413）は分岐条件が複雑なので、代表的な入力パターンに対する単体テストを追加すると安心です。

3. e2e の URL 疎通テストが %pageUrl を未置換のまま送信する

   • packages/extension/e2e/url-status.spec.ts:17

   url-status.spec.ts は searchUrl.replace("%s", "test") のみを行っており、今回追加された %pageUrl は置換されないため、新規の「Search Commands on Hub」系URLは ?q=%pageUrl という未置換の文字列のままリクエストされます（CIではスキップされるためブロッカーではありませんが、ローカル実行時の疎通確認としては本来の用途と少しずれます）。余裕があれば %pageUrl もダミー値に置換するよう更新するとよさそうです。

軽微な指摘（Nits）

• packages/extension/src/services/option/defaultSettings.ts:651-702 : 14言語分の createCommandSearchCommand(...) 呼び出しがほぼ同型の記述で並んでいます。ファイル内の他の CMD_* 定義群と同じ手書きスタイルに合わせているのは理解できますが、locale と title のペアを配列にして map() する形にすればさらに見通しがよくなりそうです（必須の変更ではありません）。
• packages/extension/src/lib/utils.ts:87-89 : String.prototype.replace は最初の一致のみを置換するため、searchUrl 内に %pageUrl が複数回出現するケースは想定されていません（既存の %s 置換も同様の制約を引き継いでいるだけなので、既存仕様との整合は取れています）。将来的に複数箇所への埋め込みが必要になった場合は replaceAll の採用を検討してください。

良い点

• pageUrl の受け渡しが ExecuteCommandParams / UrlParam の型定義（types/index.ts:241, :308）まで含めて一貫して通されており、action/index.ts:52-53 のコメントで「このモジュールはcontent script文脈でのみ実行されるため location.href が正しい」という設計判断が明記されている点は好印象です。
• background_script.ts:588（キーボードショートカット経由の実行パス）では content script を経由しないため tab?.url を使うなど、実行コンテキストごとの pageUrl 取得方法が正しく使い分けられています。
• マイグレーション（migrate1_1_0, settings.ts:389-404）は既存の migrate0_10_0 と同じパターン（IDによる重複チェック→Storage.setCommands）を踏襲しており、コードベースの規約に沿っています。
• toUrl() の %pageUrl 置換、マイグレーションの重複防止、全ロケールでのデフォルトコマンド存在確認について、それぞれ的確なテストケース（TU-14〜17, ST-34, ST-34-a, DS-24）が追加されています。

以上、大きな設計上の問題は見当たりませんでした。上記の Api コマンドの %pageUrl 未対応の点だけ確認いただければと思います。

[claude]

レビュー概要

「Search Commands on Hub」のデフォルトコマンド追加（#429対応）と、それに伴う %pageUrl プレースホルダーの導入、location.href への暗黙依存を排除した pageUrl の明示的なパラメータ化を行うPRです。全体として設計意図が明確で、テストも手厚く追加されており、品質は高いと感じました。以下、気づいた点を挙げます。

👍 良い点

• location.href への暗黙依存の排除: packages/extension/src/action/aiPrompt.ts などで直接 location.href を参照していた箇所を pageUrl パラメータ経由に変更し、packages/extension/src/action/index.ts:51-53 のコメントの通り「content scriptのページコンテキストでのみ実行される」ことを明示しています。これにより、background_script.ts:582-589 のようにbackground(service worker)コンテキストから直接実行される経路（tab?.url ?? "" を使用）でも正しいページURLが渡るようになっており、地味ながら潜在バグの修正になっています。
• 15言語ぶんのデフォルトコマンドとマイグレーション: packages/extension/src/services/option/defaultSettings.ts の createCommandSearchCommand で各locale版コマンドを生成し、packages/extension/src/services/settings/settings.ts:389-403 の migrate1_1_0 で既存ユーザーにも重複なく追加する実装は、既存の migrate0_10_0 パターン（settings.ts:274-286）を踏襲しており一貫性があります。
• テストの手厚さ: toUrl（TU-14〜17）、getDefaultCommands（DS-24）、migrate（ST-34, ST-34-a）に対するユニットテストが追加されており、%pageUrl 置換やマイグレーションの冪等性がきちんと検証されています。
• URLエンコーディング: packages/extension/src/lib/utils.ts:88 で encodeURIComponent(param.pageUrl ?? "") を使っており、pageUrlに含まれる & や # などの予約文字が正しくエスケープされます（TU-17でテスト済み）。インジェクション面でも問題は見当たりませんでした。

🔍 気になる点・提案

1. packages/extension/e2e/pages/OptionsPage.ts:75-79: COMMAND_SEARCH_ID を src/services/option/defaultSettings.ts の値とハードコードで二重管理しています。コメントで理由（__AI_SERVICES_JSON__ のbuild-time define問題）は説明されていますが、将来 defaultSettings.ts 側でIDが変わった場合、このe2eテストのカウント計算が静かにズレるリスクがあります。可能であれば、IDのみを別の依存の軽いファイルに切り出すなど、値のズレを防ぐ仕組みがあるとより安全です。

2. packages/extension/src/components/option/editor/ShortcutList.tsx:43-98: referencesPlaceholder / willUseClipboard / referencesSelection / isTextSelectionOnly のロジックがこのPRでかなり拡張されていますが（isSearchType, isPageActionType の分岐追加など）、ShortcutList.tsx 自体には対応するユニットテストファイルが存在しないようです。「ショートカットの『未選択時の挙動』表示を出し分ける」というUIロジックとしては複雑度が上がっているので、ロジック部分だけでも切り出してユニットテストを追加することを検討ください。

3. 同ファイルの referencesPlaceholder（ShortcutList.tsx:43-63）は hasPlaceholder を引数に取りますが、isSearchType / OPEN_MODE.API の分岐では引数を使わず command.searchUrl?.includes("%s") の固定チェックになっています。%s が選択テキストとクリップボードの両方を表すための意図的な設計だと思われますが、関数シグネチャだけを見ると引数が無視される分岐があるのはやや読みづらいので、一言コメントを添えるとレビューアビリティが上がると思います。

4. packages/extension/src/lib/utils.ts:87-89: toUrl の戻り値型は string ですが、searchUrl が undefined の場合 url も undefined になり得ます（searchUrl?.replace(...) のoptional chaining由来）。これは既存の実装からある問題ではありますが、%pageUrl 置換のために処理が一段伸びたこのタイミングで、searchUrl ?? "" のように明示的にフォールバックしておくと型安全性が上がります。

5. 翻訳の品質確認: defaultSettings.ts に追加された15言語分のタイトル文言（例: Hub पर कमांड खोजें, Поиск команд в Hub 等）や e2e/generated-command-urls.ts の文言は、可能であればネイティブスピーカーによるレビューを推奨します（機械翻訳であれば表現の自然さの確認）。

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

• pageUrlの伝播経路をひととおり追いましたが（content_script → useCommandExecutor → action/index.ts → executor.ts → 各action → toUrl）、すべて encodeURIComponent を通してURLに埋め込まれており、XSSやURLインジェクションの懸念は見当たりませんでした。
• manifest.json の web_accessible_resources に images/* を追加していますが、公開されるのは新規追加された search_command.png のみで、既存の assets/*.css と同様のパターンなので問題ありません。
• パフォーマンス面で懸念となる変更はありません。

総評

設計意図が一貫しており、テストカバレッジも良好です。上記の指摘は軽微なものが中心なので、必須のブロッカーではなく、余裕があれば対応をご検討いただければと思います。

[claude]

レビュー概要

「Search Commands on Hub」デフォルトコマンドの追加と、選択テキストだけでなく現在のページURL(%pageUrl)をコマンドの検索URL/プロンプトに渡せるようにする機能追加です。ExecuteCommandParams/UrlParamへのpageUrlの伝播、toUrl()での%pageUrl置換、設定マイグレーション(migrate1_1_0)、多言語のデフォルトコマンド定義、e2eテストまで一貫して実装されており、全体的に丁寧な実装だと感じました。いくつか気になった点を挙げます。

良い点

• pageUrlが「content scriptで実行されるaction/index.tsはlocation.hrefを使い、background実行パス(action/background.ts)は呼び出し元(background_script.ts:588)からtab?.urlを渡す」という形で一貫して配線されている点は正確です。特にaiPrompt.ts（packages/extension/src/action/aiPrompt.ts:416,425,434）は従来location.hrefを直接参照していましたが、これはbackground(service worker)コンテキストで実行されるとlocationはタブのURLではなくservice worker自身のスクリプトURLを指してしまうため、実質的なバグでした。今回pageUrlを明示的に引き回す形に変更したことで、この潜在バグが解消されています。
• toUrl()（packages/extension/src/lib/utils.ts:87-88）でpageUrlに対してencodeURIComponentを適用しており、URLインジェクションを防ぐ実装になっています。
• マイグレーション(migrate1_1_0, packages/extension/src/services/settings/settings.ts:389-403)はID(COMMAND_SEARCH_ID)による重複チェックがあり、べき等性が保たれています。
• e2eテストE2E-94（packages/extension/e2e/hub.spec.ts:186-241）は実際のページ遷移・複数検索結果の存在確認までカバーしており、テストの意図がコメントで明確に説明されています。

気になった点

1. ShortcutList.tsxのロジック変更に対するユニットテストが無い (packages/extension/src/components/option/editor/ShortcutList.tsx:33-79)
   referencesPlaceholder/willUseClipboard/isTextSelectionOnly/referencesSelectionという、コマンド種別（検索/PageAction/AIプロンプト/その他）ごとに分岐する複雑なロジックが追加・変更されていますが、ShortcutList.test.tsxは存在せず、このファイルに対するテストが見当たりませんでした。特にreferencesPlaceholderのフォールバック(copy/getTextStyles/linkPopupは常にtrueを返す)はOPTION/ADD_PAGE_RULEのようなopenModeにも適用されるため、意図通りか確認するテストがあると安心です。

2. COMMAND_SEARCH_IDが2箇所にハードコードされ、片方は明示的にimportを避けている (packages/extension/e2e/pages/OptionsPage.ts:9-13)
   defaultSettings.tsをimportするとaiPromptFallback.tsがビルド時defineに依存するためPlaywright実行時に落ちる、という理由でe2e側にIDを別途ハードコードしています。コメントで理由が説明されており意図は分かるのですが、将来COMMAND_SEARCH_IDを変更した際にこの重複箇所の更新を忘れるリスクが残ります。可能であれば、aiPromptFallback.tsの__AI_SERVICES_JSON__依存をdefaultSettings.tsから分離する（IDや基本コマンド定義だけを軽量な別モジュールに切り出す）方が本質的な解決になりそうです。今回のスコープでは許容範囲だと思います。

3. スコープ外と思われる変更が同一PRに混在

   • test-settings-link-preview.jsonのopenModeをpreviewPopup→previewSidePanelに変更（packages/extension/e2e/data/test-settings-link-preview.json:5）
   • hub.spec.ts内の既存テスト（restoredButtonのセレクタ変更、waitForLoadState→waitForEvent(framenavigated)への修正、packages/extension/e2e/pages/OptionsPage.ts:274-286）
     いずれもflaky対策や既存バグ修正として妥当な変更に見えますが、本PRの主題（ページURL機能）とは直接関係がないため、可能であれば別PRに分離するとレビューや今後のgit blameの追跡がしやすくなります（必須ではありません）。

4. 軽微: toUrl()内でparam.pageUrlだけ分割代入されていない (packages/extension/src/lib/utils.ts:66-71,88)
   searchUrl/selectionText/spaceEncoding/useClipboardは分割代入されている一方、pageUrlのみparam.pageUrlと直接参照しています。動作上の問題はありませんが、一貫性のため他のフィールドと同様に分割代入した方が読みやすいと思います。

5. 軽微: defaultSettings.tsのiconUrl計算がモジュールロード時に評価される (packages/extension/src/services/option/defaultSettings.ts:397-400)
   createCommandSearchCommand内のchrome.runtime?.getURL呼び出しはモジュールトップレベルで13言語分実行されます。typeof chrome !== undefinedのガードがあるため実害はなさそうで、他のCMD_*定義と同様のパターンなので問題ないと思います。

テストカバレッジ

• toUrlの%pageUrl置換に対する単体テスト(TU-14〜17)、getDefaultCommandsの全ロケール検証(DS-24)、マイグレーションの追加/非重複テスト(ST-34, ST-34-a)が揃っており、コア機能のテストは手厚いです。
• 前述の通りShortcutList.tsxのロジック変更のみユニットテストが手薄です。

セキュリティ

• %pageUrlはencodeURIComponentでエンコードされており、生成されるsearchUrlもHTTPSの自社ドメイン(selection-command.com)向けなので、インジェクションやオープンリダイレクトの懸念は特にありません。

全体として実装の正確性は高く、既存の潜在バグ(location.hrefのコンテキスト誤り)も副次的に修正されている良いPRだと思います。上記はいずれもブロッカーではなく、次のイテレーションで検討いただければ十分です。

[claude]

レビュー結果

%pageUrl プレースホルダーの追加と「Search Commands on Hub」デフォルトコマンドの実装は、全体的によく設計されており、toUrl・各種 execute 系関数への pageUrl の受け渡し、マイグレーション処理（migrate1_1_0）は既存パターンに沿っていて一貫性があります。E2E/ユニットテストも手厚く追加されています。以下、気になった点を挙げます。

1. ShortcutList.tsx の新規ロジックにユニットテストがない
packages/extension/src/components/option/editor/ShortcutList.tsx:33-98 で referencesPlaceholder / willUseClipboard / isTextSelectionOnly / referencesSelection という、検索コマンド・PageAction・AIプロンプトの3種類の openMode 分岐を含む複雑なロジックが新規追加されていますが、対応するテストファイル（ShortcutList.test.tsx 等）が見当たりません。このPRの他の変更（utils.test.ts, defaultSettings.test.ts, settings.test.ts, aiPrompt.test.ts）はロジック変更に対して丁寧にユニットテストが追加されているのに対し、ここだけテストが欠けています。特に isTextSelectionOnly の分岐は「選択なし時の挙動」設定欄（showNoSel, 同ファイル305-308行目）の表示可否を左右する重要なロジックなので、少なくとも各コマンド種別・openMode の組み合わせに対する単体テストの追加を推奨します。

2. COMMAND_SEARCH_ID の値が2箇所にハードコードされ、同期ズレのリスクがある
packages/extension/src/services/option/defaultSettings.ts:85 と packages/extension/e2e/pages/OptionsPage.ts:14 の両方に同じUUID文字列 019f470a-cea5-7d6f-86cf-e7df9fb14ff1 がリテラルで存在します。OptionsPage.ts 側のコメントで「defaultSettings.ts は __AI_SERVICES_JSON__ のビルド時defineに依存するため直接importできない」という理由が説明されており意図は理解できますが、将来 defaultSettings.ts 側でこのIDを変更した場合、E2Eテスト側は気づかずに静かにズレる可能性があります。IDだけを別の小さな定数ファイル（他の依存を持たない）に切り出して両方から参照する、あるいは値が一致することを検証するテストを1つ追加しておくと安全です。

3. 検索URLの入力欄の説明文（ヘルプテキスト）が %pageUrl に未対応
packages/extension/src/lib/utils.ts の toUrl（84-89行目）により %pageUrl は組み込みコマンドに限らず、ユーザーが作成する任意の検索コマンドの searchUrl でも利用可能な汎用機能になっています。しかし、コマンド編集ダイアログの「検索URL」欄の説明文（packages/extension/public/_locales/en/messages.json:265-267 の Option_searchUrl_desc: 'Replace %s with the selected text.'、および他言語版も同様）は %s のみの説明のままで、%pageUrl について触れられていません。packages/extension/src/components/option/editor/CommandEditDialog.tsx:582-586 でこの説明文がそのままUIに表示されるため、ユーザーはこの新しいプレースホルダーの存在をUIから知る手段がありません。ドキュメント（各locale の Option_searchUrl_desc）の更新をご検討ください。

その他（軽微）
packages/extension/package.json / packages/extension/manifest.json / ルート package.json のバージョン文字列 1.1.0 と packages/extension/src/services/settings/settings.ts:266 の versionDiff(currentVersion, "1.1.0") は、値が別々にハードコードされているため、リリース時の更新箇所として認識しておく必要があります（バグではなく運用上の注意点です）。

以上、全体的には堅牢な実装です。上記の指摘は必須の修正というよりは品質向上のための提案としてご検討ください。

[claude]

コードレビュー: Feat/add-page-url (#430)

「Search Commands on Hub」デフォルトコマンドの追加と、%pageUrl プレースホルダーによる訪問中ページURLの検索クエリへの埋め込み機能について、全体をレビューしました。テストが手厚く追加されている点は良い点として評価しつつ、いくつか気になった点を記載します。

全体的な評価

• pageUrl を ExecuteCommandParams に足して location.href への暗黙依存を排除した設計は良い変更です。特に packages/extension/src/action/aiPrompt.test.ts で window.location のグローバルモックが不要になった点はテスタビリティ向上として評価できます。
• packages/extension/src/lib/utils.ts の toUrl() での %pageUrl 実装、および %s 処理後に %pageUrl を置換する順序は、encodeURIComponent が % を %25 にエンコードするため相互汚染は発生せず、安全に実装されています。テスト(TU-14〜17)も境界値を含め妥当です。

気になった点

1. SearchUrlAssistDialog.tsx が pageUrl を渡していない（packages/extension/src/components/option/editor/SearchUrlAssistDialog.tsx:75）
   PageAction.execute() を executor.ts 経由ではなく直接呼び出しており、pageUrl を渡していません。現状 searchUrlAssistAction（packages/extension/src/services/searchUrlAssist.ts）は {{url}} プレースホルダーを使っていないため実害はありませんが、ExecuteCommandParams.pageUrl が optional なためコンパイラでは検出できず、将来 srcUrl/%pageUrl を参照するpageActionを追加した際に静かに空文字列になるリスクがあります。action/index.ts・action/background.ts 以外からアクションモジュールを直接呼ぶ箇所がないか確認し、可能であれば呼び出し元を executor.ts 経由に統一するか、コメントで意図を明記しておくと安全です。

2. ShortcutList.tsx の新規ロジックにテストが無い（packages/extension/src/components/option/editor/ShortcutList.tsx:41-98）
   referencesPlaceholder / willUseClipboard / referencesSelection という新しい判定ロジックが追加され、isTextSelectionOnly の挙動も isSearchType・isPageActionType 分岐が新設されて意味的に変わっています（例: 以前は PAGE_ACTION は常に OPEN_MODE_BG に含まれ isTextSelectionOnly=false 固定でしたが、今回は CURRENT_TAB かつプレースホルダー参照時に true を返すよう変更されています）。この振る舞い変更を検証するユニットテストが本PRには含まれていません（ShortcutList.test.tsx 自体が存在しません）。他のファイル（utils.test.ts, defaultSettings.test.ts, settings.test.ts）には手厚くテストが追加されているのに対し、ここだけ手薄なので追加を検討ください。

3. COMMAND_SEARCH_ID のハードコード重複（packages/extension/e2e/pages/OptionsPage.ts:272）
   defaultSettings.ts の COMMAND_SEARCH_ID を e2e 側で再定義しています。理由（aiPromptFallback.ts がビルド時defineに依存しPlaywrightから読み込めない）はコメントで説明されており納得感はありますが、値が変わった際に追従漏れするリスクがあるため、CIなどで一致を検証するテスト（例: defaultSettings.ts からIDをexportし、OptionsPage.ts の定数と比較するユニットテスト）があると安心です。

4. 軽微: toUrl の %pageUrl / %s は非グローバル置換（packages/extension/src/lib/utils.ts:87-89）
   .replace() は最初の一致のみ置換するため、searchUrl に同じプレースホルダーが複数回現れるケースは想定されていません（本PR由来ではなく既存の %s 処理から踏襲した挙動です）。現状の用途では問題になりませんが、将来的にテンプレートが複雑化した場合は replaceAll の検討余地があります。

良かった点

• migrate1_1_0（packages/extension/src/services/settings/settings.ts:266,388-403）は既存の migrate0_10_0 パターンに忠実に倣っており、COMMAND_SEARCH_ID の重複チェックも適切です。移行が二重実行されないことを検証するテスト(ST-34, ST-34-a)も的確です。
• e2e/hub.spec.ts の E2E-94 は、Cloudflareのbot対策やCIでの多言語表示ゆれを考慮した設計（位置ベースでのクリック、NEW_HUB_URL への差し替え）がコメントで丁寧に説明されており、保守性が高いです。
• 11言語すべてに対して「Search Commands on Hub」コマンドが提供され、DS-24 テストで全ロケールの存在を保証している点は良い網羅性です。
• manifest.json の web_accessible_resources に images/* を追加し、新規アイコン (public/images/search_command.png) を正しく参照できるようにしている点も整合しています。

セキュリティ

%pageUrl は encodeURIComponent でエスケープされてからURLに埋め込まれるため、インジェクションのリスクは低いと判断しました。AIプロンプトへの埋め込み（aiPrompt.ts）はURLエンコードせず生の文字列として渡していますが、これはプロンプトテキストであり画面遷移やDOM挿入に使われないため妥当です。

以上、大きな欠陥はなく、上記の指摘は主に保守性・テストカバレッジに関する提案です。