feat: api-client 공통 패키지 도입 및 admin auth/scores 이관#435
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough이번 변경은 API 클라이언트 코드 생성 및 관리를 위한 새로운 패키지 구조를 도입하는 작업입니다.
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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: 1
🤖 Fix all issues with AI agents
In `@packages/api-client/package.json`:
- Line 8: CI is not validating that generated API client files are up-to-date
because the package.json scripts run "sync:bruno" only during build but do not
include "codegen:check" in the CI checks; update the project's npm scripts so
that the CI validation step (the "ci:check" script) depends on or invokes
"codegen:check" (or add a dedicated CI job that runs "codegen:check") to ensure
the generated client from "sync:bruno" is verified during CI; modify the
"ci:check" script to include "codegen:check" (or add a new pipeline stage that
runs "pnpm codegen:check") so any out-of-sync generated files are caught.
🧹 Nitpick comments (6)
apps/admin/package.json (1)
15-15: workspace 의존성 추가가 적절합니다.
workspace:*프로토콜을 사용한 내부 패키지 참조가 pnpm 모노레포 관례에 부합합니다.한 가지 참고:
apps/admin에서axios: ^1.6.7을 직접 의존성으로 유지하고 있는데,@solid-connect/api-client도 동일 버전을 선언합니다. 이관이 완료되면 admin 쪽의 직접 axios 의존성이 여전히 필요한지 확인해 보시면 좋겠습니다.packages/api-client/scripts/sync-bruno.mjs (1)
55-65:run()함수에서result.error(spawn 실패) 미처리 가능성.
spawnSync가 명령어 자체를 찾지 못하는 경우(예:pnpm이 PATH에 없을 때),result.status는null이고result.error가 설정됩니다. 현재 로직은status ?? 1로 exit하지만, 에러 원인이 콘솔에 출력되지 않아 디버깅이 어려울 수 있습니다.🔧 spawn 실패 시 에러 메시지 출력 제안
function run(command, args, cwd = rootDir) { const result = spawnSync(command, args, { cwd, stdio: "inherit", env: process.env, }); + if (result.error) { + console.error(`Failed to execute ${command}:`, result.error.message); + } + if (result.status !== 0) { process.exit(result.status ?? 1); } }packages/api-client/src/runtime/axiosInstance.ts (2)
82-116: 동시 401 응답 시 중복 토큰 갱신 가능성이 있습니다.여러 요청이 동시에 401을 받으면 각각 독립적으로
reissueAccessToken을 호출합니다. 이로 인해 불필요한 갱신 API 호출이 발생하거나, 서버가 refresh token을 rotation 방식으로 관리하는 경우 이전 갱신의 결과가 무효화될 수 있습니다.일반적인 해결 방법은 진행 중인 갱신 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) return refreshPromise; + refreshPromise = runtimeConfig.reissueAccessToken(refreshToken).finally(() => { + refreshPromise = null; + }); + return refreshPromise; +};이후 인터셉터 내
reissueAccessToken(refreshToken)호출을refreshAccessToken(refreshToken)으로 교체하면 됩니다.
48-80: 요청 인터셉터에서 토큰이 유효하지 않을 때 요청이 인증 없이 진행됩니다.Lines 60-61, 68-69에서 refresh 실패 시 토큰을 정리하고
config를 그대로 반환합니다. 이는 인증 없이 요청이 서버로 전달됨을 의미합니다. 응답 인터셉터가 401/403을 잡아서 처리하므로 의도된 설계로 보이지만, 이 흐름을 코드 주석으로 명시해두면 향후 유지보수에 도움이 될 것입니다.packages/api-client/tsconfig.json (1)
4-5:moduleResolution: "Node"에서"Bundler"로 전환 검토 — 현재는 일관성 유지 우선현재 패키지의 exports 필드가
.ts소스를 직접 가리키고 있으며, Vite를 통해 소비되는 구조로"Bundler"전략이 더 적합할 수 있습니다. 다만 다음 점들을 고려하면 현재 설정 유지가 바람직합니다:
- 모노레포 일관성 — 모든 패키지(api-schema, bruno-api-typescript 등)가 이미
"Node"전략을 사용 중입니다.- TypeScript 버전 호환성 — 현재 사용 중인 ^5.3.3 버전은
"Bundler"지원하므로 향후 전환 가능합니다.- 향후 개선 사항 — 워크스페이스 전체가 moduleResolution 전략을 통일할 때 함께 검토하면 됩니다.
packages/api-client/src/index.ts (1)
1-2: 와일드카드 재수출 시 네임스페이스 충돌 가능성을 확인해 주세요.현재 소비자들은 모두 서브패스(
@solid-connect/api-client/runtime,@solid-connect/api-client/generated/admin)를 통해 import하고 있어 당장은 문제가 없습니다. 하지만 루트 배럴에서export *로 두 모듈을 합치면, 향후 generated 코드와 runtime에서 동일한 이름의 export가 생길 경우 TypeScript 컴파일 에러가 발생할 수 있습니다.generated 모듈이 확장되면 충돌 리스크가 높아지므로 참고 부탁드립니다.
| "scripts": { | ||
| "sync:bruno": "node ./scripts/sync-bruno.mjs", | ||
| "sync:bruno:remote": "BRUNO_SOURCE_MODE=remote node ./scripts/sync-bruno.mjs", | ||
| "build": "pnpm run sync:bruno", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check turbo.json build and ci:check configuration
echo "=== turbo.json build task ==="
jq '.tasks.build' turbo.json 2>/dev/null || cat turbo.json | grep -A 10 '"build"'
echo ""
echo "=== turbo.json ci:check task ==="
jq '.tasks["ci:check"]' turbo.json 2>/dev/null || cat turbo.json | grep -A 10 '"ci:check"'
echo ""
echo "=== Check sync:bruno task config ==="
jq '.tasks["sync:bruno"]' turbo.json 2>/dev/null || true
echo ""
echo "=== List CI workflow files ==="
fd -e yml -e yaml . .github/workflows 2>/dev/null | head -20Repository: solid-connection/solid-connect-web
Length of output: 763
🏁 Script executed:
#!/bin/bash
echo "=== CI workflow configuration ==="
cat .github/workflows/ci.yml | head -100
echo ""
echo "=== Search for turbo commands in CI workflow ==="
grep -n "turbo\|pnpm.*build\|pnpm.*check" .github/workflows/ci.yml || echo "No turbo/pnpm commands found in snippet"Repository: solid-connection/solid-connect-web
Length of output: 2849
🏁 Script executed:
#!/bin/bash
echo "=== Full CI workflow file ==="
wc -l .github/workflows/ci.yml
echo ""
echo "=== Search for codegen references in CI workflow ==="
grep -n "codegen" .github/workflows/ci.yml || echo "No codegen references found"
echo ""
echo "=== api-client package.json exports and build info ==="
cat packages/api-client/package.json | jq '{name, scripts, exports, type, main}'
echo ""
echo "=== Check api-client's actual output structure ==="
ls -la packages/api-client/src/ 2>/dev/null | head -20 || echo "No src directory visible"Repository: solid-connection/solid-connect-web
Length of output: 1351
codegen:check가 CI 파이프라인에 포함되지 않고 있습니다.
원래 리뷰 의견이 가정했던 상황과 실제가 다릅니다. 현재 설계의 핵심:
-
빌드 중 생성 파일 갱신:
"build": "pnpm run sync:bruno"설정은 의도된 것입니다. Admin 앱 빌드 시 turbo의^build의존성으로 인해 api-client가 빌드되며, 항상 최신 API 클라이언트를 생성합니다. -
캐시 비활성화: sync:bruno 태스크에
cache: false가 설정되어 있어서 turbo 캐시를 무시하고 매번 실행됩니다. 이는 빌드 시점에 항상 신선한 생성 파일을 보장하려는 의도입니다. -
누락된 검증: 문제는
codegen:check스크립트가 존재하지만 CI 파이프라인에 포함되지 않았다는 점입니다. 현재 CI는ci:check(lint & typecheck만)만 실행하고, 생성된 파일이 소스와 일치하는지 검증하지 않습니다.
권장사항:
ci:check태스크에codegen:check의존성을 추가하여 CI에서 생성 파일의 동기화 상태를 검증할 것- 또는 별도 CI 단계에서
codegen:check를 실행하여 개발자가 로컬에서sync:bruno를 실행하지 않은 채 커밋한 경우를 감지할 것
🤖 Prompt for AI Agents
In `@packages/api-client/package.json` at line 8, CI is not validating that
generated API client files are up-to-date because the package.json scripts run
"sync:bruno" only during build but do not include "codegen:check" in the CI
checks; update the project's npm scripts so that the CI validation step (the
"ci:check" script) depends on or invokes "codegen:check" (or add a dedicated CI
job that runs "codegen:check") to ensure the generated client from "sync:bruno"
is verified during CI; modify the "ci:check" script to include "codegen:check"
(or add a new pipeline stage that runs "pnpm codegen:check") so any out-of-sync
generated files are caught.
1 participant
Summary
packages/api-client패키지를 추가하고 runtime(axios/public axios, 토큰 재발급 인터셉터), generated(admin auth/scores), hooks 엔트리 구조를 도입했습니다.codegen:check태스크를 추가해 Bruno 기반 생성물 드리프트를 CI에서 감지할 수 있도록 했습니다.lib/api/*및types/*를@solid-connect/api-client재사용 구조로 전환해 auth/scores vertical slice를 공통 런타임으로 이관했습니다.Verification
pnpm --filter @solid-connect/api-client run typecheckpnpm --filter @solid-connect/admin exec tsc --noEmitpnpm --filter @solid-connect/admin run build