Skip to content

Dev 1.1.1 - #446

Merged
ujiro99 merged 43 commits into
mainfrom
dev-1.1.1
Aug 18, 2026
Merged

Dev 1.1.1#446
ujiro99 merged 43 commits into
mainfrom
dev-1.1.1

Conversation

@ujiro99

@ujiro99 ujiro99 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

No description provided.

ujiro99 and others added 19 commits August 8, 2026 12:09
Gemini側のUI変更でハードコードしたXPathセレクターが要素探索に失敗していたため、
ai-services.jsonでメンテナンスしている入力欄・送信ボタン・コピーボタンの
セレクターを実行時に取得して使用するようにした。コピー用セレクターが
未取得の場合(旧キャッシュ等)は現行のCSSセレクターにフォールバックする。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
searchUrl入力欄のonBlurでGeminiのMarkdown形式URLを生URLに変換した際、
setValueだけでは古い値に対するzodエラーが残ったままになっていたため、
clearErrorsを呼んでエラー表示をリセットするようにした。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Shows a one-time toast on the options page after a command is created,
inviting the user to share it to the Selection Command Hub. Once shown,
it is never shown again (hasShownHubShareToast flag), following the
same pattern as the existing review-request toast.

Closes #443

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EJVeN7n8jzatkUy99SGyhG
コマンドリストのボタン群をシンプル化し、Hubへのリンクは新規コマンド作成時の
種別選択ダイアログ右上にバナー形式で配置。リンクラベルは全言語のmessages.jsonに
Option_commandType_hubLinkキーを追加して多言語対応した。
Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>
初回シェア時、未登録ユーザーもダッシュボード経由でログイン画面にリダイレクトされ
自分で登録画面を探す必要があった。HUB_REGISTEREDフラグをローカルに永続化し、
未登録の場合はサインアップ画面を直接開くように変更。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
onAutoClose was unhandled so the "shown once" flag never persisted
when the toast timed out, letting it reappear on the next command.
Also adds unit tests for isHubShareable, HubShareToast, and the
new-command toast trigger branch in CommandList that were previously
uncovered.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nimation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oast.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
コマンド新規作成時、Hubへの共有を誘うToastを表示する
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaERoKexgDnvXJiXA3LGEB
@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.34926% with 287 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.59%. Comparing base (7564109) to head (d68b86c).
⚠️ Report is 44 commits behind head on main.

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 ⚠️
packages/extension/src/services/analytics.ts 76.56% 15 Missing ⚠️
packages/extension/src/action/aiPrompt.ts 96.23% 12 Missing ⚠️
packages/extension/tailwind.config.js 0.00% 11 Missing ⚠️
...src/components/option/editor/CommandEditDialog.tsx 0.00% 7 Missing ⚠️
...components/option/editor/SearchUrlAssistDialog.tsx 25.00% 6 Missing ⚠️
... and 7 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #446      +/-   ##
==========================================
+ Coverage   39.56%   46.59%   +7.02%     
==========================================
  Files         237      239       +2     
  Lines       25353    25710     +357     
  Branches     1886     2057     +171     
==========================================
+ Hits        10032    11979    +1947     
+ Misses      15321    13731    -1590     

☔ 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

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

レビュー結果

v1.1.1リリース向けの統合PR(複数コミット・複数機能を含む)を確認しました。全体的に実装は丁寧で、テストカバレッジも新規ロジックに対してよく作り込まれています。重大なバグは見つかりませんでしたが、いくつか気になった点を挙げます。

コード品質・保守性

  1. isHubShareable のロジック重複 (packages/extension/src/services/hubShare.ts:78-90packages/extension/src/components/option/ShareButton.tsx:70-73)
    isHubShareable() で共有可否を判定する一方、ShareButton.tsxhandleClick 内でも HUB_REGISTERED フラグを個別に読み直してUI状態(sent/idle)を分岐させています。背景スクリプト側(packages/extension/src/services/hub/background.ts:82-97)でも同じHUB_REGISTEREDチェックを行っており、「未登録ユーザーはサインアップ画面に誘導する」ロジックが2箇所に分散しています。実害はありませんが、将来的に片方だけ変更されると挙動がずれるリスクがあるため、判定ロジックを一箇所(例えばhubShare.ts)に集約することを検討してもよさそうです。

  2. AiService.copySelectors の型と実装の不整合 (packages/extension/src/types/index.ts:343 / packages/extension/src/services/searchUrlAssist.ts:19-24)
    型定義では copySelectors: string[](必須)としていますが、searchUrlAssist.ts 側では「同日キャッシュされた旧AiServiceにはこのフィールドが無い場合がある」という前提で service.copySelectors ?? [] のフォールバックを行っています。実行時の想定と型定義が矛盾しており、型だけを見た開発者が「常に存在する」と誤解する可能性があります。copySelectors?: string[] のようにoptionalにするか、コメントで「実行時は存在しない場合がある」ことを明示した方が安全です。

  3. CommandTypeSelectionDialog.tsx のフッター配置 (packages/extension/src/components/option/editor/CommandTypeSelectionDialog.tsx:101)
    DialogFooterabsolute bottom-6 right-6 を指定していますが、コマンド種別グリッド(space-y-6 py-4)が縦に長くなった場合(多言語で行が増えるケースなど)、絶対配置のキャンセルボタンが最後の行のカードに重なる可能性があります。実機での見た目確認をおすすめします。

テスト

  • HubShareToast.test.tsx / CommandList.hubShareToast.test.tsx / hubShare.test.ts / background.test.ts はいずれも新規ロジック(表示条件、onAutoClose、未登録ユーザーのリダイレクト等)を丁寧にカバーしており良い品質です。特に「onAutoCloseでもonShownが呼ばれることを確認するテスト」は、実際に過去にあった不具合(Toastがタイムアウトで消えると"表示済み"フラグが永続化されない)を狙い撃ちしていて良いテストだと思います。

セキュリティ

  • 新規追加の外部リンク(HubShareToast.tsx, CommandTypeSelectionDialog.tsx)はいずれも target="_blank" に対して rel="noopener noreferrer" が付与されており問題ありません。
  • chrome.tabs.create で開くサインアップURL (packages/extension/src/services/hub/background.ts:91) は NEW_HUB_URL 定数からの組み立てのみで、ユーザー入力を含まないため injection の懸念はありません。

その他(良かった点)

  • Gemini側のUI変更でXPathセレクターが壊れていた問題を、ai-services.json 側で一元管理するセレクターを実行時に取得する方式に切り替えた対応(packages/extension/src/services/searchUrlAssist.ts)は保守性向上として良い設計です。
  • SwitchField.tsxFormLabel からツールチップアイコンを外に出した修正(packages/extension/src/components/option/field/SwitchField.tsx:39-52)は、<label> 要素のクリック伝播でトグルが誤発火する典型的なバグへの的確な対処です。
  • 全ロケールファイル(14言語)で hub_share_toast_* / Option_commandType_hubLink キーが漏れなく追加されていることを確認しました。

全体として大きな懸念はなく、上記は軽微な改善提案です。マージ判断は開発チームにお任せします。

ujiro99 and others added 7 commits August 13, 2026 12:52
installed / option_screen_opened / hub_screen_opened / folder_create /
page_rule_create を新設し、command_add・selection_command はコマンド種別
(search/aiprompt/other)で分岐した新イベントに置き換えた。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
installed / option_screen_opened / hub_screen_opened / folder_create /
page_rule_create の分岐ロジックと command_create_*・hub_add_*・
selection_command_* のカテゴリ振り分けを検証するテストを追加。
new_command_hub.tsx はテスト容易性のため NewCommandHubBridge を
コンポーネントとして切り出した。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Selection Command Hub側のコマンド実行数集計
(ga4-client.ts)がイベント名 "selection_command" を前提にしており、
search/aiprompt/other への分割は不整合を招くため元に戻す。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Selection Command Hubにログインしたユーザーを識別できるよう、Supabaseセッションから
user_idを取得してHubUserとして保存し、GA4のsendEventで自動的にuser_idを送信するように
した。あわせてclient_id/user_idの取得をchrome.storage.localへの単一get呼び出しに
まとめ、session_idの取得と並列化して無駄なstorageアクセスを削減。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
getCommandCreateEvent/getHubAddEventの構造的重複をジェネリックヘルパー
createCategoryEventGetterに統合。あわせてgetCommandAnalyticsCategoryを
const.tsからanalytics.tsへ移動し、常にセットで使われる
getCommandCreateEvent/getHubAddEvent内に隠蔽することで凝集度を上げた。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GA4イベントを追加: アンインストールユーザーと継続ユーザーの利用状況比較
Closes #449

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown

レビュー結果

62ファイル変更・全体的にテストが手厚く追加されており(analytics.test.ts, HubShareToast.test.tsx, CommandList.hubShareToast.test.tsx, background.test.ts の signup 分岐など)、品質は高いです。以下、気になった点を挙げます。

1. [提案] console.debug が本番ビルドでも常時実行される

packages/extension/src/services/analytics.ts:107

export async function sendEvent(...) {
  console.debug(`[analytics] sendEvent: ${name}`, params, screen)   // ← ここ
  if (DISABLE_ANALYTICS || !MEASUREMENT_ID || !API_SECRET) {
    return
  }
  ...

DISABLE_ANALYTICS / isDebug のチェックより前に無条件で console.debug が呼ばれています。イベント名や params(コマンドの openModesource_type など)が本番ビルドでも常にコンソールに出力される形になるため、他の箇所(hub/background.ts など)と同様に isDebug でガードすることをお勧めします。

2. [提案] 「登録済みか」の判定ロジックが2箇所に重複

packages/extension/src/components/option/ShareButton.tsx:63-73packages/extension/src/services/hub/background.ts:82-96

ShareButton.tsx 側は shareCommandToHub()(fire-and-forgetでIPC送信するだけで、実際の送信結果は返さない)を呼んだ後、独自に Storage.get(LOCAL_STORAGE_KEY.HUB_REGISTERED) を読んでUIの表示("sent""idle" か)を決めています。一方で実際にサインアップ画面へリダイレクトするかどうかの判定は hub/background.tsshareCommandToHub 内で別途同じ HUB_REGISTERED を見て行われています。

同じ判定ロジックが2箇所に分散しているため、今後どちらか一方だけ条件を変更すると「UI上は共有成功と表示されるがバックグラウンドでは実際にはサインアップ画面に飛ばしていた」といった不整合が発生しやすい構造になっています。isHubShareable のように共通関数化して単一の場所に判定を寄せるとより堅牢になりそうです。

3. [情報] ShareButton のエラーハンドリングについて

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

shareCommandToHub() の戻り値 ok は、IPC送信が正常に行われたか(=有効なコマンドか)を示すだけで、バックグラウンド側の実際の共有処理が成功したかどうかは反映されません(hubShare.ts 内で Ipc.send(...).catch(...)console.error のみでエラーを握りつぶしています)。これは今回のPRで新規に入った挙動ではなく既存のパターンを踏襲したものですが、UI上は成功表示("sent")になる一方、実際の共有APIが失敗しても利用者には伝わらない点は留意事項として共有します。

4. [軽微] SearchUrlAssistDialog でAIサービスが見つからない場合、ユーザーへのフィードバックがない

packages/extension/src/components/option/editor/SearchUrlAssistDialog.tsx:79-83

const service = await findAiService(SEARCH_URL_ASSIST_SERVICE_ID)
if (!service) {
  console.error(`AI service not found: ${SEARCH_URL_ASSIST_SERVICE_ID}`)
  return
}

ai-services.json の取得に失敗した等で servicenull の場合、console.error だけでダイアログは何も起きたように見えません(finallyisProcessing は解除されるので無限ローディングにはなりませんが、ユーザーには何も通知されません)。エラー時にトースト等で軽くフィードバックがあるとよさそうです。

5. [良い点] Hub未登録ユーザーのサインアップ導線

packages/extension/src/services/hub/background.ts:82-96 で、Hub未登録ユーザーを共有フロー内でサインアップページへ誘導する変更は、コメントも丁寧で意図が明確です。対応する background.test.ts のテストケース(SH-02b)も追加されており、テストカバレッジも十分です。

6. [良い点] searchUrlAssist.ts のリファクタリング

Geminiのセレクタをハードコードではなく ai-services.jsonAiService)から取得するように変更した点(packages/extension/src/services/searchUrlAssist.ts)は、UI変更への耐性を高める良い改善です。copySelectors が存在しない場合のフォールバック (FALLBACK_COPY_SELECTOR) も適切に用意されています。

7. [軽微] 型安全性の改善

packages/extension/src/const.ts:43OPEN_MODE_TYPE_MAPsatisfies Record<OPEN_MODE, COMMAND_TYPE> が追加されたのは、型安全性向上の良い変更です。


全体所見

  • セキュリティ: 新規追加の外部リンク(CommandTypeSelectionDialog.tsx の Hubリンク)は rel="noopener noreferrer" が付与されており問題ありません。HubShareToast.tsx もユーザー入力を dangerouslySetInnerHTML 等で描画しておらずXSSの懸念はありません。
  • パフォーマンス: analytics.tsgetClientIdAndUserId()chrome.storage.local.get を1回にまとめた点、fetchkeepalive: true を追加した点(Service Worker終了時のイベントロス対策)は良い改善です。
  • テストカバレッジ: 新規ロジック(isHubShareable, getCommandCreateEvent, getHubAddEvent, HubShareToast 表示条件、shareCommandToHub のサインアップ分岐)に対してテストがしっかり追加されており、全体として質の高いPRです。

上記の指摘は概ね軽微〜提案レベルのもので、マージのブロッカーになるようなものはありませんでした。


🤖 Generated with Claude Code

…kip backup checks

Wraps the client_id fetch/setUninstallURL call in its own try/catch,
falling back to the plain uninstall URL on failure, and adds tests
that exercise the onInstalled listener end to end.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts:
#	packages/extension/src/background_script.test.ts
#	packages/extension/src/test/setup.ts
Add: client_id をアンインストールフォームURLに付与
@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown

コードレビュー(PR #446: Dev 1.1.1)

このPRは複数の機能(Hub共有トースト、GA4イベント追加、アンインストールURLへのclient_id付与、GeminiセレクターのAI Services JSON化など)をまとめたリリース統合PRのようです。全体としてテストカバレッジが非常に充実しており、コミット履歴を見ると既に何度かレビューコメントを反映した跡もあり、完成度は高いです。いくつか気になった点を挙げます。

1. getOrCreateClientIdgetClientIdAndUserId でロジックが重複している

packages/extension/src/services/analytics.ts:152-168(新設の getClientIdAndUserId)と packages/extension/src/services/analytics.ts:171-178(既存の getOrCreateClientId)で、client_id の「なければ生成して保存する」ロジックがほぼ同じ内容で2箇所に存在します。

  • getClientIdAndUserIdchrome.storage.local.get を直接呼び出す一方、getOrCreateClientIdStorage.get/Storage.set という抽象化レイヤーを経由しています。呼び出し方に一貫性がなく、今後どちらを使うべきか分かりにくくなります。
  • getOrCreateClientIdbackground_script.ts の uninstall URL 設定でも引き続き使われているため単純な一本化は難しいかもしれませんが、Storage 側に複数キーをまとめて取得するAPI(例: Storage.getMultiple)を用意して両方から使う形にすると重複が解消できそうです。

2. HubShareToast.tsxshareCommandToHub の戻り値(成功/失敗)を無視している

packages/extension/src/components/option/HubShareToast.tsx:1039-1049

ShareButton.tsx(packages/extension/src/components/option/ShareButton.tsx:49付近)では shareCommandToHub の戻り値 ok を見て成功/失敗を分岐していますが、トースト側では戻り値を確認せずに常に toast.dismissonShown() を呼んでいます。共有に失敗した場合でも「共有済み」のような見た目になり、かつ hasShownHubShareToast フラグが立って以後二度と案内が出なくなってしまいます。同じ関数を呼んでいるのに片方だけエラーハンドリングされていない状態です。

3. background_script.tsINSTALLED イベント送信が await されていない

packages/extension/src/background_script.ts:721

if (details.reason === chrome.runtime.OnInstalledReason.INSTALL) {
  await Settings.reset()
  sendEvent(ANALYTICS_EVENTS.INSTALLED, {}, SCREEN.SERVICE_WORKER)
}

同じリスナー内の少し下(background_script.ts:734-742)では getOrCreateClientId()setUninstallURLawait して丁寧にエラーハンドリングしているのに対し、sendEvent はfire-and-forgetです。Service Worker はイベントハンドラの完了後に終了され得るため、fetch が完了する前にワーカーが終了し、installed イベントの送信が失敗する可能性があります(sendEvent 内部で keepalive: true を付けているので緩和はされていますが、await した方が確実です)。

4. analytics.tsconsole.debug が本番でも常に実行される

packages/extension/src/services/analytics.ts:107(sendEvent の冒頭、DISABLE_ANALYTICS チェックより前)

console.debug(`[analytics] sendEvent: ${name}`, params, screen)

isDebug フラグでのガードがなく、すべてのビルドで常に呼ばれます。実害は小さいですが、必要性を再確認してもよいかもしれません。

5. CommandList.tsx の非同期処理にクリーンアップがない(軽微)

packages/extension/src/components/option/editor/CommandList.tsx:1882-1899

enhancedSettings.getSection(...).then(...) はコンポーネントのアンマウント後もPromiseが解決すれば showHubShareToast を呼び出します。Optionページはほぼ常にマウントされたままなので実害は小さいですが、念のため触れておきます。

良かった点

  • searchUrlAssist.ts(packages/extension/src/services/searchUrlAssist.ts): GeminiのUI変更でハードコードされたXPathが壊れていた問題を、ai-services.json 由来のセレクターを実行時取得する形に直したのは良い改善です。copySelectors が未取得の場合のフォールバック(FALLBACK_COPY_SELECTOR)もコメント付きで丁寧です。
  • background_script.ts の uninstall URL 設定(background_script.ts:734-742): getOrCreateClientId() の失敗を独立した try/catch で囲み、以降の日次/週次バックアップチェックをスキップしないようにした設計は堅牢で、テスト(UN-02)でも明示的に検証されています。
  • 新規追加コードに対して非常に手厚いテスト(HubShareToast.test.tsxCommandList.analytics.test.tsxCommandList.hubShareToast.test.tsxanalytics.test.ts など)が追加されており、GA4イベントのカテゴリ分岐やトーストの表示条件がきちんとカバーされています。
  • isHubShareable の抽出(hubShare.ts)により ShareButton.tsxCommandList.tsx で共有ロジックが重複しなくなった点も良いリファクタリングです。

セキュリティ観点では、GA4への user_id 送信(Supabaseの内部ID)や chrome.tabs.create で開くURLはいずれも定数由来かつユーザー入力を直接埋め込んでいないため、大きな懸念は見当たりませんでした。

ujiro99 and others added 8 commits August 14, 2026 13:45
ShareButton.tsx と hub/background.ts で個別に HUB_REGISTERED を判定していたため、片方だけ条件を変更すると UI 表示とバックグラウンドの実際の挙動が不整合になる恐れがあった。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
360行の単一関数に検証・要件分析・ステップ構築・ディスパッチが混在し、
Start/Endステップの組み立てが両分岐で重複していたため、処理の流れを
追いやすくし、ステップ生成の定型コードを createStep() に一本化した。
挙動は変更していない。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…it, add translations

HubShareToast now shows a party popper icon with popup animation and a
bold title above the message, so add the hub_share_toast_title key and
retranslate hub_share_toast_message across all locales.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

PRレビュー: Dev 1.1.1 (PR 446)

Hubへのコマンド共有導線(サインアップ誘導・共有トースト)、アナリティクスイベントの細分化、aiPrompt.ts の大規模なリファクタリングなど、広範な変更を含む大型PRです。全体的にテストが手厚く追加されており(aiPrompt.ts のロジック分割、analytics.ts のイベントマッピング、hubShare.ts の共有可否判定など)、既存挙動を壊さない丁寧なリファクタリングになっている点は良いと思います。以下、気になった点を挙げます。

1. [中] Hub共有時のアナリティクスイベントに一貫性がない

  • packages/extension/src/components/option/HubShareToast.tsx:74-84
  • packages/extension/src/components/option/ShareButton.tsx:46-84

ShareButton.tsx では、未登録ユーザーが shareCommandToHub() によってサインアップ画面へリダイレクトされるだけの場合(実際には共有されていない)、isHubRegistered() を確認したうえで COMMAND_SHARE イベントの送信をスキップするよう明示的に実装されています(コメントにもその意図が書かれています)。

一方、新設の HubShareToast.tsx の「共有する」ボタンの onClick(74-84行目)は isHubRegistered() のチェックを行わず、shareCommandToHub(command) 呼び出し後に無条件で sendEvent(ANALYTICS_EVENTS.COMMAND_SHARE, ...) を送信しています。同じ「Hubへ共有」という導線であるにもかかわらず、トースト経由の場合は未登録ユーザーがサインアップ画面に飛ばされただけのケースでも COMMAND_SHARE が計測されてしまい、分析データの正確性に影響しそうです。ShareButton.tsx と同様に isHubRegistered() を確認してからイベント送信する方が一貫性があると思います。

2. [軽微] client_id 生成ロジックが2箇所に重複している

  • packages/extension/src/services/analytics.ts:154-169(新設 getClientIdAndUserId
  • packages/extension/src/services/analytics.ts:171-178(既存 getOrCreateClientId

どちらも LOCAL_STORAGE_KEY.CLIENT_ID が未設定の場合に crypto.randomUUID() で生成・保存する、ほぼ同じ「generate-if-missing」ロジックを個別に持っています。sendEvent() は内部の getClientIdAndUserId() を使い、background_script.ts:464setUninstallURL 用)は引き続きエクスポートされた getOrCreateClientId() を使っています。

ロジックが分かれているため将来の変更漏れが起きやすいのに加え、理論上、初回インストール直後にこの2つの呼び出しがほぼ同時に発火した場合(onInstalled の uninstall URL 設定と、直後に発火する何らかの sendEvent)、両者が chrome.storage.local の読み取り時点でともに未設定と判定し、それぞれ別のUUIDを生成して後勝ちで上書きする競合が起こり得ます。発生頻度は低いと思いますが、getOrCreateClientId()getClientIdAndUserId() の内部で再利用するなど、生成ロジックを一本化しておくと安全だと思います。

3. [要確認] keepMenuOpenOnFocusChange の表示条件が変わっている

  • packages/extension/src/components/option/SettingForm.tsx:481-495

PopupAnimation<>...</> でラップする際に、直後にあった SwitchField (name="startupMethod.keepMenuOpenOnFocusChange") も同じ {startupMethod !== STARTUP_METHOD.CONTEXT_MENU && (...)} の条件ブロック内に取り込まれています。変更前はこのスイッチは startupMethod の値に関わらず常に表示されていましたが、変更後は STARTUP_METHOD.CONTEXT_MENU を選択している場合に非表示になります。

この設定はコンテキストメニュー起動時にも意味を持つように見える(background_script.ts 側の参照箇所は起動方法を問わず keepMenuOpenOnFocusChange を参照している)ため、単なるJSXの整形(Fragmentへの巻き込み)に伴う意図しない挙動変更の可能性があります。今回のPRの主目的(Hub連携・アナリティクス)とは無関係な変更に見えるので、意図的かどうかご確認いただけますでしょうか。関連するテストも見当たりませんでした。

4. [軽微] アンインストールURLへの client_id 付与について

  • packages/extension/src/background_script.ts:457-471

chrome.runtime.setUninstallURLclient_id(ランダムUUID)をクエリパラメータとして付与するようになりました。PIIではなく、try/catch でエラー時のフォールバックも用意されていて実装自体は堅牢ですが、外部URL(NEW_HUB_URL)にユーザー固有の識別子を送信する仕様変更になるため、プライバシーポリシーやストア審査上の記載が必要か念のためご確認いただくと良いと思います。

良かった点

  • packages/extension/src/action/aiPrompt.ts の大規模なリファクタリング(analyzePromptRequirements / buildQueryUrlSteps / buildDomInputSteps / runSidePanelAction などへの分割)は、挙動を保ったまま可読性・保守性を大きく改善しています。resolveOpenMode などの単純化も既存ロジックと等価であることを確認しました。
  • packages/extension/src/services/hubShare.tsisHubShareable / isHubRegistered によるHub共有可否判定の一元化は、ShareButton.tsxCommandList.tsx の両方から一貫して参照されており良い設計だと思います。
  • 新規追加ロジック(analytics.ts のイベントマッピング、hubShare.ts の共有可否、background_script.ts のインストール時イベント送信など)に対するテストが手厚く追加されている点も良いです。

以上、大きなバグは見当たりませんでした。1点目(アナリティクスの一貫性)と3点目(表示条件変更の意図確認)を中心にご検討いただければと思います。


このレビューはClaude Codeによって自動生成されました。

ujiro99 and others added 4 commits August 17, 2026 17:36
…in CI

yarn.lock resolved react-slot to 1.3.3 for both workspaces, which caused
Yarn's hoisting to nest a React 19 copy inside the shared react-slot
package even though the extension workspace uses React 18. This broke
Radix Slot components in tests ("Objects are not valid as a React child").

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ered users

Aligns with ShareButton.tsx, which already skips the event when the user
is redirected to the sign-up page instead of actually sharing.
…ser case

Fixes HST-05 failure caused by the previous commit's added isHubRegistered
call not being mocked. Also adds HST-07 covering the skip-analytics path.
@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

PR #446 コードレビュー

18コミットにまたがる大きめの統合PR(Hub共有トースト、検索URLアシストのGemini対応、GA4イベント追加、アンインストールURLのclient_id付与、オプション画面UI調整など)のため、機能領域ごとに分けてコードを確認しました。全体的にテストが手厚く追加されている点は良い一方、いくつか見落としやすいバグ・回帰リスクが見つかりました。特に🔴の2点は動作に影響するため確認をお願いします。

🔴 高: sendEvent(INSTALLED)getOrCreateClientId() の競合でclient_idが分岐する可能性

packages/extension/src/background_script.ts:447sendEvent(ANALYTICS_EVENTS.INSTALLED, ...)await されていないfire-and-forget呼び出しです。内部で analytics.ts:154-169getClientIdAndUserId()CLIENT_ID 未設定なら crypto.randomUUID() を生成して保存します。
一方、その直後(background_script.ts:464)で await getOrCreateClientId()(analytics.ts:171-178)が呼ばれ、こちらも独立に「未設定なら生成して保存」を行います。
両者はロックなしの read-then-write のため、初回インストール時にタイミング次第で異なるUUIDが生成され、後勝ちで上書きされる可能性があります。結果として installed イベントで送信されたclient_idと、アンインストールURLに付与されるclient_idが食い違い、本PRの目的である「インストール/アンインストールの相関分析」が成立しなくなる恐れがあります。
getOrCreateClientId() を唯一の入口にして getClientIdAndUserId() から再利用する、または生成処理をin-flight Promiseでキャッシュするなど、生成経路の一本化を推奨します。
なお background_script.test.ts はこの2つの呼び出しをモジュールごとモックしているため、この競合はテストで検出できません。

🔴 高: HubShareToastisHubRegistered() が失敗するとトーストが固まる

packages/extension/src/components/option/HubShareToast.tsx:74-94 のShareボタンの onClick はasync関数ですが、try/catchがありません。shareCommandToHub(command) の後 await isHubRegistered() が何らかの理由(storageエラー等)でrejectすると、toast.dismiss(toastId)onShown() が実行されず、duration: 60 * 1000onAutoClose が発火するまでトーストが表示され続け、コンソールには未処理のPromise rejectionが出ます。
.catch() を追加し、失敗時も toast.dismiss/onShown を確実に呼ぶよう修正することを推奨します。対応するテスト(isHubRegistered() reject時にトーストが閉じることを検証するケース)も見当たりませんでした。

🟡 中: HUB_REGISTERED のマイグレーション漏れ

packages/extension/src/services/storage/const.ts:21 の新規キー HUB_REGISTERED は既定値 false(storage/index.ts:78)で、handleSetSession(packages/extension/src/services/hub/background.ts:538)でサインイン時にのみ true がセットされます。
本PR以前から HUB_USER を保有している既存ユーザー(既にHubへログイン済み)は、アップデート後も再サインインするまで HUB_REGISTERED=false のままです。この状態で ShareButton/HubShareToast から共有すると、isHubRegistered() がfalseを返すため誤って /auth/signup へ誘導され(hub/background.ts:84付近)、COMMAND_SHARE イベントも発火しません。HUB_USER の有無から HUB_REGISTERED をバックフィルする移行処理の追加を検討してください。

🟡 中: Gemini検索URLアシストでサービス未取得時のユーザーフィードバックが無い

packages/extension/src/components/option/editor/SearchUrlAssistDialog.tsx:78-82handleGeminiExecutionfindAiService("gemini")undefined を返す場合(ai-services.jsonの検証で該当エントリがskipされた場合など)、console.error を出すだけで処理を中断します。ダイアログは開いたままで isProcessing がfalseに戻るのみで、ユーザーには何も表示されません。以前はハードコードされた静的定義だったため発生しなかった失敗経路です。トースト等でのエラー通知を検討してください。

🟡 中: searchUrlAssist.ts のセレクターフォールバックの非対称性

packages/extension/src/services/searchUrlAssist.ts:27-33 で、copySelectors のみ FALLBACK_COPY_SELECTOR によるフォールバックがありますが、inputSelectors/submitSelectors には同様の保険がありません。現状は aiPromptFallback.tsnormalizeServices 側でinput/submitセレクターが両方非空でないとエントリごとskipされるため実害はありませんが、その検証条件が将来変わった場合に気づきにくい構造です。
また、これまでハードコードのXPathだったクリック/入力対象が、実行時取得・キャッシュされる ai-services.json 由来のCSSセレクターに置き換わった点は、外部データへの信頼度が実質的に上がったことを意味します(ハブが侵害された場合、意図しない要素へのクリック/入力を誘導される余地)。機能としては妥当ですが認識しておくとよいと思います。

🟡 中: CommandTypeSelectionDialog のフッターが絶対配置でコンテンツと重なる可能性

packages/extension/src/components/option/editor/CommandTypeSelectionDialog.tsx:101 の新設キャンセルボタン(<DialogFooter className="absolute bottom-6 right-6">)は position: absolute のためドキュメントフローの高さに寄与しません。直前の <div className="space-y-6 py-4">(82行目、コマンド種別カード一覧)には下部余白が確保されておらず、カード数が多い・ビューポートが低い環境では最後のグループがボタンの下に隠れてクリックできなくなる恐れがあります。コンテンツ側に十分な padding-bottom を追加するか、DialogFooter を通常のフローに戻すことを推奨します。

🟢 低: その他軽微な指摘

  • packages/extension/src/components/option/editor/CommandList.tsx:257-259showHubShareToastonShown コールバック内 Settings.update("hasShownHubShareToast", ...) の戻り値Promiseが未処理です。同ファイルの他の非同期処理は.catch()しており一貫性がありません。
  • packages/extension/src/components/option/field/SwitchField.tsx のツールチップクリック修正(FormLabelからアイコンを外に出す対応)は正しい修正に見えますが、回帰防止テスト(SwitchField.test.tsx)がありません。
  • packages/extension/src/services/analytics.ts:154-169 の新規 getClientIdAndUserId() に対する単体テストが見当たりません。特に旧データで hubUser.idundefined の場合の挙動を確認するテストがあると安心です。
  • packages/extension/package.json:58"sonner": "git+https://github.com/ujiro99/sonner.git" はコミット/タグを指定していません。yarn.lock 側では固定コミットに解決されCIも--frozen-lockfileのため現状問題ありませんが、lockfile再生成時に意図せず別コミットを取得するリスクがあるため、可能であれば package.json 側にも #<commit-sha> を明記すると安全です。
  • packages/extension/src/services/analytics.ts:107console.debugisDebug フラグに関係なく常時出力されます(他の主要ログは isDebug でガードされています)。送信データと同一内容のため情報漏洩リスクは低いですが、本番ビルドでのノイズになり得ます。

👍 良かった点

  • hubShare.tsisHubShareable()/toSubmitCommandInput()/getHubLocale() へのロジック集約と、それに伴うテストは網羅的でした。
  • Option.tsx のStrictMode二重発火ガード(hasSentScreenOpenedRef)は適切に実装・テストされています。
  • CommandTypeSelectionDialog.tsx へのCommand Hubリンク移動に伴うローカライズキー Option_commandType_hubLink は14言語すべての messages.json に漏れなく追加されていました。
  • @radix-ui/react-slot のバージョンピン(ルート package.json)とPlaywright Dockerイメージのバージョン整合は妥当で、yarn.lock にも正しく反映されています。
  • aiPrompt.ts のフェーズ別ヘルパー関数への分割は既存ロジックと等価であることを確認できました。

以上、大きめのPRですがコアな回帰リスクは上記2件(🔴)に集中しているようです。よろしければご確認ください。


🤖 このレビューは Claude Code により自動生成されました。

ujiro99 and others added 2 commits August 18, 2026 10:26
Pinning react-slot to 1.2.4 in root resolutions forced extension's
@radix-ui/react-select (which needs 1.3.3) down to a version whose
Slot implementation isn't ref-memoized, causing an infinite
setState/re-render loop ("Maximum update depth exceeded") in the
options page Select component.

Removed hub's Button dependency on Slot/asChild (unused) so hub no
longer needs @radix-ui/react-slot at all, letting the whole monorepo
resolve to a single 1.3.3 instance without cross-workspace hoisting
conflicts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ujiro99
ujiro99 merged commit 7e0fb4d into main Aug 18, 2026
3 of 4 checks passed
@ujiro99
ujiro99 deleted the dev-1.1.1 branch August 18, 2026 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants