Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough도메인 URL에서 www 서브도메인을 제거하고, 로그아웃 시 익스텐션과 클라이언트 간 메시지 기반 토큰 동기화를 추가하며, MacStep 자산을 WebP에서 SVG로 변경하고, 401/403 에러 시 토큰 제거를 비활성화했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (ProfilePopup)
participant Window as window.postMessage
participant BG as Extension (background.ts)
participant Content as Extension (content.ts)
participant Storage as chrome.storage.local
Client->>Client: Remove token & email from localStorage
Client->>Window: postMessage({type: 'Extension-Logout'}, origin)
par Parallel Processing
Window->>BG: Receive Extension-Logout message
BG->>Storage: Remove 'token'
Storage-->>BG: ✓ Token removed
BG->>BG: Log 'Token removed!'
and
Window->>Content: Receive Extension-Logout message
Content->>Storage: Remove 'token'
Storage-->>Content: ✓ Token removed
Content->>Content: Log 'Token removed!'
end
Client->>Client: Navigate to login page
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 5❌ Failed checks (3 warnings, 2 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
|
✅ Storybook chromatic 배포 확인: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/client/src/shared/apis/setting/axiosInstance.ts`:
- Around line 49-52: On 401/403 handling in the axios error interceptor (the
block that currently calls window.location.href =
'/onboarding?step=SOCIAL_LOGIN' and returns Promise.reject(error)), restore the
token removal logic by removing the stored auth token (undo the commented out
localStorage.removeItem('token')) before redirecting; if the removal was
intentionally disabled, add a clear comment above this block explaining why the
token must persist and what other mechanism will clear it, referencing the axios
error interceptor where window.location.href and Promise.reject(error) are used
so reviewers can find the change.
In `@apps/extension/src/background.ts`:
- Around line 24-30: The background.ts chrome.runtime.onMessage listener for
'Extension-Logout' is never called because ProfilePopup.tsx uses
window.postMessage and content.ts handles it via
window.addEventListener('message'); either remove the unused
chrome.runtime.onMessage handler from background.ts, or change messaging to
centralize logout in background.ts by updating ProfilePopup.tsx (or content.ts)
to call chrome.runtime.sendMessage({ type: 'Extension-Logout' }) and ensure
content.ts forwards the window message to the background with
chrome.runtime.sendMessage; then keep the existing chrome.runtime.onMessage
handler in background.ts to remove the token from chrome.storage.local.
In `@apps/extension/src/content.ts`:
- Around line 14-21: The logout handler in the window message listener only
removes the token from chrome.storage.local and never notifies the background
script, leaving the background 'Extension-Logout' listener unused; update the
handler in content.ts (the window.addEventListener callback that checks
event.data.type === 'Extension-Logout') to also call
chrome.runtime.sendMessage({ type: 'Extension-Logout' }) (or include any
required payload) after removing the token so the background.ts listener
receives the logout event and can perform its cleanup.
🧹 Nitpick comments (3)
apps/extension/src/background.ts (1)
16-30: 두 개의onMessage리스너를 하나로 통합하는 것을 고려해주세요.현재
chrome.runtime.onMessage.addListener가 두 번 등록되어 있습니다. 하나의 리스너 내에서 메시지 타입을 분기 처리하면 코드가 더 깔끔해집니다.♻️ 통합 리스너 제안
-chrome.runtime.onMessage.addListener((message) => { - if (message.type === 'SET_TOKEN') { - chrome.storage.local.set({ token: message.token }, () => { - console.log('Token saved!'); - }); - } -}); - -chrome.runtime.onMessage.addListener((message) => { - if (message.type === 'Extension-Logout') { - chrome.storage.local.remove('token', () => { - console.log('Token removed!'); - }); - } -}); +chrome.runtime.onMessage.addListener((message) => { + if (message.type === 'SET_TOKEN') { + chrome.storage.local.set({ token: message.token }, () => { + console.log('Token saved!'); + }); + } else if (message.type === 'Extension-Logout') { + chrome.storage.local.remove('token', () => { + console.log('Token removed!'); + }); + } +});apps/client/src/shared/components/profilePopup/ProfilePopup.tsx (1)
45-53: 내부 함수 정의를 인라인으로 간소화할 수 있습니다.
sendExtensionLogout함수가 정의 직후 한 번만 호출됩니다. 함수 래핑 없이 직접postMessage를 호출하면 더 간결해집니다.♻️ 간소화 제안
const handleLogout = () => { localStorage.removeItem('token'); localStorage.removeItem('email'); - const sendExtensionLogout = () => { - window.postMessage( - { - type: 'Extension-Logout', - }, - window.location.origin - ); - }; - sendExtensionLogout(); + window.postMessage({ type: 'Extension-Logout' }, window.location.origin); navigate('/login'); };apps/extension/src/content.ts (1)
1-21: 두 개의 메시지 리스너를 하나로 통합하는 것을 고려해주세요.
SET_TOKEN과Extension-Logout을 같은 리스너에서 분기 처리하면 코드 중복이 줄어들고 유지보수가 용이해집니다.♻️ 통합 리스너 제안
window.addEventListener('message', (event) => { if (event.source !== window) return; + if (event.data.type === 'SET_TOKEN') { chrome.runtime.sendMessage({ type: 'SET_TOKEN', token: event.data.token, }); chrome.storage.local.set({ token: event.data.token }, () => { console.log('Token saved!', event.data.token); }); + } else if (event.data.type === 'Extension-Logout') { + chrome.storage.local.remove('token', () => { + console.log('Token removed!'); + }); } }); - -window.addEventListener('message', (event) => { - if (event.source !== window) return; - if (event.data.type === 'Extension-Logout') { - chrome.storage.local.remove('token', () => { - console.log('Token removed!'); - }); - } -});
Comment on lines
+49
to
52
| // localStorage.removeItem('token'); | ||
| window.location.href = '/onboarding?step=SOCIAL_LOGIN'; | ||
|
|
||
| return Promise.reject(error); |
There was a problem hiding this comment.
401/403 에러 시 토큰 제거 로직 비활성화에 대한 검토가 필요합니다.
인증 에러(401/403) 발생 시 토큰을 제거하지 않고 온보딩 페이지로 리다이렉트만 하는 것은 다음과 같은 문제를 야기할 수 있습니다:
- 무효한 토큰 잔존: 만료되거나 유효하지 않은 토큰이 localStorage에 남아 후속 요청에서 반복적으로 401/403 에러를 발생시킬 수 있습니다.
- 인증 루프 가능성: 리다이렉트 후에도 토큰이 남아있어 의도치 않은 동작을 유발할 수 있습니다.
- 상태 불일치: 서버에서는 토큰이 유효하지 않다고 판단했지만, 클라이언트에는 토큰이 남아있는 불일치 상태가 됩니다.
이 변경의 의도가 있다면 주석으로 사유를 명시하거나, 토큰 제거 로직을 유지하는 것을 권장합니다.
🔒 토큰 제거 로직 복원 제안
- // localStorage.removeItem('token');
+ localStorage.removeItem('token');
window.location.href = '/onboarding?step=SOCIAL_LOGIN';🤖 Prompt for AI Agents
In `@apps/client/src/shared/apis/setting/axiosInstance.ts` around lines 49 - 52,
On 401/403 handling in the axios error interceptor (the block that currently
calls window.location.href = '/onboarding?step=SOCIAL_LOGIN' and returns
Promise.reject(error)), restore the token removal logic by removing the stored
auth token (undo the commented out localStorage.removeItem('token')) before
redirecting; if the removal was intentionally disabled, add a clear comment
above this block explaining why the token must persist and what other mechanism
will clear it, referencing the axios error interceptor where
window.location.href and Promise.reject(error) are used so reviewers can find
the change.
Comment on lines
+24
to
+30
| chrome.runtime.onMessage.addListener((message) => { | ||
| if (message.type === 'Extension-Logout') { | ||
| chrome.storage.local.remove('token', () => { | ||
| console.log('Token removed!'); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "Extension-Logout" --type ts -C 5Repository: Pinback-Team/pinback-client
Length of output: 2307
background.ts의 Extension-Logout 리스너는 실제로 호출되지 않습니다.
ProfilePopup.tsx에서 window.postMessage로 'Extension-Logout' 메시지를 전송하고, content.ts의 window.addEventListener('message') 리스너가 이를 받아 직접 처리합니다. 따라서 chrome.runtime.onMessage를 사용하는 background.ts의 리스너(lines 24-30)는 절대 호출되지 않습니다.
이 리스너는 제거하거나, content.ts의 처리 로직을 background.ts로 이동하면서 chrome.runtime.sendMessage를 사용하도록 메시지 패싱 방식을 통일해야 합니다.
🧰 Tools
🪛 GitHub Check: lint
[warning] 27-27:
Unexpected console statement
🤖 Prompt for AI Agents
In `@apps/extension/src/background.ts` around lines 24 - 30, The background.ts
chrome.runtime.onMessage listener for 'Extension-Logout' is never called because
ProfilePopup.tsx uses window.postMessage and content.ts handles it via
window.addEventListener('message'); either remove the unused
chrome.runtime.onMessage handler from background.ts, or change messaging to
centralize logout in background.ts by updating ProfilePopup.tsx (or content.ts)
to call chrome.runtime.sendMessage({ type: 'Extension-Logout' }) and ensure
content.ts forwards the window message to the background with
chrome.runtime.sendMessage; then keep the existing chrome.runtime.onMessage
handler in background.ts to remove the token from chrome.storage.local.
Comment on lines
+14
to
+21
| window.addEventListener('message', (event) => { | ||
| if (event.source !== window) return; | ||
| if (event.data.type === 'Extension-Logout') { | ||
| chrome.storage.local.remove('token', () => { | ||
| console.log('Token removed!'); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
SET_TOKEN 흐름과 일관성이 없습니다.
SET_TOKEN은 chrome.runtime.sendMessage로 background에 전달하고 로컬에도 저장하지만, Extension-Logout은 로컬에서만 토큰을 제거합니다. background.ts에 Extension-Logout 리스너가 추가되었지만 이 메시지가 전달되지 않아 해당 리스너는 dead code가 됩니다.
일관성을 위해 background에도 메시지를 전달하거나, background.ts의 해당 리스너를 제거하는 것이 좋습니다.
🔧 background로 메시지 전달 추가
window.addEventListener('message', (event) => {
if (event.source !== window) return;
if (event.data.type === 'Extension-Logout') {
+ chrome.runtime.sendMessage({ type: 'Extension-Logout' });
chrome.storage.local.remove('token', () => {
console.log('Token removed!');
});
}
});📝 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
| window.addEventListener('message', (event) => { | |
| if (event.source !== window) return; | |
| if (event.data.type === 'Extension-Logout') { | |
| chrome.storage.local.remove('token', () => { | |
| console.log('Token removed!'); | |
| }); | |
| } | |
| }); | |
| window.addEventListener('message', (event) => { | |
| if (event.source !== window) return; | |
| if (event.data.type === 'Extension-Logout') { | |
| chrome.runtime.sendMessage({ type: 'Extension-Logout' }); | |
| chrome.storage.local.remove('token', () => { | |
| console.log('Token removed!'); | |
| }); | |
| } | |
| }); |
🧰 Tools
🪛 GitHub Check: lint
[warning] 18-18:
Unexpected console statement
🤖 Prompt for AI Agents
In `@apps/extension/src/content.ts` around lines 14 - 21, The logout handler in
the window message listener only removes the token from chrome.storage.local and
never notifies the background script, leaving the background 'Extension-Logout'
listener unused; update the handler in content.ts (the window.addEventListener
callback that checks event.data.type === 'Extension-Logout') to also call
chrome.runtime.sendMessage({ type: 'Extension-Logout' }) (or include any
required payload) after removing the token so the background.ts listener
receives the logout event and can perform its cleanup.
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
2 participants
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.
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
📷 Screenshot