Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 제안된 검토자
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-api-typescript/src/generator/typeGenerator.ts (1)
194-204:⚠️ Potential issue | 🟡 Minor중첩 타입 재귀 추출이 첫 번째 아이템에만 의존합니다.
Line 200에서
const val = itemType[key]는 배열의 첫 번째 아이템(value[0])만 사용합니다. 필드 타입 수집(Lines 170-191)은 모든 아이템을 순회하지만, 재귀적 중첩 타입 추출은 첫 번째 아이템의 구조에만 기반합니다.예를 들어 두 번째 아이템에만 존재하는 중첩 객체 필드는 해당 인터페이스가 생성되지 않아, 참조된 타입 이름이 정의 없이 남을 수 있습니다.
현재 실제 API 응답에서 이런 케이스가 드물다면 당장은 괜찮지만, 향후 보강 포인트로 인지해 두시면 좋겠습니다.
🤖 Fix all issues with AI agents
In `@packages/api-client/package.json`:
- Line 10: The current codegen:check script uses "pnpm run sync:bruno && git
diff --exit-code src/generated || (echo ... && exit 1)" which causes a failing
sync:bruno to be masked by the "out of sync" message; change the grouping so
sync:bruno failure propagates (e.g. run sync:bruno && (git diff --exit-code
src/generated || (echo "Generated API client is out of sync. Run: pnpm --filter
`@solid-connect/api-client` run sync:bruno" && exit 1))) — update the
codegen:check script to group the git diff check in parentheses so sync:bruno
errors are not overridden, keeping the same echo text on actual diff failures.
In `@packages/api-client/src/runtime/axiosInstance.ts`:
- Around line 82-116: The response interceptor on axiosInstance has a race when
multiple requests receive 401/403 and each calls reissueAccessToken
independently; fix by adding a shared in-flight refresh Promise (e.g.,
module-scoped let ongoingRefresh: Promise<string> | null) that is set when
reissueAccessToken(refreshToken) is first called and awaited by subsequent
interceptors, then clear ongoingRefresh on success/failure; on success save the
new token via tokenStorage.saveAccessToken, call
attachAuthorization(originalConfig, newAccessToken) and retry the original
request, and on failure call clearTokens(tokenStorage) and throw the
login-needed error—update the axiosInstance.interceptors.response.use handler to
use this shared ongoingRefresh and ensure it is nulled after completion.
In `@packages/bruno-api-typescript/src/generator/apiClientGenerator.ts`:
- Around line 171-174: The apiClientGenerator currently only adds query params
when method === "GET" (paramsList push at apiClientGenerator.ts), which diverges
from apiFactoryGenerator's logic (apiFactoryGenerator.ts uses method === "GET"
|| hasQuery via splitRequestUrl); update apiClientGenerator to parse the request
URL the same way (reuse or mirror splitRequestUrl and hasQuery detection) and
add params?: Record<string, unknown> to paramsList when either method === "GET"
or the parsed URL indicates hasQuery; ensure you reference the same parsing
function/logic so apiClientGenerator's function signatures match
apiFactoryGenerator's.
🧹 Nitpick comments (14)
packages/api-client/package.json (1)
7-7:sync:bruno:remote스크립트가 Unix 전용 환경변수 할당을 사용합니다.
BRUNO_SOURCE_MODE=remote node ...구문은 Windows에서는 동작하지 않습니다. 현재 private 패키지이고 CI 환경이 Linux 기반이라면 괜찮지만, 로컬 개발 환경에서 Windows 사용자가 있다면cross-env같은 도구를 고려해 보세요.packages/api-client/scripts/sync-bruno.mjs (1)
1-130:api-schema/scripts/sync-bruno.mjs와 약 80%의 코드가 중복됩니다.두 파일의 헬퍼 함수(
loadEnvFiles,loadEnvFile,normalizeSourceMode,run,ensureRemoteCollectionDir,resolveCollectionDir)가 거의 동일합니다. 차이점은 환경변수 로드 경로, 에러 메시지, 그리고 마지막 CLI 인자뿐입니다.향후 로직 수정 시 양쪽을 동시에 업데이트해야 하는 부담이 생길 수 있으므로, 공통 유틸리티를 별도 모듈로 추출하는 것을 고려해 보세요.
예를 들어:
packages/ shared-scripts/ bruno-sync-utils.mjs ← 공통 헬퍼 api-schema/scripts/ sync-bruno.mjs ← 설정 + CLI 인자만 api-client/scripts/ sync-bruno.mjs ← 설정 + CLI 인자만현재 규모에서는 관리 가능하므로, 급하지 않다면 다음 기회에 정리해도 충분합니다.
packages/bruno-api-typescript/src/generator/apiFactoryGenerator.ts (3)
83-86:params?.params— 생성 코드에서 혼동을 줄 수 있는 네이밍입니다.
- 외부 함수 인자 이름이
params이고, 내부 쿼리 파라미터 필드 이름도params입니다.- 결과적으로
params?.params라는 접근 패턴이 생성되어 (Line 104) 가독성이 떨어집니다.- 쿼리 파라미터 필드명을
queryParams등으로 변경하면params?.queryParams가 되어 의미가 명확해집니다.생성 코드의 소비자 입장에서 혼동 여지가 있으니, 추후 개선을 고려해주세요.
89-91:responseType.replace("Response", "Request")— 첫 번째 매치만 치환됩니다.
String.prototype.replace는 정규식이 아닌 문자열 패턴일 때 첫 번째 매치만 치환합니다. 만약responseType이"SomeResponseDataResponse"같은 형태라면"SomeRequestDataResponse"가 되어 의도와 다를 수 있습니다.현재 생성되는 타입명이 항상
~Response접미사 형태라면 문제없지만, 안전하게 마지막"Response"만 치환하는 방식이 더 견고합니다.🛡️ 더 안전한 치환 방법
- paramsList.push(`data?: ${responseType.replace("Response", "Request")}`); + paramsList.push(`data?: ${responseType.replace(/Response$/, "Request")}`);이 패턴은
apiClientGenerator.tsLine 178,apiDefinitionGenerator.tsLine 80에서도 동일하게 사용되고 있으니 일괄 적용을 권장합니다.
103-105: 쿼리 파라미터 존재 여부를 문자열 검색으로 판별하고 있습니다.
paramsList.some((p) => p.includes("params?"))— 파라미터 선언 문자열 안에서"params?"부분 문자열을 찾는 방식은 취약합니다. 예를 들어 URL 파라미터 이름에params가 포함되면 오탐이 발생할 수 있습니다.별도의 boolean 플래그(
hasQueryParams등)를 사용하면 더 안정적입니다.packages/bruno-api-typescript/src/generator/brunoHashCache.ts (1)
173-178:getStats()의total이 항상0을 반환합니다.주석에 "generateHooks에서 설정"이라고 되어 있지만, 실제로
total필드를 외부에서 업데이트할 수 있는 공개 API가 없습니다. 호출자가 반환값을 그대로 사용하면 항상total: 0이 되어 오해의 소지가 있습니다.외부에서 설정하는 구조라면
setTotal()메서드를 추가하거나,getStats(total: number)인자로 받는 방식을 고려해주세요.packages/bruno-api-typescript/src/generator/apiClientGenerator.ts (1)
135-137:hasParams판별이{{URL}}의{에 의해 false positive가 발생할 수 있습니다.
http.url.includes("{")조건은 Bruno 템플릿 변수{{URL}}에도 매칭됩니다. 실제 경로 파라미터가 없어도hasParams가true가 됩니다.기능적 영향은 미미하지만 (URL 파라미터 추출 regex가
{{URL}}을 매칭하지 않으므로), 의미적 정확성을 위해{{...}}패턴을 제외하는 것이 좋습니다.🔧 개선 제안
// URL에 파라미터가 있는지 확인 - const hasParams = http.url.includes(":") || http.url.includes("{"); + const strippedUrl = http.url.replace(/\{\{[^}]+\}\}/g, ""); + const hasParams = strippedUrl.includes(":") || strippedUrl.includes("{");packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts (1)
67-71:normalizedPath에 대해 이중 정규화가 발생합니다.
- Line 69에서
normalizeApiPath(url)로 정규화한normalizedPath를 만듭니다.- Line 70에서
extractPathParams(normalizedPath)를 호출하는데,extractPathParams내부 (Line 38)에서도normalizeApiPath를 다시 호출합니다.기능적 문제는 없지만,
extractPathParams가 이미 정규화된 경로를 받을 수 있도록 분리하거나, 내부 정규화를 제거하면 의도가 더 명확해집니다.packages/bruno-api-typescript/src/generator/index.ts (4)
64-82: 크로스 플랫폼 경로 구분자 이슈가 있을 수 있어요.
relative()은 OS에 따라 백슬래시(\)를 반환할 수 있습니다. Line 66에서"/"만으로 split하면 Windows 환경에서 도메인 추출이 실패하여 Line 81의 폴백 값을 반환하게 됩니다. 참고로 Line 307에서는 이미.replace(/\\/g, "/")변환을 적용하고 계시네요.🔧 경로 정규화 제안
function extractDomain(filePath: string, brunoDir: string): string { const relativePath = relative(brunoDir, filePath); - const parts = relativePath.split("/"); + const parts = relativePath.replace(/\\/g, "/").split("/");
108-126: 함수명 충돌 해소 로직이 잘 동작하지만, 네이밍 패턴을 확인해 주세요.중복된 함수명에 대해 첫 번째는 원본 이름을 유지하고, 두 번째부터
baseName2,baseName3... 순서로 접미사가 붙습니다.baseName1이 건너뛰어지는 형태인데, 의도된 설계라면 괜찮습니다. 다만, 이 패턴이 생성된 코드 사용자에게 혼동을 줄 수 있으니, 필요하다면baseName→baseName1,baseName2로 통일하는 것도 고려해 볼 수 있습니다.
197-199:allParsedFiles에서 파싱 실패 시 로그가 누락됩니다.
parsedChangedFiles(Line 187-195)에서는 파싱 실패 시 에러를 로깅하지만,allParsedFiles(Line 197-199)에서는 조용히 필터링만 합니다. 동일한parseFileWithDomain함수를 사용하므로changedFiles에 포함되지 않은 파일이 실패하면 아무런 피드백 없이 빠지게 됩니다.💡 로깅 추가 제안
- const allParsedFiles = brunoFiles - .map(parseFileWithDomain) - .filter((value): value is ParsedDomainFile => value !== null); + const allParsedFiles = brunoFiles + .map((filePath) => { + const parsed = parseFileWithDomain(filePath); + if (!parsed) { + console.warn(`⚠️ Skipped unparsable file: ${filePath}`); + } + return parsed; + }) + .filter((value): value is ParsedDomainFile => value !== null);
263-277: 변경된 파일을 해시 캐시 업데이트 시 다시 파싱하고 있어요.
parsedChangedFiles에서 이미 파싱된 결과가 있는데, Line 265에서parseBrunoFile과extractApiFunction을 다시 호출합니다. 파일 수가 많아지면 불필요한 I/O와 연산이 발생할 수 있습니다.♻️ 기존 파싱 결과 재활용 제안
이미 파싱된
parsedChangedFiles와domainApiFunctions의 결과를 활용하여,parseBrunoFile과extractApiFunction재호출을 제거할 수 있습니다. 예를 들어,parsedChangedFiles를Map<filePath, { apiFunc, domain }>형태로 미리 구성해두면 됩니다.packages/api-client/src/runtime/axiosInstance.ts (2)
94-97: 재시도 방지 가드(_retry플래그)가 없습니다.
originalConfig에 재시도 여부를 표시하는 플래그가 없어서, 극단적인 경우(예:axios.request(originalConfig)호출 결과가 다시 이 인터셉터를 타게 되는 시나리오) 의도치 않은 재시도가 발생할 수 있습니다. 현재는axios.request()를 사용하여axiosInstance의 인터셉터를 우회하므로 실제로는 안전하지만, 방어적 코딩 차원에서 가드를 추가하면 향후 리팩토링 시 안전합니다.🛡️ 재시도 가드 제안
const originalConfig = error.config; - if (!originalConfig) { + if (!originalConfig || (originalConfig as any)._retry) { throw error; } + (originalConfig as any)._retry = true;
121-126:configureApiClientRuntime에 config 유효성 검증이 없습니다.
baseURL이 빈 문자열이거나tokenStorage의 메서드가 올바르지 않은 경우에도 오류 없이 설정됩니다. 현재 호출부가 제한적이므로 당장 문제가 되지 않지만, 공유 패키지로서 방어적 검증을 추가하면 더 안전합니다.
| "sync:bruno:remote": "BRUNO_SOURCE_MODE=remote node ./scripts/sync-bruno.mjs", | ||
| "build": "pnpm run sync:bruno", | ||
| "typecheck": "tsc --noEmit", | ||
| "codegen:check": "pnpm run sync:bruno && git diff --exit-code src/generated || (echo \"Generated API client is out of sync. Run: pnpm --filter @solid-connect/api-client run sync:bruno\" && exit 1)" |
There was a problem hiding this comment.
codegen:check 스크립트에서 sync:bruno 실패 시 오해의 소지가 있는 에러 메시지가 출력됩니다.
현재 구조:
pnpm run sync:bruno && git diff --exit-code src/generated || (echo "out of sync..." && exit 1)
sync:bruno가 실패(네트워크 오류, 환경변수 미설정 등)하면 && 이후가 스킵되고, || 절이 실행되어 "Generated API client is out of sync"라는 실제 원인과 다른 메시지가 표시됩니다. 물론 sync:bruno의 실제 에러도 stderr로 출력되지만, CI 로그에서 혼동을 줄 수 있습니다.
🔧 sync:bruno 실패와 diff 불일치를 분리하는 제안
- "codegen:check": "pnpm run sync:bruno && git diff --exit-code src/generated || (echo \"Generated API client is out of sync. Run: pnpm --filter `@solid-connect/api-client` run sync:bruno\" && exit 1)"
+ "codegen:check": "pnpm run sync:bruno && (git diff --exit-code src/generated || (echo \"Generated API client is out of sync. Run: pnpm --filter `@solid-connect/api-client` run sync:bruno\" && exit 1))"괄호로 git diff || echo 부분을 그룹화하면, sync:bruno 실패 시에는 해당 종료 코드가 그대로 전파되고, 성공 시에만 diff 검사가 수행됩니다.
🤖 Prompt for AI Agents
In `@packages/api-client/package.json` at line 10, The current codegen:check
script uses "pnpm run sync:bruno && git diff --exit-code src/generated || (echo
... && exit 1)" which causes a failing sync:bruno to be masked by the "out of
sync" message; change the grouping so sync:bruno failure propagates (e.g. run
sync:bruno && (git diff --exit-code src/generated || (echo "Generated API client
is out of sync. Run: pnpm --filter `@solid-connect/api-client` run sync:bruno" &&
exit 1))) — update the codegen:check script to group the git diff check in
parentheses so sync:bruno errors are not overridden, keeping the same echo text
on actual diff failures.
Comment on lines
+82
to
+116
| axiosInstance.interceptors.response.use( | ||
| (response) => response, | ||
| async (error: AxiosError) => { | ||
| if (!runtimeConfig) { | ||
| throw error; | ||
| } | ||
|
|
||
| const status = error.response?.status; | ||
| if (status !== 401 && status !== 403) { | ||
| throw error; | ||
| } | ||
|
|
||
| const originalConfig = error.config; | ||
| if (!originalConfig) { | ||
| throw error; | ||
| } | ||
|
|
||
| const { tokenStorage, isTokenExpired, reissueAccessToken } = runtimeConfig; | ||
| const refreshToken = tokenStorage.loadRefreshToken(); | ||
| if (refreshToken === null || isTokenExpired(refreshToken)) { | ||
| clearTokens(tokenStorage); | ||
| throw error; | ||
| } | ||
|
|
||
| try { | ||
| const newAccessToken = await reissueAccessToken(refreshToken); | ||
| tokenStorage.saveAccessToken(newAccessToken); | ||
| attachAuthorization(originalConfig, newAccessToken); | ||
| return await axios.request(originalConfig); | ||
| } catch { | ||
| clearTokens(tokenStorage); | ||
| throw new Error("로그인이 필요합니다"); | ||
| } | ||
| }, | ||
| ); |
There was a problem hiding this comment.
동시 401 응답에 대한 토큰 갱신 경쟁 조건이 있습니다.
여러 요청이 동시에 401을 받으면, 각 요청의 response 인터셉터가 독립적으로 reissueAccessToken을 호출합니다. 이로 인해:
- 동시에 여러 번 토큰 갱신 API가 호출됩니다.
- 백엔드가 refresh token을 1회 사용 후 무효화하는 경우, 첫 번째 이후의 갱신 시도가 실패합니다.
- 실패한 요청들의 사용자는 불필요하게 로그아웃 처리됩니다.
일반적인 해결 방법은 갱신 중인 Promise를 캐싱하여 동시 요청들이 같은 Promise를 공유하도록 하는 것입니다.
🔒 동시 갱신 방지 패턴 제안
+let refreshPromise: Promise<string> | null = null;
+
axiosInstance.interceptors.response.use(
(response) => response,
async (error: AxiosError) => {
if (!runtimeConfig) {
throw error;
}
const status = error.response?.status;
if (status !== 401 && status !== 403) {
throw error;
}
const originalConfig = error.config;
if (!originalConfig) {
throw error;
}
const { tokenStorage, isTokenExpired, reissueAccessToken } = runtimeConfig;
const refreshToken = tokenStorage.loadRefreshToken();
if (refreshToken === null || isTokenExpired(refreshToken)) {
clearTokens(tokenStorage);
throw error;
}
try {
- const newAccessToken = await reissueAccessToken(refreshToken);
- tokenStorage.saveAccessToken(newAccessToken);
+ if (!refreshPromise) {
+ refreshPromise = reissueAccessToken(refreshToken).then((token) => {
+ tokenStorage.saveAccessToken(token);
+ return token;
+ });
+ }
+ const newAccessToken = await refreshPromise;
attachAuthorization(originalConfig, newAccessToken);
return await axios.request(originalConfig);
} catch {
clearTokens(tokenStorage);
throw new Error("로그인이 필요합니다");
+ } finally {
+ refreshPromise = null;
}
},
);🤖 Prompt for AI Agents
In `@packages/api-client/src/runtime/axiosInstance.ts` around lines 82 - 116, The
response interceptor on axiosInstance has a race when multiple requests receive
401/403 and each calls reissueAccessToken independently; fix by adding a shared
in-flight refresh Promise (e.g., module-scoped let ongoingRefresh:
Promise<string> | null) that is set when reissueAccessToken(refreshToken) is
first called and awaited by subsequent interceptors, then clear ongoingRefresh
on success/failure; on success save the new token via
tokenStorage.saveAccessToken, call attachAuthorization(originalConfig,
newAccessToken) and retry the original request, and on failure call
clearTokens(tokenStorage) and throw the login-needed error—update the
axiosInstance.interceptors.response.use handler to use this shared
ongoingRefresh and ensure it is nulled after completion.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/admin/src/components/features/scores/GpaScoreTable.tsx (1)
155-161:⚠️ Potential issue | 🟡 Minor
Number.parseFloat빈 입력 시NaN가능성이 있습니다.사용자가 GPA 입력 필드를 비우면
Number.parseFloat("")은NaN을 반환합니다. 이 상태로 저장하면 서버에NaN이 전송될 수 있습니다.editingGpaCriteria필드(Line 174)도 동일합니다.🛠️ 제안하는 수정
-onChange={(e) => setEditingGpa(Number.parseFloat(e.target.value))} +onChange={(e) => { + const val = Number.parseFloat(e.target.value); + if (!Number.isNaN(val)) setEditingGpa(val); +}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/components/features/scores/GpaScoreTable.tsx` around lines 155 - 161, The onChange handlers for the GPA inputs (editingGpa and editingGpaCriteria) can produce NaN when the user clears the field because Number.parseFloat("") === NaN; update the handlers (where setEditingGpa and the setter for editingGpaCriteria are called) to detect an empty string and set a safe value (e.g., undefined or null) instead of NaN, otherwise parseFloat the string; also ensure the input value prop mirrors that safe state (use an empty string for display when the value is undefined/null) so you never send NaN to the server.apps/web/src/utils/authUtils.ts (1)
39-61:⚠️ Potential issue | 🟡 MinorApple 로그인 에러 핸들링에서 개선이 필요한 부분이 있습니다.
사용자 취소 시나리오 구분 처리 -
popup_closed_by_user,user_cancelled_authorize문자열 비교로 의도된 취소와 실제 오류를 올바르게 구분하여 불필요한 에러 토스트를 방지합니다. 👍Authorization code 인코딩 -
encodeURIComponent를 사용하여 코드를 올바르게 인코딩합니다.에러 재발생 처리 필요 - Line 60에서 에러를 재발생(throw)시키는데,
LoginContent.tsxLine 133의onClick={appleLogin}에서 이를 catch하지 않으면 브라우저 콘솔에 unhandled promise rejection이 기록됩니다. 다음과 같이 수정하세요:onClick={() => appleLogin().catch(() => {})}또는 별도의 에러 핸들링 함수를 만들어 사용하면 더 명확합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/utils/authUtils.ts` around lines 39 - 61, The catch in authUtils.ts around window.AppleID.auth.signIn currently rethrows errors which can cause unhandled promise rejections if the caller (e.g., the appleLogin invoked from LoginContent.tsx) doesn't catch them; update the caller to handle the promise rejection by changing the onClick handler in LoginContent.tsx (where onClick={appleLogin}) to either wrap the call with a catch — for example onClick={() => appleLogin().catch(() => {})} — or call a small wrapper function that calls appleLogin().catch(handleError) so all rethrown errors are handled upstream; keep the existing cancellation checks in authUtils.ts and only swallow or rethrow as appropriate.apps/web/src/app/university/application/ScorePageContent.tsx (1)
33-33:⚠️ Potential issue | 🟠 Major
_setShowNeedApply가 호출되지 않아ConfirmCancelModal이 영구적으로 비활성화됩니다.
- 문제
showNeedApply는 항상false이기 때문에ConfirmCancelModal의isOpen={showNeedApply}는 절대true가 되지 않습니다.- "학교 지원이 필요합니다" 모달은 사실상 동작하지 않는 데드코드 상태입니다.
- 선택지
- 모달을 의도적으로 비활성화한 것이라면 → 관련
ConfirmCancelModal렌더링 블록도 함께 제거하는 것이 좋습니다.- 기능이 미구현 상태라면 → 모달을 열어야 하는 조건(
_setShowNeedApply(true)호출 시점)을 구현해야 합니다.모달을 여는 트리거 로직이 필요하시면 구현 방향을 제안해 드릴 수 있습니다. 새 이슈로 트래킹하기를 원하시면 알려주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/application/ScorePageContent.tsx` at line 33, The state showNeedApply is never changed because the setter _setShowNeedApply is unused, leaving ConfirmCancelModal permanently closed; either remove the ConfirmCancelModal rendering block if the modal is intentionally disabled, or wire up the setter where the modal should open (e.g., call _setShowNeedApply(true) from the relevant user action or validation path such as an apply button handler or inside the function that evaluates "needs school support"); locate references to showNeedApply, _setShowNeedApply and ConfirmCancelModal in ScorePageContent and add the _setShowNeedApply(true) call at the point where you detect the need-to-apply condition (or remove the modal JSX if deprecated).
🧹 Nitpick comments (26)
turbo.json (1)
19-23:codegen:check가ci:check에 연결되어 있지 않습니다.
codegen:check의 목적이 Bruno 소스와 생성 파일 간의 드리프트(drift)를 감지하는 것이라면,ci:check에 포함되지 않은 상태에서는 명시적으로 호출하지 않는 한 CI 파이프라인에서 자동으로 실행되지 않습니다.현재
ci:check는 다음과 같습니다:"ci:check": { "dependsOn": ["lint:check", "typecheck"], "outputs": [] }
codegen:check를 CI에서 강제 실행하려면 아래와 같이 추가하는 것을 검토해 주세요:🔗 제안하는 수정안
"ci:check": { - "dependsOn": ["lint:check", "typecheck"], + "dependsOn": ["lint:check", "typecheck", "codegen:check"], "outputs": [] }만약
codegen:check가 CI와 무관하게 개발자가 로컬에서 수동으로 실행하는 용도라면, 그 의도를 주석이나 README 등에 명시해 두는 것이 좋습니다.Also applies to: 40-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@turbo.json` around lines 19 - 23, The CI is not running the codegen drift check because the turbo task "codegen:check" is not listed as a dependency of the "ci:check" target; update the "ci:check" target to include "codegen:check" in its dependsOn array (i.e., add "codegen:check" alongside "lint:check" and "typecheck"), or if the intent is for "codegen:check" to be manual only, add a short comment/README note clarifying that "codegen:check" is intentionally excluded from "ci:check".apps/web/src/app/terms/page.tsx (2)
142-142: 📅 시행일이 하드코딩되어 있어 향후 약관 개정 시 누락 위험이 있습니다.현재 날짜가
"2026년 2월 21일"로 고정되어 있어, 약관이 개정될 때 이 값을 함께 수동으로 변경해야 한다는 사실을 놓치기 쉽습니다.
- 현황
- 날짜가 소스 코드에 직접 박혀 있어
TERMS_SECTIONS를 수정해도 날짜가 자동으로 맞춰지지 않습니다.- 권장 접근
- 날짜를
TERMS_SECTIONS상수 근처에 별도 상수로 분리하면, 개정 시 한 곳만 수정하면 된다는 사실이 명확해집니다.♻️ 날짜를 별도 상수로 분리 제안
+ const TERMS_EFFECTIVE_DATE = "2026년 2월 21일"; + const TERMS_SECTIONS = [ ... ] as const; // ... - <p className="mb-6 text-k-800 typo-regular-2">시행일: 2026년 2월 21일</p> + <p className="mb-6 text-k-800 typo-regular-2">시행일: {TERMS_EFFECTIVE_DATE}</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/terms/page.tsx` at line 142, The hardcoded effective date string should be extracted into a single named constant near TERMS_SECTIONS and referenced in the JSX instead of the literal; locate TERMS_SECTIONS in app/terms/page.tsx, add a constant like TERMS_EFFECTIVE_DATE (or similar) adjacent to it, set the date value there, and replace the inline "시행일: 2026년 2월 21일" markup with a template that uses that constant so future updates only need to change the one constant.
149-151: 💡key={item}— 긴 문자열보다는 인덱스 사용을 권장합니다.현재 각 항목의 전체 한국어 문장을 React key로 사용하고 있습니다.
as const데이터라 실용적 문제는 없지만, 두 가지 잠재적 이슈가 있습니다.
- 성능 오버헤드
- 수백 자짜리 문자열을 React 재조정(reconciliation)의 key로 비교하는 것은 숫자 인덱스보다 불필요한 비용이 발생합니다.
- 중복 key 위험
- 같은 섹션 내에 동일한 문자열 항목이 생기면 React가 조용히 중복 처리하여 렌더링이 예상과 달라질 수 있습니다.
♻️ 인덱스 기반 key 사용 제안
- {section.items.map((item) => ( - <li key={item}>{item}</li> - ))} + {section.items.map((item, idx) => ( + <li key={idx}>{item}</li> + ))}이 데이터는 정적 상수이며 순서가 변하지 않으므로, 인덱스 key 사용이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/terms/page.tsx` around lines 149 - 151, Replace the current React list key usage (key={item}) inside the section.items.map callback with an index-based key by changing the map to section.items.map((item, index) => ...) and using key={index} on the <li>; update the map callback signature where this occurs so duplicate long strings won’t be used as keys (symbol references: section.items.map and the <li key={...}> element).apps/admin/src/components/ui/textarea.tsx (2)
1-21: React 19의 새로운 ref prop 패턴 적용을 고려해보세요! 🎉컴포넌트 자체는 깔끔하고 올바르게 구현되어 있어요. 다만 이 프로젝트가 React 19를 사용하고 있는 점을 감안하면, 아래와 같이
forwardRef없이 더 간결하게 작성할 수 있습니다.
변경 이유
- React 19에서는
forwardRef가 더 이상 필요하지 않으며, 향후 릴리즈에서 공식 deprecated 예정입니다.ref를 이제 일반 prop처럼 직접 전달할 수 있어 컴포넌트 구조가 단순해지고 보일러플레이트가 줄어듭니다.추가 참고
- React 19에서
forwardRef컴포넌트는refprop이undefined인 경우에도 props의 참조 안정성이 깨져 하위 메모이제이션에 영향을 줄 수 있는 부작용이 있습니다.✨ React 19 ref-as-prop 패턴으로 리팩토링 제안
-import * as React from "react"; +import { type ComponentProps } from "react"; import { cn } from "@/lib/utils"; -const Textarea = React.forwardRef<HTMLTextAreaElement, React.ComponentProps<"textarea">>( - ({ className, ...props }, ref) => { - return ( - <textarea - className={cn( - "flex min-h-24 w-full rounded-md border border-k-200 bg-k-0 px-3 py-2 typo-regular-4 text-k-700 shadow-sm transition-colors placeholder:text-k-400 focus-visible:border-primary focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50", - className, - )} - ref={ref} - {...props} - /> - ); - }, -); -Textarea.displayName = "Textarea"; +function Textarea({ className, ...props }: ComponentProps<"textarea">) { + return ( + <textarea + className={cn( + "flex min-h-24 w-full rounded-md border border-k-200 bg-k-0 px-3 py-2 typo-regular-4 text-k-700 shadow-sm transition-colors placeholder:text-k-400 focus-visible:border-primary focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50", + className, + )} + {...props} + /> + ); +} export { Textarea };
ComponentProps<"textarea">에는ref가 이미 포함되어 있으므로 별도 타입 선언 없이도 ref 전달이 정상 동작합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/components/ui/textarea.tsx` around lines 1 - 21, 컴포넌트에서 React.forwardRef를 제거하고 React 19의 ref-as-prop 패턴으로 리팩토링하세요: Textarea 컴포넌트 선언을 forwardRef 대신 단순 함수로 바꿔서 시그니처를 (props: React.ComponentProps<"textarea">)로 받고, 내부 <textarea>에 ref를 props.ref로 전달하며 기존 className 병합(cn)과 {...props} 사용을 유지하세요; Textarea.displayName은 필요하면 유지하거나 제거할 수 있고, 파일 내 Textarea 식별자와 cn 유틸을 그대로 사용해 변경하세요.
5-18: React 19에서는forwardRef없이ref를 직접 prop으로 받을 수 있습니다.이 프로젝트가 React 19.2.0을 사용 중이므로, 다음과 같은 개선을 추천합니다:
ref를 일반 prop으로 처리: React 19부터는forwardRef래퍼가 불필요하며, 향후 릴리스에서 deprecated될 예정입니다.리팩토링 방향: 함수형 컴포넌트에서 직접
ref를 받아 처리하면 더 간단하고 명확한 코드가 됩니다.현재 코드는 정상 작동하지만, React 19의 새로운 패턴을 따르면 좋습니다.
✨ React 19 스타일 리팩토링 제안
-import * as React from "react"; +import { type ComponentProps } from "react"; import { cn } from "@/lib/utils"; -const Textarea = React.forwardRef<HTMLTextAreaElement, React.ComponentProps<"textarea">>( - ({ className, ...props }, ref) => { - return ( - <textarea - className={cn( - "flex min-h-24 w-full rounded-md border border-k-200 bg-k-0 px-3 py-2 typo-regular-4 text-k-700 shadow-sm transition-colors placeholder:text-k-400 focus-visible:border-primary focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50", - className, - )} - ref={ref} - {...props} - /> - ); - }, -); -Textarea.displayName = "Textarea"; +function Textarea({ className, ...props }: ComponentProps<"textarea">) { + return ( + <textarea + className={cn( + "flex min-h-24 w-full rounded-md border border-k-200 bg-k-0 px-3 py-2 typo-regular-4 text-k-700 shadow-sm transition-colors placeholder:text-k-400 focus-visible:border-primary focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50", + className, + )} + {...props} + /> + ); +} export { Textarea };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/components/ui/textarea.tsx` around lines 5 - 18, The Textarea component is using React.forwardRef while the project runs on React 19 where refs can be accepted as normal props; refactor the Textarea function to be a plain functional component that accepts an explicit ref prop typed as HTMLTextAreaElement (alongside React.ComponentProps<"textarea">/className) and pass that ref into the <textarea>; remove React.forwardRef usage and adjust the component signature and export accordingly, keeping the existing className handling with cn and all other props spread onto the textarea.apps/admin/src/components/layout/AdminSidebar.tsx (1)
4-6: 💡activeMenu타입 범위를 확장하는 것을 고려해보세요현재
activeMenu의 타입이"scores" | "bruno"로 제한되어 있어, 향후university,mentor,user메뉴가 라우트를 갖게 되면 타입을 함께 업데이트해야 합니다. 미리sideMenus배열의key값에서 타입을 파생시키면 관리가 더 편해집니다.♻️ 타입 자동 파생 제안
+type SideMenuKey = (typeof sideMenus)[number]["key"]; interface AdminSidebarProps { - activeMenu: "scores" | "bruno"; + activeMenu: SideMenuKey; }단,
sideMenus배열 선언이 인터페이스보다 앞에 와야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/components/layout/AdminSidebar.tsx` around lines 4 - 6, The activeMenu union in AdminSidebarProps is hard-coded; derive it from the sideMenus array instead to avoid future updates: move the sideMenus declaration before AdminSidebarProps, mark sideMenus as const, and change AdminSidebarProps.activeMenu to the derived type (e.g., typeof sideMenus[number]['key']) so its valid values come directly from the sideMenus keys; update any imports/usages of AdminSidebarProps or AdminSidebar component if necessary.apps/admin/src/lib/query/queryClient.ts (1)
4-20: 💡 mutation 기본 옵션도 함께 설정하는 것을 고려해보세요현재
queries에만 retry 정책(401/403 제외)이 적용되고 있습니다. mutation도 동일한 조건에서 불필요한 재시도가 발생하지 않도록mutations기본 옵션을 추가하면 더 일관된 동작을 보장할 수 있습니다.♻️ mutation 기본 옵션 추가 제안
defaultOptions: { queries: { staleTime: 60 * 1000, gcTime: 10 * 60 * 1000, refetchOnWindowFocus: false, retry: (failureCount, error) => { const status = (error as AxiosError | undefined)?.response?.status; if (status === 401 || status === 403) { return false; } return failureCount < 1; }, }, + mutations: { + retry: (failureCount, error) => { + const status = (error as AxiosError | undefined)?.response?.status; + if (status === 401 || status === 403) { + return false; + } + return failureCount < 1; + }, + }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/lib/query/queryClient.ts` around lines 4 - 20, Add the same retry policy to mutations so they don't retry on 401/403 like queries: in createQueryClient where QueryClient is constructed, add a defaultOptions.mutations section mirroring defaultOptions.queries (include a retry function that inspects (error as AxiosError | undefined)?.response?.status and returns false for 401/403, otherwise uses failureCount < 1); keep other relevant mutation defaults consistent with your queries settings.apps/admin/src/lib/api/admin.ts (1)
22-62: 반환 타입이 모두Promise<any>입니다.다른 API(
auth,scores)는 생성된 클라이언트를 통해 타입이 보장되는 반면, 이 파일의 모든 메서드는res.data의 타입이any입니다. 호출부에서 타입 안전성이 떨어질 수 있으니, 향후 타입 정의를 추가하거나 코드젠 대상에 포함시키는 것을 고려해보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/lib/api/admin.ts` around lines 22 - 62, The adminApi methods currently return Promise<any> because they always call .then(res => res.data) without typing the axios response; update each method in adminApi (e.g., getMentorApplicationList, getCountMentorApplicationByStatus, getMentorApplicationHistoryList, postApproveMentorApplication, postRejectMentorApplication, postMappingMentorapplicationUniversity, get권역조회, post권역생성, put권역수정, delete권역삭제, get지역조회, post지역생성, put지역수정, delete지역삭제) to use typed axios calls (or explicit response generics) so res.data has a concrete type (define/extend response interfaces like MentorApplicationListResponse, RegionResponse, CountryResponse, etc., and use axiosInstance.get<YourType>(...) / post<YourType>(...) then return res.data).apps/admin/src/components/features/scores/GpaScoreTable.tsx (1)
1-242:GpaScoreTable과LanguageScoreTable의 구조가 거의 동일합니다.두 테이블 컴포넌트가 쿼리 설정, 뮤테이션 패턴, 페이지네이션, 로딩 상태, 배너 등에서 매우 유사한 구조를 공유합니다. 향후 공통 테이블/페이지네이션 로직을 커스텀 훅이나 제네릭 컴포넌트로 추출하면 유지보수가 용이해질 수 있습니다. 지금 당장은 아니어도 괜찮습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/components/features/scores/GpaScoreTable.tsx` around lines 1 - 242, GpaScoreTable and LanguageScoreTable share nearly identical logic (query/mutation/pagination/loading/banner/edit flow); extract shared behavior into a reusable hook or generic table component to avoid duplication: move data fetching and pagination into a custom hook (e.g., useScoresTable) that wraps useQuery (queryKey ["scores", type, verifyFilter, page]) and exposes data, isLoading, isFetching, page, setPage, totalPages; move mutation logic (updateGpaMutation pattern) into the hook or a shared update function accepting id, status, reason, and score; lift common handlers (handleVerifyStatus, handleEdit, handleSave, handlePageChange) to use the hook or to be small adapters in each component; keep component-specific rendering (columns, edit inputs, S3 link) inside GpaScoreTable and LanguageScoreTable while calling the shared hook for state and operations.apps/admin/src/lib/api/client.ts (1)
35-38:reissueAccessToken콜백에서 응답 구조 검증 추가를 권장합니다.다음과 같은 이유로 방어 코드를 추가하면 좋습니다:
현재 문제점 — API 응답이 예상과 다른 구조일 경우
response.data.accessToken이undefined가 될 수 있습니다. 런타임의 try-catch는 예외만 처리하고 undefined 값은 통과시킵니다.권장 개선 — 반환 전에
accessToken존재 여부를 확인하거나, 값이 없을 경우 명시적으로 에러를 던지면 예상치 못한 상태를 조기에 감지할 수 있습니다.현재 상태 — 백엔드가 올바르게 응답한다면 문제없지만, 네트워크 오류나 서버 상태 변화 등으로 응답 구조가 달라질 수 있으므로 방어 코드가 도움이 됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/lib/api/client.ts` around lines 35 - 38, The reissueAccessToken callback should validate the API response before returning; inside reissueAccessToken (which calls reissueAccessTokenApi) check that response && response.data && typeof response.data.accessToken === 'string' (or non-empty) and if not throw a descriptive error (e.g., "Missing accessToken in reissueAccessToken response") so callers don't receive undefined; ensure the thrown error includes context (refreshToken or request id) as appropriate for debugging.apps/admin/src/routes/scores/index.tsx (1)
33-42:VerifyStatus타입 단언이 하드코딩된 옵션과 결합되어 있습니다.현재
event.target.value as VerifyStatus단언은<option>값 3개(PENDING,APPROVED,REJECTED)와VerifyStatus타입이 정확히 일치한다는 전제 하에 동작합니다. 만약 향후VerifyStatus에 새 값이 추가되면 이 select의 옵션 목록도 함께 업데이트해야 하므로, 동기화 누락이 발생할 수 있습니다.당장 문제는 아니지만,
VerifyStatus값 배열을 한 곳에서 정의하고<option>을 동적 렌더링하면 타입과 UI 간 불일치를 방지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/routes/scores/index.tsx` around lines 33 - 42, Extract the allowed VerifyStatus values into a single source (e.g., a const array like VERIFY_STATUS_OPTIONS) and render the <option> elements by mapping that array instead of hardcoding them; update the select to use verifyFilterId, verifyFilter and setVerifyFilter as before but derive the new value from the mapped option value so the UI and the VerifyStatus type stay in sync and future additions to VerifyStatus only require updating the VERIFY_STATUS_OPTIONS array.apps/admin/src/routes/auth/login.tsx (1)
23-26: 뮤테이션 파라미터 네이밍이 조금 독특합니다.
nextEmail,nextPassword라는 이름은 외부 스코프의password상태 변수와의 섀도잉을 피하기 위한 것으로 보이지만, 호출부에서{ nextEmail: email, nextPassword: password }처럼 매핑이 필요해져 가독성이 떨어집니다.
mutationFn자체가 별도 스코프이므로 동일한 이름을 사용해도 무방합니다.✏️ 제안: 파라미터 이름 단순화
const signInMutation = useMutation({ - mutationFn: ({ nextEmail, nextPassword }: { nextEmail: string; nextPassword: string }) => - adminSignInApi(nextEmail, nextPassword), + mutationFn: ({ email, password }: { email: string; password: string }) => + adminSignInApi(email, password), });호출부도 함께 변경:
- const response = await signInMutation.mutateAsync({ nextEmail: email, nextPassword: password }); + const response = await signInMutation.mutateAsync({ email, password });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/routes/auth/login.tsx` around lines 23 - 26, Change the oddly named mutation parameters in signInMutation to use the simpler, conventional names email and password: update the mutationFn signature to accept ({ email, password }) and call adminSignInApi(email, password); then update any mutation.trigger or mutate calls that currently pass { nextEmail: ..., nextPassword: ... } to pass { email: ..., password: ... } so names match and eliminate the unnecessary mapping while keeping signInMutation, mutationFn, and adminSignInApi references intact.apps/admin/src/routes/bruno/index.tsx (2)
43-49:import.meta.glob경로가 상대 경로에 강하게 의존합니다.
"../../../../../packages/api-client/src/generated/apis/*/apiDefinitions.ts"경로는 디렉토리 구조가 변경되면 조용히 빈 결과를 반환하게 됩니다. 모노레포 dev tool이라 현실적으로 큰 문제는 아니지만, 빈 결과 시 개발자에게 알려주는 방어 코드가 있으면 디버깅이 수월해집니다.🛡️ 빈 모듈 감지 경고 추가 제안
const definitionModules = import.meta.glob( "../../../../../packages/api-client/src/generated/apis/*/apiDefinitions.ts", { eager: true, import: "*", }, ) as Record<string, ApiDefinitionModule>; + +if (Object.keys(definitionModules).length === 0) { + console.warn("[BrunoApiPage] apiDefinitions 모듈을 찾지 못했습니다. glob 경로를 확인해주세요."); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/routes/bruno/index.tsx` around lines 43 - 49, The import.meta.glob call assigned to definitionModules can return an empty object if the relative path changes; add defensive detection immediately after the definitionModules declaration (the import.meta.glob(...) call) by checking Object.keys(definitionModules).length === 0 and emitting a clear warning or throwing a descriptive error that includes the glob pattern string (e.g., the literal "../../../../../packages/api-client/src/generated/apis/*/apiDefinitions.ts") so developers are alerted when no modules are found; keep the check near the definitionModules variable and use processLogger/console.warn or throw depending on how critical you want the failure to be.
188-214: 엔드포인트를 변경해도 에디터가 초기화되지 않습니다.새 엔드포인트를 선택해도 Path Params, Query, Body, Headers 에디터에는 이전 값이 그대로 남습니다. 사용자가 매번 "초기화" 버튼을 눌러야 하는 점이 혼란을 줄 수 있습니다. 엔드포인트 변경 시 에디터를 자동으로 리셋하거나, 해당 엔드포인트의 스키마 기본값으로 채워주면 UX가 향상될 수 있습니다.
✏️ 엔드포인트 선택 시 에디터 리셋 제안
+ const handleSelectEndpoint = (key: string) => { + setSelectedKey(key); + handleResetEditors(); + };그리고 Line 311의
onClick에서:- onClick={() => setSelectedKey(key)} + onClick={() => handleSelectEndpoint(key)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/routes/bruno/index.tsx` around lines 188 - 214, When the selected endpoint changes (selectedKey / selectedEndpoint), the editor state (pathParamsText, queryParamsText, bodyText, headersText) must be reset or populated with the endpoint's schema defaults; add a useEffect that watches selectedKey or selectedEndpoint and calls setPathParamsText, setQueryParamsText, setBodyText and setHeadersText to either "{}" or to values derived from selectedEndpoint.definition (schema/defaults) and also clear setRequestResult and setIsSending as appropriate; update the component to compute any default payloads from selectedEndpoint.definition before setting state so the editors reflect the newly selected endpoint immediately.apps/web/src/app/university/[homeUniversity]/_ui/UniversityListContent.tsx (1)
31-37:useEffect를 통한 외부 prop 동기화 패턴, 동작에는 문제가 없습니다.한 가지 참고사항: 사용자가 검색창에 직접 입력한 후, 부모 컴포넌트에서
initialSearchText가 변경되면 사용자 입력이 덮어쓰여질 수 있습니다. URL 파라미터 기반 동기화 용도라면 의도된 동작이겠지만, 혹시 다른 시나리오에서 문제가 될 수 있는지 확인해 보시면 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/`[homeUniversity]/_ui/UniversityListContent.tsx around lines 31 - 37, The current UniversityListContent component unconditionally syncs external props initialSearchText and initialRegion into internal state via two useEffect hooks (calling setSearchText and setSelectedRegion), which can overwrite in-progress user edits; update the sync logic to avoid clobbering user input by only applying prop updates when the input is not being edited or is empty (e.g., track an isUserEditing/hasUserModified flag or check searchText.trim() === '' before calling setSearchText), and apply a similar guard for initialRegion, or alternatively only sync when the incoming prop actually changed from a previous prop value and the user hasn't modified the field—adjust the useEffect bodies for setSearchText and setSelectedRegion accordingly in UniversityListContent.apps/web/src/components/ui/UniverSityCard/index.tsx (1)
18-24: 슬러그 매핑이 없는 대학 카드를 클릭하면 상세 페이지가 아닌 목록 페이지로 이동합니다.현재 로직을 보면:
mappedHomeUniversitySlug이 없고linkPrefix가 기본값"/university"일 때- href가
"/university"(ID 없음)로 설정됩니다이 패턴은
PopularUniversityCard에서도 동일하게 사용되고 있어 의도된 설계로 보이지만, 대학 카드를 표시한 후 클릭 시 상세 페이지가 아닌 목록 페이지로 이동하는 것은 사용자 경험 관점에서 개선할 여지가 있습니다.현재
HOME_UNIVERSITY_TO_SLUG_MAP에는 인하, 인천, 성신 3개 대학만 매핑되어 있으므로, 매핑되지 않은 대학들은 모두 이 폴백 경로를 사용합니다.📋 개선 제안
현재
hasExplicitPrefix변수는 결국 사용되지 않으므로, 슬러그가 없을 때도 항상 ID를 포함하도록 단순화할 수 있습니다:const mappedHomeUniversitySlug = getHomeUniversitySlugByName(university.homeUniversityName); - const hasExplicitPrefix = linkPrefix !== "/university"; const universityDetailHref = mappedHomeUniversitySlug ? `/university/${mappedHomeUniversitySlug}/${university.id}` - : hasExplicitPrefix - ? `${linkPrefix}/${university.id}` - : "/university"; + : `${linkPrefix}/${university.id}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ui/UniverSityCard/index.tsx` around lines 18 - 24, The current universityDetailHref logic falls back to "/university" (no ID) when getHomeUniversitySlugByName(university.homeUniversityName) returns falsy and linkPrefix is the default "/university", causing clicks to go to the list page; update the logic in UniverSityCard (and mirror in PopularUniversityCard) to always include the university.id when mappedHomeUniversitySlug is missing by removing the unused hasExplicitPrefix check and using `${linkPrefix}/${university.id}` as the fallback; reference getHomeUniversitySlugByName, mappedHomeUniversitySlug, linkPrefix, universityDetailHref, PopularUniversityCard and HOME_UNIVERSITY_TO_SLUG_MAP to locate and apply the change.apps/web/src/app/university/[homeUniversity]/page.tsx (2)
115-119:university.region as RegionEnumExtend타입 캐스팅이 안전하지 않을 수 있습니다.API 응답의
region값이RegionEnumExtend에 없는 값이라면as캐스팅으로 인해 타입 안전성이 보장되지 않습니다. 기능적으로는includes()에서 매칭되지 않아 필터링될 뿐이지만, 명시적으로 타입 가드를 사용하면 의도가 더 명확해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/`[homeUniversity]/page.tsx around lines 115 - 119, The current cast university.region as RegionEnumExtend is unsafe; replace it with a runtime type guard that confirms university.region is a valid RegionEnumExtend before calling regionParams.includes. Implement a small predicate (e.g., isValidRegion) that checks Object.values(RegionEnumExtend).includes(value) and then change the filter to: check isValidRegion(university.region) && regionParams.includes(university.region) so the intent is explicit and typing is preserved for regionParams/includes calls.
56-63:isRegionFilterValue함수가 개별 값을 하드코딩하고 있어 유지보수에 취약합니다.
RegionEnumExtend에 새로운 지역이 추가되면 이 함수도 수동으로 업데이트해야 합니다.ALL만 제외하면 되므로, enum 값 기반으로 작성하면 더 안전합니다.♻️ 개선 제안
const isRegionFilterValue = (value: string): value is RegionEnumExtend => { - return ( - value === RegionEnumExtend.AMERICAS || - value === RegionEnumExtend.EUROPE || - value === RegionEnumExtend.ASIA || - value === RegionEnumExtend.CHINA - ); + const filterableRegions = Object.values(RegionEnumExtend).filter((v) => v !== RegionEnumExtend.ALL); + return filterableRegions.includes(value as RegionEnumExtend); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/`[homeUniversity]/page.tsx around lines 56 - 63, The isRegionFilterValue function currently hardcodes each region; change it to check membership against RegionEnumExtend dynamically and exclude RegionEnumExtend.ALL instead of listing values manually — e.g., use Object.values(RegionEnumExtend) (cast to string[] if needed) to see if value is one of the enum values and then ensure value !== RegionEnumExtend.ALL; update the isRegionFilterValue function to return that boolean so new enum members are handled automatically.apps/web/src/app/university/application/_components/ApplicationBottomActionBar.tsx (1)
10-14:z-index부재로 인한 레이어링 문제 가능성
fixed포지셔닝에z-index가 지정되어 있지 않습니다.
- 영향 범위
UniversityStep처럼 Radix UISelect를 함께 사용하는 스텝에서, Select 드롭다운(body에 포털로 마운트됨)이 열릴 때 하단 바와의 레이어 순서가 의도와 다를 수 있습니다.- 권고 사항
- 프로젝트의 z-index 스케일에 맞는 값(
z-10,z-20등)을 명시적으로 추가하는 것을 권장합니다.🔧 제안 수정
- <div className="fixed bottom-14 w-full max-w-app bg-white"> + <div className="fixed bottom-14 z-10 w-full max-w-app bg-white">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/application/_components/ApplicationBottomActionBar.tsx` around lines 10 - 14, The fixed bottom bar (ApplicationBottomActionBar) lacks a z-index, causing layering conflicts with portal-mounted elements like Radix UI Select; update the root fixed div (the element rendering BlockBtn) to include a project z-index utility (e.g., add "z-10" or "z-20" to its className) so the bottom action bar stacks above/below popovers as intended.apps/web/src/app/university/application/ScorePageContent.tsx (1)
126-126:hotKeyWords를 컴포넌트 외부로 이동하세요.
- 현황
hotKeyWords배열이 컴포넌트 함수 내부에 정의되어 있어, 렌더링마다 새 배열 인스턴스가 생성됩니다.- 제안
- 정적 상수이므로 컴포넌트 외부로 이동하면 불필요한 재생성을 방지할 수 있습니다.
♻️ 제안 수정
+const HOT_KEYWORDS = ["RMIT", "오스트라바", "칼스루에", "그라츠", "추오", "프라하", "보라스", "빈", "메모리얼"]; const ScorePageContent = () => { ... - const hotKeyWords = ["RMIT", "오스트라바", "칼스루에", "그라츠", "추오", "프라하", "보라스", "빈", "메모리얼"]; ... - <ScoreSearchField keyWords={hotKeyWords} setKeyWord={handleSearchField} /> + <ScoreSearchField keyWords={HOT_KEYWORDS} setKeyWord={handleSearchField} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/application/ScorePageContent.tsx` at line 126, Move the hotKeyWords array out of the ScorePageContent component and declare it as a top‑level constant (e.g., const HOT_KEY_WORDS = [...]) so a single static instance is reused across renders; update ScorePageContent to reference that constant (keep the same identifier or rename consistently) and ensure no other logic depends on it being recreated inside the component.apps/web/src/app/university/application/apply/UniversityStep.tsx (1)
49-114: 1지망/2지망/3지망 Select 블록의 코드 중복 — 배열 map으로 개선 가능
- 현황
- 3개 Select 블록이 index와 placeholder 텍스트만 다르고 나머지 구조가 동일하게 반복됩니다.
- 제안
- 배열
map으로 리팩토링하면 향후 지망 수 변경 시에도 한 곳만 수정하면 됩니다.♻️ 제안 리팩토링
+ const PREFERENCE_OPTIONS = [ + { label: "1지망", placeholder: "학교를 선택해주세요" }, + { label: "2지망", placeholder: "선택 안 함" }, + { label: "3지망", placeholder: "선택 안 함" }, + ]; <div className="mt-4 flex flex-col gap-4"> - <div className="flex flex-col gap-1"> - <label className="block text-k-800 typo-medium-2">1지망</label> - <Select value={...} onValueChange={...}> - ... - </Select> - </div> - {/* 2지망, 3지망도 동일 */} + {PREFERENCE_OPTIONS.map(({ label, placeholder }, index) => ( + <div key={label} className="flex flex-col gap-1"> + <label className="block text-k-800 typo-medium-2">{label}</label> + <Select + value={curUniversityList[index]?.toString() ?? undefined} + onValueChange={(value) => handleSelect(index, parseInt(value, 10))} + > + <SelectTrigger> + <SelectValue placeholder={placeholder} /> + </SelectTrigger> + <SelectContent> + {universityList.map((university) => ( + <SelectItem + key={university.id} + value={university.id.toString()} + disabled={isDisabled(university.id, index)} + > + [{university.region} - {university.country}]{university.koreanName} + </SelectItem> + ))} + </SelectContent> + </Select> + </div> + ))} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/application/apply/UniversityStep.tsx` around lines 49 - 114, The three nearly identical Select blocks (for 1지망/2지망/3지망) should be collapsed into a single mapped structure: create an array like [ {label: '1지망', placeholder: '학교를 선택해주세요'}, {label: '2지망', placeholder: '선택 안 함'}, {label: '3지망', placeholder: '선택 안 함'} ] and map over it to render the Select UI, using the index to wire value={curUniversityList[index]?.toString() ?? null}, onValueChange={(value) => handleSelect(index, parseInt(value, 10))}, and disabled={isDisabled(university.id, index)} while keeping universityList, SelectTrigger, SelectContent, SelectItem, and SelectValue logic intact; this removes duplication and keeps behavior of curUniversityList, handleSelect, universityList, and isDisabled unchanged.apps/web/src/app/university/application/apply/LanguageStep.tsx (1)
27-33: 💡 (선택 사항)handleNext패턴 중복 추출 고려
GpaStep과LanguageStep모두 완전히 동일한 구조의handleNext를 가지고 있습니다.curScore === null → setIsModalOpen(true) / return onNext()현재 두 곳에서만 사용되고 로직도 단순해서 당장 문제는 아닙니다. 다만 스텝이 늘어날 경우를 대비해 공통 훅으로 분리하는 방안을 검토해 보시면 좋겠습니다.
♻️ 추출 예시 (참고용)
// _hooks/useStepGuard.ts export function useStepGuard(curScore: number | null, onNext: () => void) { const [isModalOpen, setIsModalOpen] = useState(false); const handleNext = () => { if (curScore === null) { setIsModalOpen(true); return; } onNext(); }; return { isModalOpen, setIsModalOpen, handleNext }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/application/apply/LanguageStep.tsx` around lines 27 - 33, Both LanguageStep and GpaStep duplicate the same handleNext pattern (check curScore === null → setIsModalOpen(true) → return → onNext()); extract this into a reusable hook (e.g., useStepGuard) that accepts the current score/state and onNext callback and returns { isModalOpen, setIsModalOpen, handleNext }; replace the local handleNext, isModalOpen and setIsModalOpen in LanguageStep (symbols: handleNext, curLanguageTestScore, setIsModalOpen) and GpaStep with the hook to remove duplication and centralize the guard logic.apps/web/src/app/university/application/apply/ConfirmStep.tsx (1)
15-15:ApplicationBottomActionBar배치 패턴 일관성 개선 제안
GpaStep과LanguageStep에서는ApplicationBottomActionBar가 콘텐츠<div>외부(fragment 자식)에 위치하는 반면,ConfirmStep에서는pb-40패딩을 가진 컨테이너 내부에 위치합니다. 실제로 배치 패턴이 다릅니다.다만,
ApplicationBottomActionBar는fixed포지셔닝(className="fixed bottom-14 ...)을 사용하므로:
- 레이아웃 영향 없음 — Fixed 위치 요소는 DOM 계층 구조와 무관하게 뷰포트 기준으로 렌더링됩니다.
- 시각적으로 동일 — 배치 위치가 다르더라도 시각 결과는 같습니다.
따라서 현재 구조는 기능상 문제없습니다. 다만 코드 일관성과 유지보수성을 위해 다른 Step들과 동일하게 컨테이너 외부에 배치하는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/university/application/apply/ConfirmStep.tsx` at line 15, ConfirmStep places ApplicationBottomActionBar inside the padded container (<div className="my-5 px-5 pb-40">) while GpaStep and LanguageStep render ApplicationBottomActionBar as a sibling outside that container; move the ApplicationBottomActionBar (the element with className including "fixed bottom-14") out of the inner <div> so it is a sibling/fragment child like in GpaStep and LanguageStep to keep layout pattern consistent, leaving the padded container unchanged.packages/bruno-api-typescript/src/generator/apiFactoryGenerator.ts (1)
56-69:if (rawVarName === "URL") return ""— 도달 불가능한 데드 코드
- 원인
splitRequestUrl(url)(Line 48)이url.replace(/\{\{URL\}\}/g, "")를 통해 이미{{URL}}을 제거합니다.- 따라서
requestPath에는{{URL}}이 존재하지 않고,rawVarName === "URL"조건은 절대true가 되지 않습니다.♻️ 제안 정리
processedUrl = processedUrl.replace(/\{\{([^}]+)\}\}/g, (_fullMatch, rawVarName: string) => { - if (rawVarName === "URL") { - return ""; - } - const camelVarName = rawVarName.replace(/-([a-z])/g, (_, character: string) => character.toUpperCase());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/apiFactoryGenerator.ts` around lines 56 - 69, The branch checking rawVarName === "URL" inside the processedUrl replacement is dead because splitRequestUrl already strips {{URL}}; remove the unreachable if (rawVarName === "URL") return "" branch from the processedUrl.replace handler in apiFactoryGenerator.ts (the block that updates urlParams, paramsList and processedBrunoVars) so the replacement logic only handles real variables, or if you prefer preserve intent, move the {{URL}} stripping responsibility into this handler and remove it from splitRequestUrl (refer to splitRequestUrl, processedBrunoVars, urlParams, paramsList, and the processedUrl = processedUrl.replace(...) callback to locate the code).packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts (2)
24-26: URL 정규화 로직이 세 파일에 걸쳐 중복됩니다
현황
apiClientGenerator.ts→normalizeUrlPathapiDefinitionGenerator.ts→normalizeApiPathapiFactoryGenerator.ts→splitRequestUrl(같은 로직 +hasQuery플래그)권장 사항
- 공유 유틸리티(예:
packages/bruno-api-typescript/src/generator/urlUtils.ts)로 추출하고hasQuery도 함께 반환하도록 통합하면 세 곳의 동기화 부담을 없앨 수 있습니다.// urlUtils.ts (신규) export function normalizeRequestUrl(url: string): { path: string; hasQuery: boolean } { const base = url.replace(/\{\{URL\}\}/g, ""); const qi = base.indexOf("?"); return qi === -1 ? { path: base, hasQuery: false } : { path: base.slice(0, qi), hasQuery: true }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts` around lines 24 - 26, The three duplicated URL normalization implementations (normalizeApiPath in apiDefinitionGenerator.ts, normalizeUrlPath in apiClientGenerator.ts, and splitRequestUrl in apiFactoryGenerator.ts) should be consolidated into a shared utility: create a new urlUtils.ts exporting normalizeRequestUrl that returns { path, hasQuery } (strip "{{URL}}" and detect '?' presence), then replace calls to normalizeApiPath/normalizeUrlPath/splitRequestUrl to use normalizeRequestUrl and adjust call sites to read the path and hasQuery flag accordingly; ensure splitRequestUrl consumers get the hasQuery behavior previously provided.
38-45:if (varName === "URL") continue— 도달 불가능한 데드 코드
원인
extractPathParams는normalizeApiPath(url)을 먼저 호출하는데(Line 38), 이 함수가{{URL}}을 이미 공백 문자열로 치환합니다.- 이후
brunoVarPattern(\{\{([^}]+)\}\})이 매칭할 수 있는URL변수명은 결코 존재하지 않습니다.추가로,
extractPathParams는 이미 정규화된normalizedPath를 받아 내부에서 다시normalizeApiPath를 호출(Line 38)하므로 이중 정규화가 발생합니다.♻️ 제안 정리 (두 군데 정리)
function extractPathParams(url: string): string[] { const params: string[] = []; - - const processedUrl = normalizeApiPath(url); + // 호출부(generateApiDefinitionMeta)에서 이미 정규화한 경로를 받으므로 + // 재정규화 없이 url을 그대로 사용합니다. + const processedUrl = url; const brunoVarPattern = /\{\{([^}]+)\}\}/g; for (const match of processedUrl.matchAll(brunoVarPattern)) { const varName = match[1]; - if (varName === "URL") { - continue; - } - const camelVarName = varName.replace(/-([a-z])/g, (_fullMatch, character: string) => character.toUpperCase());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts` around lines 38 - 45, The check for `if (varName === "URL") continue` is dead code because `normalizeApiPath` already strips `{{URL}}`, and you're also double-normalizing the path; to fix, remove the redundant initial or internal normalization so the path is normalized only once (decide whether to call `normalizeApiPath` outside or inside `extractPathParams`) and delete the unreachable `if (varName === "URL")` branch; update references to `processedUrl`, `extractPathParams`, and the `brunoVarPattern` handling accordingly to use the single normalized value.
Comment on lines
18
to
22
| force_redeploy: | ||
| description: "When up to date, skip divergence and print manual redeploy guidance" | ||
| required: true | ||
| default: false | ||
| type: boolean |
There was a problem hiding this comment.
force_redeploy=true가 실제 재배포를 트리거하지 않아 이름이 오해를 유발합니다.
현재 동작을 정리해 드릴게요:
-
FORCE_REDEPLOY=true인 경우 (lines 107–114)- 확장된 안내 메시지를 Step Summary에 기록
continue로 루프 반복을 종료 (push 없음)
-
FORCE_REDEPLOY=false인 경우 (lines 117–120)- 간단한 메시지를 Step Summary에 기록
- 마찬가지로
continue로 종료 (push 없음)
두 경우 모두 동작이 동일합니다 — 차이는 summary 메시지의 상세도뿐이에요. force_redeploy 라는 이름을 보면 사용자는 "Vercel 재배포가 자동으로 트리거되겠구나"라고 기대하게 됩니다. 혼란을 줄이기 위해 입력 이름과 설명을 실제 동작에 맞게 맞춰주시면 좋겠습니다.
✏️ 이름 변경 제안 예시
- force_redeploy:
- description: "When up to date, skip divergence and print manual redeploy guidance"
+ show_redeploy_guidance:
+ description: "When up to date, print manual Vercel redeploy instructions in step summary"
required: true
default: false
type: boolean그리고 스크립트 내부에서도:
- FORCE_REDEPLOY="${{ github.event.inputs.force_redeploy }}"
+ SHOW_REDEPLOY_GUIDANCE="${{ github.event.inputs.show_redeploy_guidance }}"- if [ "$FORCE_REDEPLOY" = "true" ]; then
+ if [ "$SHOW_REDEPLOY_GUIDANCE" = "true" ]; thenAlso applies to: 107-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 18 - 22, The input named
force_redeploy (env var FORCE_REDEPLOY) is misleading because it does not
trigger a real redeploy; rename it to reflect current behavior (e.g.,
show_manual_redeploy_guidance or verbose_redeploy_summary) and update its
description to: "When true, write extended manual redeploy guidance to the Step
Summary; does not trigger an automatic Vercel redeploy." Replace all occurrences
of force_redeploy and FORCE_REDEPLOY in the workflow and the step that writes
the Step Summary (the conditional block that currently logs extended vs simple
guidance) so the variable name, default, and help text match the behavior and
the summary messages mention that no automatic redeploy is performed.
Comment on lines
123
to
129
| if ! git merge-base --is-ancestor "origin/$BRANCH" origin/main; then | ||
| echo "$BRANCH is not an ancestor of main. Resolve release history before promotion." >&2 | ||
| exit 1 | ||
| { | ||
| echo "- $BRANCH: non-ancestor detected, forcing reset to main" | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
| fi | ||
|
|
||
| git push origin origin/main:"refs/heads/$BRANCH" | ||
| git push --force-with-lease origin origin/main:"refs/heads/$BRANCH" |
There was a problem hiding this comment.
non-ancestor 브랜치에 대한 강제 푸시가 릴리즈 브랜치 커밋을 덮어씁니다.
변경 전후를 비교해 드릴게요:
-
변경 전 (추정)
origin/$BRANCH가origin/main의 조상이 아닌 경우 →exit 1로 안전하게 중단
-
변경 후 (현재 코드)
- non-ancestor 감지 시 → Step Summary에 메시지 기록 후 그대로 진행
git push --force-with-lease origin origin/main:"refs/heads/$BRANCH"실행 (line 129)
이 변경으로 인해, 릴리즈 브랜치에 main을 거치지 않고 직접 커밋된 내용이 있다면 경고 없이 영구적으로 삭제됩니다. --force-with-lease는 fetch 이후 동시 변경을 막아줄 뿐, 브랜치 이력 발산 자체를 막지는 않아요.
의도적인 변경이라면 — 워크플로우가 승인된 배포 경로임을 팀이 공유하고 있다면 — 적어도 Step Summary에 "이 릴리즈 브랜치의 커밋이 사라질 수 있습니다"라는 명확한 경고를 추가하거나, 별도의 수동 확인 단계를 두는 것을 권장합니다.
⚠️ 제안: 더 명확한 경고 메시지 예시
if ! git merge-base --is-ancestor "origin/$BRANCH" origin/main; then
{
- echo "- $BRANCH: non-ancestor detected, forcing reset to main"
+ echo "- $BRANCH: ⚠️ non-ancestor detected — commits on $BRANCH not in main will be LOST after force push"
} >> "$GITHUB_STEP_SUMMARY"
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 123 - 129, The workflow currently
detects a non-ancestor via git merge-base --is-ancestor "origin/$BRANCH"
origin/main but still runs git push --force-with-lease origin
origin/main:"refs/heads/$BRANCH", risking silent deletion of release commits;
change the behavior so that when the conditional (! git merge-base --is-ancestor
...) is true you either (A) fail the step (append the warning to
GITHUB_STEP_SUMMARY and exit 1) to block the force-push, or (B) require an
explicit, auditable confirmation before allowing the force-push (e.g., check an
input/env flag like CONFIRM_FORCE_PUSH=true or a manual approval job) and
include a prominent warning in the Step Summary that this will overwrite branch
history; update the block around git merge-base and the git push
--force-with-lease line accordingly.
Comment on lines
18
to
29
| export function GpaScoreTable({ verifyFilter }: Props) { | ||
| const [scores, setScores] = useState<GpaScoreWithUser[]>([]); | ||
| const queryClient = useQueryClient(); | ||
| const [page, setPage] = useState(1); | ||
| const [totalPages, setTotalPages] = useState(1); | ||
| const [loading, setLoading] = useState(false); | ||
| const [editingId, setEditingId] = useState<number | null>(null); | ||
| const [editingGpa, setEditingGpa] = useState<number>(0); | ||
| const [editingGpaCriteria, setEditingGpaCriteria] = useState<number>(0); | ||
|
|
||
| const fetchScores = useCallback(async () => { | ||
| setLoading(true); | ||
| try { | ||
| const response = await scoreApi.getGpaScores({ verifyStatus: verifyFilter }, page); | ||
| setScores(response.content); | ||
| setTotalPages(response.totalPages); | ||
| } catch (error) { | ||
| console.error("Failed to fetch GPA scores:", error); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }, [verifyFilter, page]); | ||
| const { data, isLoading, isFetching } = useQuery({ | ||
| queryKey: ["scores", "gpa", verifyFilter, page], | ||
| queryFn: () => scoreApi.getGpaScores({ verifyStatus: verifyFilter }, page), | ||
| placeholderData: keepPreviousData, | ||
| }); |
There was a problem hiding this comment.
verifyFilter 변경 시 page가 1로 리셋되지 않습니다.
LanguageScoreTable과 동일한 이슈입니다. verifyFilter가 변경될 때 page를 1로 리셋해주세요.
🛠️ 제안하는 수정
+import { useState, useEffect } from "react";
-import { useState } from "react";
export function GpaScoreTable({ verifyFilter }: Props) {
const queryClient = useQueryClient();
const [page, setPage] = useState(1);
+
+ useEffect(() => {
+ setPage(1);
+ }, [verifyFilter]);
+
const [editingId, setEditingId] = useState<number | null>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/components/features/scores/GpaScoreTable.tsx` around lines 18
- 29, GpaScoreTable does not reset pagination when verifyFilter changes; add a
side-effect that calls setPage(1) whenever the verifyFilter prop updates (e.g.,
in a useEffect that depends on verifyFilter) so the queryKey
["scores","gpa",verifyFilter,page] always starts at page 1 for a new filter;
reference the GpaScoreTable component and the page / setPage state variables to
locate where to add the effect.
Comment on lines
33
to
44
| export function LanguageScoreTable({ verifyFilter }: Props) { | ||
| const [scores, setScores] = useState<LanguageScoreWithUser[]>([]); | ||
| const queryClient = useQueryClient(); | ||
| const [page, setPage] = useState(1); | ||
| const [totalPages, setTotalPages] = useState(1); | ||
| const [loading, setLoading] = useState(false); | ||
| const [editingId, setEditingId] = useState<number | null>(null); | ||
| const [editingScore, setEditingScore] = useState<string>(""); | ||
| const [editingType, setEditingType] = useState<LanguageTestType>("TOEIC"); | ||
|
|
||
| const fetchScores = useCallback(async () => { | ||
| setLoading(true); | ||
| try { | ||
| const response = await scoreApi.getLanguageScores({ verifyStatus: verifyFilter }, page); | ||
| setScores(response.content); | ||
| setTotalPages(response.totalPages); | ||
| } catch (error) { | ||
| console.error("Failed to fetch Language scores:", error); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }, [verifyFilter, page]); | ||
| const { data, isLoading, isFetching } = useQuery({ | ||
| queryKey: ["scores", "language", verifyFilter, page], | ||
| queryFn: () => scoreApi.getLanguageScores({ verifyStatus: verifyFilter }, page), | ||
| placeholderData: keepPreviousData, | ||
| }); |
There was a problem hiding this comment.
verifyFilter 변경 시 page가 1로 리셋되지 않습니다.
현재 verifyFilter가 변경되면 page state는 이전 값을 유지합니다. 예를 들어 사용자가 3페이지를 보다가 필터를 변경하면, 새 필터 결과의 3페이지를 요청하게 됩니다. 해당 페이지가 존재하지 않으면 빈 데이터가 표시될 수 있습니다.
useEffect로 verifyFilter 변경 시 page를 1로 리셋하는 것을 권장합니다.
🛠️ 제안하는 수정
+import { useState, useEffect } from "react";
-import { useState } from "react";
export function LanguageScoreTable({ verifyFilter }: Props) {
const queryClient = useQueryClient();
const [page, setPage] = useState(1);
+
+ useEffect(() => {
+ setPage(1);
+ }, [verifyFilter]);
+
const [editingId, setEditingId] = useState<number | null>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/components/features/scores/LanguageScoreTable.tsx` around
lines 33 - 44, In LanguageScoreTable add a useEffect that resets page to 1
whenever verifyFilter changes: inside the LanguageScoreTable component (near the
useState declarations for page/setPage) add useEffect(() => setPage(1),
[verifyFilter]) so changing verifyFilter always resets the current page to the
first page; place it after the page state and before the useQuery call to ensure
queries use the reset page.
Comment on lines
37
to
43
| if ("to" in menu) { | ||
| return ( | ||
| <a key={menu.label} href={menu.to} className={menuClassName}> | ||
| <menu.icon className="h-4 w-4" /> | ||
| {menu.label} | ||
| </a> | ||
| ); |
There was a problem hiding this comment.
🚨 <a href> 사용으로 TanStack Router 클라이언트 사이드 내비게이션이 우회됩니다
현재 내부 라우트(/scores, /bruno)로 이동 시 일반 <a> 태그를 사용하고 있어 전체 페이지 리로드가 발생합니다. TanStack Router의 Link 컴포넌트는 전체 페이지 리로드 없이 클라이언트 사이드 내비게이션을 가능하게 하는 타입 안전한 앵커 요소입니다. 또한 Link를 사용하면 라우트 프리로딩, active state 감지 등의 기능도 자동으로 활용할 수 있습니다.
🛠️ Link 컴포넌트로 교체 제안
+import { Link } from "@tanstack/react-router";
import { Building2, FileText, FlaskConical, UserCircle2 } from "lucide-react";
import { cn } from "@/lib/utils";- <a key={menu.label} href={menu.to} className={menuClassName}>
+ <Link key={menu.label} to={menu.to} className={menuClassName}>
<menu.icon className="h-4 w-4" />
{menu.label}
- </a>
+ </Link>📝 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
| if ("to" in menu) { | |
| return ( | |
| <a key={menu.label} href={menu.to} className={menuClassName}> | |
| <menu.icon className="h-4 w-4" /> | |
| {menu.label} | |
| </a> | |
| ); | |
| if ("to" in menu) { | |
| return ( | |
| <Link key={menu.label} to={menu.to} className={menuClassName}> | |
| <menu.icon className="h-4 w-4" /> | |
| {menu.label} | |
| </Link> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/components/layout/AdminSidebar.tsx` around lines 37 - 43,
Replace the plain anchor with TanStack Router's Link to enable client-side
navigation: in the AdminSidebar component where you render items for menus with
"to" (the block that returns <a key={menu.label} href={menu.to}
className={menuClassName}>...), import and use Link from `@tanstack/router` (or
your app's router export) and pass menu.to as the to prop, keeping
key={menu.label}, className={menuClassName}, and rendering <menu.icon /> and
{menu.label} unchanged; ensure you remove href usage so navigation and active
state/preloading work correctly.
apps/web/src/app/(home)/_ui/PopularUniversitySection/_ui/PopularUniversityCard.tsx
Show resolved
Hide resolved
Comment on lines
43
to
45
| // 재발급 실패 시 로그인 페이지로 리다이렉트 | ||
| setRefreshStatus("failed"); | ||
| router.push("/login?redirect=/mentor"); | ||
| router.push("/login"); |
There was a problem hiding this comment.
1. 로그인 후 /mentor 복귀 경로가 소실됩니다
토큰 재발급 실패 시 /login?redirect=/mentor로 이동하던 것이 /login으로 단순화되어, 로그인 성공 후 사용자가 /mentor가 아닌 루트(/)로 이동하게 됩니다. PR 전체 방향(redirect 파라미터 제거)과 일치하는 것으로 보이지만, 멘토 페이지에서 로그인이 강제된 사용자의 경험이 한 단계 더 복잡해질 수 있습니다. 의도적인 변경이라면 넘어가도 무방합니다. 🙂
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/mentor/_ui/MentorClient/index.tsx` around lines 43 - 45, The
redirect-to-login call on token refresh failure drops the original /mentor
return path; update the router.push in MentorClient so it includes a redirect
query (e.g., use router.push({ pathname: "/login", query: { redirect:
router.asPath || "/mentor" } })) instead of router.push("/login"), and keep
setRefreshStatus("failed") as-is; locate the call site where setRefreshStatus
and router.push are used and replace the plain push with a push that preserves
the current path or a fallback of "/mentor".
Comment on lines
52
to
68
| value={curUniversityList[0]?.toString() ?? null} | ||
| onValueChange={(value: string) => handleSelect(0, parseInt(value, 10))} | ||
| > | ||
| <SelectTrigger> | ||
| <SelectValue placeholder="학교를 선택해주세요" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {universityList.map((university) => ( | ||
| <SelectItem | ||
| key={university.id} | ||
| value={university.id.toString()} | ||
| disabled={isDisabled(university.id, 0)} | ||
| > | ||
| [{university.region} - {university.country}]{university.koreanName} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 1. Locate and examine UniversityStep.tsx
find . -name "UniversityStep.tsx" -type fRepository: solid-connection/solid-connect-web
Length of output: 144
🏁 Script executed:
# 2. Get an overview of files in the directory structure
git ls-files apps/web/src/app/university/application/apply/ 2>/dev/null | head -20Repository: solid-connection/solid-connect-web
Length of output: 565
🏁 Script executed:
# 3. Search for Select component definition/wrapper
find . -name "*Select*" -type f | grep -E "\.(tsx?|jsx?)$" | head -20Repository: solid-connection/solid-connect-web
Length of output: 551
🏁 Script executed:
# Read UniversityStep.tsx with context around lines 52-68
wc -l ./apps/web/src/app/university/application/apply/UniversityStep.tsxRepository: solid-connection/solid-connect-web
Length of output: 148
🏁 Script executed:
# Read the entire file to understand the structure
cat -n ./apps/web/src/app/university/application/apply/UniversityStep.tsxRepository: solid-connection/solid-connect-web
Length of output: 5855
🏁 Script executed:
# Examine the Select.tsx component definition
cat -n ./apps/web/src/components/ui/Select.tsxRepository: solid-connection/solid-connect-web
Length of output: 7202
🏁 Script executed:
# Check for TypeScript declarations or type overrides for Select
find . -name "*.d.ts" -o -name "types.ts" | xargs rg -l "Select" 2>/dev/null | head -10Repository: solid-connection/solid-connect-web
Length of output: 60
🏁 Script executed:
# Check package.json for Radix UI version
cat ./package.json | grep -A5 -B5 "@radix-ui/react-select"Repository: solid-connection/solid-connect-web
Length of output: 60
🏁 Script executed:
# Search for any type augmentation or custom type definitions
rg "type.*Select.*=" --type=tsx --type=ts | grep -i "value\|prop" | head -20Repository: solid-connection/solid-connect-web
Length of output: 105
🏁 Script executed:
# Check if there are any other usages of null with Select in the codebase
rg "value=.*?? null" --type=tsx --type=tsRepository: solid-connection/solid-connect-web
Length of output: 105
🏁 Script executed:
# Check package.json for Radix UI version
cat ./apps/web/package.json 2>/dev/null || cat ./package.json 2>/dev/null | grep "@radix-ui"Repository: solid-connection/solid-connect-web
Length of output: 1937
🏁 Script executed:
# Search for other Select usages with null values
rg "value=.*\?\? null" -A2 -B2Repository: solid-connection/solid-connect-web
Length of output: 1859
🏁 Script executed:
# Check Radix UI React Select type definitions to understand expected value type
npm list `@radix-ui/react-select` 2>/dev/null || rg "@radix-ui/react-select" package.jsonRepository: solid-connection/solid-connect-web
Length of output: 124
🏁 Script executed:
# Look for any TypeScript errors or type assertions around Select
rg "Select.*value" -A1 -B1 | head -40Repository: solid-connection/solid-connect-web
Length of output: 3395
🏁 Script executed:
# Verify TypeScript build to see if null causes type errors
cd apps/web && npm run typecheck 2>&1 | grep -i "select\|null\|value" || echo "No TypeScript errors for Select"Repository: solid-connection/solid-connect-web
Length of output: 109
🏁 Script executed:
# Check the actual type definition for Radix UI Select value prop
# Look in node_modules or search for documentation
find . -path "*/node_modules/@radix-ui/react-select*" -name "*.d.ts" 2>/dev/null | head -5Repository: solid-connection/solid-connect-web
Length of output: 60
🏁 Script executed:
# Search for Radix UI version info and type compatibility
python3 << 'EOF'
import subprocess
import json
# Check if value can be null in Radix UI
result = subprocess.run(['rg', '@radix-ui/react-select', '--type=json'],
capture_output=True, text=True)
print("Version info:", result.stdout)
# Also check if there are any type issues reported
result2 = subprocess.run(['rg', 'null.*string|string.*null'],
capture_output=True, text=True)
print("\nNull/string patterns:", result2.stdout[:500] if result2.stdout else "No patterns found")
EOFRepository: solid-connection/solid-connect-web
Length of output: 133
🏁 Script executed:
# Let's check the actual Radix UI documentation or types by examining how it's used elsewhere
rg "SelectPrimitive.Root" -A10 | head -30Repository: solid-connection/solid-connect-web
Length of output: 853
Select 컴포넌트의 value에 null 대신 undefined를 전달하세요.
현재 코드에서 value={curUniversityList[0]?.toString() ?? null} 패턴을 사용하면, 배열이 비어있을 때 Select의 value prop으로 null을 전달합니다. Radix UI의 공식 정의에 따르면 value prop은 string | undefined만 허용하며, undefined가 placeholder 표시를 위한 선택 해제 상태입니다.
-
null은 공식 지원 값이 아님- Radix UI 문서에 명시된 무선택 상태는
undefined입니다. null을 사용하면 의도하지 않은 동작이나 타입 불일치가 발생할 수 있습니다.
- Radix UI 문서에 명시된 무선택 상태는
-
수정 필요 항목
- 1지망, 2지망, 3지망 Select 모두에서
?? null→?? undefined로 교체 - TextModal의
title=""속성 제거 또는 실제 제목 텍스트 추가 (현재 빈 제목 렌더링)
- 1지망, 2지망, 3지망 Select 모두에서
-
선택사항: 코드 중복 정리
- 3개의 Select 블록이 거의 동일하므로, 배열 기반 루프(
map)로 리팩토링하면 유지보수성이 향상됩니다.
- 3개의 Select 블록이 거의 동일하므로, 배열 기반 루프(
🔧 필수 수정 (Select value props)
- value={curUniversityList[0]?.toString() ?? null}
+ value={curUniversityList[0]?.toString() ?? undefined}
- value={curUniversityList[1]?.toString() ?? null}
+ value={curUniversityList[1]?.toString() ?? undefined}
- value={curUniversityList[2]?.toString() ?? null}
+ value={curUniversityList[2]?.toString() ?? undefined}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/university/application/apply/UniversityStep.tsx` around
lines 52 - 68, The Select components are passing null to the value prop which
expects string | undefined; update the three Select value expressions (where you
currently use curUniversityList[0]?.toString() ?? null) to use ?? undefined
instead so the no-selection state is undefined; ensure the same change is
applied for all three selects that call handleSelect(index, parseInt(...)) and
use isDisabled(...) for items; also remove the empty title attribute from
TextModal (or provide a real title string) to avoid rendering an empty title
element.
| score={score.languageTestResponse.languageTestScore} | ||
| status={score.verifyStatus} | ||
| // date={new Date(score.issueDate).toISOString()} | ||
| date="2026-01-01" |
There was a problem hiding this comment.
하드코딩된 date="2026-01-01" — 프로덕션에서 잘못된 날짜가 노출돼요.
현재 ScoreCard에 전달되는 날짜값이 두 탭 모두 고정값으로 되어 있어, 실제 사용자에게는 항상 2026-01-01로 보이게 됩니다.
-
해당 위치
- Line 42: 공인어학 탭 ScoreCard
- Line 56: 학점 탭 ScoreCard
-
원인
- 원래 의도된 구현(
new Date(score.issueDate).toISOString())이 주석 처리되어 있고, 임시 플레이스홀더가 그대로 남아 있는 상태예요.
- 원래 의도된 구현(
-
제안
- 릴리즈 전에 주석을 해제하거나,
score.issueDate가 아직 API에서 내려오지 않는다면 해당 필드가 준비될 때까지ScoreCard에서 날짜 표시 자체를 조건부로 숨기는 방향도 고려해 보세요.
- 릴리즈 전에 주석을 해제하거나,
🛠️ 수정 제안
- // date={new Date(score.issueDate).toISOString()}
- date="2026-01-01"
+ date={new Date(score.issueDate).toISOString()}Also applies to: 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/university/score/ScoreScreen.tsx` at line 42, The ScoreCard
instances in ScoreScreen are passing a hardcoded date ("2026-01-01") instead of
using the real issue date; revert to using the original value by restoring new
Date(score.issueDate).toISOString() when score.issueDate exists (e.g., in the
ScoreCard props for both tab usages) or, if the API may not provide issueDate
yet, change ScoreCard rendering to conditionally omit the date display when
score.issueDate is falsy; update the two ScoreCard usages in the ScoreScreen
component (both tab branches) to pass the computed ISO date only when available.
Comment on lines
136
to
138
| // URL에 파라미터가 있는지 확인 | ||
| const hasParams = http.url.includes(':') || http.url.includes('{'); | ||
| const hasParams = http.url.includes(":") || http.url.includes("{"); | ||
|
|
There was a problem hiding this comment.
hasParams가 항상 true로 설정될 수 있습니다
-
원인
http.url.includes("{")조건이{{URL}}의{에도 반응하기 때문에, Bruno 환경 변수 프리픽스를 포함한 모든 URL에서hasParams가true로 고정됩니다.
-
수정 방향
normalizeUrlPath(http.url)로 정규화한 뒤 확인하거나,ApiFunction인터페이스 소비처에서hasParams를 신뢰하지 않도록 문서화해야 합니다.
🐛 제안 수정
- const hasParams = http.url.includes(":") || http.url.includes("{");
+ const normalizedForCheck = normalizeUrlPath(http.url);
+ const hasParams = normalizedForCheck.includes(":") || normalizedForCheck.includes("{");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-api-typescript/src/generator/apiClientGenerator.ts` around
lines 136 - 138, The hasParams calculation can be true incorrectly because
http.url.includes("{") matches template vars like {{URL}}; update the logic to
inspect the normalized path instead—call normalizeUrlPath(http.url) (or
otherwise strip/ignore double-curly template tokens) and then check for ":" or
"{" on that normalized path when computing hasParams (reference symbols:
hasParams, http.url, normalizeUrlPath, ApiFunction) so consumers of ApiFunction
get a correct boolean.
manNomi
force-pushed
the
feat/bruno-api-client-automation
branch
from
8cc7d06 to
e30338e
Compare
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 (1)
packages/bruno-api-typescript/src/generator/apiClientGenerator.ts (1)
186-190:⚠️ Potential issue | 🟡 Minor
generateApiFunction은 현재 사용되지 않지만, 코드 일관성을 위해 {{URL}} 제거 로직을 추가해야 합니다.이 함수는 내보내기로 정의되어 있지만 현재 생성 파이프라인에서 호출되지 않습니다. 대신
generateApiFunctionForFactory가 실제 코드 생성에 사용되며, 이 함수는splitRequestUrl을 통해 {{URL}} 접두사를 올바르게 제거합니다. 다만generateApiFunction도 동일한 패턴을 따르도록 수정하여 코드 일관성을 유지하고 향후 사용 시 버그를 방지해야 합니다.
URL 정규화 단계 추가
apiFunc.url을 템플릿 리터럴에 넣기 전에 {{URL}} 제거 및 쿼리 파라미터 분리
splitRequestUrl함수 활용
generateApiFunctionForFactory와 동일하게splitRequestUrl을 호출하여 URL 전처리수정 제안
+ const { path: cleanedUrl } = splitRequestUrl(url); - let urlExpression = `\`${url}\``; + let urlExpression = `\`${cleanedUrl}\``;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/apiClientGenerator.ts` around lines 186 - 190, The generateApiFunction currently builds urlExpression directly from url and replaces path params but doesn't normalize or strip the {{URL}} prefix and query string; update generateApiFunction to preprocess apiFunc.url by calling splitRequestUrl(apiFunc.url) (same helper used by generateApiFunctionForFactory) to obtain the normalized base URL and query portion, use the normalized URL when creating the template literal (the code around urlExpression = `\`${url}\`` and the subsequent replacements), and ensure any query params returned by splitRequestUrl are handled or appended consistently so the function behavior matches generateApiFunctionForFactory.
🧹 Nitpick comments (10)
packages/api-client/package.json (1)
21-21: 2.axios최소 버전 하한선이 다소 오래되었습니다.
- 현재 axios의 최신 버전은
1.13.5입니다.1.13.x에는mergeConfig의__proto__키를 통한 DoS 취약점 수정이 포함되어 있습니다.^1.6.7범위는>=1.6.7 <2.0.0으로 해석되어 실제 설치 시에는 최신1.x버전이 선택되므로, 현재로서는 실질적인 보안 위험은 없습니다.- 다만, 하한선을 최신 패치 버전으로 올려두면 의도가 더 명확해집니다.
⬆️ 버전 하한선 업데이트 제안
- "axios": "^1.6.7" + "axios": "^1.13.5"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api-client/package.json` at line 21, Update the axios minimum version in packages/api-client's dependency list by changing the version specifier for "axios" from "^1.6.7" to "^1.13.5" to make the lower bound explicitly include the security patch; update the package.json entry for axios and regenerate the lockfile (npm/yarn/pnpm) so the lockfile reflects the new minimum and commit both package.json and the updated lockfile.apps/admin/src/routes/bruno/index.tsx (1)
43-49:import.meta.glob경로가 상대 경로 깊이에 의존합니다.
../../../../../packages/api-client/src/generated/apis/*/apiDefinitions.ts는 디렉토리 구조 변경에 취약합니다. 개발 도구 페이지이므로 당장은 괜찮지만, 추후 모노레포 구조 변경 시 깨질 수 있습니다. Vite alias(예:@api-client/...)를 활용하면 더 견고해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/routes/bruno/index.tsx` around lines 43 - 49, The glob import for definitionModules currently uses a deep relative path which is fragile; change the glob to use a Vite path alias (e.g., replace "../../../../../packages/api-client/src/generated/apis/*/apiDefinitions.ts" with an alias-based path like "@api-client/generated/apis/*/apiDefinitions.ts") and update the project Vite/tsconfig alias configuration if needed so import.meta.glob(...) resolves via the alias; locate the code referencing definitionModules in the file (the import.meta.glob call) and switch to the alias-based glob string to make the import robust to repo layout changes.packages/api-client/scripts/sync-bruno.mjs (2)
8-15:loadEnvFiles호출이 함수 선언보다 앞에 위치합니다.ESM(
.mjs)에서도function선언은 호이스팅되므로 런타임에 문제는 없습니다. 다만 가독성을 위해 유틸리티 함수들을 상단에 배치하고 실행 로직을 하단에 두는 구조를 권장드립니다.packages/api-schema/scripts/sync-bruno.mjs와의 구조적 중복도 보이니, 추후 공통 유틸로 추출하는 것도 고려해보시면 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api-client/scripts/sync-bruno.mjs` around lines 8 - 15, The call to loadEnvFiles appears before the utility/function declarations — move the loadEnvFiles call (and any other execution logic) below the utility function declarations in this module so helpers (e.g., loadEnvFiles) are defined above their use; locate the loadEnvFiles function in this file and reorder the file so all utility helpers are at the top and the execution block (the resolve+loadEnvFiles invocation) is at the bottom, and consider extracting loadEnvFiles into a shared util for reuse with packages/api-schema/scripts/sync-bruno.mjs.
69-86:ensureRemoteCollectionDir에서 기존 checkout 디렉토리를 무조건 삭제 후 재클론합니다.현재 구현에서는 매번
rmSync+git clone을 수행합니다. CI 환경에서는 괜찮지만, 로컬 개발 시 반복 실행하면 매번 네트워크 요청이 발생합니다. 캐시 유효성 검사(예: ref 비교)를 추가하면 로컬 개발 경험을 개선할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api-client/scripts/sync-bruno.mjs` around lines 69 - 86, The function ensureRemoteCollectionDir always rmSync()s checkoutDir and reclones, causing unnecessary network work; update it to check the existing checkoutDir first: if checkoutDir exists, verify its remote URL (use git remote get-url origin) matches remoteRepoUrl and its current checked-out ref or latest origin/ref matches remoteRepoRef (use git -C checkoutDir rev-parse --abbrev-ref HEAD or git -C checkoutDir ls-remote/origin/ref), and only if the URL/ref differ perform rmSync() + git clone; otherwise reuse the cached checkoutDir and skip cloning so local iterations avoid network traffic.packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts (1)
35-39:extractPathParams내부에서 이중 정규화가 발생합니다.Line 70에서 이미
normalizeApiPath(url)로 정규화된 경로를 전달하는데,extractPathParams내부(Line 38)에서 다시normalizeApiPath를 호출합니다. 동작에는 문제 없지만(멱등성), 의도 파악이 어려울 수 있습니다. 함수가 raw URL도 직접 받을 수 있게 하려는 의도라면 현재 구조가 맞고, 아니라면 내부 호출을 제거하는 게 깔끔합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts` around lines 35 - 39, extractPathParams currently calls normalizeApiPath again causing double normalization; remove the internal normalizeApiPath call from inside extractPathParams so it operates on the URL string provided, and update callers (those that currently call normalizeApiPath before calling extractPathParams) to continue passing a normalized path or add a short doc comment on extractPathParams stating it expects a normalized path; if you prefer the function to accept raw URLs instead, do the inverse: remove external normalizeApiPath calls and keep the normalization inside extractPathParams—choose one approach and make callers and the function signature/docs consistent (references: extractPathParams, normalizeApiPath).packages/bruno-api-typescript/src/generator/brunoHashCache.ts (1)
173-178:getStats()의total: 0이 항상 고정값이라 혼동을 줄 수 있습니다.반환되는
total이 항상0이고 주석("generateHooks에서 설정")으로만 의도를 설명하고 있어, 이 메서드의 소비자가 오해할 수 있습니다.total을 생성자나 별도 setter로 관리하거나, 반환 타입에서total을 제외하고 호출부에서 조합하는 방식이 더 명확합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/brunoHashCache.ts` around lines 173 - 178, getStats() currently returns a misleading fixed total: 0; update the class to surface a real total instead of a hardcoded value by either (A) adding a numeric property (e.g., this.total or this._total) initialized in the constructor and updated in generateHooks (or provide a setTotal(total: number) setter) and return that property in getStats(), or (B) remove total from getStats()'s return and let callers combine Object.keys(this.cache.hashes).length with their own total; locate getStats(), this.cache.hashes, and generateHooks to implement the chosen approach so callers receive an accurate total value.packages/bruno-api-typescript/src/generator/apiFactoryGenerator.ts (1)
179-209:hasBody가false일 때 Request 타입 생성을 건너뛰는 분기가 명확합니다.
- Line 179의
if (apiFunc.hasBody)가드로, 불필요한 Request 타입 생성을 방지합니다.- body 파싱 실패 시
Record<string, never>폴백도 적절합니다.한 가지 작은 참고: Line 90과 133에서
responseType.replace("Response", "Request")는 첫 번째 occurrence만 치환합니다. 만약 타입명에 "Response"가 두 번 이상 포함되는 경우(예:ResponseStatusResponse→RequestStatusResponse) 의도와 다를 수 있으나, 현실적으로 그런 네이밍은 드물어 실질적 위험은 낮습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/apiFactoryGenerator.ts` around lines 179 - 209, The if (apiFunc.hasBody) guard intentionally skips generating a Request type when hasBody is false, so keep the logic in apiFactoryGenerator.ts as-is; add a short inline comment above the if (apiFunc.hasBody) explaining that when apiFunc.hasBody is false we intentionally do not emit a request type (and that fallback to Record<string, never> only occurs when hasBody is true but parsing fails or content is missing), and optionally replace any single-occurrence replace calls (e.g., responseType.replace("Response", "Request") referenced elsewhere) with replaceAll or a safer rename helper if you need to handle multiple occurrences.packages/bruno-api-typescript/src/generator/index.ts (3)
197-199: 3.allParsedFiles파싱 실패 시 조용히 무시돼요 —cleanOutput사용 시 데이터 손실 위험이 있습니다.
parsedChangedFiles와 달리allParsedFiles는 파싱 실패 시 아무런 로그도 남기지 않습니다.cleanOutput: true상태에서 특정 도메인의 파일이 일시적으로 파싱에 실패하면:
- 해당 도메인이
allDomains에 포함되지 않고,cleanStaleDomains에 의해 해당 도메인 디렉토리가 경고 없이 삭제됩니다.🔧 수정 제안 — 실패 케이스 로그 추가
const allParsedFiles = brunoFiles - .map(parseFileWithDomain) + .map((filePath) => { + const result = parseFileWithDomain(filePath); + if (!result) { + console.warn(`⚠️ Skipping unparseable file (affects domain cleanup): ${filePath}`); + } + return result; + }) .filter((value): value is ParsedDomainFile => value !== null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 197 - 199, The mapping that builds allParsedFiles using brunoFiles.map(parseFileWithDomain) silently drops parse failures (null), which combined with cleanOutput can cause domains to be removed without warning; update the code around the allParsedFiles creation to detect when parseFileWithDomain returns null (or throws), record the source filename and parser error, and emit a warning via the existing logger: include the filename and error details and mention cleanOutput is enabled so the domain won't be included in allDomains; keep the resulting filter (value is ParsedDomainFile) but ensure failures are logged (and optionally collected into an errors array) so cleanStaleDomains won't delete silently—refer to symbols allParsedFiles, parseFileWithDomain, brunoFiles, cleanOutput, allDomains, cleanStaleDomains, and parsedChangedFiles when making the change.
263-276: 4. 해시 캐시 갱신 시 이미 파싱된 파일을 다시 파싱해요.Line 265에서
parseBrunoFile(filePath)를 재호출하고 있습니다. 같은 파일은 이미allParsedFiles에 파싱된 결과가 존재하므로,Map으로 캐싱해두면 불필요한 I/O를 줄일 수 있어요.♻️ 리팩터링 제안
+ const parsedFileMap = new Map(allParsedFiles.map((f) => [f.filePath, f])); + for (const { filePath } of parsedChangedFiles) { const currentHash = hashCache.calculateHash(filePath); - const apiFunc = extractApiFunction(parseBrunoFile(filePath), filePath); + const cached = parsedFileMap.get(filePath); + const apiFunc = cached ? extractApiFunction(cached.parsed, filePath) : null; if (!apiFunc) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 263 - 276, The loop re-parses files by calling parseBrunoFile(filePath) even though parsed results are cached in allParsedFiles; replace the direct parse call with a lookup: get the parsed AST from allParsedFiles (e.g., const parsed = allParsedFiles.get(filePath) ?? parseBrunoFile(filePath)), store it back into allParsedFiles if you had to parse, then pass that parsed value to extractApiFunction(parsed, filePath) so extractApiFunction and subsequent logic use the cached parse instead of re-reading the file; keep the existing calls to extractDomain, hashCache.calculateHash, and hashCache.setHash unchanged.
293-295: 도메인 인덱스 파일에서definitionsTypeName생성 방식을 통일하면 좋아요.현재 코드를 살펴보니 다음 두 가지 불일치가 있습니다:
생성 방식의 차이
apiDefinitionGenerator.ts에서는toPascalCase(domain)을 사용index.ts에서는toCamelCase(domain).charAt(0).toUpperCase() + slice(1)수동 구현결과는 동일하지만 유지보수성 개선 가능
- 두 방식 모두 동일한 결과 생성 ("user" → "User", "user-auth" → "UserAuth" 등)
toPascalCase를apiDefinitionGenerator.ts에서 이미 사용 중이므로 재사용 권장추천 변경사항:
typeGenerator에서toPascalCase를 import하여 동일한 유틸리티 함수 사용- 로직 중복 제거 및 향후 유지보수성 향상
- import { functionNameToTypeName, toCamelCase } from "./typeGenerator"; + import { functionNameToTypeName, toCamelCase, toPascalCase } from "./typeGenerator";- const definitionsTypeName = `${camelDomain.charAt(0).toUpperCase()}${camelDomain.slice(1)}ApiDefinitions`; + const definitionsTypeName = `${toPascalCase(domain)}ApiDefinitions`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 293 - 295, The creation of definitionsTypeName in index.ts duplicates manual PascalCase logic; update it to reuse the existing toPascalCase utility instead of toCamelCase + charAt(0).toUpperCase() + slice(1). Import toPascalCase into this module and replace the current definitionsTypeName assignment (which references camelDomain and definitionsTypeName) with a call to toPascalCase(domain) to produce the PascalCase type name consistent with apiDefinitionGenerator.ts, removing the manual capitalization code path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api-client/src/runtime/axiosInstance.ts`:
- Around line 48-80: The request interceptor
(axiosInstance.interceptors.request.use) is vulnerable to the same race as the
response path because multiple requests can concurrently call
reissueAccessToken; implement the module-level shared Promise pattern used for
responses: create a single module-scoped refreshPromise and a wrapper function
refreshAccessToken(refreshToken) that returns/awaits that shared promise,
replace direct calls to reissueAccessToken(refreshToken) in the request
interceptor with refreshAccessToken(refreshToken), and retain existing behavior
for tokenStorage.saveAccessToken, clearTokens, isTokenExpired and
attachAuthorization so only one refresh is in-flight for all interceptors.
In `@packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts`:
- Around line 80-82: When deriving requestType, don't assume responseType
contains "Response"; change the logic so if responseType.includes("Response")
you do responseType.replace("Response","Request"), otherwise fall back to a
sensible request type (prefer parsed.body?.type when present, otherwise a
generic object type) so that when responseType === "void" and apiFunc.hasBody is
true you don't end up with requestType === "void" and thus bodyType === "void";
update the requestType calculation (and keep bodyType = hasBody ? requestType :
"Record<string, never>") to use parsed.body where available.
In `@packages/bruno-api-typescript/src/generator/index.ts`:
- Around line 66-67: The split on relativePath (produced by relative(brunoDir,
filePath)) fails on Windows because backslashes aren’t normalized; before
creating parts from relativePath you should normalize backslashes to forward
slashes and then split, i.e. replace all "\" with "/" on relativePath prior to
calling .split, updating the code that sets parts to use the normalized string
so domain extraction behaves consistently (matching the normalization already
used elsewhere in this file).
- Around line 331-332: The usage example currently hardcodes the "applications"
domain in the import log; replace that hardcoded string with the actual first
domain from sortedAllDomains (e.g., use sortedAllDomains[0]) when constructing
the import path so the log always shows a real generated domain; ensure you
handle the empty-array case with a sensible fallback (like using a defaultDomain
variable or skipping the message) and update the console.log that builds the
import path (the lines that call relative(process.cwd(), join(outputDir,
"applications"))) to reference join(outputDir, firstDomain) instead.
- Around line 108-126: The current ensureUniqueApiFunctionNames can produce
collisions because it only counts base names and doesn't check already-assigned
final names (e.g., natural getUser2 colliding with a suffixed getUser2); replace
the logic to maintain a Set of usedNames and, for each entry.apiFunc, pick
candidateName = entry.apiFunc.name and while usedNames.has(candidateName)
increment a numeric suffix (e.g., candidateName = `${baseName}${n}`) until
unique, then set entry.apiFunc.name = candidateName, entry.apiFunc.responseType
= functionNameToTypeName(candidateName), and usedNames.add(candidateName); keep
using the existing symbols ensureUniqueApiFunctionNames, nameCount (or remove
it) and functionNameToTypeName to locate where to change.
---
Outside diff comments:
In `@packages/bruno-api-typescript/src/generator/apiClientGenerator.ts`:
- Around line 186-190: The generateApiFunction currently builds urlExpression
directly from url and replaces path params but doesn't normalize or strip the
{{URL}} prefix and query string; update generateApiFunction to preprocess
apiFunc.url by calling splitRequestUrl(apiFunc.url) (same helper used by
generateApiFunctionForFactory) to obtain the normalized base URL and query
portion, use the normalized URL when creating the template literal (the code
around urlExpression = `\`${url}\`` and the subsequent replacements), and ensure
any query params returned by splitRequestUrl are handled or appended
consistently so the function behavior matches generateApiFunctionForFactory.
---
Duplicate comments:
In `@apps/admin/src/routes/bruno/index.tsx`:
- Around line 224-231: handleCopyResponse currently calls
navigator.clipboard.writeText without error handling; wrap the clipboard call in
a try-catch inside handleCopyResponse (the function that uses requestResult and
toPrettyJson) and on failure catch the error, log it (e.g., console.error) and
surface a user-facing failure toast (e.g., toast.error("응답을 복사하지 못했습니다.")) while
keeping the success toast on successful write; ensure you still guard for
!requestResult before attempting the clipboard operation.
In `@packages/api-client/package.json`:
- Line 10: The script codegen:check currently lets a failing pnpm run sync:bruno
trigger the final "out of sync" message due to operator precedence; fix it by
ensuring sync:bruno failure stops the script before the git diff error message
is considered — either make sync:bruno explicitly exit on failure (e.g., run
sync:bruno || exit 1) before running git diff --exit-code src/generated, or
group the git diff check so it only runs when pnpm run sync:bruno succeeds
(e.g., pnpm run sync:bruno && { git diff --exit-code src/generated || (echo
"Generated API client is out of sync. Run: pnpm --filter
`@solid-connect/api-client` run sync:bruno" && exit 1); }); apply the change to
the codegen:check npm script.
In `@packages/api-client/src/runtime/axiosInstance.ts`:
- Around line 82-116: Multiple concurrent 401 responses cause parallel calls to
reissueAccessToken; change the response interceptor
(axiosInstance.interceptors.response.use) to serialize token refreshes by using
a shared pending promise (e.g., a module-level or runtimeConfig-scoped
pendingReissue Promise): when a 401 is detected and a refresh is required, if
pendingReissue exists await it instead of calling reissueAccessToken again; if
not, set pendingReissue = reissueAccessToken(refreshToken) and await it, then on
success call tokenStorage.saveAccessToken(newAccessToken), clear pendingReissue,
attachAuthorization(originalConfig, newAccessToken) and retry the request, and
on failure clear pendingReissue, call clearTokens(tokenStorage) and throw the
login-needed error; ensure pendingReissue is cleared in both success and error
paths and preserve existing checks using isTokenExpired and
tokenStorage.loadRefreshToken().
In `@packages/bruno-api-typescript/src/generator/apiClientGenerator.ts`:
- Around line 136-138: The current hasParams check wrongly inspects raw http.url
(which may contain Bruno env vars like {{URL}} and thus a '{'), so change the
check to use the already-defined normalized path instead of http.url: compute
hasParams by testing for ":" or "{" on the output of normalizeUrlPath (the
normalized path variable/function used earlier in this module) rather than on
http.url, ensuring parameter detection ignores templated env vars; update the
reference to the normalized path wherever hasParams is used (the hasParams
binding in apiClientGenerator.ts).
- Around line 171-174: apiClientGenerator currently only adds a query params
parameter when method === "GET", causing inconsistent function signatures with
apiFactoryGenerator which uses (method === "GET" || hasQuery); update the params
detection in apiClientGenerator so it mirrors apiFactoryGenerator by
computing/using the same hasQuery condition (i.e., treat URLs containing '?' as
having query) and push "params?: Record<string, unknown>" to paramsList when
method === "GET" || hasQuery so both generators (apiClientGenerator and
apiFactoryGenerator) produce matching signatures.
---
Nitpick comments:
In `@apps/admin/src/routes/bruno/index.tsx`:
- Around line 43-49: The glob import for definitionModules currently uses a deep
relative path which is fragile; change the glob to use a Vite path alias (e.g.,
replace
"../../../../../packages/api-client/src/generated/apis/*/apiDefinitions.ts" with
an alias-based path like "@api-client/generated/apis/*/apiDefinitions.ts") and
update the project Vite/tsconfig alias configuration if needed so
import.meta.glob(...) resolves via the alias; locate the code referencing
definitionModules in the file (the import.meta.glob call) and switch to the
alias-based glob string to make the import robust to repo layout changes.
In `@packages/api-client/package.json`:
- Line 21: Update the axios minimum version in packages/api-client's dependency
list by changing the version specifier for "axios" from "^1.6.7" to "^1.13.5" to
make the lower bound explicitly include the security patch; update the
package.json entry for axios and regenerate the lockfile (npm/yarn/pnpm) so the
lockfile reflects the new minimum and commit both package.json and the updated
lockfile.
In `@packages/api-client/scripts/sync-bruno.mjs`:
- Around line 8-15: The call to loadEnvFiles appears before the utility/function
declarations — move the loadEnvFiles call (and any other execution logic) below
the utility function declarations in this module so helpers (e.g., loadEnvFiles)
are defined above their use; locate the loadEnvFiles function in this file and
reorder the file so all utility helpers are at the top and the execution block
(the resolve+loadEnvFiles invocation) is at the bottom, and consider extracting
loadEnvFiles into a shared util for reuse with
packages/api-schema/scripts/sync-bruno.mjs.
- Around line 69-86: The function ensureRemoteCollectionDir always rmSync()s
checkoutDir and reclones, causing unnecessary network work; update it to check
the existing checkoutDir first: if checkoutDir exists, verify its remote URL
(use git remote get-url origin) matches remoteRepoUrl and its current
checked-out ref or latest origin/ref matches remoteRepoRef (use git -C
checkoutDir rev-parse --abbrev-ref HEAD or git -C checkoutDir
ls-remote/origin/ref), and only if the URL/ref differ perform rmSync() + git
clone; otherwise reuse the cached checkoutDir and skip cloning so local
iterations avoid network traffic.
In `@packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts`:
- Around line 35-39: extractPathParams currently calls normalizeApiPath again
causing double normalization; remove the internal normalizeApiPath call from
inside extractPathParams so it operates on the URL string provided, and update
callers (those that currently call normalizeApiPath before calling
extractPathParams) to continue passing a normalized path or add a short doc
comment on extractPathParams stating it expects a normalized path; if you prefer
the function to accept raw URLs instead, do the inverse: remove external
normalizeApiPath calls and keep the normalization inside
extractPathParams—choose one approach and make callers and the function
signature/docs consistent (references: extractPathParams, normalizeApiPath).
In `@packages/bruno-api-typescript/src/generator/apiFactoryGenerator.ts`:
- Around line 179-209: The if (apiFunc.hasBody) guard intentionally skips
generating a Request type when hasBody is false, so keep the logic in
apiFactoryGenerator.ts as-is; add a short inline comment above the if
(apiFunc.hasBody) explaining that when apiFunc.hasBody is false we intentionally
do not emit a request type (and that fallback to Record<string, never> only
occurs when hasBody is true but parsing fails or content is missing), and
optionally replace any single-occurrence replace calls (e.g.,
responseType.replace("Response", "Request") referenced elsewhere) with
replaceAll or a safer rename helper if you need to handle multiple occurrences.
In `@packages/bruno-api-typescript/src/generator/brunoHashCache.ts`:
- Around line 173-178: getStats() currently returns a misleading fixed total: 0;
update the class to surface a real total instead of a hardcoded value by either
(A) adding a numeric property (e.g., this.total or this._total) initialized in
the constructor and updated in generateHooks (or provide a setTotal(total:
number) setter) and return that property in getStats(), or (B) remove total from
getStats()'s return and let callers combine
Object.keys(this.cache.hashes).length with their own total; locate getStats(),
this.cache.hashes, and generateHooks to implement the chosen approach so callers
receive an accurate total value.
In `@packages/bruno-api-typescript/src/generator/index.ts`:
- Around line 197-199: The mapping that builds allParsedFiles using
brunoFiles.map(parseFileWithDomain) silently drops parse failures (null), which
combined with cleanOutput can cause domains to be removed without warning;
update the code around the allParsedFiles creation to detect when
parseFileWithDomain returns null (or throws), record the source filename and
parser error, and emit a warning via the existing logger: include the filename
and error details and mention cleanOutput is enabled so the domain won't be
included in allDomains; keep the resulting filter (value is ParsedDomainFile)
but ensure failures are logged (and optionally collected into an errors array)
so cleanStaleDomains won't delete silently—refer to symbols allParsedFiles,
parseFileWithDomain, brunoFiles, cleanOutput, allDomains, cleanStaleDomains, and
parsedChangedFiles when making the change.
- Around line 263-276: The loop re-parses files by calling
parseBrunoFile(filePath) even though parsed results are cached in
allParsedFiles; replace the direct parse call with a lookup: get the parsed AST
from allParsedFiles (e.g., const parsed = allParsedFiles.get(filePath) ??
parseBrunoFile(filePath)), store it back into allParsedFiles if you had to
parse, then pass that parsed value to extractApiFunction(parsed, filePath) so
extractApiFunction and subsequent logic use the cached parse instead of
re-reading the file; keep the existing calls to extractDomain,
hashCache.calculateHash, and hashCache.setHash unchanged.
- Around line 293-295: The creation of definitionsTypeName in index.ts
duplicates manual PascalCase logic; update it to reuse the existing toPascalCase
utility instead of toCamelCase + charAt(0).toUpperCase() + slice(1). Import
toPascalCase into this module and replace the current definitionsTypeName
assignment (which references camelDomain and definitionsTypeName) with a call to
toPascalCase(domain) to produce the PascalCase type name consistent with
apiDefinitionGenerator.ts, removing the manual capitalization code path.
Comment on lines
+48
to
+80
| axiosInstance.interceptors.request.use( | ||
| async (config) => { | ||
| if (!runtimeConfig) { | ||
| return config; | ||
| } | ||
|
|
||
| const { tokenStorage, isTokenExpired, reissueAccessToken } = runtimeConfig; | ||
| let accessToken = tokenStorage.loadAccessToken(); | ||
|
|
||
| if (accessToken === null || isTokenExpired(accessToken)) { | ||
| const refreshToken = tokenStorage.loadRefreshToken(); | ||
| if (refreshToken === null || isTokenExpired(refreshToken)) { | ||
| clearTokens(tokenStorage); | ||
| return config; | ||
| } | ||
|
|
||
| try { | ||
| accessToken = await reissueAccessToken(refreshToken); | ||
| tokenStorage.saveAccessToken(accessToken); | ||
| } catch { | ||
| clearTokens(tokenStorage); | ||
| return config; | ||
| } | ||
| } | ||
|
|
||
| if (accessToken !== null) { | ||
| attachAuthorization(config, accessToken); | ||
| } | ||
|
|
||
| return config; | ||
| }, | ||
| (error) => Promise.reject(error), | ||
| ); |
There was a problem hiding this comment.
요청 인터셉터의 사전 토큰 갱신 로직도 동일한 경쟁 조건에 노출됩니다.
여러 요청이 동시에 발생하고 access token이 만료된 상태라면, 각 요청의 request interceptor가 독립적으로 reissueAccessToken을 호출합니다. 응답 인터셉터와 동일한 공유 Promise 패턴을 적용하면 양쪽 모두 해결됩니다.
🔒 모듈 레벨 공유 Promise 패턴 제안
let runtimeConfig: RuntimeConfig | null = null;
let interceptorsBound = false;
+let refreshPromise: Promise<string> | null = null;
+
+const refreshAccessToken = async (refreshToken: string): Promise<string> => {
+ if (!runtimeConfig) {
+ throw new Error("Runtime not configured");
+ }
+ if (!refreshPromise) {
+ refreshPromise = runtimeConfig.reissueAccessToken(refreshToken).finally(() => {
+ refreshPromise = null;
+ });
+ }
+ return refreshPromise;
+};이후 request/response 인터셉터 모두에서 reissueAccessToken(refreshToken) 대신 refreshAccessToken(refreshToken)을 사용합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/api-client/src/runtime/axiosInstance.ts` around lines 48 - 80, The
request interceptor (axiosInstance.interceptors.request.use) is vulnerable to
the same race as the response path because multiple requests can concurrently
call reissueAccessToken; implement the module-level shared Promise pattern used
for responses: create a single module-scoped refreshPromise and a wrapper
function refreshAccessToken(refreshToken) that returns/awaits that shared
promise, replace direct calls to reissueAccessToken(refreshToken) in the request
interceptor with refreshAccessToken(refreshToken), and retain existing behavior
for tokenStorage.saveAccessToken, clearTokens, isTokenExpired and
attachAuthorization so only one refresh is in-flight for all interceptors.
Comment on lines
+80
to
+82
| const requestType = responseType.replace("Response", "Request"); | ||
| const hasBody = apiFunc.hasBody && Boolean(parsed.body?.content?.trim()); | ||
| const bodyType = hasBody ? requestType : "Record<string, never>"; |
There was a problem hiding this comment.
responseType이 "void"일 때 requestType 도출이 올바르지 않을 수 있습니다.
responseType.replace("Response", "Request")는 responseType에 "Response" 문자열이 포함되어 있다고 가정합니다. 만약 responseType이 "void"인 API(예: 응답 본문 없는 POST/DELETE)에 요청 바디가 있다면, requestType도 "void"가 되어 bodyType이 "void"로 잘못 설정됩니다.
🛠️ 제안하는 수정
- const requestType = responseType.replace("Response", "Request");
- const hasBody = apiFunc.hasBody && Boolean(parsed.body?.content?.trim());
- const bodyType = hasBody ? requestType : "Record<string, never>";
+ const hasBody = apiFunc.hasBody && Boolean(parsed.body?.content?.trim());
+ const requestType = responseType.includes("Response")
+ ? responseType.replace("Response", "Request")
+ : "Record<string, unknown>";
+ const bodyType = hasBody ? requestType : "Record<string, never>";📝 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
| const requestType = responseType.replace("Response", "Request"); | |
| const hasBody = apiFunc.hasBody && Boolean(parsed.body?.content?.trim()); | |
| const bodyType = hasBody ? requestType : "Record<string, never>"; | |
| const hasBody = apiFunc.hasBody && Boolean(parsed.body?.content?.trim()); | |
| const requestType = responseType.includes("Response") | |
| ? responseType.replace("Response", "Request") | |
| : "Record<string, unknown>"; | |
| const bodyType = hasBody ? requestType : "Record<string, never>"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-api-typescript/src/generator/apiDefinitionGenerator.ts` around
lines 80 - 82, When deriving requestType, don't assume responseType contains
"Response"; change the logic so if responseType.includes("Response") you do
responseType.replace("Response","Request"), otherwise fall back to a sensible
request type (prefer parsed.body?.type when present, otherwise a generic object
type) so that when responseType === "void" and apiFunc.hasBody is true you don't
end up with requestType === "void" and thus bodyType === "void"; update the
requestType calculation (and keep bodyType = hasBody ? requestType :
"Record<string, never>") to use parsed.body where available.
Comment on lines
+66
to
+67
| const parts = relativePath.split("/"); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 검증 1: 66-67번 줄의 실제 코드 확인
sed -n '60,75p' packages/bruno-api-typescript/src/generator/index.tsRepository: solid-connection/solid-connect-web
Length of output: 691
🏁 Script executed:
# 검증 2: 307번 줄 근처 코드 확인
sed -n '300,315p' packages/bruno-api-typescript/src/generator/index.tsRepository: solid-connection/solid-connect-web
Length of output: 615
🏁 Script executed:
# 검증 3: 파일 전체에서 split("/") 패턴 찾기
grep -n 'split.*"/"' packages/bruno-api-typescript/src/generator/index.tsRepository: solid-connection/solid-connect-web
Length of output: 121
🏁 Script executed:
# 검증 4: 파일 전체에서 경로 관련 처리 패턴 찾기
grep -n 'replace.*\\\\' packages/bruno-api-typescript/src/generator/index.tsRepository: solid-connection/solid-connect-web
Length of output: 185
Windows 환경에서 경로 분리자 처리 누락 — 도메인 추출 로직이 깨져요.
path.relative()는 Windows에서 경로 구분자로 오직 백슬래시(\)만 반환합니다. 현재 코드는 슬래시("/")로만 경로를 분리하기 때문에, Windows 환경에서는 도메인 추출이 정상 작동하지 않습니다.
흥미롭게도 같은 파일의 307번 줄에서는 이미 .replace(/\\/g, "/") 처리가 적용되어 있어서, 일관성 문제도 함께 있습니다.
수정 방법:
66번 줄의 상대 경로 분리 시 백슬래시를 슬래시로 정규화하기
const relativePath = relative(brunoDir, filePath);
- const parts = relativePath.split("/");
+ const parts = relativePath.replace(/\\/g, "/").split("/");
이렇게 하면 Windows와 Unix 환경 모두에서 일관되게 작동합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 66 - 67,
The split on relativePath (produced by relative(brunoDir, filePath)) fails on
Windows because backslashes aren’t normalized; before creating parts from
relativePath you should normalize backslashes to forward slashes and then split,
i.e. replace all "\" with "/" on relativePath prior to calling .split, updating
the code that sets parts to use the normalized string so domain extraction
behaves consistently (matching the normalization already used elsewhere in this
file).
Comment on lines
+108
to
126
| function ensureUniqueApiFunctionNames(entries: Array<{ apiFunc: ApiFunction; parsed: ParsedBrunoFile }>): void { | ||
| const nameCount = new Map<string, number>(); | ||
|
|
||
| for (const entry of entries) { | ||
| const baseName = entry.apiFunc.name; | ||
| const currentCount = nameCount.get(baseName) ?? 0; | ||
|
|
||
| if (currentCount === 0) { | ||
| nameCount.set(baseName, 1); | ||
| entry.apiFunc.responseType = functionNameToTypeName(entry.apiFunc.name); | ||
| continue; | ||
| } | ||
|
|
||
| const nextCount = currentCount + 1; | ||
| nameCount.set(baseName, nextCount); | ||
| entry.apiFunc.name = `${baseName}${nextCount}`; | ||
| entry.apiFunc.responseType = functionNameToTypeName(entry.apiFunc.name); | ||
| } | ||
| } |
There was a problem hiding this comment.
2. ensureUniqueApiFunctionNames — 접미사 붙인 이름이 기존 자연 이름과 충돌할 수 있어요.
현재 충돌 해소 전략은 다음 상황에서 문제가 생깁니다:
-
같은 도메인에 자연적으로
getUser함수 이름을 만들어내는.bru파일이 2개 있고, -
동시에 자연적으로
getUser2를 만들어내는 파일도 1개 있는 경우→ 두 번째
getUser는getUser2로 이름이 바뀌지만, 이미getUser2라는 자연 이름이 존재하므로 중복이 발생합니다.
nameCount 맵이 원래 이름 기준으로만 계수하고 최종 확정된 이름들을 사전에 체크하지 않기 때문입니다.
🔧 수정 제안 — 2-pass 방식으로 교체
function ensureUniqueApiFunctionNames(entries: Array<{ apiFunc: ApiFunction; parsed: ParsedBrunoFile }>): void {
- const nameCount = new Map<string, number>();
-
- for (const entry of entries) {
- const baseName = entry.apiFunc.name;
- const currentCount = nameCount.get(baseName) ?? 0;
-
- if (currentCount === 0) {
- nameCount.set(baseName, 1);
- entry.apiFunc.responseType = functionNameToTypeName(entry.apiFunc.name);
- continue;
- }
-
- const nextCount = currentCount + 1;
- nameCount.set(baseName, nextCount);
- entry.apiFunc.name = `${baseName}${nextCount}`;
- entry.apiFunc.responseType = functionNameToTypeName(entry.apiFunc.name);
- }
+ // Pass 1: 모든 자연 이름 수집
+ const allNaturalNames = new Set(entries.map((e) => e.apiFunc.name));
+ const finalNames = new Set<string>();
+ const nameCount = new Map<string, number>();
+
+ for (const entry of entries) {
+ const baseName = entry.apiFunc.name;
+ const currentCount = nameCount.get(baseName) ?? 0;
+ nameCount.set(baseName, currentCount + 1);
+
+ if (currentCount === 0) {
+ finalNames.add(baseName);
+ entry.apiFunc.responseType = functionNameToTypeName(entry.apiFunc.name);
+ continue;
+ }
+
+ // 이미 사용 중이거나 자연 이름으로 존재하는 이름은 건너뜀
+ let suffix = currentCount + 1;
+ let candidateName = `${baseName}${suffix}`;
+ while (finalNames.has(candidateName) || (allNaturalNames.has(candidateName) && !finalNames.has(candidateName))) {
+ suffix++;
+ candidateName = `${baseName}${suffix}`;
+ }
+ finalNames.add(candidateName);
+ entry.apiFunc.name = candidateName;
+ entry.apiFunc.responseType = functionNameToTypeName(entry.apiFunc.name);
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 108 - 126,
The current ensureUniqueApiFunctionNames can produce collisions because it only
counts base names and doesn't check already-assigned final names (e.g., natural
getUser2 colliding with a suffixed getUser2); replace the logic to maintain a
Set of usedNames and, for each entry.apiFunc, pick candidateName =
entry.apiFunc.name and while usedNames.has(candidateName) increment a numeric
suffix (e.g., candidateName = `${baseName}${n}`) until unique, then set
entry.apiFunc.name = candidateName, entry.apiFunc.responseType =
functionNameToTypeName(candidateName), and usedNames.add(candidateName); keep
using the existing symbols ensureUniqueApiFunctionNames, nameCount (or remove
it) and functionNameToTypeName to locate where to change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/api-client/scripts/build.mjs (2)
8-18:result.error미처리 시 무음(silent) 실패 발생 가능
문제 설명
spawnSync가 프로세스를 아예 생성하지 못할 경우(예:pnpm이PATH에 없어 ENOENT),result.error가 설정되고result.status는null이 됩니다.?? 1덕분에 종료 코드는 올바르게 1이 되지만,result.error를 출력하는 코드가 없어 사용자는 오류 원인을 전혀 알 수 없습니다.근거
- Node.js 공식 문서에 따르면,
spawnSync()의 결과에는 자식 프로세스를 스폰할 수 없는 경우에만.error속성이 존재합니다.- 커맨드 자체가 존재하지 않을 때도 ENOENT 가 방출됩니다.
권장 수정 방향
♻️ result.error 처리 추가 제안
function run(command, args, cwd = rootDir) { const result = spawnSync(command, args, { cwd, stdio: "inherit", env: process.env, }); + if (result.error) { + console.error(`[api-client] Failed to spawn '${command}': ${result.error.message}`); + process.exit(1); + } + if (result.status !== 0) { process.exit(result.status ?? 1); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api-client/scripts/build.mjs` around lines 8 - 18, The run function using spawnSync (function run) currently only checks result.status and can silently fail when spawnSync sets result.error (e.g., ENOENT) because no error is logged; update run to check result.error after spawnSync and, if present, log the error (including error.message and error.code) to stderr or processLogger before exiting with result.status ?? 1, ensuring you reference spawnSync's returned result.error and the run function so the error path is visible to users.
33-37:index.ts하나만 체크하므로 일부 API 파일 누락 시 빌드가 그냥 통과될 수 있음
문제 설명
src/generated/index.ts파일만 존재 여부를 확인하고 있습니다.src/generated/apis/내 개별 API 파일들이 삭제되거나 빠진 상태라도 이 검사는 통과됩니다.권장 수정 방향
- 아래와 같이
apis디렉토리 존재 여부도 함께 검증하면 더 안전합니다.♻️ apis 디렉토리 추가 검사 제안
+import { readdirSync } from "node:fs"; + +const generatedApisDir = resolve(rootDir, "src/generated/apis"); + if (!existsSync(generatedIndexPath)) { console.error(`[api-client] Missing generated client: ${generatedIndexPath}`); console.error("[api-client] Run `pnpm --filter `@solid-connect/api-client` run sync:bruno` and commit the changes."); process.exit(1); } + +if (!existsSync(generatedApisDir) || readdirSync(generatedApisDir).length === 0) { + console.error(`[api-client] Missing generated API files: ${generatedApisDir}`); + console.error("[api-client] Run `pnpm --filter `@solid-connect/api-client` run sync:bruno` and commit the changes."); + process.exit(1); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api-client/scripts/build.mjs` around lines 33 - 37, The build script only checks existsSync(generatedIndexPath) which lets builds pass if individual API files under src/generated/apis are missing; update the check to also verify the apis directory (e.g., a new/generatedApisDir or existing path string for "src/generated/apis") exists and is non-empty, and if either the generatedIndexPath or the generated apis directory check fails, log the same descriptive error and exit via process.exit(1); locate the verification logic around existsSync(generatedIndexPath) and add the additional existsSync/emptiness check for the apis directory to hard-fail the build when API files are absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api-client/package.json`:
- Line 21: The axios dependency in package.json is pinned as "axios": "^1.6.7"
which includes high‑severity CVEs (CVE-2025-27152 and CVE-2025-58754); update
the version string in package.json to at least "^1.12.0" (or a newer safe
release), then reinstall and update the lockfile (npm install / yarn install)
and run the test suite and a dependency audit to ensure no regressions; locate
the dependency entry "axios": "^1.6.7" in package.json to make this change and
verify the updated version is reflected in the lockfile and build.
---
Duplicate comments:
In `@packages/api-client/package.json`:
- Line 10: The codegen:check npm script currently uses parentheses for the
fallback which can cause the error message grouping to misbehave; change the
fallback grouping to a brace block so the echo and exit run as a single fallback
action (replace the current "|| (echo \"...\" && exit 1)" with "|| { echo
\"Generated API client is out of sync. Run: pnpm --filter
`@solid-connect/api-client` run sync:bruno\"; exit 1; }") so the script-level exit
and message are reliably executed as one unit when the check fails.
---
Nitpick comments:
In `@packages/api-client/scripts/build.mjs`:
- Around line 8-18: The run function using spawnSync (function run) currently
only checks result.status and can silently fail when spawnSync sets result.error
(e.g., ENOENT) because no error is logged; update run to check result.error
after spawnSync and, if present, log the error (including error.message and
error.code) to stderr or processLogger before exiting with result.status ?? 1,
ensuring you reference spawnSync's returned result.error and the run function so
the error path is visible to users.
- Around line 33-37: The build script only checks existsSync(generatedIndexPath)
which lets builds pass if individual API files under src/generated/apis are
missing; update the check to also verify the apis directory (e.g., a
new/generatedApisDir or existing path string for "src/generated/apis") exists
and is non-empty, and if either the generatedIndexPath or the generated apis
directory check fails, log the same descriptive error and exit via
process.exit(1); locate the verification logic around
existsSync(generatedIndexPath) and add the additional existsSync/emptiness check
for the apis directory to hard-fail the build when API files are absent.
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)
packages/bruno-api-typescript/src/generator/index.ts (1)
197-217:⚠️ Potential issue | 🟡 Minor
allParsedFiles파싱 오류가 조용히 무시되어cleanOutput모드에서 도메인 디렉토리가 의도치 않게 삭제될 수 있어요.현재 구조에서 두 가지 문제가 연쇄적으로 작동합니다:
조용한 실패 (Lines 197-199)
parsedChangedFiles의 경우 파싱 실패 시 Line 191에서 명시적으로console.error를 출력합니다.- 그런데
allParsedFiles의 경우 파싱 실패가 로그 없이null로 필터링됩니다.
allDomains누락 조건 (Lines 211-217)
allDomains.add(domain)은extractApiFunction이null을 반환하는 경우의continue분기 이후에 실행됩니다.- 즉, 도메인의 모든
.bru파일이extractApiFunction에서null을 반환하면, 해당 도메인은allDomains에 포함되지 않습니다.결과적 위험 (Lines 235-237)
cleanOutput: true인 경우cleanStaleDomains(outputDir, allDomains)가 호출되어allDomains에 없는 도메인 디렉토리를rmSync로 영구 삭제합니다.- 파싱 오류 또는
extractApiFunction실패가 일시적인 상황에서도, 이미 생성된 도메인 출력물이 삭제될 수 있습니다.적어도
allParsedFiles의 파싱 실패도 로그를 남기는 것이 안전합니다:🛡️ 수정 제안
- const allParsedFiles = brunoFiles - .map(parseFileWithDomain) - .filter((value): value is ParsedDomainFile => value !== null); + const allParsedFiles = brunoFiles + .map((filePath) => { + const parsed = parseFileWithDomain(filePath); + if (!parsed) { + console.error(`❌ Error parsing (all-files) ${filePath}`); + } + return parsed; + }) + .filter((value): value is ParsedDomainFile => value !== null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 197 - 217, Parsing failures for allParsedFiles are being silently filtered out which can cause allDomains to miss domains and later allow cleanStaleDomains(outputDir, allDomains) to delete valid domain output; modify the logic around parseFileWithDomain/allParsedFiles so that parse failures are logged just like parsedChangedFiles (log the error and the file path), and ensure you add domains to allDomains regardless of extractApiFunction returning null (e.g., add domain when iterating over allParsedFiles before calling extractApiFunction or track domains from parsed results), referencing allParsedFiles, parseFileWithDomain, parsedChangedFiles, extractApiFunction, allDomains, and cleanStaleDomains to locate the affected code.
🧹 Nitpick comments (2)
packages/bruno-api-typescript/src/generator/index.ts (2)
284-300:definitionsTypeName생성 시 수동 PascalCase 대신 기존 유틸리티를 활용하면 일관성이 높아져요.Line 295의
camelDomain.charAt(0).toUpperCase() + camelDomain.slice(1)패턴 대신, 이미 존재하는functionNameToTypeName(camelDomain, "ApiDefinitions")를 사용하면 동일한 결과를 얻으면서 코드가 더 일관됩니다.♻️ 수정 제안
if (name === "apiDefinitions") { const camelDomain = toCamelCase(domain); const definitionsValueName = `${camelDomain}ApiDefinitions`; - const definitionsTypeName = `${camelDomain.charAt(0).toUpperCase()}${camelDomain.slice(1)}ApiDefinitions`; + const definitionsTypeName = functionNameToTypeName(camelDomain, "ApiDefinitions"); return `export { ${definitionsValueName}, ${definitionsTypeName} } from './apiDefinitions';`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 284 - 300, The current generation builds definitionsTypeName manually using camelDomain.charAt(0).toUpperCase() + camelDomain.slice(1) which is inconsistent; replace that manual PascalCase logic with the existing utility functionNameToTypeName(camelDomain, "ApiDefinitions") when constructing definitionsTypeName inside the indexContent mapping for "apiDefinitions" so the exported type name uses the shared naming helper and keeps naming consistent with other generated types.
263-277: 해시 캐시 업데이트 루프에서 불필요한 재파싱이 발생해요.
parsedChangedFiles의 각 항목에는 이미parsed필드가 있는데, Line 265에서parseBrunoFile(filePath)를 다시 호출하고 있어요.parsed를 그대로 구조분해해서 재사용하면 됩니다.♻️ 수정 제안
- for (const { filePath } of parsedChangedFiles) { + for (const { filePath, parsed } of parsedChangedFiles) { const currentHash = hashCache.calculateHash(filePath); - const apiFunc = extractApiFunction(parseBrunoFile(filePath), filePath); + const apiFunc = extractApiFunction(parsed, filePath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 263 - 277, The loop is re-parsing files unnecessarily by calling parseBrunoFile(filePath) even though parsedChangedFiles already supplies a parsed AST; modify the for-loop to destructure parsed from each item (for const { filePath, parsed } of parsedChangedFiles) and pass that parsed value into extractApiFunction instead of calling parseBrunoFile(filePath), removing the redundant parse call and keeping the rest (domain extraction and hashCache.setHash) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-api-typescript/src/generator/index.ts`:
- Around line 398-419: The code currently uses plain string replacement to strip
".ts" (handler.fileName.replace(".ts", "") and relative(...).replace(".ts",
"")), which can remove a ".ts" that appears earlier in the filename; instead
ensure only the trailing extension is removed by replacing those calls with a
method that strips only the file extension (e.g., use an anchored regex like
/\.ts$/ or use path parsing utilities to get the basename without extension) for
both the handlerName assignment (handler.fileName → handlerName) and the msw
index import path construction (relative(process.cwd(), mswIndexPath) usage).
---
Outside diff comments:
In `@packages/bruno-api-typescript/src/generator/index.ts`:
- Around line 197-217: Parsing failures for allParsedFiles are being silently
filtered out which can cause allDomains to miss domains and later allow
cleanStaleDomains(outputDir, allDomains) to delete valid domain output; modify
the logic around parseFileWithDomain/allParsedFiles so that parse failures are
logged just like parsedChangedFiles (log the error and the file path), and
ensure you add domains to allDomains regardless of extractApiFunction returning
null (e.g., add domain when iterating over allParsedFiles before calling
extractApiFunction or track domains from parsed results), referencing
allParsedFiles, parseFileWithDomain, parsedChangedFiles, extractApiFunction,
allDomains, and cleanStaleDomains to locate the affected code.
---
Duplicate comments:
In `@packages/api-client/package.json`:
- Line 10: The current codegen:check script groups operators so a failure of
pnpm run sync:bruno will trigger the "out of sync" message; change the grouping
so only the git diff failure prints the message. Update the "codegen:check"
script to run pnpm run sync:bruno and then evaluate git diff in a grouped
sub-shell (e.g., pnpm run sync:bruno && (git diff --exit-code src/generated ||
(echo "Generated API client is out of sync. Run: pnpm --filter
`@solid-connect/api-client` run sync:bruno" && exit 1))) so failures from
sync:bruno do not produce the misleading message.
In `@packages/bruno-api-typescript/src/generator/index.ts`:
- Line 66: The code uses relativePath.split("/") which fails on Windows
backslashes; update the logic that creates const parts from relativePath to
normalize separators first (e.g., call path.normalize(relativePath) and then
split using path.sep) or split using a cross-platform regex (e.g.,
relativePath.split(/[/\\]+/)); modify the place that defines parts (the const
parts assignment in generator/index.ts) to use one of these approaches and
ensure you import Node's path module if needed.
- Around line 108-126: The single-pass suffix scheme in
ensureUniqueApiFunctionNames can still produce name collisions when a generated
name already exists (e.g., baseName2). Instead of blindly appending
currentCount+1, on collision compute a candidate name and loop until you find an
unused one: use apiFunc.name as base, set nextCount = currentCount + 1, then
while nameCount.has(candidate = `${baseName}${nextCount}`) increment nextCount;
once free, set entry.apiFunc.name = candidate, set entry.apiFunc.responseType =
functionNameToTypeName(candidate), and update nameCount for both baseName and
candidate (nameCount.set(baseName, nextCount) and nameCount.set(candidate, 1) or
increment as appropriate) so future checks see the new reservation. Ensure this
logic is implemented inside ensureUniqueApiFunctionNames and references
nameCount, entry.apiFunc.name, entry.apiFunc.responseType, and
functionNameToTypeName.
---
Nitpick comments:
In `@packages/bruno-api-typescript/src/generator/index.ts`:
- Around line 284-300: The current generation builds definitionsTypeName
manually using camelDomain.charAt(0).toUpperCase() + camelDomain.slice(1) which
is inconsistent; replace that manual PascalCase logic with the existing utility
functionNameToTypeName(camelDomain, "ApiDefinitions") when constructing
definitionsTypeName inside the indexContent mapping for "apiDefinitions" so the
exported type name uses the shared naming helper and keeps naming consistent
with other generated types.
- Around line 263-277: The loop is re-parsing files unnecessarily by calling
parseBrunoFile(filePath) even though parsedChangedFiles already supplies a
parsed AST; modify the for-loop to destructure parsed from each item (for const
{ filePath, parsed } of parsedChangedFiles) and pass that parsed value into
extractApiFunction instead of calling parseBrunoFile(filePath), removing the
redundant parse call and keeping the rest (domain extraction and
hashCache.setHash) unchanged.
Comment on lines
+398
to
+419
| handlerName: handler.fileName.replace(".ts", ""), | ||
| }); | ||
| } | ||
|
|
||
| // 도메인별 index 파일 생성 | ||
| const domainIndexContent = generateDomainHandlersIndex(domain, handlerInfos); | ||
| const domainIndexPath = join(domainDir, 'index.ts'); | ||
| writeFileSync(domainIndexPath, domainIndexContent, 'utf-8'); | ||
| const domainIndexPath = join(domainDir, "index.ts"); | ||
| writeFileSync(domainIndexPath, domainIndexContent, "utf-8"); | ||
| console.log(`✅ MSW Index Generated: ${domainIndexPath}`); | ||
| } | ||
|
|
||
| // 전체 handlers index 파일 생성 | ||
| if (domains.length > 0) { | ||
| const mswIndexContent = generateMSWIndex(domains); | ||
| const mswIndexPath = join(mswOutputDir, 'handlers.ts'); | ||
| writeFileSync(mswIndexPath, mswIndexContent, 'utf-8'); | ||
| const mswIndexPath = join(mswOutputDir, "handlers.ts"); | ||
| writeFileSync(mswIndexPath, mswIndexContent, "utf-8"); | ||
| console.log(`✅ MSW Main Index Generated: ${mswIndexPath}`); | ||
|
|
||
| console.log(`\n🎭 MSW handlers generated successfully!`); | ||
| console.log(`📂 MSW Output directory: ${mswOutputDir}`); | ||
| console.log(`\n📚 Usage example:`); | ||
| console.log(`import { handlers } from './${relative(process.cwd(), mswIndexPath).replace('.ts', '')}';\n`); | ||
| console.log(`import { handlers } from './${relative(process.cwd(), mswIndexPath).replace(".ts", "")}';\n`); |
There was a problem hiding this comment.
.replace(".ts", "") — 파일명 중간에 .ts가 포함된 경우 잘못 잘릴 수 있어요.
- Line 398:
handler.fileName.replace(".ts", "")— plain string replace는 문자열 어디에서든 첫 번째".ts"출현을 대체합니다.- 예:
tsGetUser.ts→GetUser.ts(❌) vs 의도한 결과:tsGetUser(✅)
- 예:
- Line 419:
relative(...).replace(".ts", "")— 동일한 패턴입니다.
끝에 붙은 확장자만 정확히 제거하려면 앵커드 정규식을 사용하는 것이 안전합니다.
🐛 수정 제안
- handlerName: handler.fileName.replace(".ts", ""),
+ handlerName: handler.fileName.replace(/\.ts$/, ""),- console.log(`import { handlers } from './${relative(process.cwd(), mswIndexPath).replace(".ts", "")}';\n`);
+ console.log(`import { handlers } from './${relative(process.cwd(), mswIndexPath).replace(/\.ts$/, "")}';\n`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-api-typescript/src/generator/index.ts` around lines 398 - 419,
The code currently uses plain string replacement to strip ".ts"
(handler.fileName.replace(".ts", "") and relative(...).replace(".ts", "")),
which can remove a ".ts" that appears earlier in the filename; instead ensure
only the trailing extension is removed by replacing those calls with a method
that strips only the file extension (e.g., use an anchored regex like /\.ts$/ or
use path parsing utilities to get the basename without extension) for both the
handlerName assignment (handler.fileName → handlerName) and the msw index import
path construction (relative(process.cwd(), mswIndexPath) usage).
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.
요약
packages/api-client공통 패키지와 런타임을 추가하고, adminauth/scores호출을 공통 런타임 기반으로 이관했습니다.bruno-api-typescript생성기를 확장해--root-index,--clean-output, 함수명 충돌 해소, inline query URL 처리, 루트 배열 응답 타입, 캐시 버전 갱신을 지원했습니다.sync:bruno한 번으로packages/api-client/src/generated/apis/*전체 도메인과 루트 인덱스를 재현 가능하게 생성하도록 파이프라인을 정비했습니다.검증
pnpm --filter bruno-api-typescript run buildpnpm --filter @solid-connect/api-client run sync:brunopnpm --filter @solid-connect/api-client run typecheckpnpm --filter @solid-connect/admin exec tsc --noEmitpnpm --filter @solid-connect/admin run buildapps/web ci:check,apps/admin ci:check) 통과참고
packages/api-client/src/generated/apis/**,packages/api-client/src/generated/index.ts)을 함께 포함했습니다.