Release: develop を main にマージ

[otomatty]

概要

develop にマージ済みの機能追加・不具合修正・テスト強化・DB/API 修正を main に取り込むリリース PR です。ハッシュタグ同期、WikiLink／タイトル連動、検索リファクタ、大規模なテスト追加、Drizzle 0018 マイグレーションなどが含まれます。

変更点

• src/lib/ / src/stores/ / src/hooks/: ハッシュタグ（#name）同期とステータス更新の E2E 連携、ページタイトル変更の WikiLink・タグ・ゴーストリンクへの伝播、同一タイトルによる誤リライト防止、buildGlobalSearchResults の純関数化、ストレージ／同期／タグ・WikiLink ユーティリティの拡張
• src/lib/agentSlashCommands/: スラッシュコマンド周りのテスト追加
• server/api/: セキュリティ・コアサービス・ルート等のユニットテスト、onboarding および pages.kind 用 Drizzle マイグレーション 0018 とジャーナル更新
• db/migrations/: オンボーディング・ページ kind 向け SQL マイグレーション（番号付け整備を含む）
• .github/ / scripts/ / packages/shared/: デフォルトブランチを develop に揃える設定・ワークフロー・スクリプトの調整
• src/pages/NoteSettings/: 削除後リダイレクトの意図をドキュメント（notes）で明文化

変更の種類

• 🐛 バグ修正 (Bug fix)
• ✨ 新機能 (New feature)
• 💥 破壊的変更 (Breaking change)
• 📝 ドキュメント (Documentation)
• 🎨 スタイル/リファクタリング (Style/Refactor)
• 🧪 テスト (Tests)
• 🔧 ビルド/CI (Build/CI)

テスト方法

1. origin/develop が origin/main より進んでいることを確認（本 PR の差分が意図どおりであること）。
2. ステージングまたはローカルで DB マイグレーション（server/api/drizzle の 0018 および db/migrations の該当ファイル）を適用し、onboarding / pages.kind が期待どおりであることを確認。
3. クライアントでページリネーム、WikiLink、ハッシュタグ同期、検索結果表示を回帰確認。
4. CI（lint / unit test）がすべて成功することを確認。

チェックリスト

• テストがすべてパスする（マージ前に CI を確認）
• Lint エラーがない
• 必要に応じてドキュメントを更新した
• コミットメッセージが Conventional Commits に従っている

スクリーンショット（UI 変更がある場合）

エディタ・検索・NoteSettings 周りに変更があります。必要に応じて Before/After を添付してください。

関連 Issue

Related to #725, #726, #731, #732, #733, #734, #735, #736, #737, #738, #739, #740, #742, #743, #744, #745, #746, #750, #751, #752, #753, #754, #755

---

Summary by CodeRabbit

• New Features

  • Tag-type links with resolved target IDs, title-rename propagation (auto-rewrite & ghost promotion), onboarding/welcome-page support, content-preview helper, and Hocuspocus invalidation.
  • New shared constants package for tag validation.

• Database & CI

  • Drizzle migration checks in CI and migration journal updated; migrations/workflow aligned with new onboarding migration.

• Improvements

  • End-to-end link-type support (sync, storage, search), editor tag sync/targetId handling, autosave/tag handling, and improved global-search dedupe.

• Documentation

  • Updated migration, onboarding, and project-structure docs.

• Tests

  • Large expansion of unit & integration tests across editor, sync, API, providers, and tooling.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7652ae6b-6dd5-4f7b-bcce-b8349b36fb02

📥 Commits

Reviewing files that changed from the base of the PR and between 83bb072 and f0cf358.

📒 Files selected for processing (1)

• .github/workflows/ci.yml

---

📝 Walkthrough

Walkthrough

Adds link-type ("wiki"/"tag") support across domain, storage, and sync; implements title-rename propagation with Yjs rewrites and Hocuspocus invalidation; extracts shared TAG_NAME_CHAR_CLASS into @zedi/shared; relocates Drizzle migrations under server/api/drizzle/ with CI migration-check; removes legacy migrations and adds many tests and CI updates.

Changes

Cohort / File(s)	Summary
Config & CI \.coderabbit\.yaml, .github/workflows/ci.yml, .github/workflows/claude-code-review.yml	Add "main" to auto-review base_branches; add drizzle-migration-check CI job and per-package Vitest runs; update workflow header comments about default-branch/OIDC handling.
Drizzle migrations & checks db/migrations/* (deleted), server/api/drizzle/0018_add_onboarding_and_page_kind.sql, server/api/drizzle/meta/_journal.json, scripts/check-drizzle-migrations.mjs	Delete legacy db/migrations files; add migration 0018 and journal entry; add CLI to enforce schema↔migration pairing with PR-skip marker support.
Docs & policy AGENTS.md, CLAUDE.md, README.md	Document migration workflow, shared-constant guidance, and update README paths to server/api/drizzle/.
Shared package packages/shared/*, package.json	Add @zedi/shared exporting TAG_NAME_CHAR_CLASS, with TS/Vitest config and tests; include package in root test runs.
Schema & migration journal server/api/src/schema/pages.ts, server/api/src/schema/index.ts	Add partial unique index for welcome pages; re-export LINK_TYPES/LinkType.
Title rename propagation server/api/src/services/titleRenamePropagationService.ts, server/api/src/services/ydocRenameRewrite.ts, server/api/src/lib/hocuspocusInvalidation.ts, server/api/src/lib/extractPlainTextFromYXml.ts	Add propagation service, Y.Doc rewrite helper (with targetId semantics), Hocuspocus invalidation helper, and plain-text preview extraction utilities (new exports/types).
Routes using propagation server/api/src/routes/pages.ts, server/api/src/routes/pageSnapshots.ts	Detect title renames and fire-and-forget propagate; delegate invalidation to shared helper with caller logPrefix.
API sync types src/lib/api/types.ts	Allow optional `link_type?: "wiki"
Domain types & page model src/types/page.ts, server/api/src/schema/...	Add LinkType/LINK_TYPES and add linkType to Link/GhostLink models.
Storage adapters & IndexedDB src/lib/storageAdapter/types.ts, src/lib/storageAdapter/StorageAdapter.ts, src/lib/storageAdapter/IndexedDBStorageAdapter.ts	Introduce LinkType, add linkType to stored records, migrate IndexedDB to v3 with backfill to "wiki", and scope reads/writes by linkType.
Repository & adapter impl src/lib/pageRepository.ts, src/lib/pageRepository/StorageAdapterPageRepository.ts	Update repository interface and adapter to accept optional linkType (default "wiki") and persist/scoped behavior.
Guest page store src/stores/pageStore.ts	Extend store APIs to accept linkType, bump persistence migration to v3, and backfill missing linkType → "wiki".
Sync logic & hooks src/lib/syncWikiLinks.ts, src/hooks/usePageQueries.ts, src/hooks/useSyncWikiLinks.ts, src/hooks/useGlobalSearch.ts	Scope sync by linkType; useSyncWikiLinks now returns { syncLinks, syncTags }; useWikiLinkExistsChecker returns pageTitleToId; export pure global-search helpers.
Editor marks & utilities src/components/editor/extensions/WikiLinkExtension.ts, src/components/editor/extensions/TagExtension.ts, src/lib/tagUtils.ts, src/lib/wikiLinkUtils.ts	Add targetId attribute to WikiLink/Tag marks; switch tag regex to shared TAG_NAME_CHAR_CLASS; update utils to optionally accept title→id maps to populate/preserve targetId.
Editor lifecycle & autosave src/components/editor/PageEditor/useEditorAutoSave.ts, src/components/editor/PageEditor/usePageEditorAutoSaveWithMutation.ts, src/components/editor/TiptapEditor/useEditorLifecycle.ts	Wire tag sync into autosave/lifecycle, always call wiki sync (even with empty links), dedupe tags and call syncTags when provided.
Client UI & hooks changes src/components/editor/TiptapEditor/*, src/components/editor/PageEditor/*, src/pages/SearchResults.tsx, src/pages/NoteSettings/index.tsx	Expose/consume targetId in bubble menu/status sync; apply dedup of shared search rows against personal IDs; minor redirect/comment tweaks.
Wide test additions/expansions many files under server/api/src/__tests__/**, src/hooks/**, src/lib/**, src/stores/**, packages/claude-sidecar/**, packages/shared/src/*, packages/ui/**	Add/expand hundreds of Vitest suites covering services, routes, middleware, editor behavior, sync bucketing, storage migrations, AI clients/providers, rename rewrite, and drift tests.
Scripts removed scripts/setup-develop-branch.sh	Remove legacy develop-branch setup script.

Sequence Diagram(s)

sequenceDiagram
  rect rgba(240,248,255,0.5)
  participant Client as Client
  participant API as Pages API
  participant DB as Database
  participant Service as TitleRenameService
  participant Hocus as Hocuspocus
  end

  Client->>API: PUT /api/pages/:id/content (new title)
  API->>DB: SELECT current title
  DB-->>API: current title
  API->>Service: propagateTitleRename(renamedPageId, oldTitle, newTitle) [fire-and-forget]
  Service->>DB: SELECT source pages linking to renamed page
  Service->>DB: SELECT ... FOR UPDATE page_contents per source
  Service->>Service: rewriteTitleRefsInDoc (Y.Doc)
  Service->>DB: UPDATE page_contents, pages.contentPreview
  Service->>Hocus: POST /internal/invalidate/:pageId (x-internal-secret)
  Hocus-->>Service: 2xx / non-2xx (logged)
  API-->>Client: 200 OK (response not blocked)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

• feat: wire hashtag (#name) sync + status refresh end-to-end (issue #725 Phase 1) #733 — Related: overlapping link/tag sync, storage, and repository LinkType changes.
• fix: prevent same-title page rewrites by matching targetId (issue #737) #738 — Related: shared TAG_NAME_CHAR_CLASS, targetId support, and rename-rewrite logic overlap.
• fix(api): add missing 0018 migration for onboarding & pages.kind #755 — Related: Drizzle migration updates and CI migration-check additions touching migration journal/tooling.

Suggested labels

enhancement

Poem

🐰 I nibble constants, hop through tests,
Tags and wikis settle into nests.
Migrations march and journals sing,
Renames ripple — Hocuspocus ping.
A tiny hop for every CI pass, hooray!

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch develop

[gemini-code-assist]

Code Review

This pull request implements a robust system for propagating page title renames across WikiLinks and tags, introduces the @zedi/shared package for workspace-wide constants, and adds a CI check to ensure database schema changes are paired with migrations. It also updates the link graph to distinguish between WikiLinks and hashtags using a new link_type field. Feedback identifies a critical bug in the IndexedDB migration logic where asynchronous schema modifications will trigger runtime errors. Furthermore, there are performance concerns regarding the sequential execution of rename propagations and the use of database queries that bypass indexes, which could lead to scalability issues.

[gemini-code-assist]

この migrateLinkStoreToV3 関数内の実装には重大な問題があります。IndexedDB の createObjectStore および deleteObjectStore メソッドは、onupgradeneeded イベントハンドラの同期的な実行コンテキスト内でのみ呼び出すことができます。getAll() の onsuccess コールバックのような非同期コンテキストでこれらのメソッドを呼び出すと、InvalidStateError がスローされます。

IndexedDB では既存のストアの keyPath を変更できないため、データを保持したまま移行するには、onupgradeneeded 内で同期的に新しい名前のストアを作成し、そのトランザクション内でカーソル等を用いてデータをコピーする手法が一般的です。現状の実装では既存ユーザーの環境で DB アップグレードが失敗し、アプリが正常に起動できなくなるため、修正が必須です。

[otomatty]

確認しました。現実装は onupgradeneeded から取得した versionchange transaction 内の request callback で完結しており、deleteObjectStore / createObjectStore の呼び出し条件を満たしているため修正不要と判断しました。

[gemini-code-assist]

参照元ページをループで1つずつ処理していますが、特定のページが大量のページから参照されている場合（例：数百〜数千ページ）、この処理が完了するまでサーバーのリソース（DB接続やCPU）を長時間占有し続ける可能性があります。

fire-and-forget で実行されているためユーザーのレスポンスは直接ブロックしませんが、システム全体の安定性のために、Promise.all を用いた並列実行（同時実行数の制限付き）や、一定数ずつのチャンク処理、あるいはコメントにある通りバックグラウンドジョブキューへの移行を検討してください。また、Hocuspocus への無効化リクエストもループ内で順次実行されているため、ネットワーク遅延の影響を強く受けます。

[otomatty]

ご指摘ありがとうございます。並列化や job queue 化は妥当な改善ですが、今回のリリースブロッカーではなく負荷制御設計を伴うため別 Issue で扱うのが適切と判断しました。

[gemini-code-assist]

このクエリで LOWER(TRIM(...)) を使用していますが、これは ghost_links テーブルの link_text カラムに対する標準的なインデックスを無効化し、フルスキャンを発生させます。

ghost_links テーブルのレコード数が増加した場合、タイトル変更のたびにパフォーマンスが著しく低下する可能性があります。パフォーマンスを維持するためには、PostgreSQL の関数インデックス（Functional Index）を作成するか、保存時に正規化済みの値を別カラムに保持してインデックスを貼ることを検討してください。

[otomatty]

ご指摘ありがとうございます。Functional Index 追加は DB migration を伴う性能改善のため、このレビュー対応 PR ではスコープ外とし、別対応候補とします。

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 7 additional findings.

[coderabbitai]

Actionable comments posted: 13

Caution

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

⚠️ Outside diff range comments (2)

src/components/editor/TiptapEditor/useWikiLinkStatusSync.ts (1)

109-124: ⚠️ Potential issue | 🟠 Major

Guard against stale async sync results before mutating the editor.

checkExistence() is awaited here, but the effect has no generation/cancellation check after the await. If pageId, pageNoteId, or content changes while the request is in flight, the old run can still call applyWikiLinkUpdates() and onChange() with outdated exists / referenced / targetId data.

Suggested fix

 useEffect(() => {
+  let cancelled = false;
+
   if (skipSync || !editor || !content || !pageId) {
     return;
   }

   const updateWikiLinkStatus = async () => {
     if (currentWikiLinks.length === 0) {
+      if (cancelled) return;
       lastCheckedRef.current = { pageId, wikiLinkCount: 0, pageScopeSignature };
       return;
     }

     const titles = getUniqueWikiLinkTitles(currentWikiLinks);

     const { pageTitles, referencedTitles, pageTitleToId } = await checkExistence(titles, pageId);
+    if (cancelled) return;

     if (pageTitles.size === 0 && titles.length > 0) {
       return;
     }

     const updates = collectWikiLinkUpdates(editor, pageTitles, referencedTitles, pageTitleToId);

+    if (cancelled) return;
     lastCheckedRef.current = { pageId, wikiLinkCount: currentCount, pageScopeSignature };

     if (updates.length > 0) {
       applyWikiLinkUpdates(editor, updates);
+      if (cancelled) return;
       const json = JSON.stringify(editor.getJSON());
       onChange(json);
     }
   };

   const timer = setTimeout(updateWikiLinkStatus, 150);
-  return () => clearTimeout(timer);
+  return () => {
+    cancelled = true;
+    clearTimeout(timer);
+  };
 }, [skipSync, editor, content, checkExistence, pageId, onChange, pageScopeSignature]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/editor/TiptapEditor/useWikiLinkStatusSync.ts` around lines 109
- 124, After awaiting checkExistence(titles, pageId) guard against stale results
by validating the run is still current before mutating the editor: capture the
current run identity (e.g., pageId/pageNoteId/content or the computed
pageScopeSignature/currentCount) into a local variable before the await and then
compare it to the latest values (or lastCheckedRef/current refs) immediately
after the await; if they differ, return early and do not call
collectWikiLinkUpdates or applyWikiLinkUpdates. Ensure the same check is
performed before calling applyWikiLinkUpdates(editor, updates) so outdated async
responses never mutate the editor or update lastCheckedRef.current.

src/components/editor/PageEditor/useEditorAutoSave.ts (1)

85-93: ⚠️ Potential issue | 🟠 Major

Skip graph sync when unmount flush save throws

At Line 87, a thrown saveAction() is logged, but Line 92 still runs syncGraphFromContent(...). That can write wiki/tag edges for content that did not persist, causing graph/content drift.

💡 Suggested fix

           void (async () => {
+            let saveCompletedWithoutThrow = false;
             try {
               await saveAction();
+              saveCompletedWithoutThrow = true;
             } catch (e) {
               console.error("Auto-save flush on unmount failed:", e);
             }
-            try {
-              await syncGraphFromContent(pending.content);
-            } catch {
-              // Ignore sync errors during unmount flush
+            if (saveCompletedWithoutThrow) {
+              try {
+                await syncGraphFromContent(pending.content);
+              } catch {
+                // Ignore sync errors during unmount flush
+              }
             }
           })();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/editor/PageEditor/useEditorAutoSave.ts` around lines 85 - 93,
The unmount auto-save IIFE currently calls syncGraphFromContent(pending.content)
even if saveAction() failed; change the flow in useEditorAutoSave so
syncGraphFromContent is only invoked when saveAction resolves successfully—e.g.,
introduce a success flag or return early after the catch in the async IIFE
surrounding saveAction(), and reference saveAction, syncGraphFromContent, and
pending.content when updating the logic so graph sync is skipped if the save
failed.

🧹 Nitpick comments (12)

src/hooks/useMermaidGenerator.test.ts (1)

142-143: Test intent and mock behavior are mismatched (synchronous throw vs async rejection).

The test name says synchronous throw, but mockRejectedValueOnce exercises an async rejection path. Either rename the test or change the mock to a real sync throw.

Suggested alignment (sync throw path)

-  it("generate catches synchronous throws from generateMermaidDiagram and sets error", async () => {
-    mockGenerateMermaidDiagram.mockRejectedValueOnce(new Error("network"));
+  it("generate catches synchronous throws from generateMermaidDiagram and sets error", async () => {
+    mockGenerateMermaidDiagram.mockImplementationOnce(() => {
+      throw new Error("network");
+    });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useMermaidGenerator.test.ts` around lines 142 - 143, The test
"generate catches synchronous throws from generateMermaidDiagram and sets error"
currently uses mockGenerateMermaidDiagram.mockRejectedValueOnce which simulates
an async rejection; change the mock to throw synchronously instead (e.g., use
mockImplementationOnce to immediately throw new Error("network")) so the test
actually exercises the synchronous-throw path of generateMermaidDiagram; update
the mockGenerateMermaidDiagram call in the same test (the it(...) block) to use
the synchronous-throw implementation, or alternatively rename the test to
reflect an async rejection if you prefer that behavior.

src/hooks/useGeneralSettings.test.ts (1)

77-246: Consider extracting a shared renderReady() helper to remove repeated setup boilerplate.

renderHook + waitFor(isLoading === false) is repeated across many tests; a tiny helper will improve readability and reduce copy/paste drift.

♻️ Example refactor

+const renderReady = async () => {
+  const hook = renderHook(() => useGeneralSettings());
+  await waitFor(() => expect(hook.result.current.isLoading).toBe(false));
+  return hook;
+};

 it("updateTheme persists, updates state, and syncs next-themes", async () => {
-  const { result } = renderHook(() => useGeneralSettings());
-  await waitFor(() => expect(result.current.isLoading).toBe(false));
+  const { result } = await renderReady();
   mockSaveGeneralSettings.mockClear();
   mockSetTheme.mockClear();
   ...
 });

 it("updateEditorFontSize persists the new preset", async () => {
-  const { result } = renderHook(() => useGeneralSettings());
-  await waitFor(() => expect(result.current.isLoading).toBe(false));
+  const { result } = await renderReady();
   ...
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useGeneralSettings.test.ts` around lines 77 - 246, Extract a small
helper (e.g. renderReady) to replace repeated renderHook + waitFor boilerplate:
it should call renderHook(() => useGeneralSettings()), await waitFor(() =>
expect(result.current.isLoading).toBe(false)), and return the hook result (and
optionally accept a list of vi mock functions to clear after ready). Replace
each test's renderHook + waitFor sequence with a single call to renderReady(),
and remove duplicated mockClear calls where you pass mocks into the helper;
reference useGeneralSettings, renderHook, and waitFor when locating the repeated
code to change.

server/api/src/__tests__/services/subscriptionService.test.ts (1)

41-104: Consider adding an explicit cache-reset strategy per test.

Current isolation depends on unique userId values. A test-only cache clear hook (or module reset per test block) would make this suite less brittle as more cases are added.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/__tests__/services/subscriptionService.test.ts` around lines
41 - 104, Add an explicit per-test cache reset: introduce and export a test-only
clearUserTierCache() in the module that implements getUserTier (so tests can
call it), then call clearUserTierCache() in the test suite afterEach hook (in
the describe("getUserTier") block) to guarantee isolation; alternatively, if you
prefer not to add an export, call vi.resetModules() in afterEach and re-import
getUserTier in each test to ensure the module-level cache is cleared between
tests.

src/lib/aiChatConversationTitle.test.ts (1)

5-31: Harden chain(...) against empty input.

leafId is read from nodes[nodes.length - 1].id; a zero-arg call would fail with an opaque runtime error. A small guard keeps future test failures explicit.

Proposed patch

 function chain(
   ...nodes: Array<
     Partial<TreeChatMessage> & { id: string; role: TreeChatMessage["role"]; content: string }
   >
 ): {
   map: Record<string, TreeChatMessage>;
   leafId: string;
 } {
+  if (nodes.length === 0) {
+    throw new Error("chain() requires at least one node");
+  }
+
   const map: Record<string, TreeChatMessage> = {};
   let parentId: string | null = null;
   let ts = 1;
   for (const n of nodes) {
     map[n.id] = {
@@
     };
     parentId = n.id;
   }
-  return { map, leafId: nodes[nodes.length - 1].id };
+  return { map, leafId: nodes[nodes.length - 1]!.id };
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/aiChatConversationTitle.test.ts` around lines 5 - 31, The chain(...)
helper should guard against being called with zero arguments: detect if
nodes.length === 0 and throw a clear Error (e.g. "chain requires at least one
node") instead of reading nodes[nodes.length - 1].id; update the leafId
assignment in the chain function to only read nodes[nodes.length - 1].id after
that guard so empty calls fail with an explicit message and not an opaque
runtime error.

src/lib/agentSlashCommands/insertSlashAgentMarkdown.test.ts (1)

20-31: Capture and assert run() to lock command-chain execution semantics.

Right now the spy setup validates insertContentAt arguments, but not whether the chain actually executes. Exposing run in the helper and asserting it would harden this suite.

🔧 Suggested hardening

 function makeChainSpy(): {
   chain: () => {
     focus: () => { insertContentAt: ReturnType<typeof vi.fn> };
   };
   insertContentAt: ReturnType<typeof vi.fn>;
+  run: ReturnType<typeof vi.fn>;
 } {
-  const insertContentAt = vi.fn(() => ({ run: vi.fn() }));
+  const run = vi.fn();
+  const insertContentAt = vi.fn(() => ({ run }));
   const chain = vi.fn(() => ({
     focus: vi.fn(() => ({ insertContentAt })),
   }));
-  return { chain, insertContentAt };
+  return { chain, insertContentAt, run };
 }

Also applies to: 37-49

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/agentSlashCommands/insertSlashAgentMarkdown.test.ts` around lines 20
- 31, The test helper makeChainSpy currently returns insertContentAt and chain
mocks but doesn't expose the run() spy, so tests can't assert that the chain was
executed; update makeChainSpy to have insertContentAt return an object with a
run mock (e.g., const run = vi.fn()) and expose that run on the helper return
value (return { chain, insertContentAt, run }); then update tests that use
makeChainSpy to assert run was called (e.g., expect(run).toHaveBeenCalled())
after invoking the code that should execute the chain; reference the
makeChainSpy, chain, insertContentAt, and run symbols when making changes.

src/lib/agentSlashCommands/agentSlashEditorText.test.ts (1)

51-53: Use an explicit \uFFFC escape (or named constant) instead of the invisible literal.

"" is visually ambiguous and can be accidentally altered by editors. Using "\uFFFC" keeps intent explicit in reviews and diffs.

💡 Suggested tweak

-    expect(textBetween).toHaveBeenCalledWith(2, 7, "\n", "");
+    expect(textBetween).toHaveBeenCalledWith(2, 7, "\n", "\uFFFC");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/agentSlashCommands/agentSlashEditorText.test.ts` around lines 51 -
53, Replace the invisible U+FFFC literal in the assertion with an explicit
escape or constant: update the expect(...toHaveBeenCalledWith(2, 7, "\n", "")
call in the agentSlashEditorText.test.ts test to use "\uFFFC" (or a named
constant like OBJECT_REPLACEMENT = "\uFFFC") instead of the invisible character
so the test assertion on textBetween/toHaveBeenCalledWith is explicit and safe
to edit in diffs.

src/lib/agentSlashCommands/executeAgentSlashCommand.test.ts (1)

96-105: Prefer mockReset() over mockClear() here to prevent future implementation leakage between tests.

This setup otherwise resets implementations for most mocks, so aligning these two improves suite isolation consistency.

♻️ Suggested setup tweak

 beforeEach(() => {
   vi.mocked(runClaudeQueryToCompletion).mockReset();
-  vi.mocked(buildAgentSlashPrompt).mockClear();
-  vi.mocked(getAgentSlashClaudeOptions).mockClear();
+  vi.mocked(buildAgentSlashPrompt).mockReset().mockReturnValue("PROMPT");
+  vi.mocked(getAgentSlashClaudeOptions)
+    .mockReset()
+    .mockReturnValue({ maxTurns: 5, allowedTools: ["Read"] });
   vi.mocked(getEditorSelectionText).mockReset().mockReturnValue("");
   vi.mocked(insertSlashAgentMarkdownAt).mockReset();
   vi.mocked(readSlashAgentInsertPosition).mockReset().mockReturnValue("cursor");
   vi.mocked(clearLastSlashAgentSelection).mockReset();
   vi.mocked(getLastSlashAgentSelection).mockReset().mockReturnValue("");
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/agentSlashCommands/executeAgentSlashCommand.test.ts` around lines 96
- 105, The test setup mixes mockReset() and mockClear(), which risks leftover
mock implementations leaking between tests; change the two calls using
mockClear()—specifically vi.mocked(buildAgentSlashPrompt).mockClear() and
vi.mocked(getAgentSlashClaudeOptions).mockClear()—to use mockReset() instead so
all mocks are fully reset (consistent with the other mocks like
runClaudeQueryToCompletion, getEditorSelectionText, insertSlashAgentMarkdownAt,
readSlashAgentInsertPosition, clearLastSlashAgentSelection, and
getLastSlashAgentSelection) and prevent future implementation leakage.

src/hooks/useWorkflowDraft.test.ts (1)

205-231: Test title and assertions are slightly out of sync.

The test name says “sanitized filename”, but the assertions currently validate URL lifecycle/click behavior only. Either assert the download filename too, or rename the test to match current assertions.

✏️ Minimal rename option

-  it("exportJson opens a download link with sanitized filename", () => {
+  it("exportJson opens a download link and revokes object URL", () => {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useWorkflowDraft.test.ts` around lines 205 - 231, The test title
says it verifies a “sanitized filename” but the assertions only check URL
lifecycle and click behavior; update the test to either (A) add an assertion
that the created anchor's download attribute matches the expected sanitized
filename (targeting the exportJson flow in useWorkflowDraft and using the same
spies createObjectURLSpy/clickSpy/revokeObjectURLSpy and setDraft to set the
name), or (B) more simply rename the test to reflect what it actually asserts
(e.g., change the it description to something like "exportJson opens a download
link and revokes the URL") so the title and assertions match.

server/api/src/__tests__/middleware/csrfOrigin.test.ts (1)

61-64: Add HEAD to the safe-method bypass test matrix.

Line 61 currently tests only GET and OPTIONS, but this file’s contract states safe methods include HEAD as well. Please include HEAD to keep tests/spec in sync.

Suggested patch

-    it.each(["GET", "OPTIONS"] as const)("%s does not require Origin/Referer", async (method) => {
+    it.each(["GET", "HEAD", "OPTIONS"] as const)("%s does not require Origin/Referer", async (method) => {

As per coding guidelines **/*.{test,spec}.{ts,tsx,js,jsx}: Tests serve as a source of truth for specifications alongside implementation code TSDoc/JSDoc.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/__tests__/middleware/csrfOrigin.test.ts` around lines 61 - 64,
The test for safe-method bypass in the it.each block named "%s does not require
Origin/Referer" currently uses ["GET","OPTIONS"]; update that array to include
"HEAD" so the matrix becomes ["GET","HEAD","OPTIONS"] (the test uses
createApp().request("/api/state", { method }) and expects status 200); ensure
only the method list is changed so the test name and assertions remain intact.

src/lib/pageRepository/StorageAdapterPageRepository.test.ts (1)

257-279: Assert that the tag bucket is never written in the wiki-only isolation test.

This test only proves addLink("s", "t", "wiki") does not read the "tag" bucket. It would still pass if the implementation accidentally called saveLinks(..., "tag") and wiped that bucket without reading it first. Add a negative assertion on the third argument to saveLinks as well.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/pageRepository/StorageAdapterPageRepository.test.ts` around lines 257
- 279, The test currently only checks that adapter.getLinks was never called for
"tag" but misses verifying writes; update the test surrounding
repo.addLink("s","t","wiki") to also assert adapter.saveLinks was never invoked
with a linkType of "tag" (i.e., inspect/mock.calls of adapter.saveLinks and
ensure no call has the third argument equal to "tag") so the test fails if
implementation writes to the tag bucket even without reading it.

server/api/src/lib/hocuspocusInvalidation.ts (1)

67-82: Use finally for timeout cleanup

clearTimeout(timeoutId) is duplicated in both success and error paths. Moving it to finally simplifies maintenance and avoids accidental misses if logic expands.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/lib/hocuspocusInvalidation.ts` around lines 67 - 82, The
clearTimeout(timeoutId) call is duplicated in the try and catch around the fetch
in hocuspocusInvalidation.ts; remove both duplicates and place a single
clearTimeout(timeoutId) inside a finally block after the catch so the timer is
always cleared regardless of success or error, keeping the existing error
handling (the catch that computes name and logs/handles the error) and
preserving controller.signal usage for the fetch.

src/components/editor/PageEditor/useEditorAutoSave.ts (1)

63-81: Deduplicate syncGraphFromContent to prevent contract drift

The same sync routine is defined twice (unmount and normal save path). Extracting one shared callback will reduce divergence risk for future edits.

Also applies to: 122-133

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/editor/PageEditor/useEditorAutoSave.ts` around lines 63 - 81,
The sync routine is duplicated; extract the logic into a single shared async
function (e.g., syncGraphFromContent) that accepts content (and uses pageId,
extractWikiLinksFromContent, syncWikiLinks, extractTagsFromContent,
getUniqueTagNames, and syncTags) and then call that single function from both
the unmount path and the normal save path so both code paths reuse the same
implementation and avoid contract drift.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@AGENTS.md`:
- Around line 60-61: The English sentence in AGENTS.md merges inline code spans
with adjacent words (e.g.,
``server/api/drizzle/NNNN_*.sql``and``server/api/drizzle/meta/_journal.json``
and `/api/onboarding/status`and`/api/pages`); edit the line so each backticked
path and endpoint is separated by spaces or punctuation from surrounding text
(ensure a space before and after each inline code token like
`server/api/drizzle/NNNN_*.sql`, `server/api/drizzle/meta/_journal.json`,
`/api/onboarding/status`, and `/api/pages`) so the code spans render as distinct
tokens and the examples are copyable.

In `@server/api/src/lib/extractPlainTextFromYXml.ts`:
- Around line 73-77: The buildContentPreview function currently slices to
CONTENT_PREVIEW_MAX_LENGTH and then appends "..." which makes the returned
string longer than the declared max; update the logic in buildContentPreview to
account for the ellipsis length by slicing to CONTENT_PREVIEW_MAX_LENGTH - 3 (or
to 0 if that would be negative) before appending "..." so the final returned
preview never exceeds CONTENT_PREVIEW_MAX_LENGTH, and preserve the existing
trimming/whitespace collapse behavior.

In `@src/components/editor/extensions/slashSuggestionPlugin.test.ts`:
- Around line 150-155: Update the test comments in
src/components/editor/extensions/slashSuggestionPlugin.test.ts to include
Japanese translations alongside the existing English comments: for the comment
above the makeState("/foo", plugin) assertion (currently "1) Activate with
`/foo`."), the comment about expanding the selection ("2) Expand the selection
to a range; the plugin must turn off."), and the other English-only comment
referenced later (around the second assertion at line ~184). Keep the original
English text and add concise Japanese equivalents immediately after each English
sentence so both languages appear in the same comment block; no code logic
changes required—only edit the comment text surrounding makeState,
slashSuggestionPluginKey.getState, and onStateChange usages.

In `@src/components/editor/extensions/TagExtension.test.ts`:
- Around line 223-227: Replace the ad-hoc "as any" casts and eslint-disable
comments around the context passed to addAttributes.call() with an explicit
typed context object: declare a const (e.g., ctx) with a precise type matching
what addAttributes expects (for example an object type like { parent?: Node |
undefined; [key: string]: unknown } or the specific attribute/context interface
used by the TagExtension tests), spread the extension into that typed object
(instead of casting extension as any) and pass ctx to addAttributes.call(ctx,
...). Remove the eslint-disable lines and ensure you import/alias any necessary
types (e.g., Node or the extension context interface) so the test uses explicit
TypeScript types rather than "any".

In `@src/components/editor/extensions/WikiLinkExtension.test.ts`:
- Around line 111-115: Replace the unsafe `as any` casts used when invoking
addAttributes.call(...) by creating an explicit context type that combines the
extension's attribute shape plus the parent function signature, then pass a
properly typed object to addAttributes.call; specifically, define an interface
(e.g., ExtensionContext) that includes all properties from the `extension` under
test and a `parent: () => string | undefined` (or the actual parent return type
used by addAttributes), cast the constructed context to that interface instead
of `any`, and update the same pattern in TagExtension.test (lines ~222–225) so
both tests remove eslint-disable and satisfy TypeScript strict mode.

In `@src/components/editor/extensions/WikiLinkExtension.ts`:
- Around line 121-125: The parseHTML implementation currently returns
element.getAttribute("data-target-id") which yields an empty string for
data-target-id="" and breaks code expecting string | null; update parseHTML (in
WikiLinkExtension) to normalize empty values to null by checking the returned
value and returning null when it's strictly "" or not a string, so upstream
logic that expects null for "no target" receives null instead of an empty
string.

In `@src/components/editor/TiptapEditor/useWikiLinkStatusSync.test.tsx`:
- Around line 22-25: Update the test comment to reflect that the mock contract
now constructs pageTitleToId from the inScope set rather than returning an empty
Map; mention the intent remains that the test only asserts exists/referenced
changes (i.e., resolution behavior is exercised but full id resolution is
unnecessary) and point to the specific symbols used in the test (pageTitleToId
and inScope) so future readers know the map is derived from inScope for mocking
purposes.

In `@src/hooks/useImageUpload.test.ts`:
- Around line 13-18: Update the hoisted mock so the storage helper mocks match
and forward the real function signatures: change getStorageProvider and
isStorageConfiguredForUpload to accept the same parameters as the production
functions and forward them (e.g., getStorageProvider should accept (settings:
StorageSettings, context?) and return providerUploadImage;
isStorageConfiguredForUpload should accept (settings: StorageSettings) and
return a boolean). Keep getSettingsForUpload as-is, and maintain existing mocks
providerUploadImage, getToken, and convertToWebP but ensure the mocked
functions' parameter lists mirror the real signatures so arguments are passed
through exactly as in production.

In `@src/hooks/useMermaidGenerator.test.ts`:
- Around line 41-42: Update the English-only inline comments in
src/hooks/useMermaidGenerator.test.ts to include Japanese translations as
required by repo rules: for the comment "// Safety net to keep spies (e.g.
console.error if added in future tests) and any module-level setup from leaking
between tests on assertion failures." add a Japanese equivalent on the same line
or directly below it, and do the same for the other English-only block at lines
referenced (227-234); search for the exact English comment text to locate the
spots and append concise Japanese translations that convey the same meaning.

In `@src/hooks/useWorkflowDraft.test.ts`:
- Line 228: The inline comment "Flush the queued setTimeout(..., 0) that
triggers revokeObjectURL." is English-only; add a Japanese translation alongside
it so comments follow the repo JP/EN rule—for example, update the comment in
useWorkflowDraft.test.ts (the line containing that text) to include a Japanese
sentence conveying the same meaning immediately before or after the existing
English sentence.

In `@src/hooks/useWorkflowRunSession.test.ts`:
- Around line 286-287: Update the English-only inline comments in the test to be
bilingual (English + Japanese): wherever the test drives outcomes via
mockRunWorkflowExecution (e.g., the comment "First, drive a paused outcome so
the hook captures pausedState.") and the other similar comments in this file,
add a Japanese translation alongside the English text (or convert each to an
English/Japanese pair) to comply with the repository comment policy; ensure the
wording remains clear and matches the original intent for comments near
mockRunWorkflowExecution and the other mentioned test steps.

In `@src/lib/aiServiceServer.test.ts`:
- Around line 350-366: The test's ReadableStream never terminates after
pulledOnce flips, which can hang the test; update the stream's pull
implementation (the ReadableStream created in the "abortSignal が aborted なら
ABORTED エラーを onError へ送る" test that uses abortController and pulledOnce) to
close the stream after emitting the post-abort chunk by calling
controller.close() immediately after controller.enqueue(":\n") so the stream
ends cleanly.

In `@src/stores/aiChatStore.test.ts`:
- Around line 175-190: Update the inline comments in the test block surrounding
useAIChatStore.persist.rehydrate() and the subsequent assertions so they include
both Japanese and English explanations; specifically, make the Japanese-only
comments about volatile fields and the partialize behavior bilingual (e.g., next
to the comment starting with "ここでは契約として..." and the one beginning "Pin volatile
fields explicitly...") so they follow the project guideline; keep meaning
identical and place English translations inline or on the following line for the
comments that reference useAIChatStore.persist.rehydrate, partialize behavior,
and the assertion checks for state.isOpen, state.contextEnabled, and
state.selectedModel.

---

Outside diff comments:
In `@src/components/editor/PageEditor/useEditorAutoSave.ts`:
- Around line 85-93: The unmount auto-save IIFE currently calls
syncGraphFromContent(pending.content) even if saveAction() failed; change the
flow in useEditorAutoSave so syncGraphFromContent is only invoked when
saveAction resolves successfully—e.g., introduce a success flag or return early
after the catch in the async IIFE surrounding saveAction(), and reference
saveAction, syncGraphFromContent, and pending.content when updating the logic so
graph sync is skipped if the save failed.

In `@src/components/editor/TiptapEditor/useWikiLinkStatusSync.ts`:
- Around line 109-124: After awaiting checkExistence(titles, pageId) guard
against stale results by validating the run is still current before mutating the
editor: capture the current run identity (e.g., pageId/pageNoteId/content or the
computed pageScopeSignature/currentCount) into a local variable before the await
and then compare it to the latest values (or lastCheckedRef/current refs)
immediately after the await; if they differ, return early and do not call
collectWikiLinkUpdates or applyWikiLinkUpdates. Ensure the same check is
performed before calling applyWikiLinkUpdates(editor, updates) so outdated async
responses never mutate the editor or update lastCheckedRef.current.

---

Nitpick comments:
In `@server/api/src/__tests__/middleware/csrfOrigin.test.ts`:
- Around line 61-64: The test for safe-method bypass in the it.each block named
"%s does not require Origin/Referer" currently uses ["GET","OPTIONS"]; update
that array to include "HEAD" so the matrix becomes ["GET","HEAD","OPTIONS"] (the
test uses createApp().request("/api/state", { method }) and expects status 200);
ensure only the method list is changed so the test name and assertions remain
intact.

In `@server/api/src/__tests__/services/subscriptionService.test.ts`:
- Around line 41-104: Add an explicit per-test cache reset: introduce and export
a test-only clearUserTierCache() in the module that implements getUserTier (so
tests can call it), then call clearUserTierCache() in the test suite afterEach
hook (in the describe("getUserTier") block) to guarantee isolation;
alternatively, if you prefer not to add an export, call vi.resetModules() in
afterEach and re-import getUserTier in each test to ensure the module-level
cache is cleared between tests.

In `@server/api/src/lib/hocuspocusInvalidation.ts`:
- Around line 67-82: The clearTimeout(timeoutId) call is duplicated in the try
and catch around the fetch in hocuspocusInvalidation.ts; remove both duplicates
and place a single clearTimeout(timeoutId) inside a finally block after the
catch so the timer is always cleared regardless of success or error, keeping the
existing error handling (the catch that computes name and logs/handles the
error) and preserving controller.signal usage for the fetch.

In `@src/components/editor/PageEditor/useEditorAutoSave.ts`:
- Around line 63-81: The sync routine is duplicated; extract the logic into a
single shared async function (e.g., syncGraphFromContent) that accepts content
(and uses pageId, extractWikiLinksFromContent, syncWikiLinks,
extractTagsFromContent, getUniqueTagNames, and syncTags) and then call that
single function from both the unmount path and the normal save path so both code
paths reuse the same implementation and avoid contract drift.

In `@src/hooks/useGeneralSettings.test.ts`:
- Around line 77-246: Extract a small helper (e.g. renderReady) to replace
repeated renderHook + waitFor boilerplate: it should call renderHook(() =>
useGeneralSettings()), await waitFor(() =>
expect(result.current.isLoading).toBe(false)), and return the hook result (and
optionally accept a list of vi mock functions to clear after ready). Replace
each test's renderHook + waitFor sequence with a single call to renderReady(),
and remove duplicated mockClear calls where you pass mocks into the helper;
reference useGeneralSettings, renderHook, and waitFor when locating the repeated
code to change.

In `@src/hooks/useMermaidGenerator.test.ts`:
- Around line 142-143: The test "generate catches synchronous throws from
generateMermaidDiagram and sets error" currently uses
mockGenerateMermaidDiagram.mockRejectedValueOnce which simulates an async
rejection; change the mock to throw synchronously instead (e.g., use
mockImplementationOnce to immediately throw new Error("network")) so the test
actually exercises the synchronous-throw path of generateMermaidDiagram; update
the mockGenerateMermaidDiagram call in the same test (the it(...) block) to use
the synchronous-throw implementation, or alternatively rename the test to
reflect an async rejection if you prefer that behavior.

In `@src/hooks/useWorkflowDraft.test.ts`:
- Around line 205-231: The test title says it verifies a “sanitized filename”
but the assertions only check URL lifecycle and click behavior; update the test
to either (A) add an assertion that the created anchor's download attribute
matches the expected sanitized filename (targeting the exportJson flow in
useWorkflowDraft and using the same spies
createObjectURLSpy/clickSpy/revokeObjectURLSpy and setDraft to set the name), or
(B) more simply rename the test to reflect what it actually asserts (e.g.,
change the it description to something like "exportJson opens a download link
and revokes the URL") so the title and assertions match.

In `@src/lib/agentSlashCommands/agentSlashEditorText.test.ts`:
- Around line 51-53: Replace the invisible U+FFFC literal in the assertion with
an explicit escape or constant: update the expect(...toHaveBeenCalledWith(2, 7,
"\n", "") call in the agentSlashEditorText.test.ts test to use "\uFFFC" (or a
named constant like OBJECT_REPLACEMENT = "\uFFFC") instead of the invisible
character so the test assertion on textBetween/toHaveBeenCalledWith is explicit
and safe to edit in diffs.

In `@src/lib/agentSlashCommands/executeAgentSlashCommand.test.ts`:
- Around line 96-105: The test setup mixes mockReset() and mockClear(), which
risks leftover mock implementations leaking between tests; change the two calls
using mockClear()—specifically vi.mocked(buildAgentSlashPrompt).mockClear() and
vi.mocked(getAgentSlashClaudeOptions).mockClear()—to use mockReset() instead so
all mocks are fully reset (consistent with the other mocks like
runClaudeQueryToCompletion, getEditorSelectionText, insertSlashAgentMarkdownAt,
readSlashAgentInsertPosition, clearLastSlashAgentSelection, and
getLastSlashAgentSelection) and prevent future implementation leakage.

In `@src/lib/agentSlashCommands/insertSlashAgentMarkdown.test.ts`:
- Around line 20-31: The test helper makeChainSpy currently returns
insertContentAt and chain mocks but doesn't expose the run() spy, so tests can't
assert that the chain was executed; update makeChainSpy to have insertContentAt
return an object with a run mock (e.g., const run = vi.fn()) and expose that run
on the helper return value (return { chain, insertContentAt, run }); then update
tests that use makeChainSpy to assert run was called (e.g.,
expect(run).toHaveBeenCalled()) after invoking the code that should execute the
chain; reference the makeChainSpy, chain, insertContentAt, and run symbols when
making changes.

In `@src/lib/aiChatConversationTitle.test.ts`:
- Around line 5-31: The chain(...) helper should guard against being called with
zero arguments: detect if nodes.length === 0 and throw a clear Error (e.g.
"chain requires at least one node") instead of reading nodes[nodes.length -
1].id; update the leafId assignment in the chain function to only read
nodes[nodes.length - 1].id after that guard so empty calls fail with an explicit
message and not an opaque runtime error.

In `@src/lib/pageRepository/StorageAdapterPageRepository.test.ts`:
- Around line 257-279: The test currently only checks that adapter.getLinks was
never called for "tag" but misses verifying writes; update the test surrounding
repo.addLink("s","t","wiki") to also assert adapter.saveLinks was never invoked
with a linkType of "tag" (i.e., inspect/mock.calls of adapter.saveLinks and
ensure no call has the third argument equal to "tag") so the test fails if
implementation writes to the tag bucket even without reading it.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: be9c1b39-1584-4fa6-a26e-c96a7635b499

📥 Commits

Reviewing files that changed from the base of the PR and between b269e58 and 8124251.

⛔ Files ignored due to path filters (1)

• bun.lock is excluded by !**/*.lock

📒 Files selected for processing (108)

• .coderabbit.yaml
• .github/workflows/ci.yml
• .github/workflows/claude-code-review.yml
• AGENTS.md
• CLAUDE.md
• README.md
• db/migrations/001_add_notes_tables.sql
• db/migrations/002_add_page_snapshots.sql
• db/migrations/003_add_invitation_tokens.sql
• db/migrations/004_add_invitation_locale_tracking.sql
• db/migrations/005_add_onboarding_and_page_kind.sql
• package.json
• packages/shared/package.json
• packages/shared/src/index.ts
• packages/shared/src/tagCharacterClass.test.ts
• packages/shared/src/tagCharacterClass.ts
• packages/shared/tsconfig.json
• packages/shared/vitest.config.ts
• scripts/check-drizzle-migrations.mjs
• scripts/setup-develop-branch.sh
• server/api/drizzle/0018_add_onboarding_and_page_kind.sql
• server/api/drizzle/meta/_journal.json
• server/api/src/__tests__/lib/userDelete.test.ts
• server/api/src/__tests__/middleware/adminAuth.test.ts
• server/api/src/__tests__/middleware/csrfOrigin.test.ts
• server/api/src/__tests__/middleware/errorHandler.test.ts
• server/api/src/__tests__/routes/pages.test.ts
• server/api/src/__tests__/routes/syncPages.test.ts
• server/api/src/__tests__/services/magicLinkService.test.ts
• server/api/src/__tests__/services/subscriptionService.test.ts
• server/api/src/__tests__/services/titleRenamePropagationService.test.ts
• server/api/src/__tests__/services/ydocRenameRewrite.test.ts
• server/api/src/lib/extractPlainTextFromYXml.ts
• server/api/src/lib/hocuspocusInvalidation.ts
• server/api/src/routes/pageSnapshots.ts
• server/api/src/routes/pages.ts
• server/api/src/routes/syncPages.ts
• server/api/src/schema/index.ts
• server/api/src/schema/pages.ts
• server/api/src/services/titleRenamePropagationService.ts
• server/api/src/services/ydocRenameRewrite.ts
• src/components/editor/PageEditor/useEditorAutoSave.test.ts
• src/components/editor/PageEditor/useEditorAutoSave.ts
• src/components/editor/PageEditor/usePageEditorAutoSaveWithMutation.ts
• src/components/editor/TiptapEditor/EditorBubbleMenu.test.tsx
• src/components/editor/TiptapEditor/useBubbleMenuWikiLink.test.ts
• src/components/editor/TiptapEditor/useBubbleMenuWikiLink.ts
• src/components/editor/TiptapEditor/useEditorBubbleMenu.test.ts
• src/components/editor/TiptapEditor/useEditorLifecycle.ts
• src/components/editor/TiptapEditor/useTagStatusSync.test.tsx
• src/components/editor/TiptapEditor/useTagStatusSync.ts
• src/components/editor/TiptapEditor/useWikiLinkStatusSync.test.tsx
• src/components/editor/TiptapEditor/useWikiLinkStatusSync.ts
• src/components/editor/extensions/TagExtension.test.ts
• src/components/editor/extensions/TagExtension.ts
• src/components/editor/extensions/WikiLinkExtension.test.ts
• src/components/editor/extensions/WikiLinkExtension.ts
• src/components/editor/extensions/slashSuggestionPlugin.test.ts
• src/hooks/useAIChatPanelContentLogic.test.ts
• src/hooks/useAISettings.test.ts
• src/hooks/useGeneralSettings.test.ts
• src/hooks/useGlobalSearch.test.ts
• src/hooks/useGlobalSearch.ts
• src/hooks/useImageUpload.test.ts
• src/hooks/useMermaidGenerator.test.ts
• src/hooks/usePageQueries.ts
• src/hooks/useSyncWikiLinks.test.ts
• src/hooks/useWorkflowDraft.test.ts
• src/hooks/useWorkflowRunSession.test.ts
• src/lib/agentSlashCommands/agentSlashEditorText.test.ts
• src/lib/agentSlashCommands/buildAgentSlashPrompt.test.ts
• src/lib/agentSlashCommands/buildAgentSlashPromptParts.test.ts
• src/lib/agentSlashCommands/executeAgentSlashCommand.test.ts
• src/lib/agentSlashCommands/hook.test.ts
• src/lib/agentSlashCommands/insertPosition.test.ts
• src/lib/agentSlashCommands/insertSlashAgentMarkdown.test.ts
• src/lib/agentSlashCommands/slashAgentSelectionCache.test.ts
• src/lib/aiChatConversationTitle.test.ts
• src/lib/aiClient.test.ts
• src/lib/aiService.test.ts
• src/lib/aiServiceDirectProviders.test.ts
• src/lib/aiServiceModels.test.ts
• src/lib/aiServiceServer.test.ts
• src/lib/api/types.ts
• src/lib/encryption.test.ts
• src/lib/pageRepository.test.ts
• src/lib/pageRepository.ts
• src/lib/pageRepository/StorageAdapterPageRepository.test.ts
• src/lib/pageRepository/StorageAdapterPageRepository.ts
• src/lib/parseStdioArgs.test.ts
• src/lib/storage/index.test.ts
• src/lib/storageAdapter/IndexedDBStorageAdapter.ts
• src/lib/storageAdapter/StorageAdapter.ts
• src/lib/storageAdapter/types.ts
• src/lib/sync/syncWithApi.test.ts
• src/lib/sync/syncWithApi.ts
• src/lib/syncWikiLinks.ts
• src/lib/tagCharacterClassSync.test.ts
• src/lib/tagUtils.test.ts
• src/lib/tagUtils.ts
• src/lib/wikiLinkUtils.test.ts
• src/lib/wikiLinkUtils.ts
• src/pages/NoteSettings/index.tsx
• src/pages/SearchResults.tsx
• src/stores/aiChatStore.test.ts
• src/stores/pageStore.test.ts
• src/stores/pageStore.ts
• src/types/page.ts

💤 Files with no reviewable changes (6)

• db/migrations/004_add_invitation_locale_tracking.sql
• scripts/setup-develop-branch.sh
• db/migrations/003_add_invitation_tokens.sql
• db/migrations/001_add_notes_tables.sql
• db/migrations/005_add_onboarding_and_page_kind.sql
• db/migrations/002_add_page_snapshots.sql

[coderabbitai]

🧹 Nitpick comments (6)

server/api/src/__tests__/services/gemini.test.ts (1)

33-37: Add no-fetch assertion for empty API key guard

Line 33 verifies rejection, but not that the network layer is skipped. Adding that assertion hardens the guard-path contract.

Suggested patch

   it("throws when apiKey is empty", async () => {
     await expect(generateImageWithGemini("a prompt", "")).rejects.toThrow(
       /prompt and api key are required/i,
     );
+    expect(fetchSpy).not.toHaveBeenCalled();
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/__tests__/services/gemini.test.ts` around lines 33 - 37,
Update the test for generateImageWithGemini to not only expect rejection when
apiKey is empty but also assert that the network layer was not invoked; after
calling await expect(generateImageWithGemini("a prompt",
"")).rejects.toThrow(...), add an assertion that the mocked fetch (or the fetch
stub used in this test suite) was not called to ensure the implementation
short-circuits before any network request. Reference generateImageWithGemini and
the test's mocked fetch/HTTP client to locate where to add the "not called"
assertion.

server/api/src/__tests__/services/usageService.test.ts (1)

192-210: Assert the second chain performs onConflictDoUpdate

Lines 194-197 verify two inserts start, but don’t confirm the upsert behavior itself. Add an explicit onConflictDoUpdate assertion to lock in the monthly-usage update contract.

Suggested patch

     expect(chains.length).toBe(2);
     expect(chains[0]?.startMethod).toBe("insert");
     expect(chains[1]?.startMethod).toBe("insert");
+    const upsertCall = chains[1]?.ops.find((op) => op.method === "onConflictDoUpdate");
+    expect(upsertCall).toBeDefined();

     // 1 件目: values() に渡された生データ。

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/__tests__/services/usageService.test.ts` around lines 192 -
210, The test currently asserts two top-level chains but doesn't verify the
upsert operation; update the test to assert that the second chain (chains[1])
contains an op with method "onConflictDoUpdate" and that its args reflect the
expected conflict target and update payload (e.g., conflict keys like
userId/modelId/month and the monthly usage increment/update fields). Locate the
test block that inspects chains (uses chains[0] and chains[1]) and add a
find/assertion for chains[1]?.ops.find(op => op.method === "onConflictDoUpdate")
and validate its args to lock the monthly-usage update contract.

server/api/src/__tests__/routes/subscriptionManage.test.ts (1)

78-85: Tighten getEnv mock to prevent false-positive key usage

Line 83 returns "polar-token" for any env key, which can mask wrong-key regressions. Prefer a key-aware mock that throws on unexpected keys.

Suggested patch

 beforeEach(() => {
   mockSubscriptionsUpdate.mockReset();
   mockGetUserTier.mockReset();
   mockGetSubscription.mockReset();
   mockCheckUsage.mockReset();
-  mockGetEnv.mockReset().mockReturnValue("polar-token");
+  mockGetEnv.mockReset().mockImplementation((key: string) => {
+    if (key === "POLAR_ACCESS_TOKEN") return "polar-token";
+    throw new Error(`Unexpected env key in test: ${key}`);
+  });
   process.env = { ...ORIGINAL_ENV };
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/__tests__/routes/subscriptionManage.test.ts` around lines 78 -
85, The test's getEnv mock (mockGetEnv) currently returns "polar-token" for any
key which can hide wrong-key regressions; update the beforeEach to use a
key-aware mockImplementation on mockGetEnv that returns "polar-token" only for
the expected env key (e.g., "POLAR_TOKEN" or the exact key your code calls) and
throws an error for any other key, so unexpected env lookups fail the test;
locate mockGetEnv in the test setup and replace mockReturnValue("polar-token")
with a mockImplementation that checks the incoming key and returns the token
only for the correct key and throws otherwise.

server/api/src/__tests__/services/aiProviders.test.ts (1)

362-366: Assert no network side-effect on sync unknown-provider throw

On Line 362, adding a fetch non-invocation assertion makes the dispatcher failure contract stricter and clearer.

Suggested patch

   it("streamProvider throws synchronously for unknown providers", () => {
     // streamProvider は同期的に switch で投げるので、generator を返す前に throw する。
     // streamProvider throws synchronously from the switch before returning a generator.
     expect(() => streamProvider("zzz" as never, "k", "m", messages)).toThrow(/unknown provider/i);
+    expect(fetchSpy).not.toHaveBeenCalled();
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/__tests__/services/aiProviders.test.ts` around lines 362 -
366, The test for streamProvider's synchronous throw should also assert that no
network calls were attempted; in the "it('streamProvider throws synchronously
for unknown providers')" test, after invoking expect(() =>
streamProvider(...)).toThrow(...), add an assertion that the global/network
fetch mock (or the fetch spy used in the test suite) was not called to ensure no
side-effecting network activity occurred when streamProvider throws
synchronously for an unknown provider.

server/api/src/__tests__/routes/activity.test.ts (1)

142-159: Align invalid-date tests with other validator short-circuit assertions

Lines 142 and 150 assert 400, but unlike invalid kind/actor they don’t verify the service isn’t called. Adding that check keeps validation-path contracts consistent.

Suggested patch

   it("rejects invalid 'from' date with 400", async () => {
     const { app } = createTestApp([]);

     const res = await app.request("/api/activity?from=not-a-date", { headers: authHeaders() });

     expect(res.status).toBe(400);
+    expect(mockListActivity).not.toHaveBeenCalled();
   });

   it("rejects inverted from > to range with 400", async () => {
     const { app } = createTestApp([]);

     const res = await app.request(
       "/api/activity?from=2026-04-10T00:00:00Z&to=2026-04-01T00:00:00Z",
       { headers: authHeaders() },
     );

     expect(res.status).toBe(400);
+    expect(mockListActivity).not.toHaveBeenCalled();
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/__tests__/routes/activity.test.ts` around lines 142 - 159, The
two tests ("rejects invalid 'from' date with 400" and "rejects inverted from >
to range with 400") assert status 400 but don't assert the activity service
wasn't invoked; update each test to pass in or obtain the mocked
activity-fetching function created by createTestApp (the same mock used
elsewhere in this file) and add an expectation that this mock was not called
(e.g., expect(activityMock).not.toHaveBeenCalled()) after the app.request call,
keeping authHeaders() and the existing status assertions intact.

server/api/src/__tests__/routes/thumbnail/commit.test.ts (1)

88-98: Assert service non-invocation for whitespace sourceUrl too

Line 88 only checks the 400 status. Add a non-call assertion to lock in the validation short-circuit contract.

Suggested patch

   it("returns 400 when sourceUrl is whitespace only", async () => {
     const app = createTestApp();

     const res = await app.request("/api/thumbnail/commit", {
       method: "POST",
       headers: authHeaders(),
       body: JSON.stringify({ sourceUrl: "   " }),
     });

     expect(res.status).toBe(400);
+    expect(mockCommitImage).not.toHaveBeenCalled();
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/api/src/__tests__/routes/thumbnail/commit.test.ts` around lines 88 -
98, The test currently only asserts a 400 response for whitespace-only sourceUrl
but must also assert that the thumbnail commit handler/service was not invoked;
update the test in commit.test.ts to create or access the mocked thumbnail
commit function used by createTestApp (the service that handles
"/api/thumbnail/commit") and add an assertion like
expect(mockCommitFn).not.toHaveBeenCalled() after the request to lock in the
short-circuit validation behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@server/api/src/__tests__/routes/activity.test.ts`:
- Around line 142-159: The two tests ("rejects invalid 'from' date with 400" and
"rejects inverted from > to range with 400") assert status 400 but don't assert
the activity service wasn't invoked; update each test to pass in or obtain the
mocked activity-fetching function created by createTestApp (the same mock used
elsewhere in this file) and add an expectation that this mock was not called
(e.g., expect(activityMock).not.toHaveBeenCalled()) after the app.request call,
keeping authHeaders() and the existing status assertions intact.

In `@server/api/src/__tests__/routes/subscriptionManage.test.ts`:
- Around line 78-85: The test's getEnv mock (mockGetEnv) currently returns
"polar-token" for any key which can hide wrong-key regressions; update the
beforeEach to use a key-aware mockImplementation on mockGetEnv that returns
"polar-token" only for the expected env key (e.g., "POLAR_TOKEN" or the exact
key your code calls) and throws an error for any other key, so unexpected env
lookups fail the test; locate mockGetEnv in the test setup and replace
mockReturnValue("polar-token") with a mockImplementation that checks the
incoming key and returns the token only for the correct key and throws
otherwise.

In `@server/api/src/__tests__/routes/thumbnail/commit.test.ts`:
- Around line 88-98: The test currently only asserts a 400 response for
whitespace-only sourceUrl but must also assert that the thumbnail commit
handler/service was not invoked; update the test in commit.test.ts to create or
access the mocked thumbnail commit function used by createTestApp (the service
that handles "/api/thumbnail/commit") and add an assertion like
expect(mockCommitFn).not.toHaveBeenCalled() after the request to lock in the
short-circuit validation behavior.

In `@server/api/src/__tests__/services/aiProviders.test.ts`:
- Around line 362-366: The test for streamProvider's synchronous throw should
also assert that no network calls were attempted; in the "it('streamProvider
throws synchronously for unknown providers')" test, after invoking expect(() =>
streamProvider(...)).toThrow(...), add an assertion that the global/network
fetch mock (or the fetch spy used in the test suite) was not called to ensure no
side-effecting network activity occurred when streamProvider throws
synchronously for an unknown provider.

In `@server/api/src/__tests__/services/gemini.test.ts`:
- Around line 33-37: Update the test for generateImageWithGemini to not only
expect rejection when apiKey is empty but also assert that the network layer was
not invoked; after calling await expect(generateImageWithGemini("a prompt",
"")).rejects.toThrow(...), add an assertion that the mocked fetch (or the fetch
stub used in this test suite) was not called to ensure the implementation
short-circuits before any network request. Reference generateImageWithGemini and
the test's mocked fetch/HTTP client to locate where to add the "not called"
assertion.

In `@server/api/src/__tests__/services/usageService.test.ts`:
- Around line 192-210: The test currently asserts two top-level chains but
doesn't verify the upsert operation; update the test to assert that the second
chain (chains[1]) contains an op with method "onConflictDoUpdate" and that its
args reflect the expected conflict target and update payload (e.g., conflict
keys like userId/modelId/month and the monthly usage increment/update fields).
Locate the test block that inspects chains (uses chains[0] and chains[1]) and
add a find/assertion for chains[1]?.ops.find(op => op.method ===
"onConflictDoUpdate") and validate its args to lock the monthly-usage update
contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 32666c37-be32-4fb3-a85a-6c10526d8921

📥 Commits

Reviewing files that changed from the base of the PR and between 8124251 and 9f723ff.

📒 Files selected for processing (17)

• server/api/src/__tests__/routes/activity.test.ts
• server/api/src/__tests__/routes/ai/chat.test.ts
• server/api/src/__tests__/routes/checkout.test.ts
• server/api/src/__tests__/routes/health.test.ts
• server/api/src/__tests__/routes/lint.test.ts
• server/api/src/__tests__/routes/onboarding.test.ts
• server/api/src/__tests__/routes/subscriptionManage.test.ts
• server/api/src/__tests__/routes/thumbnail/commit.test.ts
• server/api/src/__tests__/routes/thumbnail/imageGenerate.test.ts
• server/api/src/__tests__/routes/thumbnail/imageSearch.test.ts
• server/api/src/__tests__/services/aiProviders.test.ts
• server/api/src/__tests__/services/gemini.test.ts
• server/api/src/__tests__/services/imageSearch.test.ts
• server/api/src/__tests__/services/usageService.test.ts
• server/api/src/services/lintEngine/rules/brokenLink.test.ts
• server/api/src/services/lintEngine/rules/ghostMany.test.ts
• server/api/src/services/lintEngine/rules/orphan.test.ts

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (5)

src/hooks/useWorkflowDraft.test.ts (1)

205-233: Align the “sanitized filename” claim with an actual filename assertion.

The test name says sanitized filename, but it currently never inspects anchor.download. Add a direct assertion (or rename the test) so this behavior is truly covered.

Proposed update

 it("exportJson opens a download link with sanitized filename", () => {
@@
-  const clickSpy = vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation(() => {});
+  let downloadName = "";
+  const clickSpy = vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation(function (
+    this: HTMLAnchorElement,
+  ) {
+    downloadName = this.download;
+  });
@@
   expect(createObjectURLSpy).toHaveBeenCalledTimes(1);
   expect(clickSpy).toHaveBeenCalledTimes(1);
+  expect(downloadName).toMatch(/\.json$/);
+  expect(downloadName).not.toContain(" ");
@@
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useWorkflowDraft.test.ts` around lines 205 - 233, The test title
promises to verify a "sanitized filename" but never inspects the created
anchor's download attribute; update the test that calls
useWorkflowDraft().exportJson to locate the created anchor (e.g.,
document.querySelector('a[download]') or the created anchor instance) and add an
assertion that its download equals the expected sanitized filename derived from
the draft name set via result.current.setDraft (or compute expected using the
same sanitize logic used by exportJson); keep the existing
URL.createObjectURL/click/revoke assertions and run vi.runAllTimers() as before
to flush revokeObjectURL.

server/hocuspocus/src/extractPlainTextFromYXml.test.ts (1)

3-3: Prefer using CONTENT_PREVIEW_MAX_LENGTH in the assertion.

Using the exported constant instead of literal 120 keeps the test aligned automatically if the limit changes.

♻️ Suggested diff

-import { buildContentPreview, extractTextFromYXml } from "./extractPlainTextFromYXml.js";
+import {
+  buildContentPreview,
+  CONTENT_PREVIEW_MAX_LENGTH,
+  extractTextFromYXml,
+} from "./extractPlainTextFromYXml.js";
...
-    expect(prev.length).toBeLessThanOrEqual(120);
+    expect(prev.length).toBeLessThanOrEqual(CONTENT_PREVIEW_MAX_LENGTH);

Also applies to: 143-143

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/hocuspocus/src/extractPlainTextFromYXml.test.ts` at line 3, Replace
hard-coded literal 120 in the test assertions with the exported constant
CONTENT_PREVIEW_MAX_LENGTH so the test stays correct when the limit changes;
update the import line to include CONTENT_PREVIEW_MAX_LENGTH (alongside
buildContentPreview and extractTextFromYXml) and change the two assertions that
compare against 120 to use CONTENT_PREVIEW_MAX_LENGTH instead (references:
buildContentPreview, extractTextFromYXml, CONTENT_PREVIEW_MAX_LENGTH).

src/components/editor/extensions/slashSuggestionPlugin.test.ts (2)

82-93: Strengthen callback assertions for transition tests

toHaveBeenCalledWith can still pass when extra unexpected notifications happen. For state-transition tests, also assert call count and/or last call payload.

Suggested pattern

-    expect(onStateChange).toHaveBeenCalledWith(pluginState);
+    expect(onStateChange).toHaveBeenCalledTimes(1);
+    expect(onStateChange).toHaveBeenLastCalledWith(pluginState);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/editor/extensions/slashSuggestionPlugin.test.ts` around lines
82 - 93, The test currently verifies the plugin callback with
expect(onStateChange).toHaveBeenCalledWith(pluginState) which can hide extra
notifications; update the assertions around the onStateChange mock (created as
onStateChange and used when calling getPlugin and reading pluginState via
slashSuggestionPluginKey.getState) to also assert call count and/or last call
payload (e.g., replace the single toHaveBeenCalledWith with
expect(onStateChange).toHaveBeenCalledTimes(1) and
expect(onStateChange).toHaveBeenLastCalledWith(pluginState) so the transition
test ensures exactly one notification and the expected payload).

---

40-48: Guard plugin extraction with an explicit empty-array check

return plugins[0] assumes plugin creation always succeeds. A small guard here gives much clearer failures if extension wiring changes later.

Suggested patch

 function getPlugin(onStateChange?: (s: SlashSuggestionState) => void): Plugin {
   const extension = SlashSuggestionPlugin.configure({ onStateChange });
   const addPlugins = extension.config.addProseMirrorPlugins;
   if (!addPlugins) throw new Error("addProseMirrorPlugins missing");
   // Tiptap stores extension options on `this.options` inside the method.
   // 拡張内部の `this.options` 経由で onStateChange を渡す必要がある。
   const plugins = addPlugins.call({ options: { onStateChange } } as never);
-  return plugins[0];
+  if (plugins.length === 0) {
+    throw new Error("SlashSuggestionPlugin produced no ProseMirror plugins");
+  }
+  return plugins[0];
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/editor/extensions/slashSuggestionPlugin.test.ts` around lines
40 - 48, The test helper getPlugin assumes addProseMirrorPlugins returns a
non-empty array and uses plugins[0] directly; add an explicit check after
calling addProseMirrorPlugins (from SlashSuggestionPlugin.configure) to verify
plugins is an array with length > 0 and throw a descriptive Error if empty or
not an array (e.g., "addProseMirrorPlugins returned no plugins for
SlashSuggestionPlugin"), so failures surface clearly if the extension wiring
changes.

src/hooks/useWorkflowRunSession.test.ts (1)

90-93: Reduce repeated mock setup with one shared beforeEach.

The same setup block is repeated across four describe blocks. Consider promoting it to a file-level beforeEach to reduce drift and maintenance overhead.

♻️ Proposed refactor

+beforeEach(() => {
+  vi.clearAllMocks();
+  mockIsTauriDesktop.mockReturnValue(true);
+});
+
 describe("useWorkflowRunSession - validation guards", () => {
-  beforeEach(() => {
-    vi.clearAllMocks();
-    mockIsTauriDesktop.mockReturnValue(true);
-  });
+  // per-suite overrides only if needed
   ...
 });
 
 describe("useWorkflowRunSession - run outcomes", () => {
-  beforeEach(() => {
-    vi.clearAllMocks();
-    mockIsTauriDesktop.mockReturnValue(true);
-  });
+  // per-suite overrides only if needed
   ...
 });

Also applies to: 189-192, 280-283, 370-373

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useWorkflowRunSession.test.ts` around lines 90 - 93, The repeated
per-describe beforeEach blocks currently calling vi.clearAllMocks() and setting
mockIsTauriDesktop.mockReturnValue(true) should be consolidated into a single
file-level beforeEach at the top of the test file so every describe inherits the
same setup; add one top-level beforeEach that calls vi.clearAllMocks() and
mockIsTauriDesktop.mockReturnValue(true), and remove the duplicate beforeEach
blocks inside the individual describe blocks to avoid drift and maintenance
overhead (look for the existing vi.clearAllMocks and
mockIsTauriDesktop.mockReturnValue usages to locate the duplicated setup).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/claude-sidecar/src/handlers/query.test.ts`:
- Around line 591-615: Add a regression that simulates a tool-use-start before
abort and assert that runQuery finalizes activeToolName by emitting a
corresponding tool-use-complete (or equivalent finalization) even when the abort
signal fires or an exception occurs: update the existing test that aborts before
results to instead start a tool (emit a tool-use-start through the mocked query
iterable), trigger ac.abort(), then call runQuery and expect both the aborted
error and a tool-use-complete event (or that activeToolName is cleared) in the
parsed output; also change runQuery's abort/exception handling paths to check
for an existing activeToolName and emit or finalize tool-use-complete (and clear
activeToolName) before returning so consumers won’t be left in a stuck tool
state.

In `@server/mcp/src/__tests__/tools/index.test.ts`:
- Around line 19-20: The JSDoc helper comments in the test file use a malformed
terminator sequence ("... /") which can be misread; update the block comments
for the ZediClient helper (the /** Build a fully-mocked ZediClient... */ comment
and the analogous comment later in the file) to use standard multiline JSDoc
termination ("*/") so each block is a proper /** ... */ JSDoc comment,
preserving the existing English and Japanese text and spacing.

In `@src/hooks/useMermaidGenerator.test.ts`:
- Around line 143-145: The test "generate catches synchronous throws from
generateMermaidDiagram and sets error" is using mockRejectedValueOnce (async
rejection) but should simulate a synchronous throw; change the mock for
mockGenerateMermaidDiagram to a synchronous throw (e.g.
mockImplementationOnce(() => { throw new Error("network") })) so the hook's
synchronous-throw path is exercised when the code calls generateMermaidDiagram;
keep the rest of the test assertions the same to verify error state is set.
- Around line 49-51: The tests currently call vi.clearAllMocks() in the
beforeEach in useMermaidGenerator.test but that doesn't reset mock
implementations; change the setup to reset implementations as well by replacing
or augmenting the beforeEach to call vi.resetAllMocks() (or vi.clearAllMocks();
vi.resetAllMocks()) so mocks created/configured for functions used by
useMermaidGenerator tests have their mockImplementation/mockResolvedValue
cleared between tests.

---

Nitpick comments:
In `@server/hocuspocus/src/extractPlainTextFromYXml.test.ts`:
- Line 3: Replace hard-coded literal 120 in the test assertions with the
exported constant CONTENT_PREVIEW_MAX_LENGTH so the test stays correct when the
limit changes; update the import line to include CONTENT_PREVIEW_MAX_LENGTH
(alongside buildContentPreview and extractTextFromYXml) and change the two
assertions that compare against 120 to use CONTENT_PREVIEW_MAX_LENGTH instead
(references: buildContentPreview, extractTextFromYXml,
CONTENT_PREVIEW_MAX_LENGTH).

In `@src/components/editor/extensions/slashSuggestionPlugin.test.ts`:
- Around line 82-93: The test currently verifies the plugin callback with
expect(onStateChange).toHaveBeenCalledWith(pluginState) which can hide extra
notifications; update the assertions around the onStateChange mock (created as
onStateChange and used when calling getPlugin and reading pluginState via
slashSuggestionPluginKey.getState) to also assert call count and/or last call
payload (e.g., replace the single toHaveBeenCalledWith with
expect(onStateChange).toHaveBeenCalledTimes(1) and
expect(onStateChange).toHaveBeenLastCalledWith(pluginState) so the transition
test ensures exactly one notification and the expected payload).
- Around line 40-48: The test helper getPlugin assumes addProseMirrorPlugins
returns a non-empty array and uses plugins[0] directly; add an explicit check
after calling addProseMirrorPlugins (from SlashSuggestionPlugin.configure) to
verify plugins is an array with length > 0 and throw a descriptive Error if
empty or not an array (e.g., "addProseMirrorPlugins returned no plugins for
SlashSuggestionPlugin"), so failures surface clearly if the extension wiring
changes.

In `@src/hooks/useWorkflowDraft.test.ts`:
- Around line 205-233: The test title promises to verify a "sanitized filename"
but never inspects the created anchor's download attribute; update the test that
calls useWorkflowDraft().exportJson to locate the created anchor (e.g.,
document.querySelector('a[download]') or the created anchor instance) and add an
assertion that its download equals the expected sanitized filename derived from
the draft name set via result.current.setDraft (or compute expected using the
same sanitize logic used by exportJson); keep the existing
URL.createObjectURL/click/revoke assertions and run vi.runAllTimers() as before
to flush revokeObjectURL.

In `@src/hooks/useWorkflowRunSession.test.ts`:
- Around line 90-93: The repeated per-describe beforeEach blocks currently
calling vi.clearAllMocks() and setting mockIsTauriDesktop.mockReturnValue(true)
should be consolidated into a single file-level beforeEach at the top of the
test file so every describe inherits the same setup; add one top-level
beforeEach that calls vi.clearAllMocks() and
mockIsTauriDesktop.mockReturnValue(true), and remove the duplicate beforeEach
blocks inside the individual describe blocks to avoid drift and maintenance
overhead (look for the existing vi.clearAllMocks and
mockIsTauriDesktop.mockReturnValue usages to locate the duplicated setup).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 32b43d1f-06a8-4f8a-963a-5e0646aafe08

📥 Commits

Reviewing files that changed from the base of the PR and between 9f723ff and 68e7de5.

📒 Files selected for processing (22)

• AGENTS.md
• packages/claude-sidecar/src/handlers/installation.test.ts
• packages/claude-sidecar/src/handlers/installation.ts
• packages/claude-sidecar/src/handlers/models.test.ts
• packages/claude-sidecar/src/handlers/query.test.ts
• packages/claude-sidecar/src/handlers/query.ts
• packages/claude-sidecar/src/handlers/status.test.ts
• server/api/src/lib/extractPlainTextFromYXml.ts
• server/hocuspocus/src/extractPlainTextFromYXml.test.ts
• server/hocuspocus/src/extractPlainTextFromYXml.ts
• server/mcp/src/__tests__/tools/index.test.ts
• src/components/editor/TiptapEditor/useWikiLinkStatusSync.test.tsx
• src/components/editor/extensions/TagExtension.test.ts
• src/components/editor/extensions/TagExtension.ts
• src/components/editor/extensions/WikiLinkExtension.test.ts
• src/components/editor/extensions/WikiLinkExtension.ts
• src/components/editor/extensions/slashSuggestionPlugin.test.ts
• src/hooks/useMermaidGenerator.test.ts
• src/hooks/useWorkflowDraft.test.ts
• src/hooks/useWorkflowRunSession.test.ts
• src/lib/aiServiceServer.test.ts
• src/stores/aiChatStore.test.ts

✅ Files skipped from review due to trivial changes (1)

• AGENTS.md

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Abort coverage is missing the active-tool cleanup case.

If cancellation happens after a tool-use-start, runQuery() currently breaks out of the loop and emits only the aborted error. No matching tool-use-complete is sent, so consumers that derive tool state from the event stream can stay stuck. Please add a regression here and finalize activeToolName in the abort/exception paths.

🧩 Possible fix in packages/claude-sidecar/src/handlers/query.ts

-    if (abortController.signal.aborted) {
-      emit({
-        type: "error",
-        id,
-        error: "Query aborted",
-        code: "aborted",
-      });
-      return;
-    }
+    if (abortController.signal.aborted) {
+      if (activeToolName) {
+        emit({ type: "tool-use-complete", id, toolName: activeToolName });
+        activeToolName = null;
+      }
+      emit({
+        type: "error",
+        id,
+        error: "Query aborted",
+        code: "aborted",
+      });
+      return;
+    }
 
     emit({
       type: "stream-complete",
       id,
       result: { content: aggregated },
     });
   } catch (err) {
+    if (activeToolName) {
+      emit({ type: "tool-use-complete", id, toolName: activeToolName });
+      activeToolName = null;
+    }
     const message = err instanceof Error ? err.message : String(err);
     emit({
       type: "error",
       id,
       error: message,

As per coding guidelines: Ensure error handling and logging are appropriate and comprehensive.

📝 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

	it("emits an aborted error when the abort signal is fired before the result arrives", async () => {
	const ac = new AbortController();
	// Trigger abort before iteration begins so the loop sees it on the first message.
	// 反復開始前に abort して、最初のメッセージでループを抜けるようにする。
	ac.abort();
	queryMock.mockReturnValue(
	makeQueryIterable([
	partial({
	type: "content_block_delta",
	delta: { type: "text_delta", text: "x" },
	}) as unknown as SDKMessage,
	]),
	);
	await runQuery({
	id: "q1",
	prompt: "p",
	writeLine: (l) => writes.push(l),
	abortController: ac,
	tracker,
	});
	expect(parsed()).toEqual([
	{ type: "error", id: "q1", error: "Query aborted", code: "aborted" },
	]);
	if (abortController.signal.aborted) {
	if (activeToolName) {
	emit({ type: "tool-use-complete", id, toolName: activeToolName });
	activeToolName = null;
	}
	emit({
	type: "error",
	id,
	error: "Query aborted",
	code: "aborted",
	});
	return;
	}
	emit({
	type: "stream-complete",
	id,
	result: { content: aggregated },
	});
	} catch (err) {
	if (activeToolName) {
	emit({ type: "tool-use-complete", id, toolName: activeToolName });
	activeToolName = null;
	}
	const message = err instanceof Error ? err.message : String(err);
	emit({
	type: "error",
	id,
	error: message,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/claude-sidecar/src/handlers/query.test.ts` around lines 591 - 615,
Add a regression that simulates a tool-use-start before abort and assert that
runQuery finalizes activeToolName by emitting a corresponding tool-use-complete
(or equivalent finalization) even when the abort signal fires or an exception
occurs: update the existing test that aborts before results to instead start a
tool (emit a tool-use-start through the mocked query iterable), trigger
ac.abort(), then call runQuery and expect both the aborted error and a
tool-use-complete event (or that activeToolName is cleared) in the parsed
output; also change runQuery's abort/exception handling paths to check for an
existing activeToolName and emit or finalize tool-use-complete (and clear
activeToolName) before returning so consumers won’t be left in a stuck tool
state.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Normalize JSDoc block formatting in helper comments.

Line [19] and Line [47] use ... / inside /** ... */, which is easy to misread as a broken terminator. Please switch to standard multiline JSDoc formatting.

💡 Suggested cleanup

-/** Build a fully-mocked ZediClient. Tools call only the methods we register, so types are safe. /
- *  全メソッドをモック化した ZediClient。 */
+/**
+ * Build a fully-mocked ZediClient. Tools call only the methods we register, so types are safe.
+ * 全メソッドをモック化した ZediClient。
+ */
 function createMockClient(): ZediClient {
@@
-/** Captures registerTool calls so we can assert which tools were registered. / 登録呼び出しを記録するスタブ */
+/**
+ * Captures registerTool calls so we can assert which tools were registered.
+ * 登録呼び出しを記録するスタブ。
+ */
 function createServerStub(): {

Also applies to: 47-47

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/mcp/src/__tests__/tools/index.test.ts` around lines 19 - 20, The JSDoc
helper comments in the test file use a malformed terminator sequence ("... /")
which can be misread; update the block comments for the ZediClient helper (the
/** Build a fully-mocked ZediClient... */ comment and the analogous comment
later in the file) to use standard multiline JSDoc termination ("*/") so each
block is a proper /** ... */ JSDoc comment, preserving the existing English and
Japanese text and spacing.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Reset implementations too, not just call history.

vi.clearAllMocks() does not reset mockImplementation / mockResolvedValue, so behavior can leak between tests as the suite grows. Prefer resetting these shared mocks in beforeEach.

Proposed diff

 beforeEach(() => {
-  vi.clearAllMocks();
+  mockGenerateMermaidDiagram.mockReset();
+  mockGetAISettingsOrThrow.mockReset();
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useMermaidGenerator.test.ts` around lines 49 - 51, The tests
currently call vi.clearAllMocks() in the beforeEach in useMermaidGenerator.test
but that doesn't reset mock implementations; change the setup to reset
implementations as well by replacing or augmenting the beforeEach to call
vi.resetAllMocks() (or vi.clearAllMocks(); vi.resetAllMocks()) so mocks
created/configured for functions used by useMermaidGenerator tests have their
mockImplementation/mockResolvedValue cleared between tests.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

This test name and setup are misaligned.

The case says “synchronous throws” but mockRejectedValueOnce exercises async rejection. Either rename the test or switch to a true sync throw to keep test intent precise.

Proposed diff (true sync throw path)

-it("generate catches synchronous throws from generateMermaidDiagram and sets error", async () => {
-  mockGenerateMermaidDiagram.mockRejectedValueOnce(new Error("network"));
+it("generate catches synchronous throws from generateMermaidDiagram and sets error", async () => {
+  mockGenerateMermaidDiagram.mockImplementationOnce(() => {
+    throw new Error("network");
+  });

As per coding guidelines, "Tests serve as a source of truth for specifications alongside implementation code TSDoc/JSDoc."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useMermaidGenerator.test.ts` around lines 143 - 145, The test
"generate catches synchronous throws from generateMermaidDiagram and sets error"
is using mockRejectedValueOnce (async rejection) but should simulate a
synchronous throw; change the mock for mockGenerateMermaidDiagram to a
synchronous throw (e.g. mockImplementationOnce(() => { throw new
Error("network") })) so the hook's synchronous-throw path is exercised when the
code calls generateMermaidDiagram; keep the rest of the test assertions the same
to verify error state is set.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (5)

admin/src/pages/users/useConfirmDialogs.test.ts (1)

157-204: Add a stale-reject race test to complete the concurrency contract.

You already cover stale resolve; adding the symmetric stale reject case would harden the catch-guard path and improve mutation resistance.

Suggested test addition

+  it("古い request の reject も新しい target を上書きしない / stale reject is ignored", async () => {
+    let rejectA: ((e?: unknown) => void) | null = null;
+    let resolveB: ((v: UserImpact) => void) | null = null;
+    vi.mocked(getUserImpact)
+      .mockImplementationOnce(
+        () =>
+          new Promise<UserImpact>((_resolve, reject) => {
+            rejectA = reject;
+          }),
+      )
+      .mockImplementationOnce(
+        () =>
+          new Promise<UserImpact>((resolve) => {
+            resolveB = resolve;
+          }),
+      );
+
+    const { result } = makeHook();
+    act(() => result.current.requestDelete(userA));
+    act(() => result.current.requestDelete(userB));
+
+    await act(async () => {
+      rejectA?.(new Error("stale"));
+      await Promise.resolve();
+    });
+    expect(result.current.deleteTarget?.user.id).toBe(userB.id);
+    expect(result.current.deleteTarget?.loadingImpact).toBe(true);
+
+    await act(async () => {
+      resolveB?.(sampleImpact);
+      await Promise.resolve();
+    });
+    expect(result.current.deleteTarget?.impact).toEqual(sampleImpact);
+    expect(result.current.deleteTarget?.loadingImpact).toBe(false);
+  });

As per coding guidelines "**/*.test.{ts,tsx}: Prioritize Mutation testing score as the primary quality metric; target 80%+ code coverage as a supporting goal".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@admin/src/pages/users/useConfirmDialogs.test.ts` around lines 157 - 204, Add
a symmetric "stale reject is ignored" concurrency test that mirrors the existing
stale-resolve test: mock getUserImpact with two sequential
mockImplementationOnce calls that each return a new Promise and capture their
reject callbacks (similar to resolveA/resolveB), call
result.current.requestDelete(userA) then requestDelete(userB) using makeHook and
act, then invoke the first promise's reject (rejectA) and assert the hook's
state (result.current.deleteTarget, .impact, .loadingImpact) remains pointing to
userB and unaffected by the stale rejection, finally rejectB and assert B's
rejection path is handled as expected; ensure you use vi.mocked(getUserImpact),
the same result.current.requestDelete flow, and act/await Promise.resolve() to
flush microtasks so the catch-guard path is exercised.

packages/ui/src/hooks/use-toast.test.ts (1)

24-24: Make this test title bilingual to match the rest of the file.

Adding the English counterpart here would keep spec style consistent across the suite.

As per coding guidelines **/*.{ts,tsx,js,jsx}: コメントやドキュメントは、原則として日本語と英語の両方を併記する。

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/hooks/use-toast.test.ts` at line 24, Update the test title
string in the it(...) call currently reading "ADD_TOAST
は配列の先頭に追加し、TOAST_LIMIT(=1) で切り詰める" to include an English translation so it is
bilingual (e.g., append " / adds toast to front of array and trims to
TOAST_LIMIT(=1)"). Modify the test declaration in use-toast.test.ts where the
it(...) for ADD_TOAST is defined to include both Japanese and English text while
preserving the original meaning and punctuation.

packages/ui/src/components/sidebar/SidebarProvider.test.tsx (2)

17-22: Consider extracting clearCookies into a shared test utility.

The same cookie-clear helper appears in multiple sidebar test files; centralizing it would reduce drift and simplify maintenance.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/components/sidebar/SidebarProvider.test.tsx` around lines 17
- 22, Extract the clearCookies helper into a shared test utility and import it
from tests/utils (or your existing test helpers); move the current clearCookies
function (the loop that parses document.cookie and sets each cookie to
max-age=0) into a named export e.g., clearCookies in a new/shared module, update
SidebarProvider.test.tsx to import { clearCookies } from that utility, and
remove the duplicate local function from other sidebar tests to reference the
shared export so all tests use the same implementation.

---

74-74: Keep test spec strings bilingual for consistency.

These two test titles are Japanese-only while the suite is otherwise bilingual; adding English counterparts keeps spec readability uniform.

As per coding guidelines **/*.{ts,tsx,js,jsx}: コメントやドキュメントは、原則として日本語と英語の両方を併記する。

Also applies to: 96-96

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/components/sidebar/SidebarProvider.test.tsx` at line 74,
Update the two test spec strings to be bilingual: modify the it(...) titles that
currently read "toggleSidebar で開閉が切り替わり cookie に永続化される" and the other
Japanese-only title at the second occurrence to include an English translation
after the Japanese (e.g. "toggleSidebar で開閉が切り替わり cookie に永続化される / toggles
sidebar and persists state to cookie"); ensure both it(...) calls in
SidebarProvider.test.tsx contain Japanese and English per the repo guideline.

admin/src/pages/ai-models/useAiModelsDragReorder.test.ts (1)

38-44: Reduce as never casting by typing test doubles to hook signatures.

Line 86 (and repeated call sites) force-casts strongly typed hook props to never, which hides contract drift in tests. Prefer typed mocks aligned to React.Dispatch and load signatures so test compile-time checks stay meaningful.
As per coding guidelines: **/*.{ts,tsx}: Use TypeScript strict mode; ban any type and require explicit type annotations.

Proposed refactor (typed args, no as never)

 interface Args {
   models: AiModelAdmin[];
-  setModels: ReturnType<typeof vi.fn>;
-  setError: ReturnType<typeof vi.fn>;
+  setModels: React.Dispatch<React.SetStateAction<AiModelAdmin[]>>;
+  setError: React.Dispatch<React.SetStateAction<string | null>>;
   isMountedRef: { current: boolean };
-  load: LoadFn & ReturnType<typeof vi.fn>;
+  load: LoadFn;
 }
 
 function createArgs(models: AiModelAdmin[]): Args {
-  const load = vi.fn().mockResolvedValue(undefined) as ReturnType<typeof vi.fn> & LoadFn;
+  const load: LoadFn = vi.fn().mockResolvedValue(undefined);
   return {
     models,
-    setModels: vi.fn(),
-    setError: vi.fn(),
+    setModels: vi.fn<React.Dispatch<React.SetStateAction<AiModelAdmin[]>>>(),
+    setError: vi.fn<React.Dispatch<React.SetStateAction<string | null>>>(),
     isMountedRef: { current: true },
     load,
   };
 }

Also applies to: 86-90, 119-124, 148-153, 177-182, 200-205, 223-228, 245-250

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@admin/src/pages/ai-models/useAiModelsDragReorder.test.ts` around lines 38 -
44, The test doubles are being cast to `never`, hiding type mismatches; update
the Args interface and the mock declarations so mocks match the real hook
signatures instead of using `as never`: change `setModels` and `setError` from
ReturnType<typeof vi.fn> to explicit React.Dispatch types (e.g.,
React.Dispatch<React.SetStateAction<AiModelAdmin[]>> and
React.Dispatch<React.SetStateAction<string | null>>), type `load` as the actual
LoadFn signature (or LoadFn & vi mock typed as that signature), and ensure
`isMountedRef` and `models` keep their concrete types; then replace all `as
never` usages with properly typed `vi.fn()` mocks that satisfy those signatures
(refer to the Args interface, setModels, setError, load, isMountedRef, models
and update the mock assignments used at the repeated call sites).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@admin/src/pages/users/useConfirmDialogs.test.ts`:
- Around line 62-255: The test titles in useConfirmDialogs.test.ts are
inconsistently bilingual (some only Japanese); update every it(...) description
(e.g., the titles around requestRoleChange/confirmRoleChange,
requestUnsuspend/confirmUnsuspend, requestDelete/cancelDelete/confirmDelete
tests) to include both Japanese and English (for example append " / <English
translation>" or " — <English translation>") so all it(...) strings follow the
JA+EN convention used elsewhere in the file and CI logs remain searchable; scan
for each it(...) in this file and make the title bilingual consistently.

---

Nitpick comments:
In `@admin/src/pages/ai-models/useAiModelsDragReorder.test.ts`:
- Around line 38-44: The test doubles are being cast to `never`, hiding type
mismatches; update the Args interface and the mock declarations so mocks match
the real hook signatures instead of using `as never`: change `setModels` and
`setError` from ReturnType<typeof vi.fn> to explicit React.Dispatch types (e.g.,
React.Dispatch<React.SetStateAction<AiModelAdmin[]>> and
React.Dispatch<React.SetStateAction<string | null>>), type `load` as the actual
LoadFn signature (or LoadFn & vi mock typed as that signature), and ensure
`isMountedRef` and `models` keep their concrete types; then replace all `as
never` usages with properly typed `vi.fn()` mocks that satisfy those signatures
(refer to the Args interface, setModels, setError, load, isMountedRef, models
and update the mock assignments used at the repeated call sites).

In `@admin/src/pages/users/useConfirmDialogs.test.ts`:
- Around line 157-204: Add a symmetric "stale reject is ignored" concurrency
test that mirrors the existing stale-resolve test: mock getUserImpact with two
sequential mockImplementationOnce calls that each return a new Promise and
capture their reject callbacks (similar to resolveA/resolveB), call
result.current.requestDelete(userA) then requestDelete(userB) using makeHook and
act, then invoke the first promise's reject (rejectA) and assert the hook's
state (result.current.deleteTarget, .impact, .loadingImpact) remains pointing to
userB and unaffected by the stale rejection, finally rejectB and assert B's
rejection path is handled as expected; ensure you use vi.mocked(getUserImpact),
the same result.current.requestDelete flow, and act/await Promise.resolve() to
flush microtasks so the catch-guard path is exercised.

In `@packages/ui/src/components/sidebar/SidebarProvider.test.tsx`:
- Around line 17-22: Extract the clearCookies helper into a shared test utility
and import it from tests/utils (or your existing test helpers); move the current
clearCookies function (the loop that parses document.cookie and sets each cookie
to max-age=0) into a named export e.g., clearCookies in a new/shared module,
update SidebarProvider.test.tsx to import { clearCookies } from that utility,
and remove the duplicate local function from other sidebar tests to reference
the shared export so all tests use the same implementation.
- Line 74: Update the two test spec strings to be bilingual: modify the it(...)
titles that currently read "toggleSidebar で開閉が切り替わり cookie に永続化される" and the
other Japanese-only title at the second occurrence to include an English
translation after the Japanese (e.g. "toggleSidebar で開閉が切り替わり cookie に永続化される /
toggles sidebar and persists state to cookie"); ensure both it(...) calls in
SidebarProvider.test.tsx contain Japanese and English per the repo guideline.

In `@packages/ui/src/hooks/use-toast.test.ts`:
- Line 24: Update the test title string in the it(...) call currently reading
"ADD_TOAST は配列の先頭に追加し、TOAST_LIMIT(=1) で切り詰める" to include an English translation
so it is bilingual (e.g., append " / adds toast to front of array and trims to
TOAST_LIMIT(=1)"). Modify the test declaration in use-toast.test.ts where the
it(...) for ADD_TOAST is defined to include both Japanese and English text while
preserving the original meaning and punctuation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 91893ba6-5101-4620-8f4a-1c0f264dd123

📥 Commits

Reviewing files that changed from the base of the PR and between 68e7de5 and 83bb072.

📒 Files selected for processing (17)

• .github/workflows/ci.yml
• admin/src/api/activity.test.ts
• admin/src/api/client.test.ts
• admin/src/api/lint.test.ts
• admin/src/lib/dateUtils.test.ts
• admin/src/pages/ai-models/useAiModelActions.test.ts
• admin/src/pages/ai-models/useAiModelsDragReorder.test.ts
• admin/src/pages/users/useConfirmDialogs.test.ts
• admin/vitest.config.ts
• package.json
• packages/ui/package.json
• packages/ui/src/components/sidebar/SidebarProvider.test.tsx
• packages/ui/src/components/sidebar/sidebarConstants.test.ts
• packages/ui/src/hooks/use-mobile.test.tsx
• packages/ui/src/hooks/use-toast.test.ts
• packages/ui/src/test/setup.ts
• packages/ui/vitest.config.ts

✅ Files skipped from review due to trivial changes (1)

• packages/ui/package.json

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Keep test titles consistently bilingual across this file.

Several it(...) descriptions are Japanese-only while others are bilingual; please align all test titles/comments to JA+EN for consistency and searchable CI logs.

As per coding guidelines "**/*.{ts,tsx,js,jsx}: コメントやドキュメントは、原則として日本語と英語の両方を併記する。".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@admin/src/pages/users/useConfirmDialogs.test.ts` around lines 62 - 255, The
test titles in useConfirmDialogs.test.ts are inconsistently bilingual (some only
Japanese); update every it(...) description (e.g., the titles around
requestRoleChange/confirmRoleChange, requestUnsuspend/confirmUnsuspend,
requestDelete/cancelDelete/confirmDelete tests) to include both Japanese and
English (for example append " / <English translation>" or " — <English
translation>") so all it(...) strings follow the JA+EN convention used elsewhere
in the file and CI logs remain searchable; scan for each it(...) in this file
and make the title bilingual consistently.