Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbit릴리스 노트
Walkthrough네이버 OAuth 엔드투엔드 구현(로그인·콜백·유틸·마이그레이션)과 E2E 대기 로직을 요소 가시성 기반으로 교체, 두 스케줄 컴포넌트의 드래그 상태를 state에서 ref로 전환하여 이벤트/글로벌 핸들러 정리. Changes테스트 안정화 및 상태 관리 리팩토링
네이버 OAuth 및 DB 마이그레이션
Sequence DiagramsequenceDiagram
participant Client
participant Naver
participant AppServer
participant SupabaseAdmin
participant SupabaseClient
Client->>AppServer: GET /api/auth/naver/login -> state cookie, redirect to Naver
Client->>Naver: authorize -> callback(code,state)
Naver->>AppServer: /api/auth/naver/callback?code=...
AppServer->>Naver: getNaverToken(code) -> access_token
AppServer->>Naver: getNaverUser(access_token) -> userInfo
AppServer->>SupabaseAdmin: query/update profiles by naver_id or email
AppServer->>SupabaseAdmin: create user if missing (Admin auth)
AppServer->>SupabaseAdmin: generateLink -> hashed_token
AppServer->>SupabaseClient: verifyOtp(hashed_token) -> set session cookie
AppServer->>Client: redirect to next page, set last_login_provider, clear state cookie
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Why: 네이버 OAuth는 서버-외부API-DB-클라이언트가 연결되는 새 기능이며, E2E는 타이밍 불안정으로 flaky해 고정 대기를 제거해야 함. 필수 코드 스니펫 (핵심 패턴 예시 — 방어적 검사): // naver token 방어적 검사
export async function getNaverToken(code: string, state: string) {
if (!code || !state) throw new Error("missing code or state");
const params = new URLSearchParams({
grant_type: "authorization_code",
client_id: process.env.NAVER_CLIENT_ID!,
client_secret: process.env.NAVER_CLIENT_SECRET!,
code,
state,
redirect_uri: process.env.NAVER_REDIRECT_URI!
});
const res = await fetch("https://nid.naver.com/oauth2.0/token", {
method: "POST",
headers: { "Content-Type": "application/x-www-form-urlencoded" },
body: params.toString()
});
if (!res.ok) throw new Error(`token fetch failed: ${res.status}`);
const data = await res.json();
if (data.error || !data.access_token) throw new Error("invalid token response");
return data as NaverTokenResponse;
}검토시 중점: 콜백의 race/error 분기, 쿠키 보안 속성(secure 환경 의존), 마이그레이션 후 기존 레코드 대응, E2E의 요소 기반 동기화가 flaky 케이스를 완전히 덮는지(타이밍 경계). 🚥 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/host-flow.spec.ts (1)
16-16: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win남아있는 고정 지연을 요소 가시성 검증으로 추가 개선 가능.
PR 목표가 "고정 지연 대신 요소 가시성 검증"인데, 여전히 다수의
waitForTimeout사용 중:
- Line 16: 회원가입 페이지 Hydration 대기 2초
- Line 45: 리다이렉션 안정 대기 2초
- Line 54: 로그인 페이지 Hydration 대기 2초
- Line 71: 스케줄 생성 페이지 Hydration 대기 3초
각 화면의 특정 요소(예: 폼 제출 버튼, 페이지 제목 등)가 visible 상태가 되는 것을 기다리면 더 안정적이고 빠른 테스트 가능.
♻️ 개선 제안 (선택적)
예를 들어 Line 71의 경우:
- // 페이지 컴파일 및 Hydration 안정을 위해 대기 - await page.waitForTimeout(3000); - const titleInput = page.getByLabel("모임 제목"); await titleInput.waitFor({ state: "visible", timeout: 15000 });이미
titleInput.waitFor가 있으므로 Line 71의 고정 3초 대기는 중복.Also applies to: 45-45, 54-54, 71-71
🤖 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 `@e2e/host-flow.spec.ts` at line 16, 파일의 고정 지연(await page.waitForTimeout(...))을 제거하고 각 화면별로 페이지가 완전히 렌더링됐는지 확인하는 요소 가시성 검사로 대체하세요: 회원가입에서는 가입 폼의 제출 버튼(또는 페이지 제목) 가시성, 리다이렉션 후에는 리다이렉션 목적 요소의 가시성, 로그인에서는 로그인 폼 버튼/타이틀 가시성, 스케줄 생성에서는 이미 사용 중인 titleInput.waitFor 또는 스케줄 제출 버튼의 visible 확인으로 2s/3s 고정을 대체하도록 수정하세요; 즉 e2e/host-flow.spec.ts 내의 모든 waitForTimeout 호출을 해당 요소의 waitForSelector / elementHandle.waitFor({ state: 'visible' }) 등으로 바꿔 중복 대기(titleInput.waitFor와 같은)는 제거하세요.
🤖 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 `@e2e/host-flow.spec.ts`:
- Around line 30-36: The password field locators (pwInput, pwConfirmInput, and
the loginPwInput) lack a visible-state wait which can cause flaky fills; update
e2e/host-flow.spec.ts to call waitFor({ state: "visible", timeout: 10000 }) on
page.locator("`#password`") and page.locator("`#passwordConfirm`") (and the login
"`#password`" locator) before calling fill and toHaveValue so the elements are
reliably visible before interaction.
---
Outside diff comments:
In `@e2e/host-flow.spec.ts`:
- Line 16: 파일의 고정 지연(await page.waitForTimeout(...))을 제거하고 각 화면별로 페이지가 완전히
렌더링됐는지 확인하는 요소 가시성 검사로 대체하세요: 회원가입에서는 가입 폼의 제출 버튼(또는 페이지 제목) 가시성, 리다이렉션 후에는
리다이렉션 목적 요소의 가시성, 로그인에서는 로그인 폼 버튼/타이틀 가시성, 스케줄 생성에서는 이미 사용 중인 titleInput.waitFor
또는 스케줄 제출 버튼의 visible 확인으로 2s/3s 고정을 대체하도록 수정하세요; 즉 e2e/host-flow.spec.ts 내의 모든
waitForTimeout 호출을 해당 요소의 waitForSelector / elementHandle.waitFor({ state:
'visible' }) 등으로 바꿔 중복 대기(titleInput.waitFor와 같은)는 제거하세요.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 3a32c564-ad9e-4312-8935-ee724fe1b4bf
📒 Files selected for processing (4)
e2e/host-flow.spec.tse2e/participant-flow.spec.tssrc/app/schedule/[id]/ScheduleRoomClient.tsxsrc/app/schedule/create/CreateScheduleClient.tsx
* feat(auth): 네이버 로그인 Supabase Admin 브리지 구현 네이버는 Supabase가 기본 지원하지 않는 OAuth 제공자라, #45 마이그레이션 이후 /api/auth/naver/* 가 naver_not_implemented 스텁으로 비활성 상태였다. - 네이버 OAuth 헬퍼(getNaverAuthUrl/getNaverToken/getNaverUser) 복원 (자체 JWT 의존 제거, fetch-with-timeout 기반) - naver/login: authorize 리다이렉트 + state 쿠키 복원 - naver/callback: 네이버 프로필 수신 → Supabase Admin(service_role)으로 유저 생성/조회 → magiclink 토큰을 verifyOtp로 교환해 일반 Supabase 세션 쿠키 발급. 카카오/구글/애플과 동일하게 getUser() 세션으로 일원화. - 이메일 미제공 시 결정적 placeholder로 동일 사용자 식별, 닉네임 unique 충돌 시 naverId 기반으로 대체. Refs #46 Part of #58 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(auth): 코드래빗 리뷰 반영 — 네이버 식별자/닉네임 보강 - naver_id를 profiles 1차 식별자로 사용 (이메일 단독 조회 시 같은 네이버 계정이 갈라지는 문제 해결). profiles.naver_id 컬럼+unique 인덱스 추가하고 handle_new_user 트리거가 user_metadata.naver_id를 채우도록 갱신. - 콜백: naver_id로 먼저 조회, 미존재 시 이메일로 매칭 후 naver_id 백필. - extractNaverUserInfo: 빈 문자열/공백 닉네임·이메일을 "값 없음"으로 취급해 name → naver_<id> fallback이 정상 동작하도록 수정. Refs #46 Part of #58 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(auth): 코드래빗 2차 리뷰 반영 — naver_id 백필 가드/정규화/보안 - 콜백: 이메일 매칭 백필 시 byEmail.naver_id가 비어있을 때만 갱신해 기존 네이버 연결을 덮어쓰지 않도록 가드. - 마이그레이션: naver_id 빈 문자열/공백을 NULL로 정규화(트리거 nullif(btrim(...))), unique 인덱스 predicate도 동일 정규화로 일치. - handle_new_user에 SET search_path = public 추가 (SECURITY DEFINER 함수의 search_path 고정). Refs #46 Part of #58 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(auth): 네이버 동시요청 race 시 기존 사용자 추가정보 오라우팅 수정 createUser가 "already exists"로 실패하는 동시요청 race에서는 기존 사용자인데도 profileComplete를 false로 고정해, 추가정보 입력 페이지로 잘못 보내던 문제를 수정. race 감지 시 naver_id(없으면 이메일)로 실제 프로필 상태를 재조회해 isNewUser=false + 정확한 profileComplete로 분기. Refs #46 Part of #58 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
e2e/host-flow.spec.ts (2)
71-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win텍스트 가시성 검증에 exact 매칭 적용 필요
가이드라인에서 "텍스트 exact 검증 적용"을 요구하고 있으나,
getByText()호출 시exact: true옵션이 누락되었습니다. 기본 substring 매칭은 부분 일치로 인한 오탐(예: "내 일정을 연동해 볼까요? (로딩중...)" 같은 텍스트도 매칭)을 허용할 수 있습니다.Why: exact 매칭으로 UI 상태를 더 정밀하게 검증하여 false positive를 방지합니다.
How:
await page.getByRole("button", { name: "다음 단계로 →" }).click(); - await expect(page.getByText("내 일정을 연동해 볼까요?")).toBeVisible({ + await expect(page.getByText("내 일정을 연동해 볼까요?", { exact: true })).toBeVisible({ timeout: 5000, }); // Step 2 -> Step 3 이동 await page.getByRole("button", { name: "다음 단계로 →" }).click(); - await expect(page.getByText("후보 시간대 설정")).toBeVisible({ + await expect(page.getByText("조율할 시간 범위를 정해주세요", { exact: true })).toBeVisible({ timeout: 5000, });Also applies to: 77-79
🤖 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 `@e2e/host-flow.spec.ts` around lines 71 - 73, The getByText assertions are using substring matching by default; update the calls to page.getByText to pass the exact: true option (e.g., page.getByText("내 일정을 연동해 볼까요?", { exact: true })) so the test verifies exact text and avoids false positives; apply the same change to the other occurrence referenced (the call around lines 77-79) where getByText is used.Source: Coding guidelines
77-79:⚠️ Potential issue | 🔴 CriticalStep 3 안내 텍스트 기대값이 실제 UI와 불일치합니다 (e2e 실패 원인)
- Why:
CreateScheduleClient.tsx에서step === 3의 h1은"조율할 시간 범위를 정해주세요"인데,e2e/host-flow.spec.ts는"후보 시간대 설정"을 찾고 있어 5초 타임아웃으로 실패합니다.- How: 기대 텍스트를 UI와 동일하게 변경
수정 diff
await page.getByRole("button", { name: "다음 단계로 →" }).click(); - await expect(page.getByText("후보 시간대 설정")).toBeVisible({ + await expect(page.getByText("조율할 시간 범위를 정해주세요")).toBeVisible({ timeout: 5000, });
- Why:
getByText()는 기본exact: false라 부분 문자열 매칭으로 오탐 가능성이 있습니다.- How: 동일 텍스트에 대해
exact: true적용await expect( page.getByText("조율할 시간 범위를 정해주세요", { exact: true }) ).toBeVisible({ timeout: 5000 });🤖 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 `@e2e/host-flow.spec.ts` around lines 77 - 79, Update the expectation in the e2e test to match the actual h1 text rendered when step === 3 in CreateScheduleClient.tsx: replace the "후보 시간대 설정" assertion in e2e/host-flow.spec.ts with a getByText lookup for "조율할 시간 범위를 정해주세요" and set the query option exact: true (keep the existing toBeVisible({ timeout: 5000 }) call).
🤖 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/app/api/auth/naver/callback/route.ts`:
- Around line 31-35: The placeholder email created by resolveEmail should use an
RFC‑reserved non‑resolvable domain to avoid potential real domain collisions;
update the resolveEmail function to return a deterministic placeholder like
`naver_{naverId}`@naver.invalid`` (preserving trimming/lowercasing behavior when
email is present) so the fallback address uses the `.invalid` TLD instead of
`.local`.
- Around line 128-141: The email lookup used to populate racedProfile ignores
errors — after the first admin.from("profiles").select(...).eq("naver_id",
naverId).maybeSingle() you fall back to another maybeSingle() for the email but
access only .data without checking .error; update the fallback to capture the
full result (e.g., const emailLookup = await
admin.from("profiles").select("phone_number, terms_agreed_at").eq("email",
authEmail).maybeSingle()), check emailLookup.error and handle or log it (or
throw) before using emailLookup.data to build racedProfile, so racedProfile
isn’t incorrectly treated as undefined on DB errors.
In `@src/app/api/auth/naver/login/route.ts`:
- Around line 6-7: STATE_COOKIE and STATE_MAX_AGE are duplicated in
login/route.ts and callback/route.ts causing potential mismatches; move these
constants into a single exported source in "`@/lib/auth/naver.ts`" (e.g., export
const STATE_COOKIE and STATE_MAX_AGE) and update both login/route.ts and
callback/route.ts to import and use those exported symbols instead of hardcoding
them so both routes share the exact same values for state cookie name and TTL.
In `@src/lib/auth/naver.ts`:
- Around line 53-62: The guard calls in getNaverToken currently call code.trim()
and state.trim() directly which throws if callers pass null/undefined; update
the checks in getNaverToken to defensively handle nullish inputs (e.g., use
optional chaining like code?.trim() and state?.trim() or explicit null/undefined
checks before trimming) and throw the same error messages when the normalized
values are empty; ensure you only change the validation logic inside the
getNaverToken function so behavior and error messages remain consistent.
- Around line 106-114: The getNaverUser function must validate the
naverAccessToken for empty or whitespace-only values before calling
fetchWithTimeout; add a fail-fast check in getNaverUser (e.g., trim() the token
and if falsy) and throw a clear error or return a rejected Promise indicating
"invalid or empty Naver access token" so the function never makes the API
request with an empty token; update any callers/tests if they rely on the
previous behavior.
In `@supabase/migrations/20260611000000_profiles_naver_id.sql`:
- Around line 21-57: The migration currently uses "CREATE OR REPLACE FUNCTION
public.handle_new_user()" which overwrites the previous trigger function without
providing a rollback path; add a down migration or include explicit rollback SQL
comments that restore the previous state (drop any created index/column like
profiles_naver_id_key and remove the naver_id column, then re-create the prior
handle_new_user definition from the earlier migration), and ensure the rollback
references the same function name handle_new_user so the previous function body
can be reapplied if needed.
---
Outside diff comments:
In `@e2e/host-flow.spec.ts`:
- Around line 71-73: The getByText assertions are using substring matching by
default; update the calls to page.getByText to pass the exact: true option
(e.g., page.getByText("내 일정을 연동해 볼까요?", { exact: true })) so the test verifies
exact text and avoids false positives; apply the same change to the other
occurrence referenced (the call around lines 77-79) where getByText is used.
- Around line 77-79: Update the expectation in the e2e test to match the actual
h1 text rendered when step === 3 in CreateScheduleClient.tsx: replace the "후보
시간대 설정" assertion in e2e/host-flow.spec.ts with a getByText lookup for "조율할 시간
범위를 정해주세요" and set the query option exact: true (keep the existing toBeVisible({
timeout: 5000 }) call).
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 880e91fa-21f3-41ff-b65f-189372f1c379
📒 Files selected for processing (5)
e2e/host-flow.spec.tssrc/app/api/auth/naver/callback/route.tssrc/app/api/auth/naver/login/route.tssrc/lib/auth/naver.tssupabase/migrations/20260611000000_profiles_naver_id.sql
Comment on lines
+31
to
+35
| /** 네이버 이메일이 없을 때도 동일 사용자로 식별되도록 결정적 placeholder를 만든다. */ | ||
| function resolveEmail(naverId: string, email?: string): string { | ||
| if (email && email.trim()) return email.trim().toLowerCase(); | ||
| return `naver_${naverId}@naver.social.local`; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
resolveEmail에서 placeholder 이메일 도메인이 실제 존재할 수 있는 형태
naver.social.local은 .local TLD라 실제로는 충돌 가능성이 낮지만, 명시적으로 무효한 도메인(예: naver.invalid)을 사용하면 RFC 2606 준수로 더 안전하다.
🤖 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 `@src/app/api/auth/naver/callback/route.ts` around lines 31 - 35, The
placeholder email created by resolveEmail should use an RFC‑reserved
non‑resolvable domain to avoid potential real domain collisions; update the
resolveEmail function to return a deterministic placeholder like
`naver_{naverId}`@naver.invalid`` (preserving trimming/lowercasing behavior when
email is present) so the fallback address uses the `.invalid` TLD instead of
`.local`.
Comment on lines
+128
to
+141
| const { data: raced } = await admin | ||
| .from("profiles") | ||
| .select("phone_number, terms_agreed_at") | ||
| .eq("naver_id", naverId) | ||
| .maybeSingle(); | ||
| const racedProfile = | ||
| raced ?? | ||
| ( | ||
| await admin | ||
| .from("profiles") | ||
| .select("phone_number, terms_agreed_at") | ||
| .eq("email", authEmail) | ||
| .maybeSingle() | ||
| ).data; |
There was a problem hiding this comment.
race 처리 시 이메일 조회 에러가 무시됨
naver_id 조회 후 email 조회 시 .data만 접근하고 .error를 체크하지 않는다. DB 오류가 발생해도 racedProfile이 undefined로 처리되어 신규 사용자로 오판될 수 있다.
Why: Supabase 쿼리 실패 시 데이터 무결성 문제 발생 가능.
How: 에러 체크 추가 또는 최소한 로그 남기기.
🐛 제안 수정
const { data: raced } = await admin
.from("profiles")
.select("phone_number, terms_agreed_at")
.eq("naver_id", naverId)
.maybeSingle();
- const racedProfile =
- raced ??
- (
- await admin
- .from("profiles")
- .select("phone_number, terms_agreed_at")
- .eq("email", authEmail)
- .maybeSingle()
- ).data;
+ let racedProfile = raced;
+ if (!racedProfile) {
+ const { data: byEmailRaced, error: byEmailRacedError } = await admin
+ .from("profiles")
+ .select("phone_number, terms_agreed_at")
+ .eq("email", authEmail)
+ .maybeSingle();
+ if (byEmailRacedError) {
+ console.error("[auth.naver.callback] race 조회 실패:", byEmailRacedError.message);
+ }
+ racedProfile = byEmailRaced;
+ }🤖 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 `@src/app/api/auth/naver/callback/route.ts` around lines 128 - 141, The email
lookup used to populate racedProfile ignores errors — after the first
admin.from("profiles").select(...).eq("naver_id", naverId).maybeSingle() you
fall back to another maybeSingle() for the email but access only .data without
checking .error; update the fallback to capture the full result (e.g., const
emailLookup = await admin.from("profiles").select("phone_number,
terms_agreed_at").eq("email", authEmail).maybeSingle()), check emailLookup.error
and handle or log it (or throw) before using emailLookup.data to build
racedProfile, so racedProfile isn’t incorrectly treated as undefined on DB
errors.
Comment on lines
+6
to
+7
| const STATE_COOKIE = "naver_oauth_state"; | ||
| const STATE_MAX_AGE = 600; // 10분 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
STATE_COOKIE 상수 중복 정의 — 단일 소스로 통합 필요
login/route.ts와 callback/route.ts가 동일한 쿠키 이름을 각각 하드코딩한다. 한쪽만 수정하면 OAuth state 검증이 실패하여 CSRF 방어가 무력화된다. @/lib/auth/naver.ts에 공통 상수로 export하고 양쪽에서 import하라.
🤖 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 `@src/app/api/auth/naver/login/route.ts` around lines 6 - 7, STATE_COOKIE and
STATE_MAX_AGE are duplicated in login/route.ts and callback/route.ts causing
potential mismatches; move these constants into a single exported source in
"`@/lib/auth/naver.ts`" (e.g., export const STATE_COOKIE and STATE_MAX_AGE) and
update both login/route.ts and callback/route.ts to import and use those
exported symbols instead of hardcoding them so both routes share the exact same
values for state cookie name and TTL.
Comment on lines
+53
to
+62
| export async function getNaverToken( | ||
| code: string, | ||
| state: string, | ||
| ): Promise<NaverTokenResponse> { | ||
| if (!code.trim()) { | ||
| throw new Error("Naver authorization code is empty."); | ||
| } | ||
| if (!state.trim()) { | ||
| throw new Error("Naver OAuth state is empty."); | ||
| } |
There was a problem hiding this comment.
code/state가 null일 때 .trim() 호출 시 런타임 에러 발생
getNaverAuthUrl은 state?.trim()으로 optional chaining을 사용하지만, 여기서는 code.trim()을 바로 호출한다. 호출자가 null을 넘기면 TypeError가 발생한다.
Why: 함수 시그니처가 string이지만 TypeScript는 런타임에 강제하지 않는다. 방어적 코딩 필요.
How: optional chaining 또는 명시적 null 체크 추가.
🛡️ 제안 수정
export async function getNaverToken(
code: string,
state: string,
): Promise<NaverTokenResponse> {
- if (!code.trim()) {
+ if (!code?.trim()) {
throw new Error("Naver authorization code is empty.");
}
- if (!state.trim()) {
+ if (!state?.trim()) {
throw new Error("Naver OAuth state is empty.");
}🤖 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 `@src/lib/auth/naver.ts` around lines 53 - 62, The guard calls in getNaverToken
currently call code.trim() and state.trim() directly which throws if callers
pass null/undefined; update the checks in getNaverToken to defensively handle
nullish inputs (e.g., use optional chaining like code?.trim() and state?.trim()
or explicit null/undefined checks before trimming) and throw the same error
messages when the normalized values are empty; ensure you only change the
validation logic inside the getNaverToken function so behavior and error
messages remain consistent.
Comment on lines
+106
to
+114
| export async function getNaverUser( | ||
| naverAccessToken: string, | ||
| ): Promise<NaverUser> { | ||
| const res = await fetchWithTimeout("https://openapi.naver.com/v1/nid/me", { | ||
| method: "GET", | ||
| headers: { | ||
| Authorization: `Bearer ${naverAccessToken}`, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
naverAccessToken 빈 문자열/공백 검증 누락
getNaverToken, getNaverAuthUrl은 입력 검증을 수행하지만, 이 함수는 토큰이 빈 문자열이어도 그대로 API 호출을 시도한다. 네이버 API가 401을 반환하겠지만, 명시적 검증으로 빠른 실패(fail-fast)가 낫다.
🛡️ 제안 수정
export async function getNaverUser(
naverAccessToken: string,
): Promise<NaverUser> {
+ if (!naverAccessToken?.trim()) {
+ throw new Error("Naver access token is empty.");
+ }
const res = await fetchWithTimeout("https://openapi.naver.com/v1/nid/me", {🤖 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 `@src/lib/auth/naver.ts` around lines 106 - 114, The getNaverUser function must
validate the naverAccessToken for empty or whitespace-only values before calling
fetchWithTimeout; add a fail-fast check in getNaverUser (e.g., trim() the token
and if falsy) and throw a clear error or return a rejected Promise indicating
"invalid or empty Naver access token" so the function never makes the API
request with an empty token; update any callers/tests if they rely on the
previous behavior.
Comment on lines
+21
to
+57
| create or replace function public.handle_new_user() | ||
| returns trigger as $$ | ||
| begin | ||
| insert into public.profiles ( | ||
| id, | ||
| email, | ||
| nickname, | ||
| avatar_url, | ||
| phone_number, | ||
| is_age_over_14, | ||
| terms_agreed_at, | ||
| privacy_agreed_at, | ||
| marketing_agreed, | ||
| event_sms_agreed, | ||
| naver_id | ||
| ) | ||
| values ( | ||
| new.id, | ||
| new.email, | ||
| coalesce( | ||
| new.raw_user_meta_data->>'nickname', | ||
| new.raw_user_meta_data->>'full_name', | ||
| new.raw_user_meta_data->>'name', | ||
| new.email | ||
| ), | ||
| new.raw_user_meta_data->>'avatar_url', | ||
| new.raw_user_meta_data->>'phone_number', | ||
| (new.raw_user_meta_data->>'is_age_over_14')::boolean, | ||
| (new.raw_user_meta_data->>'terms_agreed_at')::timestamptz, | ||
| (new.raw_user_meta_data->>'privacy_agreed_at')::timestamptz, | ||
| coalesce((new.raw_user_meta_data->>'marketing_agreed')::boolean, false), | ||
| coalesce((new.raw_user_meta_data->>'event_sms_agreed')::boolean, false), | ||
| nullif(btrim(new.raw_user_meta_data->>'naver_id'), '') | ||
| ); | ||
| return new; | ||
| end; | ||
| $$ language plpgsql security definer set search_path = public; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
handle_new_user 함수 교체 시 롤백 전략 부재
CREATE OR REPLACE로 트리거 함수를 덮어쓰면 롤백 시 이전 버전(naver_id 없는 버전)을 수동 복구해야 한다. Supabase 마이그레이션은 down 스크립트를 자동 생성하지 않는다.
Why: 배포 실패 시 롤백이 복잡해진다.
How: 별도 down 마이그레이션 파일 작성 또는 주석으로 롤백 SQL 명시.
📝 롤백 SQL 예시 (주석으로 추가)
-- ROLLBACK:
-- drop index if exists profiles_naver_id_key;
-- alter table public.profiles drop column if exists naver_id;
-- 이후 이전 handle_new_user 함수 재정의 필요 (20260607000000_profiles_auth_fields.sql 참조)🤖 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 `@supabase/migrations/20260611000000_profiles_naver_id.sql` around lines 21 -
57, The migration currently uses "CREATE OR REPLACE FUNCTION
public.handle_new_user()" which overwrites the previous trigger function without
providing a rollback path; add a down migration or include explicit rollback SQL
comments that restore the previous state (drop any created index/column like
profiles_naver_id_key and remove the naver_id column, then re-create the prior
handle_new_user definition from the earlier migration), and ensure the rollback
references the same function name handle_new_user so the previous function body
can be reapplied if needed.
4 tasks
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.
[PR] 코드레빗 리뷰 반영 — 터치 드래그 스크롤 방지 개선 및 E2E 안정화
📌 개요
이 PR은 이전 통합 PR(#56) 머지 이후 코드레빗(CodeRabbit)이 지적한 개선 사항들을 반영한 후속 refactoring 작업을 포함합니다.
터치 드래그 스크롤 방지 로직의 레이스 컨디션을 해결하고, 불필요한 이벤트 리스너 제거 및 E2E 테스트의 Flaky 요소를 수정하였습니다.
🛠️ 주요 반영 사항
1. 터치 드래그 스크롤 방지 레이스 컨디션 해결
touchstart시점의 비동기isDragging상태 업데이트로 인해, 첫touchmove이벤트가 발생할 때preventDefault()가 적용되지 않아 모바일 스크롤이 트리거되는 문제가 있었습니다.isDraggingRef(useRef)를 도입하여 드래그 상태를 동기적으로 추적하고,touchmove리스너를 빈 의존성 배열([])로 한 번만 등록하도록 하여 stale closure 레이스 컨디션을 원천 해결했습니다.src/app/schedule/[id]/ScheduleRoomClient.tsxsrc/app/schedule/create/CreateScheduleClient.tsx2. 중복 글로벌 마우스 업 이벤트 리스너 제거
CreateScheduleClient.tsx에 중복으로 등록되어 있던mouseup글로벌 윈도우 리스너(handleGlobalMouseUp)를 제거하여 불필요한 이벤트 낭비를 줄였습니다.3. ESLint 미사용 변수 정리
isDragging상태를useRef동기식 상태로 전환하면서 컴포넌트 내부에서 사용되지 않게 된isDraggingstate와setIsDragging호출문을 전면 제거하여 코드 품질을 향상시켰습니다.4. E2E 테스트 코드 Flakiness 개선
e2e/host-flow.spec.ts:waitForTimeout대기 대신 각 Wizard Step 전환 시 고유 텍스트가 노출되는지toBeVisible로 명시적 검증하도록 변경했습니다.#password,#passwordConfirm)로 수정하였습니다.e2e/participant-flow.spec.ts:15000ms로 넉넉하게 확장하여 테스트 신뢰도를 높였습니다.✅ 체크리스트