feat(ai-engine): ADR-0011 PR-E2 — OAuthClient + LoopbackCallbackServer#30
Merged
Conversation
ADR-0011 の実装段階 2。issue #28 への対応として、外部 MCP server に対する OAuth 2.1 (Authorization Code + PKCE) クライアントを実装する。本 PR でも OAuth フロー全体は接続されない (PR-E3 で API + UI、PR-E4 で SDK 統合)。 ## 設計判断 - Node 標準のみ (依存追加なし、oauth4webapi 等は使わない) - PKCE は S256 のみ (RFC 7636 推奨) - state の verify は呼び出し側 (PR-E3 の Orchestrator) の責務 - LoopbackCallbackServer は port=0 で OS 採番、host は 127.0.0.1 固定 - token endpoint は application/x-www-form-urlencoded で叩く ## 新規ファイル - packages/ai-engine/src/oauth/oauth-client.ts: PKCE / state / authorization URL / token 交換 / refresh - packages/ai-engine/src/oauth/oauth-client.test.ts: 12 件 - packages/ai-engine/src/oauth/loopback-callback-server.ts: 一時 HTTP server (127.0.0.1, port=0, callback path 限定、HTML escape、timeout、冪等 close) - packages/ai-engine/src/oauth/loopback-callback-server.test.ts: 10 件 - packages/ai-engine/src/oauth/index.ts: 公開 export ## packages/core 拡張 - OAuthProviderConfig.prompt?: string を追加 (Atlassian 用に 'consent')。 ハードコードを避けて将来の prompt 不要 provider を許容する。 - OAUTH_REGISTRY を as const satisfies Readonly<...> に変更 - OAuthProviderConfig 型を core から公開 export ## codex セカンドオピニオン対応 [MEDIUM] close() で pending awaitCallback を reject 発火 (永久 pending リーク防止) [MEDIUM] prompt=consent を OAuthProviderConfig に移して provider 制御化 [LOW] access_token を typeof + 空文字列で明示チェック [LOW] close 中の pending awaitCallback の reject test を追加 pnpm typecheck 4/4 PASS / pnpm test core 99 + storage 97 + ai-engine 235 + frontend 273 = 704 PASS / pnpm lint exit 0。
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughこのPRはOAuth 2.1クライアント実装を追加します。OSが割り当てたポートで ChangesOAuth 2.1クライアント実装
Sequence DiagramsequenceDiagram
participant User as ユーザー
participant Client as クライアント
participant LoopbackServer as ループバック<br/>サーバー
participant AuthServer as 認可<br/>サーバー
participant TokenServer as トークン<br/>エンドポイント
User->>Client: OAuth開始をリクエスト
Client->>Client: generatePkcePair()でPKCE生成
Client->>Client: generateOAuthState()でstate生成
Client->>LoopbackServer: startLoopbackCallbackServer()で開始
LoopbackServer-->>Client: redirectUri（ポート付き）を返す
Client->>Client: buildAuthorizationUrl()で認可URLを構築
Client->>User: 認可URLをブラウザで開く
User->>AuthServer: 認可URLにアクセス
AuthServer->>User: ログイン・同意画面を表示
User->>AuthServer: ログイン・同意
AuthServer->>LoopbackServer: redirectUri + code + stateでリダイレクト
LoopbackServer->>LoopbackServer: awaitCallback()でcode/stateを解決
LoopbackServer-->>AuthServer: HTML応答（200）
AuthServer-->>User: ブラウザでタブを閉じるよう指示
LoopbackServer-->>Client: code/stateを返す
Client->>TokenServer: exchangeCodeForToken()でPOST<br/>（code+codeVerifier）
TokenServer-->>Client: accessToken+refreshToken
Client->>LoopbackServer: close()でサーバーを停止
Client-->>User: 認可完了
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 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 54 minutes and 36 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ai-engine/src/oauth/oauth-client.test.ts (1)
1-3: ⚡ Quick winimport 順序を規約順に揃えてください
vitest（external）を先にし、@tally/core（internal）を次に置く順にしてください。修正例
-import { ATLASSIAN_CLOUD_OAUTH } from '@tally/core'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { ATLASSIAN_CLOUD_OAUTH } from '@tally/core'; + import { buildAuthorizationUrl, exchangeCodeForToken,As per coding guidelines, "Import order should be: external libraries → internal packages → relative paths, with blank lines between groups".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-engine/src/oauth/oauth-client.test.ts` around lines 1 - 3, The import block is not in the mandated order; move the vitest imports (afterEach, describe, expect, it, vi) to be the first import group (external libraries), then place the internal import (ATLASSIAN_CLOUD_OAUTH from '@tally/core') after a blank line so imports follow "external → internal → relative" ordering and include a separating blank line between groups.
🤖 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/ai-engine/src/oauth/loopback-callback-server.ts`:
- Around line 119-130: awaitCallback currently overwrites
resolveCallback/rejectCallback and timeoutHandle, leaking previously created
Promises and allowing calls after close to hang; modify awaitCallback to (1)
track a single active Promise (e.g. storedActivePromise) and if called while one
exists either return that same Promise or first reject/cleanup the previous one
before creating a new Promise, (2) check a closed flag set by close() and
immediately return a rejected Promise when closed, and (3) ensure cleanup()
clears resolveCallback, rejectCallback, timeoutHandle and the
storedActivePromise; update references to
resolveCallback/rejectCallback/timeoutHandle/cleanup()/close() in the same file
accordingly so no Promise can be left unresolved on multiple awaitCallback calls
or after close().
---
Nitpick comments:
In `@packages/ai-engine/src/oauth/oauth-client.test.ts`:
- Around line 1-3: The import block is not in the mandated order; move the
vitest imports (afterEach, describe, expect, it, vi) to be the first import
group (external libraries), then place the internal import
(ATLASSIAN_CLOUD_OAUTH from '@tally/core') after a blank line so imports follow
"external → internal → relative" ordering and include a separating blank line
between groups.
🪄 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: a91827ad-b2b0-4e83-a038-f6dbdca72c19
📒 Files selected for processing (8)
packages/ai-engine/src/oauth/index.tspackages/ai-engine/src/oauth/loopback-callback-server.test.tspackages/ai-engine/src/oauth/loopback-callback-server.tspackages/ai-engine/src/oauth/oauth-client.test.tspackages/ai-engine/src/oauth/oauth-client.tspackages/core/src/index.tspackages/core/src/oauth/atlassian.tspackages/core/src/oauth/index.ts
CodeRabbit (PR #30) Major 1 件対応。 awaitCallback() は内部で resolveCallback / rejectCallback を上書きする実装 だったため、同 handle で 2 回以上呼ぶと先行 Promise が未解決のまま残って リークしていた。close() 後の呼び出しも server 閉鎖済みで callback は届かず 永久 pending になっていた。 `awaitStarted` フラグと既存の `closed` フラグを冒頭で確認し、いずれかに 該当すれば即時 throw する。1 ハンドル 1 回呼出 + close 後不可の規約を 明示的に強制する。 test 2 件追加: 多重呼び出しで throw / close 後呼び出しで throw。 pnpm typecheck 4/4 PASS / pnpm test ai-engine 237 PASS / pnpm lint exit 0。
3 tasks
This was referenced May 2, 2026
This was referenced May 3, 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 (Tally 側で OAuth 2.1 フローを管理する) の実装段階 2。issue #28 への対応として、外部 MCP server に対する OAuth 2.1 (Authorization Code + PKCE) クライアントを実装する。
PR-E1 (#29 merged) で作った schema / token store の上に乗る形。本 PR でも OAuth フロー全体は接続されない (PR-E3 で API + UI、PR-E4 で SDK 統合)。
設計判断
oauth4webapi等は使わない)stateの verify は呼び出し側 (PR-E3 の Orchestrator) の責務127.0.0.1固定 (localhostだと IPv4/IPv6 衝突)application/x-www-form-urlencoded(RFC 6749 §4.1.3 準拠)client_secretは使わない変更ファイル
packages/ai-engine/src/oauth/ (新規)
oauth-client.ts: PKCE pair / state / authorization URL / token 交換 / refreshloopback-callback-server.ts: 一時 HTTP server (127.0.0.1, port=0, callback path 限定、HTML escape、timeout、冪等 close、pending reject 発火)index.ts: 公開 exportpackages/core (拡張)
OAuthProviderConfig.prompt?: string: Atlassian 用 'consent' を provider 設定で制御（ハードコード回避）OAUTH_REGISTRYをas const satisfies Readonly<...>に変更（リテラル型維持 + contract check）OAuthProviderConfig型を core から公開 exportcodex セカンドオピニオン対応 (push 直前 review)
close()で pendingawaitCallbackを reject 発火 /promptをOAuthProviderConfig化access_tokenをtypeof + 空文字列で明示チェック / close 中 pending reject の testTest plan
Related
Summary by CodeRabbit
リリースノート