feat: Phase 1-10 complete (branding, DDD models, UI/UX, test fixes, .gitignore cleanup) - #214
Merged
23 commits merged intoMay 24, 2026
Merged
feat: Phase 1-10 complete (branding, DDD models, UI/UX, test fixes, .gitignore cleanup)#21423 commits merged into
23 commits merged into
Conversation
Contributor
|
PR governance metadata gate is not ready for
|
Contributor
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 46 minutes and 14 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time. 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 (5)
Note
|
| Layer / File(s) | Summary |
|---|---|
Accounts API, RBAC/ABAC, & Settings Configuration backend/api/accounts.py, backend/core/rbac.py, backend/core/config.py |
Per-user TenantConfig endpoints with secret presence redaction; RBAC hierarchy and ABAC policy evaluation primitives; Pydantic settings configured to ignore extra environment variables. |
Ontology API & Sender Relationships backend/api/ontology.py, backend/db/models.py |
/api/ontology endpoints for listing and upserting sender relationships with confidence scoring; SenderRelationship ORM model with composite uniqueness constraint. |
Email Pending Replies & WebDAV/CalDAV backend/api/emails.py, backend/api/dav.py |
/api/emails/pending-replies endpoint filtering threads by authenticated SMTP sender; WebDAV skeleton with OPTIONS, PROPFIND, PUT handlers and multi-status XML responses. |
WebSocket Runner & Connection Management backend/api/runner_ws.py, backend/runner/agent.py |
WebSocket endpoint at /ws/runner/{token} with connection lifecycle management; standalone async runner client for instruction processing. |
Calendar Sync & Knowledge Extraction backend/services/calendar_sync.py, backend/services/knowledge_extractor.py |
generate_ics_from_task producing VTODO iCalendar strings; extract_knowledge_from_self_sent creating TicketTask records from self-sent emails. |
Email Quality Improvements backend/services/threading_service.py, backend/services/text_safety.py |
Subject-based fallback email threading with prefix stripping; enhanced HTML markup text safety with per-line tag cleanup. |
API Routing & Test Coverage backend/main.py, backend/tests/* |
New routers registered with authentication dependencies; comprehensive tests for accounts config, ontology relationships, DAV, email pending replies, knowledge extraction, and calendar ICS generation. |
Frontend UI Refactoring & Layout Components
| Layer / File(s) | Summary |
|---|---|
Eight New Layout Components frontend/src/components/AIHubLayout.tsx, CalendarLayout.tsx, DataLayout.tsx, ProjectsLayout.tsx, SearchLayout.tsx, SecurityLayout.tsx, SettingsLayout.tsx, TasksLayout.tsx |
Dedicated layout components encapsulating full page UI, internal state, mocked data, and tabbed interface management for all major app routes. |
Page Route Files Simplified to Wrappers frontend/src/app/ai-hub/page.tsx, calendar/page.tsx, data/page.tsx, projects/page.tsx, search/page.tsx, security/page.tsx, settings/page.tsx, tasks/page.tsx |
Converted from complex implementations to minimal client components that delegate rendering to layout components; removed inline logic and data. |
Workspace Home Dashboard Data Integration frontend/src/components/WorkspaceHome.tsx |
Added useDashboardData() hook for concurrent /api/emails and /api/tasks fetching; refactored StartupDashboard to render KPI cards, priority-mapped tasks, and email listings from fetched data. |
Dashboard Layout & Header Refactoring frontend/src/components/DashboardLayout.tsx |
Removed sidebar content; updated PrimaryNavLink styling; adjusted header branding and primary navigation responsive breakpoints. |
Theme Palette & Next.js Configuration frontend/src/app/globals.css, frontend/next.config.ts |
Updated light-theme color variables; configured API proxy rewrite from /api/* to backend; added allowedDevOrigins for development access. |
E2E Tests & Component Test Refactoring frontend/tests/e2e/*, frontend/src/components/DashboardLayout.test.tsx |
Updated Playwright tests to use new inbox navigation button; refactored dashboard layout assertions; changed live-smoke element verification to image role. |
Page Test Assertions & Icon Mock Updates frontend/src/app/*/page.test.tsx |
Simplified assertions across all major pages; updated lucide-react icon mocks; skipped "Home workspace action bridge" suite; aligned expectations with new layouts. |
Infrastructure, Configuration & Documentation
| Layer / File(s) | Summary |
|---|---|
APM Infrastructure & Prometheus Configuration docker-compose.apm.yml, prometheus.yml |
Docker Compose services for Prometheus and Jaeger; global 15-second scrape interval targeting local backend. |
CI/CD Workflow Updates .github/workflows/strix.yml |
Disabled runner file monitoring; updated default Strix LLM model from gemini-pro-3.1-preview to gemini-2.5-pro. |
Product Specifications & Development Governance docs/architecture/naruon-product-spec.md, docs/architecture/self-hosted-runner-design.md, docs/plans/2026-05-24-north-star-master-spec.md, AGENTS.md, README.md |
Comprehensive North Star product specification, runner architecture, Phase 10+ roadmap; updated governance standards with development tooling defaults and error-handling semantics; documented planned agentic ontology and knowledge indexing. |
Project Configuration & Utilities .gitignore, frontend/debug.html, frontend/screenshot.cjs, screenshot.mjs |
Expanded ignore patterns for OS/build artifacts; added static HTML snapshot and Playwright screenshot generation scripts. |
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly Related PRs
- Seongho-Bae/naruon#171: Overlapping modifications to
frontend/src/components/DashboardLayout.tsxsidebar and navigation routing logic usingnext/linkandusePathnameactive-state handling. - Seongho-Bae/naruon#205: Both PRs update
.github/workflows/strix.ymlfor security scanning configuration, adjusting runner behavior and model/scan parameters. - Seongho-Bae/naruon#180: Overlapping UI/UX changes to sidebar/dashboard navigation and settings page restructuring in
DashboardLayout.tsxand frontend settings components.
🐰 Whiskers twitching with joy,
Eight layout components born,
Backend dances new—
APIs hop through the wire,
North Star guides us home. 🌟
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/phase10
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| In the future, this will parse XML namespaces and bridge | ||
| Naruon's Tasks and Events into DAV compliant responses. | ||
| """ | ||
| logger.info(f"DAV Request: {request.method} /{path}") |
| if request.method == "PUT": | ||
| # Simulate accepting .ics file | ||
| body = await request.body() | ||
| logger.info(f"DAV PUT received {len(body)} bytes at /{path}") |
| await ws.accept() | ||
| # In a real scenario, validate token against WorkspaceRunnerConfig | ||
| self.active_connections[token] = ws | ||
| logger.info(f"Runner connected with token {token}") |
| def disconnect(self, token: str): | ||
| if token in self.active_connections: | ||
| del self.active_connections[token] | ||
| logger.info(f"Runner disconnected: {token}") |
Contributor
Author
|
PR governance metadata gate is not ready for
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| @@ -0,0 +1,112 @@ | |||
| from fastapi import APIRouter, Depends, HTTPException | |||
| @@ -0,0 +1,34 @@ | |||
| import logging | |||
| from sqlalchemy.ext.asyncio import AsyncSession | |||
| from sqlalchemy import select | |||
| @@ -0,0 +1,22 @@ | |||
| import pytest | |||
| @@ -0,0 +1,22 @@ | |||
| import pytest | |||
| from httpx import AsyncClient | |||
| @@ -0,0 +1,35 @@ | |||
| import pytest | |||
| from db.models import Email, TicketTask | |||
seonghobae
enabled auto-merge
May 24, 2026 00:26
| "use client"; | ||
|
|
||
| import { useState } from 'react'; | ||
| import { Sparkles, MessageSquare, Zap, Activity, Cpu, Key, FileCode2 } from 'lucide-react'; |
| "use client"; | ||
|
|
||
| import { useState } from 'react'; | ||
| import { ChevronLeft, ChevronRight, Settings, Plus, Users, Video, Paperclip, Clock, CalendarDays, CheckCircle2, X } from 'lucide-react'; |
| "use client"; | ||
|
|
||
| import { useState } from 'react'; | ||
| import { ShieldCheck, Lock, Users, AlertOctagon, CheckCircle2, XCircle } from 'lucide-react'; |
| "use client"; | ||
|
|
||
| import { useState } from 'react'; | ||
| import { Plus, Search, Filter, FolderOpen, MoreHorizontal, FileText, CheckCircle2, User, Clock, AlertCircle, CalendarDays } from 'lucide-react'; |
| "use client"; | ||
|
|
||
| import { useState } from 'react'; | ||
| import { Search, Filter, Mail, CalendarDays, FileText, UserRound, Network, Clock, ChevronRight, CheckCircle2 } from 'lucide-react'; |
| const [activeTab, setActiveTab] = useState<'워크스페이스' | '멤버' | '연결 계정' | '알림' | '자동화' | '결제' | '개발자'>('워크스페이스'); | ||
| const startupView = useWorkspaceStartupView(); | ||
|
|
||
| const handleStartupViewChange = (view: 'dashboard' | 'email' | 'calendar') => { |
Contributor
Author
|
PR governance metadata gate is not ready for
|
This was referenced May 24, 2026
- data: '데이터 관리' → '데이터와 파일' - security: '보안 및 권한' → '보안과 관리자' - tasks: '작업 관리' → '할 일 추적' - data: 'WebDAV 매핑' → 'WebDAV 원본'
Korean characters in filenames cause Strix to reject the path as unsafe. Renamed all 'ChatGPT Image 2026년...' files to 'naruon-ux-mockup-N.png'.
- Remove accidentally tracked debug HTML, screenshots, trace output - Add venv/, .venv/ (generic), **/__pycache__/, **/*.py[cod] - Add frontend debug artifact patterns to .gitignore
seonghobae
changed the title
| @@ -0,0 +1,47 @@ | |||
| import enum | |||
| from typing import Dict, List, Optional, Any | |||
…ro-tdd - Replaced '(e: any)' with '(e: unknown)' to pass typescript-eslint - Restored screenshot.cjs and test-html.cjs for visual regression testing
Correctly cast onClick values to their specific union types instead of 'unknown' to fix Next.js build type checking failures.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/app/calendar/page.test.tsx (1)
37-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTest assertion doesn't match test description.
The test is named "renders monthly weekly detail coordination candidate and CalDAV writeback workspaces" but only verifies the presence of "새 일정" text. Consider either:
- Updating the test description to match the minimal assertion, or
- Adding assertions to validate the calendar workspace sections mentioned in the description
🤖 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/src/app/calendar/page.test.tsx` around lines 37 - 47, The test named "renders monthly weekly detail coordination candidate and CalDAV writeback workspaces" currently only checks for the string "새 일정" when rendering CalendarPage; either rename the test to reflect that minimal assertion or extend the test to assert the actual workspace sections mentioned in the title by rendering <CalendarPage /> (as done with container/root/act) and adding expect checks for the workspace headings/elements (e.g., text or test-ids for "Monthly", "Weekly", "Detail", "Coordination Candidate", and "CalDAV writeback") using container.textContent or querySelector/getByText so the assertions match the description.frontend/src/app/ai-hub/page.test.tsx (1)
53-94:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRestore meaningful UI assertions in AI Hub page tests
In
frontend/src/app/ai-hub/page.test.tsx(lines 53–94), the success test now only asserts the “AI 허브”h1(duplicated check) and no longer verifies the three AI workspace sections (“맥락 종합”, “판단 포인트”, “실행 항목”). The loading and error tests stubfetchbut only assert the container exists, without validating any loading/error UI or retry behavior.🤖 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/src/app/ai-hub/page.test.tsx` around lines 53 - 94, The success test "renders the three functional AI workspace sections from API data" currently only checks the duplicated H1; update it to assert the three workspace section headings/titles ("맥락 종합", "판단 포인트", "실행 항목") are rendered from the stubbed fetch response and remove the duplicate H1 assertion (locate AIHubPage render and container queries). For the loading test "renders an accessible loading state while the AI hub loads", stub fetch to a pending promise or render immediately and assert the loading indicator/role (e.g., a spinner or element with role="status" or aria-busy) and any accessible text is present. For the error test "renders an accessible error state with retry", keep the fetch stub that returns failure and assert the error message UI and presence of a retry control (e.g., a button); also simulate clicking the retry and verify fetch is called again (use vi.stubGlobal/vi.fn and jsonResponse helpers and flushAsyncWork/act around renders).
🟡 Minor comments (7)
frontend/screenshot.cjs-4-13 (1)
4-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuarantee browser cleanup on navigation/screenshot failure.
At Line 4-13, if
gotoorscreenshotthrows,browser.close()is skipped. Wrap withtry/finallyand prefer a state-based wait over fixed sleep.Suggested diff
(async () => { - const browser = await chromium.launch(); - const page = await browser.newPage({ viewport: { width: 1280, height: 1024 } }); - - await page.goto('http://localhost:18080/settings'); - await page.waitForTimeout(2000); - await page.screenshot({ path: 'test-results/settings-screenshot.png', fullPage: true }); - await browser.close(); + const browser = await chromium.launch(); + try { + const page = await browser.newPage({ viewport: { width: 1280, height: 1024 } }); + await page.goto('http://localhost:18080/settings', { waitUntil: 'networkidle' }); + await page.screenshot({ path: 'test-results/settings-screenshot.png', fullPage: true }); + } finally { + await browser.close(); + } console.log('Screenshot saved to test-results/settings-screenshot.png'); })();🤖 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/screenshot.cjs` around lines 4 - 13, Wrap the Playwright flow that uses chromium.launch(), browser.newPage(), page.goto(), page.screenshot() and browser.close() in a try/finally so browser.close() always runs even if goto or screenshot throws; remove the fixed page.waitForTimeout(2000) and replace with a state-based wait such as page.waitForLoadState('networkidle') or a page.waitForSelector('<stable-selector>') before taking the screenshot to ensure readiness. Ensure the finally block closes the browser reference created by chromium.launch() and handle null/undefined browser defensively.frontend/src/components/CalendarLayout.tsx-23-31 (1)
23-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse valid CSS colors for checkbox
accentColor.
style={{ accentColor: cal.color }}is currently fed Tailwind class tokens (e.g.,bg-red-500), so the accent color won’t render as intended.Suggested fix
- { name: '김나루 (나)', color: 'bg-primary' }, - { name: 'Naruon PM 팀', color: 'bg-red-500' }, - { name: '제품 개발팀', color: 'bg-green-500' }, - { name: '마케팅팀', color: 'bg-purple-500' }, - { name: '회사 공용', color: 'bg-indigo-500' }, - { name: '공휴일', color: 'bg-slate-400' }, + { name: '김나루 (나)', color: 'var(--naruon-primary)' }, + { name: 'Naruon PM 팀', color: '`#ef4444`' }, + { name: '제품 개발팀', color: '`#22c55e`' }, + { name: '마케팅팀', color: '`#a855f7`' }, + { name: '회사 공용', color: '`#6366f1`' }, + { name: '공휴일', color: '`#94a3b8`' }, ... - <input type="checkbox" defaultChecked className={`size-4 rounded border-border text-primary focus:ring-primary`} style={{ accentColor: cal.color }} /> + <input type="checkbox" defaultChecked className="size-4 rounded border-border text-primary focus:ring-primary" style={{ accentColor: cal.color }} />🤖 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/src/components/CalendarLayout.tsx` around lines 23 - 31, The checkbox accentColor is being set to Tailwind class names (cal.color like 'bg-red-500') so the style doesn't apply; update CalendarLayout so the data for each calendar uses valid CSS color values (hex/rgb) or map the Tailwind token to its resolved color before assigning to the inline style (e.g., change the calendar entries or add a small helper like resolveColor(cal.color) and use style={{ accentColor: resolvedColor }} on the <input>), or alternatively remove the inline style and apply a Tailwind accent class if you prefer class-based styling; adjust the array elements (the variable named cal in the .map) or add the helper in CalendarLayout to ensure accentColor receives a real color string.frontend/src/components/WorkspaceHome.tsx-152-152 (1)
152-152:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHardcoded date/time will display incorrect information to users.
The timestamp "2026.05.25 (토) 오전 10:23" is hardcoded and will not reflect the actual current date/time. This misleads users about when they're viewing the dashboard.
🐛 Proposed fix to use dynamic date
- <span suppressHydrationWarning className="text-sm font-medium text-muted-foreground">2026.05.25 (토) 오전 10:23</span> + <span suppressHydrationWarning className="text-sm font-medium text-muted-foreground"> + {new Intl.DateTimeFormat('ko-KR', { + year: 'numeric', month: '2-digit', day: '2-digit', + weekday: 'short', hour: '2-digit', minute: '2-digit' + }).format(new Date())} + </span>🤖 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/src/components/WorkspaceHome.tsx` at line 152, The span in WorkspaceHome.tsx currently renders a hardcoded timestamp; replace it with a dynamically formatted client-side date string to avoid showing incorrect times and hydration mismatches. Inside the WorkspaceHome component, create a client-only state (e.g., clientTime) initialized empty and set it in useEffect to new Date(), then render the formatted string instead of the literal "2026.05.25 (토) 오전 10:23"; use Intl.DateTimeFormat (or your project's date util) with locale "ko-KR" and options for year, month (2-digit), day (2-digit), weekday (short), hour, minute and hour12 to match the original format, and keep the existing suppressHydrationWarning attribute on the span to prevent SSR/CSR mismatch.frontend/tests/e2e/dashboard-branding.spec.ts-37-38 (1)
37-38:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicate assertions.
Lines 36-38 contain the same assertion repeated three times. This appears to be a copy-paste error.
🧹 Proposed fix
await header.getByRole('button', { name: '답장 초안' }).click(); await expect(header.getByText('메일 상세 패널에서 답장 초안을 생성합니다.')).toBeVisible(); - await expect(header.getByText('메일 상세 패널에서 답장 초안을 생성합니다.')).toBeVisible(); - await expect(header.getByText('메일 상세 패널에서 답장 초안을 생성합니다.')).toBeVisible();🤖 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/dashboard-branding.spec.ts` around lines 37 - 38, The test contains duplicate assertions calling expect(header.getByText('메일 상세 패널에서 답장 초안을 생성합니다.')).toBeVisible() multiple times; remove the redundant duplicates and keep a single assertion using the header.getByText(...) expectation (located in the dashboard branding e2e test where the header variable is used) so the test only asserts visibility once.frontend/src/app/ai-hub/page.test.tsx-67-67 (1)
67-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicate assertion.
Line 66 and 67 contain identical assertions. Remove the duplicate.
🧹 Proposed fix
expect(container.querySelector('h1')?.textContent).toContain('AI 허브'); - expect(container.querySelector('h1')?.textContent).toContain('AI 허브');🤖 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/src/app/ai-hub/page.test.tsx` at line 67, Remove the duplicate assertion that checks the H1 text in the test: there are two identical lines calling expect(container.querySelector('h1')?.textContent).toContain('AI 허브');; delete one of them so the test only asserts this once (look for the duplicate inside the test in page.test.tsx where container.querySelector('h1') is used).backend/api/ontology.py-18-21 (1)
18-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
confidence_scorebounds at the API boundary.Unbounded float input allows invalid values (e.g., negative or >1), which will poison downstream scoring behavior.
Suggested fix
+from fastapi import APIRouter, Depends, HTTPException ... async def create_relationship( req: RelationshipCreate, auth_ctx: AuthContext = Depends(get_auth_context), db: AsyncSession = Depends(get_db) ): + if not (0.0 <= req.confidence_score <= 1.0): + raise HTTPException( + status_code=422, + detail="confidence_score must be between 0 and 1", + )Also applies to: 57-59
🤖 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 `@backend/api/ontology.py` around lines 18 - 21, The RelationshipCreate Pydantic model (and the other relationship model around lines 57-59) currently allows unbounded confidence_score; add validation to enforce 0.0 <= confidence_score <= 1.0 at the API boundary by using Pydantic constraints (e.g., change the field to use Field(..., ge=0.0, le=1.0) or add a `@validator` for confidence_score) so invalid floats are rejected early; update both RelationshipCreate and the corresponding relationship model (e.g., RelationshipUpdate/RelationshipSchema) to use the same constraint and keep the default value of 1.0.backend/runner/agent.py-21-22 (1)
21-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow websocket error handling instead of
except Exception
except Exceptionat the runner boundary will also swallow unexpected runtime/programming faults and route them through a generic “Connection failed” print, making debugging harder; handle the websocket “connection closed” case explicitly and log/raise other exceptions with their traceback.🤖 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 `@backend/runner/agent.py` around lines 21 - 22, The current broad except block that prints "Connection failed" should be replaced with specific websocket/connection handling: catch the websocket-specific closure exception (e.g., websockets.exceptions.ConnectionClosed or the library-specific ConnectionClosedError used where the except block appears) and handle it with a concise closed-connection log, then add a separate generic except Exception block that logs the full traceback (using logging.exception or traceback.print_exc) and re-raises the error so programming/runtime faults are not swallowed; update the except block referencing the same location (the existing except Exception as e: print(...)) to implement these two distinct handlers and import the websocket exception class and logging/traceback as needed.
🧹 Nitpick comments (14)
backend/tests/test_accounts_api.py (1)
47-55: ⚡ Quick winMake the config test verify cross-request persistence.
Right now Line 47-Line 48 yields a fresh
MockSessioneach request, so the test can pass even if persisted config retrieval breaks. Use one shared mock session in the fixture and add a GET-after-PUT assertion.Suggested test hardening
-async def override_get_db(): - yield MockSession() - `@pytest.fixture` def client(): - app.dependency_overrides[get_db] = override_get_db + session = MockSession() + async def override_get_db(): + yield session + app.dependency_overrides[get_db] = override_get_db with TestClient(app, headers={"X-User-Id": "testuser"}) as c: yield c app.dependency_overrides.clear() @@ response = client.put("/api/accounts/config", json=update_data) assert response.status_code == 200 data = response.json() assert data["smtp_server"] == "smtp.example.com" assert data["smtp_port"] == 587 assert data["smtp_username"] == "user@example.com" + + response = client.get("/api/accounts/config") + assert response.status_code == 200 + data = response.json() + assert data["smtp_server"] == "smtp.example.com" + assert data["smtp_port"] == 587 + assert data["smtp_username"] == "user@example.com"Also applies to: 57-75
🤖 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 `@backend/tests/test_accounts_api.py` around lines 47 - 55, The current override_get_db yields a new MockSession per request allowing tests to pass even if persistence is broken; change the client fixture to create a single shared MockSession instance (e.g., shared_session = MockSession()) and have override_get_db return/yield that shared_session so state persists across requests, then update the relevant test(s) that exercise config PUT to perform a subsequent GET and assert the persisted value (i.e., add a GET-after-PUT assertion); apply the same shared-session fix to the second fixture referenced (lines 57-75) so both tests verify cross-request persistence.backend/tests/test_knowledge_extractor.py (1)
27-31: ⚡ Quick winAssert tenant owner scope in extracted task fields.
You already validate source linkage, but this test should also assert owner scope propagation (
task.user_id,task.organization_id) from the source email.As per coding guidelines: Email-derived tasks must stay source-linked to the email/thread and tenant owner scope.
🤖 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 `@backend/tests/test_knowledge_extractor.py` around lines 27 - 31, The test currently asserts linkage fields but misses tenant owner scope; add assertions to ensure the extracted task preserves owner scope by asserting task.user_id and task.organization_id equal the source email's owner fields (e.g., assert task.user_id == email.user_id and assert task.organization_id == email.organization_id or use the fixture name used in this test), placing these checks alongside the existing assertions for task.title/source_type/related_email_id/related_thread_id in test_knowledge_extractor.py..gitignore (1)
16-16: ⚡ Quick winAvoid globally ignoring
package-lock.jsonin-repo.Ignoring lockfiles at Line 16 can hide dependency drift and reduce reproducibility for CI/local installs. Keep the “no
~/package-lock.json” rule as developer guidance, but don’t suppress repo lockfile changes.Suggested diff
- package-lock.json🤖 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 @.gitignore at line 16, Remove the repository-level ignore entry "package-lock.json" from .gitignore so the project's lockfile is tracked; instead, keep developer/local ignores (e.g., a note or a global gitignore entry in users' global gitconfig) but do not suppress the repo lockfile — locate the "package-lock.json" line in .gitignore and delete it so changes to package-lock.json are committed.frontend/test-html.cjs (2)
4-20: ⚡ Quick winAdd error handling for async operations.
The script lacks try-catch error handling, which could mask failures during debugging sessions.
🛡️ Proposed fix to add error handling
(async () => { + try { const browser = await chromium.launch(); const page = await browser.newPage({ viewport: { width: 1280, height: 1024 } }); page.on('console', msg => console.log('PAGE LOG:', msg.text())); page.on('pageerror', error => console.log('PAGE ERROR:', error.message)); await page.route('**/api/**', async (route) => { if (route.request().method() === 'OPTIONS') return route.fulfill({ status: 204, headers: {'Access-Control-Allow-Origin': '*'} }); return route.fulfill({ status: 200, contentType: 'application/json', body: '{"emails": [], "tasks": []}', headers: {'Access-Control-Allow-Origin': '*'} }); }); await page.goto('http://localhost:18080/'); await page.waitForTimeout(2000); await browser.close(); + console.log('Test completed successfully'); + } catch (error) { + console.error('Test failed:', error); + process.exit(1); + } })();🤖 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/test-html.cjs` around lines 4 - 20, Wrap the top-level async IIFE body in a try/catch/finally so async failures are surfaced and resources are cleaned: surround the awaits (chromium.launch(), browser.newPage(), page.route(), page.goto(), page.waitForTimeout()) with try { ... } catch (err) { console.error('test-html error', err); throw err; } and ensure finally { if (browser) await browser.close(); } so the browser is always closed even on error; reference the existing anonymous async function, and the browser and page variables to locate where to add try/catch/finally.
17-17: ⚡ Quick winReplace arbitrary timeout with deterministic wait condition.
Using
waitForTimeoutcreates brittle, non-deterministic tests. Prefer waiting for specific elements, network idle, or load state.⏱️ Proposed fix using network idle
- await page.waitForTimeout(2000); + await page.waitForLoadState('networkidle');Or wait for a specific element if you need to verify rendering:
- await page.waitForTimeout(2000); + await page.waitForSelector('[data-testid="dashboard"]', { state: 'visible' });🤖 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/test-html.cjs` at line 17, The test currently uses a brittle fixed delay via page.waitForTimeout(2000); replace that with a deterministic wait on page state or a specific element: use page.waitForLoadState('networkidle') if you want to wait for network activity to finish, or use page.waitForSelector('<selector>') (or page.locator(...).waitFor()) to wait until the UI element you assert on is present; update the call sites where waitForTimeout is used (reference: the page object and its waitForTimeout invocation) to the appropriate deterministic wait.screenshot.mjs (2)
3-12: ⚡ Quick winAdd error handling and ensure output directory exists.
The script lacks error handling and doesn't verify the output directory exists before writing the screenshot.
🛡️ Proposed fix to add error handling and directory check
+import { mkdir } from 'fs/promises'; +import { dirname } from 'path'; import { chromium } from '`@playwright/test`'; (async () => { + try { const browser = await chromium.launch(); const page = await browser.newPage({ viewport: { width: 1280, height: 1024 } }); await page.goto('http://localhost:3000'); - // Wait for network idle or a specific element - await page.waitForTimeout(2000); + await page.waitForLoadState('networkidle'); + + const outputPath = 'frontend/test-results/manual-screenshot.png'; + await mkdir(dirname(outputPath), { recursive: true }); - await page.screenshot({ path: 'frontend/test-results/manual-screenshot.png', fullPage: true }); + await page.screenshot({ path: outputPath, fullPage: true }); await browser.close(); - console.log('Screenshot saved to frontend/test-results/manual-screenshot.png'); + console.log(`Screenshot saved to ${outputPath}`); + } catch (error) { + console.error('Screenshot failed:', error); + process.exit(1); + } })();🤖 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 `@screenshot.mjs` around lines 3 - 12, Wrap the top-level async IIFE in a try/catch/finally to catch and log errors and to always close the browser; before calling page.screenshot use Node's fs (fs.mkdirSync or fs.promises.mkdir with { recursive: true }) to ensure the frontend/test-results directory exists; reference the async IIFE, chromium.launch(), page.screenshot({ path: 'frontend/test-results/manual-screenshot.png' }), and browser.close() when adding the try/catch/finally and directory-creation logic so the browser is closed in finally and errors are logged in catch.
8-8: ⚡ Quick winReplace arbitrary timeout with deterministic wait condition.
Using
waitForTimeoutis non-deterministic and can lead to flaky results. Wait for a specific load state or element instead.⏱️ Proposed fix using network idle
- // Wait for network idle or a specific element - await page.waitForTimeout(2000); + // Wait for network idle to ensure page is fully loaded + await page.waitForLoadState('networkidle');Or wait for a specific element to ensure content is rendered:
- // Wait for network idle or a specific element - await page.waitForTimeout(2000); + // Wait for main content to be visible + await page.waitForSelector('main', { state: 'visible' });🤖 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 `@screenshot.mjs` at line 8, Replace the arbitrary sleep call await page.waitForTimeout(2000) with a deterministic wait: use page.waitForLoadState('networkidle') to wait for network activity to finish or page.waitForSelector('<your-target-selector>') to wait for a specific element to render; locate the occurrence of page.waitForTimeout in screenshot.mjs and replace it with one of those deterministic waits (choose the selector that best represents the content you need rendered).docker-compose.apm.yml (2)
2-9: 💤 Low valueConsider adding a volume for Prometheus data persistence.
The Prometheus service has no data volume configured. Metrics will be lost when the container restarts. For development this may be acceptable, but for any persistent monitoring you'll want to add:
prometheus: image: prom/prometheus:v2.51.1 ports: - "9090:9090" command: - '--config.file=/etc/prometheus/prometheus.yml' volumes: - ./prometheus.yml:/etc/prometheus/prometheus.yml:ro + - prometheus-data:/prometheusAnd declare the volume at the end of the file:
volumes: prometheus-data:🤖 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 `@docker-compose.apm.yml` around lines 2 - 9, The Prometheus service (service name "prometheus") lacks a persistent data volume; add a named volume mount (e.g., prometheus-data:/prometheus) to the prometheus service's volumes list in the docker-compose.apm.yml so metrics persist across restarts, keep the existing ./prometheus.yml bind for config, and declare the named volume at the bottom of the file with a top-level "volumes:" section that contains "prometheus-data:".
1-18: Document how to wire the backend to this APM stack.This compose file defines the observability infrastructure, but the integration contract isn't explicit. Based on the backend code, you'll need:
For tracing (Jaeger): Set
OTEL_EXPORTER_OTLP_ENDPOINTenvironment variable for the backend:
- If backend runs on host:
http://localhost:4317- If backend runs in same Docker network:
http://jaeger:4317For metrics (Prometheus): The backend must be reachable at
host.docker.internal:8000(already configured in prometheus.yml), which requires the backend to run on the host machine.Consider adding a README section or docker-compose comments explaining:
- How to start the APM stack alongside the backend
- Required environment variables for the backend
- How to access Prometheus UI (http://localhost:9090) and Jaeger UI (http://localhost:16686)
🤖 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 `@docker-compose.apm.yml` around lines 1 - 18, Add documentation and comments to the docker-compose.apm.yml explaining how the backend should be wired to the APM stack: state that the backend must set OTEL_EXPORTER_OTLP_ENDPOINT to point at Jaeger (e.g., http://localhost:4317 when running the backend on the host, or http://jaeger:4317 when running the backend in the same Docker network), note that Prometheus expects the backend to be reachable at host.docker.internal:8000 as defined in prometheus.yml (so the backend should run on the host or publish that endpoint), and include brief startup/access instructions and URLs for the Prometheus UI (http://localhost:9090) and Jaeger UI (http://localhost:16686).frontend/src/components/WorkspaceHome.tsx (2)
74-97: ⚡ Quick winMissing AbortController prevents cancellation of in-flight requests.
Unlike
useStartupSearchwhich properly usesAbortController, this hook only sets acancelledflag. The fetch requests will complete in the background even after unmount, potentially causing unnecessary network traffic on rapid navigation.♻️ Proposed fix to add AbortController
function useDashboardData() { const [emails, setEmails] = useState<EmailItem[]>([]); const [tasks, setTasks] = useState<TaskItem[]>([]); const [loading, setLoading] = useState(true); useEffect(() => { let cancelled = false; + const controller = new AbortController(); Promise.all([ - apiClient.get<{ emails: EmailItem[] }>('/api/emails').catch(() => ({ emails: [] })), - apiClient.get<TaskItem[]>('/api/tasks').catch(() => []) + apiClient.get<{ emails: EmailItem[] }>('/api/emails', { signal: controller.signal }).catch(() => ({ emails: [] })), + apiClient.get<TaskItem[]>('/api/tasks', { signal: controller.signal }).catch(() => []) ]).then(([emailRes, tasksRes]) => { if (cancelled) return; setEmails(emailRes.emails || []); setTasks(tasksRes || []); setLoading(false); }); return () => { cancelled = true; + controller.abort(); }; }, []); return { emails, tasks, loading }; }🤖 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/src/components/WorkspaceHome.tsx` around lines 74 - 97, The hook useDashboardData currently only uses a cancelled flag so in-flight apiClient.get requests keep running after unmount; modify useDashboardData to create an AbortController inside the useEffect, pass controller.signal into both apiClient.get calls, and call controller.abort() in the cleanup instead of setting cancelled; update the Promise.all handlers to ignore AbortError responses (or rely on the catch fallback) and still call setEmails/setTasks only if the request was not aborted (check controller.signal.aborted or catch the abort explicitly) before setting loading to false.
148-148: 💤 Low valueHardcoded user name should be dynamic.
"김나루님" is hardcoded. This should come from user context/session data for a personalized greeting.
🤖 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/src/components/WorkspaceHome.tsx` at line 148, The greeting in WorkspaceHome is hardcoded ("안녕하세요, 김나루님 👋"); update the WorkspaceHome component to read the current user name from the app's user/session state (e.g., a UserContext, auth hook like useSession/useUser, or props) and render it instead of the literal string, using a safe fallback (e.g., "안녕하세요, 사용자님 👋") when name is missing; change the <h1> to interpolate the resolved displayName (e.g., displayName or user.name) so the greeting becomes dynamic and resilient to null/undefined user data.frontend/src/app/projects/page.test.tsx (1)
42-54: ⚡ Quick winTest description doesn't match assertions.
The test description mentions "decision logs and source boundaries" but the assertions only verify basic project UI text ("새 프로젝트", "진행 중", "제품 개발"). Consider updating the test description to match the actual validation scope.
🤖 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/src/app/projects/page.test.tsx` around lines 42 - 54, The test name it("renders project execution surfaces with decision logs and source boundaries", ...) is misleading because the assertions only check basic project UI text; update the test description string in frontend/src/app/projects/page.test.tsx (the it(...) for ProjectsPage) to accurately describe what is asserted (e.g., that ProjectsPage renders project UI labels like "새 프로젝트", "진행 중", "제품 개발") so the test name matches the actual validation scope.frontend/src/app/search/page.test.tsx (1)
32-43: ⚡ Quick winTest assertions don't match the test description.
The test is named "renders integrated search results detail graph and timeline states" but only verifies two text strings (
"Q2 런칭 캠페인 기획안.pdf"and"통합 검색"). Consider either updating the test name to reflect the simplified assertions, or adding back assertions for the "detail", "graph", and "timeline" elements mentioned in the description.🤖 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/src/app/search/page.test.tsx` around lines 32 - 43, The test named "renders integrated search results detail graph and timeline states" (in the test that mounts SearchPage) currently only asserts two text strings; either rename the test to reflect those simplified assertions or add assertions that actually verify the "detail", "graph", and "timeline" UI pieces on SearchPage (for example, locate them via text, role, or data-testid and assert they exist or are visible). Update the test title string if you choose to simplify, or add specific assertions (e.g., getByText/getByTestId/getByRole for the detail panel, graph container, and timeline component) to match the original description.frontend/src/app/security/page.test.tsx (1)
35-48: ⚡ Quick winTest assertions don't match the comprehensive test description.
The test is named "renders security dashboard access audit sharing and policy governance screens" but only verifies three text strings (
"보안과 관리자","감사 로그","인증 연동"). The test name promises verification of "dashboard", "access", "audit", "sharing", and "policy governance" but doesn't assert their presence.Consider either updating the test name to match the simplified assertions, or adding back verification for the governance and sharing UI elements mentioned in the description.
🤖 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/src/app/security/page.test.tsx` around lines 35 - 48, The test named "renders security dashboard access audit sharing and policy governance screens" only checks three strings but promises more; either rename the test to reflect that it only asserts presence of "보안과 관리자", "감사 로그", and "인증 연동", or add assertions to verify the remaining UI elements (e.g., dashboard, access, sharing, policy governance) by querying the rendered SecurityPage (container or root) and asserting container.textContent or specific selectors contain the expected labels/text for "dashboard", "access", "sharing", and "policy governance" (or their localized equivalents) so the test name and assertions match; update the test block containing SecurityPage accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fbe65808-a4ca-4e97-9753-850c1c360a47
⛔ Files ignored due to path filters (97)
.DS_Storeis excluded by!**/.DS_Storebackend/__pycache__/import_fixtures.cpython-310.pycis excluded by!**/*.pycbackend/__pycache__/main.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/__init__.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/accounts.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/auth.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/calendar.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/dav.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/emails.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/llm.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/llm_providers.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/network.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/ontology.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/prompts.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/runner_config.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/runner_ws.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/runtime_config.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/search.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/tasks.cpython-310.pycis excluded by!**/*.pycbackend/api/__pycache__/tenant_config.cpython-310.pycis excluded by!**/*.pycbackend/core/__pycache__/__init__.cpython-310.pycis excluded by!**/*.pycbackend/core/__pycache__/config.cpython-310.pycis excluded by!**/*.pycbackend/core/__pycache__/exceptions.cpython-310.pycis excluded by!**/*.pycbackend/db/__pycache__/__init__.cpython-310.pycis excluded by!**/*.pycbackend/db/__pycache__/models.cpython-310.pycis excluded by!**/*.pycbackend/db/__pycache__/session.cpython-310.pycis excluded by!**/*.pycbackend/scripts/__pycache__/bootstrap_db.cpython-310.pycis excluded by!**/*.pycbackend/scripts/__pycache__/import_fixtures.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/__init__.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/access_policy.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/archive.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/calendar_service.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/calendar_sync.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/email_client.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/email_parser.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/embedding.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/exceptions.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/imap_worker.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/knowledge_extractor.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/llm_provider_urls.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/llm_service.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/text_safety.cpython-310.pycis excluded by!**/*.pycbackend/services/__pycache__/threading_service.cpython-310.pycis excluded by!**/*.pycbackend/tests/__pycache__/__init__.cpython-310.pycis excluded by!**/*.pycbackend/tests/__pycache__/conftest.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_access_policy.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_accounts_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_apm_observability.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_archive.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_auth_real.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_bootstrap_db.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_calendar_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_calendar_service.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_calendar_sync.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_config.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_dav_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_db.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_email_client.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_email_client_smtp.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_email_parser.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_emails_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_embedding.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_imap_worker_sync.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_import_fixtures.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_infra_evaluations.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_knowledge_extractor.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_llm_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_llm_providers_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_llm_service.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_main.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_network_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_ontology_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_prompts_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_release_governance.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_repo_hygiene.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_runner_config_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_runtime_config_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_search.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_tasks_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_tenant_config_api.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_tenant_config_model.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_text_safety.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/__pycache__/test_threading_service.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/live/__pycache__/conftest.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/live/__pycache__/mail_smoke_test.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycbackend/tests/live/__pycache__/test_live_api_sequence.cpython-310-pytest-9.0.3.pycis excluded by!**/*.pycfrontend/branding/naruon-ux-mockup-1.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-10.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-2.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-3.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-4.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-5.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-6.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-7.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-8.pngis excluded by!**/*.pngfrontend/branding/naruon-ux-mockup-9.pngis excluded by!**/*.pngfrontend/dev.logis excluded by!**/*.log
📒 Files selected for processing (65)
.github/workflows/strix.yml.gitignoreAGENTS.mdREADME.mdbackend/api/accounts.pybackend/api/dav.pybackend/api/emails.pybackend/api/ontology.pybackend/api/runner_ws.pybackend/core/config.pybackend/core/rbac.pybackend/db/models.pybackend/main.pybackend/runner/agent.pybackend/services/calendar_sync.pybackend/services/knowledge_extractor.pybackend/services/text_safety.pybackend/services/threading_service.pybackend/tests/test_accounts_api.pybackend/tests/test_calendar_sync.pybackend/tests/test_dav_api.pybackend/tests/test_emails_api.pybackend/tests/test_knowledge_extractor.pybackend/tests/test_ontology_api.pydocker-compose.apm.ymldocs/architecture/naruon-product-spec.mddocs/architecture/self-hosted-runner-design.mddocs/plans/2026-05-24-north-star-master-spec.mdfrontend/debug.htmlfrontend/next.config.tsfrontend/screenshot.cjsfrontend/src/app/ai-hub/page.test.tsxfrontend/src/app/ai-hub/page.tsxfrontend/src/app/calendar/page.test.tsxfrontend/src/app/calendar/page.tsxfrontend/src/app/data/page.test.tsxfrontend/src/app/data/page.tsxfrontend/src/app/globals.cssfrontend/src/app/page.test.tsxfrontend/src/app/projects/page.test.tsxfrontend/src/app/projects/page.tsxfrontend/src/app/search/page.test.tsxfrontend/src/app/search/page.tsxfrontend/src/app/security/page.test.tsxfrontend/src/app/security/page.tsxfrontend/src/app/settings/page.tsxfrontend/src/app/tasks/page.test.tsxfrontend/src/app/tasks/page.tsxfrontend/src/components/AIHubLayout.tsxfrontend/src/components/CalendarLayout.tsxfrontend/src/components/DashboardLayout.test.tsxfrontend/src/components/DashboardLayout.tsxfrontend/src/components/DataLayout.tsxfrontend/src/components/ProjectsLayout.tsxfrontend/src/components/SearchLayout.tsxfrontend/src/components/SecurityLayout.tsxfrontend/src/components/SettingsLayout.tsxfrontend/src/components/TasksLayout.tsxfrontend/src/components/WorkspaceHome.tsxfrontend/test-html.cjsfrontend/tests/e2e/dashboard-branding.spec.tsfrontend/tests/e2e/dashboard-flows.spec.tsfrontend/tests/e2e/live-smoke.spec.tsprometheus.ymlscreenshot.mjs
Comment on lines
+53
to
+57
| if not config: | ||
| config = TenantConfig(user_id=auth_ctx.user_id) | ||
| db.add(config) | ||
| await db.commit() | ||
| await db.refresh(config) |
Contributor
There was a problem hiding this comment.
Handle first-write races on tenant config creation.
Both handlers do a read-then-insert flow on a unique key. Two concurrent requests for a new user can collide and throw a 500 on commit.
Suggested hardening
+from sqlalchemy.exc import IntegrityError
...
if not config:
config = TenantConfig(user_id=auth_ctx.user_id)
db.add(config)
- await db.commit()
+ try:
+ await db.commit()
+ except IntegrityError:
+ await db.rollback()
+ config = await db.scalar(
+ select(TenantConfig).where(TenantConfig.user_id == auth_ctx.user_id)
+ )
+ if config is None:
+ raise
await db.refresh(config)Also applies to: 86-95
🤖 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 `@backend/api/accounts.py` around lines 53 - 57, The read-then-insert for
TenantConfig can race and cause a 500 on commit; change the create path to be
idempotent by catching unique-constraint/IntegrityError around db.commit() (the
block that calls db.add(config); await db.commit(); await db.refresh(config)),
rollback the session, then re-query for the existing TenantConfig and use that
result; apply the same pattern for the other identical create site (the block
around TenantConfig creation at the second handler). Ensure you import and catch
the DB IntegrityError from your DB library (e.g., sqlalchemy.exc.IntegrityError)
and only re-query after rollback so concurrent inserts resolve to a single
persisted TenantConfig.
Comment on lines
+8
to
+9
| @router.api_route("/{path:path}", methods=["PROPFIND", "REPORT", "MKCOL", "GET", "PUT", "DELETE", "OPTIONS"]) | ||
| async def dav_handler(request: Request, path: str): |
Contributor
There was a problem hiding this comment.
Require authentication on DAV methods before processing requests.
The handler currently accepts unauthenticated DAV traffic (including PUT), which exposes write-capable behavior to anonymous clients.
Suggested hardening
-from fastapi import APIRouter, Request, Response
+from fastapi import APIRouter, Depends, Request, Response
+from api.auth import AuthContext, get_auth_context
...
async def dav_handler(
request: Request,
path: str,
+ auth_ctx: AuthContext = Depends(get_auth_context),
):Also applies to: 44-48
🤖 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 `@backend/api/dav.py` around lines 8 - 9, The DAV endpoint defined by
router.api_route and handled in dav_handler currently allows unauthenticated
access (including write methods like PUT/DELETE); modify dav_handler to enforce
authentication at the start of the function by invoking the existing auth check
(e.g., call your auth helper or use FastAPI dependency like
Depends(get_current_user)) and return an appropriate 401/403 response for
unauthenticated requests before any DAV processing; apply the same
authentication guard to the other DAV route block referenced (lines 44-48) so
all PROPFIND, REPORT, MKCOL, GET, PUT, DELETE, OPTIONS traffic requires a valid
user/session.
Comment on lines
+29
to
+37
| xml_response = f"""<?xml version="1.0" encoding="utf-8" ?> | ||
| <D:multistatus xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav"> | ||
| <D:response> | ||
| <D:href>/api/dav/{path}</D:href> | ||
| <D:propstat> | ||
| <D:prop> | ||
| <D:resourcetype>{resourcetype}</D:resourcetype> | ||
| <D:displayname>{path.split("/")[-1] or "Root"}</D:displayname> | ||
| </D:prop> |
Contributor
There was a problem hiding this comment.
Build DAV XML values safely and with the correct DAV base path.
href currently points to /api/dav/... (route is /dav/...), and raw path values are inserted into XML without escaping.
Suggested fix
+from urllib.parse import quote
+from xml.sax.saxutils import escape
...
- xml_response = f"""<?xml version="1.0" encoding="utf-8" ?>
+ safe_path = quote(path, safe="/")
+ display_name = escape(path.split("/")[-1] or "Root")
+ xml_response = f"""<?xml version="1.0" encoding="utf-8" ?>
<D:multistatus xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
<D:response>
- <D:href>/api/dav/{path}</D:href>
+ <D:href>/dav/{safe_path}</D:href>
...
- <D:displayname>{path.split("/")[-1] or "Root"}</D:displayname>
+ <D:displayname>{display_name}</D:displayname>🤖 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 `@backend/api/dav.py` around lines 29 - 37, The XML response construction in
xml_response incorrectly hardcodes the DAV base path as "/api/dav/" and injects
unescaped user-controlled strings (path, resourcetype, displayname) directly
into the XML; update the href to use the correct base "/dav/" and HTML-escape/
XML-escape all inserted values (the variables used when building xml_response:
path, resourcetype, and the display name computed via path.split("/")[-1])
before interpolation so the generated <D:href>, <D:resourcetype> and
<D:displayname> are safe and correct.
Comment on lines
+29
to
+31
| stmt = select(SenderRelationship).where(SenderRelationship.user_id == user_id) | ||
| result = await db.execute(stmt) | ||
| rels = result.scalars().all() |
Contributor
There was a problem hiding this comment.
Scope relationship reads/writes by organization, not only user.
Current queries/upsert keys ignore organization_id, so records can be read/overwritten across org contexts for the same user identity.
Suggested fix
stmt = select(SenderRelationship).where(
- SenderRelationship.user_id == user_id
+ SenderRelationship.user_id == user_id,
+ SenderRelationship.organization_id == auth_ctx.organization_id,
)
...
stmt = select(SenderRelationship).where(
SenderRelationship.user_id == user_id,
+ SenderRelationship.organization_id == auth_ctx.organization_id,
SenderRelationship.sender_email == req.sender_email
)
...
rel = SenderRelationship(
user_id=user_id,
+ organization_id=auth_ctx.organization_id,
sender_email=req.sender_email,
relationship_type=req.relationship_type,
confidence_score=req.confidence_score
)Also applies to: 49-65
🤖 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 `@backend/api/ontology.py` around lines 29 - 31, The query and any upsert logic
for SenderRelationship currently only filters by user_id, allowing
cross-organization reads/overwrites; update the select statement
(select(SenderRelationship).where(SenderRelationship.user_id == user_id)) to
also filter by SenderRelationship.organization_id == organization_id and modify
any create/update/upsert paths to use the composite key (user_id,
organization_id) when locating or inserting records; apply the same change to
the other affected block(s) referenced (lines 49-65) so all reads and writes are
scoped by organization_id.
Comment on lines
+13
to
+17
| async def connect(self, ws: WebSocket, token: str): | ||
| await ws.accept() | ||
| # In a real scenario, validate token against WorkspaceRunnerConfig | ||
| self.active_connections[token] = ws | ||
| logger.info(f"Runner connected with token {token}") |
Contributor
There was a problem hiding this comment.
Block unauthenticated runner websocket connections before accept().
Any client can connect with any token today, and token collisions can overwrite active runner channels.
Also applies to: 26-28
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 17-17: Log Injection
This log entry depends on a user-provided value.
🤖 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 `@backend/api/runner_ws.py` around lines 13 - 17, The connect method currently
accepts the WebSocket before validating the token and then stores it in
self.active_connections (which can allow unauthenticated connects and token
collisions); update RunnerWebSocket.connect to validate the token (using the
existing WorkspaceRunnerConfig or an auth helper) BEFORE calling ws.accept(),
and if the token is invalid or already in self.active_connections reject/close
the socket (return early) instead of accepting; also apply the same pre-accept
validation logic to the related disconnect/registration block referenced around
lines 26-28 to avoid overwriting active runner channels.
| } | ||
|
|
||
| describe("Home workspace action bridge", () => { | ||
| describe.skip("Home workspace action bridge", () => { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for E2E tests covering workspace action bridge, mobile workspace, and API integration
rg -n -C3 --type=ts -g 'tests/e2e/**' 'naruon:header-action|naruon:mobile-workspace|setMobileWorkspaceView'Repository: Seongho-Bae/naruon
Length of output: 211
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm whether tests/e2e exists (and list top-level test dirs)
echo "== test directory discovery =="
ls -la
find . -maxdepth 3 -type d \( -iname 'e2e' -o -iname 'tests' \) | sed 's|^\./||' | sort | head -n 200
# 2) Locate the skipped suite and confirm range/contents
echo
echo "== locate describe.skip in frontend/src/app/page.test.tsx =="
ls -la frontend/src/app/page.test.tsx || true
rg -n --type=tsx 'describe\.skip\("Home workspace action bridge"' frontend/src/app/page.test.tsx || true
# 3) Determine what test runner uses E2E vs unit tests and where E2E lives
echo
echo "== locate e2e config references =="
rg -n --hidden -S --glob='!node_modules/**' 'playwright|cypress|webdriver|e2e|test:e2e' . || true
# 4) Re-run search for the specific strings across likely E2E dirs (without restricting to tests/e2e/**)
echo
echo "== search for likely E2E coverage markers =="
# List dirs that plausibly contain E2E tests
for d in tests e2e playwright cypress frontend/tests; do
if [ -d "./$d" ]; then echo "--- scanning ./$d ---"; fi
done
rg -n -C2 --type=ts --type=tsx --type=js --type=jsx \
'naruon:header-action|naruon:mobile-workspace|setMobileWorkspaceView|Home workspace action bridge' \
. --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' || true
# 5) Search for signed-session markers in tests (regardless of E2E/units)
echo
echo "== search signed-session markers in tests =="
rg -n -C2 --type=ts --type=tsx --type=js --type=jsx \
'naruon_session_token|Authorization: Bearer|/api/search|/api/tasks' \
. --glob '*test*' --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' || trueRepository: Seongho-Bae/naruon
Length of output: 38738
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) List Playwright E2E specs
echo "== frontend Playwright e2e specs =="
ls -ლა frontend/tests/e2e
echo
# 2) Search E2E specs for the skipped-suite markers and signed-session/API routes
echo "== Search in frontend/tests/e2e for key markers =="
rg -n --glob 'frontend/tests/e2e/**/*.{ts,tsx,js,jsx}' \
'Home workspace action bridge|setMobileWorkspaceView|mobile-workspace|hash|`#/`|/api/search|/api/tasks|naruon_session_token|Authorization: Bearer|X-User-Id|X-Organization-Id|X-Group-Id|X-User-Role|X-Dev-Auth-Token' \
frontend/tests/e2e || true
echo
# 3) Inspect playwright config to see which projects/resolutions exist
echo "== frontend/playwright.config.ts =="
sed -n '1,220p' frontend/playwright.config.ts
echo
# 4) Re-check the skipped suite block in frontend/src/app/page.test.tsx (start + end + any rationale comment)
echo "== Skipped suite block excerpt (page.test.tsx around describe.skip) =="
sed -n '60,140p' frontend/src/app/page.test.tsx
echo
echo "== Skipped suite block excerpt (end around line 730) =="
sed -n '700,770p' frontend/src/app/page.test.tsx
echo
# 5) Confirm whether there are other tests (non-skipped) covering these user flows
echo "== Search in frontend/src/** tests for naruon_session_token or /api/search+/api/tasks =="
rg -n --glob 'frontend/src/**/*.{ts,tsx}' \
'naruon_session_token|/api/search|/api/tasks|Authorization: Bearer|X-User-Id|X-Organization-Id|X-Group-Id|X-User-Role|X-Dev-Auth-Token' \
frontend/src || true
echo
echo "== Search in frontend/** tests (non-e2e included) for setMobileWorkspaceView / mobile-workspace markers =="
rg -n --glob 'frontend/**/*.{ts,tsx,js,jsx}' \
'setMobileWorkspaceView|mobile-workspace|hash routing|hash.*route|Home workspace action bridge' \
frontend/tests frontend/src || trueRepository: Seongho-Bae/naruon
Length of output: 180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== frontend/tests/e2e directory listing =="
ls -la frontend/tests/e2e
echo
echo "== Search Playwright E2E specs for skipped-suite markers =="
rg -n -C2 \
-g'*.{ts,tsx,js,jsx}' \
'Home workspace action bridge|setMobileWorkspaceView|mobile-workspace|hash routing|hash.*route|/api/search|/api/tasks|naruon_session_token|Authorization:\s*Bearer|X-User-Id|X-Organization-Id|X-Group-Id|X-User-Role|X-Dev-Auth-Token' \
frontend/tests/e2e || true
echo
echo "== Search page.test.tsx describe.skip block (start) =="
sed -n '70,110p' frontend/src/app/page.test.tsx
echo
echo "== Search page.test.tsx describe.skip block (end) =="
sed -n '700,760p' frontend/src/app/page.test.tsx
echo
echo "== Count occurrences of signed-session /api usage in non-e2e tests for context =="
rg -n -C1 \
-g'*.{ts,tsx,js,jsx}' \
'naruon_session_token|/api/search|/api/tasks|Authorization:\s*Bearer|X-User-Id|X-Organization-Id|X-Group-Id|X-User-Role|X-Dev-Auth-Token' \
frontend/src frontend/tests \
|| trueRepository: Seongho-Bae/naruon
Length of output: 12018
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect relevant E2E specs for mobile/hash/workspace switching and header-action bridging
echo "== Peek dashboard-flows.spec.ts (top) =="
sed -n '1,260p' frontend/tests/e2e/dashboard-flows.spec.ts
echo
echo "== Search dashboard-flows.spec.ts for mobile/workspace/hash/header-action markers =="
rg -n 'mobile-workspace|setMobileWorkspaceView|#|hash routing|naruon:header-action|reply-draft|email|shell action|/api/search|/api/tasks' \
frontend/tests/e2e/dashboard-flows.spec.ts || true
echo
echo "== Search dashboard-branding.spec.ts for mobile-workspace/menu/hash markers =="
rg -n 'mobile-workspace|#|hash|mobile-search|mobile-calendar|일정 후보|워크스페이스|naruon:header-action|/api/search|/api/tasks' \
frontend/tests/e2e/dashboard-branding.spec.ts || true
echo
echo "== Inspect helpers.ts where /api/search and /api/tasks are mocked =="
sed -n '1,260p' frontend/tests/e2e/helpers.ts
echo
echo "== Search helpers.ts for Authorization/Bearer/naruon_session_token validation =="
rg -n 'Authorization|Bearer|naruon_session_token|X-User-Id|X-Organization-Id|X-Group-Id|X-Group-Ids|X-User-Role|X-Dev-Auth-Token' \
frontend/tests/e2e/helpers.ts || trueRepository: Seongho-Bae/naruon
Length of output: 11574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search skipped suite for naruon_session_token / Authorization expectations =="
rg -n 'describe\.skip\("Home workspace action bridge"|naruon_session_token|Authorization:\s*Bearer|X-User-Id|X-Organization-Id|X-Group-Id|X-User-Role|X-Dev-Auth-Token|naruon:header-action|setMobileWorkspaceView|mobile-workspace|/api/tasks/from-email|reply-draft|reply send|캘린더 반영' \
frontend/src/app/page.test.tsx || true
echo
echo "== Extract the portion where header-action bridge is exercised (find naruon:header-action) =="
rg -n -C3 'naruon:header-action' frontend/src/app/page.test.tsx || true
echo
echo "== Extract the portion where signed session token is used (find naruon_session_token) =="
rg -n -C3 'naruon_session_token' frontend/src/app/page.test.tsx || true
echo
echo "== Re-scan Playwright E2E specs for naruon:header-action markers =="
rg -n 'naruon:header-action|setMobileWorkspaceView|mobile-workspace' frontend/tests/e2e || trueRepository: Seongho-Bae/naruon
Length of output: 4209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Playwright config projects/viewports
echo "== frontend/playwright.config.ts (projects/devices) =="
sed -n '1,220p' frontend/playwright.config.ts
echo
echo "== Search within skipped suite for naruon_session_token and Authorization/Bearer =="
rg -n 'naruon_session_token|Authorization:\s*Bearer|X-User-Id|X-Organization-Id|X-Group-Id|X-User-Role|X-Dev-Auth-Token' frontend/src/app/page.test.tsx || true
echo
echo "== Search for mobile resize / tablet coverage assertions in skipped suite =="
rg -n 'resize|viewport|1024|768|tablet|mobile|matchMedia' frontend/src/app/page.test.tsx || true
echo
echo "== Peek mobile resize/hash related assertions in skipped suite =="
sed -n '520,720p' frontend/src/app/page.test.tsxRepository: Seongho-Bae/naruon
Length of output: 13096
Unskip or re-home missing assertions in the skipped “Home workspace action bridge” suite.
frontend/src/app/page.test.tsx currently skips the entire describe.skip("Home workspace action bridge", ...) block (~660 lines), removing fast regression coverage for:
naruon:header-actionbridge behavior (e.g.,reply-draft,calendar-sync)- mobile workspace + hash/deep-link override logic (
setMobileWorkspaceView,naruon:mobile-workspace,hashchange) - matchMedia-driven desktop↔tablet↔mobile resize/replay prevention
Playwright E2E exists (frontend/tests/e2e/dashboard-flows.spec.ts, frontend/tests/e2e/dashboard-branding.spec.ts), but it uses mockDashboardApi to fulfill /api/** (including /api/search, /api/tasks, /api/calendar/writeback-intent) without asserting signed-session request headers—so it doesn’t exercise the guideline requiring browser writes to signed backend routes to send Authorization: Bearer <naruon_session_token>.
Restore this suite (or split out the missing header-action + signed-session header assertions into non-skipped tests) rather than skipping wholesale.
🤖 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/src/app/page.test.tsx` at line 80, Unskip or extract the assertions
currently hidden by describe.skip("Home workspace action bridge", ...) in
frontend/src/app/page.test.tsx: either remove .skip on that describe block or
move the specific tests that assert naruon:header-action behaviors (e.g.,
reply-draft, calendar-sync), naruon:mobile-workspace and
setMobileWorkspaceView/hashchange deep-link logic, and matchMedia resize/replay
prevention into focused test files; additionally add tests that exercise
signed-session browser writes by asserting the Authorization: Bearer
<naruon_session_token> header is sent to signed backend routes (mirror what
mockDashboardApi covers in frontend/tests/e2e/* but include header assertions),
referencing the existing mocks and helpers used in the suite so tests can locate
and assert against the same handlers.
Comment on lines
+42
to
+46
| <div | ||
| key={proj.id} | ||
| onClick={() => setActiveProject(proj)} | ||
| className={`cursor-pointer rounded-lg px-3 py-3 transition-colors ${activeProject.id === proj.id ? 'bg-secondary border border-primary/20' : 'hover:bg-secondary/50 border border-transparent'}`} | ||
| > |
Contributor
There was a problem hiding this comment.
Project selection cards should be keyboard-accessible controls.
Using clickable <div> for project selection blocks proper keyboard interaction. Use semantic <button> (or add full button semantics/handlers) for each project row.
Suggested fix
- <div
+ <button
+ type="button"
key={proj.id}
onClick={() => setActiveProject(proj)}
className={`cursor-pointer rounded-lg px-3 py-3 transition-colors ${activeProject.id === proj.id ? 'bg-secondary border border-primary/20' : 'hover:bg-secondary/50 border border-transparent'}`}
>
...
- </div>
+ </button>📝 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
| <div | |
| key={proj.id} | |
| onClick={() => setActiveProject(proj)} | |
| className={`cursor-pointer rounded-lg px-3 py-3 transition-colors ${activeProject.id === proj.id ? 'bg-secondary border border-primary/20' : 'hover:bg-secondary/50 border border-transparent'}`} | |
| > | |
| <button | |
| type="button" | |
| key={proj.id} | |
| onClick={() => setActiveProject(proj)} | |
| className={`cursor-pointer rounded-lg px-3 py-3 transition-colors ${activeProject.id === proj.id ? 'bg-secondary border border-primary/20' : 'hover:bg-secondary/50 border border-transparent'}`} | |
| > |
🤖 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/src/components/ProjectsLayout.tsx` around lines 42 - 46, The project
tiles in ProjectsLayout.tsx use a clickable <div>, which is not
keyboard-accessible; replace the <div key={proj.id} ...> used to render each
project with a semantic <button type="button"> (preserving the same className,
onClick={() => setActiveProject(proj)}, and key={proj.id}) or, if you must keep
a non-button, add full button semantics (role="button", tabIndex={0}, onKeyDown
handling Enter/Space to call setActiveProject(proj)) and set aria-pressed (or
aria-current) based on activeProject.id === proj.id so keyboard and assistive
tech can interact correctly. Ensure styling and focus outline remain usable when
switching to a button.
Comment on lines
+55
to
+59
| <div | ||
| key={res.id} | ||
| onClick={() => setActiveResult(res)} | ||
| className={`cursor-pointer p-4 transition-colors ${activeResult.id === res.id ? 'bg-secondary/50 border-l-4 border-primary' : 'hover:bg-secondary/20 border-l-4 border-transparent'}`} | ||
| > |
Contributor
There was a problem hiding this comment.
Search result items need semantic interactive elements.
Result selection is bound to clickable <div> elements, so keyboard activation/focus behavior is incomplete. Convert to <button type="button"> for accessible selection.
🤖 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/src/components/SearchLayout.tsx` around lines 55 - 59, Replace the
clickable <div> used for search results with a semantic interactive element:
change the element rendered in SearchLayout.tsx that currently uses key={res.id}
onClick={() => setActiveResult(res)} and the className that checks
activeResult.id to a <button type="button"> so it supports keyboard
focus/activation; keep the same key, className, and onClick handler (or use
onKeyDown if you need custom keyboard handling), and remove any non-button
roles/ARIA that conflict—ensure setActiveResult(res) remains the handler and the
activeResult.id conditional styling is preserved.
Comment on lines
+30
to
+31
| const [viewMode, setViewMode] = useState<'내 작업' | '위임한 작업' | '칸반'>('칸반'); | ||
|
|
Contributor
There was a problem hiding this comment.
View mode switch does not affect rendered content.
viewMode updates, but the page always renders the Kanban board. Selecting “내 작업” or “위임한 작업” currently has no effect.
Also applies to: 40-48, 66-112
🤖 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/src/components/TasksLayout.tsx` around lines 30 - 31, The viewMode
state is being updated but never used to conditionally render content; update
the TasksLayout component to switch rendered UI based on viewMode (the useState
hook named viewMode and setter setViewMode). Implement a clear
conditional/render switch (e.g., if viewMode === '칸반' render the KanbanBoard
component, if viewMode === '내 작업' render the MyTasksList filtered by current
user, if viewMode === '위임한 작업' render the DelegatedTasksList filtered by tasks
delegated by current user), ensure the view-mode buttons call setViewMode
correctly, and pass the appropriate filtered task data or props to the target
components (KanbanBoard, MyTasksList, DelegatedTasksList) so the selected mode
actually changes the displayed content.
Comment on lines
+11
to
+14
| await page.route('**/api/**', async (route) => { | ||
| if (route.request().method() === 'OPTIONS') return route.fulfill({ status: 204, headers: {'Access-Control-Allow-Origin': '*'} }); | ||
| return route.fulfill({ status: 200, contentType: 'application/json', body: '{"emails": [], "tasks": []}', headers: {'Access-Control-Allow-Origin': '*'} }); | ||
| }); |
Contributor
There was a problem hiding this comment.
API mock should exercise signed-session authentication path.
The mock intercepts all /api/** requests but doesn't validate or require Authorization: Bearer headers. As per coding guidelines, tests and mocks must exercise the signed-session path to accurately reflect production authentication flows.
🔐 Proposed fix to validate authentication headers
await page.route('**/api/**', async (route) => {
- if (route.request().method() === 'OPTIONS') return route.fulfill({ status: 204, headers: {'Access-Control-Allow-Origin': '*'} });
- return route.fulfill({ status: 200, contentType: 'application/json', body: '{"emails": [], "tasks": []}', headers: {'Access-Control-Allow-Origin': '*'} });
+ if (route.request().method() === 'OPTIONS') {
+ return route.fulfill({ status: 204, headers: {'Access-Control-Allow-Origin': '*'} });
+ }
+
+ const authHeader = route.request().headers()['authorization'];
+ if (!authHeader || !authHeader.startsWith('Bearer ')) {
+ return route.fulfill({ status: 401, contentType: 'application/json', body: '{"error": "Unauthorized"}' });
+ }
+
+ return route.fulfill({ status: 200, contentType: 'application/json', body: '{"emails": [], "tasks": []}', headers: {'Access-Control-Allow-Origin': '*'} });
});Based on coding guidelines: Browser frontend writes to signed backend routes must carry the stored naruon_session_token as Authorization: Bearer; tests/mocks must exercise the signed-session path.
🤖 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/test-html.cjs` around lines 11 - 14, Update the Playwright route
handler to enforce the signed-session authentication path: in the page.route
handler (the async callback passed to page.route), read the expected session
token from the test browser state (e.g., await page.evaluate(() =>
localStorage.getItem('naruon_session_token')) or from the cookie named
"naruon_session_token"), then inspect route.request().headers()['authorization']
and reject requests that do not equal `Bearer ${token}` by fulfilling with a
401/403 response; only fulfill with the 200 mock body when the Authorization
header matches. Ensure OPTIONS still returns 204 as before.
This was referenced May 24, 2026
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
2 participants
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.
Phase 1-10 Consolidated PR
This PR consolidates all work from Phase 1 through Phase 10 into a single merge to master.
Key changes:
Closes chained PRs #210, #211, #212, #213 and stale fix PRs #199, #200, #203, #204, #209.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Infrastructure