Skip to content

Enterprise RBAC / SSO 기반 정리 + Workspace 반응형 UX 보강 - #190

Closed
seonghobae wants to merge 8 commits into
masterfrom
feature/enterprise-rbac-20260513
Closed

Enterprise RBAC / SSO 기반 정리 + Workspace 반응형 UX 보강#190
seonghobae wants to merge 8 commits into
masterfrom
feature/enterprise-rbac-20260513

Conversation

@seonghobae

@seonghobae seonghobae commented May 13, 2026

Copy link
Copy Markdown
Contributor

목표

Issue #188, #189를 함께 처리합니다. 실제 SaaS 운영에 필요한 다층 RBAC 준비 기반을 도입하고, MacBook M1/브라우저 축소 환경에서의 워크스페이스 접근성 문제(사이드바 스크롤, 오늘의 인사이트 접근, DAG 축소 미대응)를 함께 정리합니다.

구현 사항

  1. AuthContext / scoped role foundation
    • , , , 역할축을 가진 를 도입했습니다.
    • 현재는 header fallback 기반이지만, Keycloak/Casdoor OIDC claims로 자연스럽게 이어질 수 있도록 구조를 정리했습니다.
    • 를 신뢰하지 않고, 중심으로 workspace를 유도하도록 수정해 cross-tenant 우회를 제거했습니다.
  2. Organization-scoped resource authorization 정렬
    • 와 가 동일한 scoped auth 규칙( 또는 )을 사용하도록 일치시켰습니다.
    • 개인 메일박스 설정은 기존처럼 사용자 소유(personal scope)로 유지했습니다.
  3. Responsive workspace shell
    • 좌측 사이드바에 독립 스크롤 영역을 추가해 가 축소 환경에서도 도달 가능하도록 만들었습니다.
    • 관계 DAG 그래프가 viewport resize에 따라 되도록 보강했습니다.

검증

  • backend:
  • frontend:
  • frontend:
  • direct browser UAT:
    • sidebar scroll region:
    • graph width: (1280 -> 1100 resize 시 축소 확인)

관련 이슈

Resolves: #188
Resolves: #189

Summary by CodeRabbit

  • New Features

    • Scoped auth contexts with platform/organization/group/member roles and org-scoped workspace derivation
    • Dashboard sidebar now independently scrollable
    • Network graph auto-refits on viewport/resize
  • Bug Fixes

    • Clearer, specific authorization error messages and stricter tenant/mailbox/org access enforcement
  • Tests

    • Expanded backend and frontend tests for scoped auth, org-scoped runner flows, tenant access, and UI behaviors
  • Documentation

    • Updated architecture/auth docs and added enterprise RBAC & workspace responsiveness plan

Review Change Stack

Normalize trusted dev headers into an auth context so organization-scoped runner resources stop assuming one workspace per user while preserving current header-based local flows. Clarify tenant config ownership boundaries so mailbox settings remain user-owned as broader org roles are introduced.
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'version'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

This PR introduces a structured AuthContext with scoped roles and org/group scope, adds organization/group/role-assignment ORM models, migrates multiple API endpoints to use AuthContext-based dependencies with tightened authorization, updates tenant error messages, and makes frontend sidebar and network graph responsive to container sizing.

Changes

Enterprise RBAC and Responsive Workspace

Layer / File(s) Summary
Docs: architecture & plan
ARCHITECTURE.md, README.md, docs/plans/2026-05-13-enterprise-rbac-and-responsive-workspace.md
Docs updated to describe backend-normalized auth context, scoped roles, org/group scoping, and an implementation roadmap.
Auth context types and helpers
backend/api/auth.py
RoleName and frozen AuthContext dataclass; header normalization, role derivation, group parsing, workspace-id computation, ensure_organization_access(), and dependency get_auth_context / build_auth_context.
FastAPI dependency graph updates
backend/api/auth.py
Adds get_auth_context() and updates get_current_user(), get_current_workspace_id(), and get_current_user_role() to return values derived from AuthContext.
Organization and role assignment ORM models
backend/db/models.py
Adds Organization, OrganizationGroup, and ScopedRoleAssignment models with relationships and adds organization_id to WorkspaceRunnerConfig.
Tenant config authorization with error clarity
backend/api/tenant_config.py, backend/tests/test_tenant_config_api.py
Introduces MAILBOX_MANAGE_FORBIDDEN/MAILBOX_VIEW_FORBIDDEN and updates endpoints/tests to reject cross-user manage/view with specific messages.
LLM providers API auth context migration
backend/api/llm_providers.py, backend/tests/test_llm_providers_api.py
Migrates endpoints to get_auth_context, enforces platform_admin/organization_admin roles, and refactors tests with structured MockSession and role-scoped client fixtures; tightens CRUD assertions including delete.
AuthContext and ensure_organization_access tests
backend/tests/test_auth_real.py
Autouse fixtures and tests validating scoped role parsing, legacy dev fallback, and cross-organization access rejection.
Runner config API auth context and org scoping
backend/api/runner_config.py, backend/tests/test_runner_config_api.py
Uses _check_org_admin returning AuthContext, derives workspace_id from context, queries by organization_id, and expands tests for org-admin/platform-admin flows and shared workspace behavior.
Dashboard sidebar scrollable region
frontend/src/components/DashboardLayout.tsx, frontend/src/components/DashboardLayout.test.tsx
Adds overflow-hidden outer aside and data-testid="sidebar-scroll-region" wrapper for scrollable sidebar content; test asserts scroll region contains the insights card and expected overflow classes.
Network graph viewport resize handling
frontend/src/components/NetworkGraph.tsx, frontend/src/components/NetworkGraph.test.tsx
Renders vis Network in a ref container, uses ResizeObserver to call network.fit({ animation: false }) on resize with proper cleanup; updates min-heights and adds tests mocking ResizeObserver and fit/destroy.

Sequence Diagram(s)

sequenceDiagram
  participant Client as HTTP Client (with headers)
  participant FastAPI as FastAPI Dependency Graph
  participant AuthModule as backend/api/auth.py
  participant Handler as API Endpoint (e.g., /api/llm-providers)
  Client->>FastAPI: Request with X-User-Id, X-User-Role, X-Organization-Id, X-Group-Ids
  FastAPI->>AuthModule: get_auth_context()
  AuthModule->>AuthModule: normalize headers, derive role, parse groups, derive workspace
  AuthModule->>FastAPI: AuthContext (user_id, role, organization_id, group_ids, workspace_id)
  FastAPI->>Handler: endpoint(auth_context)
  Handler->>FastAPI: authorize (role/org checks via ensure_organization_access)
  FastAPI->>Client: 200 OK or 401/403
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

"🐰 I nibble headers, stitch roles with care,
platform, org, and group now share.
Sidebar scrolls, the graph finds its fit,
tokens rotate, permissions knit.
Hop—tests green, then merge, commit!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.64% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly summarizes the two main changes: enterprise RBAC/SSO foundation setup and workspace responsive UX improvements, directly matching the linked issues #188 and #189.
Linked Issues check ✅ Passed All coding requirements from #188 and #189 are met: AuthContext/scoped roles foundation introduced, organization-scoped authorization aligned, and responsive workspace UI improvements implemented (sidebar scrolling, graph viewport resize handling).
Out of Scope Changes check ✅ Passed All changes directly support the PR objectives: backend auth architecture changes enable scoped RBAC, frontend layout/graph improvements address responsive UX goals, and documentation captures the implementation plan.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/enterprise-rbac-20260513

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for a5f764b19c97fc1f3f766ed94fb92a7af898f24f:

  • 3 required check(s) are not successful on a5f764b.\n- Missing current-head CodeRabbit/coderabbitai evidence for a5f764b.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for a5f764b19c97fc1f3f766ed94fb92a7af898f24f:

  • 2 required check(s) are not successful on a5f764b.\n- Missing current-head CodeRabbit/coderabbitai evidence for a5f764b.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for a5f764b19c97fc1f3f766ed94fb92a7af898f24f:

  • 2 required check(s) are not successful on a5f764b.\n- Missing current-head CodeRabbit/coderabbitai evidence for a5f764b.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai approve

@greptile-apps

greptile-apps Bot commented May 13, 2026

Copy link
Copy Markdown

Greptile Summary

이 PR은 다층 RBAC 기반(AuthContext with platform/organization/group/member 역할)을 도입하고, 워크스페이스 사이드바 독립 스크롤 및 관계 그래프 뷰포트 반응형 대응을 추가합니다. 백엔드는 헤더 기반 개발 auth를 유지하면서 구조를 Keycloak/Casdoor 이행에 맞게 정리했습니다.

  • AuthContext 도입: get_current_user / get_current_workspace_id 래퍼가 AuthContext를 경유하도록 리팩터링, ensure_organization_access, _check_org_admin, _get_target_organization_id 헬퍼 추가.
  • 스키마 변경: WorkspaceRunnerConfigorganization_id (NOT NULL, UNIQUE) 컬럼 추가 및 Organization, OrganizationGroup, ScopedRoleAssignment 신규 모델 추가 — Alembic 없이 create_all 기반이므로 기존 테이블 마이그레이션 스크립트가 별도로 필요합니다.
  • 프론트엔드: 사이드바를 data-testid=\"sidebar-scroll-region\" div로 래핑하여 min-h-0 flex-1 overflow-y-auto 독립 스크롤 적용, NetworkGraph에 ResizeObserver 기반 fit() 재계산 추가.

Confidence Score: 4/5

프론트엔드 변경과 auth 리팩터링은 안전하지만, workspace_runner_configs 테이블에 NOT NULL 컬럼이 마이그레이션 없이 추가되어 기존 DB 환경에서 배포 시 런타임 쿼리 오류가 발생할 수 있습니다.

organization_id 컬럼이 NOT NULL로 추가됐지만 프로젝트는 create_all() 기반이므로 이미 테이블이 생성된 환경에서는 컬럼이 추가되지 않아 runner-config 엔드포인트 전체가 즉시 실패합니다.

backend/db/models.py — WorkspaceRunnerConfig.organization_id 컬럼 추가에 대한 데이터 마이그레이션 경로 확인 필요

Important Files Changed

Filename Overview
backend/api/auth.py AuthContext 도입 및 scoped role 파생 로직 추가. 구조 자체는 명확하고 테스트도 충분. 이전 스레드에서 지적된 tautological ensure_organization_access 패턴은 아직 남아있음.
backend/db/models.py WorkspaceRunnerConfig에 NOT NULL organization_id 컬럼 추가 + Organization/OrganizationGroup/ScopedRoleAssignment 신규 모델. create_all 기반 환경에서 기존 테이블 마이그레이션 없이 배포 시 쿼리 실패 위험.
backend/api/runner_config.py organization_id 기반 쿼리로 전환, _get_target_organization_id 헬퍼 추가. config 존재/비존재 시 workspace_id 반환 경로가 다름(로컬 계산 vs DB값).
frontend/src/components/DashboardLayout.tsx 사이드바 콘텐츠 전체를 sidebar-scroll-region div로 래핑하여 독립 스크롤 적용. min-h-0 + flex-1 + overflow-y-auto 조합이 올바르게 사용됨. 테스트 커버리지도 충분.
frontend/src/components/NetworkGraph.tsx ResizeObserver로 viewport 변경 시 fit() 호출 추가. cleanup(disconnect + destroy) 올바름. min-h 반응형 값도 적절히 조정됨.

Reviews (4): Last reviewed commit: "fix(auth): key runner configs by organiz..." | Re-trigger Greptile

Comment thread backend/api/auth.py
Comment thread backend/api/auth.py
Comment thread backend/api/auth.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
backend/api/auth.py (2)

43-51: 💤 Low value

Consider documenting or removing the unused workspace_id parameter.

The workspace_id parameter is accepted but intentionally unused, as the function derives the workspace from organization_id or user_id instead. This prevents clients from overriding workspace boundaries (good security per the PR objectives: "workspace override를 신뢰하지 않고").

However, the unused parameter might confuse future maintainers.

📝 Suggested clarification
 def _derive_workspace_id(
     user_id: str,
     organization_id: str | None,
-    workspace_id: str | None,
+    workspace_id: str | None,  # Accepted but ignored; derived from org/user for security
 ) -> str:
+    """
+    Derive workspace ID from organization or user context.
+    
+    The workspace_id parameter is intentionally unused to prevent client override.
+    """
🤖 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/auth.py` around lines 43 - 51, The parameter workspace_id on
_derive_workspace_id is unused and confusing; either remove it from the
signature and update all callers to stop passing it, or mark it intentionally
ignored by renaming it to _workspace_id and add a one-line docstring/comment on
_derive_workspace_id explaining that workspace_id is intentionally ignored to
prevent workspace override; update any type hints or references to match the new
name so linters won't warn.

29-35: ⚡ Quick win

Document the temporary nature of header-based role derivation.

The current implementation always returns "member" in production (when DEBUG and TRUST_DEV_HEADERS are both false), regardless of the requested role. While the docstring in build_auth_context mentions "future token-derived scope claims from Keycloak or Casdoor," this behavior should be clearly documented to prevent confusion.

📝 Suggested documentation
 def _derive_role(user_id: str, requested_role: str | None) -> RoleName:
+    """
+    Derive user role from request headers (dev/test only) or future JWT claims.
+    
+    In production, always returns 'member' until SSO/OIDC integration is complete.
+    TODO(`#188`): Extract role from Keycloak/Casdoor JWT claims in production.
+    """
     if not (settings.DEBUG or settings.TRUST_DEV_HEADERS):
         return "member"
🤖 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/auth.py` around lines 29 - 35, Update the documentation to
clearly mark header-based role derivation as temporary: in the _derive_role
function and the build_auth_context docstring mention that when settings.DEBUG
and settings.TRUST_DEV_HEADERS are both false the function will always return
"member" (i.e., header/requested_role is ignored), that scoped roles are only
accepted in dev/trust mode (SCOPED_ROLES), and that future behavior will switch
to token-derived scope claims from Keycloak/Casdoor; add a short note explaining
this is a dev-only convenience and should not be relied on in production.
backend/db/models.py (2)

121-122: ⚖️ Poor tradeoff

Clarify the business logic for unscoped role assignments.

Both organization_id and group_id are nullable, which allows three scenarios:

  1. Organization-scoped (org set, group NULL)
  2. Group-scoped (group set, org should also be set for consistency)
  3. Platform-wide (both NULL)

However, it's unclear whether having both NULL is intentionally supported for platform-wide roles (e.g., platform_admin) or if this represents an invalid state. Consider adding:

  • Documentation clarifying when NULL values are valid
  • A check constraint ensuring group assignments reference a valid organization
  • Application-level validation in the auth layer
🤖 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/db/models.py` around lines 121 - 122, The model currently allows
organization_id and group_id to be NULL which permits ambiguous states; update
the RoleAssignment model (the organization_id and group_id mapped_column
definitions) by adding a DB CHECK constraint that enforces if group_id IS NOT
NULL then organization_id IS NOT NULL (and that group_id belongs to the
referenced organization), add a brief docstring/comment on the RoleAssignment
class describing the three valid scopes (org-scoped, group-scoped with org set,
platform-wide) and implement corresponding application-level validation in the
auth layer (e.g., validate_role_scope or in assign_role function) to reject or
normalize invalid combinations before persisting.

108-108: ⚡ Quick win

Verify cascade behavior for organization deletion.

The foreign key to organizations.id does not specify an ondelete action. When an Organization is deleted, the default behavior depends on the database (typically RESTRICT). Consider adding explicit cascade rules to prevent orphaned groups or clarify the intended deletion policy.

🔧 Suggested explicit cascade
-    organization_id: Mapped[str] = mapped_column(ForeignKey("organizations.id"), index=True)
+    organization_id: Mapped[str] = mapped_column(ForeignKey("organizations.id", ondelete="CASCADE"), index=True)
🤖 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/db/models.py` at line 108, The foreign key mapped_column for
organization_id currently lacks an ondelete policy; decide the intended deletion
behavior (typically cascade or restrict) and make it explicit by updating the
ForeignKey on organization_id (e.g., ForeignKey("organizations.id",
ondelete="CASCADE")) and, if using SQLAlchemy relationships, set the
corresponding relationship (e.g., Organization.groups or Group.organization) to
support deletes (for cascade: cascade="all, delete-orphan" on the parent side
and passive_deletes=True on the child side) so that deleting an Organization
produces the expected behavior and avoids orphaned Group records.
🤖 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 `@backend/db/models.py`:
- Around line 115-126: ScopedRoleAssignment allows duplicate entries because
there is no uniqueness enforced on (user_id, role, organization_id, group_id);
add a UniqueConstraint for those four columns to the ScopedRoleAssignment model
(use __table_args__ =
(UniqueConstraint("user_id","role","organization_id","group_id",
name="uq_scoped_role_assignment"),) and import UniqueConstraint from sqlalchemy)
so the DB rejects duplicate role assignments for the same user and scope.

In `@frontend/src/components/DashboardLayout.tsx`:
- Line 186: The decorative SVG icons in DashboardLayout.tsx (the inline <svg>
elements used in the section headers) should be hidden from assistive tech;
update each SVG (the one at line ~186 and the one at line ~196) to include
aria-hidden="true" and focusable="false" so they are not announced or focusable
by screen readers/keyboard navigation.

---

Nitpick comments:
In `@backend/api/auth.py`:
- Around line 43-51: The parameter workspace_id on _derive_workspace_id is
unused and confusing; either remove it from the signature and update all callers
to stop passing it, or mark it intentionally ignored by renaming it to
_workspace_id and add a one-line docstring/comment on _derive_workspace_id
explaining that workspace_id is intentionally ignored to prevent workspace
override; update any type hints or references to match the new name so linters
won't warn.
- Around line 29-35: Update the documentation to clearly mark header-based role
derivation as temporary: in the _derive_role function and the build_auth_context
docstring mention that when settings.DEBUG and settings.TRUST_DEV_HEADERS are
both false the function will always return "member" (i.e., header/requested_role
is ignored), that scoped roles are only accepted in dev/trust mode
(SCOPED_ROLES), and that future behavior will switch to token-derived scope
claims from Keycloak/Casdoor; add a short note explaining this is a dev-only
convenience and should not be relied on in production.

In `@backend/db/models.py`:
- Around line 121-122: The model currently allows organization_id and group_id
to be NULL which permits ambiguous states; update the RoleAssignment model (the
organization_id and group_id mapped_column definitions) by adding a DB CHECK
constraint that enforces if group_id IS NOT NULL then organization_id IS NOT
NULL (and that group_id belongs to the referenced organization), add a brief
docstring/comment on the RoleAssignment class describing the three valid scopes
(org-scoped, group-scoped with org set, platform-wide) and implement
corresponding application-level validation in the auth layer (e.g.,
validate_role_scope or in assign_role function) to reject or normalize invalid
combinations before persisting.
- Line 108: The foreign key mapped_column for organization_id currently lacks an
ondelete policy; decide the intended deletion behavior (typically cascade or
restrict) and make it explicit by updating the ForeignKey on organization_id
(e.g., ForeignKey("organizations.id", ondelete="CASCADE")) and, if using
SQLAlchemy relationships, set the corresponding relationship (e.g.,
Organization.groups or Group.organization) to support deletes (for cascade:
cascade="all, delete-orphan" on the parent side and passive_deletes=True on the
child side) so that deleting an Organization produces the expected behavior and
avoids orphaned Group records.
🪄 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: 258fbb38-1bed-4ffb-8a28-474b1d46dd5c

📥 Commits

Reviewing files that changed from the base of the PR and between 29da69e and a5f764b.

📒 Files selected for processing (16)
  • ARCHITECTURE.md
  • README.md
  • backend/api/auth.py
  • backend/api/llm_providers.py
  • backend/api/runner_config.py
  • backend/api/tenant_config.py
  • backend/db/models.py
  • backend/tests/test_auth_real.py
  • backend/tests/test_llm_providers_api.py
  • backend/tests/test_runner_config_api.py
  • backend/tests/test_tenant_config_api.py
  • docs/plans/2026-05-13-enterprise-rbac-and-responsive-workspace.md
  • frontend/src/components/DashboardLayout.test.tsx
  • frontend/src/components/DashboardLayout.tsx
  • frontend/src/components/NetworkGraph.test.tsx
  • frontend/src/components/NetworkGraph.tsx

Comment thread backend/db/models.py
Comment thread frontend/src/components/DashboardLayout.tsx
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Comments resolved and changes approved.

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for a5f764b19c97fc1f3f766ed94fb92a7af898f24f:

  • 3 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on a5f764b.\n- Missing current-head CodeRabbit/coderabbitai evidence for a5f764b.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 2f241bd97ef703df83be768bbf521841b2a1ab39:

  • 3 unresolved current review thread(s) remain.\n- 2 required check(s) are not successful on 2f241bd.\n- Missing current-head CodeRabbit/coderabbitai evidence for 2f241bd.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 2f241bd97ef703df83be768bbf521841b2a1ab39:

  • 3 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on 2f241bd.\n- Missing current-head CodeRabbit/coderabbitai evidence for 2f241bd.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai approve

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 2f241bd97ef703df83be768bbf521841b2a1ab39:

  • 3 unresolved current review thread(s) remain.\n- 2 required check(s) are not successful on 2f241bd.\n- Missing current-head CodeRabbit/coderabbitai evidence for 2f241bd.\n

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@backend/tests/test_runner_config_api.py`:
- Line 58: Tests currently call app.dependency_overrides.clear(), which wipes
all overrides and risks cross-test coupling; replace those calls with the same
try/finally pattern used elsewhere: set the override for get_db (or other
specific dependency) inside the try block, run assertions, and in finally call
app.dependency_overrides.pop(get_db, None) to remove only that override; apply
this change to all affected places (the fixtures and the
test_org_admin_without_org_scope_is_rejected test) so cleanup happens even on
assertion failures and no global clears are used.
🪄 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: 1204d18d-7bbf-46a0-b260-8b27abf8963e

📥 Commits

Reviewing files that changed from the base of the PR and between a5f764b and 2f241bd.

📒 Files selected for processing (2)
  • backend/api/runner_config.py
  • backend/tests/test_runner_config_api.py

Comment thread backend/tests/test_runner_config_api.py Outdated
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Comments resolved and changes approved.

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for db10303d4da730136f9c2bbf6aef07298690b84f:

  • 1 unresolved current review thread(s) remain.\n- 2 required check(s) are not successful on db10303.\n- Missing current-head CodeRabbit/coderabbitai evidence for db10303.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for db10303d4da730136f9c2bbf6aef07298690b84f:

  • 1 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on db10303.\n- Missing current-head CodeRabbit/coderabbitai evidence for db10303.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai approve

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for db10303d4da730136f9c2bbf6aef07298690b84f:

  • 1 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on db10303.\n- Missing current-head CodeRabbit/coderabbitai evidence for db10303.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for db10303d4da730136f9c2bbf6aef07298690b84f:

  • 1 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on db10303.\n- Missing current-head CodeRabbit/coderabbitai evidence for db10303.\n

Comment thread backend/api/runner_config.py Outdated
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 25649d5bf294938f3e93ebb412f4ca816d6f4192:

  • 2 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on 25649d5.\n- Missing current-head CodeRabbit/coderabbitai evidence for 25649d5.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai approve

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 25649d5bf294938f3e93ebb412f4ca816d6f4192:

  • 2 unresolved current review thread(s) remain.\n- Missing current-head CodeRabbit/coderabbitai evidence for 25649d5.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 25649d5bf294938f3e93ebb412f4ca816d6f4192:

  • 2 unresolved current review thread(s) remain.\n- Missing current-head CodeRabbit/coderabbitai evidence for 25649d5.\n

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Comments resolved and changes approved.

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 25649d5bf294938f3e93ebb412f4ca816d6f4192:

  • 2 unresolved current review thread(s) remain.\n- Missing current-head CodeRabbit/coderabbitai evidence for 25649d5.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai approve 모든 체크가 녹색이고 스레드가 모두 해결되었습니다. 현재 헤드(25649d5)에 대해 공식 GitHub APPROVED 리뷰를 남겨주세요.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
backend/api/runner_config.py (1)

50-52: 💤 Low value

Redundant self-check in ensure_organization_access.

The call ensure_organization_access(auth_context, auth_context.organization_id) compares the auth context's organization against itself, which always passes. Since workspace_id is derived from auth_context.organization_id, authorization is already enforced by construction.

Consider removing this no-op or, if kept for future resource-based checks, add a comment explaining the intent.

♻️ Proposed simplification
     workspace_id = auth_context.workspace_id
-    if auth_context.organization_id:
-        ensure_organization_access(auth_context, auth_context.organization_id)
     result = await db.execute(
🤖 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_config.py` around lines 50 - 52, The call to
ensure_organization_access(auth_context, auth_context.organization_id) is
redundant because auth_context.organization_id is being compared to itself and
workspace_id is already derived from it; remove this no-op call from the block
that sets workspace_id, or if you intend to keep it for future resource-based
validation, replace it with a brief comment explaining its purpose and that it
is intentionally a no-op for now; reference ensure_organization_access and
auth_context when making the change so reviewers can see the de-duplicated
authorization logic.
🤖 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.

Nitpick comments:
In `@backend/api/runner_config.py`:
- Around line 50-52: The call to ensure_organization_access(auth_context,
auth_context.organization_id) is redundant because auth_context.organization_id
is being compared to itself and workspace_id is already derived from it; remove
this no-op call from the block that sets workspace_id, or if you intend to keep
it for future resource-based validation, replace it with a brief comment
explaining its purpose and that it is intentionally a no-op for now; reference
ensure_organization_access and auth_context when making the change so reviewers
can see the de-duplicated authorization logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b362037b-358e-4eef-ab9d-3fe33f9c3f67

📥 Commits

Reviewing files that changed from the base of the PR and between 2f241bd and 25649d5.

📒 Files selected for processing (3)
  • backend/api/auth.py
  • backend/api/runner_config.py
  • backend/tests/test_runner_config_api.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/tests/test_runner_config_api.py

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Comments resolved and changes approved.

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

@seonghobae 모든 체크가 녹색이고 스레드가 모두 해결되었음을 확인했습니다. 현재 헤드(25649d5)에 대해 공식 GitHub APPROVED 리뷰를 제출합니다.

✅ Actions performed

Comments resolved and changes approved.

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 25649d5bf294938f3e93ebb412f4ca816d6f4192:

  • 2 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on 25649d5.\n- Missing current-head CodeRabbit/coderabbitai evidence for 25649d5.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for ba449bc2381c71a284ff7734acacdc04eea4b9c6:

  • 1 unresolved current review thread(s) remain.\n- 2 required check(s) are not successful on ba449bc.\n- Missing current-head CodeRabbit/coderabbitai evidence for ba449bc.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai approve

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for ba449bc2381c71a284ff7734acacdc04eea4b9c6:

  • 1 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on ba449bc.\n- Missing current-head CodeRabbit/coderabbitai evidence for ba449bc.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for ba449bc2381c71a284ff7734acacdc04eea4b9c6:

  • 1 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on ba449bc.\n- Missing current-head CodeRabbit/coderabbitai evidence for ba449bc.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai approve

Comment thread backend/db/models.py
__tablename__ = "workspace_runner_configs"

id: Mapped[int] = mapped_column(primary_key=True)
organization_id: Mapped[str] = mapped_column(String, unique=True, index=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 workspace_runner_configs 기존 테이블에 NOT NULL 컬럼 추가 — 마이그레이션 없음

organization_id: Mapped[str]은 nullable=False(NOT NULL)로 선언됩니다. 프로젝트가 Alembic 없이 bootstrap_db.pyBase.metadata.create_all()만 사용하는데, create_all이미 존재하는 테이블에 컬럼을 추가하지 않습니다. 따라서 이미 workspace_runner_configs 테이블이 생성된 환경(개발 DB, 스테이징 등)에서는 새 컬럼이 생성되지 않아, runner_config.py에서 WHERE workspace_runner_configs.organization_id = :org 쿼리가 실행될 때 column "organization_id" does not exist 오류로 즉시 실패합니다.

bootstrap_db.py 또는 별도 SQL 마이그레이션 스크립트에서 다음을 수행해야 합니다:

  1. ALTER TABLE workspace_runner_configs ADD COLUMN organization_id VARCHAR UNIQUE;
  2. 기존 행의 organization_idworkspace_id에서 역산하여 백필(예: UPDATE ... SET organization_id = replace(workspace_id, 'workspace-', ''))
  3. NOT NULL 제약 적용

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for ba449bc2381c71a284ff7734acacdc04eea4b9c6:

  • 1 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on ba449bc.\n- Missing current-head CodeRabbit/coderabbitai evidence for ba449bc.\n

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
backend/api/runner_config.py (1)

74-80: 💤 Low value

Consider catching a specific exception type instead of bare Exception.

Catching Exception and checking the error message string is fragile—if the message changes or another exception contains similar text, behavior could break silently. The encryption error originates from get_fernet() raising RuntimeError.

♻️ Proposed fix to catch specific exception
-    except Exception as exc:
-        if "ENCRYPTION_KEY is required" not in str(exc):
-            raise
+    except RuntimeError as exc:
+        if "ENCRYPTION_KEY is required" not in str(exc):
+            raise
         raise HTTPException(
             status_code=503,
             detail="Server encryption key is not configured. Contact your workspace administrator.",
         ) from exc
🤖 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_config.py` around lines 74 - 80, Change the broad except
Exception block to catch the specific RuntimeError raised by get_fernet() and
preserve the current behavior: in the except RuntimeError as exc handler, if the
message contains "ENCRYPTION_KEY is required" raise the HTTPException with the
503 detail, otherwise re-raise the RuntimeError; reference get_fernet() and the
existing HTTPException block to locate where to replace the generic except with
the specific RuntimeError handler.
🤖 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.

Nitpick comments:
In `@backend/api/runner_config.py`:
- Around line 74-80: Change the broad except Exception block to catch the
specific RuntimeError raised by get_fernet() and preserve the current behavior:
in the except RuntimeError as exc handler, if the message contains
"ENCRYPTION_KEY is required" raise the HTTPException with the 503 detail,
otherwise re-raise the RuntimeError; reference get_fernet() and the existing
HTTPException block to locate where to replace the generic except with the
specific RuntimeError handler.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 985fed2d-bd6f-4b79-8a16-019ce6ad6abf

📥 Commits

Reviewing files that changed from the base of the PR and between 25649d5 and ba449bc.

📒 Files selected for processing (3)
  • backend/api/runner_config.py
  • backend/db/models.py
  • backend/tests/test_runner_config_api.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/tests/test_runner_config_api.py

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Comments resolved and changes approved.

1 similar comment
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Comments resolved and changes approved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

이 PR은 동일한 변경을 clean replay로 다시 올린 #191에 의해 supersede 되었고, 실제 병합본은 #191입니다. 최신 릴리스 기준은 forthcoming 0.14.x 입니다.

@seonghobae seonghobae closed this May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workspace UI/UX 반응형 보강: 사이드바 스크롤 + DAG 축소 대응 Enterprise RBAC / SSO 준비 기반 구현

1 participant