모바일 워크스페이스 액션 탐색 개선 - #202
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
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:
Note
|
| Layer / File(s) | Summary |
|---|---|
Bearer auth and scoped queries backend/api/auth.py, backend/api/emails.py, backend/api/search.py, backend/api/network.py, backend/core/config.py, backend/db/models.py |
Switches to HS256 HMAC bearer sessions, validates claims, removes dev headers; scopes email/search/graph queries by user/org; tightens config and encryption requirements. |
Tasks and calendar intent APIs backend/api/tasks.py, backend/api/calendar.py, backend/services/calendar_service.py, backend/services/exceptions.py |
Adds ticket-like tasks API and server-authoritative calendar sync and writeback-intent selection with safety validation. |
SMTP/ZIP hardening backend/services/email_client.py, backend/services/archive.py |
Enforces SMTP allowlists, DNS/IP safety, pinned sockets, header newline checks; ZIP extraction blocks traversal/symlinks. |
Mobile workspace and startup frontend/src/components/WorkspaceHome.tsx, frontend/src/lib/mobile-workspace.ts, frontend/src/lib/workspace-preferences.ts, frontend/src/app/page.tsx |
Implements hash-based mobile views, startup preference store, and unified responsive workspace home. |
Dashboard layout and pages frontend/src/components/DashboardLayout.tsx, frontend/src/app/*/page.tsx |
Refactors header actions, primary nav, mobile popovers; adds route pages (mail/search/tasks/calendar/data/security/projects). |
Safe text and panels frontend/src/lib/safe-text.ts, frontend/src/components/mobile-workspace-panels.tsx, frontend/src/components/EmailList.tsx |
Adds safe text helper, API-backed mobile panels, inbox request caching, sanitized list rendering. |
AI Hub workspace frontend/src/app/ai-hub/page.tsx |
Rewrites AI Hub into cards with loading/empty/error and retry. |
Governance and CI .github/workflows/pr-governance.yml, scripts/ci/* |
Materializes trusted gate script, metadata-only governance, Strix PR-scan robustness and retryability checks. |
Tests and docs backend/tests/*, frontend/**/*test*.tsx, docs/*, ARCHITECTURE.md, README.md |
Extensive unit/E2E updates covering auth, scoping, UI flows, and plans/ops docs alignment. |
Sequence Diagram(s)
Skipped.
Estimated code review effort
🎯 5 (Critical) | ⏱️ ~120 minutes
Possibly related PRs
- Seongho-Bae/naruon#191: Also modifies backend auth context; related to this PR’s shift to bearer-session derived AuthContext.
- Seongho-Bae/naruon#176: Touches DashboardLayout mobile navigation; overlaps with this PR’s hash-based mobile workspace and menu refactor.
- Seongho-Bae/naruon#155: Changes ApiClient usage; related to this PR’s ApiClient header/session-token behavior.
Poem
새벽에 토끼는 코튼 발로 탭탭,
해시가 춤추는 모바일 탭탭탭.
곰손 메일도 안전 글자 변환, 햇살 같아! ✨
베어러 서명, 문지기 단단,
할 일은 줄 서고, 캘린더는 고개 끄덕.
깃발은 푸르고, 게이트는 지켰다. 🥕
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/branding-mobile-action-workspace
1 similar comment
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/plans/2026-05-16-branding-mobile-action-workspace.md`:
- Around line 31-49: 문서의 모바일 내비게이션 계약이 실제 구현과 다르므로 문서 내용(해당 섹션의 Line 31–49 및
74–79)을 현재 구현으로 맞춰주세요: 모바일 내비게이션은 button 클릭과 직접 이벤트 디스패치가 아니라 hash 기반 앵커(`<a
href="`#mobile-`{view}">`)와 setMobileWorkspaceView() 흐름을 사용하므로, 테스트/설명은 button
텍스트와 커스텀 이벤트 기대 대신 앵커 href 값(예: '`#mobile-actions`') 또는 해시 변경을 확인하거나
setMobileWorkspaceView 호출 흐름을 명시하도록 수정하고 mobile nav 관련 설명에 `<a
href="`#mobile-`{view}">`, setMobileWorkspaceView, 및 "naruon:mobile-workspace" 이벤트
사용 방식의 차이를 분명히 기재하세요.
In `@frontend/src/app/page.tsx`:
- Around line 16-19: handleSelectEmail currently always calls
setMobileWorkspaceView('detail'), overwriting desktop URL/hash; change it to
only call setMobileWorkspaceView('detail') when in a mobile context. Inside
handleSelectEmail (and when you setSelectedEmail), gate the call with an
existing isMobile/isSmallScreen flag or evaluate window.matchMedia('(max-width:
Xpx)') / a mobileWorkspaceVisible boolean and only invoke
setMobileWorkspaceView('detail') when that check is true so desktop interactions
no longer mutate the mobile view/hash.
- Around line 60-65: The mobile detail panel currently hides when mobileView ===
'detail' but selectedEmail === null, which can leave the whole mobile UI blank;
update the rendering logic around the element with id "mobile-detail" (using
mobileView and selectedEmail) to handle the empty-detail case: either render a
safe placeholder inside the detail panel when selectedEmail is null or
programmatically fallback mobileView to 'inbox' (or another visible view) before
computing the className—ensure the condition that produces 'flex' covers the
case mobileView === 'detail' && selectedEmail === null by providing a fallback
UI or view switch so the screen is never completely hidden.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a591ab06-fe6a-426b-a8e7-fa825ba881af
📒 Files selected for processing (10)
docs/plans/2026-05-16-branding-mobile-action-workspace.mddocs/plans/2026-05-16-branding-roadmap-next-gaps.mdfrontend/src/app/globals.cssfrontend/src/app/page.tsxfrontend/src/components/DashboardLayout.test.tsxfrontend/src/components/DashboardLayout.tsxfrontend/src/components/EmailList.tsxfrontend/src/lib/mobile-workspace.tsfrontend/tests/e2e/dashboard-branding.spec.tsfrontend/tests/e2e/helpers.ts
Greptile Summary이 PR은 모바일 환경에서 inbox/search/actions/calendar 워크스페이스를 빠르게 전환할 수 있는 하단 내비게이션과 URL hash 동기화 상태 저장소를 추가하고, 헤더 액션 버튼을 실제 동작하는 버튼으로 교체하며, 백엔드 인증을 서명된 Bearer JWT 방식으로 전환하는 대규모 풀스택 변경입니다. 이전 리뷰 스레드에서 지적된 여러 사항(AUTH_SESSION_HMAC_SECRET 미설정 시 전체 401, DATABASE_URL 기본값 제거, ALLOWED_SMTP_HOSTS 미설정 시 이메일 전송 차단, openStartupView race condition, StartupDashboard 이중 마운트)이 아직 열려 있습니다.
Confidence Score: 3/5이전 리뷰에서 지적된 인증 전환(AUTH_SESSION_HMAC_SECRET 미설정 시 전체 API 401), DATABASE_URL 기본값 제거, ALLOWED_SMTP_HOSTS 기본값으로 인한 이메일 차단 등 배포 환경을 즉시 중단시킬 수 있는 브레이킹 체인지가 미해결인 채로 남아 있어 현재 상태로 머지는 권장되지 않습니다. 인증 방식 교체(Bearer JWT)와 여러 config 기본값 제거는 환경 변수가 준비되지 않은 기존 배포를 즉시 중단시키는 브레이킹 체인지이며, 해당 이슈들은 이전 리뷰 스레드에서 이미 지적되었으나 이번 PR HEAD에서도 수정되지 않았습니다. backend/core/config.py, backend/api/auth.py, backend/services/email_client.py, frontend/src/app/page.tsx Important Files Changed
Sequence DiagramsequenceDiagram
participant User as 모바일 사용자
participant Nav as 하단 워크스페이스 Nav
participant Store as mobile-workspace Store
participant Hash as window.location.hash
participant Panel as 워크스페이스 패널
User->>Nav: 탭 클릭 (search/actions/calendar)
Nav->>Nav: event.preventDefault()
Nav->>Store: setMobileWorkspaceView(view)
Store->>Hash: "history.pushState(#mobile-view)"
Store->>Store: listeners 알림
Store->>Panel: useMobileWorkspaceView 재렌더
User->>Hash: 브라우저 뒤로가기
Hash->>Store: hashchange 이벤트
Store->>Panel: getHashMobileWorkspaceView() 반환
Panel-->>User: 이전 패널 복원
Reviews (46): Last reviewed commit: "fix: extend Strix PR scan budget" | Re-trigger Greptile |
1 similar comment
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/tests/e2e/helpers.ts (1)
107-107:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd CORS headers to the 404 fallback response.
The fallback response for unmocked endpoints lacks CORS headers, which could cause CORS errors in tests instead of clear 404 responses.
🛡️ Proposed fix to add CORS headers
- await route.fulfill({ status: 404, body: 'Not mocked' }); + await route.fulfill({ + status: 404, + headers: CORS_HEADERS, + body: 'Not mocked', + });Note: This assumes you've extracted CORS_HEADERS as suggested in the earlier comment. If not, inline the headers object here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/tests/e2e/helpers.ts` at line 107, The 404 fallback response uses route.fulfill({ status: 404, body: 'Not mocked' }) but omits CORS headers causing CORS failures in tests; update the fallback to include the CORS header set (use the shared CORS_HEADERS constant if present or inline the headers object) by passing headers: CORS_HEADERS (or the inline headers) into the route.fulfill call so the response includes Access-Control-Allow-Origin and related CORS headers.
🧹 Nitpick comments (1)
frontend/tests/e2e/helpers.ts (1)
29-33: ⚡ Quick winExtract duplicated CORS headers to a shared constant.
The CORS headers are duplicated between
fulfillJsonand the OPTIONS handler (lines 47-51). Extracting them to a constant improves maintainability.♻️ Proposed refactor to eliminate duplication
+const CORS_HEADERS = { + 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Headers': 'Content-Type, X-User-Id', + 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', +}; + async function fulfillJson(route: Route, body: unknown) { await route.fulfill({ status: 200, contentType: 'application/json', - headers: { - 'Access-Control-Allow-Origin': '*', - 'Access-Control-Allow-Headers': 'Content-Type, X-User-Id', - 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', - }, + headers: CORS_HEADERS, body: JSON.stringify(body), }); }Then update the OPTIONS handler to use the same constant:
if (request.method() === 'OPTIONS') { await route.fulfill({ status: 204, - headers: { - 'Access-Control-Allow-Origin': '*', - 'Access-Control-Allow-Headers': 'Content-Type, X-User-Id', - 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', - }, + headers: CORS_HEADERS, }); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/tests/e2e/helpers.ts` around lines 29 - 33, Extract the duplicated CORS header object used in fulfillJson and the OPTIONS handler into a single shared constant (e.g., CORS_HEADERS) in frontend/tests/e2e/helpers.ts, replace the literal header objects in the fulfillJson function and the server's OPTIONS handler with that constant, and ensure any imports/exports are adjusted if needed so both uses reference the same constant to remove duplication and keep behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/tests/e2e/helpers.ts`:
- Line 107: The 404 fallback response uses route.fulfill({ status: 404, body:
'Not mocked' }) but omits CORS headers causing CORS failures in tests; update
the fallback to include the CORS header set (use the shared CORS_HEADERS
constant if present or inline the headers object) by passing headers:
CORS_HEADERS (or the inline headers) into the route.fulfill call so the response
includes Access-Control-Allow-Origin and related CORS headers.
---
Nitpick comments:
In `@frontend/tests/e2e/helpers.ts`:
- Around line 29-33: Extract the duplicated CORS header object used in
fulfillJson and the OPTIONS handler into a single shared constant (e.g.,
CORS_HEADERS) in frontend/tests/e2e/helpers.ts, replace the literal header
objects in the fulfillJson function and the server's OPTIONS handler with that
constant, and ensure any imports/exports are adjusted if needed so both uses
reference the same constant to remove duplication and keep behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d21543e5-989a-487c-9279-732d24fdac52
📒 Files selected for processing (5)
docs/plans/2026-05-16-branding-mobile-action-workspace.mdfrontend/src/app/page.tsxfrontend/src/components/DashboardLayout.test.tsxfrontend/tests/e2e/dashboard-branding.spec.tsfrontend/tests/e2e/helpers.ts
✅ Files skipped from review due to trivial changes (1)
- docs/plans/2026-05-16-branding-mobile-action-workspace.md
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/DashboardLayout.test.tsx
- frontend/src/app/page.tsx
- frontend/tests/e2e/dashboard-branding.spec.ts
|
PR governance metadata gate is not ready for
|
1 similar comment
|
PR governance metadata gate is not ready for
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
|
PR governance metadata gate is not ready for
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
PR governance metadata gate is not ready for
|
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
No linked issue.
목적 (Purpose)
모바일 환경에서 inbox/search/actions/calendar 작업 공간을 빠르게 전환하고, 브랜딩 및 액션 UX 갭을 문서화해 다음 개선 흐름을 명확히 합니다.
주요 변경 사항 (Key Changes)
mobile-workspace상태 저장소와 URL hash 동기화 추가EmailListdecision marker 및 E2E helper 업데이트검증 (Verification)
npm run lintnpm run typechecknpm test -- src/components/DashboardLayout.test.tsxLIVE_BASE_URL=http://127.0.0.1:18081 npm run test:e2e -- dashboard-branding.spec.ts변경 사항 표
docs/plans/2026-05-16-branding-mobile-action-workspace.mddocs/plans/2026-05-16-branding-roadmap-next-gaps.mdfrontend/src/lib/mobile-workspace.tsfrontend/src/app/page.tsxfrontend/src/components/DashboardLayout.tsxSummary by CodeRabbit
New Features
Improvements