Feat/45#46
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 40 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
워크스루이 PR은 사용자가 QRScan 페이지에서 URL을 직접 입력하여 스캔하는 기능을 추가하고, Report 페이지에 분석 실패 알림과 V3 설치 버튼을 추가하며, 스캔 히스토리에서 세부 정보를 기반으로 위험 상태를 강화하는 기능을 구현합니다. 변경 사항텍스트 URL 스캔 및 보고서 향상
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/shared/store/scanSessionStore.ts (1)
82-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
pendingTextScanUrl도 persist/rehydrate 경로에 포함해 주세요.지금 구현은
setPendingTextScanUrl()로 텍스트 URL을 세션 상태에 넣지만,partialize()와mergePersistedLightSession()어디에서도 이 필드를 저장/복원하지 않습니다.src/pages/QRScan/hooks/useQRScanPage.ts:573-597는 이 값을 넣고/loading으로 이동하고,src/pages/Loading/hooks/useLoadingPage.ts:138-167는pendingTextScanUrl이 있을 때만submitScanUrl()을 호출합니다. 그래서 로딩 진입 전후에 새로고침이 발생하면decodedUrl만 복원되고 실제 제출 트리거는 사라져서 텍스트 URL 스캔이 재개되지 않습니다.🔧 제안 수정
function mergePersistedLightSession( persistedState: unknown, currentState: ScanSessionState, ): ScanSessionState { @@ return { ...currentState, decodedUrl: typeof persisted.decodedUrl === 'string' ? persisted.decodedUrl : null, historySelection: persisted.historySelection ?? null, isUrl: typeof persisted.isUrl === 'boolean' ? persisted.isUrl : null, + pendingTextScanUrl: + typeof persisted.pendingTextScanUrl === 'string' ? persisted.pendingTextScanUrl : null, riskLevel: persisted.riskLevel ?? null, schemeType: typeof persisted.schemeType === 'string' ? normalizeScanSchemeTypeAlias(persisted.schemeType) : null, @@ partialize: (state) => ({ decodedUrl: state.decodedUrl, historySelection: state.historySelection, isUrl: state.isUrl, + pendingTextScanUrl: state.pendingTextScanUrl, riskLevel: state.riskLevel, schemeType: state.schemeType, }),Also applies to: 156-163
🤖 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/shared/store/scanSessionStore.ts` around lines 82 - 103, The persisted snapshot is missing the pendingTextScanUrl field so the text-scan submit trigger is lost on rehydrate; update the persistence and merge logic to include pendingTextScanUrl: add pendingTextScanUrl to the list returned by partialize (so setPendingTextScanUrl is serialized) and restore it in mergePersistedLightSession by reading persisted.pendingTextScanUrl with a proper type check (string|null/undefined) and assigning it into the returned ScanSessionState; reference pendingTextScanUrl, partialize, mergePersistedLightSession, ScanSessionSnapshot and ScanSessionState when making these changes.src/pages/Loading/hooks/useLoadingPage.ts (1)
69-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
scanResponse갱신으로 진행 상태를 다시 초기화하면 안 됩니다.텍스트 URL 플로우에서는 SSE progress가
submitScanUrl()응답보다 먼저 올 수 있는데, 그 뒤 Line 70 조건이 다시 타면서resetProgress()/setConnecting()이 이미 표시 중인 진행 상태를"연결 중"으로 덮어씁니다.detailResolutionFailedRef와detailResolutionStartedRef도 같이 초기화돼서 terminal progress 이후의 상세 조회 상태까지 흔들릴 수 있습니다. 이 초기화는 새 스캔을 시작할 때만 실행되도록scanResponse변화와 분리하는 편이 안전합니다.🤖 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/pages/Loading/hooks/useLoadingPage.ts` around lines 69 - 83, The effect is wrongly resetting progress on any scanResponse update; split the logic so scanResponse changes do not trigger resetProgress/detailResolutionFailedRef/detailResolutionStartedRef/setConnecting. Keep the existing useEffect that depends on scanResponse to handle navigation and the isSameScanSource early return, but move the resetProgress/detailResolutionFailedRef.current = false/detailResolutionStartedRef.current = false and setConnecting() into a new useEffect that only depends on pendingTextScanUrl, finalResult (and the setters/resetProgress refs) and only runs when starting a new scan (e.g., when pendingTextScanUrl is truthy or finalResult transitions to a new scan), ensuring scanResponse is not in that dependency list so SSE progress updates won't overwrite the current progress UI.
🤖 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/features/scan-history/api/fetchScanHistoryData.ts`:
- Around line 125-127: The current code blocks fetchRecentScanHistoryData on
await Promise.all(...) when options.enrichRiskFromDetail is true; instead,
return the base historyItems immediately and kick off enrichment in the
background: don't await Promise.all for enrichHistoryItemRiskLevel, start an
async task (using Promise.allSettled) that maps enriched results back to history
item ids and then emits/updates via the same update path (e.g., call the
consumer callback or setScanHistoryItems) when enrichment completes; ensure you
reference options.enrichRiskFromDetail, enrichedHistoryItems, and
enrichHistoryItemRiskLevel and handle failures gracefully so a slow detail fetch
updates status later without blocking initial render.
In `@src/features/scan-url/api/submitScanUrl.ts`:
- Around line 10-16: submitScanUrl currently sends the user-provided URL in the
GET query (axiosBe3.get to apiEndpoints.scanText), which leaks sensitive tokens
to logs; change it to use a POST request (axiosBe3.post) sending { url } in the
request body instead of params, preserve the timeout by passing the same
getUploadTimeoutMs() option, keep the response generic type BackendScanResponse,
and update any calling code or backend contract if necessary to accept POST/body
for submitScanUrl.
In `@src/pages/QRScan/hooks/useQRScanPage.ts`:
- Around line 143-165: The normalizeUrlInput function currently prepends
"https://" to any input that doesn't start with http(s) which transforms
non-HTTP schemes like "ftp://..." into a different URL; change the logic to
first detect if the input already contains a scheme (e.g. using a scheme regex
like ^[a-zA-Z][\w+.-]*://) and if a scheme exists only accept it when
parsedUrl.protocol is 'http:' or 'https:' (otherwise return null), but if no
scheme exists then prepend 'https://' and continue parsing as before; keep error
handling in the try/catch and return null on parse failure so only true HTTP(S)
targets are normalized and accepted.
In `@src/pages/Report/constants/threatText.ts`:
- Around line 400-405: The CERT TIMEOUT entry (title '인증서 검증 시간 초과 감지',
englishLabel 'CERT TIMEOUT') and the separate CERTIFICATE REQUEST TIMEOUT entry
both register the same aliases (e.g., 'certificate_timeout',
'tls_certificate_timeout'), causing the later entry to shadow the earlier one
when building riskDetectionLookup; fix by consolidating these into a single
canonical entry or removing the duplicate aliases from one of them so each alias
appears exactly once (update the names arrays on the CERT TIMEOUT and/or
CERTIFICATE REQUEST TIMEOUT objects to remove duplication and ensure
riskDetectionLookup maps the alias to the intended description).
In `@src/pages/Report/ReportPage.tsx`:
- Around line 56-58: The V3 install button uses V3_INSTALL_URL and disables
based only on empty string, so if VITE_V3_INSTALL_URL is unsafe
openExternalLink() returns false and the button appears active but does nothing;
fix by validating the URL at load: run the same safety check used by
openExternalLink (or extract a helper like isSafeExternalUrl(url)) for
V3_INSTALL_URL, and if the check fails set V3_INSTALL_URL =
DEFAULT_V3_INSTALL_URL (or set a boolean isV3UrlSafe = false) and use that
boolean to disable the button; update both the V3_INSTALL_URL initialization and
the button disabled logic (references: V3_INSTALL_URL, DEFAULT_V3_INSTALL_URL,
openExternalLink, and the install button rendering) so unsafe overrides won’t
leave the button clickable but inert.
---
Outside diff comments:
In `@src/pages/Loading/hooks/useLoadingPage.ts`:
- Around line 69-83: The effect is wrongly resetting progress on any
scanResponse update; split the logic so scanResponse changes do not trigger
resetProgress/detailResolutionFailedRef/detailResolutionStartedRef/setConnecting.
Keep the existing useEffect that depends on scanResponse to handle navigation
and the isSameScanSource early return, but move the
resetProgress/detailResolutionFailedRef.current =
false/detailResolutionStartedRef.current = false and setConnecting() into a new
useEffect that only depends on pendingTextScanUrl, finalResult (and the
setters/resetProgress refs) and only runs when starting a new scan (e.g., when
pendingTextScanUrl is truthy or finalResult transitions to a new scan), ensuring
scanResponse is not in that dependency list so SSE progress updates won't
overwrite the current progress UI.
In `@src/shared/store/scanSessionStore.ts`:
- Around line 82-103: The persisted snapshot is missing the pendingTextScanUrl
field so the text-scan submit trigger is lost on rehydrate; update the
persistence and merge logic to include pendingTextScanUrl: add
pendingTextScanUrl to the list returned by partialize (so setPendingTextScanUrl
is serialized) and restore it in mergePersistedLightSession by reading
persisted.pendingTextScanUrl with a proper type check (string|null/undefined)
and assigning it into the returned ScanSessionState; reference
pendingTextScanUrl, partialize, mergePersistedLightSession, ScanSessionSnapshot
and ScanSessionState when making these changes.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: db8a5244-88e6-4283-8753-a96f719ef4c4
📒 Files selected for processing (28)
.env.examplesrc/features/scan-history/api/fetchScanHistoryData.test.tssrc/features/scan-history/api/fetchScanHistoryData.tssrc/features/scan-url/api/submitScanUrl.tssrc/pages/Loading/hooks/useLoadingPage.tssrc/pages/QRScan/QRScanPage.tsxsrc/pages/QRScan/hooks/useQRScanPage.tssrc/pages/QRScan/styles/qrScanPage.css.tssrc/pages/Report/ReportPage.tsxsrc/pages/Report/constants/riskDetectionCatalog.test.tssrc/pages/Report/constants/riskDetectionCatalog.tssrc/pages/Report/constants/threatText.tssrc/pages/Report/lib/analysisFailureNotice.test.tssrc/pages/Report/lib/analysisFailureNotice.tssrc/pages/Report/lib/reportPageDataContext.test.tssrc/pages/Report/lib/toReportPageData.test.tssrc/pages/Report/styles/reportPage.css.tssrc/pages/Result/lib/toResultPageData.test.tssrc/pages/ResultNonUrl/lib/toResultNonUrlPageData.test.tssrc/shared/api/endpoints.tssrc/shared/api/responseAccess/payloadAccess.test.tssrc/shared/api/responseAccess/payloadAccess.tssrc/shared/lib/sse/useScanSubscription.tssrc/shared/store/scanSessionStore.test.tssrc/shared/store/scanSessionStore.tssrc/shared/store/scanSessionTransitions.test.tssrc/shared/store/scanSessionTransitions.tsvite.config.ts
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
#45
Tasks
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항