feat: implement human roles and access control levels#856
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🧰 Additional context used📓 Path-based instructions (2)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (9)📚 Learning: 2026-03-19T07:12:14.508ZApplied to files:
📚 Learning: 2026-03-14T16:18:57.267ZApplied to files:
📚 Learning: 2026-03-17T22:08:13.456ZApplied to files:
📚 Learning: 2026-03-15T16:55:07.730ZApplied to files:
📚 Learning: 2026-03-17T06:43:14.114ZApplied to files:
📚 Learning: 2026-03-16T07:22:28.134ZApplied to files:
📚 Learning: 2026-03-26T21:34:31.847ZApplied to files:
📚 Learning: 2026-03-15T18:38:44.202ZApplied to files:
📚 Learning: 2026-03-14T15:43:05.601ZApplied to files:
🔇 Additional comments (8)
WalkthroughThis PR adds role-based guards and a CEO-only user management API. It introduces a Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a new UserController for CEO-only user management and refactors the API guard system to support more granular role-based access control. Key changes include the introduction of a require_roles guard factory and specific named guards like require_ceo and require_ceo_or_manager, which replace generic write access checks across multiple controllers. Additionally, HumanRole.BOARD_MEMBER has been reclassified from a write role to a read-only role. Feedback was provided regarding the confusing refactoring of guard functions in src/synthorg/api/guards.py, where a function was renamed to an existing name while the original was removed, potentially hindering maintainability.
|
|
||
|
|
||
| def require_system_or_write_access( | ||
| def require_read_access( |
There was a problem hiding this comment.
The function require_system_or_write_access (from the previous version of the file) was renamed to require_read_access in this change, and the original require_read_access function was removed. This repurposing of a function name can be confusing for maintainers, especially since the PR description states require_system_or_write_access was 'removed dead code'. It would be clearer to explicitly remove require_system_or_write_access and retain the original require_read_access (with its updated logic if needed), rather than renaming and repurposing.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/users.py`:
- Around line 156-161: The logger.info calls currently include PII
(user.username and deleted_by.username); remove those username fields and log
only non-PII identifiers instead — keep user.id (and role via user.role.value if
needed) for API_USER_CREATED and replace any deleted_by.username with
deleted_by.id (or deleted_by_id) in the delete-user logging block; update the
logger.info invocations that reference user.username and deleted_by.username to
use the id-only fields (and adjust the structured keys to user_id /
deleted_by_id) so logs no longer contain usernames.
- Around line 200-205: Add a WARNING log entry with contextual info immediately
before raising NotFoundError in get_user, update_user_role, and delete_user:
call the module/class logger (or process logger in this file) to log a warning
that includes the user_id (and operation name, e.g.,
"get_user"/"update_user_role"/"delete_user") and any relevant identifiers, then
raise NotFoundError as before; locate the checks that call
app_state.persistence.users.get(...) in get_user, update_user_role, and
delete_user and insert the logger.warning call just prior to raising the
exception.
In `@tests/unit/api/controllers/test_users.py`:
- Around line 331-345: The current test_delete_ceo_rejected conflates the
"cannot delete CEO" and "cannot delete your own account" checks; add a new test
called test_delete_self_rejected that creates a non-CEO user (use
_create_payload and POST to _BASE with _CEO_HEADERS), then obtain auth headers
for that newly created user (use or add a helper/fixture to generate
user-specific headers), call DELETE f"{_BASE}/{user_id}" with those
user-specific headers, and assert the response is 409 and indicates
self-deletion is rejected to verify the self-deletion branch in the controller
independently of the CEO check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e9a0c8d7-e27a-459a-a661-e9215dfcdb6f
📒 Files selected for processing (14)
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/users.pysrc/synthorg/api/guards.pysrc/synthorg/observability/events/api.pytests/unit/api/controllers/test_approvals.pytests/unit/api/controllers/test_users.pytests/unit/api/test_guards.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Do NOT use 'from future import annotations' - Python 3.14 has PEP 649 native lazy annotations.
Use PEP 758 except syntax: use 'except A, B:' (no parentheses) - ruff enforces this on Python 3.14.
All public functions must have type hints. Code must pass mypy in strict mode.
Docstrings must use Google style format and are required on all public classes and functions - enforced by ruff D rules.
Create new objects for immutability - never mutate existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement. For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves (e.g. agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g. TokenUsage.total_tokens); use NotBlankStr (from core.types) for all identifier/name fields - including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants - instead of manual whitespace validators.
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Maximum line length is 88 characters - enforced by ruff.
Functions must be less than 50 lines; files must be less than 800 lines.
Always handle errors explicitly - never silently swallow exceptions.
Validate input at system boundaries (user input, external APIs, config files).
Every module...
Files:
src/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/controllers/settings.pysrc/synthorg/observability/events/api.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/guards.pysrc/synthorg/api/controllers/users.py
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use 'cd' in Bash commands - the working directory is already set to the project root. Use absolute paths or run commands directly. Do NOT prefix commands with 'cd C:/Users/Aurelio/synthorg &&'.
Files:
src/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/backup.pytests/unit/api/controllers/test_approvals.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/controllers/settings.pytests/unit/api/test_guards.pysrc/synthorg/observability/events/api.pysrc/synthorg/api/controllers/setup.pytests/unit/api/controllers/test_users.pysrc/synthorg/api/guards.pysrc/synthorg/api/controllers/users.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark all tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowmarkers.
Maintain 80% minimum code coverage - enforced in CI.
asyncio_mode = "auto" is configured - no manual@pytest.mark.asyncioneeded. Global timeout is 30 seconds per test in pyproject.toml - do not add per-file pytest.mark.timeout(30) markers; non-default overrides like timeout(60) are allowed.
Always run pytest with '-n auto' for parallelism via pytest-xdist - never run tests sequentially.
Prefer@pytest.mark.parametrizefor testing similar cases.
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases. Tests must use test-provider, test-small-001, etc.
For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins. For tasks that must block indefinitely until cancelled, use asyncio.Event().wait() instead of asyncio.sleep(large_number) - it is cancellation-safe and carries no timing assumptions. NEVER skip, dismiss, or ignore flaky tests - always fix them fully and fundamentally.
Use Hypothesis for property-based testing with@given+@settings. Hypothesis profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var.
Files:
tests/unit/api/controllers/test_approvals.pytests/unit/api/test_guards.pytests/unit/api/controllers/test_users.py
🧠 Learnings (24)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/controllers/settings.pysrc/synthorg/observability/events/api.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/users.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/api/**/*.py : Authentication uses JWT + API key. Approval gate integration for high-risk operations.
Applied to files:
src/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/guards.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/backup/**/*.py : Backup package (backup/): scheduled/manual/lifecycle backups of persistence DB, agent memory, company config. BackupService orchestrator, BackupScheduler (periodic asyncio task), RetentionManager (count + age pruning), tar.gz compression, SHA-256 checksums, manifest tracking, validated restore with atomic rollback and safety backup. handlers/ subpackage: ComponentHandler protocol + concrete handlers (PersistenceComponentHandler, MemoryComponentHandler, ConfigComponentHandler)
Applied to files:
src/synthorg/api/controllers/backup.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/api/**/*.py : REST API: Litestar framework, controllers with guards, channels for WebSocket, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint. RFC 9457 structured errors (ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation).
Applied to files:
src/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST + WebSocket API. Controllers, guards, channels, JWT + API key + WS ticket auth, RFC 9457 structured errors.
Applied to files:
src/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/api/controllers/providers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings package (settings/): runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge (JSON serialization for Pydantic/collections), ConfigResolver (typed accessors), validation, registry, change notifications via message bus, SettingsSubscriber protocol, SettingsChangeDispatcher (polls `#settings` channel, routes to subscribers, restart_required filtering)
Applied to files:
src/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings use runtime-editable persistence with precedence: DB > env > YAML > code defaults. 8 namespaces with Fernet encryption for sensitive values.
Applied to files:
src/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-26T19:11:08.600Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T19:11:08.600Z
Learning: Applies to src/**/*.py : Always use constants from domain-specific modules under synthorg.observability.events for event names (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly: 'from synthorg.observability.events.<domain> import EVENT_CONSTANT'.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
src/synthorg/api/guards.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/security/**/*.py : Security module includes SecOps agent, rule engine (soft-allow/hard-deny), audit log, output scanner, risk classifier, autonomy levels (4 strategies), timeout policies.
Applied to files:
src/synthorg/api/guards.py
🔇 Additional comments (20)
src/synthorg/observability/events/api.py (1)
74-77: LGTM!The new user lifecycle event constants follow the established
api.<resource>.<action>naming convention and are properly typed withFinal[str], consistent with the existing event constants in this module.src/synthorg/api/guards.py (3)
40-51: LGTM!The role set definitions correctly implement the PR objectives:
BOARD_MEMBERis removed from_WRITE_ROLES(so board members cannot perform write operations) and added to_READ_ROLESalongsideOBSERVER. The use offrozensetensures immutability.
131-159: LGTM!The
require_rolesfactory is well-designed:
- Converts roles to a
frozensetfor efficient membership checks- Generates a sorted label for deterministic, readable log output
- The inner guard follows the established pattern with proper structured logging using
API_GUARD_DENIED
162-175: LGTM!The pre-built guard constants align with the design spec:
require_ceofor CEO-only operations (setup, user management)require_ceo_or_managerfor administrative operations (settings, providers)require_approval_rolescorrectly includesBOARD_MEMBERfor approve/reject actions, fulfilling the "observe + major approvals only" requirementsrc/synthorg/api/controllers/backup.py (1)
51-53: LGTM!The controller-level guard correctly restricts backup operations to CEO and SYSTEM roles. This allows both human administrators and CLI-based automation (
synthorg backup/synthorg wipe) while appropriately excluding other roles from sensitive backup/restore operations.src/synthorg/api/controllers/collaboration.py (1)
212-212: LGTM!The collaboration override endpoints appropriately use
require_ceo_or_managerfor write operations (set/clear override) while maintainingrequire_read_accessfor read operations. This aligns with the PR's role-based access control strategy.Also applies to: 277-281
src/synthorg/api/controllers/settings.py (1)
159-162: LGTM!The settings controller correctly applies
require_ceo_or_managerguards to mutation endpoints (PUT/DELETE) while inheritingrequire_read_accessfrom the controller-level guard for read operations. This follows the established pattern of least-privilege access.Also applies to: 199-203
src/synthorg/api/controllers/approvals.py (1)
622-626: LGTM!The approve/reject endpoints now use
require_approval_roles, which correctly includes board members alongside CEO and managers. Combined withcreate_approvalretainingrequire_write_access(which excludes board members), this implements the design spec requirement that board members can "observe + approve/reject" but cannot initiate new approvals.Also applies to: 678-682
src/synthorg/api/controllers/providers.py (1)
80-83: LGTM!All provider management write endpoints consistently use
require_ceo_or_manager, including CRUD operations, connection testing, model discovery, and discovery policy management. Read endpoints appropriately retainrequire_read_access. This provides a clear separation between read and write access levels.Also applies to: 249-252, 291-294, 335-338, 381-384, 411-414, 460-463, 525-528, 554-557
src/synthorg/api/controllers/__init__.py (1)
26-26: LGTM!The
UserControlleris properly registered: imported from the newusersmodule, added toALL_CONTROLLERSfor automatic route registration, and exported via__all__. This follows the established pattern for all controllers in this package.Also applies to: 51-51, 78-78
src/synthorg/api/controllers/autonomy.py (1)
8-8: LGTM! Guard reclassification aligns with role-based access control design.The import and guard change from
require_write_accesstorequire_ceo_or_managercorrectly restricts autonomy level modifications to CEO and Manager roles, as specified in the PR objectives. This is properly covered by parameterized tests intests/unit/api/test_guards.py(lines 162-185).Also applies to: 83-83
tests/unit/api/controllers/test_approvals.py (1)
612-654: LGTM! Comprehensive test coverage for board member approval access.The new
TestBoardMemberApprovalAccessclass properly validates the role reclassification where board members can approve/reject (viarequire_approval_roles) but cannot create approvals. The tests correctly:
- Verify 200 status for approve/reject operations
- Verify 403 status for create operation
- Assert response payload fields (
decided_by,status)src/synthorg/api/controllers/setup.py (1)
46-46: LGTM! Consistent CEO-only guard enforcement across setup write endpoints.The guard reclassification from
require_write_accesstorequire_ceois consistently applied to all setup write endpoints:
POST /company(line 217)POST /agent(line 297)PUT /agents/{index}/model(line 389)PUT /agents/{index}/name(line 456)POST /agents/{index}/randomize-name(line 514)PUT /name-locales(line 635)POST /complete(line 684)Read endpoints appropriately retain
require_read_access. This is verified by tests intests/unit/api/test_guards.py(lines 131-141).Also applies to: 217-217, 297-297, 389-389, 456-456, 514-514, 635-635, 684-684
tests/unit/api/test_guards.py (3)
23-31: Good refactor: DRY improvement with shared payload constant.Extracting the task payload into
_TASK_PAYLOADeliminates duplication across multiple test methods and improves maintainability.
52-58: LGTM! Board member role reclassification correctly tested.The test updates properly reflect the design change where board members are removed from write roles:
test_blocks_board_membernow expects 403 (was previously allowing creation)test_allows_board_memberconfirms read access remains availableThis aligns with the PR objective: "board members can only observe and perform approve/reject actions."
Also applies to: 101-106
127-185: LGTM! Comprehensive role-guard test coverage.The
TestRequireRolesclass provides excellent coverage:
- Validates CEO-only setup endpoint (manager blocked)
- Confirms board member read-only behavior
- Parameterized test covers all five roles against
require_ceo_or_managerguardThe parameterized approach is well-suited for testing role combinations systematically.
tests/unit/api/controllers/test_users.py (2)
1-27: LGTM! Well-structured test module setup.The module setup is clean with:
_SYSTEM_USER_IDcorrectly derived using the same UUID5 pattern asconftest._seed_test_users- DRY
_create_payloadhelper for consistent test data- Clear constant definitions for base URL and CEO headers
30-146: LGTM! Comprehensive user creation test coverage.The
TestCreateUserclass thoroughly covers:
- Successful creation for all assignable roles (manager, board_member, pair_programmer, observer)
- Rejection cases: second CEO (409), system role (422), duplicate username (409), short password (422)
- Authorization: parameterized test confirms non-CEO roles are blocked (403)
src/synthorg/api/controllers/users.py (2)
40-79: LGTM! Well-designed DTOs following Pydantic v2 conventions.The request/response models correctly use:
ConfigDict(frozen=True)for immutabilityNotBlankStrfor identifier fieldsField(max_length=...)for input validation- Clean separation between request DTOs and public response (omitting
password_hash)
85-323: LGTM! Solid implementation of CEO-only user management.The
UserControllercorrectly implements:
- CEO-only access via controller-level guard (line 93)
- Business rules: single CEO, no SYSTEM role assignment, password validation, username uniqueness
- Immutable state updates via
model_copy(line 264)- Self-deletion prevention (line 312-314)
- Proper use of observability event constants
The role validation logic for CEO demotion/promotion (lines 246-261) correctly handles edge cases.
| def test_delete_ceo_rejected( | ||
| self, | ||
| test_client: TestClient[Any], | ||
| ) -> None: | ||
| # The seeded CEO user -- find its ID from list | ||
| list_resp = test_client.get(_BASE, headers=_CEO_HEADERS) | ||
| ceo_users = [u for u in list_resp.json()["data"] if u["role"] == "ceo"] | ||
| assert len(ceo_users) > 0 | ||
| ceo_id = ceo_users[0]["id"] | ||
|
|
||
| resp = test_client.delete( | ||
| f"{_BASE}/{ceo_id}", | ||
| headers=_CEO_HEADERS, | ||
| ) | ||
| assert resp.status_code == 409 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add explicit test for self-deletion scenario.
The controller has a distinct check preventing users from deleting their own account (line 312-314 in users.py). The current test_delete_ceo_rejected test conflates two different error conditions—it deletes the CEO user while authenticated as the CEO, so the 409 could be from either:
- "Cannot delete the CEO user" (line 309-310)
- "Cannot delete your own account" (line 313-314)
Consider adding an explicit test that creates a non-CEO user, authenticates as that user, and attempts self-deletion to verify the self-deletion check independently.
🧪 Proposed additional test
def test_delete_self_rejected(
self,
test_client: TestClient[Any],
) -> None:
# Create a manager user
create_resp = test_client.post(
_BASE,
json=_create_payload(username="self-delete-test"),
headers=_CEO_HEADERS,
)
user_id = create_resp.json()["data"]["id"]
# Attempt to delete self (would require authenticating as the new user)
# Note: This requires test infrastructure to generate auth headers for
# the newly created user, which may need fixture updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/controllers/test_users.py` around lines 331 - 345, The current
test_delete_ceo_rejected conflates the "cannot delete CEO" and "cannot delete
your own account" checks; add a new test called test_delete_self_rejected that
creates a non-CEO user (use _create_payload and POST to _BASE with
_CEO_HEADERS), then obtain auth headers for that newly created user (use or add
a helper/fixture to generate user-specific headers), call DELETE
f"{_BASE}/{user_id}" with those user-specific headers, and assert the response
is 409 and indicates self-deletion is rejected to verify the self-deletion
branch in the controller independently of the CEO check.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #856 +/- ##
==========================================
- Coverage 92.33% 92.31% -0.03%
==========================================
Files 584 585 +1
Lines 30576 30745 +169
Branches 2941 2958 +17
==========================================
+ Hits 28233 28381 +148
- Misses 1854 1870 +16
- Partials 489 494 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add fine-grained per-role access control matching the design spec: - Add require_roles() guard factory for arbitrary role sets with named constants (require_ceo, require_ceo_or_manager, require_approval_roles) - Remove BOARD_MEMBER from _WRITE_ROLES -- board members can now only observe and approve/reject, not perform general writes - Reclassify guards on 7 controllers: approvals (approve/reject use require_approval_roles), backup (CEO + SYSTEM only), settings/providers/autonomy/collaboration (CEO or Manager), setup (CEO only) - Add UserController with CEO-only CRUD for user management (create, list, get, update role, delete) with business rules (single CEO, no SYSTEM role assignment, password validation) - Add user management observability events Closes #257 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove dead code: _SYSTEM_WRITE_ROLES and require_system_or_write_access (no consumers after backup controller migration) - Restore inadvertently deleted urgency_level assertion in test_expired_approval_shows_zero_seconds - Replace empty test_ceo_only_allows_ceo_on_setup with test_setup_blocks_manager (verifies require_ceo guard) - Add missing tests: CEO demotion prevention, promote-to-second-CEO via update, password_hash leak assertion, system user ID fix - Update require_write_access docstring to reference require_roles() Pre-reviewed by 5 agents, 10 findings addressed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…iewers Guards: add empty-roles validation, set __name__ on closures, fix require_read_access docstring to clarify SYSTEM exclusion. UserController: add asyncio.Lock for CEO uniqueness TOCTOU, move password validation before DB reads, extract validation helpers (functions now under 50 lines), add logger.warning on all error paths, check delete() return value, remove PII from logs, reorder delete checks so self-deletion is testable, use tuple instead of list return. Frontend: remove board_member from WRITE_ROLES (matches backend), add board_member exclusion assertion in constants test. Tests: parametrize role creation tests, tighten list assertion, assert first POST in duplicate test, add system user update test, add backup controller HTTP-level guard tests. Docs: add /api/v1/users to API Surface table, update Board Member and CEO role descriptions, fix escalation chain role value, update CLAUDE.md package structure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1924353 to
ca64cf1
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
tests/unit/api/controllers/test_users.py (1)
330-345:⚠️ Potential issue | 🟡 MinorAdd a non-CEO self-deletion test to isolate the self-delete guard path.
This still validates
409for CEO self-delete only. A dedicated non-CEO self-delete test would confirm the self-deletion branch independently from CEO-specific protections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/api/controllers/test_users.py` around lines 330 - 345, Add a new unit test that mirrors test_delete_ceo_self_rejected but uses a non-CEO authenticated user to exercise the self-deletion guard independently: create or locate a non-CEO user by calling test_client.get(_BASE, headers=_USER_HEADERS or by creating a test user), pick that user’s id and call test_client.delete(f"{_BASE}/{user_id}", headers=_USER_HEADERS) where the headers represent the same authenticated non-CEO user, and assert the response status_code is 409; name the new test something like test_delete_non_ceo_self_rejected to differentiate it from test_delete_ceo_self_rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/users.py`:
- Around line 40-41: The module-level asyncio.Lock (_CEO_LOCK) only serializes
within a single process and does not prevent duplicate CEO creation across
multiple workers; update the code and DB to enforce uniqueness: add a DB
migration that creates a unique partial index on users(role) WHERE role = 'ceo'
(e.g., CREATE UNIQUE INDEX idx_ceo_role ON users(role) WHERE role = 'ceo'); keep
or remove the _CEO_LOCK as desired but ensure the create flow that calls
count_by_role() and save() catches unique-constraint/IntegrityError from the
database insert and maps it to a user-friendly response (e.g., raise a
409/validation error) so concurrent inserts from different workers are safely
handled; reference functions/classes: _CEO_LOCK, count_by_role, save, and the
users table/index in your migration.
In `@tests/unit/api/controllers/test_backup.py`:
- Around line 347-357: The test currently verifies CEO access in
test_ceo_can_access but misses asserting that the SYSTEM role is allowed; update
the test to also call test_client.get("/api/v1/admin/backups",
headers=make_auth_headers("system")) and assert resp.status_code == 200 (reuse
the same pattern as the CEO check), and apply the same addition to the
subsequent similar test block (lines referenced around the second block) so
BackupController's explicit SYSTEM allow rule is covered; look for usages of
test_ceo_can_access, make_auth_headers, and the "/api/v1/admin/backups" request
to locate where to insert the extra assertion.
In `@tests/unit/api/controllers/test_users.py`:
- Around line 143-148: The list/get/update responses currently lack assertions
that the sensitive field password_hash is absent; update the tests in
test_users.py to assert that no returned user object contains the
"password_hash" key: for the list response (where you call
test_client.get(_BASE, headers=_CEO_HEADERS)) iterate over body["data"] and
assert "password_hash" not in each item, for the single-get response assert
"password_hash" not in body["data"], and for the update response assert
"password_hash" not in the returned body (match whichever variable holds the
response JSON); reuse the existing resp/body variables and keep assertions
consistent with the create-response check already present.
- Around line 21-27: The test helper currently mutates the defaults dict
in-place via defaults.update(overrides); instead return a new dict instead (e.g.
new_payload = {**defaults, **overrides} or use copy.deepcopy(defaults) then
update the copy) and return that new object; for stricter immutability wrap the
returned mapping in MappingProxyType if needed. Ensure you change the code that
references the local defaults and overrides variables so no in-place update is
performed.
- Around line 30-33: Add Google-style docstrings to the public test class
TestCreateUser and its public test methods (e.g., test_create_manager) as well
as the other public test classes/methods noted (lines ~137-139, 161-163,
193-195, 287-289); for each class provide a short triple-quoted description
under the class definition and for each test method add a one-line triple-quoted
description explaining what the test asserts, following Google-style conventions
(brief summary, optional Args/Returns sections if needed) so ruff D rules pass.
---
Duplicate comments:
In `@tests/unit/api/controllers/test_users.py`:
- Around line 330-345: Add a new unit test that mirrors
test_delete_ceo_self_rejected but uses a non-CEO authenticated user to exercise
the self-deletion guard independently: create or locate a non-CEO user by
calling test_client.get(_BASE, headers=_USER_HEADERS or by creating a test
user), pick that user’s id and call test_client.delete(f"{_BASE}/{user_id}",
headers=_USER_HEADERS) where the headers represent the same authenticated
non-CEO user, and assert the response status_code is 409; name the new test
something like test_delete_non_ceo_self_rejected to differentiate it from
test_delete_ceo_self_rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7e7c04a-2351-4894-bf42-d9d412f272a0
📒 Files selected for processing (19)
CLAUDE.mddocs/design/operations.mdsrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/users.pysrc/synthorg/api/guards.pysrc/synthorg/observability/events/api.pytests/unit/api/controllers/test_approvals.pytests/unit/api/controllers/test_backup.pytests/unit/api/controllers/test_users.pytests/unit/api/test_guards.pyweb/src/__tests__/utils/constants.test.tsweb/src/utils/constants.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax:except A, B:(no parentheses) instead ofexcept (A, B):-- ruff enforces this on Python 3.14
All public functions and classes must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones; for non-Pydantic internal collections usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) over barecreate_taskfor structured concurrency
Keep functions under 50 lines and files under 800 lines
Line length must be 88 characters as enforced by ruff
Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors
Files:
src/synthorg/api/controllers/__init__.pytests/unit/api/controllers/test_backup.pysrc/synthorg/api/controllers/settings.pytests/unit/api/controllers/test_approvals.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/observability/events/api.pysrc/synthorg/api/controllers/providers.pytests/unit/api/test_guards.pysrc/synthorg/api/controllers/collaboration.pytests/unit/api/controllers/test_users.pysrc/synthorg/api/guards.pysrc/synthorg/api/controllers/users.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and use logger:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the structured logger fromsynthorg.observability
Always use the logger variable namelogger(not_logger, notlog)
Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTEDfromevents.api); import directly and always use structured kwargs:logger.info(EVENT, key=value)never string formatting
All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (withmodel_copy(update=...)) for runtime state that evolves
Use Pydantic v2 with@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStr(fromcore.types) for all identifier/name fields instead of manual validators
All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically; never implement retry logic in driver subclasses or calling code
Validate at system boundaries: user input, external APIs, config files; handle errors explicitly, never silently swallow
Files:
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/observability/events/api.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/guards.pysrc/synthorg/api/controllers/users.py
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript 6.0+ syntax; remove
baseUrl(deprecated in TS 6), usemoduleResolution: 'bundler'or'nodenext', list required types intypesarray
Files:
web/src/utils/constants.tsweb/src/__tests__/utils/constants.test.ts
web/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Import
cnfrom@/lib/utilsfor conditional class merging in TypeScript utility code
Files:
web/src/utils/constants.tsweb/src/__tests__/utils/constants.test.ts
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowto categorize tests
Maintain 80% minimum code coverage as enforced in CI
Useasyncio_mode = 'auto'for async tests; no manual@pytest.mark.asyncioneeded
Use@pytest.mark.parametrizefor testing similar cases instead of test duplication
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, tests, or config examples; use generic names:example-provider,example-large-001,test-provider,test-small-001,large/medium/small
Use Hypothesis with@given+@settingsfor property-based testing; avoid flaky timing-based tests by mockingtime.monotonic()andasyncio.sleep()or usingasyncio.Event().wait()for indefinite blocks
Files:
tests/unit/api/controllers/test_backup.pytests/unit/api/controllers/test_approvals.pytests/unit/api/test_guards.pytests/unit/api/controllers/test_users.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS read the relevant design specification page in
docs/design/before implementing any feature or planning any issue; design spec is the starting point for architecture, data models, and behavior; alert user and explain if implementation deviates from spec; update the spec page when deviations are approved
Files:
docs/design/operations.md
🧠 Learnings (38)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/__init__.pyCLAUDE.mdsrc/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/approvals.pydocs/design/operations.mdsrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/controllers/users.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
CLAUDE.mddocs/design/operations.mdsrc/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/api/**/*.py : REST API: Litestar framework, controllers with guards, channels for WebSocket, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint. RFC 9457 structured errors (ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation).
Applied to files:
CLAUDE.mdsrc/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/collaboration.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST + WebSocket API. Controllers, guards, channels, JWT + API key + WS ticket auth, RFC 9457 structured errors.
Applied to files:
CLAUDE.mdsrc/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/setup.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
CLAUDE.mdsrc/synthorg/api/guards.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/security/**/*.py : Security module includes SecOps agent, rule engine (soft-allow/hard-deny), audit log, output scanner, risk classifier, autonomy levels (4 strategies), timeout policies.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
CLAUDE.mdsrc/synthorg/api/controllers/providers.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings package (settings/): runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge (JSON serialization for Pydantic/collections), ConfigResolver (typed accessors), validation, registry, change notifications via message bus, SettingsSubscriber protocol, SettingsChangeDispatcher (polls `#settings` channel, routes to subscribers, restart_required filtering)
Applied to files:
src/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings use runtime-editable persistence with precedence: DB > env > YAML > code defaults. 8 namespaces with Fernet encryption for sensitive values.
Applied to files:
src/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/autonomy.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/autonomy.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/autonomy.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/autonomy.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (with `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/api/controllers/settings.pysrc/synthorg/api/controllers/autonomy.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/api/**/*.py : Authentication uses JWT + API key. Approval gate integration for high-risk operations.
Applied to files:
src/synthorg/api/controllers/approvals.pysrc/synthorg/api/controllers/backup.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/controllers/collaboration.pysrc/synthorg/api/guards.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Settings: Runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge, ConfigResolver (typed composed reads for controllers), validation, registry, change notifications via message bus. Per-namespace setting definitions in definitions/ submodule (api, company, providers, memory, budget, security, coordination, observability, backup).
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/backup/**/*.py : Backup package (backup/): scheduled/manual/lifecycle backups of persistence DB, agent memory, company config. BackupService orchestrator, BackupScheduler (periodic asyncio task), RetentionManager (count + age pruning), tar.gz compression, SHA-256 checksums, manifest tracking, validated restore with atomic rollback and safety backup. handlers/ subpackage: ComponentHandler protocol + concrete handlers (PersistenceComponentHandler, MemoryComponentHandler, ConfigComponentHandler)
Applied to files:
src/synthorg/api/controllers/backup.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and always use structured kwargs: `logger.info(EVENT, key=value)` never string formatting
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/api/controllers/users.py
🔇 Additional comments (25)
web/src/utils/constants.ts (1)
53-53: WRITE_ROLES update correctly enforces policy.Line 53 aligns frontend write capability with the updated RBAC model by excluding
board_memberfrom write-capable roles.web/src/__tests__/utils/constants.test.ts (1)
49-53: Test coverage improvement is solid.These assertions directly lock in the intended
WRITE_ROLEScontract and provide a clear regression check forboard_memberwrite access.src/synthorg/observability/events/api.py (1)
74-77: LGTM!The new user lifecycle event constants follow the established
api.<noun>.<verb>naming convention and are consistent with existing patterns in the file (e.g.,API_APPROVAL_CREATED,API_TASK_DELETED).docs/design/operations.md (3)
998-1000: LGTM!The escalation chain role change from
ceo_or_boardtoceocorrectly aligns with the PR's removal of Board Member from write roles. Board members now have "read-only + approve/reject" permissions, which matches therequire_approval_rolesguard allowing them to approve/reject but not to be escalation targets for general operations.
1101-1101: LGTM!The new
/api/v1/usersendpoint documentation correctly describes CEO-only access and CRUD operations, matching theUserControllerimplementation guarded byrequire_ceo.
1186-1187: LGTM!The updated human role descriptions accurately reflect the implementation:
- Board Member: "Read-only + approve/reject" matches inclusion in
_READ_ROLESandrequire_approval_rolesbut exclusion from_WRITE_ROLES.- CEO: "Sole authority to create, modify, and delete user accounts" matches the
require_ceoguard onUserController.src/synthorg/api/guards.py (3)
40-51: LGTM!The role set updates correctly implement the PR objective:
- Board Member removed from
_WRITE_ROLES(can only observe + approve)- Board Member added to
_READ_ROLESalongside ObserverThe frozenset usage ensures immutability.
133-170: LGTM!The
require_rolesfactory is well-designed:
- Input validation rejects empty role sets
- Uses
frozensetfor O(1) membership checks- Sorted label ensures deterministic guard naming for logs
- Sets
__name__/__qualname__for debuggability
175-186: LGTM!The pre-built guard constants cover the three permission tiers needed by the PR:
require_ceo: CEO-only (user management, setup)require_ceo_or_manager: CEO or Manager (settings, autonomy, collaboration)require_approval_roles: CEO, Manager, or Board Member (approve/reject actions)src/synthorg/api/controllers/users.py (5)
47-76: LGTM!The DTOs are well-designed:
- Frozen
ConfigDictfor immutabilityNotBlankStrfor identifier fields- Password hash correctly omitted from
UserResponseAwareDatetimeensures timezone-aware timestamps
93-161: LGTM!All validation helpers follow the coding guidelines:
- Each error path logs at WARNING with context before raising
- Uses appropriate event constants (
API_VALIDATION_FAILED,API_RESOURCE_CONFLICT,API_RESOURCE_NOT_FOUND)- Structured kwargs in all logger calls
177-233: LGTM!The
create_userimplementation is solid:
- Validates role and password before acquiring lock
- Lock scope correctly covers CEO check, username check, and save
- Password hashing uses the async service method
- State transition logged at INFO level
- Returns wrapped response with
_to_responseto omit password hash
277-326: LGTM!The
update_user_roleimplementation correctly:
- Validates assignable role before fetching user
- Blocks modification of SYSTEM user
- Acquires lock for CEO-related role change validation
- Uses
model_copy(update=...)for immutable update pattern- Logs role transition at INFO level with old and new values
328-377: LGTM!The
delete_userimplementation has proper safeguards:
- Prevents self-deletion
- Blocks deletion of SYSTEM and CEO users
- Logs deletion with actor context (deleted_by_user_id)
- All error paths log before raising
CLAUDE.md (1)
106-106: LGTM!The package structure documentation correctly reflects the new capabilities added by this PR: role-based access control guards and user management API.
src/synthorg/api/controllers/__init__.py (1)
26-26: LGTM!
UserControlleris correctly:
- Imported in alphabetical order
- Added to
ALL_CONTROLLERStuple for routing- Exported in
__all__for public APIAlso applies to: 51-51, 78-78
src/synthorg/api/controllers/autonomy.py (1)
8-8: LGTM!The guard change from
require_write_accesstorequire_ceo_or_managercorrectly restricts autonomy level changes to CEO and Manager roles, aligning with the PR's access control refinements. The read endpoint appropriately retains the less restrictiverequire_read_accessguard.Also applies to: 83-83
src/synthorg/api/controllers/settings.py (1)
14-14: LGTM!The guard changes correctly tier access control:
- Controller-level
require_read_accessfor GET operations (schema, list, get)- Route-level
require_ceo_or_managerfor mutating operations (PUT, DELETE)This ensures read-only roles (Observer, Board Member) can view settings but cannot modify them.
Also applies to: 159-161, 199-202
src/synthorg/api/controllers/setup.py (1)
46-46: Guard migration is consistent and correctly scoped.Switching mutating setup routes to
require_ceowhile keeping read routes on
require_read_accessmatches the intended role split.Also applies to: 217-217, 297-297, 389-389, 456-456, 514-515, 635-636, 684-685
src/synthorg/api/controllers/approvals.py (1)
29-33: Approve/reject guard update is correct.Using
require_approval_roleson decision endpoints is the right access boundary
for the updated RBAC policy.Also applies to: 624-625, 680-681
src/synthorg/api/controllers/collaboration.py (1)
18-18: Override mutation guards are correctly tightened.
set_overrideandclear_overridenow consistently enforce CEO/Manager access.Also applies to: 212-213, 277-281
src/synthorg/api/controllers/backup.py (1)
3-4: Class-level backup guard change looks correct.Using
require_roles(HumanRole.CEO, HumanRole.SYSTEM)makes the allowed admin
surface explicit and matches the updated policy.Also applies to: 17-17, 47-48, 53-53
src/synthorg/api/controllers/providers.py (1)
31-31: Provider write-route guard migration is consistent.All mutating provider/discovery-policy routes now use the same
require_ceo_or_managerpolicy with no read-route regressions.Also applies to: 82-83, 251-252, 293-294, 337-338, 383-384, 413-414, 462-463, 527-528, 556-557
tests/unit/api/controllers/test_approvals.py (1)
612-655: Board-member approval access coverage is solid.These tests correctly capture the new behavior: approve/reject allowed, create
still forbidden.tests/unit/api/test_guards.py (1)
23-31: Guard test updates are well-structured and aligned with RBAC changes.The shared payload + new
TestRequireRolesmatrix gives clear, maintainable
coverage for the updated role boundaries.Also applies to: 52-59, 101-107, 127-185
| def test_ceo_can_access( | ||
| self, | ||
| test_client: TestClient[Any], | ||
| ) -> None: | ||
| resp = test_client.get( | ||
| "/api/v1/admin/backups", | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| # 200 = guard passed (may return empty list) | ||
| assert resp.status_code == 200 | ||
|
|
There was a problem hiding this comment.
Add positive guard coverage for the SYSTEM role.
BackupController explicitly allows SYSTEM, but this test block only verifies
CEO success plus non-admin denials. Add a system success assertion to prevent
regressions in CLI/internal access.
🧪 Suggested test adjustment
`@pytest.mark.unit`
class TestBackupGuards:
"""HTTP-level guard tests for backup controller access control."""
- def test_ceo_can_access(
+ `@pytest.mark.parametrize`("role", ["ceo", "system"])
+ def test_admin_roles_can_access(
self,
test_client: TestClient[Any],
+ role: str,
) -> None:
resp = test_client.get(
"/api/v1/admin/backups",
- headers=make_auth_headers("ceo"),
+ headers=make_auth_headers(role),
)
- # 200 = guard passed (may return empty list)
assert resp.status_code == 200Also applies to: 358-372
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/controllers/test_backup.py` around lines 347 - 357, The test
currently verifies CEO access in test_ceo_can_access but misses asserting that
the SYSTEM role is allowed; update the test to also call
test_client.get("/api/v1/admin/backups", headers=make_auth_headers("system"))
and assert resp.status_code == 200 (reuse the same pattern as the CEO check),
and apply the same addition to the subsequent similar test block (lines
referenced around the second block) so BackupController's explicit SYSTEM allow
rule is covered; look for usages of test_ceo_can_access, make_auth_headers, and
the "/api/v1/admin/backups" request to locate where to insert the extra
assertion.
- Add partial unique index on users(role) WHERE role='ceo' for DB-level
CEO uniqueness enforcement across multiple workers
- Catch QueryError on CEO save paths and convert to 409 ConflictError
- Add password_hash absence assertions on list, get, and update responses
- Use {**defaults, **overrides} instead of in-place dict mutation
- Add Google-style docstrings to all test classes
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Change _make_user default role from CEO to MANAGER and update count_by_role test to use MANAGER for multi-user assertions, since the new partial unique index enforces at most one CEO. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/users.py`:
- Around line 324-329: The except QueryError block in the update flow (around
app_state.persistence.users.save(updated)) incorrectly assumes every QueryError
is a CEO-constraint violation; change the handler in the save/update path to
inspect the caught QueryError (or its message/code) and determine the real
cause, then log the actual error details with
logger.warning(API_RESOURCE_CONFLICT, reason=...) and raise a ConflictError with
a clear, appropriate message (or a generic conflict message that includes the
underlying error text) instead of always "A CEO user already exists"; reference
the save call, the QueryError exception, and the existing logger.warning and
ConflictError symbols when making the change.
- Around line 227-232: The current except QueryError handler around await
app_state.persistence.users.save(user) always returns "A CEO user already
exists" which can be incorrect when the QueryError is due to a username UNIQUE
violation; modify the except block to inspect the caught QueryError (e.g.,
examine its .constraint/.detail/.args or error message) and branch: if it
indicates the username UNIQUE constraint was violated, set a username-specific
conflict message and raise ConflictError(username_msg), otherwise if it
indicates the CEO constraint use the existing CEO message, and fall back to a
generic conflict message; ensure you still log the chosen reason with
logger.warning(API_RESOURCE_CONFLICT, reason=msg) before raising.
In `@tests/unit/api/controllers/test_users.py`:
- Around line 344-359: Add a new unit test alongside
test_delete_ceo_self_rejected that verifies the self-deletion guard for a
non-CEO user: create a non-CEO user (via test_client POST to _BASE or existing
user factory), obtain authentication headers for that new user (use the
project’s auth helper or perform a login request to get a token), then call
DELETE f"{_BASE}/{new_user_id}" with those headers and assert a 409 response;
reference the existing test_delete_ceo_self_rejected, _BASE, and TestClient to
locate where to add the test and reuse request patterns/fixtures for user
creation and auth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e66833da-1439-42d4-a466-c6924c2ace50
📒 Files selected for processing (5)
src/synthorg/api/controllers/users.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/api/controllers/test_users.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/sqlite/test_user_repo.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax:except A, B:(no parentheses) instead ofexcept (A, B):-- ruff enforces this on Python 3.14
All public functions and classes must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones; for non-Pydantic internal collections usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) over barecreate_taskfor structured concurrency
Keep functions under 50 lines and files under 800 lines
Line length must be 88 characters as enforced by ruff
Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors
Files:
tests/unit/persistence/sqlite/test_user_repo.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/api/controllers/test_users.pysrc/synthorg/api/controllers/users.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowto categorize tests
Maintain 80% minimum code coverage as enforced in CI
Useasyncio_mode = 'auto'for async tests; no manual@pytest.mark.asyncioneeded
Use@pytest.mark.parametrizefor testing similar cases instead of test duplication
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, tests, or config examples; use generic names:example-provider,example-large-001,test-provider,test-small-001,large/medium/small
Use Hypothesis with@given+@settingsfor property-based testing; avoid flaky timing-based tests by mockingtime.monotonic()andasyncio.sleep()or usingasyncio.Event().wait()for indefinite blocks
Files:
tests/unit/persistence/sqlite/test_user_repo.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/api/controllers/test_users.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and use logger:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the structured logger fromsynthorg.observability
Always use the logger variable namelogger(not_logger, notlog)
Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTEDfromevents.api); import directly and always use structured kwargs:logger.info(EVENT, key=value)never string formatting
All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (withmodel_copy(update=...)) for runtime state that evolves
Use Pydantic v2 with@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStr(fromcore.types) for all identifier/name fields instead of manual validators
All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically; never implement retry logic in driver subclasses or calling code
Validate at system boundaries: user input, external APIs, config files; handle errors explicitly, never silently swallow
Files:
src/synthorg/api/controllers/users.py
🧠 Learnings (11)
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to **/*.py : Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Applied to files:
tests/unit/api/controllers/test_users.py
📚 Learning: 2026-03-16T10:40:25.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T10:40:25.144Z
Learning: Applies to **/*.py : Docstrings: Google style, required on public classes and functions (enforced by ruff D rules).
Applied to files:
tests/unit/api/controllers/test_users.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/api/controllers/users.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/api/controllers/users.py
🔇 Additional comments (14)
src/synthorg/api/controllers/users.py (5)
1-33: LGTM!Module setup is clean: proper docstring, correct logger import from
synthorg.observability, and appropriate imports organized by source. The use ofget_logger(__name__)follows the coding guidelines.
48-88: LGTM!The DTOs are well-designed:
- All use
frozen=Truefor immutability as per guidelinesNotBlankStris used for identifier fields_to_responsecorrectly maps the domain model while omitting the sensitivepassword_hashfield
94-163: LGTM!All validation helpers properly log at WARNING level before raising exceptions, satisfying the coding guideline that "all error paths must log at WARNING or ERROR with context before raising."
241-281: LGTM!The
list_usersandget_userendpoints are well-implemented with appropriate logging levels (DEBUG for listing, WARNING for not-found via_get_user_or_404).
339-388: LGTM!The
delete_userendpoint properly:
- Validates constraints in logical order (self-deletion, system user, CEO)
- Handles the race condition where a user could be deleted between fetch and delete
- Logs at appropriate levels with non-PII identifiers
tests/unit/api/controllers/test_users.py (5)
1-27: LGTM!Module setup is clean with proper docstring and the
_create_payloadhelper now uses non-mutating dict merge ({**defaults, **overrides}) as per coding guidelines.
29-136: LGTM!The
TestCreateUserclass provides comprehensive coverage:
- Success cases with various allowed roles using
@pytest.mark.parametrize- Error cases for second CEO, system role, duplicate username, short password
- Guard tests for non-CEO roles
- Properly asserts
password_hashis not in responses
138-163: LGTM!The list tests properly verify the seeded user count and assert that
password_hashis not present in any response item.
165-198: LGTM!The get user tests cover both the success case (with
password_hashabsence check) and the 404 case for nonexistent users.
201-296: LGTM!The update role tests comprehensively cover:
- Successful role change with
password_hashabsence check- Rejection of SYSTEM role assignment
- Protection of system user from modification
- CEO business constraints (cannot demote only CEO, cannot create second CEO)
src/synthorg/persistence/sqlite/schema.sql (1)
186-186: LGTM!The partial unique index
idx_single_ceocorrectly enforces the single-CEO constraint at the database level, addressing the race condition concern with the application-level_CEO_LOCK. TheIF NOT EXISTSclause ensures idempotent schema application.tests/unit/persistence/sqlite/test_migrations.py (1)
54-54: LGTM!The expected indexes set is correctly updated to include
idx_single_ceo, aligning with the schema change.tests/unit/persistence/sqlite/test_user_repo.py (2)
41-56: LGTM!The default role change from
HumanRole.CEOtoHumanRole.MANAGERin_make_useris appropriate given the new partial unique index constraint that enforces a single CEO. This prevents test failures when multiple users are created with default values.
129-144: LGTM!The
test_count_by_role_filters_correctlytest data and assertions are correctly updated to reflect the MANAGER role instead of CEO, avoiding unique constraint violations while still effectively testing thecount_by_rolefunctionality.
| try: | ||
| await app_state.persistence.users.save(updated) | ||
| except QueryError: | ||
| msg = "A CEO user already exists" | ||
| logger.warning(API_RESOURCE_CONFLICT, reason=msg) | ||
| raise ConflictError(msg) # noqa: B904 |
There was a problem hiding this comment.
Same misleading error message concern as in create_user.
The except QueryError block assumes the constraint violation is always CEO-related, but other database constraints could trigger this error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/users.py` around lines 324 - 329, The except
QueryError block in the update flow (around
app_state.persistence.users.save(updated)) incorrectly assumes every QueryError
is a CEO-constraint violation; change the handler in the save/update path to
inspect the caught QueryError (or its message/code) and determine the real
cause, then log the actual error details with
logger.warning(API_RESOURCE_CONFLICT, reason=...) and raise a ConflictError with
a clear, appropriate message (or a generic conflict message that includes the
underlying error text) instead of always "A CEO user already exists"; reference
the save call, the QueryError exception, and the existing logger.warning and
ConflictError symbols when making the change.
| def test_delete_ceo_self_rejected( | ||
| self, | ||
| test_client: TestClient[Any], | ||
| ) -> None: | ||
| # The authenticated CEO attempts to delete themselves -- | ||
| # self-deletion check fires before the CEO role check. | ||
| list_resp = test_client.get(_BASE, headers=_CEO_HEADERS) | ||
| ceo_users = [u for u in list_resp.json()["data"] if u["role"] == "ceo"] | ||
| assert len(ceo_users) > 0 | ||
| ceo_id = ceo_users[0]["id"] | ||
|
|
||
| resp = test_client.delete( | ||
| f"{_BASE}/{ceo_id}", | ||
| headers=_CEO_HEADERS, | ||
| ) | ||
| assert resp.status_code == 409 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a test for non-CEO self-deletion.
The comment on lines 348-349 states "self-deletion check fires before the CEO role check", but since the authenticated user is the CEO, both checks would reject this action. To truly verify the self-deletion guard independently, consider adding a test that creates a non-CEO user, authenticates as that user, and attempts self-deletion.
This would require test infrastructure to generate auth headers for dynamically created users.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/controllers/test_users.py` around lines 344 - 359, Add a new
unit test alongside test_delete_ceo_self_rejected that verifies the
self-deletion guard for a non-CEO user: create a non-CEO user (via test_client
POST to _BASE or existing user factory), obtain authentication headers for that
new user (use the project’s auth helper or perform a login request to get a
token), then call DELETE f"{_BASE}/{new_user_id}" with those headers and assert
a 409 response; reference the existing test_delete_ceo_self_rejected, _BASE, and
TestClient to locate where to add the test and reuse request patterns/fixtures
for user creation and auth.
Distinguish between CEO uniqueness and username uniqueness constraint violations in the create_user save path by inspecting the underlying sqlite3.IntegrityError message. The update path only changes role (not username), so it correctly assumes the CEO constraint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🤖 I have created a release *beep* *boop* --- #MAJOR CHANGES; We got a somewhat working webui :) ## [0.5.0](v0.4.9...v0.5.0) (2026-03-30) ### Features * add analytics trends and budget forecast API endpoints ([#798](#798)) ([16b61f5](16b61f5)) * add department policies to default templates ([#852](#852)) ([7a41548](7a41548)) * add remaining activity event types (task_started, tool_used, delegation, cost_incurred) ([#832](#832)) ([4252fac](4252fac)) * agent performance, activity, and history API endpoints ([#811](#811)) ([9b75c1d](9b75c1d)) * Agent Profiles and Detail pages (biography, career, performance) ([#874](#874)) ([62d7880](62d7880)) * app shell, Storybook, and CI/CD pipeline ([#819](#819)) ([d4dde90](d4dde90)) * Approvals page with risk grouping, urgency indicators, batch actions ([#889](#889)) ([4e9673d](4e9673d)) * Budget Panel page (P&L dashboard, breakdown charts, forecast) ([#890](#890)) ([b63b0f1](b63b0f1)) * build infrastructure layer (API client, auth, WebSocket) ([#815](#815)) ([9f01d3e](9f01d3e)) * CLI global options infrastructure, UI modes, exit codes, env vars ([#891](#891)) ([fef4fc5](fef4fc5)) * CodeMirror editor and theme preferences toggle ([#905](#905), [#807](#807)) ([#909](#909)) ([41fbedc](41fbedc)) * Company page (department/agent management) ([#888](#888)) ([cfb88b0](cfb88b0)) * comprehensive hint coverage across all CLI commands ([#900](#900)) ([937974e](937974e)) * config system extensions, per-command flags for init/start/stop/status/logs ([#895](#895)) ([32f83fe](32f83fe)) * configurable currency system replacing hardcoded USD ([#854](#854)) ([b372551](b372551)) * Dashboard page (metric cards, activity feed, budget burn) ([#861](#861)) ([7d519d5](7d519d5)) * department health, provider status, and activity feed endpoints ([#818](#818)) ([6d5f196](6d5f196)) * design tokens and core UI components ([#833](#833)) ([ed887f2](ed887f2)) * extend approval, meeting, and budget API responses ([#834](#834)) ([31472bf](31472bf)) * frontend polish -- real-time UX, accessibility, responsive, performance ([#790](#790), [#792](#792), [#791](#791), [#793](#793)) ([#917](#917)) ([f04a537](f04a537)) * implement human roles and access control levels ([#856](#856)) ([d6d8a06](d6d8a06)) * implement semantic conflict detection in workspace merge ([#860](#860)) ([d97283b](d97283b)) * interaction components and animation patterns ([#853](#853)) ([82d4b01](82d4b01)) * Login page + first-run bootstrap + Company page ([#789](#789), [#888](#888)) ([#896](#896)) ([8758e8d](8758e8d)) * Meetings page with timeline viz, token bars, contribution formatting ([#788](#788)) ([#904](#904)) ([b207f46](b207f46)) * Messages page with threading, channel badges, sender indicators ([#787](#787)) ([#903](#903)) ([28293ad](28293ad)) * Org Chart force-directed view and drag-drop reassignment ([#872](#872), [#873](#873)) ([#912](#912)) ([a68a938](a68a938)) * Org Chart page (living nodes, status, CRUD, department health) ([#870](#870)) ([0acbdae](0acbdae)) * per-command flags for remaining commands, auto-behavior wiring, help/discoverability ([#897](#897)) ([3f7afa2](3f7afa2)) * Providers page with backend rework -- health, CRUD, subscription auth ([#893](#893)) ([9f8dd98](9f8dd98)) * scaffold React + Vite + TypeScript + Tailwind project ([#799](#799)) ([bd151aa](bd151aa)) * Settings page with search, dependency indicators, grouped rendering ([#784](#784)) ([#902](#902)) ([a7b9870](a7b9870)) * Setup Wizard rebuild with template comparison, cost estimator, theme customization ([#879](#879)) ([ae8b50b](ae8b50b)) * setup wizard UX -- template filters, card metadata, provider form reuse ([#910](#910)) ([7f04676](7f04676)) * setup wizard UX overhaul -- mode choice, step reorder, provider fixes ([#907](#907)) ([ee964c4](ee964c4)) * structured ModelRequirement in template agent configs ([#795](#795)) ([7433548](7433548)) * Task Board page (rich Kanban, filtering, dependency viz) ([#871](#871)) ([04a19b0](04a19b0)) ### Bug Fixes * align frontend types with backend and debounce WS refetches ([#916](#916)) ([134c11b](134c11b)) * auto-cleanup targets newly pulled images instead of old ones ([#884](#884)) ([50e6591](50e6591)) * correct wipe backup-skip flow and harden error handling ([#808](#808)) ([c05860f](c05860f)) * improve provider setup in wizard, subscription auth, dashboard bugs ([#914](#914)) ([87bf8e6](87bf8e6)) * improve update channel detection and add config get command ([#814](#814)) ([6b137f0](6b137f0)) * resolve all ESLint warnings, add zero-warnings enforcement ([#899](#899)) ([079b46a](079b46a)) * subscription auth uses api_key, base URL optional for cloud providers ([#915](#915)) ([f0098dd](f0098dd)) ### Refactoring * semantic analyzer cleanup -- shared filtering, concurrency, extraction ([#908](#908)) ([81372bf](81372bf)) ### Documentation * brand identity and UX design system from [#765](#765) exploration ([#804](#804)) ([389a9f4](389a9f4)) * page structure and information architecture for v0.5.0 dashboard ([#809](#809)) ([f8d6d4a](f8d6d4a)) * write UX design guidelines with WCAG-verified color system ([#816](#816)) ([4a4594e](4a4594e)) ### Tests * add unit tests for agent hooks and page components ([#875](#875)) ([#901](#901)) ([1d81546](1d81546)) ### CI/CD * bump actions/deploy-pages from 4.0.5 to 5.0.0 in the major group ([#831](#831)) ([01c19de](01c19de)) * bump astral-sh/setup-uv from 7.6.0 to 8.0.0 in /.github/actions/setup-python-uv in the all group ([#920](#920)) ([5f6ba54](5f6ba54)) * bump codecov/codecov-action from 5.5.3 to 6.0.0 in the major group ([#868](#868)) ([f22a181](f22a181)) * bump github/codeql-action from 4.34.1 to 4.35.0 in the all group ([#883](#883)) ([87a4890](87a4890)) * bump sigstore/cosign-installer from 4.1.0 to 4.1.1 in the minor-and-patch group ([#830](#830)) ([7a69050](7a69050)) * bump the all group with 3 updates ([#923](#923)) ([ff27c8e](ff27c8e)) * bump wrangler from 4.76.0 to 4.77.0 in /.github in the minor-and-patch group ([#822](#822)) ([07d43eb](07d43eb)) * bump wrangler from 4.77.0 to 4.78.0 in /.github in the all group ([#882](#882)) ([f84118d](f84118d)) ### Maintenance * add design system enforcement hook and component inventory ([#846](#846)) ([15abc43](15abc43)) * add dev-only auth bypass for frontend testing ([#885](#885)) ([6cdcd8a](6cdcd8a)) * add pre-push rebase check hook ([#855](#855)) ([b637a04](b637a04)) * backend hardening -- eviction/size-caps and model validation ([#911](#911)) ([81253d9](81253d9)) * bump axios from 1.13.6 to 1.14.0 in /web in the all group across 1 directory ([#922](#922)) ([b1b0232](b1b0232)) * bump brace-expansion from 5.0.4 to 5.0.5 in /web ([#862](#862)) ([ba4a565](ba4a565)) * bump eslint-plugin-react-refresh from 0.4.26 to 0.5.2 in /web ([#801](#801)) ([7574bb5](7574bb5)) * bump faker from 40.11.0 to 40.11.1 in the minor-and-patch group ([#803](#803)) ([14d322e](14d322e)) * bump https://github.com/astral-sh/ruff-pre-commit from v0.15.7 to 0.15.8 ([#864](#864)) ([f52901e](f52901e)) * bump nginxinc/nginx-unprivileged from `6582a34` to `f99cc61` in /docker/web in the all group ([#919](#919)) ([df85e4f](df85e4f)) * bump nginxinc/nginx-unprivileged from `ccbac1a` to `6582a34` in /docker/web ([#800](#800)) ([f4e9450](f4e9450)) * bump node from `44bcbf4` to `71be405` in /docker/sandbox ([#827](#827)) ([91bec67](91bec67)) * bump node from `5209bca` to `cf38e1f` in /docker/web ([#863](#863)) ([66d6043](66d6043)) * bump picomatch in /site ([#842](#842)) ([5f20bcc](5f20bcc)) * bump recharts 2->3 and @types/node 22->25 in /web ([#802](#802)) ([a908800](a908800)) * Bump requests from 2.32.5 to 2.33.0 ([#843](#843)) ([41daf69](41daf69)) * bump smol-toml from 1.6.0 to 1.6.1 in /site ([#826](#826)) ([3e5dbe4](3e5dbe4)) * bump the all group with 3 updates ([#921](#921)) ([7bace0b](7bace0b)) * bump the minor-and-patch group across 1 directory with 2 updates ([#829](#829)) ([93e611f](93e611f)) * bump the minor-and-patch group across 1 directory with 3 updates ([#841](#841)) ([7010c8e](7010c8e)) * bump the minor-and-patch group across 1 directory with 3 updates ([#869](#869)) ([548cee5](548cee5)) * bump the minor-and-patch group in /site with 2 updates ([#865](#865)) ([9558101](9558101)) * bump the minor-and-patch group with 2 updates ([#867](#867)) ([4830706](4830706)) * consolidate Dependabot groups to 1 PR per ecosystem ([06d2556](06d2556)) * consolidate Dependabot groups to 1 PR per ecosystem ([#881](#881)) ([06d2556](06d2556)) * improve worktree skill with full dep sync and status enhancements ([#906](#906)) ([772c625](772c625)) * remove Vue remnants and document framework decision ([#851](#851)) ([bf2adf6](bf2adf6)) * update web dependencies and fix brace-expansion CVE ([#880](#880)) ([a7a0ed6](a7a0ed6)) * upgrade to Storybook 10 and TypeScript 6 ([#845](#845)) ([52d95f2](52d95f2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
require_roles()guard factory for fine-grained per-role endpoint authorization, with named constants (require_ceo,require_ceo_or_manager,require_approval_roles)_WRITE_ROLES-- board members can now only observe and approve/reject, matching the design spec's "observe + major approvals only" definitionUserControllerwith CEO-only CRUD for user management (create, list, get, update role, delete) with business rules: single CEO constraint, SYSTEM role protection, password validation, username uniqueness_SYSTEM_WRITE_ROLES,require_system_or_write_access) after backup controller migrationTest plan
require_ceo_or_managertest across all rolesCloses #257
🤖 Generated with Claude Code