[ujiro99]

概要

Closes #507

コマンド一覧で、Hubに共有済みかつHubへ未反映のローカル変更があるコマンドは、同期ボタンを「更新あり」の表示にします。

変更内容

• hasUnpushedEdits() を追加（services/hub/commandContent.ts）
  • contentHash と現在の内容のハッシュを比較
  • contentHash が無いコマンド（v1.3.2より前に共有）は「変更なし」扱い。次回Hubへ送信した時点からハッシュ判定が有効になる
  • Hubからの更新時の上書き確認で使う isLocallyModified() は安全側（ハッシュ無し＝変更あり）のまま変更なし
• ShareButton に「共有済み・変更あり」状態を追加
  • CloudUpload アイコン＋オレンジのバッジ＋薄いオレンジの背景
  • クリックで pushEditToHub を実行し、変更をHubへ反映
  • 反映後は contentHash が保存され、フォームがストレージから再読込されるためマークが消える
• ツールチップ文言 Option_shareButton_modified_tooltip を全14ロケールに追加
• TEST_IDS.modifiedSharedCommandButton を追加
• 未共有コマンドの共有ボタンのアイコンを Share から CloudUpload に変更

状態	アイコン	クリック時
未共有	CloudUpload	Hubに共有
共有済み・変更なし	CloudCheck	Hubのダッシュボードを開く
共有済み・変更あり	CloudUpload＋オレンジのバッジ＋薄いオレンジの背景	変更をHubへ反映

テスト

• ユニットテスト追加（hasUnpushedEdits: CC-07a〜c、ShareButton: SB-05・SB-06）
• locales.test.ts パス
• tsc --noEmit / eslint パス
• E2E は未実行
• 実機確認: Hubから導入したコマンドを内容を変えずに編集ダイアログで保存しても、マークが付かないこと（フォームの既定値補完やタイトル正規化で誤検知しないか）

🤖 Generated with Claude Code

[claude]

レビュー結果

変更範囲が小さく意図も明確で、既存の isLocallyModified を変えずに hasUnpushedEdits を追加した設計は妥当です。ロケール14言語分の追加も確認しました。気になった点は以下です。

指摘事項

1. packages/extension/src/components/option/ShareButton.tsx 45–52行付近（push処理）

   • pushEditToHub は fire-and-forget です（services/hubShare.ts:147）。戻り値は「入力が組み立てられたか」だけなので、Ipc が失敗しても UI は sent（成功）表示になります。成功表示が誤解を招く可能性があります。
   • 2秒間の sent 状態中の連打で二重送信にならないか、disabled 条件を確認してください（共有側は pending で無効化している旨のコメントがあります）。
   • setTimeout がアンマウント後も走ります（既存コードと同様で軽微）。

2. packages/extension/src/components/option/editor/CommandTreeRenderer.tsx 84–87行付近

   • hasUnpushedEdits が共有済みコマンドごとに毎レンダーでハッシュ計算を行います。&& で短絡しているため影響は小さいですが、PageAction のステップが大きい場合は useMemo 等を検討してください。
   • isModified が boolean | undefined になります。isShared && isModified の判定は ShareButton 側でも行っているため、どちらか一方に寄せると読みやすくなります。

3. 誤検知リスク（PR本文の未確認項目）

   • 「内容を変えずに保存してもマークが付かないか」の実機確認は、このPRで最大のリスクです。フォームの既定値補完やタイトル正規化で calcContentHash の結果が変わると、多数のコマンドに「更新あり」が出ます。マージ前に確認するか、フォーム保存を模した往復テストで calcContentHash の安定性を確認すると安心です。
   • contentHash が無い旧コマンドが検知対象外になる点は、コメントに明記されており許容できます。

4. テスト（packages/extension/src/components/option/__tests__/ShareButton.test.tsx）

   • SB-05 は pushEditToHub の呼び出しのみを確認しています。false を返したときの error 状態と、sent→idle の遷移（fake timers）が未検証です。
   • SB-06 の名称は「shared state を表示」ですが、実際の検証は「未共有ボタンが表示される」ことです。名称を実態に合わせてください。
   • CommandTreeRenderer が isModified を正しく渡すかのテストがありません。
   • E2E は未実行です。TEST_IDS.modifiedSharedCommandButton を使うシナリオの追加も検討してください。

5. UI / アクセシビリティ（ShareButton.tsx 137–147行付近）

   • オレンジのバッジは色のみで状態を示しています。ボタンに aria-label があれば、状態がスクリーンリーダーにも伝わります。
   • CloudUpload の hover 時の色が stroke-green-600 で、「変更あり」でも成功色になります。意図通りか確認してください。

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

• セキュリティ上の問題は見当たりません。新しい入力経路や外部通信はありません。
• 性能面は上記2のみです。

指摘3の実機確認が済めば、概ね問題ないと思います。

🤖 Generated with Claude Code

[claude]

レビュー結果

全体として小さくまとまっており、hasUnpushedEdits を isLocallyModified と分けて安全側の挙動を維持している点は良いと思います。以下、気になった点です。

指摘事項

1. CommandTreeRenderer.tsx:84-87 – isModified が boolean | undefined になる
   sharedCommandIds?.has(...) && hasUnpushedEdits(...) は undefined になり得ます。動作上は問題ありませんが、isShared と合わせて !! か === true にするとよりクリーンです。また、再レンダーごとに全共有コマンドで calcContentHash を計算します。コマンド数が多い場合のコストを確認してください(useMemo 化の検討、または未共有なら評価しない現在の短絡評価で十分かの確認)。

2. ShareButton.tsx:45-50 – 更新(push)の結果が即時に反映されない
   pushEditToHub は fire-and-forget(hubShare.ts:147)で、true はキュー投入成功を意味するだけです。そのため、Hub 側で失敗しても sent 表示になり、contentHash が更新されない限りバッジは残ります。失敗時のユーザー通知は既存仕様かもしれませんが、少なくとも意図を確認してください。また、連打すると二重送信されるため、pending / sent 中はクリックを無視するガードがあると安全です(既存の share 処理と揃えてください)。

3. ShareButton.tsx:50 – setTimeout のクリーンアップなし
   既存コードと同様ですが、アンマウント後の setStatus が発生し得ます。新規コードなので useRef + cleanup を検討してください。

4. ShareButton.tsx:130-148 – 重複したアイコン定義とネストした三項演算子
   CloudUpload の className が2箇所で重複しています。共通化、またはステート(unshared | shared | modified)を一度算出して data-testid・アイコン・tooltip で再利用すると可読性が上がります。

5. アクセシビリティ: 赤バッジ(ShareButton.tsx:147)は色のみで状態を示しています。tooltip はありますが、aria-label にも状態文言を付与することを推奨します。

6. UX の変更: 未共有コマンドのアイコンを Share → CloudUpload に変更していますが、Issue コマンド一覧で、共有済みでかつローカル変更があるコマンドは、syncボタンを更新ありのマークにする #507 の範囲外に見えます。意図的であれば PR 説明どおりで問題ありません。

テスト

• SB-05 は pushEditToHub の呼び出しのみ検証しています。status の遷移(sent/error)や false 返却時の挙動のテストが無いので追加を推奨します。
• SB-06 のテスト名が "shows the shared state ..." ですが、実際には未共有ボタンが表示されることを検証しており、名称が不一致です。
• CommandTreeRenderer で isModified が正しく配線されること(共有済み×変更あり)のテストが無いです。
• PR 説明にある未確認項目(Hub 導入コマンドを無変更で保存した際の誤検知)は、フォームの既定値補完・タイトル正規化に関わるため、マージ前に実機確認が重要です。可能なら calcContentHash が既定値補完後の値で保存されるかをユニットテストで担保してください。

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

セキュリティ上の懸念は見当たりません。ロケール文言は全14言語に追加されており問題ありません。

軽微な指摘が中心で、誤検知の実機確認が済めば LGTM だと思います。

[claude]

レビュー結果

全体として小さく筋の通った変更で、hasUnpushedEdits を isLocallyModified と分けて安全側の挙動を維持している点は良いです。以下、気になった点です。

指摘事項

1. 押下後に「更新あり」表示が残る/ステータスが実態と合わない (components/option/ShareButton.tsx 約45-51行)
   pushEditToHub は void Ipc.send(...).catch(...) の fire-and-forget で、戻り値 true は入力変換が成功しただけの意味です (services/hubShare.ts:138-151)。そのため送信失敗でも "sent" になり、直後に idle へ戻ります。PR 説明の「反映後に contentHash が保存されフォームが再読込されてマークが消える」に依存するため、失敗時はマークが残るだけでユーザーに失敗が伝わりません。可能なら Ipc の結果を await してエラー状態を反映するのが望ましいです。
   また setTimeout がアンマウント時にクリアされない点は既存コードと同様ですが、連打すると多重送信になります。status === "pending" 中は早期 return する、または disabled にすることを推奨します。

2. isModified の型が boolean | undefined (CommandTreeRenderer.tsx 84-87行)
   sharedCommandIds?.has(...) && hasUnpushedEdits(...) は動作上問題ありませんが、ShareButton 側の isShared && isModified と二重に条件を持っています。isModified は shared 前提と props コメントに明記されているので、Renderer 側では hasUnpushedEdits のみ渡す(ShareButton 側で isShared と AND)形にすると重複が減ります。

3. レンダー毎のハッシュ計算 (CommandTreeRenderer.tsx 84-87行)
   共有済みコマンドごとに毎レンダーで stableStringify + cyrb53 を実行します。コマンド数が数百でもコストは小さいですが、編集中のフォーム入力で再レンダーが頻発する画面なので、気になるなら useMemo 化や共有済みのみ計算(現状 && の短絡で既に共有済みのみなので許容範囲)を検討してください。

4. 誤検知リスク(PR 本文にも記載済みの未確認項目)
   フォーム既定値補完・タイトル正規化で、内容を変えずに保存してもハッシュが変わる可能性があります。実機確認が未チェックのため、マージ前に確認をお願いします。誤検知が起きると全共有コマンドに橙バッジが出て UX が悪化します。

5. アイコン変更のスコープ (ShareButton.tsx 約130行)
   未共有の Share → CloudUpload 変更は機能と別の意図の変更です。意図的ならよいですが、PR 説明の表には載っているものの、「未共有=CloudUpload」と「更新あり=CloudUpload+バッジ」が似ているため、バッジの視認性(w-1.5 h-1.5 は小さめ)とアクセシビリティ(色のみで区別、aria-label なし)に注意してください。

テスト

• hasUnpushedEdits の3ケースと SB-05/06 は妥当です。
• 不足: SB-05 では pushEditToHub が false を返した場合(エラー状態)のテストがありません。また CommandTreeRenderer 側で isModified が正しく渡されること(共有済み×変更あり/なし)のテストがありません。
• SB-06 のテスト名「shows the shared state when isModified is set on an unshared command」は、実際には「未共有ボタンのまま」を検証しているため、名前が内容と逆になっています(ShareButton.test.tsx 約131行)。例: "keeps the unshared state when isModified is set without isShared"。
• E2E 未実行とのことなので、TEST_IDS.modifiedSharedCommandButton を使ったシナリオの確認を推奨します。

ロケール

14ロケールすべてにキーが追加されており、en のみ description 付きで他は省略という既存パターンにも沿っています。問題ありません。

セキュリティ

特記事項なし。ハッシュは編集検知のみの用途で、コメントにもその旨が記載されています。

軽微〜中程度の指摘が中心で、1(失敗時のフィードバックと多重送信)と4(誤検知の実機確認)を対応いただければ良いと思います。

[claude]

レビュー結果

全体として小さく読みやすい変更です。hasUnpushedEdits を isLocallyModified と分離して後方互換(ハッシュ無し=変更なし)を保つ設計も妥当です。以下、気になった点です。

指摘事項

1. isModified の型が boolean | undefined(packages/extension/src/components/option/editor/CommandTreeRenderer.tsx:84-87)
   sharedCommandIds?.has(...) && hasUnpushedEdits(...) は動作上問題ありませんが、ShareButton 側でも isShared && isModified を3か所で判定しています。isModified は「共有済みかつ変更あり」の意味なので、ShareButton 内で const modified = isShared && isModified と1度だけ算出すると、ShareButton.tsx:45, 118, 121-125, 136, 162-166 の分岐が整理できます。

2. push 後のフィードバックが実質ない(packages/extension/src/components/option/ShareButton.tsx:45-51)
   pushEditToHub は fire-and-forget(services/hubShare.ts:147)で、Ipc の失敗は console.error のみ。ok は入力変換の成否しか表すため、Ipc/Hub側が失敗しても sent 表示になり、変更マークも残ったままになります。ユーザーには失敗が見えず、再クリックの判断ができません。pushEditToHub が Promise を返す形にするか、少なくとも失敗時にエラー表示できると安心です。また await が無いのに setStatus("pending") → 即 sent となるため、pending は描画されません。
   なお、連打すると複数回 push され得ます。status !== "idle" の間は早期 return する、またはボタンを disabled にすることを推奨します。

3. setTimeout のクリーンアップ(ShareButton.tsx:49)
   アンマウント後に setStatus が走る可能性があります(push 後にフォームが再読込されると再マウントされる前提のため、実際に起こりやすい)。既存の共有処理と同じ書き方なら許容ですが、useEffect でのクリア等を検討してください。

4. 誤検知リスク(PR本文の未確認項目)(packages/extension/src/services/hub/commandContent.ts:105-108)
   Hub から導入したコマンドを編集ダイアログで無変更保存した際、フォームの既定値補完やタイトル正規化で calcContentHash が変わると、全ユーザーに「更新あり」が出てしまいます。pickHubContent が既定値(undefined/空文字/false 等)を同一視しているか、無変更保存のテストを追加するのが望ましいです。実機確認が済むまでマージは待った方がよいと思います。

5. 一覧描画ごとのハッシュ計算(CommandTreeRenderer.tsx:84-87)
   sharedCommandIds.has が真のときのみ計算するため軽量ですが、ツリー再描画のたびに全共有コマンドで stableStringify + cyrb53 が走ります。共有コマンド数が多い場合は useMemo 化を検討してください(現状は許容範囲)。

6. アイコン変更の影響(CommandEditDialog.tsx:911, 930)
   Share → CloudUpload はPR概要にあるとおり意図的ですが、未共有ボタンと「更新あり」ボタンが同じ CloudUpload で、バッジの有無と背景色のみの差になります。色のみに依存した区別はアクセシビリティ上弱いので、バッジに aria-hidden を付けつつ、ボタンに aria-label(ツールチップと同文言)を付与することを推奨します。

テスト

• hasUnpushedEdits の3ケース(CC-07a〜c)は良いです。
• SB-05: pushEditToHub が false を返した場合(error 状態)と、クリック後の status 遷移(2秒後に idle)のテストがありません。
• SB-06 のタイトルは "shows the shared state" ですが、実際は「未共有ボタンが表示される」ことを検証しています。名称を実態に合わせてください(isModified 単独では変更表示にならない、など)。
• CommandTreeRenderer の isModified 受け渡し(共有済みかつハッシュ不一致のときのみ true)のテストがありません。
• E2E が未実行なので、TEST_IDS.sharedCommandButton を参照している既存E2Eが、変更ありのコマンドで新しい testid に変わって壊れないか確認してください。

ロケール

14ロケールすべてに追加されており、en のみ description 付きで既存の慣習にも沿っています。問題ありません。

まとめ

致命的な問題はありません。誤検知の実機確認(項目4)と、push の失敗/連打時の挙動(項目2)を対応すればマージ可能と考えます。

🤖 Generated with Claude Code

[claude]

レビュー結果

全体として小さく読みやすい変更で、isLocallyModified（安全側）と hasUnpushedEdits（誤検知回避側）を分けた設計は妥当です。以下、気になった点です。

指摘事項

1. packages/extension/src/components/option/ShareButton.tsx の handleClick（約L45-52）: 送信失敗が検知できない
   pushEditToHub（services/hubShare.ts:138-151）は Ipc.send を void で投げ、同期的に true を返すだけです。setStatus(ok ? "sent" : "error") の ok は「入力変換に成功したか」しか表さず、Service Worker/通信側の失敗でも "sent" 扱いになります。また setStatus("pending") 直後に同期で sent へ遷移するため pending が実質無効です。

2. 連打時の二重送信（同ブロック）
   反映後にストレージ再読込でマークが消えるまでの間、ボタンが押せます。status が pending / sent の間は早期 return（または disabled）を推奨します。setTimeout もアンマウント時にクリアされません。

3. packages/extension/src/components/option/editor/CommandTreeRenderer.tsx（isModified 付近、L84-87）
   sharedCommandIds?.has(...) && hasUnpushedEdits(...) は boolean | undefined になります。動作上は問題ありませんが、再レンダーごとに共有済みコマンドのハッシュ計算（stableStringify + cyrb53）が走るため、数が多い場合は useMemo 化も検討してください。

4. 誤検知リスク（PR本文の未確認項目）
   Hub由来コマンドを内容変更なしで編集ダイアログ保存した際、フォームの既定値補完・タイトル正規化でハッシュが変わると誤って「更新あり」になります。マージ前に実機確認をお願いします。可能なら、補完後のオブジェクトで calcContentHash が一致することをテストで固定すると安心です。

5. アクセシビリティ（ShareButton.tsx のバッジ部）
   状態の違いが色と小さなドットのみで、aria-label がありません。ツールチップ以外でも状態が伝わるよう付与を推奨します。

6. スコープ
   未共有ボタンおよび CommandEditDialog.tsx（L11, L911, L930）の Share → CloudUpload 変更は Issue コマンド一覧で、共有済みでかつローカル変更があるコマンドは、syncボタンを更新ありのマークにする #507 の主目的外です。意図的であれば問題ありません。

テスト

• hasUnpushedEdits（CC-07a〜c）は十分です。
• SB-05 は pushEditToHub 呼び出しのみの確認です。false を返す場合の error 状態と、isShared && !isModified（ダッシュボードを開く）の回帰テストがあると良いです。
• SB-06 のテスト名（"shows the shared state when isModified is set on an unshared command"）は実際の検証内容（未共有ボタンが表示される）と食い違っています。修正を推奨します。
• ロケール追加（14ロケール）は locales.test.ts で担保されており問題ありません。E2E は未実行のため、modifiedSharedCommandButton を使うシナリオの追加/実行を検討してください。

良い点

• contentHash 無しを「変更なし」とする判断とコメントが明確。
• コメントが英語で規約に沿っている。