Fix/option screen

[ujiro99]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 12.50000% with 238 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.52%. Comparing base (7564109) to head (fd10a24).

Files with missing lines	Patch %	Lines
packages/extension/src/services/searchUrlAssist.ts	0.00%	86 Missing ⚠️
...nents/option/editor/CommandTypeSelectionDialog.tsx	0.00%	47 Missing ⚠️
...n/src/components/option/editor/CommandListMenu.tsx	0.00%	33 Missing ⚠️
...es/extension/src/components/option/SettingForm.tsx	0.00%	30 Missing ⚠️
...es/extension/src/components/option/ShareButton.tsx	0.00%	24 Missing ⚠️
...src/components/option/editor/CommandEditDialog.tsx	0.00%	7 Missing ⚠️
...components/option/editor/SearchUrlAssistDialog.tsx	25.00%	6 Missing ⚠️
...ension/src/components/option/field/SwitchField.tsx	0.00%	5 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##           dev-1.1.1     #445      +/-   ##
=============================================
- Coverage      39.56%   39.52%   -0.04%     
=============================================
  Files            237      237              
  Lines          25353    25419      +66     
  Branches        1886     1890       +4     
=============================================
+ Hits           10032    10048      +16     
- Misses         15321    15371      +50     

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

コードレビュー(PR #445)

複数の独立した修正がまとまったPRですが、全体的に各コミットの意図が明確で、コメントも丁寧に書かれています。特に SwitchField.tsx の FormLabel からツールチップアイコンを外す修正(packages/extension/src/components/option/field/SwitchField.tsx:40-52)は、htmlFor によるネイティブ<label>のクリック転送という根本原因を正しく捉えた良い修正だと思います。

以下、気になった点を優先度順に記載します。

1. ShareButton.tsx: 連打によるシェア/サインアップの多重実行 (correctness)

packages/extension/src/components/option/ShareButton.tsx:41-70

const handleClick = async (e: React.MouseEvent) => {
  ...
  const ok = shareCommandToHub(commandToShare)

  const registered = await Storage.get<boolean>(
    LOCAL_STORAGE_KEY.HUB_REGISTERED,
  )
  if (!registered) {
    return
  }

  setStatus(ok ? "sent" : "error")
  ...
}

ボタンの disabled は status !== "idle" (ShareButton.tsx:99) で制御されていますが、setStatus が呼ばれるのは Storage.get の await が解決した後です。以前は handleClick が同期処理で、クリック直後に setStatus されていたため実質的に連打をブロックできていましたが、この変更で await Storage.get(...) の分だけ「idleのまま無効化されない時間」が生まれています。

未登録ユーザーの場合は services/hub/background.ts:80-97 の share() 内で毎回 chrome.tabs.create によりサインアップタブが新規に開かれるため、素早く連打すると複数のタブが開いたり、登録済みユーザーでも複数の共有リクエストが飛ぶ可能性があります。クリック時点で即座に何らかの「処理中」状態にしてボタンを無効化する方が安全です。

2. ShareButton.tsx: registered チェックの順序により、無効なコマンドの共有失敗が握りつぶされる (correctness)

packages/extension/src/components/option/ShareButton.tsx:59-70

ok(shareCommandToHub の戻り値。コマンドが toSubmitCommandInput でバリデーションを通ったかどうかを表す、services/hubShare.ts:65-78 参照)が false の場合は、そもそもバックグラウンドへの共有処理自体が呼ばれていないため HUB_REGISTERED の状態とは無関係のはずです。しかし現在のコードでは ok の値に関わらず先に registered をチェックして未登録なら即 return してしまうため、「未登録ユーザーがコマンドとして不正な(searchUrl が無いなど)コマンドを共有しようとした」場合に、エラー表示(赤いアイコン)が一切出ずボタンが単に何も起きないように見えてしまいます。if (!ok) { setStatus("error"); ...; return } を先に評価してから registered チェックに進む方が意図に合うと思います。

3. テストカバレッジ: ShareButton.tsx に対するユニットテストが存在しない (test coverage)

packages/extension/src/components/option/ShareButton.tsx

このPRで handleClick が非同期化され、HUB_REGISTERED の判定という新しい分岐ロジックが追加されましたが、ShareButton.test.tsx に相当するテストファイルが見当たりません(services/hub/background.test.ts 側には SH-02b として該当ロジックのテストが追加されていますが、コンポーネント側は未検証です)。上記1・2のような分岐は単体テストで拾いやすい類のものなので、追加を検討いただけると良さそうです。

4. マイナー: サインアップURLだけロケールが付与されていない (nit)

packages/extension/src/services/hub/background.ts:91

const signupUrl = `${NEW_HUB_URL}/auth/signup`

他のHubリンク(例: CommandTypeSelectionDialog.tsx:40-45 や ShareButton.tsx:46-47)は全て ${NEW_HUB_URL}/${locale}/... の形でロケールを付与していますが、ここだけ locale が付いていません。意図的(サインアップページ自体が未ローカライズ)であれば問題ありませんが、念のため確認をお願いします。

5. 確認事項: 既存の登録済みユーザーが一時的にサインアップ画面に誘導される可能性 (要確認)

packages/extension/src/services/hub/background.ts:82-97, services/storage/const.ts:21

新しい HUB_REGISTERED フラグは handleSetSession(background.ts:513-546)内でのみ true に設定されます。既にHubにサインイン済み(HUB_USER は設定済み)だが、このPR適用後まだHubページを訪問しておらず SetSession メッセージを受け取っていないユーザーは、HUB_REGISTERED がデフォルトの false(storage/index.ts:77)のままとなり、共有ボタン押下時にダッシュボードではなくサインアップ画面へ誘導されてしまう可能性があります。SetSession がどのタイミング(Hubページ読み込み毎か、サインイン操作時のみか)で送られてくるか次第では、既存ユーザーの一部が一時的に不便を感じる可能性があるため、移行パスとして許容範囲か確認いただけると安心です。

---

その他確認した点(問題なし)

• searchUrlAssist.ts のセレクター取得元を ai-services.json 由来に変更した修正は妥当で、copySelectors 未取得時のフォールバック (FALLBACK_COPY_SELECTOR, searchUrlAssist.ts:16-18) も適切です。
• CommandEditDialog.tsx:612-613 の clearErrors("searchUrl") 追加はzodエラーの残留を正しく解消しています。
• CommandListMenu.tsx / CommandTypeSelectionDialog.tsx へのHubリンク移設はUIの簡素化として妥当で、rel="noopener noreferrer" もきちんと付与されています。
• セキュリティ上の懸念(XSS、injection等)は見当たりませんでした。

全体として品質は高いですが、上記1・2はユーザーが実際に踏む可能性のある挙動なので、マージ前にご確認いただければと思います。

[claude]

コードレビュー (Claude)

複数の独立した修正が1つのPRにまとまっていますが、各コミットの意図が明確で、コメントも丁寧に書かれています。既に本PRには過去のレビューコメントが付いており、指摘の一部（連打防止のための即時 setStatus("pending") 化、okチェックの順序修正）は fd10a24（Fix review comments）で対応済みであることを確認しました。以下、現状のコードを再確認した上で、未対応の点および新たに気になった点を記載します。

1. HUB_REGISTERED フラグが既存の登録済みユーザーに正しく引き継がれない可能性 (correctness / 要確認)

packages/extension/src/services/hub/background.ts:82-97, packages/extension/src/services/storage/const.ts:21, packages/extension/src/services/storage/index.ts:77

新しい HUB_REGISTERED フラグは handleSetSession（background.ts:513-546）内でのみ true に設定され、デフォルトは false（storage/index.ts:77）です。一方、fetchMyCommandIds（background.ts:735-759）は getSupabase().auth.getSession() の永続化されたセッション（persistSession: true、background.ts:50）で実際のログイン状態を判定しており、こちらは本PR以前からログイン済みのユーザーでも有効です。

つまり、本PR適用前から既にHubにサインイン済み（HUB_USER 設定済み・Supabaseセッションも有効）だが、SetSession メッセージをまだ一度も受信していないユーザーは、HUB_REGISTERED がデフォルトの false のままとなり、共有ボタン押下時に本来開くべきダッシュボード（dashboard/mycommands）ではなく NEW_HUB_URL/auth/signup に誘導されてしまいます（background.ts:91）。SetSession がHubページ訪問のたびに送られる実装であれば実害は小さいですが、サインイン操作時のみ送信される実装の場合、既存ユーザーの一部が「共有しようとしたらサインアップ画面に飛ばされる」という体験をしてしまう可能性があります。移行パスとして許容範囲か、あるいは getSupabase().auth.getSession() の結果も加味して判定すべきか、ご確認をお願いします。

2. CommandTypeSelectionDialog.tsx: DialogFooter の絶対配置がカード一覧と重なる可能性 (UI / 要確認)

packages/extension/src/components/option/editor/CommandTypeSelectionDialog.tsx:101-107

<DialogFooter className="absolute bottom-6 right-6">
  <DialogClose asChild>
    <Button type="button" variant="secondary" size="lg">
      {t("labelCancel")}
    </Button>
  </DialogClose>
</DialogFooter>

DialogFooter に absolute を指定しているため、レイアウトの高さ計算からは除外されます。DialogContent（packages/extension/src/components/ui/dialog.tsx:42-59）自体に max-height や overflow-y-auto が設定されておらず、コンテンツの高さはキャンセルボタンを除いた space-y-6 py-4 の一覧（CommandTypeSelectionDialog.tsx:82-100）だけで決まります。そのため bottom-6 はダイアログ本来の下端（＝一覧の直後）を基準に配置され、末尾のグループ（commandGroup_experimental_title の1件のカード）にキャンセルボタンが重なって見える可能性があります。実機で表示を確認の上、必要であれば一覧側に pb-16 等の余白を追加するか、DialogFooter を通常フローに戻すことをご検討ください。

3. テストカバレッジ: ShareButton.tsx に対するユニットテストが引き続き見当たらない (test coverage)

packages/extension/src/components/option/ShareButton.tsx

handleClick が非同期化され、HUB_REGISTERED 判定という新しい分岐ロジックが追加されましたが（ShareButton.tsx:43-93）、対応する ShareButton.test.tsx が存在しません。services/hub/background.test.ts 側には該当ロジックのテスト（SH-02b）が追加されていますが、コンポーネント側の「pending中はボタンが無効化される」「未登録時は sent にならない」といった分岐は未検証のままです。Codecovのレポートでもこのファイルは0%となっており、追加をご検討いただけると良さそうです。

4. マイナー: サインアップURLだけロケールが付与されていない (nit)

packages/extension/src/services/hub/background.ts:91

const signupUrl = `${NEW_HUB_URL}/auth/signup`

他のHubリンク（CommandTypeSelectionDialog.tsx:40-45、ShareButton.tsx:48-49 等）は全て ${NEW_HUB_URL}/${locale}/... の形でロケールを付与していますが、ここだけ locale がありません。サインアップページ自体が未ローカライズであれば問題ありませんが、意図的かどうか確認をお願いします。

---

良かった点

• SwitchField.tsx:39-52: ツールチップアイコンを FormLabel（ネイティブ <label> としてクリックがスイッチに転送される）の外に出した修正は、根本原因を正しく捉えた良い直し方です。
• CommandEditDialog.tsx:612-613: Gemini URL変換後に clearErrors("searchUrl") を呼び、zodのバリデーションエラー残留を正しく解消しています。
• searchUrlAssist.ts のセレクター取得元を ai-services.json 由来に変更した修正は妥当で、copySelectors 未取得時のフォールバック（FALLBACK_COPY_SELECTOR、searchUrlAssist.ts:16-18）も、normalizeServices（aiPromptFallback.ts:34）による正規化と合わせて堅牢です。
• 全ロケールの messages.json に Option_commandType_hubLink キーが漏れなく追加されており、多言語対応が丁寧です。
• セキュリティ上の懸念（XSS・injection等）は見当たりませんでした。rel="noopener noreferrer" も新規の外部リンクに付与されています。

まとめ

全体として品質は高く、既存の指摘の多く（連打防止・エラー表示の優先順位）は既に修正されています。残る懸念のうち、特に1（既存ユーザーの移行パス）はユーザー体験に直結する可能性があるため、マージ前にご確認いただければと思います。2は見た目の問題なので実機確認を推奨します。