feat(ai-engine,frontend): ADR-0011 PR-E3b — OAuth Route Handler + AuthRequestCard 改修#32
Merged
Merged
Conversation
…hRequestCard 改修 ADR-0011 PR-E3b: 外部 MCP の OAuth 2.1 フローを Tally Route Handler 経由で 開始/監視/中止できるようにし、AuthRequestCard の paste UX を廃止して 1 ステップ化。 新規 Route Handler: - POST /api/projects/[id]/mcp/[mcpServerId]/oauth → orchestrator を start し authorizationUrl を返す - GET /api/projects/[id]/mcp/[mcpServerId]/oauth → 現在の OAuthFlowStatus (未開始は 404) - DELETE /api/projects/[id]/mcp/[mcpServerId]/oauth → 進行中フローを clear (UI の「やり直す」) - error 分類: project/mcpServer 不在 → 404、oauth 未設定 → 400、二重 start → 409 (固定文言)、その他 → 500 AuthRequestCard 全面リライト: - 旧 paste UI 削除。callback URL を Tally の loopback が直接受けるため。 - ボタン → POST → window.open → 内部 cardState で polling (2s setTimeout self-rescheduling)。 - block.status / block.authUrl は無視 (PR-E4 で block 自体が消える過渡期 prop)。 - 失敗時は「やり直す」→ DELETE → idle。 codex セカンドオピニオン Major 対応: - orchestrator の flow key を composite (projectId:mcpServerId) に変更。複数 project が 同名 mcpServerId='atlassian' を持っても flow がクロス汚染しない。getOAuthFlowStatus / awaitOAuthFlowSettled / clearOAuthFlow の signature を (projectId, mcpServerId) に変更。 - AuthRequestCard マウント時に GET で orchestrator 状態を rehydrate する effect を追加。 リマウント時に pending/completed が UI から消えて 409 で詰まるバグを防ぐ。 ai-engine: oauth API を package root から re-export (frontend Route Handler が import するため)。 テスト追加: - ai-engine: 別 project の同名 mcpServerId は flow が独立 (cross-project isolation) - frontend: Route Handler の POST/GET/DELETE 全 9 ケース - frontend: AuthRequestCard の hydrate / completed 表示 / 認証 / 失敗 / やり直す / projectId 未設定 disable
📝 WalkthroughWalkthroughOAuthフロー管理を複合キー（projectId + mcpServerId）で実装し、プロジェクト間の状態衝突を防止。ai-engineパッケージがOAuth関連ユーティリティを再エクスポートし、フロントエンドに新規ルートハンドラーとサーバー駆動型UIフローを追加。 ChangesOAuth フロー オーケストレーション統合
Sequence DiagramsequenceDiagram
participant User as User<br>(Frontend)
participant Card as AuthRequestCard<br>Component
participant Route as OAuth Route<br>Handler
participant Orchestrator as OAuthFlow<br>Orchestrator
participant Storage as Flow State<br>Storage
rect rgb(200, 150, 255, 0.5)
Note over User,Storage: OAuth Flow Initiation (Composite Key)
User->>Card: Click Auth Button
Card->>Route: POST /api/projects/{id}/mcp/{mcpServerId}/oauth
Route->>Orchestrator: startOAuthFlow({projectId, mcpServerId, ...})
Orchestrator->>Storage: Reserve pending entry<br>via flowKey=(projectId, mcpServerId)
Orchestrator-->>Route: {authorizationUrl}
Route-->>Card: {authorizationUrl}
Card->>User: window.open(authorizationUrl)<br>Transition to "pending" UI
end
rect rgb(150, 200, 255, 0.5)
Note over User,Storage: Polling & Rehydration
Card->>Route: GET /api/projects/{id}/mcp/{mcpServerId}/oauth
Route->>Orchestrator: getOAuthFlowStatus(projectId, mcpServerId)
Orchestrator->>Storage: Lookup flowKey state
Storage-->>Orchestrator: {status, ...}
Orchestrator-->>Route: {status: "pending"|"completed"}
Route-->>Card: API Response
Card->>Card: Re-render based on status
end
rect rgb(255, 200, 150, 0.5)
Note over User,Storage: Retry / Cleanup
User->>Card: Click Retry Button
Card->>Route: DELETE /api/projects/{id}/mcp/{mcpServerId}/oauth
Route->>Orchestrator: clearOAuthFlow(projectId, mcpServerId)
Orchestrator->>Storage: Delete flowKey entry<br>Close callback server
Storage-->>Orchestrator: Cleared
Orchestrator-->>Route: {ok: true}
Route-->>Card: Success
Card->>Card: Reset to "idle" state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/ai-engine/src/oauth/oauth-flow-orchestrator.ts (1)
156-158: 💤 Low valueコメント内の変数名が古い参照のまま
Line 157 のコメントで
flows.get(mcpServerId)と書かれていますが、実際にはflowKeyを使用しています。コメントの整合性のため更新をお勧めします。📝 コメント修正案
// bg promise を IIFE で生成。IIFE 内部から自分自身の promise 変数を直接参照すると - // tsc の definite-assignment 解析でエラーになるため、`flows.get(mcpServerId)?.promise` + // tsc の definite-assignment 解析でエラーになるため、`flows.get(flowKey)?.promise` // 経由で取り出して再 set する。これは外側で flows.set した後の値を読むので安全。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-engine/src/oauth/oauth-flow-orchestrator.ts` around lines 156 - 158, Update the outdated variable name in the explanatory comment inside oauth-flow-orchestrator.ts: replace the reference to flows.get(mcpServerId) with flows.get(flowKey)?.promise (or otherwise reference flowKey consistently) in the comment that explains the bg promise IIFE and definite-assignment behavior so the comment matches the actual code using flowKey and flows.set(...).packages/frontend/src/components/chat/auth-request-card.test.tsx (1)
44-71: ⚖️ Poor tradeoffpolling 中の状態遷移テストの追加を検討
現在のテストは POST 成功後の即座の状態遷移をカバーしていますが、pending 状態での polling による completed/failed への遷移はカバーされていません。polling ロジックの回帰を防ぐため、以下のようなテストを追加することを検討してください:
- pending 状態で GET が completed を返したら "認証済" に遷移
- pending 状態で GET が failed を返したら "失敗" に遷移 + failureMessage 表示
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/components/chat/auth-request-card.test.tsx` around lines 44 - 71, The test suite for AuthRequestCard lacks coverage for the polling transitions from pending to completed/failed; add tests that render <AuthRequestCard block={pendingBlock} />, stub global fetch (the existing fetchMock/EXPECTED_BASE_URL logic) to first respond to the POST with the authorizationUrl and then on subsequent GET polls return JSON with status: "completed" (and separately a case with status: "failed" plus failureMessage), advance timers or wait for the polling interval so the component executes its polling logic, and assert that the UI transitions to show "認証済" for completed and "失敗" plus the failureMessage for failed; keep using vi.stubGlobal('fetch', ...) and waitFor/assertions similar to the POST test to verify state changes and that the callback textbox is removed.packages/ai-engine/src/oauth/oauth-flow-orchestrator.test.ts (1)
259-261: 💤 Low valueテストの堅牢性向上の提案
現在のテストは単一の project + mcpServerId の組み合わせのみを確認していますが、異なる projectId でも null を返すことを追加で検証すると、composite key の正しさがより明確になります。
💡 追加アサーション案
it('未開始の mcpServerId は getOAuthFlowStatus が null を返す', () => { expect(getOAuthFlowStatus(TEST_PID, 'never-started')).toBeNull(); + // 異なる projectId でも同様に null + expect(getOAuthFlowStatus('other-project', 'atlassian')).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-engine/src/oauth/oauth-flow-orchestrator.test.ts` around lines 259 - 261, The test case only asserts getOAuthFlowStatus(TEST_PID, 'never-started') is null, which misses verifying the composite key logic; update the same it block to also call getOAuthFlowStatus with a second distinct project id (e.g., a new constant like ANOTHER_TEST_PID or a different literal) and assert that getOAuthFlowStatus(ANOTHER_TEST_PID, 'never-started') is also null so both projectId+mcpServerId combinations return null; reference getOAuthFlowStatus, TEST_PID and the 'never-started' mcpServerId when adding the additional assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ai-engine/src/oauth/oauth-flow-orchestrator.test.ts`:
- Around line 259-261: The test case only asserts getOAuthFlowStatus(TEST_PID,
'never-started') is null, which misses verifying the composite key logic; update
the same it block to also call getOAuthFlowStatus with a second distinct project
id (e.g., a new constant like ANOTHER_TEST_PID or a different literal) and
assert that getOAuthFlowStatus(ANOTHER_TEST_PID, 'never-started') is also null
so both projectId+mcpServerId combinations return null; reference
getOAuthFlowStatus, TEST_PID and the 'never-started' mcpServerId when adding the
additional assertion.
In `@packages/ai-engine/src/oauth/oauth-flow-orchestrator.ts`:
- Around line 156-158: Update the outdated variable name in the explanatory
comment inside oauth-flow-orchestrator.ts: replace the reference to
flows.get(mcpServerId) with flows.get(flowKey)?.promise (or otherwise reference
flowKey consistently) in the comment that explains the bg promise IIFE and
definite-assignment behavior so the comment matches the actual code using
flowKey and flows.set(...).
In `@packages/frontend/src/components/chat/auth-request-card.test.tsx`:
- Around line 44-71: The test suite for AuthRequestCard lacks coverage for the
polling transitions from pending to completed/failed; add tests that render
<AuthRequestCard block={pendingBlock} />, stub global fetch (the existing
fetchMock/EXPECTED_BASE_URL logic) to first respond to the POST with the
authorizationUrl and then on subsequent GET polls return JSON with status:
"completed" (and separately a case with status: "failed" plus failureMessage),
advance timers or wait for the polling interval so the component executes its
polling logic, and assert that the UI transitions to show "認証済" for completed
and "失敗" plus the failureMessage for failed; keep using vi.stubGlobal('fetch',
...) and waitFor/assertions similar to the POST test to verify state changes and
that the callback textbox is removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 27ecc71b-a294-445f-837d-d14f5fdeae2d
📒 Files selected for processing (7)
packages/ai-engine/src/index.tspackages/ai-engine/src/oauth/oauth-flow-orchestrator.test.tspackages/ai-engine/src/oauth/oauth-flow-orchestrator.tspackages/frontend/src/app/api/projects/[id]/mcp/[mcpServerId]/oauth/route.test.tspackages/frontend/src/app/api/projects/[id]/mcp/[mcpServerId]/oauth/route.tspackages/frontend/src/components/chat/auth-request-card.test.tsxpackages/frontend/src/components/chat/auth-request-card.tsx
This was referenced May 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ADR-0011 PR-E3b: 外部 MCP の OAuth 2.1 フローを Tally の Route Handler 経由で開始/監視/中止できるようにし、AuthRequestCard の paste UX を廃止して 1 ステップ化する。PR-E3a で merge 済みの OAuthFlowOrchestrator を Route Handler でラップし、UI 側は新エンドポイントを polling する。
/api/projects/[id]/mcp/[mcpServerId]/oauth(POST=start / GET=status / DELETE=clear)ADR-0011 ロードマップ進捗
主な変更点
Route Handler (新規)
/api/projects/[id]/mcp/[mcpServerId]/oauthエラー分類:
AuthRequestCard 改修
idle | starting | pending | completed | failed) で UI 駆動codex セカンドオピニオン Major 対応
projectId:mcpServerIdの composite key に変更。getOAuthFlowStatus/awaitOAuthFlowSettled/clearOAuthFlowの signature も(projectId, mcpServerId)に変更。Test plan
既知の限界 (PR-E4 で解消)
mcp__<id>__authenticateAI tool を発行する。AuthRequestCard は新 Route Handler を使うので、SDK 側の loopback は orphan になる (実害は無いが冗長)。mcp_oauth_completedへの移行は未実施。本 PR では HTTP polling で完了を検知。Summary by CodeRabbit
リリースノート
新機能
テスト