Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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. 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: 4
🧹 Nitpick comments (2)
src/app/sitemap.ts (1)
4-4: Avoid duplicating/api/v1at call sites.You already centralize this in
publicApiBaseUrl; keeping version prefix in one place prevents silent drift.♻️ Proposed refactor
-import { getPublicApiUrl, publicBaseUrl } from "@/shared/lib/public-env" +import { publicApiBaseUrl, publicBaseUrl } from "@/shared/lib/public-env" @@ - const response = await fetch(getPublicApiUrl(`/api/v1/ranking?page=${page}&size=20`), { + const response = await fetch(`${publicApiBaseUrl}/ranking?page=${page}&size=20`, {Also applies to: 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/sitemap.ts` at line 4, The sitemap code duplicates the "/api/v1" prefix at call sites instead of using the centralized base constant; update callers in src/app/sitemap.ts to stop appending "/api/v1" manually and use the existing publicApiBaseUrl (or derive it via getPublicApiUrl/publicBaseUrl) so the versioned API prefix is defined in one place — locate occurrences around the import line referencing getPublicApiUrl/publicBaseUrl and the call sites at line ~56 and replace concatenated "/api/v1" usages with the single publicApiBaseUrl symbol.src/features/user/components/badge-generator.tsx (1)
11-11: Prefer site URL helper over manual string concatenation.Using the shared URL builder keeps slash normalization/encoding consistent.
♻️ Proposed refactor
-import { getBadgeImageUrl, publicBaseUrl } from "@/shared/lib/public-env" +import { getBadgeImageUrl, getPublicSiteUrl } from "@/shared/lib/public-env" @@ - const baseUrl = publicBaseUrl const badgeUrl = getBadgeImageUrl(nodeId) - const profileUrl = `${baseUrl}${localizePathname(`/users/${username}`, locale)}` + const profilePath = localizePathname(`/users/${encodeURIComponent(username)}`, locale) + const profileUrl = getPublicSiteUrl(profilePath)Also applies to: 54-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/user/components/badge-generator.tsx` at line 11, Replace manual string concatenation of site URLs with the shared URL helper to ensure proper slash normalization and encoding; specifically, stop building badge/image URLs by concatenating publicBaseUrl with path/query and instead call getBadgeImageUrl(...) (or the project's site URL builder) wherever you currently construct `${publicBaseUrl}/...` (notably in the badge generation code and the occurrences around lines 54–56). Update the code in the badge-generator component to pass the path and query params into getBadgeImageUrl (or the shared site URL helper) and remove manual joins so all badge URLs are generated consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/users/`[username]/user-profile-client.tsx:
- Around line 84-91: handleCopyBadge currently hardcodes the production site URL
when building the markdown; replace that hardcoded string with the centralized
public-base URL helper used elsewhere (e.g., call the project's public env
helper like getPublicBaseUrl() or the exported publicUrl constant) and import it
into this file, then build the markdown as `[](${publicBaseUrl})`; update handleCopyBadge to use that
helper (with a sensible fallback such as window.location.origin or
NEXT_PUBLIC_BASE_URL) and ensure the helper is imported at the top.
In `@src/proxy.ts`:
- Around line 19-29: applySecurityHeaders currently unconditionally includes
'unsafe-eval' in the script-src CSP via joinCspSources, weakening production
security; change applySecurityHeaders so that 'unsafe-eval' is appended to the
script-src sources only when running in development (e.g. process.env.NODE_ENV
=== 'development' or a Next dev flag), otherwise omit it in production. Update
the logic that builds cspDirectives (and the callsite of joinCspSources for
script-src) to conditionally include "'unsafe-eval'" so production responses
never contain it.
In `@src/shared/lib/public-env.ts`:
- Around line 34-35: The getBadgeImageUrl function currently injects the raw
nodeId into the path which can break routes for base64-like IDs; update
getBadgeImageUrl to encode nodeId (e.g., with encodeURIComponent) before
interpolating it into the `/api/v1/badges/${...}` path so the resulting URL from
buildAbsoluteUrl(publicApiOrigin, ...) is safe for all ID values; keep
publicApiOrigin and buildAbsoluteUrl usage unchanged, only replace the raw
nodeId with its encoded form in getBadgeImageUrl.
- Around line 1-30: The env values are treated as raw strings but must be parsed
and normalized as URLs so any path component in NEXT_PUBLIC_API_URL or
NEXT_PUBLIC_BASE_URL is ignored when deriving origins and building API/oauth
URLs; update getRequiredPublicEnv (or introduce a new parsePublicUrl helper) to
validate the env exists, construct a URL object from the string, throw a clear
error on invalid URL, and return the URL.origin (no trailing slash) instead of
the raw string; then have publicBaseUrl/publicApiOrigin use that origin value
and keep buildAbsoluteUrl to append paths (use new URL(pathname, origin) or
existing buildAbsoluteUrl) to produce publicApiBaseUrl and githubOAuthStartUrl.
---
Nitpick comments:
In `@src/app/sitemap.ts`:
- Line 4: The sitemap code duplicates the "/api/v1" prefix at call sites instead
of using the centralized base constant; update callers in src/app/sitemap.ts to
stop appending "/api/v1" manually and use the existing publicApiBaseUrl (or
derive it via getPublicApiUrl/publicBaseUrl) so the versioned API prefix is
defined in one place — locate occurrences around the import line referencing
getPublicApiUrl/publicBaseUrl and the call sites at line ~56 and replace
concatenated "/api/v1" usages with the single publicApiBaseUrl symbol.
In `@src/features/user/components/badge-generator.tsx`:
- Line 11: Replace manual string concatenation of site URLs with the shared URL
helper to ensure proper slash normalization and encoding; specifically, stop
building badge/image URLs by concatenating publicBaseUrl with path/query and
instead call getBadgeImageUrl(...) (or the project's site URL builder) wherever
you currently construct `${publicBaseUrl}/...` (notably in the badge generation
code and the occurrences around lines 54–56). Update the code in the
badge-generator component to pass the path and query params into
getBadgeImageUrl (or the shared site URL helper) and remove manual joins so all
badge URLs are generated consistently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b73229a-0e9e-4b66-91a3-86db89ace6fa
⛔ Files ignored due to path filters (1)
src/fonts/JetBrainsMonoVariable.ttfis excluded by!**/*.ttf
📒 Files selected for processing (19)
.env.exampleDockerfileREADME.mddocker-compose.ymlsrc/app/layout.tsxsrc/app/login/layout.tsxsrc/app/login/page.tsxsrc/app/page.tsxsrc/app/ranking/layout.tsxsrc/app/robots.tssrc/app/sitemap.tssrc/app/users/[username]/opengraph-image.tsxsrc/app/users/[username]/page.tsxsrc/app/users/[username]/user-profile-client.tsxsrc/features/user/components/badge-generator.tsxsrc/fonts/JetBrainsMono-OFL.txtsrc/proxy.tssrc/shared/lib/api-client.tssrc/shared/lib/public-env.ts
|
Review feedback reflected in Applied changes:
Re-verified with |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 518d344f14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| try { | ||
| return new URL(value).origin |
There was a problem hiding this comment.
getRequiredPublicEnv currently returns new URL(value).origin, which silently strips any configured pathname from NEXT_PUBLIC_BASE_URL and NEXT_PUBLIC_API_URL. If deployment uses a path prefix (for example https://example.com/git-ranker or an API mounted under /backend), all derived URLs (publicBaseUrl, publicApiBaseUrl, OAuth start URL, sitemap/canonical links) are rebuilt without that prefix, leading to broken links and 404s only in those environments.
Useful? React with 👍 / 👎.
1 participant
1) 요약
JetBrains Mono를 공식 로컬 자산으로 번들링해next/font/google외부 fetch 의존성을 제거했습니다.middleware.ts를proxy.ts로 전환하고 Google Fonts 관련 CSP/preconnect를 정리했습니다.NEXT_PUBLIC_BASE_URL,NEXT_PUBLIC_API_URL를 공용 helper 기준 필수 env로 통일하고 README/.env.example을 갱신했습니다.2) 연관 이슈
3) 사용자 관점 결과
4) 영향 범위
src/app/layout.tsx, login/ranking/user metadata route,src/proxy.ts,src/shared/lib/public-env.ts, badge/login consumer5) 스크린샷 또는 영상
6) 검증 증거
npm run lintNEXT_PUBLIC_BASE_URL=http://localhost:3000 NEXT_PUBLIC_API_URL=http://localhost:8080 npm run buildnpx tsc --noEmit미구축(GRC-04 예정)npm run dev후HEAD /ko 200, local font preload 확인, 기존 env console error 미재현7) API / 백엔드 연동 확인
/api/v1/ranking,/api/v1/users/{username},/api/v1/auth/me,/api/v1/badges/{nodeId},/oauth2/authorization/github8) 리스크 및 롤백
NEXT_PUBLIC_BASE_URL,NEXT_PUBLIC_API_URL가 이제 필수이므로 누락된 환경에서는 즉시 실패합니다. 로컬 폰트 자산 교체로 타이포그래피가 미세하게 달라질 수 있습니다.proxy.ts,public-env.ts, local font 자산, README/.env.example 변경을 포함한 이번 커밋을 revert 하면 됩니다.9) 체크리스트
Summary by CodeRabbit
Documentation
Chores
Refactor