-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add Authorization header support #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: cf694c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@JounQin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 46 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (7)
WalkthroughAdds a release changeset, removes a dev proxy route, localizes a radio label, removes a non-prod "local" auth origin, adds optional cloud auth token, and implements an x-fetch interceptor that injects Authorization and CLOUD_AUTH_ORIGIN for /smart/* requests and logs out on 401; adjusts session creation and error handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as AIAssistant UI
participant Hook as Auth useEffect
participant XF as x-fetch (interceptors)
participant API as /smart/api
rect rgba(230,245,255,0.6)
note over Hook: on authInfo / onLogout change
Hook->>XF: interceptors.use(ApiInterceptor)
note over XF: If request -> /smart/*\n• ensure Authorization: Bearer {token}\n• ensure CLOUD_AUTH_ORIGIN
end
UI->>API: POST /smart/api/new_session (no manual auth headers)
XF-->>API: Request (headers injected)
API-->>XF: 200 {sessionId} / 401
alt 200 OK
XF-->>UI: sessionId
else 401 Unauthorized
XF->>Hook: throw ResponseError(401)
Hook->>UI: onLogout()
end
sequenceDiagram
autonumber
participant UI as AIAssistant UI
participant XF as x-fetch
participant API as /smart/*
UI->>API: Any /smart/* request
XF-->>API: Adds auth/origin headers if missing
API-->>XF: Error (network / 4xx / 5xx)
alt No active session
XF-->>UI: Error (suppressed NetworkError UI)
else Active session
XF-->>UI: Error surfaced to UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Authorization header support to the Intelligence AI Assistant by integrating authentication tokens into API requests. It removes local development environment support and consolidates cloud-based authentication.
- Adds interceptor to automatically include Authorization and CLOUD_AUTH_ORIGIN headers for API requests
- Removes local development environment configuration and related proxy settings
- Updates CloudAuth interface to include optional token field
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| types.ts | Removes 'local' from CloudAuthRegion name union type |
| context.tsx | Adds optional token field to CloudAuth interface and returns token in getCloudAuth |
| constants.ts | Removes local development environment configuration and imports |
| AIAssistant/index.tsx | Adds API interceptor for Authorization headers and error handling |
| LoginForm/index.tsx | Removes local-specific label handling and fixes JSX formatting |
| load-config.ts | Removes local API proxy configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
commit: |
597355a to
a7977fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/doom/src/global/Intelligence/AIAssistant/index.tsx (1)
63-88: Fix TS error on err.response.status and remove unsafe non-null assertions in interceptor.
- Pipeline shows a TS error at Line 77;
responsecan be undefined.- Using
authInfo!risksBearer undefinedand crashes when unauthenticated. This was raised previously; apply safe guards.Apply this diff:
const interceptor: ApiInterceptor = async (req, next) => { if (!req.url.startsWith('/smart/')) { return next(req) } - if (!req.headers.has('Authorization')) { - req.headers.set('Authorization', `Bearer ${authInfo!.token}`) - } - if (!req.headers.has('CLOUD_AUTH_ORIGIN')) { - req.headers.set('CLOUD_AUTH_ORIGIN', authInfo!.origin) - } + if (!req.headers.has('Authorization') && authInfo?.token) { + req.headers.set('Authorization', `Bearer ${authInfo.token}`) + } + if (!req.headers.has('CLOUD_AUTH_ORIGIN') && authInfo?.origin) { + req.headers.set('CLOUD_AUTH_ORIGIN', authInfo.origin) + } try { return await next(req) } catch (err) { - if (err instanceof ResponseError && err.response.status === 401) { + if (err instanceof ResponseError && err.response?.status === 401) { onLogout() } throw err } }Optional: also treat 403 as logout depending on backend semantics.
🧹 Nitpick comments (1)
packages/doom/src/global/Intelligence/types.ts (1)
36-39: Derive region name type from the source of truth to avoid drift.Hardcoding
'global' | 'china'can desync fromCLOUD_AUTH_ORIGINS. Consider deriving the union from the constant.-import interface CloudAuthRegion { - name: 'global' | 'china' +export interface CloudAuthRegion { + name: typeof CLOUD_AUTH_ORIGINS[number]['name'] value: string }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/metal-adults-greet.md(1 hunks)packages/doom/src/cli/load-config.ts(0 hunks)packages/doom/src/global/Intelligence/AIAssistant/Preamble/LoginForm/index.tsx(1 hunks)packages/doom/src/global/Intelligence/AIAssistant/index.tsx(3 hunks)packages/doom/src/global/Intelligence/constants.ts(0 hunks)packages/doom/src/global/Intelligence/context.tsx(2 hunks)packages/doom/src/global/Intelligence/types.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/doom/src/cli/load-config.ts
- packages/doom/src/global/Intelligence/constants.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/doom/src/global/Intelligence/context.tsx (1)
packages/doom/src/global/Intelligence/utils.ts (1)
getAuthInfoFromToken(13-39)
🪛 GitHub Actions: CI
packages/doom/src/global/Intelligence/AIAssistant/index.tsx
[error] 77-77: TypeScript error reported at AIAssistant/index.tsx:77:45.
🔇 Additional comments (5)
.changeset/metal-adults-greet.md (1)
1-6: Changeset looks good.Patch bump with concise note is fine. Optionally add “Closes IDP-1291” in the body for cross-linking.
packages/doom/src/global/Intelligence/context.tsx (1)
9-11: Token removal semantics verified
setLocalStoragealready callslocalStorage.removeItem(key)whenvalueisnullorundefined, so the token is cleared on logout as intended.packages/doom/src/global/Intelligence/AIAssistant/Preamble/LoginForm/index.tsx (1)
147-153: i18n keys verified –customer_portal_globalandcustomer_portal_chinaare defined inpackages/doom/src/runtime/translation.ts(lines 28–29, 95–96, 155–156).packages/doom/src/global/Intelligence/AIAssistant/index.tsx (2)
2-10: Imports update is appropriate.
186-188: Good guard to suppress spurious network error messaging.Skipping the error message when no active session avoids confusing UX.
close IDP-1291
a7977fe to
cf694c9
Compare
close IDP-1291
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores