Skip to content

DM APIとWebSocket整理#613

Open
tako0614 wants to merge 1 commit intomasterfrom
codex/remove-e2ee-routes-and-add-dm-endpoint-ccgi2s
Open

DM APIとWebSocket整理#613
tako0614 wants to merge 1 commit intomasterfrom
codex/remove-e2ee-routes-and-add-dm-endpoint-ccgi2s

Conversation

@tako0614
Copy link
Copy Markdown
Owner

@tako0614 tako0614 commented Aug 22, 2025

概要

  • フロントエンドからMLS依存コードを削除し、DMを平文送受信に簡素化
  • 設定画面やプロフィールなど関連コンポーネントをMLS不要な構成へ更新
  • microblogのユーザー情報取得をメモリキャッシュのみに変更

テスト

  • deno fmt app/client/src/components/Application.tsx app/client/src/components/microblog/api.ts app/client/src/components/Chat.tsx app/client/src/components/Profile.tsx app/client/src/components/Setting/index.tsx app/client/src/components/chat/ChatSettingsOverlay.tsx app/client/src/components/chat/ChatTitleBar.tsx
  • deno lint app/client/src/components/Application.tsx app/client/src/components/Chat.tsx app/client/src/components/Profile.tsx app/client/src/components/Setting/index.tsx app/client/src/components/chat/ChatSettingsOverlay.tsx app/client/src/components/chat/ChatTitleBar.tsx app/client/src/components/microblog/api.ts

https://chatgpt.com/codex/tasks/task_e_68a8ed9b4f2c8328ae8427aebd9f891d

Summary by CodeRabbit

  • 新機能
    • 1対1のダイレクトメッセージ(DM)に対応。RESTエンドポイントとWebSocket通知で送受信可能。
  • 変更
    • 単一宛先の投稿はDMとして処理され、双方に軽量通知を送信。
    • ユーザー情報キャッシュをメモリのみで運用(高速化)。
  • リファクタ
    • MLS/E2EE関連機能を全面的に撤去(鍵管理・暗号メッセージ・ルーム・自動補充・関連UI)。
    • WebSocketのバイナリメッセージ受付を無効化。
    • ActivityPubのユーザー情報からkeyPackages公開を削除。
    • 設定/チャット画面を簡素化。

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Aug 22, 2025

Walkthrough

DM(ダイレクトメッセージ)機能を新規追加し、MLS/チャットルーム/鍵パッケージ/ハンドシェイク関連のサーバ・クライアント実装とAPIを大規模に削除。DBインターフェイス・モデル・ルート・WS・ActivityPubハンドラをDM対応へ変更し、e2ee関連のモデル/ルート/ストレージ/テスト/UIを撤去。

Changes

Cohort / File(s) Summary
DB API 置換(DM化)
app/api/DB/mod.ts, app/api/DB/mongo.ts, app/shared/db.ts, app/api/models/takos/dm_message.ts
公開APIから startKeyPackageCleanupJob とMLS/チャット関連メソッド削除。saveDMMessage/listDMsBetween 追加。DMMessage スキーマ・モデル新設。Mongo実装からチャットルーム/暗号/鍵/ハンドシェイク系を全面撤去。
削除:Takos/TakosHost モデル(MLS/Handshake/Encrypted/KeyPackage)
app/api/models/takos/encrypted_message.ts, .../handshake_message.ts, .../key_package.ts, app/api/models/takos_host/encrypted_message.ts, .../handshake_message.ts, .../key_package.ts
暗号メッセージ、ハンドシェイク、鍵パッケージ関連のスキーマ/モデルを削除。対応するホスト側モデルも削除。
ルート追加/削除とサーバ配線
app/api/routes/dm.ts, app/api/routes/e2ee.ts, app/api/server.ts, app/api/routes/ws.ts
新規 /dm 追加(POST保存+WS配信、GET履歴)。/e2ee 全面削除。サーバは dm/api にマウントし、e2ee と起動時 startKeyPackageCleanupJob 呼び出しを除去。WSはバイナリハンドラを撤去。
ActivityPub 関連変更
app/api/activity_handlers.ts, app/api/routes/activitypub.ts, app/api/utils/activitypub.ts
Createで単一宛先NoteをDMとしてDB保存/WS通知/外部配送。MLS検証/コレクション拒否ロジックを削除。Actorの keyPackages とMLSコンテキスト付与を削除。Add/Remove/Delete ハンドラを削除。
クライアント:E2EE層の削除
app/client/src/components/e2ee/api.ts, .../binding.ts, .../mls_message.ts, .../mls_test.ts, .../mls_wrapper.ts, .../storage.ts, .../useMLS.ts
MLS API/バインディング/メッセージTLV/ラッパー/永続ストレージ/テスト/フックを一括削除。公開型・関数を全面撤去。
クライアント:UIの簡素化/DM移行
app/client/src/components/Application.tsx, app/client/src/components/Profile.tsx, app/client/src/components/Setting/index.tsx, app/client/src/components/chat/ChatSettingsOverlay.tsx, app/client/src/components/chat/ChatTitleBar.tsx
鍵パッケージ自動補充やMLSキー生成UIを削除。プロフィールの部屋作成呼び出し削除。チャット設定/タイトルバーからバインディング表示等を削除し静的UIへ。
マイクロブログAPIのキャッシュ変更
app/client/src/components/microblog/api.ts
ユーザー情報キャッシュをメモリのみへ単純化。get/setCachedUserInfo を同期化し accountId 引数を削除。fetchUserInfo* から永続キャッシュ処理を削除。

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant API as /api/dm (Hono)
  participant DB as MongoDB
  participant WS as WebSocket Hub
  Note over API,DB: 新規DMフロー(POST /dm)
  C->>API: POST /dm { from, to, content }
  API->>DB: saveDMMessage(from,to,content)
  DB-->>API: {_id, from, to, content, createdAt}
  API->>WS: sendToUser(from, {type:"dm", payload})
  API->>WS: sendToUser(to, {type:"dm", payload})
  API-->>C: 200 JSON { id, from, to, content }
  opt 履歴取得
    C->>API: GET /dm?user1=A&user2=B
    API->>DB: listDMsBetween(A,B)
    DB-->>API: [messages]
    API-->>C: 200 JSON [messages]
  end
Loading
sequenceDiagram
  autonumber
  participant Fed as Remote/Local Sender
  participant Act as Activity Create Handler
  participant DB as MongoDB
  participant WS as WebSocket Hub
  participant AP as ActivityPub Delivery
  Note over Act: Create受信時の分岐(DM vs 非DM)
  Fed->>Act: Activity(Create { object: Note, to/cc })
  alt 単一宛先でDM条件を満たす
    Act->>DB: saveDMMessage(actor, recipient, content)
    Act->>WS: sendToUser(actor, {type:"dm", payload})
    Act->>WS: sendToUser(recipient, {type:"dm", payload})
    opt 外部宛先
      Act->>AP: deliver DM payload to remote host
    end
    Act-->>Fed: 200
  else 非DM
    Act->>DB: saveObject(...)
    Act->>WS: broadcast timelines/recipient
    Act-->>Fed: 200
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

ぴょんと跳ねて DM 投函
鍵も部屋も 今日はお別れ
風は軽くて メッセは直行
WS ひゅるるん 届けてくれる
草むらで待つよ、返信サイン 📨🐇

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/remove-e2ee-routes-and-add-dm-endpoint-ccgi2s

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/api/activity_handlers.ts (1)

114-118: 環境変数の取得は getEnv(c) を使用してください(ガイドライン準拠)。

(c as { get: ... }).get("env") はガイドライン違反です。getEnv(c) に置換を。

提案差分:

+import { getEnv } from "../shared/config.ts";
@@
-      const env = (c as { get: (k: string) => unknown }).get("env") as Record<
-        string,
-        string
-      >;
+      const env = getEnv(c as Context);
@@
-    const env = (c as { get: (k: string) => unknown }).get("env") as Record<
-      string,
-      string
-    >;
+    const env = getEnv(c as Context);

Also applies to: 161-165

🧹 Nitpick comments (14)
app/client/src/components/chat/ChatSettingsOverlay.tsx (4)

13-23: モーダルのアクセシビリティ(role/aria、Esc、外側クリック)を足して最小限でも対話性を確保しませんか?

現在は視覚的には問題ありませんが、支援技術にはダイアログとして伝わらず、Esc で閉じる操作もできません。外側クリックで閉じる挙動も一般的には期待されます。以下の最小変更で改善できます(内側はクリック伝播を止める)。

-      <div class="fixed inset-0 z-50 flex items-center justify-center bg-black/50">
-        <div class="bg-white rounded p-4 w-72 text-black">
-          <h2 class="text-lg font-bold mb-2">チャット設定</h2>
-          <p class="mb-4">現在、設定項目はありません。</p>
+      <div
+        class="fixed inset-0 z-50 flex items-center justify-center bg-black/50"
+        role="dialog"
+        aria-modal="true"
+        aria-labelledby="chat-settings-title"
+        aria-describedby="chat-settings-desc"
+        tabIndex={-1}
+        onKeyDown={(e) => e.key === "Escape" && props.onClose()}
+        onClick={props.onClose}
+      >
+        <div class="bg-white rounded p-4 w-72 text-black" onClick={(e) => e.stopPropagation()}>
+          <h2 id="chat-settings-title" class="text-lg font-bold mb-2">チャット設定</h2>
+          <p id="chat-settings-desc" class="mb-4">現在、設定項目はありません。</p>
           <button
             type="button"
             class="px-4 py-2 rounded bg-gray-700 text-white"
             onClick={props.onClose}
           >
             閉じる
           </button>
         </div>
       </div>

補足(任意):

  • フォーカストラップは本PRでは省略でOKですが、今後導入予定があれば headless UI か自前のフォーカストラップを検討ください。

1-1: Z軸コンテキストを避けるため Portal の利用を検討ください(任意)

親要素のスタッキングコンテキストに影響されない描画を保証したい場合、Solid の Portal で body 直下に描画するのが堅実です。既存UIとの重なり問題が無いならスキップ可です。

追加の変更例(変更箇所が選択行外のため参考コードで提示):

import { Show } from "solid-js";
import { Portal } from "solid-js/web";

export function ChatSettingsOverlay(props: ChatSettingsOverlayProps) {
  return (
    <Show when={props.isOpen}>
      <Portal>
        {/* ここに現在のモーダルDOMをそのまま入れる */}
      </Portal>
    </Show>
  );
}

15-23: i18n: ハードコード文字列の翻訳キー化を検討

「チャット設定」「現在、設定項目はありません。」「閉じる」は固定文言です。国際化ポリシーがある場合は翻訳関数経由に。ポリシーが未整備なら現状維持でもOKです。


4-8: Props の整合性: ChatSettingsOverlay の room を活用 or 呼び出し側を修正
app/client/src/components/Chat.tsx の 224–226 行目で

<ChatSettingsOverlay
  isOpen={showSettings()}
  room={currentRoom()}
  onClose={closeSettings}
/>

room が渡されていますが、ChatSettingsOverlay 側では未使用です。API と呼び出し側の期待仕様を合わせるため、いずれかを選択してください:

  • 案A: 呼び出し側の room 渡しを削除し、Props 定義からも room を削除
    interface ChatSettingsOverlayProps {
      isOpen: boolean;
      onClose: () => void;
    }
  • 案B: Props を維持し、モーダルヘッダーに部屋名を表示
    <h2 id="chat-settings-title" className="text-lg font-bold mb-2">
      {props.room?.displayName ?? props.room?.name ?? "チャット設定"}
    </h2>

対象箇所:

  • app/client/src/components/ChatSettingsOverlay.tsx(Props 定義)
  • app/client/src/components/Chat.tsx:224-226(呼び出し)

ご対応をお願いします。

app/client/src/components/Setting/index.tsx (1)

14-24: ログアウト処理の簡素化は妥当。セッション以外のクライアント状態リセット要否を確認してください。

MLS関連の副作用が消えたのでUI側で強制ログアウトする方針は理解できますが、以下の状態を併せてクリアすべきか検討ください(不要なら明記):

  • 選択中アプリ/部屋(selectedAppState, selectedRoomState)
  • プロフィール/タイムライン等のメモリキャッシュ
  • 通知購読やWS登録状態(utils/ws.ts で切断/再接続の扱い)

API失敗時でも finally でログアウト扱いにする仕様はOKですが、サーバがクッキーを消せなかったケースのUX影響(次回アクセス時に再ログインが求められない等)がないか念のため確認を。

app/client/src/components/Profile.tsx (1)

242-249: openChat: ローカルユーザー名に @Domain を補完して一意キー化することを推奨

現在 normalizeActor は http 始まりのときのみ正規化し、素の "alice" はそのまま扱います。チャットの部屋キーとして "alice" と "alice@" が混在するとDMスレが分岐する恐れがあります。以下のように、"@" を含まない場合は getDomain() を補完してから normalize するのが堅実です。

-  const openChat = () => {
-    const name = username();
-    const user = account();
-    if (!name || !user) return;
-    const handle = normalizeActor(name);
-    setRoom(handle);
-    setApp("chat");
-  };
+  const openChat = () => {
+    const name = username();
+    const user = account();
+    if (!name || !user) return;
+    const base = name.includes("@") ? name : `${name}@${getDomain()}`;
+    const handle = normalizeActor(base);
+    setRoom(handle);
+    setApp("chat");
+  };

また、この関数を同期化したことで、他の呼び出し箇所で Promise を前提にしていないか軽く走査しておいてください(本ファイル内の onClick は問題なし)。

app/client/src/components/Application.tsx (1)

35-40: registerUser の送信タイミングをWS接続確立後に保証できているか確認

connectWebSocket() が内部で再送キューや open 待ちをしていれば問題ありませんが、未実装なら open イベントで register を送る設計に統一した方が事故が減ります。utils/ws.ts の実装方針に依存するため、挙動を一度確認ください。

app/client/src/components/microblog/api.ts (2)

355-371: 同期化後も await が残存しています(不要なマイクロタスク/可読性低下)

getCachedUserInfo/setCachedUserInfo は同期なので await は外してください。

-    const cached = await getCachedUserInfo(identifier);
+    const cached = getCachedUserInfo(identifier);
@@
-    await setCachedUserInfo(identifier, userInfo);
+    setCachedUserInfo(identifier, userInfo);

387-413: 同様にバッチ処理内の await/Promise.all が不要

setCachedUserInfo は同期なので Promise.all も不要です。簡潔にしましょう。

-    for (const identifier of identifiers) {
-      const cachedInfo = await getCachedUserInfo(identifier);
+    for (const identifier of identifiers) {
+      const cachedInfo = getCachedUserInfo(identifier);
       if (cachedInfo) {
         cachedMap[identifier] = cachedInfo;
       } else {
         uncached.push(identifier);
       }
     }
@@
-        await Promise.all(
-          fetchedInfos.map((info, index) =>
-            setCachedUserInfo(uncached[index], info)
-          ),
-        );
+        fetchedInfos.forEach((info, index) => {
+          setCachedUserInfo(uncached[index], info);
+        });
app/api/activity_handlers.ts (2)

92-110: DM判定ロジックの改善提案(任意)。

現状は「toが1件かつコレクションでない」=DM とみなしていますが、cc が第三者を含む場合でもDM扱いになります。実運用上の誤検知を避けるなら以下のどちらかを推奨:

  • Array.isArray(obj.cc) && obj.cc.length === 0 をDM条件に追加
  • もしくは audience/bcc を明示的に拒否

差分は参考(設計判断に応じて適用):

-    if (objTypes.includes("Note") && toList.length === 1) {
+    if (objTypes.includes("Note") && toList.length === 1 && (!Array.isArray(obj.cc) || obj.cc.length === 0)) {

148-155: 外部配達の失敗ログはOK。追加でリトライ設計を検討(任意)。

deliverActivityPubObject(...).catch でのログは有用。必要に応じてバックオフ/キュー投入(例: BullMQ/cron job)を検討してください。

app/shared/db.ts (2)

69-70: DM APIにページングと具体的な戻り値型を追加してI/Fを明確化してください

大量履歴に備え、listDMsBetween に ListOpts(limit/before)を受けるようにし、戻り値は具体型にしましょう。saveDMMessage も同様に具体型を返すと型安全性と可読性が上がります。

適用例(このハンク内の最小差分):

-  saveDMMessage(from: string, to: string, content: string): Promise<unknown>;
-  listDMsBetween(user1: string, user2: string): Promise<unknown[]>;
+  saveDMMessage(from: string, to: string, content: string): Promise<DMMessageDoc>;
+  listDMsBetween(user1: string, user2: string, opts?: ListOpts): Promise<DMMessageDoc[]>;

補足:

  • 別途 shared/types.ts に DMMessageDoc を定義し、本ファイル先頭の型インポートへ追加してください。

必要なら DMMessageDoc の型定義案をこちらで用意します。


69-70: DM参加者ID表現の明示化をお願いします

以下の対応をお願いします。

  • app/shared/db.ts(69–70行目)
    • saveDMMessage(from: string, to: string, content: string) および
      listDMsBetween(user1: string, user2: string) のインターフェースコメント(JSDoc 等)に、from/to(あるいは user1/user2)が受け取るIDの形式(例:ユーザー名、Actor URL、acct@domain など)を明記してください。
  • app/api/routes/dm.ts(31行目/51行目 呼び出し箇所)
    • リクエストから取得した from/to が上記で定めた形式になっているか、型チェック・バリデーションを追加して保証してください。

これにより、将来のフェデレーション対応やクロステナント運用時の混用リスクを低減できます。

app/api/DB/mongo.ts (1)

360-368: ホストモードのテナント伝搬OK。戻り値の型付けと軽微な正規化を検討

  • doc.toObject() の戻り値を DMMessageDoc に型付けすると利用側が扱いやすくなります。
  • 余力があれば content のトリム・空文字拒否など軽微な入力正規化も追加すると事故が減ります(ただしレイヤ的にRouteでのバリデーションでも可)。
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a5e7892 and d799586.

📒 Files selected for processing (31)
  • app/api/DB/mod.ts (1 hunks)
  • app/api/DB/mongo.ts (2 hunks)
  • app/api/activity_handlers.ts (2 hunks)
  • app/api/models/takos/dm_message.ts (1 hunks)
  • app/api/models/takos/encrypted_message.ts (0 hunks)
  • app/api/models/takos/handshake_message.ts (0 hunks)
  • app/api/models/takos/key_package.ts (0 hunks)
  • app/api/models/takos_host/encrypted_message.ts (0 hunks)
  • app/api/models/takos_host/handshake_message.ts (0 hunks)
  • app/api/models/takos_host/key_package.ts (0 hunks)
  • app/api/routes/activitypub.ts (0 hunks)
  • app/api/routes/dm.ts (1 hunks)
  • app/api/routes/e2ee.ts (0 hunks)
  • app/api/routes/e2ee_test.ts (0 hunks)
  • app/api/routes/ws.ts (1 hunks)
  • app/api/server.ts (3 hunks)
  • app/api/utils/activitypub.ts (0 hunks)
  • app/client/src/components/Application.tsx (1 hunks)
  • app/client/src/components/Profile.tsx (1 hunks)
  • app/client/src/components/Setting/index.tsx (1 hunks)
  • app/client/src/components/chat/ChatSettingsOverlay.tsx (1 hunks)
  • app/client/src/components/chat/ChatTitleBar.tsx (0 hunks)
  • app/client/src/components/e2ee/api.ts (0 hunks)
  • app/client/src/components/e2ee/binding.ts (0 hunks)
  • app/client/src/components/e2ee/mls_message.ts (0 hunks)
  • app/client/src/components/e2ee/mls_test.ts (0 hunks)
  • app/client/src/components/e2ee/mls_wrapper.ts (0 hunks)
  • app/client/src/components/e2ee/storage.ts (0 hunks)
  • app/client/src/components/e2ee/useMLS.ts (0 hunks)
  • app/client/src/components/microblog/api.ts (4 hunks)
  • app/shared/db.ts (1 hunks)
💤 Files with no reviewable changes (18)
  • app/client/src/components/e2ee/mls_test.ts
  • app/client/src/components/e2ee/binding.ts
  • app/api/models/takos/encrypted_message.ts
  • app/api/models/takos_host/key_package.ts
  • app/api/utils/activitypub.ts
  • app/api/models/takos/key_package.ts
  • app/api/models/takos_host/handshake_message.ts
  • app/client/src/components/e2ee/useMLS.ts
  • app/api/models/takos_host/encrypted_message.ts
  • app/api/models/takos/handshake_message.ts
  • app/client/src/components/chat/ChatTitleBar.tsx
  • app/client/src/components/e2ee/mls_wrapper.ts
  • app/api/routes/activitypub.ts
  • app/api/routes/e2ee_test.ts
  • app/client/src/components/e2ee/storage.ts
  • app/client/src/components/e2ee/mls_message.ts
  • app/api/routes/e2ee.ts
  • app/client/src/components/e2ee/api.ts
🧰 Additional context used
📓 Path-based instructions (5)
app/api/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

環境変数は常に getEnv(c) を用いてコンテキストから取得する

Files:

  • app/api/models/takos/dm_message.ts
  • app/api/routes/dm.ts
  • app/api/routes/ws.ts
  • app/api/DB/mod.ts
  • app/api/server.ts
  • app/api/DB/mongo.ts
  • app/api/activity_handlers.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: TypeScriptの型定義を活用する
エラーハンドリングを適切に実装する

Files:

  • app/api/models/takos/dm_message.ts
  • app/api/routes/dm.ts
  • app/api/routes/ws.ts
  • app/client/src/components/Setting/index.tsx
  • app/api/DB/mod.ts
  • app/shared/db.ts
  • app/client/src/components/Profile.tsx
  • app/client/src/components/chat/ChatSettingsOverlay.tsx
  • app/api/server.ts
  • app/api/DB/mongo.ts
  • app/client/src/components/microblog/api.ts
  • app/client/src/components/Application.tsx
  • app/api/activity_handlers.ts
app/api/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Honoフレームワークのミドルウェアパターンに従う

Files:

  • app/api/models/takos/dm_message.ts
  • app/api/routes/dm.ts
  • app/api/routes/ws.ts
  • app/api/DB/mod.ts
  • app/api/server.ts
  • app/api/DB/mongo.ts
  • app/api/activity_handlers.ts
app/api/models/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

MongooseのスキーマでDBモデルを定義する

Files:

  • app/api/models/takos/dm_message.ts
app/api/{activity_handlers.ts,utils/activitypub*.ts}

📄 CodeRabbit inference engine (CLAUDE.md)

ActivityPubの仕様に準拠したJSON-LD形式を使用する

Files:

  • app/api/activity_handlers.ts
🧠 Learnings (3)
📚 Learning: 2025-08-13T10:14:15.274Z
Learnt from: CR
PR: takoserver/takos#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T10:14:15.274Z
Learning: Applies to app/api/models/**/*.ts : MongooseのスキーマでDBモデルを定義する

Applied to files:

  • app/api/models/takos/dm_message.ts
  • app/api/DB/mongo.ts
📚 Learning: 2025-08-13T10:14:15.274Z
Learnt from: CR
PR: takoserver/takos#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T10:14:15.274Z
Learning: Applies to app/api/**/*.{ts,tsx} : Honoフレームワークのミドルウェアパターンに従う

Applied to files:

  • app/api/routes/dm.ts
📚 Learning: 2025-08-13T10:14:15.274Z
Learnt from: CR
PR: takoserver/takos#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T10:14:15.274Z
Learning: Applies to app/api/{activity_handlers.ts,utils/activitypub*.ts} : ActivityPubの仕様に準拠したJSON-LD形式を使用する

Applied to files:

  • app/api/DB/mongo.ts
  • app/client/src/components/microblog/api.ts
  • app/api/activity_handlers.ts
🧬 Code graph analysis (3)
app/api/routes/dm.ts (3)
app/takos_host/auth.ts (1)
  • authRequired (203-220)
app/shared/config.ts (1)
  • getEnv (49-56)
app/api/routes/ws.ts (1)
  • sendToUser (23-30)
app/client/src/components/chat/ChatSettingsOverlay.tsx (1)
app/client/src/components/chat/types.ts (1)
  • Room (21-39)
app/client/src/components/microblog/api.ts (1)
app/api/services/user-info.ts (1)
  • UserInfo (6-12)
🔇 Additional comments (10)
app/client/src/components/chat/ChatSettingsOverlay.tsx (1)

10-12: 簡素化の方向性は妥当です

機能削減に合わせた最小UIで、Show when によるマウント制御も明快。複雑な依存を持たない構成は保守性に寄与します。

app/client/src/components/microblog/api.ts (2)

329-337: メモリキャッシュAPIの同期化はシンプルで良いです

TTL 判定も明確で、呼び出し側の負担軽減に寄与しています。


421-422: サーバ側でバッチレスポンス順序保証を確認しました

サーバ実装の getUserInfoBatch は、引数として受け取った identifiers: string[] の順序どおりに結果配列を組み立てて返却しているため、フロントエンドの

return identifiers.map((id) => cachedMap[id] ?? fetchedMap[id]);

undefined が返ってしまうケースはありません。

– app/api/services/user-info.ts の getUserInfoBatch では

// 結果を順序通りに並べる
for (const identifier of identifiers) {
  results.push(cache[identifier]);
}
return results;

と明示的に入力配列の順序を維持しています。
– app/client/src/components/microblog/api.ts 側のマッピング実装は正しく機能します。

【今後の推奨】
バッチレスポンスの順序保証という前提契約を、コードまたはドキュメントコメントで明示的に残しておくと、可読性/保守性がさらに向上します。

app/api/DB/mod.ts (1)

8-8: 公開APIの整理は妥当です(KeyPackage掃除ジョブの除去と整合)。

startKeyPackageCleanupJob の公開をやめ、DM移行に沿ってエクスポートを絞った判断は整合的です。createDB の署名はそのままなので既存呼び出し側への影響も最小です。追加の変更は不要です。

app/api/models/takos/dm_message.ts (1)

16-17: モデルの再利用・コレクション名の固定は妥当です。

mongoose.models の再利用と dm_messages の固定は運用上の事故を防ぎます。LGTM。

app/api/server.ts (1)

27-27: DMルートの組み込みは適切です。

  • /api 配下に dm を追加した構成は既存のレートリミットと相性良好です(/api/ws のみ免除)。
  • KeyPackage関連の起動ジョブ除去と整合しています。

Also applies to: 36-36, 75-76

app/api/activity_handlers.ts (3)

81-85: Createの入力バリデーション(null/非オブジェクト判定)は妥当。

APオブジェクトでないケースの早期returnはよいです。


170-187: タイムライン配信のロジックは現状維持で妥当。

DM分岐後、非DMの通常投稿フローにフォールバックする制御は明確で、latest/following 双方へのブロードキャストも適切です。


119-129: レビューコメントに誤りがあります:saveMessage は汎用メッセージ用、saveDMMessage は DM 専用

activity_handlers.ts 内で呼ばれている db.saveMessage(domain, actor, …) は ActivityPub の一般メッセージ(公開・グループ等)向けの保存処理であり、DM のルーティングは別ファイル(app/api/routes/dm.ts)の db.saveDMMessage(from, to, content) で担われています。
スクリプトによる呼び出し・宣言の調査結果からも、各メソッドは以下のように定義・使用箇所が分かれています。

  • saveMessage(domain: string, author: string, content: string, extra: Record<string, unknown>, opts: {to, cc}): Promise<unknown[]>
    定義:app/shared/db.ts:53–55, 実装:app/api/DB/mongo.ts:309–311
    呼び出し:app/api/activity_handlers.ts:119–122
  • saveDMMessage(from: string, to: string, content: string): Promise
    定義:app/shared/db.ts:69, 実装:app/api/DB/mongo.ts:360–362
    呼び出し:app/api/routes/dm.ts:31

よって、activity_handlers.ts での saveMessage 呼び出しは意図どおりであり、saveDMMessage への置き換えは不要です。

Likely an incorrect or invalid review comment.

app/api/DB/mongo.ts (1)

15-15: DMMessage モデルの導入は妥当です

ホストモード用のテナント分離にも後続で対応しているため、この import は整合的です。

Comment on lines +370 to +380
async listDMsBetween(user1: string, user2: string) {
const query = this.withTenant(
DMMessage.find({
$or: [
{ from: user1, to: user2 },
{ from: user2, to: user1 },
],
}),
);
return await query.sort({ createdAt: 1 }).lean();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

DM取得にページングを追加し、スケール・UXを両立

全件返却は会話が伸びると負荷が大きくなります。ListOpts を受け、limit/before を適用してください。タイムラインに倣い降順で取得し、UIで並べ替える運用も選択肢です。

適用例(差分):

-  async listDMsBetween(user1: string, user2: string) {
+  async listDMsBetween(user1: string, user2: string, opts: ListOpts = {}) {
     const query = this.withTenant(
       DMMessage.find({
         $or: [
           { from: user1, to: user2 },
           { from: user2, to: user1 },
         ],
       }),
     );
-    return await query.sort({ createdAt: 1 }).lean();
+    if (opts.before) query.where("createdAt").lt(opts.before);
+    return await query
+      .sort({ createdAt: -1 })
+      .limit(opts.limit ?? 50)
+      .lean();
   }

あわせて DMMessage 側に以下のインデックスを追加するか、participants([user1,user2]をソートした配列)や conversationKey を導入すると $or を避けられ索引効率が上がります:

  • { from: 1, to: 1, createdAt: -1 }
  • もしくは { participants: 1, createdAt: -1 } / { conversationKey: 1, createdAt: -1 }

また、timestamps: true で createdAt が確実に生えることも確認してください。

以下で DM モデルの timestamps/インデックス有無を確認できます:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# DMモデルのスキーマ定義の要点を確認
fd -a dm_message.ts
echo '--- dm_message.ts (timestamps/index/participants/conversationKey の有無) ---'
rg -n -C2 -e 'timestamps' -e 'index' -e 'participants' -e 'conversationKey' -e 'createdAt' app/api/models/takos/dm_message.ts

Length of output: 640


ページング対応とインデックス最適化を実装してください

DMメッセージ量の増加時に全件取得は負荷が大きくなるため、以下の対応をお願いします。

  • app/api/DB/mongo.ts
    • listDMsBetweenopts: ListOpts = {} を追加
    • opts.before による createdAt フィルタリング
    • 降順取得 (.sort({ createdAt: -1 }))+件数制限 (.limit(opts.limit ?? 50))
  • app/api/models/takos/dm_message.ts
    • mongoose.Schema のオプションに timestamps: true を追加し、手動定義の createdAt は削除
    • 下記の複合インデックスを追加(高速化のため)
      • { from: 1, to: 1, createdAt: -1 }
      • ※将来的に participants 配列や conversationKey を導入して $or を排除する案も検討可

以下はサンプル差分です。

--- a/app/api/DB/mongo.ts
+++ b/app/api/DB/mongo.ts
@@ -370,7 +370,14 @@
-  async listDMsBetween(user1: string, user2: string) {
-    const query = this.withTenant(
-      DMMessage.find({
+  async listDMsBetween(user1: string, user2: string, opts: ListOpts = {}) {
+    let query = this.withTenant(
+      DMMessage.find({
         $or: [
           { from: user1, to: user2 },
           { from: user2, to: user1 },
         ],
-    return await query.sort({ createdAt: 1 }).lean();
+    if (opts.before) query.where("createdAt").lt(opts.before);
+    return await query
+      .sort({ createdAt: -1 })
+      .limit(opts.limit ?? 50)
+      .lean();
   }
--- a/app/api/models/takos/dm_message.ts
+++ b/app/api/models/takos/dm_message.ts
@@ -1,14 +1,16 @@
-const dmMessageSchema = new mongoose.Schema({
+const dmMessageSchema = new mongoose.Schema(
+  {
     from: { type: String, required: true, index: true },
     to: { type: String, required: true, index: true },
     content: { type: String, required: true },
-  createdAt: { type: Date, default: Date.now },
-});
+  },
+  {
+    timestamps: true,   // createdAt/updatedAt を自動生成
+  },
+);
+
+// 複合インデックスの追加
+dmMessageSchema.index({ from: 1, to: 1, createdAt: -1 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async listDMsBetween(user1: string, user2: string) {
const query = this.withTenant(
DMMessage.find({
$or: [
{ from: user1, to: user2 },
{ from: user2, to: user1 },
],
}),
);
return await query.sort({ createdAt: 1 }).lean();
}
async listDMsBetween(user1: string, user2: string, opts: ListOpts = {}) {
let query = this.withTenant(
DMMessage.find({
$or: [
{ from: user1, to: user2 },
{ from: user2, to: user1 },
],
}),
);
if (opts.before) query.where("createdAt").lt(opts.before);
return await query
.sort({ createdAt: -1 })
.limit(opts.limit ?? 50)
.lean();
}
Suggested change
async listDMsBetween(user1: string, user2: string) {
const query = this.withTenant(
DMMessage.find({
$or: [
{ from: user1, to: user2 },
{ from: user2, to: user1 },
],
}),
);
return await query.sort({ createdAt: 1 }).lean();
}
const dmMessageSchema = new mongoose.Schema(
{
from: { type: String, required: true, index: true },
to: { type: String, required: true, index: true },
content: { type: String, required: true },
},
{
timestamps: true, // createdAt/updatedAt を自動生成
},
);
// 複合インデックスの追加
dmMessageSchema.index({ from: 1, to: 1, createdAt: -1 });
🤖 Prompt for AI Agents
In app/api/DB/mongo.ts around lines 370-380, update listDMsBetween to accept
opts: ListOpts = {}, apply an optional createdAt filter when opts.before is
provided, change to sort by createdAt descending (.sort({ createdAt: -1 })), and
limit results to opts.limit ?? 50 (keep .lean()). In
app/api/models/takos/dm_message.ts update the schema options to timestamps: true
and remove any manually defined createdAt field, then add the compound indexes {
from: 1, to: 1, createdAt: -1 } (and any equivalent reverse if needed) to
improve query performance.

Comment on lines +4 to +9
const dmMessageSchema = new mongoose.Schema({
from: { type: String, required: true, index: true },
to: { type: String, required: true, index: true },
content: { type: String, required: true },
createdAt: { type: Date, default: Date.now },
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

DM検索を見据えた制約・インデックスを付与してください。

  • content に上限(例: 4000文字)を設けてWS/DB双方の負荷を抑制。
  • createdAt でのソート・範囲検索を想定し、複合インデックスを併用する前提でフィールド定義はそのままでOK。

提案差分:

 const dmMessageSchema = new mongoose.Schema({
   from: { type: String, required: true, index: true },
   to: { type: String, required: true, index: true },
-  content: { type: String, required: true },
+  content: { type: String, required: true, maxlength: 4000 },
   createdAt: { type: Date, default: Date.now },
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const dmMessageSchema = new mongoose.Schema({
from: { type: String, required: true, index: true },
to: { type: String, required: true, index: true },
content: { type: String, required: true },
createdAt: { type: Date, default: Date.now },
});
const dmMessageSchema = new mongoose.Schema({
from: { type: String, required: true, index: true },
to: { type: String, required: true, index: true },
content: { type: String, required: true, maxlength: 4000 },
createdAt: { type: Date, default: Date.now },
});
🤖 Prompt for AI Agents
In app/api/models/takos/dm_message.ts around lines 4 to 9, the schema needs
limits and an index for DM search: add a maxlength (e.g. 4000) to the content
field to cap message size and reduce WS/DB load, and add an index: true to
createdAt so range/sort queries are efficient (leaving field definitions
otherwise unchanged to allow future compound indexes).

Comment on lines +11 to +14
dmMessageSchema.plugin(tenantScope, { envKey: "ACTIVITYPUB_DOMAIN" });

const DMMessage = mongoose.models.DMMessage ??
mongoose.model("DMMessage", dmMessageSchema, "dm_messages");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

DMクエリ性能のための複合インデックスを追加しましょう。

当該APIは「当事者2名間の時系列取得」がホットパスです。{ from, to, createdAt } の複合インデックスでクエリ計画を安定化させてください(両方向検索はアプリ側で正規化 or $or で対応)。

提案差分:

 dmMessageSchema.plugin(tenantScope, { envKey: "ACTIVITYPUB_DOMAIN" });
+
+// 当事者間の時系列取得を高速化
+dmMessageSchema.index({ from: 1, to: 1, createdAt: 1 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dmMessageSchema.plugin(tenantScope, { envKey: "ACTIVITYPUB_DOMAIN" });
const DMMessage = mongoose.models.DMMessage ??
mongoose.model("DMMessage", dmMessageSchema, "dm_messages");
dmMessageSchema.plugin(tenantScope, { envKey: "ACTIVITYPUB_DOMAIN" });
// 当事者間の時系列取得を高速化
dmMessageSchema.index({ from: 1, to: 1, createdAt: 1 });
const DMMessage = mongoose.models.DMMessage ??
mongoose.model("DMMessage", dmMessageSchema, "dm_messages");
🤖 Prompt for AI Agents
In app/api/models/takos/dm_message.ts around lines 11 to 14, the DM queries need
a compound index to stabilize query plans for time-ordered retrieval between two
parties; add a schema-level compound index on { from: 1, to: 1, createdAt: -1 }
(or equivalent ordering your queries use) by calling dmMessageSchema.index(...)
before model compilation so MongoDB can use the index for the hot path; ensure
this addition is included alongside the tenantScope plugin and note that
bidirectional queries are handled by the application ($or or normalization) as
stated.

Comment on lines +12 to +13
app.use("/dm/*", authRequired);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

認証ミドルウェアが /dm に適用されていません(重大)。

app.use("/dm/*", ...) では /dm 自体(POST/GET)が保護されません。現状だと未認証アクセスが通ります。直ちに /dm にも適用してください。

提案差分:

 const app = new Hono();
-app.use("/dm/*", authRequired);
+app.use("/dm/*", authRequired);
+app.use("/dm", authRequired);
🤖 Prompt for AI Agents
In app/api/routes/dm.ts around lines 12-13, the authentication middleware is
currently registered as app.use("/dm/*", authRequired) which does not protect
the /dm path itself; change the registration to apply to the base path (e.g.
app.use("/dm", authRequired)) or add a separate app.use("/dm", authRequired)
before any /dm route definitions so that both /dm and /dm/* endpoints are
protected, and ensure this middleware is mounted before the route handlers.

Comment on lines +14 to +37
app.post(
"/dm",
zValidator(
"json",
z.object({
from: z.string(),
to: z.string(),
content: z.string(),
}),
),
async (c) => {
const { from, to, content } = c.req.valid("json") as {
from: string;
to: string;
content: string;
};
const db = createDB(getEnv(c));
const doc = await db.saveDMMessage(from, to, content) as { _id: string };
const payload = { id: doc._id, from, to, content };
sendToUser(to, { type: "dm", payload });
sendToUser(from, { type: "dm", payload });
return c.json(payload);
},
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

送信者偽装が可能です(重大):from をクライアントから受け取らないでください。

認証済みユーザーであっても、現状は任意の from を指定して他人になりすまして送信できます。from はサーバ側でセッションから導出し、リクエストボディの from を廃止・無視してください。併せて入力制約と201 Createdを返すよう修正を提案します。

提案差分:

 app.post(
   "/dm",
   zValidator(
     "json",
     z.object({
-      from: z.string(),
-      to: z.string(),
-      content: z.string(),
+      to: z.string().min(1),
+      content: z.string().min(1).max(4000),
     }),
   ),
   async (c) => {
-    const { from, to, content } = c.req.valid("json") as {
-      from: string;
-      to: string;
-      content: string;
-    };
-    const db = createDB(getEnv(c));
-    const doc = await db.saveDMMessage(from, to, content) as { _id: string };
-    const payload = { id: doc._id, from, to, content };
+    const { to, content } = c.req.valid("json") as { to: string; content: string };
+    const env = getEnv(c);
+    const domain = env["ACTIVITYPUB_DOMAIN"] ?? env["ROOT_DOMAIN"] ?? "";
+    // セッションから送信者を導出(文字列 or { username } 想定)
+    const sessionUser = (c as unknown as { get: (k: string) => unknown }).get("user") as unknown;
+    const expectedFrom =
+      typeof sessionUser === "string"
+        ? `${sessionUser}@${domain}`
+        : (sessionUser && typeof sessionUser === "object" && "username" in (sessionUser as Record<string, unknown>")
+          ? `${(sessionUser as { username: string }).username}@${domain}`
+          : "");
+    if (!expectedFrom) {
+      return c.json({ error: "unauthorized" }, 401);
+    }
+    const db = createDB(env);
+    const doc = await db.saveDMMessage(expectedFrom, to, content) as { _id: string; createdAt?: string | Date };
+    const payload = { id: String(doc._id), from: expectedFrom, to, content, createdAt: doc.createdAt };
-    sendToUser(to, { type: "dm", payload });
-    sendToUser(from, { type: "dm", payload });
-    return c.json(payload);
+    sendToUser(to, { type: "dm", payload });
+    sendToUser(expectedFrom, { type: "dm", payload });
+    return c.json(payload, 201);
   },
 );

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In app/api/routes/dm.ts around lines 14 to 37, the route currently trusts a
client-supplied "from" allowing sender spoofing; instead derive the sender from
the authenticated session (e.g., c.env/session or c.get("user") depending on
auth middleware), ignore or remove the request body "from" field, and validate
only "to" and "content" (apply any length/format constraints). Use the
server-derived sender when calling db.saveDMMessage and when building the
payload to send via sendToUser, and change the response to return HTTP 201
Created with the created payload. Ensure proper error handling for
unauthenticated requests (return 401) and adjust types so { from: string } is
not read from c.req.valid("json").

Comment on lines +39 to +54
app.get(
"/dm",
zValidator(
"query",
z.object({ user1: z.string(), user2: z.string() }),
),
async (c) => {
const { user1, user2 } = c.req.valid("query") as {
user1: string;
user2: string;
};
const db = createDB(getEnv(c));
const messages = await db.listDMsBetween(user1, user2);
return c.json(messages);
},
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

DM取得のアクセス制御がありません(重大)。

現状は認証済みであれば第三者の会話履歴も取得できます。クエリuser1/user2の少なくとも片方が認証ユーザーに一致することを検証してください。あわせて(インデックス活用のために)会話ペアの正規化も推奨します。

提案差分:

 app.get(
   "/dm",
   zValidator(
     "query",
     z.object({ user1: z.string(), user2: z.string() }),
   ),
   async (c) => {
-    const { user1, user2 } = c.req.valid("query") as {
+    const { user1, user2 } = c.req.valid("query") as {
       user1: string;
       user2: string;
     };
-    const db = createDB(getEnv(c));
+    const env = getEnv(c);
+    const domain = env["ACTIVITYPUB_DOMAIN"] ?? env["ROOT_DOMAIN"] ?? "";
+    const sessionUser = (c as unknown as { get: (k: string) => unknown }).get("user") as unknown;
+    const me =
+      typeof sessionUser === "string"
+        ? `${sessionUser}@${domain}`
+        : (sessionUser && typeof sessionUser === "object" && "username" in (sessionUser as Record<string, unknown>")
+          ? `${(sessionUser as { username: string }).username}@${domain}`
+          : "");
+    if (!me || (me !== user1 && me !== user2)) {
+      return c.json({ error: "forbidden" }, 403);
+    }
+    const db = createDB(env);
+    // 正規化(辞書順)でクエリのキャッシュ/インデックス効率を改善
+    const [a, b] = [user1, user2].sort();
-    const messages = await db.listDMsBetween(user1, user2);
+    const messages = await db.listDMsBetween(a, b);
     return c.json(messages);
   },
 );

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In app/api/routes/dm.ts around lines 39 to 54, the endpoint lacks access control
and pair normalization: require that at least one of the query params (user1 or
user2) equals the authenticated user ID (reject with 403 if not), normalize the
pair (e.g., order the two user IDs deterministically) before calling
db.listDMsBetween so the DB can use a single indexed key, and use the normalized
values for both authorization check and the DB query; ensure validation uses the
normalized pair consistently and return a forbidden response when the caller is
not a participant.

Comment on lines +92 to 104
// WebSocket は DM 通知用に使用する。
// 文字列メッセージは JSON を期待し、type フィールドでハンドラを呼ぶ。
if (typeof evt.data === "string") {
try {
const msg = JSON.parse(evt.data);
if (msg && msg.type === "handshake") {
// ハンドシェイク本体は REST (/rooms/:room/handshakes) へ移行済み
// ログを追加して、どの接続・ユーザーから来たかを追跡可能にする(リプレイや誤送信の確認用)
try {
const user = state.user as string | undefined;
console.warn("websocket: rejected handshake message", {
user: user ?? "unknown",
origin: (ws as any)?.context?.req?.headers?.get?.("origin") ??
null,
});
} catch (e) {
console.warn(
"websocket: rejected handshake (failed to read state)",
e,
);
}
ws.send(JSON.stringify({
error: "handshake_not_allowed_on_websocket",
message: "Use REST /rooms/:room/handshakes for MLS handshakes",
}));
return;
}
const handler = messageHandlers.get(msg.type);
handler?.(msg.payload, ws, state);
} catch {
ws.send(JSON.stringify({ error: "invalid message" }));
}
} else {
// バイナリデータは MLS ハンドシェイク等を含む可能性があるため拒否する
ws.send(
JSON.stringify({
error: "binary_payload_not_allowed",
message:
"Binary payloads (MLS) are not allowed over websocket; use REST APIs",
}),
);
ws.send(JSON.stringify({ error: "binary_payload_not_allowed" }));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

onMessage: 非同期ハンドラの未awaitにより未処理例外がWSを不安定化する恐れ

MessageHandler は Promise を返し得ますが、現状 await していないため未処理拒否や例外が表面化しません。ハンドラが失敗した場合のエラー応答も出ません。onMessage を async にし、型/存在チェックと await を入れて堅牢化してください。

-      onMessage(evt, ws) {
-        // WebSocket は DM 通知用に使用する。
-        // 文字列メッセージは JSON を期待し、type フィールドでハンドラを呼ぶ。
-        if (typeof evt.data === "string") {
-          try {
-            const msg = JSON.parse(evt.data);
-            const handler = messageHandlers.get(msg.type);
-            handler?.(msg.payload, ws, state);
-          } catch {
-            ws.send(JSON.stringify({ error: "invalid message" }));
-          }
-        } else {
-          ws.send(JSON.stringify({ error: "binary_payload_not_allowed" }));
-        }
-      },
+      async onMessage(evt, ws) {
+        // WebSocket は DM 通知用に使用する。
+        // 文字列メッセージは JSON を期待し、type フィールドでハンドラを呼ぶ。
+        if (typeof evt.data !== "string") {
+          ws.send(JSON.stringify({ error: "binary_payload_not_allowed" }));
+          return;
+        }
+        try {
+          const msg = JSON.parse(evt.data);
+          if (!msg || typeof msg.type !== "string") {
+            ws.send(JSON.stringify({ error: "invalid_message_shape" }));
+            return;
+          }
+          const handler = messageHandlers.get(msg.type);
+          if (!handler) {
+            ws.send(JSON.stringify({ error: "unknown_type", type: msg.type }));
+            return;
+          }
+          await handler(msg.payload, ws, state);
+        } catch {
+          ws.send(JSON.stringify({ error: "invalid message" }));
+        }
+      },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// WebSocket は DM 通知用に使用する。
// 文字列メッセージは JSON を期待し、type フィールドでハンドラを呼ぶ。
if (typeof evt.data === "string") {
try {
const msg = JSON.parse(evt.data);
if (msg && msg.type === "handshake") {
// ハンドシェイク本体は REST (/rooms/:room/handshakes) へ移行済み
// ログを追加して、どの接続・ユーザーから来たかを追跡可能にする(リプレイや誤送信の確認用)
try {
const user = state.user as string | undefined;
console.warn("websocket: rejected handshake message", {
user: user ?? "unknown",
origin: (ws as any)?.context?.req?.headers?.get?.("origin") ??
null,
});
} catch (e) {
console.warn(
"websocket: rejected handshake (failed to read state)",
e,
);
}
ws.send(JSON.stringify({
error: "handshake_not_allowed_on_websocket",
message: "Use REST /rooms/:room/handshakes for MLS handshakes",
}));
return;
}
const handler = messageHandlers.get(msg.type);
handler?.(msg.payload, ws, state);
} catch {
ws.send(JSON.stringify({ error: "invalid message" }));
}
} else {
// バイナリデータは MLS ハンドシェイク等を含む可能性があるため拒否する
ws.send(
JSON.stringify({
error: "binary_payload_not_allowed",
message:
"Binary payloads (MLS) are not allowed over websocket; use REST APIs",
}),
);
ws.send(JSON.stringify({ error: "binary_payload_not_allowed" }));
}
async onMessage(evt, ws) {
// WebSocket は DM 通知用に使用する。
// 文字列メッセージは JSON を期待し、type フィールドでハンドラを呼ぶ。
if (typeof evt.data !== "string") {
ws.send(JSON.stringify({ error: "binary_payload_not_allowed" }));
return;
}
try {
const msg = JSON.parse(evt.data);
if (!msg || typeof msg.type !== "string") {
ws.send(JSON.stringify({ error: "invalid_message_shape" }));
return;
}
const handler = messageHandlers.get(msg.type);
if (!handler) {
ws.send(JSON.stringify({ error: "unknown_type", type: msg.type }));
return;
}
await handler(msg.payload, ws, state);
} catch {
ws.send(JSON.stringify({ error: "invalid message" }));
}
},
🤖 Prompt for AI Agents
In app/api/routes/ws.ts around lines 92 to 104, the message handler invocation
is not awaited and lacks robust type/existence checks so async rejections or
exceptions can destabilize the WebSocket and no error response is sent; change
the onMessage callback to async, ensure msg is validated (msg && typeof msg.type
=== "string"), verify handler exists and is a function before calling, await the
handler invocation inside a try/catch, and on error send a JSON error response
(including a concise message) back to the client; keep the existing JSON.parse
try/catch for malformed messages and retain the binary payload rejection branch.

@@ -1,8 +1,8 @@
import { createEffect, createSignal, onCleanup, onMount, Show } from "solid-js";
import { createEffect, createSignal, onMount } from "solid-js";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Show を未インポートのまま使用しています(ビルドエラー)

JSX 内で を使っているため、solid-js からのインポートが必要です。

-import { createEffect, createSignal, onMount } from "solid-js";
+import { createEffect, createSignal, onMount, Show } from "solid-js";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { createEffect, createSignal, onMount } from "solid-js";
import { createEffect, createSignal, onMount, Show } from "solid-js";
🤖 Prompt for AI Agents
In app/client/src/components/Application.tsx around line 1, the JSX uses the
<Show> component but it is not imported from "solid-js", causing a build error;
update the import statement to include Show from "solid-js" (e.g., add Show to
the existing import list alongside createEffect, createSignal, onMount) so the
component can be used in the JSX without error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant