feat: Litestar REST API, WebSocket feed, and approval queue (M6)#189
Merged
feat: Litestar REST API, WebSocket feed, and approval queue (M6)#189
Conversation
…d guards (#31, #33) Implements the full API layer with Litestar: - App factory (create_app) with CORS, GZip, OpenAPI (Scalar + Redoc) - Request logging middleware, exception handlers, stub route guards - 13 REST controllers: health, company, agents, departments, projects, tasks (full CRUD), messages, meetings, artifacts, budget, analytics, providers, approvals (stub) - Response envelope pattern (ApiResponse/PaginatedResponse) - In-memory pagination helper - 56 unit tests covering all controllers, middleware, guards, and config Closes #31 Closes #33
Implements WebSocket infrastructure for real-time event delivery: - 5 channels: tasks, agents, budget, messages, system - WsEvent model with 14 event types (StrEnum) - ChannelsPlugin factory with MemoryChannelsBackend - MessageBusBridge polling internal MessageBus and re-publishing as WsEvent to Litestar channels - WsHandler (WebsocketListener) at /ws with subscribe/unsubscribe JSON protocol - 11 unit tests covering channels, models, bridge, and handler Closes #34
Add 'api' key to default_config_dict and 'api' to expected event module discovery set.
Pre-reviewed by 9 agents, 32 findings addressed: - Rename ValidationError to ApiValidationError (avoid pydantic shadow) - Add structured logging to all controllers with event constants - Fix exception handler log levels (ERROR for 5xx, WARNING for 4xx) - Remove global ValueError handler (was hiding bugs as 422) - Add bounds clamping in pagination helper - Strip api_key from provider responses - Fix bus bridge error handling (skip failed subscriptions) - Add lifecycle guard to bus bridge (prevent double-start) - Extract app.py lifecycle hooks to reduce complexity - Fix stub endpoints to return 501 status code (not 200) - Use UUID for task ID generation instead of id()-based hash - Add try/finally in middleware for reliable completion logging - Add ServiceStatus enum in health controller - Truncate reflected action in WS error responses - Add channel field validation in WsEvent model
Pre-reviewed by 10 agents, 59 findings addressed: Source fixes: - Scrub all error messages returned to clients (exception_handlers) - Add security headers (X-Content-Type-Options, X-Frame-Options, Referrer-Policy) - Add require_read_access guard to analytics, budget, company controllers - Add ApiResponse cross-field validation (success/error/data invariant) - Add CorsConfig wildcard+credentials validator - Add RateLimitTimeUnit StrEnum for type-safe time units - Move startup_time from global to frozen AppState dataclass - Add filter bounds validation to WebSocket handler - Replace factory pattern in server.py with direct app instance - Extract _cleanup_on_failure helper for safe startup reverse cleanup - Add _MAX_CONSECUTIVE_ERRORS escalation in bus bridge - Narrow exception catching in bus_bridge.start() - Fix unbounded DB fetch in messages controller - Add assigned_to to Task constructor in tasks controller - Use NotBlankStr for ws_models channel field - Fix middleware status_code None handling - Add prominent security stub warning in guards module - Type-narrow ALL_CONTROLLERS tuple Test fixes: - Add X-Human-Role headers to all guarded endpoint tests - Update assertions for scrubbed error messages - Update analytics tasks_by_status to expect all-statuses dict - Add tests for CORS wildcard+credentials validation - Add tests for RateLimitTimeUnit StrEnum - Add tests for WS filter bounds (too many keys, value too long) - Add guard denial tests for analytics, budget, company endpoints
Add full CRUD approval queue replacing the 501 stub: - ApprovalItem domain model with status/decision validators - In-memory ApprovalStore with lazy expiration - REST endpoints: list, get, create, approve, reject - WebSocket notifications via CHANNEL_APPROVALS - Guards: require_read_access for list/get, require_write_access for mutations - decided_by populated from X-Human-Role header - ApprovalStatus, ApprovalRiskLevel enums - CreateApprovalRequest, ApproveRequest, RejectRequest DTOs - ValidationException handler (400) in exception_handlers - Scalar-only API docs at /docs/api (dropped RedDoc) - DESIGN_SPEC updated for Scalar docs references - 51 new tests (domain model, store, controller)
Pre-reviewed by 10 agents, 55 findings addressed: - Security: read guards on all controllers, message size limits, metadata validation, security headers (HSTS, CSP, Permissions-Policy) - Error handling: ApiError default_message pattern, ServiceUnavailableError (503), narrowed exception catches, log-before-raise consistency - AppState: refactored to class with property-based service access, raises 503 instead of opaque 500 for unconfigured services - Logging: event constants for all log calls, structured kwargs - Input validation: max_length on DTO fields, metadata bounds - WebSocket: transport error handling, send failure logging - Docs: updated DESIGN_SPEC §15.3 project structure, README M6 status - Tests: default observer header on TestClient, guard test fixes
Contributor
Dependency ReviewThe following issues were found:
License Issuesuv.lock
OpenSSF ScorecardScorecard details
Scanned Files
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (27)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdds a Litestar-based API layer: application factory, config models, middleware, exception handlers, channels and WebSocket feed, message-bus bridge, approval queue (domain model + in-memory store + controller), many Litestar controllers, observability events, AppState, and extensive unit tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant App as Litestar App
participant Persistence
participant MessageBus
participant Bridge
participant Channels as ChannelsPlugin
Client->>App: create_app(config, persistence, message_bus, ...)
App->>App: on_startup hook
App->>Persistence: connect()
App->>MessageBus: start()
App->>Bridge: Bridge.start()
Bridge->>MessageBus: subscribe ALL_CHANNELS
Bridge->>Bridge: spawn _poll_channel tasks
Note over App,Channels: app ready (OpenAPI, /docs)
Client->>App: GET /api/v1/health
App->>Persistence: health_check()
App->>MessageBus: is_running()
App-->>Client: 200 OK (HealthStatus)
Client->>App: shutdown
App->>Bridge: Bridge.stop()
Bridge->>Bridge: cancel polling tasks
App->>MessageBus: stop()
App->>Persistence: disconnect()
sequenceDiagram
autonumber
participant WSClient
participant WSHandler as Litestar WS handler
participant Channels as ChannelsPlugin
participant Bridge
participant MessageBus
WSClient->>WSHandler: WebSocket CONNECT (/api/v1/ws)
WSHandler->>WSHandler: accept, start receive loop
WSClient->>WSHandler: {"action":"subscribe","channels":["tasks","approvals"]}
WSHandler->>WSHandler: validate channels, add subscriptions
WSHandler-->>WSClient: {"action":"subscribed",...}
MessageBus->>Bridge: new envelope on "tasks"
Bridge->>Channels: publish(WsEvent)
Channels->>WSHandler: deliver event to subscribed client
WSHandler->>WSClient: {"event_type":..., "channel":"tasks", "payload":...}
WSClient->>WSHandler: close
WSHandler->>WSHandler: cleanup subscriptions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests (beta)
✨ Simplify code
|
Greptile SummaryThis PR introduces a full Litestar REST API (14 controllers), a WebSocket real-time feed with channel-based subscriptions, a human approval queue, stub role-based guards, security headers, and an Critical issue — blocking merge:
Logic issue:
Confidence Score: 1/5
Sequence DiagramsequenceDiagram
participant C as HTTP Client
participant G as Guard (require_read/write)
participant Ctrl as Controller
participant AS as ApprovalStore
participant CH as ChannelsPlugin
participant WS as WebSocket Client
C->>G: POST /approvals (X-Human-Role: manager)
G-->>Ctrl: role validated
Ctrl->>AS: add(ApprovalItem)
AS-->>Ctrl: OK
Ctrl->>CH: publish(APPROVAL_SUBMITTED, approvals)
CH-->>WS: WsEvent{approval.submitted}
Ctrl-->>C: 201 ApiResponse[ApprovalItem]
C->>G: POST /approvals/{id}/approve
G-->>Ctrl: role validated
Ctrl->>AS: get(id) → _check_expiration()
AS-->>Ctrl: item (PENDING or EXPIRED)
alt item is PENDING
Ctrl->>AS: save(updated APPROVED)
AS-->>Ctrl: saved
Ctrl->>CH: publish(APPROVAL_APPROVED, approvals)
CH-->>WS: WsEvent{approval.approved}
Ctrl-->>C: 200 ApiResponse[ApprovalItem]
else item is EXPIRED
Ctrl-->>C: 409 ConflictError
end
Note over AS,CH: Lazy expiry path
WS->>CH: subscribe {action: subscribe, channels: [approvals]}
AS->>AS: _check_expiration() → status=EXPIRED
AS->>CH: on_expire callback → publish(APPROVAL_EXPIRED)
CH-->>WS: WsEvent{approval.expired}
Last reviewed commit: 358e94f |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request marks a significant architectural shift by transitioning the core API from FastAPI to Litestar, delivering a comprehensive set of REST and WebSocket endpoints. The change enables robust real-time communication and introduces a human approval queue, crucial for critical operations. It also lays foundational security measures and improves error handling, advancing the project to its M6 milestone. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Introduces the M6 Litestar-based API layer for the AI Company system, including REST controllers, WebSocket real-time feeds, and a human approval queue, with supporting config/state/error handling and comprehensive unit tests.
Changes:
- Add Litestar app factory with controllers, middleware, exception handlers, guards, and health endpoint under
/api/v1. - Implement WebSocket channels + message-bus bridge + WS event models for real-time feeds.
- Implement approval queue domain model + in-memory store + approval REST endpoints and notifications.
Reviewed changes
Copilot reviewed 67 out of 70 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/observability/test_events.py | Registers new api observability domain module in discovery test. |
| tests/unit/core/test_approval.py | Unit tests for ApprovalItem invariants (decision fields, expiry, metadata). |
| tests/unit/api/test_ws_models.py | Tests WsEventType values and WsEvent serialization/frozen behavior. |
| tests/unit/api/test_middleware.py | Smoke tests for request logging middleware behavior via status codes. |
| tests/unit/api/test_health.py | Tests health endpoint response envelope and degraded/down states. |
| tests/unit/api/test_guards.py | Tests role-based read/write guard behavior via X-Human-Role. |
| tests/unit/api/test_exception_handlers.py | Tests exception→HTTP mapping and error scrubbing. |
| tests/unit/api/test_config.py | Tests API config model defaults/validation/frozen behavior. |
| tests/unit/api/test_channels.py | Tests channel constants and channels plugin factory. |
| tests/unit/api/test_bus_bridge.py | Tests message bus → WS event conversion behavior. |
| tests/unit/api/test_approval_store.py | Tests in-memory approval store CRUD + filtering + lazy expiry. |
| tests/unit/api/test_app.py | Tests Litestar app factory and OpenAPI schema endpoint. |
| tests/unit/api/controllers/test_ws.py | Tests WS subscribe/unsubscribe parsing and filter bounds. |
| tests/unit/api/controllers/test_tasks.py | Tests task CRUD controller behavior and filtering. |
| tests/unit/api/controllers/test_providers.py | Tests provider controller list/get/model routes (including 404s). |
| tests/unit/api/controllers/test_projects.py | Tests stub project controller (empty list + 501 get). |
| tests/unit/api/controllers/test_messages.py | Tests message listing and channel discovery endpoint. |
| tests/unit/api/controllers/test_meetings.py | Tests stub meetings controller behavior. |
| tests/unit/api/controllers/test_departments.py | Tests department list/get behavior and 404. |
| tests/unit/api/controllers/test_company.py | Tests company config endpoint and read guard enforcement. |
| tests/unit/api/controllers/test_budget.py | Tests budget config, cost record listing, and agent spending. |
| tests/unit/api/controllers/test_artifacts.py | Tests stub artifact controller behavior. |
| tests/unit/api/controllers/test_approvals.py | End-to-end tests for approval lifecycle endpoints and guards. |
| tests/unit/api/controllers/test_analytics.py | Tests analytics overview metrics and guard enforcement. |
| tests/unit/api/controllers/test_agents.py | Tests agent list/get behavior, including app wiring in one test. |
| tests/unit/api/controllers/init.py | Package marker for controller tests. |
| tests/unit/api/conftest.py | Shared Litestar TestClient + fake persistence/bus fixtures and builders. |
| tests/unit/api/init.py | Package marker for API tests. |
| src/ai_company/observability/events/task.py | Adds TASK_CREATED event constant used by task controller. |
| src/ai_company/observability/events/api.py | Adds API-specific observability event constants. |
| src/ai_company/core/enums.py | Adds ApprovalStatus and ApprovalRiskLevel enums. |
| src/ai_company/core/approval.py | Adds ApprovalItem domain model with validation invariants. |
| src/ai_company/core/init.py | Re-exports approval model/enums from ai_company.core. |
| src/ai_company/config/schema.py | Adds api: ApiConfig to RootConfig. |
| src/ai_company/config/defaults.py | Includes api in default config dict. |
| src/ai_company/api/ws_models.py | Defines WsEventType and WsEvent models for WS payloads. |
| src/ai_company/api/state.py | Introduces AppState with typed service access and 503-on-missing behavior. |
| src/ai_company/api/server.py | Adds uvicorn runner using RootConfig.api.server settings. |
| src/ai_company/api/pagination.py | Adds offset/limit pagination helpers and query param types. |
| src/ai_company/api/middleware.py | Adds request logging ASGI middleware emitting API request events. |
| src/ai_company/api/guards.py | Adds role-based guards using X-Human-Role header (stub auth). |
| src/ai_company/api/exception_handlers.py | Adds exception handler mapping with scrubbed error responses + logging. |
| src/ai_company/api/errors.py | Adds API error hierarchy (ApiError, NotFoundError, etc.). |
| src/ai_company/api/dto.py | Adds response envelopes, pagination meta, and request DTOs with bounds checks. |
| src/ai_company/api/controllers/ws.py | Adds WS handler supporting subscribe/unsubscribe with per-channel filters. |
| src/ai_company/api/controllers/tasks.py | Adds task CRUD controller and task transition endpoint. |
| src/ai_company/api/controllers/providers.py | Adds read-only provider endpoints with API key stripping. |
| src/ai_company/api/controllers/projects.py | Adds stub projects controller (until persistence exists). |
| src/ai_company/api/controllers/messages.py | Adds message history and channel listing endpoints. |
| src/ai_company/api/controllers/meetings.py | Adds stub meetings controller (until persistence exists). |
| src/ai_company/api/controllers/health.py | Adds health endpoint with persistence/bus checks and uptime/version. |
| src/ai_company/api/controllers/departments.py | Adds department list/get endpoints from config. |
| src/ai_company/api/controllers/company.py | Adds curated company config endpoints. |
| src/ai_company/api/controllers/budget.py | Adds budget config, cost record list, and agent spend endpoints. |
| src/ai_company/api/controllers/artifacts.py | Adds stub artifacts controller (until persistence exists). |
| src/ai_company/api/controllers/approvals.py | Adds approval queue endpoints + WS publish hooks. |
| src/ai_company/api/controllers/analytics.py | Adds derived analytics overview endpoint. |
| src/ai_company/api/controllers/agents.py | Adds agent list/get endpoints from config. |
| src/ai_company/api/controllers/init.py | Collects controllers into ALL_CONTROLLERS for app wiring. |
| src/ai_company/api/config.py | Adds API config models (CORS, rate limit, server, prefix). |
| src/ai_company/api/channels.py | Defines WS channel constants and ChannelsPlugin factory. |
| src/ai_company/api/bus_bridge.py | Adds message-bus polling bridge that republishes as WS events. |
| src/ai_company/api/approval_store.py | Adds in-memory approval storage with lazy expiry. |
| src/ai_company/api/app.py | Adds Litestar app factory wiring routes, plugins, middleware, lifecycle. |
| src/ai_company/api/init.py | Adds API package marker and guidance to import create_app directly. |
| pyproject.toml | Adds Litestar dependency, mypy override, and warning filter updates. |
| README.md | Updates milestone status and tech stack to reflect Litestar API (M6 complete). |
| DESIGN_SPEC.md | Updates design spec to reflect Litestar, API-first architecture, and project structure. |
| CLAUDE.md | Updates package structure notes for new API implementation and CLI deferral. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+57
to
+61
| subscribed: set[str] = set() | ||
| filters: dict[str, dict[str, str]] = {} | ||
|
|
||
| subscriber = await channels_plugin.subscribe(list(ALL_CHANNELS)) | ||
|
|
There was a problem hiding this comment.
ws_handler subscribes the backend subscriber to all channels immediately, regardless of what the client subscribes to. This means every connection will parse every published event and then drop most of them, which can become a CPU hot-spot with many clients / high event volume. Consider subscribing only to requested channels (and updating the subscription on subscribe/unsubscribe) so the backend does the filtering.
Comment on lines
+64
to
+76
| try: | ||
| event = json.loads(event_data) | ||
| except json.JSONDecodeError, TypeError: | ||
| logger.warning( | ||
| API_WS_INVALID_MESSAGE, | ||
| data_preview=str(event_data)[:100], | ||
| source="channels_backend", | ||
| ) | ||
| return | ||
|
|
||
| channel = event.get("channel", "") | ||
| if channel not in subscribed: | ||
| return |
There was a problem hiding this comment.
_on_event assumes json.loads() returns a dict and immediately calls event.get(). If a publisher sends valid JSON that's not an object (e.g., a list/string/number), this will raise AttributeError and can break the background subscriber task. Add an isinstance(event, dict) check (and treat non-dict payloads as invalid) before accessing keys.
src/ai_company/api/controllers/ws.py
Outdated
Comment on lines
+138
to
+139
| if len(data) > _MAX_WS_MESSAGE_BYTES: | ||
| return json.dumps({"error": "Message too large"}) |
There was a problem hiding this comment.
The size guard uses len(data) (characters), but _MAX_WS_MESSAGE_BYTES is expressed in bytes. For non-ASCII payloads this can undercount and allow larger-than-intended messages. Consider checking the UTF-8 byte length (or rename the constant to reflect character count).
Comment on lines
+141
to
+158
| try: | ||
| msg = json.loads(data) | ||
| except json.JSONDecodeError, TypeError: | ||
| logger.warning( | ||
| API_WS_INVALID_MESSAGE, | ||
| data_preview=str(data)[:100], | ||
| ) | ||
| return json.dumps({"error": "Invalid JSON"}) | ||
|
|
||
| action = msg.get("action") | ||
| channels: list[str] = msg.get("channels", []) | ||
| client_filters: dict[str, str] = msg.get("filters", {}) | ||
|
|
||
| if action == "subscribe": | ||
| # Validate filter bounds to prevent memory abuse. | ||
| if len(client_filters) > _MAX_FILTER_KEYS or any( | ||
| len(str(v)) > _MAX_FILTER_VALUE_LEN for v in client_filters.values() | ||
| ): |
There was a problem hiding this comment.
After json.loads(), msg is assumed to be a dict and channels/filters are assumed to be list/dict. A client can send valid JSON that isn't an object (or with wrong field types), which will raise (e.g., AttributeError on msg.get or on client_filters.values()) and tear down the connection via the catch-all in _receive_loop. Add explicit shape/type validation (e.g., require msg to be a dict, channels to be a list of strings, filters to be a dict[str,str]) and return a structured error instead of throwing.
Comment on lines
+41
to
+48
| if item.id in self._items: | ||
| msg = f"Approval {item.id!r} already exists" | ||
| logger.warning( | ||
| API_APPROVAL_EXPIRED, | ||
| error="duplicate", | ||
| approval_id=item.id, | ||
| ) | ||
| raise ConflictError(msg) |
There was a problem hiding this comment.
On duplicate add(), this logs API_APPROVAL_EXPIRED, which misclassifies the event and will skew observability (this is a conflict, not an expiry). Consider logging API_APPROVAL_CONFLICT (or a dedicated duplicate event) instead.
Comment on lines
+217
to
+223
| if persistence is None or message_bus is None or cost_tracker is None: | ||
| msg = ( | ||
| "create_app called without persistence, message_bus, " | ||
| "and/or cost_tracker — controllers accessing missing " | ||
| "services will return 500. Use test fakes for testing." | ||
| ) | ||
| logger.warning(API_APP_STARTUP, note=msg) |
There was a problem hiding this comment.
The warning message claims missing services will cause controllers to return 500, but AppState property accessors raise ServiceUnavailableError (503). Update the message (and/or the behavior) so it matches the actual HTTP status clients will see.
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: a Litestar-based REST API, WebSocket feed, and a human approval queue. The migration from FastAPI to Litestar is well-justified, and the new API is thoughtfully designed with controllers, DTOs, custom error handling, and security measures. However, there are critical concerns regarding the authentication stub and missing object-level authorization. Additionally, several files contain Python syntax errors in exception handling blocks that will cause runtime failures.
Comment on lines
+48
to
+107
| def _get_role(connection: ASGIConnection) -> str | None: # type: ignore[type-arg] | ||
| """Extract the human role from the request header.""" | ||
| value = connection.headers.get("x-human-role") | ||
| if value is not None: | ||
| return value.strip().lower() | ||
| return None | ||
|
|
||
|
|
||
| def require_write_access( | ||
| connection: ASGIConnection, # type: ignore[type-arg] | ||
| _: object, | ||
| ) -> None: | ||
| """Guard that allows only write-capable roles. | ||
|
|
||
| Checks the ``X-Human-Role`` header for ``ceo``, ``manager``, | ||
| ``board_member``, or ``pair_programmer``. | ||
|
|
||
| Args: | ||
| connection: The incoming connection. | ||
| _: Route handler (unused). | ||
|
|
||
| Raises: | ||
| PermissionDeniedException: If the role is not permitted. | ||
| """ | ||
| role = _get_role(connection) | ||
| if role not in _WRITE_ROLES: | ||
| logger.warning( | ||
| API_GUARD_DENIED, | ||
| guard="require_write_access", | ||
| role=role, | ||
| path=str(connection.url.path), | ||
| ) | ||
| raise PermissionDeniedException(detail="Write access denied") | ||
|
|
||
|
|
||
| def require_read_access( | ||
| connection: ASGIConnection, # type: ignore[type-arg] | ||
| _: object, | ||
| ) -> None: | ||
| """Guard that allows all recognised roles. | ||
|
|
||
| Checks the ``X-Human-Role`` header for any valid role | ||
| including ``observer``. | ||
|
|
||
| Args: | ||
| connection: The incoming connection. | ||
| _: Route handler (unused). | ||
|
|
||
| Raises: | ||
| PermissionDeniedException: If the role is not permitted. | ||
| """ | ||
| role = _get_role(connection) | ||
| if role not in _READ_ROLES: | ||
| logger.warning( | ||
| API_GUARD_DENIED, | ||
| guard="require_read_access", | ||
| role=role, | ||
| path=str(connection.url.path), | ||
| ) | ||
| raise PermissionDeniedException(detail="Read access denied") |
Contributor
There was a problem hiding this comment.
The application relies on a self-asserted HTTP header (X-Human-Role) to determine user roles and permissions without any verification (e.g., via JWT, session tokens, or cryptographic signatures). An attacker can gain full administrative access by providing a header such as X-Human-Role: ceo. While documented as a stub for M6, this is a critical security flaw if the API is exposed to any untrusted network.
| for channel_name in ALL_CHANNELS: | ||
| try: | ||
| await self._bus.subscribe(channel_name, _SUBSCRIBER_ID) | ||
| except OSError, RuntimeError, ConnectionError: |
Contributor
There was a problem hiding this comment.
This except syntax with a comma is from Python 2 and is a SyntaxError in Python 3. To catch multiple exceptions, you should wrap them in a tuple.
except (OSError, RuntimeError, ConnectionError):| event.model_dump_json(), | ||
| channels=[CHANNEL_APPROVALS], | ||
| ) | ||
| except RuntimeError, OSError: |
Contributor
There was a problem hiding this comment.
This except syntax is for Python 2. In Python 3, multiple exceptions must be caught as a tuple.
except (RuntimeError, OSError):|
|
||
| try: | ||
| msg = json.loads(data) | ||
| except json.JSONDecodeError, TypeError: |
Contributor
There was a problem hiding this comment.
This except syntax is for Python 2. In Python 3, multiple exceptions must be caught as a tuple.
Suggested change
| except json.JSONDecodeError, TypeError: | |
| except (json.JSONDecodeError, TypeError): |
Comment on lines
+137
to
+216
| @patch("/{task_id:str}", guards=[require_write_access]) | ||
| async def update_task( | ||
| self, | ||
| state: State, | ||
| task_id: str, | ||
| data: UpdateTaskRequest, | ||
| ) -> ApiResponse[Task]: | ||
| """Update task fields. | ||
|
|
||
| Args: | ||
| state: Application state. | ||
| task_id: Task identifier. | ||
| data: Fields to update. | ||
|
|
||
| Returns: | ||
| Updated task envelope. | ||
|
|
||
| Raises: | ||
| NotFoundError: If the task is not found. | ||
| """ | ||
| app_state: AppState = state.app_state | ||
| task = await app_state.persistence.tasks.get(task_id) | ||
| if task is None: | ||
| msg = f"Task {task_id!r} not found" | ||
| logger.warning(API_RESOURCE_NOT_FOUND, resource="task", id=task_id) | ||
| raise NotFoundError(msg) | ||
|
|
||
| updates = data.model_dump(exclude_none=True) | ||
| if updates: | ||
| task = task.model_copy(update=updates) | ||
| await app_state.persistence.tasks.save(task) | ||
| logger.info(API_TASK_UPDATED, task_id=task_id, fields=list(updates)) | ||
| return ApiResponse(data=task) | ||
|
|
||
| @post( | ||
| "/{task_id:str}/transition", | ||
| guards=[require_write_access], | ||
| ) | ||
| async def transition_task( | ||
| self, | ||
| state: State, | ||
| task_id: str, | ||
| data: TransitionTaskRequest, | ||
| ) -> ApiResponse[Task]: | ||
| """Perform a status transition on a task. | ||
|
|
||
| Args: | ||
| state: Application state. | ||
| task_id: Task identifier. | ||
| data: Transition payload. | ||
|
|
||
| Returns: | ||
| Transitioned task envelope. | ||
|
|
||
| Raises: | ||
| NotFoundError: If the task is not found. | ||
| """ | ||
| app_state: AppState = state.app_state | ||
| task = await app_state.persistence.tasks.get(task_id) | ||
| if task is None: | ||
| msg = f"Task {task_id!r} not found" | ||
| logger.warning(API_RESOURCE_NOT_FOUND, resource="task", id=task_id) | ||
| raise NotFoundError(msg) | ||
|
|
||
| overrides: dict[str, object] = {} | ||
| if data.assigned_to is not None: | ||
| overrides["assigned_to"] = data.assigned_to | ||
|
|
||
| try: | ||
| new_task = task.with_transition(data.target_status, **overrides) | ||
| except ValueError as exc: | ||
| raise ApiValidationError(str(exc)) from exc | ||
| await app_state.persistence.tasks.save(new_task) | ||
| logger.info( | ||
| TASK_STATUS_CHANGED, | ||
| task_id=task_id, | ||
| from_status=task.status.value, | ||
| to_status=new_task.status.value, | ||
| ) | ||
| return ApiResponse(data=new_task) |
Contributor
There was a problem hiding this comment.
The task update and transition endpoints lack object-level authorization checks. Any user with a 'write' role can modify or transition any task in the system, regardless of whether they are the creator or assignee. This could allow a user to approve their own work by transitioning a task from IN_REVIEW to COMPLETED without proper oversight.
| try: | ||
| await self._bus.subscribe(channel_name, _SUBSCRIBER_ID) | ||
| except OSError, RuntimeError, ConnectionError: | ||
| logger.warning( |
Contributor
There was a problem hiding this comment.
Invalid Python syntax in except block. Catching multiple exceptions in Python 3 requires them to be enclosed in a tuple. This will cause a SyntaxError at parse time or a crash when the error path is executed, potentially leading to a Denial of Service.
except (OSError, RuntimeError, ConnectionError):| channels=[CHANNEL_APPROVALS], | ||
| ) | ||
| except RuntimeError, OSError: | ||
| logger.warning( |
Contributor
There was a problem hiding this comment.
Invalid Python syntax in except block. Catching multiple exceptions in Python 3 requires them to be enclosed in a tuple. This will cause a SyntaxError at parse time or a crash when the error path is executed.
except (RuntimeError, OSError):
Comment on lines
+32
to
+35
| allow_methods: tuple[str, ...] = Field( | ||
| default=("GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"), | ||
| description="HTTP methods permitted in cross-origin requests", | ||
| ) |
Contributor
There was a problem hiding this comment.
The type hint for allow_methods is tuple[str, ...], which is too broad. This leads to a type: ignore[arg-type] in src/ai_company/api/app.py because Litestar's CORSConfig expects a sequence of literals for HTTP methods. You can improve type safety by using typing.Literal to define the allowed methods, which will also allow removing the type: ignore. You'll need to add from typing import Literal to the file.
Suggested change
| allow_methods: tuple[str, ...] = Field( | |
| default=("GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"), | |
| description="HTTP methods permitted in cross-origin requests", | |
| ) | |
| allow_methods: tuple[Literal["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS", "HEAD", "TRACE", "CONNECT"], ...] = Field( | |
| default=("GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"), | |
| description="HTTP methods permitted in cross-origin requests", | |
| ) |
Comment on lines
+69
to
+71
| for agent in app_state.config.agents: | ||
| if agent.name == agent_name: | ||
| return ApiResponse(data=agent) |
Contributor
There was a problem hiding this comment.
This linear scan to find an agent by name has O(n) complexity. If the number of agents can be large, this could become a performance bottleneck. Consider creating a dictionary mapping agent names to agent configs in AppState during initialization for O(1) lookups.
Comment on lines
+69
to
+71
| for dept in app_state.config.departments: | ||
| if dept.name == name: | ||
| return ApiResponse(data=dept) |
Contributor
There was a problem hiding this comment.
Similar to the agent lookup, this is an O(n) lookup for a department. For better performance with a large number of departments, consider using a dictionary in AppState for O(1) lookups.
There was a problem hiding this comment.
Actionable comments posted: 31
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Around line 206-209: The global warning filter entry "ignore:Core Pydantic V1
functionality:UserWarning" should be narrowed to the specific emitter to avoid
masking future regressions; update the filterwarnings list entry to include the
third-party module (e.g. replace it with "ignore:Core Pydantic V1
functionality:UserWarning:litestar") or move a scoped filter to the specific
tests that trigger it so only the Litestar-emitted Pydantic V1 warning is
suppressed.
In `@src/ai_company/api/app.py`:
- Around line 286-289: Tighten the Content-Security-Policy set via
ResponseHeader by removing script-src 'unsafe-inline' for production: generate a
per-response nonce (e.g., attach a secure random value to the request/response),
include that nonce in the CSP header as script-src 'self' 'nonce-<value>' and
ensure the Scalar OpenAPI docs renderer injects that nonce into its inline
script tags; add a concise comment near the ResponseHeader explaining the
trade-off and keep a more permissive CSP only for the docs route(s) (e.g.,
conditionally set the ResponseHeader for the docs renderer vs. the rest of the
app) so non-docs routes use the stricter policy in M7.
In `@src/ai_company/api/approval_store.py`:
- Around line 43-48: Replace the wrong event constant used in the
duplicate-conflict log: in the logger.warning call inside approval handling (the
call that currently passes API_APPROVAL_EXPIRED with error="duplicate" and
approval_id=item.id), import and use API_APPROVAL_CONFLICT from
ai_company.observability.events.api instead of API_APPROVAL_EXPIRED so the event
name matches the duplicate-ID conflict; update the import or reference where
API_APPROVAL_EXPIRED is used and keep the rest of the logger.warning call and
the subsequent raise ConflictError(msg) unchanged.
In `@src/ai_company/api/bus_bridge.py`:
- Around line 145-159: The _to_ws_event function indiscriminately maps every
internal Message to WsEventType.MESSAGE_SENT, losing semantic distinctions;
update _to_ws_event to inspect the Message (e.g., message.type, message.kind, or
specific fields like status/task_id) and map to the appropriate WsEventType
(e.g., TASK_UPDATED, AGENT_STATUS_CHANGED, etc.), falling back to MESSAGE_SENT
when unknown, and include the original message.type in the payload for
traceability; ensure you update the WsEvent construction (event_type and
payload) in _to_ws_event and add any missing enum members to WsEventType if
needed.
In `@src/ai_company/api/config.py`:
- Around line 128-133: The config currently allows workers >1 which causes
split-brain because create_channels_plugin() uses MemoryChannelsBackend and
AppState.approval_store is in-memory; change the workers Field in the config
model to forbid multi-process mode by setting an upper bound of 1 (e.g., le=1)
or add a validator on the workers attribute that raises an error if >1 while
using MemoryChannelsBackend/AppState.approval_store, and update the Field
default/constraints accordingly so the app cannot be configured with multiple
workers in process-local mode.
In `@src/ai_company/api/controllers/artifacts.py`:
- Around line 1-9: The module artifacts.py is missing the required logger; add
the import and instantiation per guidelines by importing get_logger from
ai_company.observability and creating logger = get_logger(__name__) at module
scope so controllers like Artifact controller (Controller, Response, get) and
functions that will use require_read_access, paginate, PaginatedResponse,
ApiResponse can call logger; ensure the import uses the exact symbol get_logger
and the variable name logger.
In `@src/ai_company/api/controllers/budget.py`:
- Around line 85-107: The get_agent_spending handler accepts an empty/whitespace
agent_id which later fails Pydantic NotBlankStr validation when constructing
AgentSpending; update get_agent_spending to validate agent_id upfront (e.g., if
not agent_id or not agent_id.strip()) and return a 400 Bad Request (or raise the
framework's BadRequest/HTTPException) with a clear message instead of proceeding
to call State.app_state.cost_tracker.get_agent_cost and constructing
AgentSpending; alternatively change the path parameter type to a constrained
non-blank string type used by AgentSpending so the framework rejects bad input
before handler execution.
In `@src/ai_company/api/controllers/company.py`:
- Around line 49-63: The company controller's list_departments method returns a
plain ApiResponse[tuple[Department, ...]] but must match the paginated shape
used by DepartmentController.list_departments; update
CompanyController.list_departments to return the same paginated envelope (reuse
the PaginatedResponse/Pagination type and pagination fields) and construct the
response from app_state.config.departments the same way
DepartmentController.list_departments does (or call/delegate to
DepartmentController.list_departments) so both endpoints expose an identical
paginated resource shape.
In `@src/ai_company/api/controllers/departments.py`:
- Around line 69-74: The current exact-match lookup (if dept.name == name) can
fail on casing differences; change the comparison to a case-insensitive one by
comparing dept.name.casefold() to name.casefold() (or both .lower()) so get
by-name lookups in the loop return the intended department regardless of casing;
update the comparison inside the loop that iterates over
app_state.config.departments (the dept.name vs name check) to use casefold() for
robust Unicode-aware case-insensitive matching.
In `@src/ai_company/api/controllers/meetings.py`:
- Around line 1-9: Add a module-level structured logger named logger to this
controller module: import structlog (or the project's logging helper) and create
logger = structlog.get_logger(__name__) at the top of
src/ai_company/api/controllers/meetings.py alongside the other imports, ensuring
it follows the project's logger convention so other functions/classes in this
module (e.g., Controller, any future handlers) can use logger.debug/info/error
for structured logs.
- Around line 58-64: Add an observability log statement immediately before the
stubbed Response in the meetings controller so every hit is recorded; locate the
return that constructs Response(ApiResponse(..., error="Meeting persistence not
implemented yet"), status_code=501) in
src/ai_company/api/controllers/meetings.py and insert a logger call (e.g.,
logger.info or logger.warning) that records the endpoint name (or function name
handling the request), timestamp and any available request identifiers (user id,
request id) to help track demand for this unimplemented endpoint; ensure a
module-level logger is used or created (logger = logging.getLogger(__name__))
consistent with the project's logging conventions.
In `@src/ai_company/api/controllers/messages.py`:
- Around line 48-55: The controller currently fetches the entire channel history
via app_state.persistence.messages.get_history and then calls paginate(page,
meta) which causes unbounded reads; update the code so pagination is enforced at
the repository level by adding offset/limit (or at minimum a bounded fetch
window) to app_state.persistence.messages.get_history and call it with offset
and limit before calling paginate/PaginatedResponse, or modify the messages
repository API (get_history) to accept offset and limit and return a sliced
result plus total count so paginate can be applied without loading the full
history.
- Around line 28-30: Replace the channel parameter's type from plain str | None
to NotBlankStr | None to prevent whitespace-only values: import NotBlankStr from
core.types and update the function signature where channel: str | None is
declared (alongside PaginationOffset and PaginationLimit) to channel:
NotBlankStr | None, and adjust any related type checks/usages if they assume
plain str. Ensure the new import is added near the existing imports that bring
in PaginationOffset/PaginationLimit so static typing and runtime validation use
NotBlankStr.
In `@src/ai_company/api/controllers/projects.py`:
- Around line 1-9: Add the required module logger: import get_logger from
ai_company.observability and instantiate logger = get_logger(__name__) near the
top of the projects.py module (alongside the existing imports). Ensure the
import is added (from ai_company.observability import get_logger) and the logger
variable is declared at module scope so controller code (e.g., Controller, any
functions or handlers in this file) can use logger for structured logging per
project coding guidelines.
In `@src/ai_company/api/controllers/providers.py`:
- Around line 51-56: The route handlers currently accept raw str for provider
identifiers (e.g., get_provider) which allows blank/whitespace names to reach
the repo and produce 404s; update the route boundary to use NotBlankStr from
core.types for the name parameter (e.g., change the get_provider name param type
to NotBlankStr) so FastAPI validates non-blank input, and apply the same change
to the other provider-related endpoint(s) referenced at lines 77–82 (replace
their str name/identifier params with NotBlankStr or NotBlankStr | None /
tuple[NotBlankStr, ...] as appropriate).
- Around line 10-12: Replace direct serialization of config models
(ProviderConfig, ProviderModelConfig) with dedicated response DTOs: add
ProviderDTO and ProviderModelDTO (containing only public fields, excluding
internal retry/rate-limit/subscription/degradation internals and api_key) and
map instances to these DTOs before returning from the controller. Update places
that currently call or rely on _safe_provider() to instead construct and return
ProviderDTO (and map nested ProviderModelConfig -> ProviderModelDTO), and modify
any functions that previously returned ProviderConfig/ProviderModelConfig to
return the DTOs; ensure all fields are explicitly mapped so external contract is
decoupled from the config schema.
In `@src/ai_company/api/controllers/tasks.py`:
- Around line 205-215: The code calls task.with_transition(...) which already
emits the TASK_STATUS_CHANGED log, then redundantly logs the same event here via
logger.info(...); remove the duplicate logger.info call after await
app_state.persistence.tasks.save(new_task) (or guard it behind a flag) so
transitions are only logged once; specifically update the block that calls
task.with_transition, await app_state.persistence.tasks.save(new_task), and
remove the subsequent logger.info(...) that references TASK_STATUS_CHANGED,
task_id, from_status, and to_status.
- Around line 164-168: The code currently uses task.model_copy(update=updates)
which bypasses Task validators; instead merge the original task data with
updates and re-validate by creating a new validated Task instance via
Task.model_validate(...) (or task.__class__.model_validate(...)) before calling
app_state.persistence.tasks.save(task); ensure you catch/propagate validation
errors appropriately and only call logger.info(API_TASK_UPDATED, ...) after a
successful validation and save.
In `@src/ai_company/api/middleware.py`:
- Around line 52-74: The code mixes two different fallbacks for status_code:
change capture_send so it does not default to 500 (use message.get("status") /
None) and let the existing finally block continue to log 0 when status_code is
None; update the assignment in capture_send (inside function capture_send) to
set status_code to the raw message status or None, keeping the
API_REQUEST_COMPLETED logging logic that writes status_code if not None else 0
so sentinel 0 consistently means "no response sent."
In `@src/ai_company/api/pagination.py`:
- Around line 7-11: This module is missing the required module logger; add the
observability import and logger initialization by inserting "from
ai_company.observability import get_logger" and "logger = get_logger(__name__)"
near the top of the file (above or alongside existing imports that reference
PaginationMeta, MAX_LIMIT, or Parameter) so the module that defines
PaginationMeta/MAX_LIMIT logging follows project convention.
In `@src/ai_company/api/server.py`:
- Around line 50-58: The current call to uvicorn.run(app, ...) passes an app
instance returned by create_app(config=config), which will fail when
server.workers > 1 because Uvicorn needs an import string for subprocesses;
update run_server (or the block using create_app and uvicorn.run) to either
enforce server.workers == 1 (raise or clamp and log a clear error if >1) or
accept/construct an import string for the app and pass that string to
uvicorn.run when server.workers > 1; reference create_app and the uvicorn.run
invocation so the change can be made where the app instance is created and
passed to uvicorn.run.
In `@src/ai_company/core/approval.py`:
- Around line 57-58: The current validation accepts whitespace-only strings for
decision_reason (and the similar field around lines 78-80); update the
validation/assignment for decision_reason (and the other checked field) to first
strip whitespace and then verify non-emptiness (e.g., normalize by stripping on
assignment or in the class validator and reject if the stripped string is empty)
rather than using a simple truthiness check; apply the same strip-and-check
logic to any other checks that currently use constructs like "if not
self.decision_reason" so whitespace-only values are rejected.
In `@tests/unit/api/controllers/test_approvals.py`:
- Around line 237-242: The test test_create_blocks_no_role currently relies on
test_client which has a default header "X-Human-Role" seeded in conftest, so to
actually test the missing-header path update test_create_blocks_no_role to send
the POST with an explicit headers argument that omits the role (e.g., an empty
headers dict or a copy of test_client.headers with "X-Human-Role" removed) so
the request truly has no role header; ensure the change is applied where _BASE
and _create_payload() are used so the assertion resp.status_code == 403
validates the missing-header scenario.
In `@tests/unit/api/controllers/test_messages.py`:
- Around line 17-22: Update the test_list_channels test to assert the actual
channel names returned rather than just the list type: after calling the
endpoint "/api/v1/messages/channels" and parsing body, compare body["data"] to
the expected channel set (e.g., the ALL_CHANNELS constant) or assert that the
returned list contains the expected channel names; use the test function
test_list_channels and the response body["data"] for the assertion so the test
fails if channel wiring changes.
In `@tests/unit/api/test_app.py`:
- Around line 29-33: Extend the existing test_openapi_schema_accessible to also
request the interactive docs route (e.g., perform a GET on "/docs/api" using the
TestClient) and assert it returns HTTP 200 and an HTML response that contains a
marker for the interactive UI (for example check Content-Type includes
"text/html" and that the body contains the API title or a Swagger/Redoc
identifier); update the test to use test_client (TestClient[Any]) and keep
assertions alongside the existing openapi.json checks so the
ScalarRenderPlugin(path="/api") wiring is exercised.
In `@tests/unit/api/test_channels.py`:
- Around line 30-32: The test test_create_channels_plugin should assert the
plugin's contract, not just construction: update it to verify that
create_channels_plugin() registers the expected ALL_CHANNELS and does not allow
arbitrary channel creation. Locate create_channels_plugin and assert the
plugin's registered channel set equals ALL_CHANNELS (or contains all members of
ALL_CHANNELS) and assert whatever flag or behavior prevents arbitrary channels
(e.g., plugin.allow_arbitrary_channel creation is False or
plugin.register/lookup rejects unknown channel names) so the test will fail if
registration or the restriction is removed.
In `@tests/unit/api/test_config.py`:
- Around line 35-38: Replace the loop in test_rate_limit_time_unit_values with a
pytest parameterized test: use `@pytest.mark.parametrize` to iterate over
RateLimitTimeUnit values and pass each into the test function, then instantiate
RateLimitConfig(time_unit=unit) and assert rl.time_unit == unit; update the test
name if desired and reference the existing test_rate_limit_time_unit_values,
RateLimitTimeUnit, and RateLimitConfig identifiers when making the change.
In `@tests/unit/api/test_guards.py`:
- Around line 11-52: Replace the three nearly-identical tests
(test_write_guard_allows_ceo, test_write_guard_blocks_observer,
test_write_guard_blocks_missing_role) with a single parametrized test using
`@pytest.mark.parametrize` that iterates over (headers, expected_status); reuse
the common JSON payload and the same POST to "/api/v1/tasks" via the test_client
fixture, asserting response.status_code == expected_status for each case; ensure
the param list includes ({"X-Human-Role":"ceo"},201),
({"X-Human-Role":"observer"},403), and (None,403) (or {} for missing headers) so
test semantics remain identical.
- Around line 39-52: The test test_write_guard_blocks_missing_role currently
inherits the default X-Human-Role header from the shared TestClient, so it never
exercises the missing-role branch; before making the POST call in
test_write_guard_blocks_missing_role remove or clear the default role header on
the TestClient (e.g., delete or pop "X-Human-Role" from test_client.headers or
explicitly set an empty headers dict for this request) so the request has no
role header and the missing-role (403) path is tested.
In `@tests/unit/api/test_middleware.py`:
- Around line 9-19: Add assertions that RequestLoggingMiddleware actually emits
logs: update the tests (e.g., test_request_completes_with_status and
test_not_found_returns_correct_status) to use a log-capture fixture (such as
caplog) and assert that expected log messages or levels appear when invoking
test_client.get("/api/v1/health") and "/api/v1/agents/nonexistent"; reference
the RequestLoggingMiddleware behavior by checking for its characteristic log
text or logger name and assert at least one INFO/ERROR entry is emitted to
ensure the middleware’s logging side-effect is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3cbf3944-1260-40a0-85eb-3193ad9df420
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
CLAUDE.mdDESIGN_SPEC.mdREADME.mdpyproject.tomlsrc/ai_company/api/__init__.pysrc/ai_company/api/app.pysrc/ai_company/api/approval_store.pysrc/ai_company/api/bus_bridge.pysrc/ai_company/api/channels.pysrc/ai_company/api/config.pysrc/ai_company/api/controllers/__init__.pysrc/ai_company/api/controllers/agents.pysrc/ai_company/api/controllers/analytics.pysrc/ai_company/api/controllers/approvals.pysrc/ai_company/api/controllers/artifacts.pysrc/ai_company/api/controllers/budget.pysrc/ai_company/api/controllers/company.pysrc/ai_company/api/controllers/departments.pysrc/ai_company/api/controllers/health.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/controllers/messages.pysrc/ai_company/api/controllers/projects.pysrc/ai_company/api/controllers/providers.pysrc/ai_company/api/controllers/tasks.pysrc/ai_company/api/controllers/ws.pysrc/ai_company/api/dto.pysrc/ai_company/api/errors.pysrc/ai_company/api/exception_handlers.pysrc/ai_company/api/guards.pysrc/ai_company/api/middleware.pysrc/ai_company/api/pagination.pysrc/ai_company/api/server.pysrc/ai_company/api/state.pysrc/ai_company/api/ws_models.pysrc/ai_company/config/defaults.pysrc/ai_company/config/schema.pysrc/ai_company/core/__init__.pysrc/ai_company/core/approval.pysrc/ai_company/core/enums.pysrc/ai_company/observability/events/api.pysrc/ai_company/observability/events/task.pytests/unit/api/__init__.pytests/unit/api/conftest.pytests/unit/api/controllers/__init__.pytests/unit/api/controllers/test_agents.pytests/unit/api/controllers/test_analytics.pytests/unit/api/controllers/test_approvals.pytests/unit/api/controllers/test_artifacts.pytests/unit/api/controllers/test_budget.pytests/unit/api/controllers/test_company.pytests/unit/api/controllers/test_departments.pytests/unit/api/controllers/test_meetings.pytests/unit/api/controllers/test_messages.pytests/unit/api/controllers/test_projects.pytests/unit/api/controllers/test_providers.pytests/unit/api/controllers/test_tasks.pytests/unit/api/controllers/test_ws.pytests/unit/api/test_app.pytests/unit/api/test_approval_store.pytests/unit/api/test_bus_bridge.pytests/unit/api/test_channels.pytests/unit/api/test_config.pytests/unit/api/test_exception_handlers.pytests/unit/api/test_guards.pytests/unit/api/test_health.pytests/unit/api/test_middleware.pytests/unit/api/test_ws_models.pytests/unit/core/test_approval.pytests/unit/observability/test_events.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). (2)
- GitHub Check: Agent
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649
Use PEP 758 except syntax:except A, B:(no parentheses) — ruff enforces this on Python 3.14
Type hints: all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement on non-Pydantic internal collections (registries,BaseTool)
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves
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)
UseNotBlankStr(fromcore.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants — instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Line length: 88 characters (ruff)
Functions: < 50 lines, files < 800 lines
Handle errors explicitly, never silently swallow
Validate at system boundaries (user input, external APIs, config files)
Files:
tests/unit/api/controllers/test_providers.pysrc/ai_company/api/controllers/projects.pysrc/ai_company/api/controllers/departments.pysrc/ai_company/api/guards.pysrc/ai_company/api/middleware.pytests/unit/api/controllers/test_company.pysrc/ai_company/api/controllers/company.pytests/unit/api/controllers/test_artifacts.pysrc/ai_company/api/controllers/health.pysrc/ai_company/api/controllers/providers.pysrc/ai_company/api/controllers/agents.pysrc/ai_company/api/server.pytests/unit/core/test_approval.pysrc/ai_company/api/controllers/artifacts.pytests/unit/api/test_config.pysrc/ai_company/api/state.pytests/unit/api/controllers/test_departments.pysrc/ai_company/api/channels.pytests/unit/api/test_approval_store.pysrc/ai_company/api/controllers/messages.pysrc/ai_company/api/controllers/analytics.pysrc/ai_company/config/schema.pysrc/ai_company/api/exception_handlers.pytests/unit/api/test_app.pytests/unit/api/controllers/test_ws.pytests/unit/api/controllers/test_messages.pysrc/ai_company/api/pagination.pytests/unit/api/controllers/test_tasks.pytests/unit/api/controllers/test_meetings.pysrc/ai_company/api/errors.pysrc/ai_company/api/ws_models.pysrc/ai_company/core/approval.pysrc/ai_company/api/controllers/ws.pysrc/ai_company/core/enums.pytests/unit/observability/test_events.pytests/unit/api/test_middleware.pysrc/ai_company/api/bus_bridge.pysrc/ai_company/api/__init__.pytests/unit/api/controllers/test_analytics.pytests/unit/api/controllers/test_agents.pytests/unit/api/conftest.pysrc/ai_company/config/defaults.pytests/unit/api/test_channels.pysrc/ai_company/api/app.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/controllers/approvals.pysrc/ai_company/api/config.pysrc/ai_company/api/approval_store.pytests/unit/api/test_guards.pytests/unit/api/test_ws_models.pytests/unit/api/test_exception_handlers.pytests/unit/api/controllers/test_projects.pytests/unit/api/test_bus_bridge.pytests/unit/api/test_health.pysrc/ai_company/api/controllers/__init__.pytests/unit/api/controllers/test_budget.pysrc/ai_company/api/controllers/tasks.pysrc/ai_company/observability/events/task.pysrc/ai_company/api/dto.pysrc/ai_company/observability/events/api.pysrc/ai_company/api/controllers/budget.pysrc/ai_company/core/__init__.pytests/unit/api/controllers/test_approvals.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use markers@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test classification
Maintain 80% minimum test coverage (enforced in CI)
Useasyncio_mode = "auto"for async tests — no manual@pytest.mark.asyncioneeded
30 second timeout per test
Prefer@pytest.mark.parametrizefor testing similar cases
Files:
tests/unit/api/controllers/test_providers.pytests/unit/api/controllers/test_company.pytests/unit/api/controllers/test_artifacts.pytests/unit/core/test_approval.pytests/unit/api/test_config.pytests/unit/api/controllers/test_departments.pytests/unit/api/test_approval_store.pytests/unit/api/test_app.pytests/unit/api/controllers/test_ws.pytests/unit/api/controllers/test_messages.pytests/unit/api/controllers/test_tasks.pytests/unit/api/controllers/test_meetings.pytests/unit/observability/test_events.pytests/unit/api/test_middleware.pytests/unit/api/controllers/test_analytics.pytests/unit/api/controllers/test_agents.pytests/unit/api/conftest.pytests/unit/api/test_channels.pytests/unit/api/test_guards.pytests/unit/api/test_ws_models.pytests/unit/api/test_exception_handlers.pytests/unit/api/controllers/test_projects.pytests/unit/api/test_bus_bridge.pytests/unit/api/test_health.pytests/unit/api/controllers/test_budget.pytests/unit/api/controllers/test_approvals.py
+(src|tests)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
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/smallas aliases; vendor names may only appear in: (1) DESIGN_SPEC.md provider list, (2).claude/skill/agent files, (3) third-party import paths/module names; tests must usetest-provider,test-small-001, etc.
Files:
tests/unit/api/controllers/test_providers.pysrc/ai_company/api/controllers/projects.pysrc/ai_company/api/controllers/departments.pysrc/ai_company/api/guards.pysrc/ai_company/api/middleware.pytests/unit/api/controllers/test_company.pysrc/ai_company/api/controllers/company.pytests/unit/api/controllers/test_artifacts.pysrc/ai_company/api/controllers/health.pysrc/ai_company/api/controllers/providers.pysrc/ai_company/api/controllers/agents.pysrc/ai_company/api/server.pytests/unit/core/test_approval.pysrc/ai_company/api/controllers/artifacts.pytests/unit/api/test_config.pysrc/ai_company/api/state.pytests/unit/api/controllers/test_departments.pysrc/ai_company/api/channels.pytests/unit/api/test_approval_store.pysrc/ai_company/api/controllers/messages.pysrc/ai_company/api/controllers/analytics.pysrc/ai_company/config/schema.pysrc/ai_company/api/exception_handlers.pytests/unit/api/test_app.pytests/unit/api/controllers/test_ws.pytests/unit/api/controllers/test_messages.pysrc/ai_company/api/pagination.pytests/unit/api/controllers/test_tasks.pytests/unit/api/controllers/test_meetings.pysrc/ai_company/api/errors.pysrc/ai_company/api/ws_models.pysrc/ai_company/core/approval.pysrc/ai_company/api/controllers/ws.pysrc/ai_company/core/enums.pytests/unit/observability/test_events.pytests/unit/api/test_middleware.pysrc/ai_company/api/bus_bridge.pysrc/ai_company/api/__init__.pytests/unit/api/controllers/test_analytics.pytests/unit/api/controllers/test_agents.pytests/unit/api/conftest.pysrc/ai_company/config/defaults.pytests/unit/api/test_channels.pysrc/ai_company/api/app.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/controllers/approvals.pysrc/ai_company/api/config.pysrc/ai_company/api/approval_store.pytests/unit/api/test_guards.pytests/unit/api/test_ws_models.pytests/unit/api/test_exception_handlers.pytests/unit/api/controllers/test_projects.pytests/unit/api/test_bus_bridge.pytests/unit/api/test_health.pysrc/ai_company/api/controllers/__init__.pytests/unit/api/controllers/test_budget.pysrc/ai_company/api/controllers/tasks.pysrc/ai_company/observability/events/task.pysrc/ai_company/api/dto.pysrc/ai_company/observability/events/api.pysrc/ai_company/api/controllers/budget.pysrc/ai_company/core/__init__.pytests/unit/api/controllers/test_approvals.py
src/ai_company/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/ai_company/**/*.py: Every module with business logic MUST have:from ai_company.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code
Variable name: alwayslogger(not_logger, notlog)
Event names: always use constants from the domain-specific module underai_company.observability.events(e.g.PROVIDER_CALL_STARTfromevents.provider,BUDGET_RECORD_ADDEDfromevents.budget,CFO_ANOMALY_DETECTEDfromevents.cfo,CONFLICT_DETECTEDfromevents.conflict,MEETING_STARTEDfromevents.meeting,CLASSIFICATION_STARTfromevents.classification,CONSOLIDATION_STARTfromevents.consolidation,ORG_MEMORY_QUERY_STARTfromevents.org_memory)
Structured kwargs: alwayslogger.info(EVENT, key=value)— neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO
DEBUG logging for object creation, internal flow, entry/exit of key functions
Files:
src/ai_company/api/controllers/projects.pysrc/ai_company/api/controllers/departments.pysrc/ai_company/api/guards.pysrc/ai_company/api/middleware.pysrc/ai_company/api/controllers/company.pysrc/ai_company/api/controllers/health.pysrc/ai_company/api/controllers/providers.pysrc/ai_company/api/controllers/agents.pysrc/ai_company/api/server.pysrc/ai_company/api/controllers/artifacts.pysrc/ai_company/api/state.pysrc/ai_company/api/channels.pysrc/ai_company/api/controllers/messages.pysrc/ai_company/api/controllers/analytics.pysrc/ai_company/config/schema.pysrc/ai_company/api/exception_handlers.pysrc/ai_company/api/pagination.pysrc/ai_company/api/errors.pysrc/ai_company/api/ws_models.pysrc/ai_company/core/approval.pysrc/ai_company/api/controllers/ws.pysrc/ai_company/core/enums.pysrc/ai_company/api/bus_bridge.pysrc/ai_company/api/__init__.pysrc/ai_company/config/defaults.pysrc/ai_company/api/app.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/controllers/approvals.pysrc/ai_company/api/config.pysrc/ai_company/api/approval_store.pysrc/ai_company/api/controllers/__init__.pysrc/ai_company/api/controllers/tasks.pysrc/ai_company/observability/events/task.pysrc/ai_company/api/dto.pysrc/ai_company/observability/events/api.pysrc/ai_company/api/controllers/budget.pysrc/ai_company/core/__init__.py
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: All versions use==inpyproject.tomlfor pinned dependencies
Dependency groups:test(pytest + plugins),dev(includes test + ruff, mypy, pre-commit, commitizen); install withuv sync
Files:
pyproject.toml
🧠 Learnings (8)
📚 Learning: 2026-03-09T15:40:36.937Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T15:40:36.937Z
Learning: Applies to src/ai_company/**/*.py : Event names: always use constants from the domain-specific module under `ai_company.observability.events` (e.g. `PROVIDER_CALL_START` from `events.provider`, `BUDGET_RECORD_ADDED` from `events.budget`, `CFO_ANOMALY_DETECTED` from `events.cfo`, `CONFLICT_DETECTED` from `events.conflict`, `MEETING_STARTED` from `events.meeting`, `CLASSIFICATION_START` from `events.classification`, `CONSOLIDATION_START` from `events.consolidation`, `ORG_MEMORY_QUERY_START` from `events.org_memory`)
Applied to files:
CLAUDE.mdsrc/ai_company/api/channels.pysrc/ai_company/api/ws_models.pytests/unit/observability/test_events.pysrc/ai_company/observability/events/task.pysrc/ai_company/observability/events/api.pysrc/ai_company/core/__init__.py
📚 Learning: 2026-03-09T15:40:36.937Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T15:40:36.937Z
Learning: Applies to src/ai_company/**/*.py : Every module with business logic MUST have: `from ai_company.observability import get_logger` then `logger = get_logger(__name__)`
Applied to files:
CLAUDE.mdsrc/ai_company/observability/events/api.py
📚 Learning: 2026-03-09T15:40:36.937Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T15:40:36.937Z
Learning: Applies to src/ai_company/providers/**/*.py : `RetryConfig` and `RateLimiterConfig` are set per-provider in `ProviderConfig`
Applied to files:
src/ai_company/api/controllers/providers.pysrc/ai_company/api/config.py
📚 Learning: 2026-03-09T15:40:36.937Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T15:40:36.937Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`)
Applied to files:
src/ai_company/config/schema.py
📚 Learning: 2026-03-09T15:40:36.937Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T15:40:36.937Z
Learning: Applies to src/ai_company/providers/**/*.py : Retryable errors (`is_retryable=True`): `RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`
Applied to files:
src/ai_company/api/errors.py
📚 Learning: 2026-03-09T15:40:36.937Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T15:40:36.937Z
Learning: Always read `DESIGN_SPEC.md` before implementing any feature or planning any issue; the design spec is the starting point for architecture, data models, and behavior
Applied to files:
DESIGN_SPEC.md
📚 Learning: 2026-03-09T15:40:36.937Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T15:40:36.937Z
Learning: When approved deviations occur, update `DESIGN_SPEC.md` to reflect the new reality
Applied to files:
DESIGN_SPEC.md
📚 Learning: 2026-03-09T15:40:36.937Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T15:40:36.937Z
Learning: Applies to **/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/ai_company/api/config.py
🧬 Code graph analysis (44)
tests/unit/api/controllers/test_providers.py (1)
tests/unit/api/conftest.py (1)
test_client(258-274)
src/ai_company/api/controllers/projects.py (3)
src/ai_company/api/dto.py (2)
ApiResponse(31-59)PaginatedResponse(78-108)src/ai_company/api/guards.py (1)
require_read_access(83-107)src/ai_company/api/pagination.py (1)
paginate(26-53)
src/ai_company/api/controllers/departments.py (5)
src/ai_company/api/dto.py (2)
ApiResponse(31-59)PaginatedResponse(78-108)src/ai_company/api/errors.py (1)
NotFoundError(23-29)src/ai_company/api/pagination.py (1)
paginate(26-53)src/ai_company/api/state.py (1)
AppState(20-95)src/ai_company/core/company.py (1)
Department(257-322)
src/ai_company/api/guards.py (1)
src/ai_company/observability/_logger.py (1)
get_logger(8-28)
src/ai_company/api/middleware.py (1)
src/ai_company/observability/_logger.py (1)
get_logger(8-28)
tests/unit/api/controllers/test_company.py (1)
tests/unit/api/conftest.py (1)
test_client(258-274)
src/ai_company/api/controllers/company.py (5)
src/ai_company/api/dto.py (1)
ApiResponse(31-59)src/ai_company/api/guards.py (1)
require_read_access(83-107)src/ai_company/api/state.py (1)
AppState(20-95)src/ai_company/core/company.py (1)
Department(257-322)src/ai_company/observability/_logger.py (1)
get_logger(8-28)
tests/unit/api/controllers/test_artifacts.py (1)
tests/unit/api/conftest.py (2)
test_client(258-274)get(37-38)
src/ai_company/api/controllers/health.py (3)
src/ai_company/api/dto.py (1)
ApiResponse(31-59)src/ai_company/api/state.py (3)
AppState(20-95)persistence(62-71)message_bus(74-83)src/ai_company/observability/_logger.py (1)
get_logger(8-28)
src/ai_company/api/controllers/providers.py (4)
src/ai_company/api/dto.py (1)
ApiResponse(31-59)src/ai_company/api/errors.py (1)
NotFoundError(23-29)src/ai_company/api/guards.py (1)
require_read_access(83-107)src/ai_company/config/schema.py (2)
ProviderConfig(163-236)ProviderModelConfig(121-160)
src/ai_company/api/controllers/agents.py (5)
src/ai_company/api/dto.py (2)
ApiResponse(31-59)PaginatedResponse(78-108)src/ai_company/api/errors.py (1)
NotFoundError(23-29)src/ai_company/api/pagination.py (1)
paginate(26-53)src/ai_company/api/state.py (1)
AppState(20-95)src/ai_company/config/schema.py (1)
AgentConfig(312-361)
src/ai_company/api/server.py (3)
src/ai_company/api/app.py (1)
create_app(191-304)src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/config/schema.py (1)
RootConfig(462-686)
tests/unit/core/test_approval.py (2)
src/ai_company/core/approval.py (1)
ApprovalItem(24-96)src/ai_company/core/enums.py (2)
ApprovalRiskLevel(418-424)ApprovalStatus(409-415)
src/ai_company/api/controllers/artifacts.py (3)
src/ai_company/api/dto.py (2)
ApiResponse(31-59)PaginatedResponse(78-108)src/ai_company/api/guards.py (1)
require_read_access(83-107)src/ai_company/api/pagination.py (1)
paginate(26-53)
tests/unit/api/test_config.py (1)
src/ai_company/api/config.py (4)
CorsConfig(15-58)RateLimitConfig(70-96)RateLimitTimeUnit(61-67)ServerConfig(99-143)
tests/unit/api/controllers/test_departments.py (1)
tests/unit/api/conftest.py (1)
test_client(258-274)
src/ai_company/api/controllers/messages.py (7)
tests/unit/api/conftest.py (4)
get(37-38)messages(154-155)get_history(101-110)list_channels(213-214)src/ai_company/api/dto.py (2)
ApiResponse(31-59)PaginatedResponse(78-108)src/ai_company/api/guards.py (1)
require_read_access(83-107)src/ai_company/api/pagination.py (1)
paginate(26-53)src/ai_company/api/state.py (3)
AppState(20-95)persistence(62-71)message_bus(74-83)src/ai_company/communication/channel.py (1)
Channel(14-39)src/ai_company/communication/message.py (1)
Message(88-138)
src/ai_company/api/controllers/analytics.py (7)
src/ai_company/api/dto.py (1)
ApiResponse(31-59)src/ai_company/api/guards.py (1)
require_read_access(83-107)src/ai_company/api/state.py (3)
AppState(20-95)persistence(62-71)cost_tracker(86-95)src/ai_company/core/enums.py (1)
TaskStatus(197-223)src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/api/controllers/tasks.py (1)
list_tasks(43-72)src/ai_company/budget/tracker.py (1)
get_total_cost(114-137)
src/ai_company/config/schema.py (1)
src/ai_company/api/config.py (1)
ApiConfig(146-173)
src/ai_company/api/exception_handlers.py (4)
src/ai_company/api/dto.py (1)
ApiResponse(31-59)src/ai_company/api/errors.py (1)
ApiError(8-20)src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/persistence/errors.py (3)
DuplicateRecordError(29-30)PersistenceError(8-9)RecordNotFoundError(20-26)
tests/unit/api/test_app.py (2)
src/ai_company/api/app.py (1)
create_app(191-304)src/ai_company/api/state.py (3)
cost_tracker(86-95)persistence(62-71)message_bus(74-83)
tests/unit/api/controllers/test_ws.py (1)
src/ai_company/api/controllers/ws.py (1)
_handle_message(123-185)
tests/unit/api/controllers/test_messages.py (1)
tests/unit/api/conftest.py (1)
test_client(258-274)
src/ai_company/api/pagination.py (1)
src/ai_company/api/dto.py (1)
PaginationMeta(62-75)
src/ai_company/core/approval.py (1)
src/ai_company/core/enums.py (2)
ApprovalRiskLevel(418-424)ApprovalStatus(409-415)
src/ai_company/api/controllers/ws.py (2)
src/ai_company/api/guards.py (1)
require_read_access(83-107)src/ai_company/observability/_logger.py (1)
get_logger(8-28)
tests/unit/api/test_middleware.py (1)
tests/unit/api/conftest.py (2)
test_client(258-274)get(37-38)
src/ai_company/api/bus_bridge.py (4)
src/ai_company/api/ws_models.py (2)
WsEvent(47-72)WsEventType(20-44)src/ai_company/communication/bus_protocol.py (1)
MessageBus(20-209)src/ai_company/communication/message.py (1)
Message(88-138)src/ai_company/api/state.py (1)
message_bus(74-83)
tests/unit/api/controllers/test_analytics.py (1)
src/ai_company/core/enums.py (1)
TaskStatus(197-223)
tests/unit/api/test_channels.py (1)
src/ai_company/api/channels.py (1)
create_channels_plugin(27-38)
src/ai_company/api/controllers/meetings.py (3)
src/ai_company/api/dto.py (2)
ApiResponse(31-59)PaginatedResponse(78-108)src/ai_company/api/guards.py (1)
require_read_access(83-107)src/ai_company/api/pagination.py (1)
paginate(26-53)
src/ai_company/api/approval_store.py (4)
src/ai_company/api/errors.py (1)
ConflictError(41-47)src/ai_company/core/approval.py (1)
ApprovalItem(24-96)src/ai_company/core/enums.py (2)
ApprovalRiskLevel(418-424)ApprovalStatus(409-415)src/ai_company/observability/_logger.py (1)
get_logger(8-28)
tests/unit/api/test_guards.py (1)
tests/unit/api/conftest.py (1)
test_client(258-274)
tests/unit/api/test_ws_models.py (1)
src/ai_company/api/ws_models.py (2)
WsEvent(47-72)WsEventType(20-44)
tests/unit/api/controllers/test_projects.py (1)
tests/unit/api/conftest.py (1)
test_client(258-274)
tests/unit/api/test_bus_bridge.py (3)
src/ai_company/api/bus_bridge.py (1)
_to_ws_event(146-159)src/ai_company/api/ws_models.py (1)
WsEventType(20-44)src/ai_company/communication/message.py (1)
Message(88-138)
tests/unit/api/test_health.py (1)
tests/unit/api/conftest.py (4)
test_client(258-274)get(37-38)fake_message_bus(236-239)fake_persistence(229-232)
src/ai_company/api/controllers/__init__.py (13)
src/ai_company/api/controllers/agents.py (1)
AgentController(18-74)src/ai_company/api/controllers/analytics.py (1)
AnalyticsController(38-72)src/ai_company/api/controllers/artifacts.py (1)
ArtifactController(11-64)src/ai_company/api/controllers/budget.py (1)
BudgetController(33-107)src/ai_company/api/controllers/company.py (1)
CompanyController(17-63)src/ai_company/api/controllers/departments.py (1)
DepartmentController(18-74)src/ai_company/api/controllers/health.py (1)
HealthController(53-118)src/ai_company/api/controllers/meetings.py (1)
MeetingController(11-64)src/ai_company/api/controllers/messages.py (1)
MessageController(17-72)src/ai_company/api/controllers/projects.py (1)
ProjectController(11-64)src/ai_company/api/controllers/providers.py (1)
ProviderController(25-101)src/ai_company/api/controllers/tasks.py (1)
TaskController(35-243)src/ai_company/api/controllers/ws.py (1)
ws_handler(39-99)
tests/unit/api/controllers/test_budget.py (3)
src/ai_company/budget/cost_record.py (1)
CostRecord(15-56)src/ai_company/budget/tracker.py (1)
CostTracker(68-455)src/ai_company/api/state.py (1)
cost_tracker(86-95)
src/ai_company/api/controllers/tasks.py (5)
src/ai_company/api/dto.py (5)
ApiResponse(31-59)CreateTaskRequest(114-139)PaginatedResponse(78-108)TransitionTaskRequest(164-175)UpdateTaskRequest(142-161)src/ai_company/api/errors.py (2)
ApiValidationError(32-38)NotFoundError(23-29)src/ai_company/api/pagination.py (1)
paginate(26-53)src/ai_company/core/enums.py (1)
TaskStatus(197-223)src/ai_company/core/task.py (1)
Task(45-261)
src/ai_company/api/dto.py (1)
src/ai_company/core/enums.py (5)
ApprovalRiskLevel(418-424)Complexity(246-252)Priority(237-243)TaskStatus(197-223)TaskType(226-234)
src/ai_company/api/controllers/budget.py (6)
src/ai_company/api/dto.py (2)
ApiResponse(31-59)PaginatedResponse(78-108)src/ai_company/api/pagination.py (1)
paginate(26-53)src/ai_company/api/state.py (2)
AppState(20-95)cost_tracker(86-95)src/ai_company/budget/config.py (1)
BudgetConfig(151-227)src/ai_company/budget/cost_record.py (1)
CostRecord(15-56)src/ai_company/budget/tracker.py (1)
get_agent_cost(139-174)
src/ai_company/core/__init__.py (2)
src/ai_company/core/approval.py (1)
ApprovalItem(24-96)src/ai_company/core/enums.py (2)
ApprovalRiskLevel(418-424)ApprovalStatus(409-415)
tests/unit/api/controllers/test_approvals.py (4)
tests/unit/api/conftest.py (4)
approval_store(248-249)make_approval(309-337)test_client(258-274)get(37-38)src/ai_company/api/approval_store.py (3)
ApprovalStore(22-136)add(32-49)get(51-63)src/ai_company/core/approval.py (1)
ApprovalItem(24-96)src/ai_company/core/enums.py (2)
ApprovalRiskLevel(418-424)ApprovalStatus(409-415)
🪛 LanguageTool
README.md
[typographical] ~27-~27: To join two clauses or introduce examples, consider using an em dash.
Context: ...ing reports - Litestar REST API (M6) - 14 controllers covering company, agents,...
(DASH_RULE)
[typographical] ~28-~28: To join two clauses or introduce examples, consider using an em dash.
Context: ...ime feed - Human Approval Queue (M6) - Approval submission, approve/reject with...
(DASH_RULE)
[typographical] ~29-~29: To join two clauses or introduce examples, consider using an em dash.
Context: ...ents - WebSocket Real-Time Feed (M6) - Channel-based subscriptions (tasks, agen...
(DASH_RULE)
[typographical] ~30-~30: To join two clauses or introduce examples, consider using an em dash.
Context: ...ssage-bus bridge - Route Guards (M6) - Role-based read/write access control (st...
(DASH_RULE)
Comment on lines
206
to
209
| filterwarnings = [ | ||
| "error", | ||
| "ignore:Core Pydantic V1 functionality:UserWarning", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find project-owned Pydantic V1 APIs that the global warning filter would hide.
rg -n --glob 'src/**/*.py' --glob 'tests/**/*.py' \
-e '\bpydantic\.v1\b' \
-e 'from pydantic\.v1 import' \
-e '\broot_validator\s*\(' \
-e '\bvalidator\s*\(' \
-e '\bparse_obj_as\s*\('Repository: Aureliolo/ai-company
Length of output: 46
🏁 Script executed:
grep -n "pydantic" pyproject.tomlRepository: Aureliolo/ai-company
Length of output: 221
🏁 Script executed:
fd --type f --extension py | head -20 | xargs grep -l "BaseModel\|model_validator\|ConfigDict" | head -10Repository: Aureliolo/ai-company
Length of output: 218
🏁 Script executed:
grep -n "litestar" pyproject.tomlRepository: Aureliolo/ai-company
Length of output: 163
🏁 Script executed:
# Check git context to see if there's a reason for this filter
git log --oneline --all -- pyproject.toml | head -10Repository: Aureliolo/ai-company
Length of output: 781
🏁 Script executed:
# Search for any comments or notes about why Pydantic V1 warnings are being suppressed
rg -n "Pydantic\s*V1|pydantic.*v1" pyproject.tomlRepository: Aureliolo/ai-company
Length of output: 124
🏁 Script executed:
# Check if any code comments reference this warning suppression
rg -n "Core Pydantic V1 functionality" .Repository: Aureliolo/ai-company
Length of output: 141
Scope the Pydantic V1 warning filter to the specific third-party emitter.
Line 208 suppresses Core Pydantic V1 functionality warnings globally. While the project is fully Pydantic v2 compliant (verified in code), suppressing warnings at the test level masks the signal for any future regressions. Rather than "ignore:Core Pydantic V1 functionality:UserWarning", scope this to the specific library emitting the warning (likely Litestar), for example: "ignore:Core Pydantic V1 functionality:UserWarning:litestar" or add filterwarnings only for specific affected tests if warnings originate from Litestar's own code path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` around lines 206 - 209, The global warning filter entry
"ignore:Core Pydantic V1 functionality:UserWarning" should be narrowed to the
specific emitter to avoid masking future regressions; update the filterwarnings
list entry to include the third-party module (e.g. replace it with "ignore:Core
Pydantic V1 functionality:UserWarning:litestar") or move a scoped filter to the
specific tests that trigger it so only the Litestar-emitted Pydantic V1 warning
is suppressed.
Comment on lines
+145
to
+159
| @staticmethod | ||
| def _to_ws_event(message: Message, channel_name: str) -> WsEvent: | ||
| """Convert an internal ``Message`` to a ``WsEvent``.""" | ||
| payload: dict[str, Any] = { | ||
| "message_id": str(message.id), | ||
| "sender": message.sender, | ||
| "to": message.to, | ||
| "content": message.content, | ||
| } | ||
| return WsEvent( | ||
| event_type=WsEventType.MESSAGE_SENT, | ||
| channel=channel_name, | ||
| timestamp=datetime.now(UTC), | ||
| payload=payload, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extending _to_ws_event to handle diverse message types.
Currently, all messages are mapped to WsEventType.MESSAGE_SENT. If the internal MessageBus carries messages that represent different semantic events (task updates, agent status changes, etc.), this mapping loses that information.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/api/bus_bridge.py` around lines 145 - 159, The _to_ws_event
function indiscriminately maps every internal Message to
WsEventType.MESSAGE_SENT, losing semantic distinctions; update _to_ws_event to
inspect the Message (e.g., message.type, message.kind, or specific fields like
status/task_id) and map to the appropriate WsEventType (e.g., TASK_UPDATED,
AGENT_STATUS_CHANGED, etc.), falling back to MESSAGE_SENT when unknown, and
include the original message.type in the payload for traceability; ensure you
update the WsEvent construction (event_type and payload) in _to_ws_event and add
any missing enum members to WsEventType if needed.
Comment on lines
+93
to
+96
| exclude_paths: tuple[str, ...] = Field( | ||
| default=("/api/v1/health",), | ||
| description="Paths excluded from rate limiting", | ||
| ) |
There was a problem hiding this comment.
Keep the health-check exclusion aligned with api_prefix.
exclude_paths is hardcoded to "/api/v1/health", but api_prefix is configurable. As soon as the prefix moves, the health endpoint stops being exempt from rate limiting and can start returning 429s to probes.
Also applies to: 170-173
Comment on lines
+30
to
+32
| def test_create_channels_plugin(self) -> None: | ||
| plugin = create_channels_plugin() | ||
| assert plugin is not None |
There was a problem hiding this comment.
Assert the plugin contract, not just construction.
assert plugin is not None will still pass if create_channels_plugin() stops registering ALL_CHANNELS or re-enables arbitrary channel creation. Since src/ai_company/api/channels.py:26-37 hard-codes those behaviors, pin them here so this test actually protects the factory contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/test_channels.py` around lines 30 - 32, The test
test_create_channels_plugin should assert the plugin's contract, not just
construction: update it to verify that create_channels_plugin() registers the
expected ALL_CHANNELS and does not allow arbitrary channel creation. Locate
create_channels_plugin and assert the plugin's registered channel set equals
ALL_CHANNELS (or contains all members of ALL_CHANNELS) and assert whatever flag
or behavior prevents arbitrary channels (e.g., plugin.allow_arbitrary_channel
creation is False or plugin.register/lookup rejects unknown channel names) so
the test will fail if registration or the restriction is removed.
Comment on lines
+35
to
+38
| def test_rate_limit_time_unit_values(self) -> None: | ||
| for unit in RateLimitTimeUnit: | ||
| rl = RateLimitConfig(time_unit=unit) | ||
| assert rl.time_unit == unit |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using @pytest.mark.parametrize for enum iteration.
Per coding guidelines, prefer @pytest.mark.parametrize for testing similar cases. This provides better test isolation and clearer failure reporting.
📝 Parametrized alternative
- def test_rate_limit_time_unit_values(self) -> None:
- for unit in RateLimitTimeUnit:
- rl = RateLimitConfig(time_unit=unit)
- assert rl.time_unit == unit
+ `@pytest.mark.parametrize`("unit", list(RateLimitTimeUnit))
+ def test_rate_limit_time_unit_values(self, unit: RateLimitTimeUnit) -> None:
+ rl = RateLimitConfig(time_unit=unit)
+ assert rl.time_unit == unitAs per coding guidelines: "Prefer @pytest.mark.parametrize for testing similar cases"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/test_config.py` around lines 35 - 38, Replace the loop in
test_rate_limit_time_unit_values with a pytest parameterized test: use
`@pytest.mark.parametrize` to iterate over RateLimitTimeUnit values and pass each
into the test function, then instantiate RateLimitConfig(time_unit=unit) and
assert rl.time_unit == unit; update the test name if desired and reference the
existing test_rate_limit_time_unit_values, RateLimitTimeUnit, and
RateLimitConfig identifiers when making the change.
Comment on lines
+11
to
+52
| def test_write_guard_allows_ceo(self, test_client: TestClient[Any]) -> None: | ||
| response = test_client.post( | ||
| "/api/v1/tasks", | ||
| json={ | ||
| "title": "Test", | ||
| "description": "Test desc", | ||
| "type": "development", | ||
| "project": "proj", | ||
| "created_by": "alice", | ||
| }, | ||
| headers={"X-Human-Role": "ceo"}, | ||
| ) | ||
| assert response.status_code == 201 | ||
|
|
||
| def test_write_guard_blocks_observer(self, test_client: TestClient[Any]) -> None: | ||
| response = test_client.post( | ||
| "/api/v1/tasks", | ||
| json={ | ||
| "title": "Test", | ||
| "description": "Test desc", | ||
| "type": "development", | ||
| "project": "proj", | ||
| "created_by": "alice", | ||
| }, | ||
| headers={"X-Human-Role": "observer"}, | ||
| ) | ||
| assert response.status_code == 403 | ||
|
|
||
| def test_write_guard_blocks_missing_role( | ||
| self, test_client: TestClient[Any] | ||
| ) -> None: | ||
| response = test_client.post( | ||
| "/api/v1/tasks", | ||
| json={ | ||
| "title": "Test", | ||
| "description": "Test desc", | ||
| "type": "development", | ||
| "project": "proj", | ||
| "created_by": "alice", | ||
| }, | ||
| ) | ||
| assert response.status_code == 403 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Parametrize the role matrix.
These tests duplicate the same payload and endpoint three times, which makes it easy for cases to drift. A single parametrized test over (headers, expected_status) will be shorter and easier to extend.
As per coding guidelines, "Prefer @pytest.mark.parametrize for testing similar cases".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/test_guards.py` around lines 11 - 52, Replace the three
nearly-identical tests (test_write_guard_allows_ceo,
test_write_guard_blocks_observer, test_write_guard_blocks_missing_role) with a
single parametrized test using `@pytest.mark.parametrize` that iterates over
(headers, expected_status); reuse the common JSON payload and the same POST to
"/api/v1/tasks" via the test_client fixture, asserting response.status_code ==
expected_status for each case; ensure the param list includes
({"X-Human-Role":"ceo"},201), ({"X-Human-Role":"observer"},403), and (None,403)
(or {} for missing headers) so test semantics remain identical.
Comment on lines
+39
to
+52
| def test_write_guard_blocks_missing_role( | ||
| self, test_client: TestClient[Any] | ||
| ) -> None: | ||
| response = test_client.post( | ||
| "/api/v1/tasks", | ||
| json={ | ||
| "title": "Test", | ||
| "description": "Test desc", | ||
| "type": "development", | ||
| "project": "proj", | ||
| "created_by": "alice", | ||
| }, | ||
| ) | ||
| assert response.status_code == 403 |
There was a problem hiding this comment.
This never reaches the missing-header path.
test_client is preconfigured with X-Human-Role: observer in tests/unit/api/conftest.py:257-273, so this request still carries a role header and only re-tests the observer-denied case. Clear that default header before the request to cover the actual missing-role branch.
Proposed fix
def test_write_guard_blocks_missing_role(
self, test_client: TestClient[Any]
) -> None:
+ test_client.headers.pop("X-Human-Role", None)
response = test_client.post(
"/api/v1/tasks",
json={
"title": "Test",
"description": "Test desc",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/test_guards.py` around lines 39 - 52, The test
test_write_guard_blocks_missing_role currently inherits the default X-Human-Role
header from the shared TestClient, so it never exercises the missing-role
branch; before making the POST call in test_write_guard_blocks_missing_role
remove or clear the default role header on the TestClient (e.g., delete or pop
"X-Human-Role" from test_client.headers or explicitly set an empty headers dict
for this request) so the request has no role header and the missing-role (403)
path is tested.
Comment on lines
+9
to
+19
| @pytest.mark.unit | ||
| class TestRequestLoggingMiddleware: | ||
| def test_request_completes_with_status(self, test_client: TestClient[Any]) -> None: | ||
| response = test_client.get("/api/v1/health") | ||
| assert response.status_code == 200 | ||
|
|
||
| def test_not_found_returns_correct_status( | ||
| self, test_client: TestClient[Any] | ||
| ) -> None: | ||
| response = test_client.get("/api/v1/agents/nonexistent") | ||
| assert response.status_code == 404 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider expanding middleware test coverage.
The tests verify that requests complete with expected status codes, but don't assert on the actual logging behavior of RequestLoggingMiddleware. If the middleware's primary purpose is logging, consider adding assertions that verify log entries are produced (e.g., using a log capture fixture).
If these tests are intentionally scoped to verify the middleware doesn't break request handling, the current implementation is acceptable as a smoke test.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/test_middleware.py` around lines 9 - 19, Add assertions that
RequestLoggingMiddleware actually emits logs: update the tests (e.g.,
test_request_completes_with_status and test_not_found_returns_correct_status) to
use a log-capture fixture (such as caplog) and assert that expected log messages
or levels appear when invoking test_client.get("/api/v1/health") and
"/api/v1/agents/nonexistent"; reference the RequestLoggingMiddleware behavior by
checking for its characteristic log text or logger name and assert at least one
INFO/ERROR entry is emitted to ensure the middleware’s logging side-effect is
covered.
Address 42 review findings from 8 local agents + 4 external reviewers (Greptile, Copilot, Gemini, CodeRabbit). Phase 1 — Code fixes (16 source files): - Convert ApiResponse.success to @computed_field (derived from error) - Fix wrong event constant in ApprovalStore.add (API_APPROVAL_CONFLICT) - Fix dict mutation during iteration in list_items - Add warning log for save() returning None - Add on_expire callback for WS APPROVAL_EXPIRED on lazy expiry - Add type validation for WS channels/filters and _on_event dict check - Fix WS size check: len(data.encode()) vs len(data) - Remove CSP unsafe-inline from Content-Security-Policy - Fix startup warning to say 503 instead of 500 - Refine _cleanup_on_failure docstring - Add zero-channel start guard to MessageBusBridge - Split broad exception in _poll_channel (retriable vs programming) - Add missing logger to artifacts, meetings, projects controllers - Fix status_code=0 sentinel in middleware (log warning for unknown) - Add reload param to uvicorn.run, rewrite server docstring - Fix hardcoded metadata error messages to use constants - Change ApproveRequest.comment to NotBlankStr - Change ApprovalItem.decision_reason to NotBlankStr - Add log before raise in task transition error path - Remove success=False from all ApiResponse constructions Phase 2 — Documentation fixes (3 files): - Update implementation snapshot (Done M0-M6) - Add missing entries: core/approval.py, events/api.py, budget/_optimizer_helpers.py - Remove stale M4 engine entries - Add /health to API surface listing - Fix "14 controllers" wording - Add security/approval.py clarification - Update cli/ directory listing - Add API_REQUEST_STARTED to CLAUDE.md logging examples - Fix README "14 controllers" wording Phase 3 — Test additions (9 test files, 45+ new tests): - New: test_pagination.py, test_state.py, test_dto.py - Extended: test_tasks, test_providers, test_app, test_bus_bridge, test_guards, test_ws
editorconfig 0.17.1 (via jsbeautifier via litestar[standard]) declares Python-2.0.1 in its license metadata. This is the PSF License v2.0.1, a permissive license compatible with the project.
| event.model_dump_json(), | ||
| channels=[CHANNEL_APPROVALS], | ||
| ) | ||
| except RuntimeError, OSError: |
There was a problem hiding this comment.
Python 2-style multi-exception except syntax (SyntaxError in Python 3)
except ExcType1, ExcType2: is Python 2 syntax — in Python 3 it is a SyntaxError because the parser interprets the comma as the old variable-binding separator. This pattern appears in five locations across four files:
src/ai_company/api/app.py:86—except RuntimeError, OSError:src/ai_company/api/controllers/approvals.py:96—except RuntimeError, OSError:src/ai_company/api/bus_bridge.py:76—except OSError, RuntimeError, ConnectionError:src/ai_company/api/controllers/ws.py:66—except json.JSONDecodeError, TypeError:src/ai_company/api/controllers/ws.py:151—except json.JSONDecodeError, TypeError:
Because Python validates syntax at compile time, every affected module will fail to import, making the entire API unloadable. All five must be wrapped in parentheses.
Suggested change
| except RuntimeError, OSError: | |
| except (RuntimeError, OSError): |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/api/app.py
Line: 86
Comment:
**Python 2-style multi-exception `except` syntax (SyntaxError in Python 3)**
`except ExcType1, ExcType2:` is Python 2 syntax — in Python 3 it is a `SyntaxError` because the parser interprets the comma as the old variable-binding separator. This pattern appears in five locations across four files:
- `src/ai_company/api/app.py:86` — `except RuntimeError, OSError:`
- `src/ai_company/api/controllers/approvals.py:96` — `except RuntimeError, OSError:`
- `src/ai_company/api/bus_bridge.py:76` — `except OSError, RuntimeError, ConnectionError:`
- `src/ai_company/api/controllers/ws.py:66` — `except json.JSONDecodeError, TypeError:`
- `src/ai_company/api/controllers/ws.py:151` — `except json.JSONDecodeError, TypeError:`
Because Python validates syntax at compile time, every affected module will fail to import, making the entire API unloadable. All five must be wrapped in parentheses.
```suggestion
except (RuntimeError, OSError):
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+177
to
+181
| valid = [c for c in channels if c in _ALL_CHANNELS_SET] | ||
| subscribed.update(valid) | ||
| for c in valid: | ||
| if client_filters: | ||
| filters[c] = dict(client_filters) |
There was a problem hiding this comment.
Re-subscribing without filters silently preserves stale channel filters
When client_filters is falsy (empty dict), the if client_filters: guard skips the body, so any previously set filter for that channel persists in the filters dict. This means a client that first subscribes to "tasks" with {"agent_id": "abc"} and then re-subscribes to "tasks" with no filters will still only receive task events for agent "abc" — silently, with no error.
A subscribe without explicit filters should clear any prior filter for that channel:
Suggested change
| valid = [c for c in channels if c in _ALL_CHANNELS_SET] | |
| subscribed.update(valid) | |
| for c in valid: | |
| if client_filters: | |
| filters[c] = dict(client_filters) | |
| valid = [c for c in channels if c in _ALL_CHANNELS_SET] | |
| subscribed.update(valid) | |
| for c in valid: | |
| if client_filters: | |
| filters[c] = dict(client_filters) | |
| else: | |
| filters.pop(c, None) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/api/controllers/ws.py
Line: 177-181
Comment:
**Re-subscribing without filters silently preserves stale channel filters**
When `client_filters` is falsy (empty dict), the `if client_filters:` guard skips the body, so any previously set filter for that channel persists in the `filters` dict. This means a client that first subscribes to `"tasks"` with `{"agent_id": "abc"}` and then re-subscribes to `"tasks"` with no filters will *still* only receive task events for agent `"abc"` — silently, with no error.
A subscribe without explicit filters should clear any prior filter for that channel:
```suggestion
valid = [c for c in channels if c in _ALL_CHANNELS_SET]
subscribed.update(valid)
for c in valid:
if client_filters:
filters[c] = dict(client_filters)
else:
filters.pop(c, None)
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Pull request overview
Copilot reviewed 71 out of 74 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+206
to
+213
| new_task = task.with_transition(data.target_status, **overrides) | ||
| except ValueError as exc: | ||
| logger.warning( | ||
| TASK_STATUS_CHANGED, | ||
| task_id=task_id, | ||
| error=str(exc), | ||
| ) | ||
| raise ApiValidationError(str(exc)) from exc |
There was a problem hiding this comment.
In transition_task(), the invalid-transition path logs the TASK_STATUS_CHANGED event, which is also used for successful transitions. This makes observability ambiguous and ignores the existing TASK_TRANSITION_INVALID constant. Log TASK_TRANSITION_INVALID (and reserve TASK_STATUS_CHANGED for the successful transition log) to keep event semantics consistent.
Comment on lines
+275
to
+283
| role = request.headers.get("x-human-role", "unknown") | ||
| now = datetime.now(UTC) | ||
| updated = item.model_copy( | ||
| update={ | ||
| "status": ApprovalStatus.APPROVED, | ||
| "decided_at": now, | ||
| "decided_by": role, | ||
| "decision_reason": data.comment, | ||
| }, |
There was a problem hiding this comment.
decided_by is populated directly from the raw X-Human-Role header here, without the same normalization (strip/lower) that the guards apply. This can lead to inconsistent decided_by values like " CEO " vs "ceo" depending on client formatting. Normalize the header value (or reuse the guard’s role parsing helper) before storing it.
Comment on lines
+358
to
+366
| role = request.headers.get("x-human-role", "unknown") | ||
| now = datetime.now(UTC) | ||
| updated = item.model_copy( | ||
| update={ | ||
| "status": ApprovalStatus.REJECTED, | ||
| "decided_at": now, | ||
| "decided_by": role, | ||
| "decision_reason": data.reason, | ||
| }, |
There was a problem hiding this comment.
Same as approve(): decided_by is taken from the raw X-Human-Role header without normalization, which can produce inconsistent audit fields. Normalize (strip/lower) before storing.
Comment on lines
+20
to
+27
| class WsEventType(StrEnum): | ||
| """Types of real-time WebSocket events.""" | ||
|
|
||
| TASK_CREATED = "task.created" | ||
| TASK_UPDATED = "task.updated" | ||
| TASK_STATUS_CHANGED = "task.status_changed" | ||
| TASK_ASSIGNED = "task.assigned" | ||
|
|
There was a problem hiding this comment.
WsEventType.TASK_STATUS_CHANGED is defined as "task.status_changed", but the rest of the codebase’s task event naming uses dotted segments (e.g. "task.status.changed" in observability.events.task). Consider aligning the WS event type naming with the established dotted convention to avoid confusion and mismatched client-side routing/filtering.
Aureliolo
added a commit
that referenced
this pull request
Mar 10, 2026
🤖 I have created a release *beep* *boop* --- ## [0.1.1](ai-company-v0.1.0...ai-company-v0.1.1) (2026-03-10) ### Features * add autonomy levels and approval timeout policies ([#42](#42), [#126](#126)) ([#197](#197)) ([eecc25a](eecc25a)) * add CFO cost optimization service with anomaly detection, reports, and approval decisions ([#186](#186)) ([a7fa00b](a7fa00b)) * add code quality toolchain (ruff, mypy, pre-commit, dependabot) ([#63](#63)) ([36681a8](36681a8)) * add configurable cost tiers and subscription/quota-aware tracking ([#67](#67)) ([#185](#185)) ([9baedfa](9baedfa)) * add container packaging, Docker Compose, and CI pipeline ([#269](#269)) ([435bdfe](435bdfe)), closes [#267](#267) * add coordination error taxonomy classification pipeline ([#146](#146)) ([#181](#181)) ([70c7480](70c7480)) * add cost-optimized, hierarchical, and auction assignment strategies ([#175](#175)) ([ce924fa](ce924fa)), closes [#173](#173) * add design specification, license, and project setup ([8669a09](8669a09)) * add env var substitution and config file auto-discovery ([#77](#77)) ([7f53832](7f53832)) * add FastestStrategy routing + vendor-agnostic cleanup ([#140](#140)) ([09619cb](09619cb)), closes [#139](#139) * add HR engine and performance tracking ([#45](#45), [#47](#47)) ([#193](#193)) ([2d091ea](2d091ea)) * add issue auto-search and resolution verification to PR review skill ([#119](#119)) ([deecc39](deecc39)) * add memory retrieval, ranking, and context injection pipeline ([#41](#41)) ([873b0aa](873b0aa)) * add pluggable MemoryBackend protocol with models, config, and events ([#180](#180)) ([46cfdd4](46cfdd4)) * add pluggable MemoryBackend protocol with models, config, and events ([#32](#32)) ([46cfdd4](46cfdd4)) * add pluggable PersistenceBackend protocol with SQLite implementation ([#36](#36)) ([f753779](f753779)) * add progressive trust and promotion/demotion subsystems ([#43](#43), [#49](#49)) ([3a87c08](3a87c08)) * add retry handler, rate limiter, and provider resilience ([#100](#100)) ([b890545](b890545)) * add SecOps security agent with rule engine, audit log, and ToolInvoker integration ([#40](#40)) ([83b7b6c](83b7b6c)) * add shared org memory and memory consolidation/archival ([#125](#125), [#48](#48)) ([4a0832b](4a0832b)) * design unified provider interface ([#86](#86)) ([3e23d64](3e23d64)) * expand template presets, rosters, and add inheritance ([#80](#80), [#81](#81), [#84](#84)) ([15a9134](15a9134)) * implement agent runtime state vs immutable config split ([#115](#115)) ([4cb1ca5](4cb1ca5)) * implement AgentEngine core orchestrator ([#11](#11)) ([#143](#143)) ([f2eb73a](f2eb73a)) * implement basic tool system (registry, invocation, results) ([#15](#15)) ([c51068b](c51068b)) * implement built-in file system tools ([#18](#18)) ([325ef98](325ef98)) * implement communication foundation — message bus, dispatcher, and messenger ([#157](#157)) ([8e71bfd](8e71bfd)) * implement company template system with 7 built-in presets ([#85](#85)) ([cbf1496](cbf1496)) * implement conflict resolution protocol ([#122](#122)) ([#166](#166)) ([e03f9f2](e03f9f2)) * implement core entity and role system models ([#69](#69)) ([acf9801](acf9801)) * implement crash recovery with fail-and-reassign strategy ([#149](#149)) ([e6e91ed](e6e91ed)) * implement engine extensions — Plan-and-Execute loop and call categorization ([#134](#134), [#135](#135)) ([#159](#159)) ([9b2699f](9b2699f)) * implement enterprise logging system with structlog ([#73](#73)) ([2f787e5](2f787e5)) * implement graceful shutdown with cooperative timeout strategy ([#130](#130)) ([6592515](6592515)) * implement hierarchical delegation and loop prevention ([#12](#12), [#17](#17)) ([6be60b6](6be60b6)) * implement LiteLLM driver and provider registry ([#88](#88)) ([ae3f18b](ae3f18b)), closes [#4](#4) * implement LLM decomposition strategy and workspace isolation ([#174](#174)) ([aa0eefe](aa0eefe)) * implement meeting protocol system ([#123](#123)) ([ee7caca](ee7caca)) * implement message and communication domain models ([#74](#74)) ([560a5d2](560a5d2)) * implement model routing engine ([#99](#99)) ([d3c250b](d3c250b)) * implement parallel agent execution ([#22](#22)) ([#161](#161)) ([65940b3](65940b3)) * implement per-call cost tracking service ([#7](#7)) ([#102](#102)) ([c4f1f1c](c4f1f1c)) * implement personality injection and system prompt construction ([#105](#105)) ([934dd85](934dd85)) * implement single-task execution lifecycle ([#21](#21)) ([#144](#144)) ([c7e64e4](c7e64e4)) * implement subprocess sandbox for tool execution isolation ([#131](#131)) ([#153](#153)) ([3c8394e](3c8394e)) * implement task assignment subsystem with pluggable strategies ([#172](#172)) ([c7f1b26](c7f1b26)), closes [#26](#26) [#30](#30) * implement task decomposition and routing engine ([#14](#14)) ([9c7fb52](9c7fb52)) * implement Task, Project, Artifact, Budget, and Cost domain models ([#71](#71)) ([81eabf1](81eabf1)) * implement tool permission checking ([#16](#16)) ([833c190](833c190)) * implement YAML config loader with Pydantic validation ([#59](#59)) ([ff3a2ba](ff3a2ba)) * implement YAML config loader with Pydantic validation ([#75](#75)) ([ff3a2ba](ff3a2ba)) * initialize project with uv, hatchling, and src layout ([39005f9](39005f9)) * initialize project with uv, hatchling, and src layout ([#62](#62)) ([39005f9](39005f9)) * Litestar REST API, WebSocket feed, and approval queue (M6) ([#189](#189)) ([29fcd08](29fcd08)) * make TokenUsage.total_tokens a computed field ([#118](#118)) ([c0bab18](c0bab18)), closes [#109](#109) * parallel tool execution in ToolInvoker.invoke_all ([#137](#137)) ([58517ee](58517ee)) * testing framework, CI pipeline, and M0 gap fixes ([#64](#64)) ([f581749](f581749)) * wire all modules into observability system ([#97](#97)) ([f7a0617](f7a0617)) ### Bug Fixes * address Greptile post-merge review findings from PRs [#170](https://github.com/Aureliolo/ai-company/issues/170)-[#175](https://github.com/Aureliolo/ai-company/issues/175) ([#176](#176)) ([c5ca929](c5ca929)) * address post-merge review feedback from PRs [#164](https://github.com/Aureliolo/ai-company/issues/164)-[#167](https://github.com/Aureliolo/ai-company/issues/167) ([#170](#170)) ([3bf897a](3bf897a)), closes [#169](#169) * enforce strict mypy on test files ([#89](#89)) ([aeeff8c](aeeff8c)) * harden Docker sandbox, MCP bridge, and code runner ([#50](#50), [#53](#53)) ([d5e1b6e](d5e1b6e)) * harden git tools security + code quality improvements ([#150](#150)) ([000a325](000a325)) * harden subprocess cleanup, env filtering, and shutdown resilience ([#155](#155)) ([d1fe1fb](d1fe1fb)) * incorporate post-merge feedback + pre-PR review fixes ([#164](#164)) ([c02832a](c02832a)) * pre-PR review fixes for post-merge findings ([#183](#183)) ([26b3108](26b3108)) * strengthen immutability for BaseTool schema and ToolInvoker boundaries ([#117](#117)) ([7e5e861](7e5e861)) ### Performance * harden non-inferable principle implementation ([#195](#195)) ([02b5f4e](02b5f4e)), closes [#188](#188) ### Refactoring * adopt NotBlankStr across all models ([#108](#108)) ([#120](#120)) ([ef89b90](ef89b90)) * extract _SpendingTotals base class from spending summary models ([#111](#111)) ([2f39c1b](2f39c1b)) * harden BudgetEnforcer with error handling, validation extraction, and review fixes ([#182](#182)) ([c107bf9](c107bf9)) * harden personality profiles, department validation, and template rendering ([#158](#158)) ([10b2299](10b2299)) * pre-PR review improvements for ExecutionLoop + ReAct loop ([#124](#124)) ([8dfb3c0](8dfb3c0)) * split events.py into per-domain event modules ([#136](#136)) ([e9cba89](e9cba89)) ### Documentation * add ADR-001 memory layer evaluation and selection ([#178](#178)) ([db3026f](db3026f)), closes [#39](#39) * add agent scaling research findings to DESIGN_SPEC ([#145](#145)) ([57e487b](57e487b)) * add CLAUDE.md, contributing guide, and dev documentation ([#65](#65)) ([55c1025](55c1025)), closes [#54](#54) * add crash recovery, sandboxing, analytics, and testing decisions ([#127](#127)) ([5c11595](5c11595)) * address external review feedback with MVP scope and new protocols ([#128](#128)) ([3b30b9a](3b30b9a)) * expand design spec with pluggable strategy protocols ([#121](#121)) ([6832db6](6832db6)) * finalize 23 design decisions (ADR-002) ([#190](#190)) ([8c39742](8c39742)) * update project docs for M2.5 conventions and add docs-consistency review agent ([#114](#114)) ([99766ee](99766ee)) ### Tests * add e2e single agent integration tests ([#24](#24)) ([#156](#156)) ([f566fb4](f566fb4)) * add provider adapter integration tests ([#90](#90)) ([40a61f4](40a61f4)) ### CI/CD * add Release Please for automated versioning and GitHub Releases ([#278](#278)) ([a488758](a488758)) * bump actions/checkout from 4 to 6 ([#95](#95)) ([1897247](1897247)) * bump actions/upload-artifact from 4 to 7 ([#94](#94)) ([27b1517](27b1517)) * harden CI/CD pipeline ([#92](#92)) ([ce4693c](ce4693c)) * split vulnerability scans into critical-fail and high-warn tiers ([#277](#277)) ([aba48af](aba48af)) ### Maintenance * add /worktree skill for parallel worktree management ([#171](#171)) ([951e337](951e337)) * add design spec context loading to research-link skill ([8ef9685](8ef9685)) * add post-merge-cleanup skill ([#70](#70)) ([f913705](f913705)) * add pre-pr-review skill and update CLAUDE.md ([#103](#103)) ([92e9023](92e9023)) * add research-link skill and rename skill files to SKILL.md ([#101](#101)) ([651c577](651c577)) * bump aiosqlite from 0.21.0 to 0.22.1 ([#191](#191)) ([3274a86](3274a86)) * bump pyyaml from 6.0.2 to 6.0.3 in the minor-and-patch group ([#96](#96)) ([0338d0c](0338d0c)) * bump ruff from 0.15.4 to 0.15.5 ([a49ee46](a49ee46)) * fix M0 audit items ([#66](#66)) ([c7724b5](c7724b5)) * pin setup-uv action to full SHA ([#281](#281)) ([4448002](4448002)) * post-audit cleanup — PEP 758, loggers, bug fixes, refactoring, tests, hookify rules ([#148](#148)) ([c57a6a9](c57a6a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
12 tasks
Aureliolo
added a commit
that referenced
this pull request
Mar 11, 2026
🤖 I have created a release *beep* *boop* --- ## [0.1.0](v0.0.0...v0.1.0) (2026-03-11) ### Features * add autonomy levels and approval timeout policies ([#42](#42), [#126](#126)) ([#197](#197)) ([eecc25a](eecc25a)) * add CFO cost optimization service with anomaly detection, reports, and approval decisions ([#186](#186)) ([a7fa00b](a7fa00b)) * add code quality toolchain (ruff, mypy, pre-commit, dependabot) ([#63](#63)) ([36681a8](36681a8)) * add configurable cost tiers and subscription/quota-aware tracking ([#67](#67)) ([#185](#185)) ([9baedfa](9baedfa)) * add container packaging, Docker Compose, and CI pipeline ([#269](#269)) ([435bdfe](435bdfe)), closes [#267](#267) * add coordination error taxonomy classification pipeline ([#146](#146)) ([#181](#181)) ([70c7480](70c7480)) * add cost-optimized, hierarchical, and auction assignment strategies ([#175](#175)) ([ce924fa](ce924fa)), closes [#173](#173) * add design specification, license, and project setup ([8669a09](8669a09)) * add env var substitution and config file auto-discovery ([#77](#77)) ([7f53832](7f53832)) * add FastestStrategy routing + vendor-agnostic cleanup ([#140](#140)) ([09619cb](09619cb)), closes [#139](#139) * add HR engine and performance tracking ([#45](#45), [#47](#47)) ([#193](#193)) ([2d091ea](2d091ea)) * add issue auto-search and resolution verification to PR review skill ([#119](#119)) ([deecc39](deecc39)) * add mandatory JWT + API key authentication ([#256](#256)) ([c279cfe](c279cfe)) * add memory retrieval, ranking, and context injection pipeline ([#41](#41)) ([873b0aa](873b0aa)) * add pluggable MemoryBackend protocol with models, config, and events ([#180](#180)) ([46cfdd4](46cfdd4)) * add pluggable MemoryBackend protocol with models, config, and events ([#32](#32)) ([46cfdd4](46cfdd4)) * add pluggable output scan response policies ([#263](#263)) ([b9907e8](b9907e8)) * add pluggable PersistenceBackend protocol with SQLite implementation ([#36](#36)) ([f753779](f753779)) * add progressive trust and promotion/demotion subsystems ([#43](#43), [#49](#49)) ([3a87c08](3a87c08)) * add retry handler, rate limiter, and provider resilience ([#100](#100)) ([b890545](b890545)) * add SecOps security agent with rule engine, audit log, and ToolInvoker integration ([#40](#40)) ([83b7b6c](83b7b6c)) * add shared org memory and memory consolidation/archival ([#125](#125), [#48](#48)) ([4a0832b](4a0832b)) * design unified provider interface ([#86](#86)) ([3e23d64](3e23d64)) * expand template presets, rosters, and add inheritance ([#80](#80), [#81](#81), [#84](#84)) ([15a9134](15a9134)) * implement agent runtime state vs immutable config split ([#115](#115)) ([4cb1ca5](4cb1ca5)) * implement AgentEngine core orchestrator ([#11](#11)) ([#143](#143)) ([f2eb73a](f2eb73a)) * implement AuditRepository for security audit log persistence ([#279](#279)) ([94bc29f](94bc29f)) * implement basic tool system (registry, invocation, results) ([#15](#15)) ([c51068b](c51068b)) * implement built-in file system tools ([#18](#18)) ([325ef98](325ef98)) * implement communication foundation — message bus, dispatcher, and messenger ([#157](#157)) ([8e71bfd](8e71bfd)) * implement company template system with 7 built-in presets ([#85](#85)) ([cbf1496](cbf1496)) * implement conflict resolution protocol ([#122](#122)) ([#166](#166)) ([e03f9f2](e03f9f2)) * implement core entity and role system models ([#69](#69)) ([acf9801](acf9801)) * implement crash recovery with fail-and-reassign strategy ([#149](#149)) ([e6e91ed](e6e91ed)) * implement engine extensions — Plan-and-Execute loop and call categorization ([#134](#134), [#135](#135)) ([#159](#159)) ([9b2699f](9b2699f)) * implement enterprise logging system with structlog ([#73](#73)) ([2f787e5](2f787e5)) * implement graceful shutdown with cooperative timeout strategy ([#130](#130)) ([6592515](6592515)) * implement hierarchical delegation and loop prevention ([#12](#12), [#17](#17)) ([6be60b6](6be60b6)) * implement LiteLLM driver and provider registry ([#88](#88)) ([ae3f18b](ae3f18b)), closes [#4](#4) * implement LLM decomposition strategy and workspace isolation ([#174](#174)) ([aa0eefe](aa0eefe)) * implement meeting protocol system ([#123](#123)) ([ee7caca](ee7caca)) * implement message and communication domain models ([#74](#74)) ([560a5d2](560a5d2)) * implement model routing engine ([#99](#99)) ([d3c250b](d3c250b)) * implement parallel agent execution ([#22](#22)) ([#161](#161)) ([65940b3](65940b3)) * implement per-call cost tracking service ([#7](#7)) ([#102](#102)) ([c4f1f1c](c4f1f1c)) * implement personality injection and system prompt construction ([#105](#105)) ([934dd85](934dd85)) * implement single-task execution lifecycle ([#21](#21)) ([#144](#144)) ([c7e64e4](c7e64e4)) * implement subprocess sandbox for tool execution isolation ([#131](#131)) ([#153](#153)) ([3c8394e](3c8394e)) * implement task assignment subsystem with pluggable strategies ([#172](#172)) ([c7f1b26](c7f1b26)), closes [#26](#26) [#30](#30) * implement task decomposition and routing engine ([#14](#14)) ([9c7fb52](9c7fb52)) * implement Task, Project, Artifact, Budget, and Cost domain models ([#71](#71)) ([81eabf1](81eabf1)) * implement tool permission checking ([#16](#16)) ([833c190](833c190)) * implement YAML config loader with Pydantic validation ([#59](#59)) ([ff3a2ba](ff3a2ba)) * implement YAML config loader with Pydantic validation ([#75](#75)) ([ff3a2ba](ff3a2ba)) * initialize project with uv, hatchling, and src layout ([39005f9](39005f9)) * initialize project with uv, hatchling, and src layout ([#62](#62)) ([39005f9](39005f9)) * Litestar REST API, WebSocket feed, and approval queue (M6) ([#189](#189)) ([29fcd08](29fcd08)) * make TokenUsage.total_tokens a computed field ([#118](#118)) ([c0bab18](c0bab18)), closes [#109](#109) * parallel tool execution in ToolInvoker.invoke_all ([#137](#137)) ([58517ee](58517ee)) * testing framework, CI pipeline, and M0 gap fixes ([#64](#64)) ([f581749](f581749)) * wire all modules into observability system ([#97](#97)) ([f7a0617](f7a0617)) ### Bug Fixes * address Greptile post-merge review findings from PRs [#170](https://github.com/Aureliolo/ai-company/issues/170)-[#175](https://github.com/Aureliolo/ai-company/issues/175) ([#176](#176)) ([c5ca929](c5ca929)) * address post-merge review feedback from PRs [#164](https://github.com/Aureliolo/ai-company/issues/164)-[#167](https://github.com/Aureliolo/ai-company/issues/167) ([#170](#170)) ([3bf897a](3bf897a)), closes [#169](#169) * enforce strict mypy on test files ([#89](#89)) ([aeeff8c](aeeff8c)) * harden Docker sandbox, MCP bridge, and code runner ([#50](#50), [#53](#53)) ([d5e1b6e](d5e1b6e)) * harden git tools security + code quality improvements ([#150](#150)) ([000a325](000a325)) * harden subprocess cleanup, env filtering, and shutdown resilience ([#155](#155)) ([d1fe1fb](d1fe1fb)) * incorporate post-merge feedback + pre-PR review fixes ([#164](#164)) ([c02832a](c02832a)) * pre-PR review fixes for post-merge findings ([#183](#183)) ([26b3108](26b3108)) * resolve circular imports, bump litellm, fix release tag format ([#286](#286)) ([a6659b5](a6659b5)) * strengthen immutability for BaseTool schema and ToolInvoker boundaries ([#117](#117)) ([7e5e861](7e5e861)) ### Performance * harden non-inferable principle implementation ([#195](#195)) ([02b5f4e](02b5f4e)), closes [#188](#188) ### Refactoring * adopt NotBlankStr across all models ([#108](#108)) ([#120](#120)) ([ef89b90](ef89b90)) * extract _SpendingTotals base class from spending summary models ([#111](#111)) ([2f39c1b](2f39c1b)) * harden BudgetEnforcer with error handling, validation extraction, and review fixes ([#182](#182)) ([c107bf9](c107bf9)) * harden personality profiles, department validation, and template rendering ([#158](#158)) ([10b2299](10b2299)) * pre-PR review improvements for ExecutionLoop + ReAct loop ([#124](#124)) ([8dfb3c0](8dfb3c0)) * split events.py into per-domain event modules ([#136](#136)) ([e9cba89](e9cba89)) ### Documentation * add ADR-001 memory layer evaluation and selection ([#178](#178)) ([db3026f](db3026f)), closes [#39](#39) * add agent scaling research findings to DESIGN_SPEC ([#145](#145)) ([57e487b](57e487b)) * add CLAUDE.md, contributing guide, and dev documentation ([#65](#65)) ([55c1025](55c1025)), closes [#54](#54) * add crash recovery, sandboxing, analytics, and testing decisions ([#127](#127)) ([5c11595](5c11595)) * address external review feedback with MVP scope and new protocols ([#128](#128)) ([3b30b9a](3b30b9a)) * expand design spec with pluggable strategy protocols ([#121](#121)) ([6832db6](6832db6)) * finalize 23 design decisions (ADR-002) ([#190](#190)) ([8c39742](8c39742)) * update project docs for M2.5 conventions and add docs-consistency review agent ([#114](#114)) ([99766ee](99766ee)) ### Tests * add e2e single agent integration tests ([#24](#24)) ([#156](#156)) ([f566fb4](f566fb4)) * add provider adapter integration tests ([#90](#90)) ([40a61f4](40a61f4)) ### CI/CD * add Release Please for automated versioning and GitHub Releases ([#278](#278)) ([a488758](a488758)) * bump actions/checkout from 4 to 6 ([#95](#95)) ([1897247](1897247)) * bump actions/upload-artifact from 4 to 7 ([#94](#94)) ([27b1517](27b1517)) * bump anchore/scan-action from 6.5.1 to 7.3.2 ([#271](#271)) ([80a1c15](80a1c15)) * bump docker/build-push-action from 6.19.2 to 7.0.0 ([#273](#273)) ([dd0219e](dd0219e)) * bump docker/login-action from 3.7.0 to 4.0.0 ([#272](#272)) ([33d6238](33d6238)) * bump docker/metadata-action from 5.10.0 to 6.0.0 ([#270](#270)) ([baee04e](baee04e)) * bump docker/setup-buildx-action from 3.12.0 to 4.0.0 ([#274](#274)) ([5fc06f7](5fc06f7)) * bump sigstore/cosign-installer from 3.9.1 to 4.1.0 ([#275](#275)) ([29dd16c](29dd16c)) * harden CI/CD pipeline ([#92](#92)) ([ce4693c](ce4693c)) * split vulnerability scans into critical-fail and high-warn tiers ([#277](#277)) ([aba48af](aba48af)) ### Maintenance * add /worktree skill for parallel worktree management ([#171](#171)) ([951e337](951e337)) * add design spec context loading to research-link skill ([8ef9685](8ef9685)) * add post-merge-cleanup skill ([#70](#70)) ([f913705](f913705)) * add pre-pr-review skill and update CLAUDE.md ([#103](#103)) ([92e9023](92e9023)) * add research-link skill and rename skill files to SKILL.md ([#101](#101)) ([651c577](651c577)) * bump aiosqlite from 0.21.0 to 0.22.1 ([#191](#191)) ([3274a86](3274a86)) * bump pyyaml from 6.0.2 to 6.0.3 in the minor-and-patch group ([#96](#96)) ([0338d0c](0338d0c)) * bump ruff from 0.15.4 to 0.15.5 ([a49ee46](a49ee46)) * fix M0 audit items ([#66](#66)) ([c7724b5](c7724b5)) * **main:** release ai-company 0.1.1 ([#282](#282)) ([2f4703d](2f4703d)) * pin setup-uv action to full SHA ([#281](#281)) ([4448002](4448002)) * post-audit cleanup — PEP 758, loggers, bug fixes, refactoring, tests, hookify rules ([#148](#148)) ([c57a6a9](c57a6a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Signed-off-by: Aurelio <19254254+Aureliolo@users.noreply.github.com>
This was referenced Mar 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
X-Human-Roleheader (stub auth for M6; real JWT/OAuth in M7)ServiceUnavailableError(503) instead of opaqueAttributeError(500) for unconfigured servicesdefault_messageclass attribute pattern,ServiceUnavailableError, narrowed exception catches, log-before-raise consistencyCloses #31, closes #33, closes #34, closes #37
Test plan
Review coverage
Pre-reviewed by 10 specialized agents (55 findings consolidated, all implemented):
code-reviewer— general code qualitypython-reviewer— Pythonic idioms, PEP compliancepr-test-analyzer— test coverage gapssilent-failure-hunter— swallowed errorscomment-analyzer— docstring accuracytype-design-analyzer— type/model designlogging-audit— logging convention complianceresilience-audit— retry/error handling patternssecurity-reviewer— security vulnerabilitiesdocs-consistency— documentation drift🤖 Generated with Claude Code