feat(web): add admin AI inspector intake and cron worker#488
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
워크스루
예상 코드 리뷰 난이도🎯 4 (Complex) | ⏱️ ~50분 제안된 리뷰어
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98ee199f31
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await taskRef.update({ | ||
| status: "processing", | ||
| startedAt: FieldValue.serverTimestamp(), | ||
| updatedAt: FieldValue.serverTimestamp(), | ||
| }); |
There was a problem hiding this comment.
The worker reads a pending document and then updates its status in a separate step, so two overlapping runs can select and process the same task before either update is observed. This is realistic here because the workflow is scheduled every 15 minutes and also exposes workflow_dispatch without a concurrency guard (.github/workflows/ai-inspector-worker.yml), so duplicate branch pushes/PR attempts or inconsistent task states can occur. Use a Firestore transaction (or conditional write) to transition pending -> processing atomically when claiming a task.
Useful? React with 👍 / 👎.
| await sendDiscordNotification({ | ||
| taskId, | ||
| prUrl, | ||
| previewUrl, | ||
| instruction: task.instruction, |
There was a problem hiding this comment.
A Discord delivery error is currently treated as a full task failure even after the PR was created and the document was already marked done, because sendDiscordNotification is inside the main try and any exception falls into markTaskFailed. In that case the same completed task ends up labeled failed, which can trigger unnecessary retries and duplicate processing despite successful code/PR output.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/ai-inspector-worker.mjs:
- Around line 310-324: The task is marking the task as "done" with
taskRef.update(...) and then calling sendDiscordNotification(...), but if
sendDiscordNotification throws the outer catch will call markTaskFailed() and
flip a real success to failed; fix by wrapping the sendDiscordNotification(...)
call in its own try/catch so notification errors are non-fatal: after the main
update to set status:"done" (the taskRef.update block), call
sendDiscordNotification inside a try block and on catch log the error and write
a non-fatal notificationError field (e.g., update taskRef with notificationError
and notificationSent:false or similar) instead of rethrowing; ensure
markTaskFailed remains only for real task failures and not triggered by
notification exceptions (refer to sendDiscordNotification, taskRef.update, and
markTaskFailed).
- Around line 233-254: The current two-step claim (querying `pending` then
`taskRef.update`) is not atomic and can allow duplicate processing; change it to
an atomic conditional claim using a Firestore transaction or an update with a
precondition/CAS so that only one runner can flip status to "processing".
Specifically, replace the read-then-update flow around
`db.collection(collectionName).where(...).get()` and `taskRef.update({...
startedAt: FieldValue.serverTimestamp(), status: "processing", updatedAt:
FieldValue.serverTimestamp() })` with a single transactional operation that
re-reads the document inside the transaction and updates its status only if the
current `status === "pending"` (or use `precondition: {lastUpdateTime:
...}`/`exists` checks), and fail/skip if the condition is not met so no two runs
can claim the same `taskDoc`.
In @.github/workflows/ai-inspector-worker.yml:
- Around line 32-33: The workflow hardcodes AI_INSPECTOR_FIRESTORE_COLLECTION to
"aiInspectorTasks" while the web app (AIInspectorFab in
apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx) and
README allow overriding the collection name, causing a mismatch; change the
worker workflow to use the same environment variable used by the web (remove the
hardcoded value and reference the shared env var name) so both web and worker
read the identical AI_INSPECTOR_FIRESTORE_COLLECTION value, and update any
documentation (README) to document the single canonical env var.
In `@apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx`:
- Around line 74-86: The client currently derives requesterId via
tokenParse(accessToken) and performs Firestore writes directly from handleSave,
which is insecure; remove direct client-side Firestore writes from handleSave
and instead call a protected backend API endpoint (e.g., POST
/api/ai-inspector/pending) passing the accessToken and payload, have the backend
verify the token (or Firebase Auth/App Check) and set
requestedBy.role/server-side metadata before writing to Firestore;
alternatively, if keeping client writes, integrate Firebase Authentication and
App Check across the app (useAuthStore, tokenParse) and add strict Firestore
rules/firestore.rules to enforce server-verified roles so client-side
requestedBy.role cannot be trusted.
- Around line 94-125: handleMouseMove and handleClick currently bail out when
event.target is an SVGElement because you only accept HTMLElement; change both
handlers to normalize the target to the nearest ancestor that is an HTMLElement
(walk up parentElement until you find an HTMLElement) and then use that element
for the data-ai-inspector-ui closest() check, getBoundingClientRect, and for
calling preventDefault()/stopPropagation() in handleClick so SVG/icon clicks are
intercepted properly; update references inside handleMouseMove and handleClick
to use this normalized HTMLElement (e.g., normalizedTarget) instead of
event.target.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 66e6a0b8-3f22-4f66-b3a4-570519d3f54b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.github/scripts/ai-inspector-worker.mjs.github/workflows/ai-inspector-worker.ymlREADME.mdapps/web/package.jsonapps/web/src/components/layout/GlobalLayout/index.tsxapps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsxapps/web/src/lib/firebase/client.ts
| const buildTaskMarkdown = (taskId, task) => { | ||
| const selector = task.selector ?? task.element?.selector ?? ""; | ||
| const pageUrl = task.pageUrl ?? ""; | ||
| const instruction = task.instruction ?? ""; | ||
| const requestedBy = task.requestedBy?.userId ?? "unknown"; | ||
| const role = task.requestedBy?.role ?? "unknown"; | ||
| const textSnippet = task.element?.textSnippet ?? ""; | ||
|
|
||
| return `# AI Inspector Task ${taskId} | ||
|
|
||
| - createdAt: ${task.createdAt?.toDate?.()?.toISOString?.() ?? "unknown"} | ||
| - requestedBy: ${escapeMarkdown(requestedBy)} (${escapeMarkdown(role)}) | ||
| - pageUrl: ${escapeMarkdown(pageUrl)} | ||
| - selector: \`${escapeMarkdown(selector)}\` | ||
| - textSnippet: ${escapeMarkdown(textSnippet)} | ||
|
|
||
| ## Instruction | ||
| ${escapeMarkdown(instruction)} | ||
| `; |
There was a problem hiding this comment.
1. 원문 페이지 정보와 사용자 식별자를 그대로 GitHub 기록에 남기지 않는 편이 안전합니다.
여기서는 task의 pageUrl, requestedBy.userId, textSnippet를 markdown 파일에 그대로 커밋하고, 아래 PR 본문에도 pageUrl를 노출합니다. apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx Line 191-201에서 이 값들이 클라이언트 원문으로 들어오므로, 쿼리스트링·내부 URL·이름/이메일 같은 민감 정보가 GitHub 기록에 영구 보존될 수 있습니다. 최소한 URL은 origin+pathname만 남기고, requestedBy와 textSnippet는 Firestore 내부 메타데이터로만 두는 쪽이 더 안전합니다.
Also applies to: 285-296
apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 432e32fb97
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .where("status", "==", "queued") | ||
| .orderBy("createdAt", "asc") |
There was a problem hiding this comment.
The worker only fetches documents with status == "queued" here, but it transitions a task to processing before doing network/git work and has no recovery path for interrupted runs. If the job is cancelled or the runner dies after claim (so the catch block never executes), that task remains processing indefinitely and will never be picked up again by later scheduled runs.
Useful? React with 👍 / 👎.
| const commitMessage = patchApplied | ||
| ? `[ai-inspector] apply task ${taskId}` | ||
| : `[ai-inspector] capture task ${taskId}`; | ||
| runGit(["commit", "-m", commitMessage]); |
There was a problem hiding this comment.
This commit call assumes user.name/user.email are already configured, but the workflow only does checkout, Node setup, dependency install, and script execution; there is no git config step in .github/workflows/ai-inspector-worker.yml. On runners without preconfigured git identity, git commit fails with Author identity unknown, causing otherwise valid tasks to be marked failed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/scripts/ai-inspector-worker.mjs (1)
256-257:⚠️ Potential issue | 🟡 Minor5. 컬렉션 이름도 writer와 같은 방식으로
trim()해야 합니다.
apps/web/src/lib/server/aiInspector/firestoreRepo.tsLine 30은AI_INSPECTOR_FIRESTORE_COLLECTION?.trim()을 쓰는데, 여기 Line 257은 raw env를 그대로 읽습니다.- GitHub Actions 변수에 앞뒤 공백이 섞이면 웹과 worker가 서로 다른 컬렉션을 바라볼 수 있습니다.
🔧 예시 수정
- const collectionName = process.env.AI_INSPECTOR_FIRESTORE_COLLECTION || "aiInspectorTasks"; + const collectionName = process.env.AI_INSPECTOR_FIRESTORE_COLLECTION?.trim() || "aiInspectorTasks";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/ai-inspector-worker.mjs around lines 256 - 257, The worker reads AI_INSPECTOR_FIRESTORE_COLLECTION without trimming which can cause mismatched collection names; update the collectionName initialization (the constant collectionName) to mirror the web repo's approach by reading process.env.AI_INSPECTOR_FIRESTORE_COLLECTION, applying .trim() (with a safe fallback to the default "aiInspectorTasks" if the env is undefined or empty), so collectionName and baseBranch are normalized the same way as the writer code that uses AI_INSPECTOR_FIRESTORE_COLLECTION?.trim().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/ai-inspector-worker.mjs:
- Around line 66-85: Add a common fetch-with-timeout helper and use it in
githubRequest, findOpenPrByHead, sendDiscordNotification, and
requestPatchFromAiEndpoint so external HTTP calls abort after a configurable
timeout (e.g., use AbortController with a default timeout and clear the timer on
success); update each function to call the helper instead of raw fetch and
surface/throw the fetch timeout error consistently. Also normalize the Firestore
collection env var by applying .trim() when reading
process.env.AI_INSPECTOR_FIRESTORE_COLLECTION in the worker so it matches the
web app’s behavior.
In `@apps/web/src/app/api/ai-inspector-requests/route.ts`:
- Around line 46-68: verifyAdminRole currently collapses all failures into
false; change it to return detailed failure types or throw distinct errors so
the POST handler can map them to correct HTTP responses. Update verifyAdminRole
to inspect response.status: return true for 200 with role ADMIN, throw an
authentication error (or return a specific enum/value) for 401/403, throw an
upstream error for 5xx, and surface network/JSON errors (don’t swallow them in
the catch). Then update the POST handler that currently treats a false as 403 to
handle these distinct cases: return 401 for authentication errors, 502/503 for
upstream failures, and 403 only when the user is authenticated but not an admin.
Reference verifyAdminRole and the POST handler that checks its result to locate
where to implement these changes.
In `@apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx`:
- Around line 37-40: The toSelector function currently builds an ID-based
selector as `#${target.id}` which breaks when IDs contain special characters;
update toSelector to use CSS.escape on the id (e.g., call CSS.escape(target.id))
before prepending '#' so the returned selector is always a valid CSS selector
and elements can be reliably queried later; ensure this change is applied inside
the toSelector function that accepts a target: HTMLElement.
---
Duplicate comments:
In @.github/scripts/ai-inspector-worker.mjs:
- Around line 256-257: The worker reads AI_INSPECTOR_FIRESTORE_COLLECTION
without trimming which can cause mismatched collection names; update the
collectionName initialization (the constant collectionName) to mirror the web
repo's approach by reading process.env.AI_INSPECTOR_FIRESTORE_COLLECTION,
applying .trim() (with a safe fallback to the default "aiInspectorTasks" if the
env is undefined or empty), so collectionName and baseBranch are normalized the
same way as the writer code that uses AI_INSPECTOR_FIRESTORE_COLLECTION?.trim().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a3abae52-f574-4e0c-b1ce-e622c3070d7c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/scripts/ai-inspector-worker.mjsREADME.mdapps/web/package.jsonapps/web/src/app/api/ai-inspector-requests/route.tsapps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsxapps/web/src/lib/server/aiInspector/firestoreRepo.tsapps/web/src/lib/server/aiInspector/taskService.tsapps/web/src/lib/server/firebaseAdmin.ts
✅ Files skipped from review due to trivial changes (2)
- apps/web/package.json
- README.md
| const githubRequest = async (method, pathName, body) => { | ||
| const token = required("GITHUB_TOKEN"); | ||
| const response = await fetch(`https://api.github.com${pathName}`, { | ||
| method, | ||
| headers: { | ||
| Accept: "application/vnd.github+json", | ||
| Authorization: `Bearer ${token}`, | ||
| "Content-Type": "application/json", | ||
| "X-GitHub-Api-Version": "2022-11-28", | ||
| }, | ||
| body: body ? JSON.stringify(body) : undefined, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const text = await response.text(); | ||
| throw new Error(`GitHub API error (${response.status}): ${text}`); | ||
| } | ||
|
|
||
| return response.json(); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and its structure
find . -name "ai-inspector-worker.mjs" -type fRepository: solid-connection/solid-connect-web
Length of output: 119
🏁 Script executed:
# Get line count and examine the relevant sections
wc -l .github/scripts/ai-inspector-worker.mjs
head -n 260 .github/scripts/ai-inspector-worker.mjs | tail -n 200Repository: solid-connection/solid-connect-web
Length of output: 5525
🏁 Script executed:
# Look specifically at the functions mentioned: githubRequest, findOpenPrByHead, sendDiscordNotification, requestPatchFromAiEndpoint
cat -n .github/scripts/ai-inspector-worker.mjs | grep -A 20 "const githubRequest\|const findOpenPrByHead\|const sendDiscordNotification\|const requestPatchFromAiEndpoint"Repository: solid-connection/solid-connect-web
Length of output: 3102
🏁 Script executed:
# Check for collection name handling in main and compare with firestoreRepo
grep -n "AI_INSPECTOR_FIRESTORE_COLLECTION" .github/scripts/ai-inspector-worker.mjs
find . -path "*/apps/web/src/lib/server/aiInspector/firestoreRepo.ts" -type fRepository: solid-connection/solid-connect-web
Length of output: 230
🏁 Script executed:
# Check the firestoreRepo.ts for getCollectionName function
cat -n apps/web/src/lib/server/aiInspector/firestoreRepo.ts | grep -A 5 "getCollectionName"Repository: solid-connection/solid-connect-web
Length of output: 575
타임아웃 없는 외부 HTTP 호출로 작업 대기 시간 증가 위험
현재 다음의 네 함수에서 무제한 fetch를 사용하고 있어, 외부 서비스의 느린 응답으로 인해 claimed task가 오래 정체될 수 있습니다.
githubRequest(라인 68)findOpenPrByHead(라인 89)sendDiscordNotification(라인 115)requestPatchFromAiEndpoint(라인 162)
라인 257에서 이미 작업을 processing 상태로 선점하므로, 이들 중 하나의 서비스만 응답을 지연시켜도 전체 처리 흐름이 오래 막혀있게 됩니다. 공통 타임아웃 헬퍼를 추가하고 네 호출에 모두 적용해 주세요.
🔧 예시 수정
+const fetchWithTimeout = (url, options = {}, timeoutMs = 15000) =>
+ fetch(url, {
+ ...options,
+ signal: AbortSignal.timeout(timeoutMs),
+ });
+
const githubRequest = async (method, pathName, body) => {
const token = required("GITHUB_TOKEN");
- const response = await fetch(`https://api.github.com${pathName}`, {
+ const response = await fetchWithTimeout(`https://api.github.com${pathName}`, {
method,
headers: {
Accept: "application/vnd.github+json",
@@
const findOpenPrByHead = async (owner, repo, branchName) => {
const token = required("GITHUB_TOKEN");
- const response = await fetch(
+ const response = await fetchWithTimeout(
`https://api.github.com/repos/${owner}/${repo}/pulls?state=open&head=${owner}:${encodeURIComponent(branchName)}`,
{
headers: {
@@
const sendDiscordNotification = async ({ taskId, prUrl, previewUrl, instruction }) => {
const webhook = process.env.AI_INSPECTOR_DISCORD_WEBHOOK_URL;
if (!webhook) {
return;
}
const timestamp = toIso();
- const response = await fetch(webhook, {
+ const response = await fetchWithTimeout(webhook, {
method: "POST",
headers: {
"Content-Type": "application/json",
@@
const requestPatchFromAiEndpoint = async ({ taskId, task, branchName }) => {
const endpoint = process.env.AI_INSPECTOR_PATCH_ENDPOINT;
if (!endpoint) {
return {
@@
}
const apiKey = process.env.AI_INSPECTOR_PATCH_API_KEY;
- const response = await fetch(endpoint, {
+ const response = await fetchWithTimeout(endpoint, {
method: "POST",
headers: {
"Content-Type": "application/json",환경 변수 파싱 불일치: AI_INSPECTOR_FIRESTORE_COLLECTION 공백 처리
현재 워커 스크립트 (라인 257)와 웹 앱 코드 (firestoreRepo.ts 라인 16)가 컬렉션 이름을 다르게 읽고 있습니다.
- 워커:
process.env.AI_INSPECTOR_FIRESTORE_COLLECTION || "aiInspectorTasks"(공백 미처리) - 웹 앱:
process.env.AI_INSPECTOR_FIRESTORE_COLLECTION?.trim() || "aiInspectorTasks"(공백 제거)
환경 변수에 의도하지 않은 공백이 포함되면 워커와 웹 앱이 서로 다른 Firestore 컬렉션에 접근하게 되어 작업이 동기화되지 않을 수 있습니다. 워커 코드도 .trim()을 적용해 일관성을 맞춰 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/scripts/ai-inspector-worker.mjs around lines 66 - 85, Add a common
fetch-with-timeout helper and use it in githubRequest, findOpenPrByHead,
sendDiscordNotification, and requestPatchFromAiEndpoint so external HTTP calls
abort after a configurable timeout (e.g., use AbortController with a default
timeout and clear the timer on success); update each function to call the helper
instead of raw fetch and surface/throw the fetch timeout error consistently.
Also normalize the Firestore collection env var by applying .trim() when reading
process.env.AI_INSPECTOR_FIRESTORE_COLLECTION in the worker so it matches the
web app’s behavior.
| const verifyAdminRole = async (accessToken: string): Promise<boolean> => { | ||
| const apiServerUrl = process.env.NEXT_PUBLIC_API_SERVER_URL?.trim(); | ||
| if (!apiServerUrl) { | ||
| throw new Error("NEXT_PUBLIC_API_SERVER_URL is not configured."); | ||
| } | ||
|
|
||
| try { | ||
| const response = await fetch(`${apiServerUrl}/my`, { | ||
| headers: { | ||
| Authorization: `Bearer ${accessToken}`, | ||
| }, | ||
| cache: "no-store", | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| return false; | ||
| } | ||
|
|
||
| const data = (await response.json()) as { role?: string }; | ||
| return data.role === UserRole.ADMIN; | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "route.ts" apps/web/src/app/api/ai-inspector-requests/ | head -20Repository: solid-connection/solid-connect-web
Length of output: 129
🏁 Script executed:
cat -n apps/web/src/app/api/ai-inspector-requests/route.tsRepository: solid-connection/solid-connect-web
Length of output: 4471
/my 검증 결과를 다양한 실패 원인에서 구분해 주세요.
API 검증 로직이 여러 원인을 하나로 뭉치고 있어, 클라이언트와 운영 대응이 어려워질 수 있습니다.
-
현재 문제 상황
- Lines 46–68의
verifyAdminRole에서 인증 실패(401), 서버 오류(5xx), 네트워크 오류, JSON 파싱 실패를 모두false로 반환합니다 - Lines 106–107의 POST 핸들러에서 그
false값을 곧바로 HTTP 403(권한 필요)으로 내려보냅니다
- Lines 46–68의
-
실제 영향
- 세션 만료(401) → 403 "관리자 권한이 필요합니다" (잘못된 메시지)
- 업스트림 장애(5xx) → 403 "관리자 권한이 필요합니다" (운영 팀 혼동)
- 실제 권한 없음 → 403 (올바름)
-
개선 방향
/my응답 상태를 구분하여 인증 실패와 인프라 실패를 따로 처리하거나verifyAdminRole에서 상태별 오류를 던져 POST 핸들러가 적절한 응답을 내릴 수 있도록 해 주세요
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/ai-inspector-requests/route.ts` around lines 46 - 68,
verifyAdminRole currently collapses all failures into false; change it to return
detailed failure types or throw distinct errors so the POST handler can map them
to correct HTTP responses. Update verifyAdminRole to inspect response.status:
return true for 200 with role ADMIN, throw an authentication error (or return a
specific enum/value) for 401/403, throw an upstream error for 5xx, and surface
network/JSON errors (don’t swallow them in the catch). Then update the POST
handler that currently treats a false as 403 to handle these distinct cases:
return 401 for authentication errors, 502/503 for upstream failures, and 403
only when the user is authenticated but not an admin. Reference verifyAdminRole
and the POST handler that checks its result to locate where to implement these
changes.
apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/ai-inspector/src/dom.ts (1)
18-20:⚠️ Potential issue | 🟠 Major2. ID selector를 escape하지 않아 유효하지 않은 selector가 저장될 수 있습니다.
id에:,.,[, 공백 같은 문자가 섞이면#${target.id}는 바로 깨집니다. 저장된 selector를 나중에 다시 쓰려면 여기서CSS.escape(target.id)로 정규화해야 합니다.🔧 예시 수정
const toSelector = (target: HTMLElement, maxDepth = 6): string => { if (target.id) { - return `#${target.id}`; + return `#${CSS.escape(target.id)}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-inspector/src/dom.ts` around lines 18 - 20, The toSelector function returns an ID selector without escaping which can produce invalid selectors for ids containing characters like :, ., [, or spaces; update the branch that returns `#${target.id}` to use CSS.escape (e.g., `#${CSS.escape(target.id)}`) so stored selectors are normalized and safe to reuse, and ensure code guards or polyfills for CSS.escape if runtime may not support it; the change should be made inside the toSelector function where target.id is handled.packages/ai-inspector/src/useAiInspectorSelection.ts (1)
61-88:⚠️ Potential issue | 🟠 Major3. SVG/icon 클릭이 inspect mode를 빠져나가게 만듭니다.
event.target가SVGElement면 두 핸들러가 바로 반환합니다. 그래서 아이콘 버튼이나 링크를 누를 때 선택은 실패하고, 원래 클릭 동작만 실행될 수 있습니다. 가장 가까운HTMLElement조상으로 정규화한 뒤 기존 무시/선택 로직을 적용해 주세요.🔧 예시 수정
+ const toHTMLElement = (target: EventTarget | null): HTMLElement | null => { + let current = target instanceof Element ? target : null; + while (current && !(current instanceof HTMLElement)) { + current = current.parentElement; + } + return current; + }; + const handleMouseMove = (event: MouseEvent) => { - const target = event.target; - if (!(target instanceof HTMLElement)) { + const target = toHTMLElement(event.target); + if (!target) { return; } @@ const handleClick = (event: MouseEvent) => { - const target = event.target; - if (!(target instanceof HTMLElement)) { + const target = toHTMLElement(event.target); + if (!target) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-inspector/src/useAiInspectorSelection.ts` around lines 61 - 88, event.target can be an SVGElement so both handleMouseMove and handleClick bail early; normalize the target to the nearest HTMLElement ancestor before applying ignore/selection logic. In handleMouseMove and handleClick, if event.target is not an HTMLElement but is a Node/Element, find the closest ancestor HTMLElement (e.g., via Element.closest or walking parents), then use that normalized element for isIgnoredTarget, toHoverRect, and toElementSelection(maxSelectorDepth) and call setHoverRect/setSelection and preventDefault/stopPropagation (in handleClick) on the normalized element instead of returning early; keep existing functions isIgnoredTarget, toHoverRect, toElementSelection, setHoverRect, setSelection, handleMouseMove, handleClick and maxSelectorDepth references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx`:
- Around line 50-56: The code currently passes the full URL
(window.location.href) into createAiInspectorRequest which can leak sensitive
query/hash params; change the pageUrl payload to build a sanitized URL using
window.location.origin + window.location.pathname and optionally append only a
vetted allowlist of query parameters (or none) and drop the hash; locate the
createAiInspectorRequest call in AIInspectorFab and replace the pageUrl value
with this sanitized URL construction, ensuring instruction.trim() and selection
are left unchanged.
In `@packages/ai-inspector/src/api.ts`:
- Around line 39-55: The success path currently returns taskId: data?.taskId ??
"" and status: data?.status ?? "queued" even when the response body is missing
or malformed; update the success handling in the function that parses
response.json() to validate that data?.taskId and data?.status exist and are
non-empty for 2xx responses and, if either is missing, throw an
AiInspectorRequestError (including response.status and a clear message) instead
of returning default values so callers don't treat broken responses as
successful.
---
Duplicate comments:
In `@packages/ai-inspector/src/dom.ts`:
- Around line 18-20: The toSelector function returns an ID selector without
escaping which can produce invalid selectors for ids containing characters like
:, ., [, or spaces; update the branch that returns `#${target.id}` to use
CSS.escape (e.g., `#${CSS.escape(target.id)}`) so stored selectors are
normalized and safe to reuse, and ensure code guards or polyfills for CSS.escape
if runtime may not support it; the change should be made inside the toSelector
function where target.id is handled.
In `@packages/ai-inspector/src/useAiInspectorSelection.ts`:
- Around line 61-88: event.target can be an SVGElement so both handleMouseMove
and handleClick bail early; normalize the target to the nearest HTMLElement
ancestor before applying ignore/selection logic. In handleMouseMove and
handleClick, if event.target is not an HTMLElement but is a Node/Element, find
the closest ancestor HTMLElement (e.g., via Element.closest or walking parents),
then use that normalized element for isIgnoredTarget, toHoverRect, and
toElementSelection(maxSelectorDepth) and call setHoverRect/setSelection and
preventDefault/stopPropagation (in handleClick) on the normalized element
instead of returning early; keep existing functions isIgnoredTarget,
toHoverRect, toElementSelection, setHoverRect, setSelection, handleMouseMove,
handleClick and maxSelectorDepth references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2b2e84b2-cb23-468b-ad54-73c4a4aab83a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
README.mdapps/web/next.config.mjsapps/web/package.jsonapps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsxapps/web/src/lib/server/aiInspector/firestoreRepo.tspackages/ai-inspector/package.jsonpackages/ai-inspector/src/api.tspackages/ai-inspector/src/dom.tspackages/ai-inspector/src/index.tspackages/ai-inspector/src/types.tspackages/ai-inspector/src/useAiInspectorSelection.tspackages/ai-inspector/tsconfig.json
✅ Files skipped from review due to trivial changes (6)
- apps/web/package.json
- README.md
- packages/ai-inspector/src/index.ts
- packages/ai-inspector/tsconfig.json
- packages/ai-inspector/package.json
- apps/web/src/lib/server/aiInspector/firestoreRepo.ts
| const result = await createAiInspectorRequest({ | ||
| accessToken, | ||
| payload: { | ||
| instruction: instruction.trim(), | ||
| pageUrl: window.location.href, | ||
| selection, | ||
| }, |
There was a problem hiding this comment.
1. pageUrl에 전체 URL을 넣으면 민감한 query/hash도 함께 저장됩니다.
window.location.href는 초대 토큰, 이메일 인증 코드, 사용자 식별 파라미터까지 그대로 실어 보냅니다. 이 값이 Firestore 작업, 워커 아티팩트, Discord 알림으로 이어지면 내부 도구여도 민감정보가 장기 보관될 수 있습니다. 최소한 origin + pathname만 저장하고, 꼭 필요한 query만 선별해서 붙여 주세요.
🔧 예시 수정
payload: {
instruction: instruction.trim(),
- pageUrl: window.location.href,
+ pageUrl: `${window.location.origin}${window.location.pathname}`,
selection,
},📝 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.
| const result = await createAiInspectorRequest({ | |
| accessToken, | |
| payload: { | |
| instruction: instruction.trim(), | |
| pageUrl: window.location.href, | |
| selection, | |
| }, | |
| const result = await createAiInspectorRequest({ | |
| accessToken, | |
| payload: { | |
| instruction: instruction.trim(), | |
| pageUrl: `${window.location.origin}${window.location.pathname}`, | |
| selection, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/layout/GlobalLayout/ui/AIInspectorFab/index.tsx`
around lines 50 - 56, The code currently passes the full URL
(window.location.href) into createAiInspectorRequest which can leak sensitive
query/hash params; change the pageUrl payload to build a sanitized URL using
window.location.origin + window.location.pathname and optionally append only a
vetted allowlist of query parameters (or none) and drop the hash; locate the
createAiInspectorRequest call in AIInspectorFab and replace the pageUrl value
with this sanitized URL construction, ensuring instruction.trim() and selection
are left unchanged.
| const data = (await response.json().catch(() => null)) as { | ||
| message?: string; | ||
| taskId?: string; | ||
| status?: string; | ||
| } | null; | ||
|
|
||
| if (!response.ok) { | ||
| throw new AiInspectorRequestError( | ||
| data?.message ?? "요청 저장에 실패했습니다. 잠시 후 다시 시도해주세요.", | ||
| response.status, | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| taskId: data?.taskId ?? "", | ||
| status: data?.status ?? "queued", | ||
| }; |
There was a problem hiding this comment.
4. 성공 응답을 기본값으로 덮으면 깨진 2xx도 저장 성공처럼 보입니다.
빈 바디나 예상 밖 JSON이 와도 지금은 taskId: ""와 status: "queued"로 성공 처리됩니다. 그러면 UI는 토스트를 띄우고 폼을 닫지만, 실제로는 추적 가능한 task 정보가 없습니다. 성공 케이스에서는 taskId와 status를 검증하고 빠지면 바로 에러로 올려 주세요.
🔧 예시 수정
if (!response.ok) {
throw new AiInspectorRequestError(
data?.message ?? "요청 저장에 실패했습니다. 잠시 후 다시 시도해주세요.",
response.status,
);
}
- return {
- taskId: data?.taskId ?? "",
- status: data?.status ?? "queued",
- };
+ if (!data?.taskId || !data?.status) {
+ throw new AiInspectorRequestError("요청 저장 응답이 올바르지 않습니다. 잠시 후 다시 시도해주세요.", 500);
+ }
+
+ return {
+ taskId: data.taskId,
+ status: data.status,
+ };
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ai-inspector/src/api.ts` around lines 39 - 55, The success path
currently returns taskId: data?.taskId ?? "" and status: data?.status ??
"queued" even when the response body is missing or malformed; update the success
handling in the function that parses response.json() to validate that
data?.taskId and data?.status exist and are non-empty for 2xx responses and, if
either is missing, throw an AiInspectorRequestError (including response.status
and a clear message) instead of returning default values so callers don't treat
broken responses as successful.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1dec9ee44
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| const toSelector = (target: HTMLElement, maxDepth = 6): string => { | ||
| if (target.id) { | ||
| return `#${target.id}`; |
There was a problem hiding this comment.
When an element has an ID containing CSS-special characters (for example React-generated IDs like :r0:), returning #${target.id} produces an invalid selector string, so downstream consumers cannot reliably re-target the chosen node. This breaks the core task context for those pages because the stored selector may not be queryable; use CSS.escape(target.id) when constructing ID-based selectors.
Useful? React with 👍 / 👎.
| node-version: "22" | ||
|
|
||
| - name: Install worker dependencies | ||
| run: npm install firebase-admin |
There was a problem hiding this comment.
The workflow runs npm install firebase-admin, and npm’s default save=true behavior updates tracked project files (notably package.json, and often creates package-lock.json) inside the checked-out repo. That leaves the worktree dirty before the worker logic starts, which makes the later git status --porcelain change-detection unreliable and can cause branch checkout/commit behavior to diverge from actual task changes; install with --no-save (and preferably pin the version) to keep the checkout clean.
Useful? React with 👍 / 👎.
1 participant
Summary
Web UI flow
aiInspectorTasksby default)Worker flow
ai-inspector-worker.yml)AI_INSPECTOR_PATCH_ENDPOINTand applies patchSetup
Validation