Skip to content

Feat/Share command to the new Hub#392

Merged
ujiro99 merged 9 commits into
mainfrom
feat/hub-submit-command
May 3, 2026
Merged

Feat/Share command to the new Hub#392
ujiro99 merged 9 commits into
mainfrom
feat/hub-submit-command

Conversation

@ujiro99

@ujiro99 ujiro99 commented May 2, 2026

Copy link
Copy Markdown
Owner

No description provided.

@claude

claude Bot commented May 2, 2026

Copy link
Copy Markdown

コードレビュー

概要

新しい Hub(selection-command-hub.pages.dev)へのコマンド共有機能を追加するPRです。window.postMessage を利用したリトライ + ack プロトコルで安全にメッセージを送信し、15言語の i18n 対応も含まれています。全体的に設計はよく練られており、テストも充実しています。


🐛 バグ・問題点

1. CommandList.tsx — コピー時のsourceType未設定(重要)

packages/extension/src/components/option/editor/CommandList.tsx L248–L253

-    cmd.sourceType = COMMAND_SOURCE_TYPE.SELF_CREATED
-    cmd.sourceId = COMMAND_SOURCE_ID.SELF_CREATED

この行が削除されたことにより、Hub コミュニティから取得したコマンド(sourceType: HUB_COMMUNITY)をコピーしても、コピー元の sourceType がそのまま引き継がれます。ShareButton.tsxVALID_SOURCE_TYPES チェック(L568–L572)で HUB_COMMUNITY が弾かれるため、未編集のコピーには共有ボタンが表示されません

  • ユーザーが「Hub から取得→コピー→カスタマイズして共有」という用途を想定する場合、コピー直後に編集して初めて SELF_UPDATED になるため問題は軽減されますが、コピー直後の状態が旧来の動作と変わることは破壊的変更の可能性があります。
  • 意図的な変更であれば、設計ドキュメントまたはコメントで理由を明記することを推奨します。

2. SubmitCommandInput.targetUrl の型不整合

packages/extension/src/services/hubShare.ts L1154, L1215

export interface SubmitCommandInput {
  targetUrl: string  // ← string 型
  ...
}

しかし PAGE_ACTION の変換処理では:

const targetUrl = pa.pageActionOption?.startUrl ?? null  // ← null になり得る

string と宣言されているのに null を代入しています。targetUrl: string | null に修正が必要です。テスト PA-02 でも null を期待しているため、型と実装が乖離しています。


3. getHubLocale()shareCommandToHub() 内で二重呼び出し

packages/extension/src/services/hubShare.ts L1259

toSubmitCommandInput() 内部(L1193)でも getHubLocale() が呼ばれており、shareCommandToHub() 内で再度呼び出しています(L1259)。結果は同じになりますが、一方を他方から取得するよう整理できます:

const input = toSubmitCommandInput(command)
// input.locale を使い回す
const hubUrl = `${NEW_HUB_URL}/${input.locale}/dashboard/commands`

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

4. ポルトガル語のロケール解決順序

packages/extension/src/const.ts L753–L767

NEW_HUB_SUPPORTED_LOCALES の順序は "pt-BR""pt-PT" より先です。ブラウザ言語が "pt"(汎用ポルトガル語)の場合、前方一致で常に "pt-BR" にフォールバックします。ポルトガル(pt-PT)ユーザーへの影響を考慮してください。


5. useEffect での setValue 冗長呼び出し

packages/extension/src/components/option/editor/CommandEditDialog.tsx L649–L658

useEffect(() => {
  if (!initialized) return
  if (!isUpdate) return
  if (!isSearchType(command)) return
  if (command.sourceType === COMMAND_SOURCE_TYPE.SELF_CREATED) return
  if (searchUrl !== command.searchUrl) {
    setValue("sourceType", COMMAND_SOURCE_TYPE.SELF_UPDATED)
    ...
  }
}, [initialized, isUpdate, searchUrl, command, setValue])

command.sourceType がすでに SELF_UPDATED の場合でも、searchUrl が変わるたびに setValue が再呼び出しされます。SELF_UPDATED チェックを追加するか、React Hook Form の getValues() で現在値を確認してから setValue することで無駄な再レンダーを防げます。


6. postMessage のエラーハンドリング

packages/extension/src/services/hubShare.ts L1290–L1297

try {
  hubWindow.postMessage(...)
} catch {
  // hub ウィンドウがまだロード中 → 次のリトライへ
}

すべての例外をサイレントに無視しています。ウィンドウがロード中以外の理由(セキュリティエラー等)でも同じ処理になります。少なくとも instanceof DOMException など特定のエラーのみをキャッチし、それ以外はログに出力することを検討してください。


7. OPEN_MODE.AI_PROMPT が設計ドキュメントに未記載

packages/extension/src/const.ts L772–L780

export const NEW_HUB_SHAREABLE_OPEN_MODES: ReadonlySet<string> = new Set([
  ...
  OPEN_MODE.AI_PROMPT,  // 設計ドキュメントには記載なし
])

docs/composed-swimming-island.md の設計では AI_PROMPT は含まれていませんでしたが、実装では追加されています。意図的なスコープ拡張であればドキュメントへの反映を推奨します。


✅ 良い点

  • ack ベースのリトライ設計hubShare.ts L1269–L1298): setInterval + share-command-ack でリトライを停止するパターンは堅牢で、競合状態も適切に処理されています。
  • targetOrigin"*" を使用していない(L1292): セキュリティ上の良いプラクティスです。
  • テストカバレッジ: hubShare.test.ts でロケール解決・変換・共有フローが網羅されており、タイマーの fake も適切に使用されています。
  • sourceType による共有ガードShareButton.tsx L568–L593): Hub コミュニティコマンドの再共有を防ぐロジックは適切です。
  • 15言語の i18n 対応: 全言語に翻訳が追加されています。

まとめ

機能の方向性と実装品質は高く評価できます。上記のうち 型不整合(#2コピー時の sourceType 挙動(#1 は破壊的変更または実行時エラーになる可能性があるため、マージ前の確認を推奨します。

@codecov

codecov Bot commented May 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.06452% with 104 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.66%. Comparing base (2287b48) to head (8460805).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
...es/extension/src/components/option/ShareButton.tsx 0.00% 52 Missing ⚠️
...src/components/option/editor/CommandEditDialog.tsx 0.00% 35 Missing ⚠️
packages/extension/src/services/hubShare.ts 89.84% 13 Missing ⚠️
...c/components/option/editor/CommandTreeRenderer.tsx 0.00% 3 Missing ⚠️
...nsion/src/components/option/editor/CommandList.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #392      +/-   ##
==========================================
+ Coverage   26.42%   26.66%   +0.23%     
==========================================
  Files         312      314       +2     
  Lines       31700    31956     +256     
  Branches     1646     1680      +34     
==========================================
+ Hits         8378     8522     +144     
- Misses      23322    23434     +112     

☔ 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

claude Bot commented May 2, 2026

Copy link
Copy Markdown

PR レビュー: Feat/Share command to the new Hub

全体的に良く構成されたPRです。新しい Hub へのコマンド共有機能が適切に設計されており、postMessage プロトコルの実装も理にかなっています。以下にいくつかの指摘事項をまとめます。


🔴 バグ・型エラー

1. SubmitCommandInput の型定義と実装の不一致

packages/extension/src/services/hubShare.ts:29 / :96

インターフェースは targetUrl: string を定義していますが、PAGE_ACTION で startUrl が未設定の場合に null を返しています:

// hubShare.ts:29 - インターフェース定義
export interface SubmitCommandInput {
  targetUrl: string  // ← string と定義
  ...
}

// hubShare.ts:96 - 実装
const targetUrl = pa.pageActionOption?.startUrl ?? null  // ← null になりえる

テストの PA-02 も targetUrlnull になることを期待しており、型定義と矛盾しています。

修正案:

export interface SubmitCommandInput {
  targetUrl: string | null
  ...
}

🟡 設計上の懸念事項

2. onAck リスナーのメモリリーク可能性

packages/extension/src/services/hubShare.ts:128-165

Hub ウィンドウがユーザーによって手動で閉じられた場合、onAck のイベントリスナーが MAX_RETRIES(20回 = 10秒)を超えるまで残り続けます。共有操作を連続して行った場合、複数のリスナーが蓄積する可能性があります。

// cleanup が呼ばれるのは ack 受信時か MAX_RETRIES 超過時のみ
// ウィンドウが閉じられた場合のクリーンアップがない
window.addEventListener("message", onAck)

修正案: Hub ウィンドウが閉じられたことを検知する仕組み(例: hubWindow.closed のポーリング)を追加するか、AbortController を使用してリスナー管理を改善することを検討してください。

3. useEffectcommand 依存による過剰実行リスク

packages/extension/src/components/option/editor/CommandEditDialog.tsx:462-503

3つの useEffect いずれも command オブジェクト全体を依存配列に含めています。親コンポーネントがレンダリングのたびに新しいオブジェクト参照を渡す場合、エフェクトが不必要に再実行されます:

useEffect(() => {
  ...
  if (searchUrl !== command.searchUrl) {
    setValue("sourceType", COMMAND_SOURCE_TYPE.SELF_UPDATED)
  }
}, [initialized, isUpdate, searchUrl, command, setValue])  // ← command 全体

修正案: 必要なプロパティだけを依存に指定するか、useMemo で値を安定化させることを検討してください:

}, [initialized, isUpdate, searchUrl, command.searchUrl, command.sourceType, setValue])

4. コピー時の sourceType 設定削除の影響

packages/extension/src/components/option/editor/CommandList.tsx:248-252

-    cmd.sourceType = COMMAND_SOURCE_TYPE.SELF_CREATED
-    cmd.sourceId = COMMAND_SOURCE_ID.SELF_CREATED

DEFAULTHUB_COMMUNITY のコマンドをコピーした場合、複製したコマンドに Share ボタンが表示されません(VALID_SOURCE_TYPES にこれらが含まれないため)。ユーザーが編集すれば SELF_UPDATED になりますが、編集なしでそのまま共有したい場合はできません。

これが意図的な設計であれば、コメントやドキュメントで明示することを推奨します。


🟡 テストの懸念事項

5. テスト SH-05 での定数のインライン定義

packages/extension/src/services/hubShare.test.ts:252-254

// テスト内でインライン定義している
const NEW_HUB_URL =
  import.meta.env.VITE_NEW_HUB_URL ??
  "https://selection-command-hub.pages.dev"

他のテストと一貫性を保つため、@/const から NEW_HUB_URL をインポートして使用することを推奨します。


🟢 軽微な指摘事項

6. .env.development のポート競合

packages/extension/.env.development:1

VITE_NEW_HUB_URL=http://localhost:3000

旧 Hub の HUB_URL (デバッグ時) も http://localhost:3000 を使用しています(const.ts:322)。開発環境で両 Hub を同時に起動する場合はポートが競合します。設計ドキュメントでは localhost:3001 が記載されていました。意図的な変更であれば問題ありませんが、確認をお勧めします。

7. CommandTreeRenderer.tsx の型キャスト

packages/extension/src/components/option/editor/CommandTreeRenderer.tsx:72

{isCommand(field.content) && (
  <ShareButton command={field.content as SelectionCommand} />
)}

isCommand() でチェック済みのため実用上は安全ですが、isCommand の型ガードが SelectionCommand を返すよう改善できれば as キャストが不要になります。

8. 設計ドキュメントと実装の乖離

docs/composed-swimming-island.md

設計ドキュメントには旧実装(clearInterval を postMessage 成功後に即実行)が記載されていますが、実際の実装は share-command-ack による双方向通信を使用しています。また、OPEN_MODE.AI_PROMPT がドキュメントに記載がないまま実装されています。ドキュメントを最新の実装に合わせて更新するか、内容を PR 説明に移動してリポジトリから除去することを検討してください。


✅ 良い点

  • postMessagetargetOriginNEW_HUB_URL を指定し、"*" を避けているセキュリティ面の配慮が良い
  • ACK によるリトライ停止で無駄な postMessage を防ぐ設計が優れている
  • VALID_SOURCE_TYPES によって Hub コミュニティのコマンドを再共有できないよう制御している点が適切
  • COMMAND_SOURCE_TYPE.SELF_UPDATED の追加と CommandEditDialog.tsx での変更追跡ロジックが整合している
  • 15言語すべてに i18n キーが追加されている点が丁寧
  • テストカバレッジが充実しており、主要なケースが網羅されている

@claude

claude Bot commented May 2, 2026

Copy link
Copy Markdown

コードレビュー: Feat/Share command to the new Hub

全体的に設計がよく考えられており、テストカバレッジも充実しています。以下にフィードバックをまとめます。


🔴 バグ・重大な問題

1. AiPromptCommand の変更追跡が不完全 (CommandEditDialog.tsx)

aiPromptOption.prompt のみを監視しており、serviceIdopenMode の変更は追跡されていません。

// CommandEditDialog.tsx (追加された useEffect, ~497行目付近)
if (aiPromptPrompt !== command.aiPromptOption.prompt) {
  setValue("sourceType", COMMAND_SOURCE_TYPE.SELF_UPDATED)
  setValue("sourceId", COMMAND_SOURCE_ID.SELF_UPDATED)
}

ユーザーが AI サービス(例: Gemini → ChatGPT)を変更してもプロンプトを変えなかった場合、sourceTypeSELF_UPDATED に更新されず、共有ボタンが表示されません。aiPromptOption 全体を useWatch で取得し、JSON.stringify で比較する方法(pageActionOption と同様のアプローチ)が一貫性の面で望ましいです。

2. SearchCommand の変更追跡が不完全 (CommandEditDialog.tsx)

searchUrl のみ追跡しており、openModeSecondaryspaceEncoding の変更が追跡されていません。

// CommandEditDialog.tsx (~462行目付近)
if (searchUrl !== command.searchUrl) {
  // openModeSecondary / spaceEncoding の変更はここに到達しない

ユーザーが二次開放モードやスペースエンコーディングを変更しても SELF_UPDATED に更新されません。

3. ハブウィンドウがクローズされても再試行が継続する (hubShare.ts)

// packages/extension/src/services/hubShare.ts (~149行目)
const timer = setInterval(() => {
  retries++
  if (retries > MAX_RETRIES) { ... }
  try {
    hubWindow.postMessage(...)
  } catch (err) {
    if (err instanceof DOMException) {
      // ローディング中として扱い再試行 → ウィンドウが閉じられた場合も同様
    }
  }
}, RETRY_INTERVAL_MS)

ユーザーがハブタブを閉じると hubWindow.closed === true になりますが、DOMException として処理されるため再試行が 10 秒間継続します。以下のチェックを追加すべきです:

if (hubWindow.closed) {
  cleanup()
  return
}

🟡 UX・設計の問題

4. タイムアウト時に誤った "送信済み" ステータスを表示 (ShareButton.tsx)

// packages/extension/src/components/option/ShareButton.tsx (26-28行目)
const ok = shareCommandToHub(command)
setStatus(ok ? "sent" : "error")
setTimeout(() => setStatus("idle"), 2000)

shareCommandToHub()window.open が成功した時点で true を返しますが、実際のメッセージ送信はその後の非同期処理です。ハブがレスポンスしなくても(タイムアウト・ウィンドウクローズ)、UIは緑のチェックを表示し続けます。ハブからの share-command-ack を受け取ってから成功表示するよう、コールバック方式への変更を検討してください。

5. コマンド複製時の sourceType 維持 (CommandList.tsx)

// packages/extension/src/components/option/editor/CommandList.tsx (旧 250-251行目、削除)
- cmd.sourceType = COMMAND_SOURCE_TYPE.SELF_CREATED
- cmd.sourceId = COMMAND_SOURCE_ID.SELF_CREATED

HUB_COMMUNITY コマンドを複製すると、コピーも sourceType: "hubCommunity" を保持するため共有ボタンが表示されません。ユーザーが複製後にコマンドを編集しない限り、共有できないことに気づかない可能性があります。これが意図的な設計であれば、コードコメントで明記することを推奨します。

6. 連続クリックによる複数 event listener の蓄積 (hubShare.ts)

// packages/extension/src/services/hubShare.ts (~133行目)
window.addEventListener("message", onAck)

ユーザーが短時間に複数回クリックすると、複数の onAck リスナーが同時に存在します。各クリーンアップで対応するリスナーは削除されますが、同時に複数のポーリングループが動作するリスクがあります。ShareButton.tsx でクリック中は無効化するか、shareCommandToHub が既に実行中の場合に早期リターンする処理を追加してください。


🔵 軽微な問題

7. iconUrl の空文字列が正規化されていない (hubShare.ts)

// packages/extension/src/services/hubShare.ts (~79行目)
const baseInput = {
  title,
  iconUrl,  // 空文字列がそのまま渡される可能性がある
  openMode,
  locale: getHubLocale(),
}

設計ドキュメントでは iconUrl || undefined を使用するとされていましたが、実装では空文字列がそのまま Hub へ送信されます。iconUrl: iconUrl || undefined に変更してください。

8. docs/composed-swimming-island.md の本番リポジトリへのコミット

このファイルは実装計画書であり、通常は PR の説明文やプロジェクト管理ツールに置くべき内容です。また、ファイル名(composed-swimming-island.md)がランダムに見え、意図が伝わりにくいです。リポジトリへのコミットは避けることを推奨します。

9. .env.development のポート番号の不整合

# packages/extension/.env.development
VITE_NEW_HUB_URL=http://localhost:3000

ドキュメント(docs/composed-swimming-island.md)では localhost:3001 と記載されています。どちらが正しいか確認が必要です。

10. テストの不足: navigator.language フォールバック (hubShare.test.ts)

// packages/extension/src/services/hubShare.test.ts
describe("getHubLocale", () => { ... })

chrome.i18n.getUILanguage()null を返した際に navigator.language を使用するフォールバックのテストがありません。


✅ 良い点

  • postMessagetargetOrigin"*" ではなく NEW_HUB_URL を指定しており、セキュリティ的に正しい実装です
  • share-command-ack による確認応答でポーリングを停止する仕組みは優れた設計です
  • 15 言語への i18n 対応が網羅されています
  • テストケースの命名規則(GL-01, SC-01, etc.)が体系的で読みやすいです
  • COMMAND_SOURCE_ID.SELF_UPDATED に適切な UUID が割り当てられています

まとめ

主要な懸念点は以下の 2 点です:

  1. AiPromptCommand と SearchCommand の変更追跡の不完全さ(共有ボタンが正しく表示されないケースが存在する)
  2. ウィンドウクローズ時の再試行継続(軽微なリソース無駄遣いと UX 問題)

その他の点は比較的軽微ですが、特に ShareButton の誤った成功表示は UX に直接影響するため対応を検討してください。

@claude

claude Bot commented May 3, 2026

Copy link
Copy Markdown

コードレビュー

概要

新しい Selection Command Hub(selection-command-hub.pages.dev)へコマンドを共有する機能を追加するPRです。window.postMessage プロトコルを使用し、オプションページから直接Hubタブへデータを送信します。また、COMMAND_SOURCE_TYPE.SELF_UPDATED の追加によりコマンドの変更追跡機能も実装されています。


🔴 重要な問題

1. CommandList.tsx — コピー時の sourceType 未設定(行 251-252 削除部分)

packages/extension/src/components/option/editor/CommandList.tsxduplicateCommand 関数から以下が削除されています:

- cmd.sourceType = COMMAND_SOURCE_TYPE.SELF_CREATED
- cmd.sourceId = COMMAND_SOURCE_ID.SELF_CREATED

この変更により、Hubからダウンロードしたコマンド(HUB_COMMUNITY)をコピーした場合、コピーも HUB_COMMUNITY のままになります。ShareButton はこのSourceTypeを除外するため、Hubコマンドのコピーは編集しない限りShareボタンが表示されません。

意図的な変更であれば、コメントで理由を明記してください。そうでない場合、もともと SELF_CREATED を設定していたことには理由があったはずです(コピーしたものはユーザーが作成したとみなす)。

2. hubShare.tsnavigator.language フォールバックがデッドコード(行 34-36)

// packages/extension/src/services/hubShare.ts
const lang = (
  chrome.i18n.getUILanguage() ??
  navigator.language ??   // ← 到達不可能
  "en"
).toLowerCase()

chrome.i18n.getUILanguage()null/undefined を返さず、常に string(未設定時は "")を返します。?? 演算子は null/undefined のみチェックするため、navigator.language は絶対に評価されません。

修正案:

const uiLang = chrome.i18n.getUILanguage()
const lang = (uiLang || navigator.language || "en").toLowerCase()

🟡 中程度の問題

3. hubShare.ts — モジュールレベルの isSharing シングルトン(行 109)

// packages/extension/src/services/hubShare.ts
let isSharing = false

モジュールスコープの状態であるため、テストで isSharing をリセットできません。テスト SH-04 では2回呼び出しが起きますが、isSharing=true のまま2回目を呼ぶとブロックされます。現在のテストがこれを考慮しているか確認が必要です。また、cleanup が何らかの理由で未実行になった場合(拡張機能の予期しない再読み込みなど)、共有が永続的にブロックされます。

4. hubShare.tsiconUrl の空文字列が正規化されない(行 83)

// packages/extension/src/services/hubShare.ts
const baseInput = {
  title,
  iconUrl,  // ← "" (空文字列) がそのまま Hub API に送られる可能性
  openMode,
  locale: getHubLocale(),
}

SubmitCommandInputiconUrl?: string は省略可能ですが、空文字列が渡される場合があります。

修正案:

iconUrl: iconUrl || undefined,

5. CommandEditDialog.tsx — ほぼ同一の useEffect が3つ(行 462-505)

searchUrlpageActionOptionaiPromptOption.prompt の変更を検知する3つのエフェクトが、ほぼ同じ構造で記述されています。現在の実装は動作しますが、今後コマンドタイプが増えた場合にメンテナンスが困難になります。共通のヘルパー関数化を検討してください。

6. docs/composed-swimming-island.md — 設計ドキュメントをリポジトリにコミット

docs/composed-swimming-island.md は実装計画ドキュメントです。通常、このような設計文書はPR説明欄や別ドキュメントリポジトリで管理し、ソースコードリポジトリには含めません。最終的なマージ前に削除することを推奨します。


🟢 軽微な問題

7. CommandTreeRenderer.tsx — 不要な型キャスト(行 71)

// packages/extension/src/components/option/editor/CommandTreeRenderer.tsx
{isCommand(field.content) && (
  <ShareButton command={field.content as SelectionCommand} />  // ← キャスト
)}

isCommand が型述語 (field is SelectionCommand など) であれば、as SelectionCommand は不要です。型定義を確認してください。

8. .env.development — 旧Hubと同じポート番号

# packages/extension/.env.development
VITE_NEW_HUB_URL=http://localhost:3000

const.tsHUB_URL も開発時は http://localhost:3000 を使用しています。両方のHubを同時にローカルで開発する場合、ポートが競合します。ポート番号を分けること(例: 3001)を検討してください(設計ドキュメントには 3001 と記載されていますが、実装では 3000 になっています)。

9. getHubLocale()pt プレフィックスの解決順

// packages/extension/src/services/hubShare.ts
// NEW_HUB_SUPPORTED_LOCALES は辞書順: ["de", "en", ..., "pt-BR", "pt-PT", ...]
// "pt" → 最初にマッチする "pt-BR" になる

ブラウザ言語が "pt"(地域なし)の場合、アルファベット順で先に並ぶ pt-BR にマッチします。コメントで言及されていますが、pt-PT ユーザーには意図しない結果になる可能性があります。許容範囲であればコメントだけで十分ですが、ptpt-PT のような明示的なマッピングも選択肢です。


✅ 良い点

  • セキュリティ: postMessagetargetOrigin"*" ではなく NEW_HUB_URL を使用している点は正しいセキュリティ対応です
  • ACK機構: share-command-ack を受信してリトライを停止する設計は堅牢です
  • 閉じられたウィンドウの検出: hubWindow.closed チェックで不要なリトライを防いでいます
  • テストカバレッジ: hubShare.test.ts のテストケースが充実しており、主要なシナリオを網羅しています
  • i18n: 15言語への翻訳追加が完備されています
  • フィルタリング: VALID_SOURCE_TYPES によるデフォルトコマンドの除外が適切に実装されています

まとめ

機能の方向性と設計は適切です。マージ前に以下の対応を推奨します:

  1. navigator.language フォールバックの修正(デッドコード)
  2. iconUrl の空文字列正規化
  3. 設計ドキュメント(docs/composed-swimming-island.md)の削除
  4. .env.development のポート番号確認(3000 vs 3001
  5. CommandList.tsxsourceType 削除が意図的かどうかの確認とコメント追加

@ujiro99 ujiro99 merged commit 9b69ac4 into main May 3, 2026
3 of 4 checks passed
@ujiro99 ujiro99 deleted the feat/hub-submit-command branch May 3, 2026 06:15
@claude

claude Bot commented May 3, 2026

Copy link
Copy Markdown

コードレビュー: PR #392 - Feat/Share command to the new Hub

概要

コマンドを新しいSelection Command Hubに共有する機能を追加するPRです。postMessage APIを用いたウィンドウ間通信、ロケール解決、コマンドデータ変換のロジックが実装されています。全体的に設計は明確で、テストカバレッジも充実しています。


潜在的なバグ・動作上の懸念点

1. コマンド複製時の sourceType 引き継ぎ問題 (重要)

ファイル: packages/extension/src/components/option/editor/CommandList.tsx (旧 L364-365)

Hubコミュニティから取得したコマンドを複製した場合、削除されたコードの影響で sourceTypeHUB_COMMUNITY のまま引き継がれます。この状態では ShareButtonVALID_SOURCE_TYPES に含まれないため、複製直後は共有ボタンが非表示になります。ユーザーが複製後にコマンドを編集すれば SELF_UPDATED に更新されますが、この動作は直感的ではありません。意図的な仕様であれば、コメントで明示することを推奨します。

// CommandList.tsx (変更前)
cmd.sourceType = COMMAND_SOURCE_TYPE.SELF_CREATED  // ← 削除された行
cmd.sourceId = COMMAND_SOURCE_ID.SELF_CREATED      // ← 削除された行

2. エラー時のフィードバック不足

ファイル: packages/extension/src/components/option/ShareButton.tsx L262-263

status === "error" 時にアイコンの色が赤に変わるのみで、ツールチップテキストは常に "Share to Hub" のままです。ユーザーが失敗に気づきにくい可能性があります。

// 現状: status に関わらず同じツールチップ
<Tooltip
  positionElm={buttonRef.current}
  text={t("Option_shareButton_tooltip")}  // エラー時も同じメッセージ
/>

3. 成功状態のビジュアルフィードバックなし

ファイル: packages/extension/src/components/option/ShareButton.tsx L260-266

status === "sent" 時にボタンが disabled になるだけで、アイコンの変化(例: チェックマーク)がなく、成功を視覚的に伝えられていません。


セキュリティ

4. postMessage の origin 検証

ファイル: packages/extension/src/services/hubShare.ts L937-941

受信メッセージの origin 検証は適切に実装されています。ただし、NEW_HUB_URL.env.development では http://localhost:3000 になるため、開発環境での動作確認時に origin 不一致が起きないよう注意が必要です(テストの SH-05 で import.meta.env.VITE_NEW_HUB_URL を直接参照しているのと同様の理由)。

// hubShare.ts L937
const onAck = (event: MessageEvent) => {
  if (event.origin !== NEW_HUB_URL) return  // 開発時は localhost:3000 と比較される

コード品質

5. テスト SH-05 での URL 重複定義

ファイル: packages/extension/src/services/hubShare.test.ts L734-736

テスト内で NEW_HUB_URL を手動で再定義しています。@/const からインポートすることで保守性が向上します。

// 現状
const NEW_HUB_URL =
  import.meta.env.VITE_NEW_HUB_URL ??
  "https://selection-command-hub.pages.dev"

// 改善案
import { NEW_HUB_URL } from "@/const"

6. ロケール前方一致の曖昧さ

ファイル: packages/extension/src/services/hubShare.ts L827-829

前方一致で最初にマッチしたロケールを返す実装のため、pt-TW(架空の例)のようなケースで pt-PT にマッチしてしまいます。現在の対応言語では問題になりませんが、将来的なロケール追加時の考慮が必要です。

// 前方一致は NEW_HUB_SUPPORTED_LOCALES の並び順に依存する
for (const locale of NEW_HUB_SUPPORTED_LOCALES) {
  if (lang.startsWith(locale.split("-")[0].toLowerCase())) return locale
}

7. useEffect の依存配列に getValues が含まれる

ファイル: packages/extension/src/components/option/editor/CommandEditDialog.tsx L335

getValuesreact-hook-form の安定した参照ですが、依存配列に含めることでリンターへの対応は適切です。ただし command オブジェクトが親から再生成される場合、このエフェクトが意図せず再実行される可能性があります。


テストカバレッジ

充実している点:

  • getHubLocale (GL-01〜04)、toSubmitCommandInput (SC-01〜06, PA-01〜02, AI-01〜04)、shareCommandToHub (SH-01〜07) と網羅的なユニットテストがある

不足している点:

  • ShareButton コンポーネントの UI テスト(idle/sent/error 各状態のレンダリング)
  • CommandEditDialoguseEffect による SELF_UPDATED 追跡ロジックのテスト
  • コマンド複製後の sourceType 引き継ぎ動作のテスト

総評

機能の設計は明確で、postMessage による非同期通信のリトライ・ACK機構は堅牢です。上記の点のうち、複製時の sourceType 引き継ぎ(#1エラー時のUXフィードバック(#2 については、ユーザー体験に直結するため対応を検討してください。

@ujiro99 ujiro99 restored the feat/hub-submit-command branch May 3, 2026 06:22
@ujiro99 ujiro99 deleted the feat/hub-submit-command branch May 3, 2026 06:22
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.

1 participant