Open
Conversation
Walkthrough認証周りをMongoose直使用からDB抽象レイヤー（HostDataStore）経由に統合。ユーザー・セッションの作成/検索/更新/削除をdb.hostUsers／db.hostSessionsへ移行。型とリポジトリIFを追加し、Mongo実装を拡張。公開関数のシグネチャは保持しつつ内部実装を置換。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant AuthApp as Auth App
participant Users as DB.hostUsers
participant Sessions as DB.hostSessions
participant Mailer as Mailer
participant Cookie as Cookie
rect rgba(227,242,253,0.5)
note over Client,AuthApp: Registration
Client->>AuthApp: POST /register (userName,email,password)
AuthApp->>Users: findByUserNameOrEmail()
alt user exists
AuthApp->>Users: update(...verifyCode, verifyCodeExpires)
else new user
AuthApp->>Users: create(user, creds, verifyCode, expires)
end
AuthApp-->>Mailer: send verification email
AuthApp-->>Client: 200 Accepted
end
rect rgba(232,245,233,0.5)
note over Client,AuthApp: Verification
Client->>AuthApp: POST /verify (code)
AuthApp->>Users: update(emailVerified=true, clear verify fields)
AuthApp->>Sessions: create(sessionId, userId, expiresAt)
AuthApp->>Cookie: set(sessionId, expiresAt)
AuthApp-->>Client: 200
end
rect rgba(255,243,224,0.5)
note over Client,AuthApp: Login
Client->>AuthApp: POST /login (userName,password)
AuthApp->>Users: findByUserName()
AuthApp->>Sessions: create(sessionId, userId, expiresAt)
AuthApp->>Cookie: set(sessionId, expiresAt)
AuthApp-->>Client: 200
end
rect rgba(252,228,236,0.5)
note over Client,AuthApp: Status (session refresh)
Client->>AuthApp: GET /status (cookie)
AuthApp->>Sessions: findById(sessionId)
alt valid
AuthApp->>Sessions: updateExpires(sessionId, newExpires)
AuthApp->>Cookie: refresh(expiresAt)
AuthApp-->>Client: 200 with user
else invalid/expired
AuthApp->>Sessions: deleteById(sessionId)
AuthApp->>Cookie: clear()
AuthApp-->>Client: 401
end
end
rect rgba(237,231,246,0.5)
note over Client,AuthApp: Logout
Client->>AuthApp: POST /logout
AuthApp->>Sessions: deleteById(sessionId)
AuthApp->>Cookie: clear()
AuthApp-->>Client: 204
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
app/takos_host/repositories/session.ts (1)
11-13: 上記 getDB の問題を継承ここでの
getDB()呼び出しは、上記指摘の修正（シングルトン化）を適用すれば解消します。ロジック自体はシンプルで妥当です。実際に多重生成箇所がないか確認用:
#!/bin/bash rg -nP --type=ts '\bcreateDB\s*\(\s*\{\s*\}\s*\)'Also applies to: 20-22, 28-30
🧹 Nitpick comments (4)
app/takos_host/db/types.ts (2)
3-13: HostUser 型の公開境界を明確化（内部専用の旨をドキュメント化）hashedPassword/salt/verifyCode 等の機微情報を含むため、外部 API の戻り値としては絶対に露出しない前提を型コメントで明示しておくと安全です（誤用防止）。可能なら PublicHostUser（公開用）を別型で用意。
82-91: セッション検索に user 逆引きが必要なケースへ拡張余地
findByUser(userId: string)があるとユーザー削除時のセッションクリーンアップ等が楽になります（今は利用予定がなければスキップ可）。app/takos_host/auth.ts (2)
54-70: 登録処理に最低限のエラーハンドリングを追加DB 例外で 500 が漏れると UX が悪化します。try/catch で 500/共通エラー応答へ。
- app.post("/register", async (c) => { - const { userName, email, password, accepted } = await c.req.json(); + app.post("/register", async (c) => { + try { + const { userName, email, password, accepted } = await c.req.json(); // ...現行ロジック... - } + } catch (e) { + // TODO: logger 連携 + return c.json({ error: "internal" }, 500); + } });
208-221: authRequired でも毎回 getDB() 実行—ミドルウェアをファクトリ化して注入可能に共通 DB を注入できるようにすると一貫性が増します（既存定数 export は残しつつ新ファクトリを追加でも可）。
-export const authRequired: MiddlewareHandler = createAuthMiddleware({ +export const createHostAuthRequired = (db = getDB()): MiddlewareHandler => + createAuthMiddleware({ cookieName: "hostSessionId", errorMessage: "unauthorized", findSession: async (sid) => { - const db = getDB(); return await db.hostSessions.findById(sid); }, deleteSession: async (sid) => { - const db = getDB(); await db.hostSessions.deleteById(sid); }, updateSession: async (session, expires) => { - const db = getDB(); await db.hostSessions.updateExpires(session.sessionId, expires); }, attach: (c, session) => { c.set("user", { _id: session.user }); }, -}); + }); +// 既存エクスポート（後方互換） +export const authRequired: MiddlewareHandler = createHostAuthRequired();
📜 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.
📒 Files selected for processing (5)
app/takos_host/auth.ts(11 hunks)app/takos_host/db/mod.ts(1 hunks)app/takos_host/db/mongo_store.ts(2 hunks)app/takos_host/db/types.ts(2 hunks)app/takos_host/repositories/session.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Leverage TypeScript type definitions comprehensively
Implement proper error handling (do not swallow errors; return meaningful responses)
Files:
app/takos_host/db/types.tsapp/takos_host/db/mod.tsapp/takos_host/db/mongo_store.tsapp/takos_host/repositories/session.tsapp/takos_host/auth.ts
🧠 Learnings (2)
📚 Learning: 2025-08-24T00:10:09.315Z
Learnt from: CR
PR: takoserver/takos#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T00:10:09.315Z
Learning: Applies to app/api/models/**/*.ts : Define database models using Mongoose schemas (avoid ad-hoc collections)
Applied to files:
app/takos_host/db/mongo_store.ts
📚 Learning: 2025-08-24T00:10:09.315Z
Learnt from: CR
PR: takoserver/takos#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T00:10:09.315Z
Learning: Applies to app/api/models/**/*.ts : All MongoDB schemas must include a tenant_id field equal to the ActivityPub domain (tenant isolation)
Applied to files:
app/takos_host/db/mongo_store.ts
🧬 Code graph analysis (1)
app/takos_host/repositories/session.ts (1)
app/core/db/mod.ts (1)
getDB(33-35)
🔇 Additional comments (7)
app/takos_host/db/types.ts (2)
15-19: LGTM: HostSession 型は妥当必要最小限のフィールドで整っています。
100-101: LGTM: HostDataStore へのリポジトリ追加依存箇所の型安全性が上がります。
app/takos_host/db/mod.ts (1)
8-11: LGTM: 型の再エクスポート下流からの import 統一に有用です。循環参照の懸念も現状の構成では低いです。
app/takos_host/db/mongo_store.ts (3)
1-1: LGTM: 型のインポート追加HostUser の静的型付けが明確になっています。
7-8: LGTM: モデル依存の集約モデルの直接参照をストアに閉じ込める方針は良いです。
279-310: インデックスは既に設定済み、明示キャストは任意
sessionId に unique、expiresAt に TTL インデックスが app/takos_host/models/session.ts で設定済みです。また Mongoose は ObjectId 型フィールドに対して文字列を自動キャストするため、明示的な new ObjectId() は必須ではありません（必要があれば mongoose.Types.ObjectId.isValid() で検証可能）。app/takos_host/auth.ts (1)
173-178: LGTM: セッション延長と Cookie 更新の順序が適切状態遷移が明確で副作用も最小です。
Comment on lines
+7
to
+12
| import { createDB } from "@takos_host/db"; | ||
| import type { HostDataStore } from "./db/types.ts"; | ||
|
|
||
| function getDB(): HostDataStore { | ||
| return createDB({}) as HostDataStore; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
DB 初期化を遅延シングルトン化し、環境を注入（createDB({}) の都度生成を排除）
接続多重化とテナント/環境情報の欠落を防ぎます。
-import { createDB } from "@takos_host/db";
-import type { HostDataStore } from "./db/types.ts";
-
-function getDB(): HostDataStore {
- return createDB({}) as HostDataStore;
-}
+import { createDB } from "@takos_host/db";
+import type { HostDataStore } from "./db/types.ts";
+
+const getDB = (() => {
+ let store: HostDataStore | null = null;
+ return (): HostDataStore => {
+ if (store) return store;
+ const env = (globalThis as any).Deno?.env?.toObject?.() ?? {};
+ store = createDB(env) as HostDataStore;
+ return store;
+ };
+})();📝 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 { createDB } from "@takos_host/db"; | |
| import type { HostDataStore } from "./db/types.ts"; | |
| function getDB(): HostDataStore { | |
| return createDB({}) as HostDataStore; | |
| } | |
| import { createDB } from "@takos_host/db"; | |
| import type { HostDataStore } from "./db/types.ts"; | |
| const getDB = (() => { | |
| let store: HostDataStore | null = null; | |
| return (): HostDataStore => { | |
| if (store) return store; | |
| const env = (globalThis as any).Deno?.env?.toObject?.() ?? {}; | |
| store = createDB(env) as HostDataStore; | |
| return store; | |
| }; | |
| })(); |
Comment on lines
+130
to
+134
| await db.hostUsers.update(user._id, { | ||
| emailVerified: true, | ||
| verifyCode: undefined, | ||
| verifyCodeExpires: undefined, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
verifyCode の消去方法を $unset 化（types/mongo_store の提案に追従）
ここで undefined を渡すと物理削除されない恐れ。opts.unset を使うように変更してください（前述の IF 変更を適用した前提）。
- await db.hostUsers.update(user._id, {
- emailVerified: true,
- verifyCode: undefined,
- verifyCodeExpires: undefined,
- });
+ await db.hostUsers.update(
+ user._id,
+ { emailVerified: true },
+ { unset: ["verifyCode", "verifyCodeExpires"] },
+ );📝 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
| await db.hostUsers.update(user._id, { | |
| emailVerified: true, | |
| verifyCode: undefined, | |
| verifyCodeExpires: undefined, | |
| }); | |
| await db.hostUsers.update( | |
| user._id, | |
| { emailVerified: true }, | |
| { unset: ["verifyCode", "verifyCodeExpires"] }, | |
| ); |
🤖 Prompt for AI Agents
In app/takos_host/auth.ts around lines 130 to 134, the update currently sets
verifyCode and verifyCodeExpires to undefined which may not physically remove
the fields; change the update call to use the db update option opts.unset (per
types/mongo_store convention) to explicitly unset those fields (e.g., move
verifyCode and verifyCodeExpires into the unset object) while keeping
emailVerified: true in the normal $set part and assuming the prior IF change is
applied.
Comment on lines
+245
to
+278
| hostUsers: { | ||
| findByUserName: async (userName) => { | ||
| const doc = await HostUserModel.findOne({ userName }) | ||
| .lean<HostUser & { _id: mongoose.Types.ObjectId } | null>(); | ||
| return doc ? { ...doc, _id: String(doc._id) } : null; | ||
| }, | ||
| findByUserNameOrEmail: async (userName, email) => { | ||
| const doc = await HostUserModel.findOne({ | ||
| $or: [{ userName }, { email }], | ||
| }) | ||
| .lean<HostUser & { _id: mongoose.Types.ObjectId } | null>(); | ||
| return doc ? { ...doc, _id: String(doc._id) } : null; | ||
| }, | ||
| create: async (data) => { | ||
| const doc = new HostUserModel({ | ||
| userName: data.userName, | ||
| email: data.email, | ||
| hashedPassword: data.hashedPassword, | ||
| salt: data.salt, | ||
| verifyCode: data.verifyCode, | ||
| verifyCodeExpires: data.verifyCodeExpires, | ||
| emailVerified: false, | ||
| createdAt: new Date(), | ||
| }); | ||
| await doc.save(); | ||
| const obj = doc.toObject() as HostUser & { | ||
| _id: mongoose.Types.ObjectId; | ||
| }; | ||
| return { ...obj, _id: String(obj._id) }; | ||
| }, | ||
| update: async (id, update) => { | ||
| await HostUserModel.updateOne({ _id: id }, { $set: update }); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
HostUser 更新で $unset を使わず undefined を $set してしまう恐れ
update: async (id, update) => HostUserModel.updateOne({$set: update}) は verifyCode: undefined 等が消えない可能性が高いです。$set/$unset に分離してください。
- update: async (id, update) => {
- await HostUserModel.updateOne({ _id: id }, { $set: update });
- },
+ update: async (id, update, opts) => {
+ const set: Record<string, unknown> = {};
+ const unset: Record<string, "" | 1> = {};
+ for (const [k, v] of Object.entries(update)) {
+ if (v === undefined) unset[k] = "";
+ else set[k] = v;
+ }
+ if (opts?.unset) {
+ for (const k of opts.unset) unset[k as string] = "";
+ }
+ const u: Record<string, unknown> = {};
+ if (Object.keys(set).length) u.$set = set;
+ if (Object.keys(unset).length) u.$unset = unset;
+ if (Object.keys(u).length) {
+ await HostUserModel.updateOne({ _id: id }, u);
+ }
+ },追加で、email/userName のユニーク制約とローワーケース正規化はモデル側で要確認（存在しない場合は追加推奨）。
🏁 Script executed:
#!/bin/bash
rg -n 'schema|index|unique' app/takos_host/models/user.tsLength of output: 180
HostUser.update で undefined フィールドを削除するため $unset を利用＆ユーザー名/メールを小文字化してユニーク制約を安定化
- update 関数を以下のように、
undefinedのプロパティは$unset、それ以外は$setするロジックに分離してください。 - app/takos_host/models/user.ts 内の
userNameとemailフィールドにlowercase: trueを追加するか、pre-save フックで小文字化を保証してください（unique: trueは既に設定済み）。
🤖 Prompt for AI Agents
In app/takos_host/db/mongo_store.ts around lines 245-278, the HostUser.update
currently always $set fields which leaves undefined properties in documents;
change the update logic to separate updates into a $set object for properties
with defined values and a $unset object for properties explicitly set to
undefined, then call updateOne with only the non-empty operators (e.g., { $set:
{...} } and/or { $unset: {...} }). Also ensure stable uniqueness by normalizing
userName and email to lowercase in app/takos_host/models/user.ts — either add
lowercase: true to those schema fields or add a pre-save hook that lowercases
userName and email before saving.
Comment on lines
+65
to
+80
| export interface HostUserRepo { | ||
| findByUserName(userName: string): Promise<HostUser | null>; | ||
| findByUserNameOrEmail( | ||
| userName: string, | ||
| email: string, | ||
| ): Promise<HostUser | null>; | ||
| create(data: { | ||
| userName: string; | ||
| email: string; | ||
| hashedPassword: string; | ||
| salt: string; | ||
| verifyCode: string; | ||
| verifyCodeExpires: Date; | ||
| }): Promise<HostUser>; | ||
| update(id: string, update: Partial<HostUser>): Promise<void>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
update での「undefined によるフィールド消去」は危険—$unset をサポートする API に
呼び出し側（auth.ts 等）で verifyCode: undefined のように渡していますが、Mongo の $set に undefined が混在すると期待通り削除されない可能性があります。unset キーを受け取れるように IF を拡張してください。
例: 互換維持しつつオプションで unset を追加
-export interface HostUserRepo {
+export interface HostUserRepo {
findByUserName(userName: string): Promise<HostUser | null>;
findByUserNameOrEmail(
userName: string,
email: string,
): Promise<HostUser | null>;
create(data: {
userName: string;
email: string;
hashedPassword: string;
salt: string;
verifyCode: string;
verifyCodeExpires: Date;
}): Promise<HostUser>;
- update(id: string, update: Partial<HostUser>): Promise<void>;
+ update(
+ id: string,
+ update: Partial<HostUser>,
+ opts?: { unset?: (keyof HostUser)[] },
+ ): Promise<void>;
}📝 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
| export interface HostUserRepo { | |
| findByUserName(userName: string): Promise<HostUser | null>; | |
| findByUserNameOrEmail( | |
| userName: string, | |
| email: string, | |
| ): Promise<HostUser | null>; | |
| create(data: { | |
| userName: string; | |
| email: string; | |
| hashedPassword: string; | |
| salt: string; | |
| verifyCode: string; | |
| verifyCodeExpires: Date; | |
| }): Promise<HostUser>; | |
| update(id: string, update: Partial<HostUser>): Promise<void>; | |
| } | |
| export interface HostUserRepo { | |
| findByUserName(userName: string): Promise<HostUser | null>; | |
| findByUserNameOrEmail( | |
| userName: string, | |
| email: string, | |
| ): Promise<HostUser | null>; | |
| create(data: { | |
| userName: string; | |
| email: string; | |
| hashedPassword: string; | |
| salt: string; | |
| verifyCode: string; | |
| verifyCodeExpires: Date; | |
| }): Promise<HostUser>; | |
| update( | |
| id: string, | |
| update: Partial<HostUser>, | |
| opts?: { unset?: (keyof HostUser)[] }, | |
| ): Promise<void>; | |
| } |
🤖 Prompt for AI Agents
In app/takos_host/db/types.ts around lines 65-80, the HostUserRepo.update
signature must be extended to support explicit unset semantics so callers don't
attempt to clear fields by passing undefined; change the method to accept an
optional unset parameter (e.g. a list of field keys or a map of keys->boolean)
in addition to the Partial<HostUser> update payload, and ensure the repo
implementation translates that unset parameter into a MongoDB $unset operation
while keeping backward compatibility with existing callers that still pass only
the update object.
Comment on lines
+1
to
+5
| import { createDB } from "@takos_host/db"; | ||
| import type { HostDataStore, HostSession } from "../db/types.ts"; | ||
|
|
||
| export interface HostSessionData { | ||
| _id?: string; | ||
| sessionId: string; | ||
| expiresAt: Date; | ||
| user: unknown; | ||
| } | ||
| const getDB = (): HostDataStore => createDB({}) as HostDataStore; | ||
|
|
There was a problem hiding this comment.
createDB({}) を都度呼ぶ設計は危険（接続多重化・テナント情報欠落）—遅延シングルトン＋環境注入へ
毎回ストアを生成すると接続肥大・設定逸脱のリスクがあります。環境変数を注入し、同一プロセスで単一のストアを共有してください。
-import { createDB } from "@takos_host/db";
+import { createDB } from "@takos_host/db";
import type { HostDataStore, HostSession } from "../db/types.ts";
-const getDB = (): HostDataStore => createDB({}) as HostDataStore;
+// 環境注入＋遅延シングルトン
+const getDB = (() => {
+ let store: HostDataStore | null = null;
+ return (): HostDataStore => {
+ if (store) return store;
+ // Deno 実行環境を前提。別環境では適切な env 取得手段に差し替え。
+ const env = (globalThis as any).Deno?.env?.toObject?.() ?? {};
+ store = createDB(env) as HostDataStore;
+ return store;
+ };
+})();📝 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 { createDB } from "@takos_host/db"; | |
| import type { HostDataStore, HostSession } from "../db/types.ts"; | |
| export interface HostSessionData { | |
| _id?: string; | |
| sessionId: string; | |
| expiresAt: Date; | |
| user: unknown; | |
| } | |
| const getDB = (): HostDataStore => createDB({}) as HostDataStore; | |
| import { createDB } from "@takos_host/db"; | |
| import type { HostDataStore, HostSession } from "../db/types.ts"; | |
| // 環境注入＋遅延シングルトン | |
| const getDB = (() => { | |
| let store: HostDataStore | null = null; | |
| return (): HostDataStore => { | |
| if (store) return store; | |
| // Deno 実行環境を前提。別環境では適切な env 取得手段に差し替え。 | |
| const env = (globalThis as any).Deno?.env?.toObject?.() ?? {}; | |
| store = createDB(env) as HostDataStore; | |
| return store; | |
| }; | |
| })(); |
🤖 Prompt for AI Agents
In app/takos_host/repositories/session.ts lines 1-5, calling createDB({}) on
every invocation can create multiple connections and omit tenant/env config;
change to a lazily-initialized singleton that accepts injected
environment/config once and reuses the same HostDataStore for the process.
Implement a module-level private variable (e.g., store: HostDataStore | null)
and an init/get pattern: an initStore(config) that calls createDB(config) and
stores the result, and a getStore() that throws if not initialized (or
initializes lazily using process/env defaults), returning the singleton typed as
HostDataStore; ensure tenant/env configuration is passed through to createDB and
that the exported API uses the singleton, not repeated createDB calls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
テスト
deno fmt app/takos_host/db/types.ts app/takos_host/db/mod.ts app/takos_host/db/mongo_store.ts app/takos_host/auth.ts app/takos_host/repositories/session.tsdeno lint app/takos_host/db/types.ts app/takos_host/db/mod.ts app/takos_host/db/mongo_store.ts app/takos_host/auth.ts app/takos_host/repositories/session.tshttps://chatgpt.com/codex/tasks/task_e_68afe759b63c83288dea0f6b4f846dc0
Summary by CodeRabbit