test(api): add route integration tests for low-coverage endpoints (#1032)#1040
Conversation
) Expand or add Hono route integration tests for ingest, clip, compose sessions, compose session projection, notes/tags, and thumbnail serve. Each endpoint covers auth, authorization, validation, success paths, and error handling via mocked services and createMockDb. Closes #1032
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds and restructures many server/api tests to a Hono test harness with hoisted Vitest mocks, extends the mock DB proxy to support execute-chaining, and updates a PDF hook cleanup call to use pdfDoc.loadingTask.destroy(). ChangesRoute Integration Tests for Low-Coverage Endpoints
Sequence Diagram(s) sequenceDiagram
participant Client
participant HonoApp
participant GraphRunner
participant Checkpointer
Client->>HonoApp: GET /api/pages/:pageId/compose-sessions/:id
HonoApp->>GraphRunner: streamEvents / resume
GraphRunner->>Checkpointer: getState / request checkpoint
Checkpointer-->>HonoApp: checkpoint state / projection
HonoApp-->>Client: JSON response (projection / status)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly expands test coverage across several API routes, including clip extraction, compose sessions, ingestion planning, note-wide tag aggregation, and thumbnail serving. The feedback highlights a potential logical issue in a compose session test case where a 403 Forbidden status is expected, even though the mock setup indicates the requesting user is the owner of the resource.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| it("returns 403 when caller cannot view the page", async () => { | ||
| const { app } = createComposeApp([ | ||
| [{ id: PAGE_ID, ownerId: OTHER_USER_ID, noteId: NOTE_ID }], | ||
| [{ email: "other@example.com" }], | ||
| [mockNote({ ownerId: OTHER_USER_ID, visibility: "private" })], | ||
| [], | ||
| [], | ||
| ]); | ||
| const res = await app.request(`/api/pages/${PAGE_ID}/compose-sessions/sess-2`, { | ||
| headers: authHeaders(OTHER_USER_ID), | ||
| }); | ||
| expect(res.status).toBe(403); |
There was a problem hiding this comment.
このテストケースの意図が少し不明確です。セットアップでは、ページとノートの両方が OTHER_USER_ID によって所有されており、リクエストも OTHER_USER_ID から送信されています。この場合、ユーザーは所有者であるため、assertPageViewAccess によるアクセスチェックは成功するはずです。したがって、期待されるステータスコードは 403 Forbidden ではなく、後続の処理でセッションが見つからないことによる 404 Not Found になるべきではないでしょうか。
もし 403 が意図した結果であるなら、アクセスが拒否される具体的な理由を明確にするためのコメントをテストに追加すると、可読性が向上すると思われます。
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Add missing GraphContext.feature field and ComposeContentLocale literal. Fix compose-sessions 403 test to use a non-member caller on a private note (per Gemini review).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/api/src/__tests__/routes/ingest.test.ts`:
- Around line 366-478: The tests for POST /api/ingest/graph/run and
/api/ingest/graph/resume are missing coverage for cross-user access denial on
stateful resources; add tests that simulate a thread/session owned by another
user and assert the API returns the expected 403/404. Specifically, in the "run"
and "resume" suites write cases using createIngestApp and authHeaders for a
different user context (or mock the ownership lookup) to call endpoints with a
threadId owned by TEST_USER_ID but authenticated as another user, and assert the
response is 403/404 and that mocks like mockGraphRunnerInvoke,
mockGraphRunnerResume, and mockResolveCheckpointerForRun are not invoked when
access is denied; reuse existing helpers (createIngestApp, authHeaders) and
existing mocks (mockGraphRunnerInvoke, mockGraphRunnerResume,
mockResolveCheckpointerForRun, mockAssertComposeBackendReady) to implement these
assertions.
In `@server/api/src/__tests__/routes/thumbnail/serve.test.ts`:
- Around line 166-173: Update the "returns 404 when thumbnail row is not found
for caller" test to instead seed an existing thumbnail row owned by TEST_USER_ID
and issue the request as ATTACKER_ID so we exercise the "row exists but belongs
to another user" authz path; use createServeApp with a dataset containing a
thumbnail object keyed by OBJECT_ID and ownerId set to TEST_USER_ID, call
app.request(`/api/thumbnail/serve/${OBJECT_ID}`) with header "x-test-user-id":
ATTACKER_ID, and assert res.status is 404 and mockS3Send was not called (keep
test name and assertions but change the seeded row to belong to TEST_USER_ID).
🪄 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: 8ebf83bd-3918-41f0-8133-77c48e4ae8f6
📒 Files selected for processing (7)
server/api/src/__tests__/createMockDb.tsserver/api/src/__tests__/routes/clip.test.tsserver/api/src/__tests__/routes/composeSessionProjection.test.tsserver/api/src/__tests__/routes/composeSessions.test.tsserver/api/src/__tests__/routes/ingest.test.tsserver/api/src/__tests__/routes/notes/tags.test.tsserver/api/src/__tests__/routes/thumbnail/serve.test.ts
| it("returns 404 when thumbnail row is not found for caller", async () => { | ||
| const { app } = createServeApp([[]]); | ||
| const res = await app.request(`/api/thumbnail/serve/${OBJECT_ID}`, { | ||
| headers: { "x-test-user-id": ATTACKER_ID }, | ||
| }); | ||
| expect(res.status).toBe(404); | ||
| expect(mockS3Send).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
Strengthen the authz case with an existing foreign-owned thumbnail row.
This case currently validates only “row missing,” not “row exists but belongs to another user.” It can still pass if ownership filtering regresses. Use an existing row owned by TEST_USER_ID and call with ATTACKER_ID, then assert 404 and no S3 call.
Suggested test adjustment
- it("returns 404 when thumbnail row is not found for caller", async () => {
- const { app } = createServeApp([[]]);
+ it("returns 404 when thumbnail exists but belongs to another user", async () => {
+ const { app } = createServeApp([[THUMB_ROW]]);
const res = await app.request(`/api/thumbnail/serve/${OBJECT_ID}`, {
headers: { "x-test-user-id": ATTACKER_ID },
});
expect(res.status).toBe(404);
expect(mockS3Send).not.toHaveBeenCalled();
});📝 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.
| it("returns 404 when thumbnail row is not found for caller", async () => { | |
| const { app } = createServeApp([[]]); | |
| const res = await app.request(`/api/thumbnail/serve/${OBJECT_ID}`, { | |
| headers: { "x-test-user-id": ATTACKER_ID }, | |
| }); | |
| expect(res.status).toBe(404); | |
| expect(mockS3Send).not.toHaveBeenCalled(); | |
| }); | |
| it("returns 404 when thumbnail exists but belongs to another user", async () => { | |
| const { app } = createServeApp([[THUMB_ROW]]); | |
| const res = await app.request(`/api/thumbnail/serve/${OBJECT_ID}`, { | |
| headers: { "x-test-user-id": ATTACKER_ID }, | |
| }); | |
| expect(res.status).toBe(404); | |
| expect(mockS3Send).not.toHaveBeenCalled(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/src/__tests__/routes/thumbnail/serve.test.ts` around lines 166 -
173, Update the "returns 404 when thumbnail row is not found for caller" test to
instead seed an existing thumbnail row owned by TEST_USER_ID and issue the
request as ATTACKER_ID so we exercise the "row exists but belongs to another
user" authz path; use createServeApp with a dataset containing a thumbnail
object keyed by OBJECT_ID and ownerId set to TEST_USER_ID, call
app.request(`/api/thumbnail/serve/${OBJECT_ID}`) with header "x-test-user-id":
ATTACKER_ID, and assert res.status is 404 and mockS3Send was not called (keep
test name and assertions but change the seeded row to belong to TEST_USER_ID).
pdfjs-dist 6.0 removed PDFDocumentProxy.destroy(); migrate cleanup to loadingTask.destroy() so Type Check passes after develop picked up #1004.
Add ingest graph run/resume 403 cases for foreign checkpoint threadIds. Clarify thumbnail GET scopes lookup with where/limit chain assertions.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/pdfReader/usePdfDocument.ts`:
- Line 12: The inline comment about the pdfjs-dist v6 API change is
English-only; update the comment near pdfDoc.loadingTask.destroy() (and the note
about PDFDocumentProxy.destroy) to include a concise Japanese translation as
well, keeping both languages side-by-side per coding guidelines (e.g., English
sentence followed by Japanese sentence explaining that pdfjs-dist v6 removed
PDFDocumentProxy.destroy and to call pdfDoc.loadingTask.destroy()). Make sure
the Japanese text is clear, succinct, and mirrors the existing English meaning
without changing code or behavior.
🪄 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: 4044da3f-40a7-4f16-bf72-2e937d406dfd
📒 Files selected for processing (1)
src/components/pdfReader/usePdfDocument.ts
Combine bilingual TSDoc for pdfjs-dist v6 loadingTask.destroy() migration.
2 participants
概要
Issue #1032 の受け入れ条件に沿い、カバレッジが低かった API ルート層へ統合テストを追加・拡充した。既存の Hono +
authRequiredモック +createMockDbパターンを踏襲している。変更点
ingest.test.ts:/plan・/graph/run・/graph/resume・/applyの認証・バリデーション・成功系・エラー変換を追加clip.test.ts:/youtubeエンドポイントのテストを追加（認証・バリデーション・成功・502）composeSessions.test.ts: GET projection・POST run (SSE)・PATCH resume 成功系・認可テストを追加composeSessionProjection.test.ts:loadComposeSessionProjectionと追加 interrupt 分岐のテストnotes/tags.test.ts:GET /api/notes/:noteId/tagsを新規追加（認可・ETag/304）thumbnail/serve.test.ts:GET /api/thumbnail/serve/:idのストリーム配信テストを追加createMockDb.ts:db.executeチェーン記録を追加（ingest / tags 向け）変更の種類
テスト方法
対象ファイルのみ:
チェックリスト
cd server/api && bunx vitest run— 1488 tests）カバレッジ（対象ルートのラインカバレッジ）
ingest.tsclip.tscomposeSessions.tscomposeSessionProjection.tsnotes/tags.tsthumbnail/serve.ts関連 Issue
Closes #1032
Summary by CodeRabbit