diff --git a/docs/map/api-routes-schemas.md b/docs/map/api-routes-schemas.md index c92ce06a..396dff04 100644 --- a/docs/map/api-routes-schemas.md +++ b/docs/map/api-routes-schemas.md @@ -104,6 +104,8 @@ The FastAPI surface of RoboCo: every HTTP route under `roboco/api/routes/` (the | `require_orchestrator_ceo` | dep | api/deps.py:790 | Router-level CEO-HMAC guard on orchestrator control routes — relocated from a per-file `_require_ceo(agent)` wrapper that used to live in `routes/orchestrator.py`; `router = APIRouter(dependencies=[Depends(require_orchestrator_ceo)])`. | | `validate_agent_id_param` | fn | api/deps.py:839 | Path-injection guard (rejects empty/`.`/`..`/`/`/`\`/NUL) then normalizes via `_resolve_to_slug` — spawn/stop/status/resolve-wait/mark-waiting accept either a DB UUID or a slug and address the runtime container by the resolved slug; an unknown UUID passes through unchanged. Relocated from a route-local `_validated_agent_id` helper in `routes/orchestrator.py`. | | `require_ceo_role` / `require_pm_or_above` | fn | api/deps.py:627 / api/deps.py:618 | Shared role-check guards a2a.py/orchestrator.py/video.py/roadmap.py route handlers now call directly, replacing redundant per-file `_require_ceo(agent)` partial-application wrappers each of those route files used to define locally. | +| `require_auditor_or_ceo` | fn | api/deps.py:666 | Auditor-or-CEO 403 gate, added in the batch-B relocation (task `f8480831`) for dashboard.py's flag/report mutations and playbooks.py's curation endpoints — the two route files' identical inline role-check collapsed into one shared `deps.py` helper. | +| `require_role_in` | fn | api/deps.py:652 | Generic "role must be a member of this set" 403 gate for an endpoint-specific role set with no standing named tier — added in the batch-B relocation (task `f8480831`), used by secretary.py's directive/state endpoints. | | `task_to_response` / `task_list_to_response` / `finding_to_response` | fn | api/schemas/tasks.py:889,964,969 | DTO conversion helpers (`TaskTable` -> `TaskResponse`/`TaskFindingResponse`); relocated out of `routes/tasks.py` into the schema module they convert to. | | `setup_middleware` | fn | api/middleware.py | Register exception handlers (422 scrub, HTTP, RobocoError, generic). | | `request_validation_handler` | fn | api/middleware.py:407 | Log 422 body (secrets scrubbed) + uuid remediate hint. | @@ -258,6 +260,7 @@ roboco/api/ > - `461a6e1a`+`96401f4c`+`5f32d876` (2026-07-18/19, forge Phases 1-4, #571/#575/#581) — no new HTTP routes (the forge routing is internal to `GitService`), but `roboco/api/schemas/project.py`/`project_fields.py` gain `git_provider` (project CRUD schemas) and the shared `task_project_fields` helper the X/video routes now call — see `docs/map/worksession-git.md` and `docs/map/product-strategy-research-pitch.md`. > - ("panel-perf-p3-p4") adds `GET /api/dashboard/metrics/members` (batch scorecard fetch) — see `docs/map/metrics-observability.md`. > - (task `4baffaa3`, "Batch A: extract route helpers") placement-only refactor, no route/schema/behavior change: moves every non-`@router`-decorated top-level helper out of `tasks.py`, `a2a.py`, `orchestrator.py`, `video.py`, `v1/_role_dep.py`, `roadmap.py`, `prompter_live.py` (`journals.py` had none) per `.roboco/conventions.yml`'s `no_helpers_in_routes` rule — DB/side-effecting logic to the paired `roboco/services/*` module, DTO-conversion helpers to the matching `roboco/api/schemas/*.py` (e.g. `task_to_response`), and small HTTP-layer auth guards (`envelope_to_response`, `require_orchestrator_ceo`, `validate_agent_id_param`, `require_ceo_role`, `require_pm_or_above`) into `roboco/api/deps.py`, replacing several route-files' redundant local `_require_ceo(agent)` wrappers with direct calls to the shared `deps.py` guard. Two real regressions surfaced during the relocation's revision rounds and were fixed before merge: `envelope_to_response`'s "verb rejected" structlog event was dropped in the move (restored — see the Key Symbols row above), and `_auth_required()` was narrowed to a truthy-only check that silently dropped its unset-value production fallback, which would have accepted unauthenticated `X-Agent-Role: ceo` header spoofing on an unconfigured production deploy (GHSA-4f7g-w95g-5q2c) — the three-branch fallback logic was restored. +> - (task `f8480831`, "Batch B: extract route helpers in remaining smaller-offender route files") placement-only refactor, no route/schema/behavior change: moved 28 non-`@router`-decorated helper functions out of 15 of the 24 batch-B route files (`optimal.py`, `project.py`, `release.py`, `dashboard.py`, `pitch.py`, `x.py`, `docs.py`, `git.py`, `playbooks.py`, `product.py`, `provider.py`, `research.py`, `secretary.py`, `system.py`, `work_session.py`) into their paired `roboco/services/*` module (DB/service-calling helpers), the route's own `roboco/api/schemas/*.py` as a converter (pure response/request shaping, mirroring `task_to_response`), or `roboco/utils/converters.py` (pure generic helpers); the other 9 files (`notifications.py`, `agents.py`, `cockpit.py`, `company_goals.py`, `kanban.py`, `secretary_live.py`, `settings.py`, `stream.py`, `usage.py`) had zero helpers by the precise `classify_python.py` classifier already. Added two small shared role-check helpers to `roboco/api/deps.py` (`require_auditor_or_ceo`, `require_role_in`) for endpoint-specific role gates that had no existing home. ## Regression Risks diff --git a/roboco/api/deps.py b/roboco/api/deps.py index 778fe8da..9aa92c7d 100644 --- a/roboco/api/deps.py +++ b/roboco/api/deps.py @@ -649,6 +649,31 @@ def require_developer_or_above(role: Any, action: str) -> None: ) +def require_role_in(role: Any, allowed: frozenset[Role], detail: str) -> None: + """Raise 403 unless ``role`` is a member of ``allowed``. + + A generic sibling to the named ``require_*`` checks above, for callers + (e.g. the Secretary surface) that gate a single endpoint to an arbitrary, + endpoint-specific role set rather than one of the standing tiers. + """ + if role not in allowed: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=detail) + + +_AUDITOR_OR_CEO_ROLES: frozenset[Role] = frozenset({Role.AUDITOR, Role.CEO}) + + +def require_auditor_or_ceo(role: Any, detail: str) -> None: + """Raise 403 unless caller is the Auditor or the CEO. + + The Auditor is the silent-observer role; the CEO overrides. Shared by the + dashboard's auditor-flag/report mutations and the playbook curation + endpoints — both gate to this same role pair with their own 403 wording. + """ + if _role_value(role) not in _AUDITOR_OR_CEO_ROLES: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=detail) + + _GLOBAL_CELL_ACCESS_ROLES: frozenset[Role] = (BOARD_ROLES - {Role.HEAD_MARKETING}) | { Role.MAIN_PM, Role.CEO, diff --git a/roboco/api/routes/dashboard.py b/roboco/api/routes/dashboard.py index 96d51fd1..048ada21 100644 --- a/roboco/api/routes/dashboard.py +++ b/roboco/api/routes/dashboard.py @@ -10,7 +10,12 @@ from uuid import UUID from fastapi import APIRouter, Depends, HTTPException, Query, status -from roboco.api.deps import CurrentAgentContext, DbSession, require_panel_token +from roboco.api.deps import ( + CurrentAgentContext, + DbSession, + require_auditor_or_ceo, + require_panel_token, +) from roboco.api.schemas.dashboard import ( AuditorDashboard, AuditorFlag, @@ -22,7 +27,6 @@ from roboco.api.schemas.dashboard import ( TeamHealth, UsageSummary, ) -from roboco.models import AgentRole from roboco.models.base import Team from roboco.models.dashboard import CreateFlagParams from roboco.services.dashboard import get_dashboard_service @@ -37,19 +41,12 @@ router = APIRouter(dependencies=[Depends(require_panel_token)]) # The auditor flag/report mutating routes are gated to the Auditor and the # CEO. The Auditor is the silent-observer role whose flags/reports feed the -# CEO; the CEO overrides. Mirrors ``_require_curator`` in playbooks.py and -# ``_require_ceo`` in release.py. Read-only auditor views (``GET -# /auditor/flags``, ``GET /auditor/reports``, ``GET /auditor``) stay open — -# the dashboard is observable by any authenticated operator. -_AUDITOR_OR_CEO_ROLES = frozenset({AgentRole.AUDITOR, AgentRole.CEO}) - - -def _require_auditor_or_ceo(agent: CurrentAgentContext) -> None: - if agent.role not in _AUDITOR_OR_CEO_ROLES: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Only the Auditor or CEO may mutate auditor flags or reports", - ) +# CEO; the CEO overrides. Read-only auditor views (``GET /auditor/flags``, +# ``GET /auditor/reports``, ``GET /auditor``) stay open — the dashboard is +# observable by any authenticated operator. +_MUTATE_FLAGS_OR_REPORTS_DETAIL = ( + "Only the Auditor or CEO may mutate auditor flags or reports" +) # ============================================================================= @@ -162,7 +159,7 @@ async def create_auditor_flag( agent: CurrentAgentContext, ) -> AuditorFlag: """Create a new auditor flag.""" - _require_auditor_or_ceo(agent) + require_auditor_or_ceo(agent.role, _MUTATE_FLAGS_OR_REPORTS_DETAIL) service = get_dashboard_service(db) params = CreateFlagParams( severity=data.severity.value, @@ -195,7 +192,7 @@ async def resolve_auditor_flag( notes: str | None = None, ) -> dict[str, str]: """Resolve an auditor flag.""" - _require_auditor_or_ceo(agent) + require_auditor_or_ceo(agent.role, _MUTATE_FLAGS_OR_REPORTS_DETAIL) service = get_dashboard_service(db) if not service.resolve_flag(flag_id, notes): raise HTTPException( @@ -238,7 +235,7 @@ async def create_auditor_report( agent: CurrentAgentContext, ) -> AuditorReport: """Create a new auditor report.""" - _require_auditor_or_ceo(agent) + require_auditor_or_ceo(agent.role, _MUTATE_FLAGS_OR_REPORTS_DETAIL) service = get_dashboard_service(db) report = service.create_report( report_type=data.report_type, @@ -264,7 +261,7 @@ async def send_auditor_report( agent: CurrentAgentContext, ) -> dict[str, str]: """Mark a report as sent to CEO.""" - _require_auditor_or_ceo(agent) + require_auditor_or_ceo(agent.role, _MUTATE_FLAGS_OR_REPORTS_DETAIL) service = get_dashboard_service(db) if not service.send_report(report_id): raise HTTPException( diff --git a/roboco/api/routes/docs.py b/roboco/api/routes/docs.py index 6b5647c3..ff7e91a5 100644 --- a/roboco/api/routes/docs.py +++ b/roboco/api/routes/docs.py @@ -21,26 +21,11 @@ from roboco.api.schemas.docs import ( ) from roboco.services.base import NotFoundError, UnauthorizedError, ValidationError from roboco.services.docs import WriteDocInput, get_docs_service -from roboco.services.gateway.kb_authz import docs_denial_envelope +from roboco.services.gateway.kb_authz import docs_unauthorized_response router = APIRouter() -def _unauthorized_response(err: UnauthorizedError) -> JSONResponse: - """Render a docs-service denial as the gateway Envelope (HTTP 403). - - The RBAC decision is made in ``DocsService`` (it raises - ``UnauthorizedError``); this only renders that denial at the HTTP - boundary. The body is the Envelope wire-dict at top level so the agent - receives a non-null ``remediate`` instead of a bare ``detail`` string. - """ - envelope = docs_denial_envelope(err.action, err.reason) - return JSONResponse( - status_code=status.HTTP_403_FORBIDDEN, - content=envelope.as_dict(), - ) - - # Module-level Query defaults _list_task_id_query: UUID | None = Query(None, description="Filter by task ID") _read_path_query: str = Query( @@ -106,7 +91,7 @@ async def write_doc( detail=e.message, ) from e except UnauthorizedError as e: - return _unauthorized_response(e) + return docs_unauthorized_response(e) except NotFoundError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, @@ -149,7 +134,7 @@ async def read_doc( detail=e.message, ) from e except UnauthorizedError as e: - return _unauthorized_response(e) + return docs_unauthorized_response(e) except NotFoundError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, @@ -203,7 +188,7 @@ async def list_docs( count=len(docs), ) except UnauthorizedError as e: - return _unauthorized_response(e) + return docs_unauthorized_response(e) except NotFoundError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, @@ -243,7 +228,7 @@ async def delete_doc( detail=e.message, ) from e except UnauthorizedError as e: - return _unauthorized_response(e) + return docs_unauthorized_response(e) except NotFoundError as e: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, diff --git a/roboco/api/routes/git.py b/roboco/api/routes/git.py index 0cae2be5..545a4ae4 100644 --- a/roboco/api/routes/git.py +++ b/roboco/api/routes/git.py @@ -22,10 +22,8 @@ Workspace Structure: """ from datetime import datetime -from uuid import UUID from fastapi import APIRouter, HTTPException, Query, status -from sqlalchemy.ext.asyncio import AsyncSession from roboco.api.deps import CurrentAgentContext, DbSession from roboco.api.schemas.git import ( @@ -57,7 +55,7 @@ from roboco.api.schemas.git import ( GitRebaseResponse, GitStatusResponse, ) -from roboco.exceptions import GitCommandError, GitError, GitTimeoutError +from roboco.exceptions import GitError from roboco.logging import get_logger from roboco.models.base import AgentRole from roboco.security import ( @@ -65,15 +63,11 @@ from roboco.security import ( prompt_injection_validator, secret_exfil_validator, ) -from roboco.services.base import ( - NotFoundError, - ServiceError, - UnauthorizedError, - ValidationError, -) -from roboco.services.git import get_git_service +from roboco.services.base import ServiceError +from roboco.services.git import get_git_service, translate_git_error from roboco.services.project import get_project_service from roboco.services.task import get_task_service +from roboco.utils.converters import compute_file_range, parse_branch_line logger = get_logger(__name__) @@ -92,40 +86,6 @@ _TranslatableError = (ServiceError, GitError) # Cap an unbounded whole-file read so a huge file can't flood the panel. _FILE_MAX_LINES = 2000 - -def _compute_file_range( - *, - total: int, - line: int | None, - context: int, - start: int | None, - end: int | None, -) -> tuple[int, int, bool]: - """Resolve the (start, end, truncated) slice for a file-content read. - - Explicit ``start``/``end`` win; else ``line`` centers a context window; - else the whole file. Whichever branch resolves the window, it is capped - at ``_FILE_MAX_LINES`` lines afterward. Returns 1-based inclusive - [start, end] and whether the slice is shorter than the file. - """ - if start is not None and end is not None: - s, e_ = start, end - elif line is not None: - s = max(1, line - context) - e_ = min(total, line + context) - else: - s, e_ = 1, total - - s = max(1, min(s, total)) - e_ = max(s, min(e_, total)) - - truncated = e_ < total - if e_ - s + 1 > _FILE_MAX_LINES: - e_ = s + _FILE_MAX_LINES - 1 - truncated = True - return s, e_, truncated - - # Roles permitted to rebase branches via the /rebase endpoint. # Rebase is a history-rewriting operation that should be authorised only by # PM-level or CEO-level callers. Developers are intentionally excluded: @@ -137,51 +97,6 @@ _REBASE_ALLOWED_ROLES: frozenset[AgentRole] = frozenset( ) -def _translate_error(e: ServiceError | GitError) -> HTTPException: - """Translate service errors to HTTP exceptions.""" - if isinstance(e, NotFoundError): - return HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.message) - if isinstance(e, UnauthorizedError): - return HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=e.message) - if isinstance(e, ValidationError): - return HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=e.message) - if isinstance(e, GitTimeoutError): - return HTTPException( - status_code=status.HTTP_504_GATEWAY_TIMEOUT, detail=e.message - ) - if isinstance(e, GitCommandError): - return HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.message - ) - return HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.message - ) - - -async def _resolve_project_slug(identifier: str, db: AsyncSession) -> str: - """Resolve a project identifier (UUID string or slug) to its slug. - - Callers pass whatever string they have — a human-readable slug like - "roboco" or a UUID like "3fa85f64-5717-4562-b3fc-2c963f66afa6". - We try UUID first; if the string is not a valid UUID we treat it as - a slug directly. In both cases we verify the project exists and - return the canonical slug so downstream git-service calls work. - """ - service = get_project_service(db) - try: - uuid = UUID(identifier) - project = await service.get(uuid) - except ValueError: - project = await service.get_by_slug(identifier) - - if not project: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail=f"Project not found: {identifier}", - ) - return str(project.slug) - - # ============================================================================= # READ-ONLY ENDPOINTS # ============================================================================= @@ -195,7 +110,7 @@ async def get_git_status( _task_id: str | None = Query(default=None), ) -> GitStatusResponse: """Get git status for a project.""" - project_slug = await _resolve_project_slug(project_slug, db) + project_slug = await get_project_service(db).resolve_slug_or_404(project_slug) git_service = get_git_service(db) try: @@ -210,7 +125,7 @@ async def get_git_status( behind, ) = await git_service.get_status(workspace) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitStatusResponse( project_slug=project_slug, @@ -233,7 +148,7 @@ async def get_git_log( branch: str | None = Query(default=None), ) -> GitLogResponse: """Get git log for a project.""" - project_slug = await _resolve_project_slug(project_slug, db) + project_slug = await get_project_service(db).resolve_slug_or_404(project_slug) git_service = get_git_service(db) try: @@ -277,7 +192,7 @@ async def get_git_log( ) return GitLogResponse(project_slug=project_slug, branch=branch, commits=[]) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e commits = [] for line in log_result.stdout.strip().split("\n"): @@ -302,28 +217,6 @@ async def get_git_log( ) -def _parse_branch_line(line: str) -> tuple[str, bool, str | None] | None: - """Classify one `%(refname)|%(objectname:short)` line as (name, is_remote, - last_commit), or None for skippable entries (blank, origin/HEAD, other ref - namespaces). Full refname, not `:short` — a remote-tracking ref shortens to - `origin/`, indistinguishable from a local branch literally named - that; classify on the `refs/heads/` vs `refs/remotes/` prefix instead. - """ - if not line: - return None - parts = line.split("|") - ref = parts[0] - last_commit = parts[1] if len(parts) > 1 else None - if ref.startswith("refs/heads/"): - return ref.removeprefix("refs/heads/"), False, last_commit - if ref.startswith("refs/remotes/"): - _remote_name, _, name = ref.removeprefix("refs/remotes/").partition("/") - if not name or name == "HEAD": - return None # origin/HEAD is a symbolic pointer, not a branch - return name, True, last_commit - return None - - @router.get("/branches", response_model=GitBranchListResponse) async def list_branches( db: DbSession, @@ -332,7 +225,7 @@ async def list_branches( include_remote: bool = Query(default=False), ) -> GitBranchListResponse: """List git branches for a project.""" - project_slug = await _resolve_project_slug(project_slug, db) + project_slug = await get_project_service(db).resolve_slug_or_404(project_slug) git_service = get_git_service(db) try: @@ -350,11 +243,11 @@ async def list_branches( branch_result = await git_service._run_git(workspace, args) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e branches = [] for line in branch_result.stdout.strip().split("\n"): - parsed = _parse_branch_line(line) + parsed = parse_branch_line(line) if parsed is None: continue name, is_remote, last_commit = parsed @@ -383,7 +276,7 @@ async def get_git_diff( file_path: str | None = Query(default=None), ) -> GitDiffResponse: """Get git diff for a project.""" - project_slug = await _resolve_project_slug(project_slug, db) + project_slug = await get_project_service(db).resolve_slug_or_404(project_slug) git_service = get_git_service(db) try: @@ -403,7 +296,7 @@ async def get_git_diff( stat_args.append("--staged") stat_result = await git_service._run_git(workspace, stat_args) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e files_changed = stat_result.stdout.count("\n") - 1 if stat_result.stdout else 0 @@ -444,7 +337,7 @@ async def get_git_file( branch_name=branch, path=path, actor_agent_id=agent.agent_id ) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e if content is None: raise HTTPException( @@ -455,8 +348,12 @@ async def get_git_file( all_lines = content.splitlines() total = len(all_lines) - s, e_, truncated = _compute_file_range( - total=total, line=line, context=context, start=start, end=end + s, e_, truncated = compute_file_range( + total=total, + line=line, + context=context, + explicit_range=(start, end) if start is not None and end is not None else None, + max_lines=_FILE_MAX_LINES, ) sliced = all_lines[s - 1 : e_] @@ -497,7 +394,7 @@ async def create_commit( deletions, ) = await git_service.commit_for_task(agent.agent_id, data) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitCommitResponse( commit_hash=commit_hash, @@ -525,7 +422,7 @@ async def push_commits( agent.agent_id, agent.role, data ) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitPushResponse( branch=branch, @@ -555,7 +452,7 @@ async def create_branch( agent.agent_id, data ) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitCreateBranchResponse( branch_name=branch_name, @@ -585,7 +482,7 @@ async def checkout_branch( try: await git_service.checkout_branch_for_agent(agent.agent_id, data) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitCheckoutResponse( branch=data.branch, @@ -615,7 +512,7 @@ async def create_pull_request( target_branch, ) = await git_service.create_pr_for_task(agent.agent_id, data) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitCreatePRResponse( pr_number=pr_number, @@ -643,7 +540,7 @@ async def merge_pull_request( agent.agent_id, agent.role, data ) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitMergePRResponse( pr_number=data.pr_number, @@ -664,7 +561,7 @@ async def pull_commits( agent: CurrentAgentContext, ) -> GitPullResponse: """Pull latest changes from origin into the agent workspace.""" - project_slug = await _resolve_project_slug(data.project_slug, db) + project_slug = await get_project_service(db).resolve_slug_or_404(data.project_slug) git_service = get_git_service(db) try: @@ -679,7 +576,7 @@ async def pull_commits( behind, ) = await git_service.pull(workspace) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitPullResponse( project_slug=project_slug, @@ -704,7 +601,7 @@ async def fetch_commits( agent: CurrentAgentContext, ) -> GitFetchResponse: """Fetch changes from origin without merging.""" - project_slug = await _resolve_project_slug(data.project_slug, db) + project_slug = await get_project_service(db).resolve_slug_or_404(data.project_slug) git_service = get_git_service(db) try: @@ -719,7 +616,7 @@ async def fetch_commits( behind, ) = await git_service.fetch(workspace) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitFetchResponse( project_slug=project_slug, @@ -782,7 +679,7 @@ async def rebase_branch( "you. Only the task's assigned agent or CEO may rebase it." ), ) - project_slug = await _resolve_project_slug(data.project_slug, db) + project_slug = await get_project_service(db).resolve_slug_or_404(data.project_slug) git_service = get_git_service(db) try: @@ -791,7 +688,7 @@ async def rebase_branch( workspace, data.target_branch, project_slug ) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitRebaseResponse( project_slug=project_slug, @@ -829,7 +726,7 @@ async def cleanup_stale_branches( "main_pm) may use this endpoint." ), ) - project_slug = await _resolve_project_slug(data.project_slug, db) + project_slug = await get_project_service(db).resolve_slug_or_404(data.project_slug) git_service = get_git_service(db) try: @@ -844,7 +741,7 @@ async def cleanup_stale_branches( project_slug, after_task_id=data.after_cursor ) except _TranslatableError as e: - raise _translate_error(e) from e + raise translate_git_error(e) from e return GitBranchCleanupResponse( project_slug=project_slug, diff --git a/roboco/api/routes/optimal.py b/roboco/api/routes/optimal.py index 3e5ad4db..2b2925db 100644 --- a/roboco/api/routes/optimal.py +++ b/roboco/api/routes/optimal.py @@ -76,7 +76,7 @@ from roboco.security import ( prompt_injection_validator, secret_exfil_validator, ) -from roboco.services.gateway.kb_authz import authorize_kb_action +from roboco.services.gateway.kb_authz import kb_denial_response from roboco.services.optimal import ( IndexType, QueryContext, @@ -93,28 +93,6 @@ logger = structlog.get_logger() router = APIRouter() -def _kb_denial_response( - permissions: PermissionServiceDep, - agent: CurrentAgentContext, - action: str, -) -> JSONResponse | None: - """Gateway Envelope (HTTP 403) when the KB action is denied, else None. - - The authorization decision itself lives in the gateway - (``authorize_kb_action``); this only renders a denial verdict at the HTTP - boundary. The body is the Envelope wire-dict at top level — not nested - under ``detail`` — so the agent receives a non-null ``remediate`` it can - act on, matching the gateway Envelope contract. - """ - denial = authorize_kb_action(permissions, agent, action) - if denial is None: - return None - return JSONResponse( - status_code=status.HTTP_403_FORBIDDEN, - content=denial.as_dict(), - ) - - # ============================================================================= # INDEXING ENDPOINTS # ============================================================================= @@ -142,7 +120,7 @@ async def index_code( - Directories - Glob patterns (e.g., "src/**/*.py") """ - denied = _kb_denial_response(permissions, agent, KBAction.INDEX_CODE) + denied = kb_denial_response(permissions, agent, KBAction.INDEX_CODE) if denied is not None: return denied @@ -181,7 +159,7 @@ async def index_documentation( - URLs (single page or crawl with /**) - Glob patterns """ - denied = _kb_denial_response(permissions, agent, KBAction.INDEX_DOCS) + denied = kb_denial_response(permissions, agent, KBAction.INDEX_DOCS) if denied is not None: return denied @@ -467,7 +445,7 @@ async def get_stats( permissions: PermissionServiceDep, ) -> IndexStatsResponse | JSONResponse: """Get statistics about all indexes.""" - denied = _kb_denial_response(permissions, agent, KBAction.VIEW_STATS) + denied = kb_denial_response(permissions, agent, KBAction.VIEW_STATS) if denied is not None: return denied @@ -493,7 +471,7 @@ async def check_staleness( Declared BEFORE `/stats/{index_type}` so FastAPI matches the literal `staleness` segment instead of treating it as an `index_type` param. """ - denied = _kb_denial_response(permissions, agent, KBAction.VIEW_STATS) + denied = kb_denial_response(permissions, agent, KBAction.VIEW_STATS) if denied is not None: return denied @@ -508,7 +486,7 @@ async def get_single_index_stats( permissions: PermissionServiceDep, ) -> SingleIndexStatsResponse | JSONResponse: """Get statistics for a specific index type.""" - denied = _kb_denial_response(permissions, agent, KBAction.VIEW_STATS) + denied = kb_denial_response(permissions, agent, KBAction.VIEW_STATS) if denied is not None: return denied @@ -564,7 +542,7 @@ async def clear_index( Warning: This permanently deletes all documents in the index. """ - denied = _kb_denial_response(permissions, agent, KBAction.CLEAR_INDEX) + denied = kb_denial_response(permissions, agent, KBAction.CLEAR_INDEX) if denied is not None: return denied @@ -595,7 +573,7 @@ async def list_documents( pagination: PaginationDep, ) -> DocumentListResponse | JSONResponse: """List documents in a specific index (paginated).""" - denied = _kb_denial_response(permissions, agent, KBAction.VIEW_STATS) + denied = kb_denial_response(permissions, agent, KBAction.VIEW_STATS) if denied is not None: return denied try: @@ -645,7 +623,7 @@ async def refresh_index( Re-indexes the specified sources to pick up changes. """ - denied = _kb_denial_response(permissions, agent, KBAction.REFRESH_INDEX) + denied = kb_denial_response(permissions, agent, KBAction.REFRESH_INDEX) if denied is not None: return denied @@ -706,7 +684,7 @@ async def reindex_all( """ import asyncio - denied = _kb_denial_response(permissions, agent, KBAction.INDEX_CODE) + denied = kb_denial_response(permissions, agent, KBAction.INDEX_CODE) if denied is not None: return denied diff --git a/roboco/api/routes/pitch.py b/roboco/api/routes/pitch.py index f4a1b7e3..36644fd4 100644 --- a/roboco/api/routes/pitch.py +++ b/roboco/api/routes/pitch.py @@ -12,17 +12,21 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, status from roboco.api.deps import CurrentAgentContext, DbSession -from roboco.api.schemas.pitch import PitchCreateRequest, PitchDecision, PitchResponse -from roboco.db.tables import PitchTable -from roboco.foundation.identity import CELL_TEAMS, Team +from roboco.api.schemas.pitch import ( + PitchCreateRequest, + PitchDecision, + PitchResponse, + pitch_to_response, +) from roboco.models import AgentRole from roboco.models.pitch import PitchCreate, PitchStatus from roboco.services.base import ConflictError, NotFoundError, ValidationError -from roboco.services.github_provisioning import ( - ProvisioningDisabledError, - ProvisioningError, +from roboco.services.github_provisioning import ProvisioningError +from roboco.services.pitch import ( + get_pitch_service, + parse_cell_teams, + pitch_error_to_http_exc, ) -from roboco.services.pitch import get_pitch_service router = APIRouter() @@ -38,70 +42,6 @@ _VIEW_ROLES = frozenset( ) -_SERVICE_ERROR_HTTP: tuple[tuple[type[Exception], int], ...] = ( - (NotFoundError, status.HTTP_404_NOT_FOUND), - (ProvisioningDisabledError, status.HTTP_400_BAD_REQUEST), - (ProvisioningError, status.HTTP_502_BAD_GATEWAY), - (ConflictError, status.HTTP_409_CONFLICT), - (ValidationError, status.HTTP_400_BAD_REQUEST), -) - - -def _to_http_exc(exc: Exception) -> HTTPException: - """Translate a known service/provisioning error into an HTTPException. - - ProvisioningDisabledError is listed before ProvisioningError (its parent) - so the more specific 400 wins. - """ - detail = getattr(exc, "message", None) or str(exc) - for exc_type, code in _SERVICE_ERROR_HTTP: - if isinstance(exc, exc_type): - return HTTPException(status_code=code, detail=detail) - return HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=detail - ) - - -def _to_response(pitch: PitchTable) -> PitchResponse: - return PitchResponse( - id=str(pitch.id), - title=pitch.title, - slug=pitch.slug, - problem=pitch.problem, - proposed_solution=pitch.proposed_solution, - target_cells=list(pitch.target_cells or []), - status=pitch.status, - created_by=str(pitch.created_by), - decided_by=str(pitch.decided_by) if pitch.decided_by else None, - decision_notes=pitch.decision_notes, - provisioned_product_id=( - str(pitch.provisioned_product_id) if pitch.provisioned_product_id else None - ), - provisioned_project_ids=list(pitch.provisioned_project_ids or []), - seed_task_id=str(pitch.seed_task_id) if pitch.seed_task_id else None, - created_at=pitch.created_at.isoformat() if pitch.created_at else None, - ) - - -def _parse_cells(raw: list[str]) -> list[Team]: - cells: list[Team] = [] - for c in raw: - try: - team = Team(c) - except ValueError as exc: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, - detail=f"unknown cell '{c}'", - ) from exc - if team not in CELL_TEAMS: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, - detail=f"'{c}' is not a cell team", - ) - cells.append(team) - return cells - - @router.post("", response_model=PitchResponse, status_code=status.HTTP_201_CREATED) async def create_pitch( data: PitchCreateRequest, db: DbSession, agent: CurrentAgentContext @@ -117,15 +57,15 @@ async def create_pitch( slug=data.slug, problem=data.problem, proposed_solution=data.proposed_solution, - target_cells=_parse_cells(data.target_cells), + target_cells=parse_cell_teams(data.target_cells), ) service = get_pitch_service(db) try: pitch = await service.create(create, created_by=agent.agent_id) except ConflictError as exc: - raise _to_http_exc(exc) from exc + raise pitch_error_to_http_exc(exc) from exc await db.commit() - return _to_response(pitch) + return pitch_to_response(pitch) @router.get("", response_model=list[PitchResponse]) @@ -148,7 +88,7 @@ async def list_pitches( detail=f"unknown pitch status '{status_filter}'", ) from exc pitches = await get_pitch_service(db).list_pitches(parsed) - return [_to_response(p) for p in pitches] + return [pitch_to_response(p) for p in pitches] @router.get("/{pitch_id}", response_model=PitchResponse) @@ -166,7 +106,7 @@ async def get_pitch( raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="pitch not found" ) - return _to_response(pitch) + return pitch_to_response(pitch) @router.post("/{pitch_id}/approve", response_model=PitchResponse) @@ -192,9 +132,9 @@ async def approve_pitch( ValidationError, ProvisioningError, ) as exc: - raise _to_http_exc(exc) from exc + raise pitch_error_to_http_exc(exc) from exc await db.commit() - return _to_response(pitch) + return pitch_to_response(pitch) @router.post("/{pitch_id}/reject", response_model=PitchResponse) @@ -219,6 +159,6 @@ async def reject_pitch( try: pitch = await service.reject(pitch_id, data.notes, agent.agent_id) except (NotFoundError, ConflictError) as exc: - raise _to_http_exc(exc) from exc + raise pitch_error_to_http_exc(exc) from exc await db.commit() - return _to_response(pitch) + return pitch_to_response(pitch) diff --git a/roboco/api/routes/playbooks.py b/roboco/api/routes/playbooks.py index 7ae38d1f..729e1c2e 100644 --- a/roboco/api/routes/playbooks.py +++ b/roboco/api/routes/playbooks.py @@ -9,24 +9,15 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, Query, status -from roboco.api.deps import CurrentAgentContext, DbSession +from roboco.api.deps import CurrentAgentContext, DbSession, require_auditor_or_ceo from roboco.api.schemas.playbook import PlaybookRejectBody -from roboco.models import AgentRole from roboco.models.playbook import Playbook from roboco.services.base import ConflictError, NotFoundError from roboco.services.playbook import get_playbook_service router = APIRouter() -_CURATOR_ROLES = frozenset({AgentRole.AUDITOR, AgentRole.CEO}) - - -def _require_curator(agent: CurrentAgentContext) -> None: - if agent.role not in _CURATOR_ROLES: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Only the Auditor or CEO may curate playbooks", - ) +_CURATOR_DETAIL = "Only the Auditor or CEO may curate playbooks" @router.get("", response_model=list[Playbook]) @@ -36,7 +27,7 @@ async def list_playbooks( status_filter: str = Query(default="draft", alias="status"), ) -> list[Playbook]: """List playbooks by status (default: drafts awaiting review).""" - _require_curator(agent) + require_auditor_or_ceo(agent.role, _CURATOR_DETAIL) svc = get_playbook_service(db) rows = ( await svc.list_approved() @@ -51,7 +42,7 @@ async def approve_playbook( playbook_id: UUID, db: DbSession, agent: CurrentAgentContext ) -> Playbook: """Approve a draft playbook → approved (and indexed into the KB).""" - _require_curator(agent) + require_auditor_or_ceo(agent.role, _CURATOR_DETAIL) try: svc = get_playbook_service(db) playbook = await svc.approve(playbook_id, approver_id=agent.agent_id) @@ -81,7 +72,7 @@ async def reject_playbook( agent: CurrentAgentContext, ) -> Playbook: """Reject a draft playbook → archived, with the Auditor's reason.""" - _require_curator(agent) + require_auditor_or_ceo(agent.role, _CURATOR_DETAIL) try: svc = get_playbook_service(db) playbook = await svc.reject( @@ -106,7 +97,7 @@ async def archive_playbook( playbook_id: UUID, db: DbSession, agent: CurrentAgentContext ) -> Playbook: """Retire an approved playbook → archived (and de-indexed from the KB).""" - _require_curator(agent) + require_auditor_or_ceo(agent.role, _CURATOR_DETAIL) try: svc = get_playbook_service(db) playbook = await svc.archive(playbook_id, approver_id=agent.agent_id) diff --git a/roboco/api/routes/product.py b/roboco/api/routes/product.py index f81e1e15..33aebcfd 100644 --- a/roboco/api/routes/product.py +++ b/roboco/api/routes/product.py @@ -15,20 +15,17 @@ from roboco.api.schemas.product import ( ProductResponse, ProductSummaryResponse, ProductUpdateRequest, + cell_mappings_from_request, product_to_response, product_to_summary, ) -from roboco.models.product import ProductCellMapping, ProductCreate, ProductUpdate +from roboco.models.product import ProductCreate, ProductUpdate from roboco.services.base import ConflictError from roboco.services.product import get_product_service router = APIRouter() -def _to_mappings(cells: list) -> list[ProductCellMapping]: - return [ProductCellMapping(team=c.team, project_id=c.project_id) for c in cells] - - @router.get("", response_model=list[ProductSummaryResponse]) async def list_products( db: DbSession, @@ -68,7 +65,7 @@ async def create_product( name=data.name, slug=data.slug, description=data.description, - cells=_to_mappings(data.cells), + cells=cell_mappings_from_request(data.cells), ) # The service raises ConflictError (slug already taken) before any flush. # Replacing cells then flushes child rows that can violate the @@ -128,7 +125,9 @@ async def update_product( update_data = ProductUpdate( name=data.name, description=data.description, - cells=_to_mappings(data.cells) if data.cells is not None else None, + cells=cell_mappings_from_request(data.cells) + if data.cells is not None + else None, ) # Replacing cells flushes child rows that can violate the # uq_product_projects_product_team UNIQUE (two cells with the same team in diff --git a/roboco/api/routes/project.py b/roboco/api/routes/project.py index d86b0e43..1455b94a 100644 --- a/roboco/api/routes/project.py +++ b/roboco/api/routes/project.py @@ -4,14 +4,11 @@ Project API Routes CRUD operations for managing git projects/repositories. """ -from typing import TYPE_CHECKING, Annotated, cast +from typing import Annotated, cast from uuid import UUID from fastapi import APIRouter, HTTPException, Query, status -if TYPE_CHECKING: - from roboco.db.tables import ProjectTable - from roboco.api.deps import ( CurrentAgentContext, DbSession, @@ -30,17 +27,15 @@ from roboco.api.schemas.project import ( ProjectUpdateRequest, SetWorkspaceRequest, SyncStateRequest, + conventions_action_to_response, project_to_response, project_to_summary, ) from roboco.foundation.policy.conventions.models import ConventionsStandard from roboco.models.base import Team from roboco.models.project import ProjectCreate, ProjectUpdate -from roboco.services.conventions import ( - ScaffoldResult, - get_conventions_service, -) -from roboco.services.project import ProjectService, get_project_service +from roboco.services.conventions import get_conventions_service +from roboco.services.project import get_project_service router = APIRouter() @@ -469,28 +464,6 @@ async def remove_agent_access( # ============================================================================= -async def _get_project_or_404( - service: ProjectService, project_id: str -) -> "ProjectTable": - """Resolve a project by UUID or slug, raising 404 when absent.""" - try: - project = await service.get(UUID(project_id)) - except ValueError: - project = await service.get_by_slug(project_id) - if project is None: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail=f"Project not found: {project_id}", - ) - return project - - -def _action_response(result: ScaffoldResult) -> ConventionsActionResponse: - return ConventionsActionResponse( - pr_number=result.pr_number, branch=result.branch, created=result.created - ) - - @router.get("/{project_id}/conventions", response_model=ConventionsResponse) async def get_conventions( project_id: str, @@ -498,7 +471,7 @@ async def get_conventions( _agent: CurrentAgentContext, ) -> ConventionsResponse: """Return the project's effective conventions map + its current health.""" - project = await _get_project_or_404(get_project_service(db), project_id) + project = await get_project_service(db).get_by_id_or_slug_or_404(project_id) conv = get_conventions_service(db) # Ensure a default-branch read clone once, then read the map + health from # it. This is the backfill: a project created before the standard existed @@ -526,10 +499,10 @@ async def update_conventions( ) -> ConventionsActionResponse: """Commit an edited conventions standard back to the repo via a PR (PM+).""" require_pm_or_above(agent.role, "edit conventions") - project = await _get_project_or_404(get_project_service(db), project_id) + project = await get_project_service(db).get_by_id_or_slug_or_404(project_id) result = await get_conventions_service(db).commit_standard(project, standard) await db.commit() - return _action_response(result) + return conventions_action_to_response(result) @router.post( @@ -542,10 +515,10 @@ async def restore_conventions( ) -> ConventionsActionResponse: """Re-commit the conventions file from the last-good map via a PR (PM+).""" require_pm_or_above(agent.role, "restore conventions") - project = await _get_project_or_404(get_project_service(db), project_id) + project = await get_project_service(db).get_by_id_or_slug_or_404(project_id) result = await get_conventions_service(db).restore(project) await db.commit() - return _action_response(result) + return conventions_action_to_response(result) @router.get( @@ -559,7 +532,7 @@ async def get_conventions_findings( limit: Annotated[int, Query(ge=1, le=200)] = 50, ) -> list[ConventionFinding]: """Recent architectural-conventions findings for the project (violations feed).""" - project = await _get_project_or_404(get_project_service(db), project_id) + project = await get_project_service(db).get_by_id_or_slug_or_404(project_id) rows = await get_conventions_service(db).recent_findings( UUID(str(project.id)), limit ) diff --git a/roboco/api/routes/provider.py b/roboco/api/routes/provider.py index 8144d61d..20bfb86d 100644 --- a/roboco/api/routes/provider.py +++ b/roboco/api/routes/provider.py @@ -33,6 +33,8 @@ from roboco.api.schemas.provider import ( SetGrokKeyRequest, SetOllamaKeyRequest, assignment_to_response, + parse_complexity_override, + provider_remediation, routing_preset_to_summary, ) from roboco.billing.pricing import input_price_per_million @@ -59,42 +61,6 @@ _COMPLEXITY_OVERRIDE_ROLES: frozenset[str] = frozenset( {"developer", "qa", "documenter"} ) -# Human remediation hint per provider type, for a complexity override that -# resolves to a not-ready (disabled / unconfigured) provider. -_PROVIDER_REMEDIATION: dict[ModelProvider, str] = { - ModelProvider.GROK: "Save the Grok (xAI) API key first (PUT /providers/grok-key).", - ModelProvider.OLLAMA_CLOUD: ( - "Save an Ollama Cloud API key first (PUT /providers/ollama-key)." - ), - ModelProvider.LOCAL: ( - "Configure + test the self-hosted server first (PUT /providers/self-hosted)." - ), - ModelProvider.ANTHROPIC: "The Anthropic provider is disabled — re-enable it first.", - ModelProvider.OPENAI: ( - "Codex authenticates via a mounted ChatGPT-subscription ~/.codex " - "directory, not a key — enable it via the Codex mode button, or " - "assign a Codex model to an agent in Mix mode (both force-enable " - "the row)." - ), - ModelProvider.GEMINI: ( - "Gemini authenticates via a mounted OAuth ~/.gemini credential, not " - "a key — enable it via the Gemini mode button, or assign a Gemini " - "model to an agent in Mix mode (both force-enable the row)." - ), - ModelProvider.KIMI: ( - "Kimi authenticates via a shared, symlinked-in ~/.kimi-code " - "subscription credential, not a key — enable it via the Kimi mode " - "button, or assign a Kimi model to an agent in Mix mode (both " - "force-enable the row)." - ), -} - - -def _provider_remediation(provider_type: ModelProvider) -> str: - return _PROVIDER_REMEDIATION.get( - provider_type, f"The {provider_type.value} provider is not configured." - ) - # ============================================================================= # CATALOG @@ -474,26 +440,6 @@ async def apply_mode( # ============================================================================= -def _parse_complexity_override( - scope_value: str, model_name: str -) -> ComplexityOverrideResponse | None: - """Parse a ROLE scope_value into a response row, or None if not a - well-formed "role:low"/"role:high" compound key (a plain role row, or a - malformed compound value, are both silently skipped).""" - role, sep, complexity = scope_value.partition(":") - if not sep or not role: - return None - if complexity == "low": - return ComplexityOverrideResponse( - role=role, complexity="low", model_name=model_name - ) - if complexity == "high": - return ComplexityOverrideResponse( - role=role, complexity="high", model_name=model_name - ) - return None - - @router.get("/complexity-overrides", response_model=list[ComplexityOverrideResponse]) async def get_complexity_overrides( db: DbSession, @@ -507,7 +453,7 @@ async def get_complexity_overrides( for row in rows: if row.scope != AssignmentScope.ROLE or not row.scope_value: continue - parsed = _parse_complexity_override(row.scope_value, row.model_name) + parsed = parse_complexity_override(row.scope_value, row.model_name) if parsed is not None: overrides.append(parsed) return overrides @@ -582,7 +528,7 @@ async def set_complexity_override( if not provider.enabled or ( provider.type == ModelProvider.LOCAL and not provider.base_url ): - remediation = _provider_remediation(provider.type) + remediation = provider_remediation(provider.type) raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=( diff --git a/roboco/api/routes/release.py b/roboco/api/routes/release.py index 78752e61..00ec7f8a 100644 --- a/roboco/api/routes/release.py +++ b/roboco/api/routes/release.py @@ -6,7 +6,7 @@ cancels the proposal (freeing the one-open dedup for a fresh re-assessment). Nothing here publishes without the CEO's explicit POST. """ -from typing import TYPE_CHECKING, cast +from typing import cast from uuid import UUID from fastapi import APIRouter, HTTPException, status @@ -15,70 +15,31 @@ from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from roboco.api.deps import CurrentAgentContext, DbSession, require_ceo_role from roboco.api.schemas.release import ( ReleaseExecuteResponse, - ReleaseGapModel, ReleaseProposalResponse, ReleaseRejectRequest, - ReleaseReportModel, ) -from roboco.foundation.policy.content import markers from roboco.security import guard_deco from roboco.services.release_proposal import ( dispatch_approve, get_release_proposal_service, - is_approve_in_flight, + task_to_proposal_response, ) -if TYPE_CHECKING: - from roboco.db.tables import TaskTable - router = APIRouter() -def _require_ceo(agent: CurrentAgentContext) -> None: - require_ceo_role(agent.role, action="view or act on release proposals") - - -def _status_value(task: "TaskTable") -> str: - raw = task.status - return raw.value if hasattr(raw, "value") else str(raw) - - -def _to_response(task: "TaskTable") -> ReleaseProposalResponse: - report = markers.get_release_report(task) or {} - outcome = markers.get_release_execute_outcome(task) - return ReleaseProposalResponse( - task_id=str(task.id), - title=task.title, - status=_status_value(task), - required_changes=markers.get_release_required_changes(task), - execute_status=outcome[0] if outcome else None, - execute_detail=outcome[1] if outcome else None, - execute_in_flight=is_approve_in_flight(UUID(str(task.id))), - report=ReleaseReportModel( - proposed_version=report.get("proposed_version", ""), - bump_kind=report.get("bump_kind", ""), - change_summary=report.get("change_summary", []), - drafted_changelog=report.get("drafted_changelog", ""), - version_bump_plan=report.get("version_bump_plan", []), - gaps=[ReleaseGapModel(**gap) for gap in report.get("gaps", [])], - migration_notes=report.get("migration_notes", []), - gate_state=report.get("gate_state", "unknown"), - ), - ) - - @router.get("/proposal", response_model=ReleaseProposalResponse) async def get_release_proposal( db: DbSession, agent: CurrentAgentContext ) -> ReleaseProposalResponse: """The single held release proposal awaiting the CEO (404 when none).""" - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on release proposals") task = await get_release_proposal_service(db).open_proposal() if task is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="No open release proposal" ) - return _to_response(task) + return task_to_proposal_response(task) @router.post( @@ -103,7 +64,7 @@ async def approve_release_proposal( published/already_published, else the proposal stays open for retry). A second click is refused by the Redis mutex (``already_in_progress``). """ - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on release proposals") svc = get_release_proposal_service(db) task = await svc.open_proposal() if task is None: @@ -141,7 +102,7 @@ async def reject_release_proposal( ) -> ReleaseProposalResponse: """Reject the held proposal with required changes; it is cancelled so the release manager re-assesses and may originate a fresh proposal next cycle.""" - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on release proposals") svc = get_release_proposal_service(db) task = await svc.open_proposal() if task is None: @@ -160,4 +121,4 @@ async def reject_release_proposal( ), ) await db.commit() - return _to_response(revised) + return task_to_proposal_response(revised) diff --git a/roboco/api/routes/research.py b/roboco/api/routes/research.py index bba7b150..48240321 100644 --- a/roboco/api/routes/research.py +++ b/roboco/api/routes/research.py @@ -28,7 +28,7 @@ from roboco.services.research import ( ResearchUnsupportedError, get_research_service, ) -from roboco.services.research_quota import ResearchQuotaTracker +from roboco.services.research_quota import ResearchQuotaTracker, enforce_research_quota router = APIRouter() @@ -48,28 +48,6 @@ RESEARCH_ROLES = frozenset( _quota_tracker = ResearchQuotaTracker() -def _require_research_role(agent: CurrentAgentContext) -> None: - if agent.role not in RESEARCH_ROLES: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=f"role '{agent.role}' may not use web research", - ) - - -async def _enforce_quota(agent: CurrentAgentContext) -> None: - result = await _quota_tracker.check_and_consume( - str(agent.agent_id), settings.research_daily_quota_per_agent - ) - if not result.allowed: - raise HTTPException( - status_code=status.HTTP_429_TOO_MANY_REQUESTS, - detail=( - f"daily research quota exhausted " - f"({result.limit}/day, resets {result.day} 24:00 UTC)" - ), - ) - - @router.post("/search", response_model=SearchResponse) @guard_deco.rate_limit(requests=20, window=60) @guard_deco.max_request_size(size_bytes=65536) @@ -80,8 +58,14 @@ async def research_search( data: SearchRequest, agent: CurrentAgentContext ) -> SearchResponse: """Search the public web via the configured provider (Board + PM only).""" - _require_research_role(agent) - await _enforce_quota(agent) + if agent.role not in RESEARCH_ROLES: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"role '{agent.role}' may not use web research", + ) + await enforce_research_quota( + _quota_tracker, str(agent.agent_id), settings.research_daily_quota_per_agent + ) service = get_research_service() try: outcome = await service.search(data.query, data.max_results) @@ -118,8 +102,14 @@ async def research_fetch( data: FetchRequest, agent: CurrentAgentContext ) -> FetchResponse: """Extract readable content for a URL via the provider (Board + PM only).""" - _require_research_role(agent) - await _enforce_quota(agent) + if agent.role not in RESEARCH_ROLES: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"role '{agent.role}' may not use web research", + ) + await enforce_research_quota( + _quota_tracker, str(agent.agent_id), settings.research_daily_quota_per_agent + ) service = get_research_service() try: outcome = await service.fetch(data.url, data.max_chars) diff --git a/roboco/api/routes/secretary.py b/roboco/api/routes/secretary.py index bd55d2b9..ee9baae5 100644 --- a/roboco/api/routes/secretary.py +++ b/roboco/api/routes/secretary.py @@ -10,7 +10,7 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, Query, status -from roboco.api.deps import CurrentAgentContext, DbSession +from roboco.api.deps import CurrentAgentContext, DbSession, require_role_in from roboco.api.schemas.secretary import ( CompanyStateResponse, DirectiveDecision, @@ -26,20 +26,16 @@ from roboco.services.secretary import get_secretary_service router = APIRouter() _SECRETARY_OR_CEO = frozenset({AgentRole.SECRETARY, AgentRole.CEO}) - - -def _require(agent: CurrentAgentContext, allowed: frozenset[AgentRole]) -> None: - if agent.role not in allowed: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=f"role '{agent.role}' not permitted on the Secretary surface", - ) +_CEO_ONLY = frozenset({AgentRole.CEO}) +_SURFACE_DETAIL = "role '{role}' not permitted on the Secretary surface" @router.get("/state", response_model=CompanyStateResponse) async def read_state(db: DbSession, agent: CurrentAgentContext) -> CompanyStateResponse: """Compact company-state snapshot (Secretary or CEO).""" - _require(agent, _SECRETARY_OR_CEO) + require_role_in( + agent.role, _SECRETARY_OR_CEO, _SURFACE_DETAIL.format(role=agent.role) + ) state = await get_secretary_service(db).read_company_state() return CompanyStateResponse(**state) @@ -56,7 +52,9 @@ async def search_tasks( The CEO refers to tasks by NAME in the Secretary chat; this resolves a name to concrete ids so a directive can target the right task. """ - _require(agent, _SECRETARY_OR_CEO) + require_role_in( + agent.role, _SECRETARY_OR_CEO, _SURFACE_DETAIL.format(role=agent.role) + ) from roboco.services.task import get_task_service rows = await get_task_service(db).search_tasks(q, limit=limit) @@ -78,7 +76,9 @@ async def read_task( ) -> dict[str, object]: """Read one task's full detail — content, notes, plan, progress, PR ref (Secretary or CEO). Secretary FULL task access.""" - _require(agent, _SECRETARY_OR_CEO) + require_role_in( + agent.role, _SECRETARY_OR_CEO, _SURFACE_DETAIL.format(role=agent.role) + ) try: return await get_secretary_service(db).read_task(task_id) except NotFoundError as exc: @@ -101,7 +101,9 @@ async def submit_directive( data: DirectiveSubmit, db: DbSession, agent: CurrentAgentContext ) -> DirectiveResponse: """Submit a directive (Secretary or CEO). Gated kinds queue; others run.""" - _require(agent, _SECRETARY_OR_CEO) + require_role_in( + agent.role, _SECRETARY_OR_CEO, _SURFACE_DETAIL.format(role=agent.role) + ) try: kind = DirectiveKind(data.kind) except ValueError as exc: @@ -125,7 +127,7 @@ async def list_directives( db: DbSession, agent: CurrentAgentContext, status_filter: str | None = None ) -> list[DirectiveResponse]: """List directives (CEO only); optional status filter.""" - _require(agent, frozenset({AgentRole.CEO})) + require_role_in(agent.role, _CEO_ONLY, _SURFACE_DETAIL.format(role=agent.role)) parsed: DirectiveStatus | None = None if status_filter: try: @@ -147,7 +149,7 @@ async def confirm_directive( directive_id: UUID, db: DbSession, agent: CurrentAgentContext ) -> DirectiveResponse: """CEO confirms a pending directive — it executes with CEO authority.""" - _require(agent, frozenset({AgentRole.CEO})) + require_role_in(agent.role, _CEO_ONLY, _SURFACE_DETAIL.format(role=agent.role)) service = get_secretary_service(db) try: row = await service.confirm_directive(directive_id, agent.agent_id) @@ -174,7 +176,7 @@ async def reject_directive( agent: CurrentAgentContext, ) -> DirectiveResponse: """CEO rejects a pending directive.""" - _require(agent, frozenset({AgentRole.CEO})) + require_role_in(agent.role, _CEO_ONLY, _SURFACE_DETAIL.format(role=agent.role)) service = get_secretary_service(db) try: row = await service.reject_directive(directive_id, agent.agent_id, data.reason) diff --git a/roboco/api/routes/system.py b/roboco/api/routes/system.py index 605720ea..c932af1e 100644 --- a/roboco/api/routes/system.py +++ b/roboco/api/routes/system.py @@ -13,28 +13,15 @@ Currently exposed: from __future__ import annotations -from datetime import datetime, timedelta - from fastapi import APIRouter, Depends from roboco.api.deps import require_panel_token from roboco.api.schemas.system import RateLimitEntry, RateLimitListResponse -from roboco.services.gateway.rate_limit_tracker import RateLimitStateTracker +from roboco.services.gateway.rate_limit_tracker import RateLimitStateTracker, resume_at router = APIRouter(dependencies=[Depends(require_panel_token)]) -def _resume_at(hit_at: str | None, retry_after: float | None) -> str | None: - """Estimated lift time = hit_at + retry_after, ISO; falls back to hit_at.""" - if not hit_at or retry_after is None: - return hit_at - try: - lifted = datetime.fromisoformat(hit_at) + timedelta(seconds=retry_after) - except (ValueError, TypeError): - return hit_at - return lifted.isoformat() - - @router.get( "/rate-limits", summary="List per-provider rate-limit state", @@ -58,7 +45,7 @@ async def get_rate_limits() -> RateLimitListResponse: provider=provider, affected_agents=state.get("affected_agents", []), hit_at=state.get("activated_at"), - resume_at=_resume_at(state.get("activated_at"), state.get("retry_after")), + resume_at=resume_at(state.get("activated_at"), state.get("retry_after")), retry_after_seconds=state.get("retry_after"), ) for provider, state in states diff --git a/roboco/api/routes/work_session.py b/roboco/api/routes/work_session.py index a5883fd6..0110cf70 100644 --- a/roboco/api/routes/work_session.py +++ b/roboco/api/routes/work_session.py @@ -26,60 +26,15 @@ from roboco.api.schemas.work_session import ( session_to_response, session_to_summary, ) -from roboco.models import AgentRole -from roboco.models.permissions import AgentContext from roboco.models.work_session import WorkSessionCreate, WorkSessionStatus -from roboco.services.work_session import WorkSessionService, get_work_session_service +from roboco.services.work_session import ( + assert_session_ownership, + get_work_session_service, +) router = APIRouter() -# ============================================================================= -# OWNERSHIP GUARD -# -# Every mutating route keys off session_id alone, so without a re-check any -# developer could mutate a peer's session and any PM could merge any cell's PR -# — bypassing the verb layer's active-claimant gate. Re-assert the caller owns -# the session (dev ops) or owns the session's task cell (PM ops) before the -# service call (#158). -# ============================================================================= - - -async def _assert_ownership( - service: WorkSessionService, - session_id: UUID, - agent: AgentContext, - *, - pm_op: bool, -) -> None: - """Fetch the session and verify the caller may mutate it. - - Raises 404 for a missing session, 403 for a wrong-owner / wrong-cell caller. - Dev ops require the caller to BE the session's agent. PM ops (merge_pr) - require a cell PM to own the session's task cell; main PM / CEO / board - coordinate every cell and are admitted by the role gate alone. - """ - session = await service.get(session_id) - if not session: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail=f"Work session not found: {session_id}", - ) - if pm_op: - if agent.role == AgentRole.CELL_PM: - team = await service.task_team_for_session(session_id) - if agent.team is None or team is None or team != agent.team: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="cell PM does not own this session's task cell", - ) - elif session.agent_id != agent.agent_id: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="not the owner of this work session", - ) - - # ============================================================================= # LIST & GET ENDPOINTS # ============================================================================= @@ -213,7 +168,7 @@ async def add_commit( require_developer_or_above(agent.role, "add commits") service = get_work_session_service(db) - await _assert_ownership(service, session_id, agent, pm_op=False) + await assert_session_ownership(service, session_id, agent, pm_op=False) session = await service.add_commit(session_id, data.commit_sha) await db.commit() @@ -238,7 +193,7 @@ async def add_files_modified( require_developer_or_above(agent.role, "add files") service = get_work_session_service(db) - await _assert_ownership(service, session_id, agent, pm_op=False) + await assert_session_ownership(service, session_id, agent, pm_op=False) session = await service.add_files_modified(session_id, data.file_paths) await db.commit() @@ -268,7 +223,7 @@ async def create_pr( require_developer_or_above(agent.role, "create PRs") service = get_work_session_service(db) - await _assert_ownership(service, session_id, agent, pm_op=False) + await assert_session_ownership(service, session_id, agent, pm_op=False) session = await service.create_pr(session_id, data.pr_number, data.pr_url) await db.commit() @@ -293,7 +248,7 @@ async def update_pr_status( require_developer_or_above(agent.role, "update PR status") service = get_work_session_service(db) - await _assert_ownership(service, session_id, agent, pm_op=False) + await assert_session_ownership(service, session_id, agent, pm_op=False) session = await service.update_pr_status(session_id, data.pr_status) await db.commit() @@ -323,7 +278,7 @@ async def merge_pr( require_pm_or_above(agent.role, "merge PRs") service = get_work_session_service(db) - await _assert_ownership(service, session_id, agent, pm_op=True) + await assert_session_ownership(service, session_id, agent, pm_op=True) session = await service.merge_pr(session_id, agent.agent_id) await db.commit() @@ -352,7 +307,7 @@ async def complete_session( require_developer_or_above(agent.role, "complete sessions") service = get_work_session_service(db) - await _assert_ownership(service, session_id, agent, pm_op=False) + await assert_session_ownership(service, session_id, agent, pm_op=False) session = await service.complete(session_id) await db.commit() @@ -377,7 +332,7 @@ async def abandon_session( require_developer_or_above(agent.role, "abandon sessions") service = get_work_session_service(db) - await _assert_ownership(service, session_id, agent, pm_op=False) + await assert_session_ownership(service, session_id, agent, pm_op=False) session = await service.abandon(session_id, reason=reason) await db.commit() diff --git a/roboco/api/routes/x.py b/roboco/api/routes/x.py index 3dc3a848..8e145b4c 100644 --- a/roboco/api/routes/x.py +++ b/roboco/api/routes/x.py @@ -2,27 +2,22 @@ credentials. CEO-only throughout. Nothing here posts except an explicit ``approve``; credentials are write-only (the API never returns plaintext).""" -from typing import TYPE_CHECKING from uuid import UUID from fastapi import APIRouter, HTTPException, Query, status from roboco.api.deps import CurrentAgentContext, DbSession, require_ceo_role -from roboco.api.schemas.project_fields import task_project_fields from roboco.api.schemas.x import ( - XBarflyRefModel, - XCampaignRefModel, XCredentialsSetRequest, XCredentialsStatus, - XFeatureRefModel, - XMentionRefModel, XPostApproveRequest, XPostExecuteResponse, XPostHistoryResponse, XPostRejectRequest, XPostResponse, + task_to_post_history_response, + task_to_post_response, ) -from roboco.foundation.policy.content import markers from roboco.security import guard_deco from roboco.services.x_credentials import ( XCredentialsValidationError, @@ -30,81 +25,17 @@ from roboco.services.x_credentials import ( ) from roboco.services.x_post_service import XPostBodyTooLongError, get_x_post_service -if TYPE_CHECKING: - from roboco.db.tables import TaskTable - router = APIRouter() -def _require_ceo(agent: CurrentAgentContext) -> None: - require_ceo_role(agent.role, action="view or act on the X engine queue") - - -def _status_value(task: "TaskTable") -> str: - raw = task.status - return raw.value if hasattr(raw, "value") else str(raw) - - -def _to_response(task: "TaskTable") -> XPostResponse: - body = markers.get_x_draft_body(task) or task.description or "" - mention = markers.get_x_mention_ref(task) - feature = markers.get_x_feature_ref(task) - campaign = markers.get_x_campaign_ref(task) - barfly = markers.get_barfly_reply_ref(task) - project_slug, project_name = task_project_fields(task) - return XPostResponse( - task_id=str(task.id), - source=task.source, - title=task.title, - status=_status_value(task), - body=body, - char_count=len(body), - release_version=markers.get_x_release_version(task), - mention=XMentionRefModel(**mention) if mention else None, - feature=XFeatureRefModel(**feature) if feature else None, - campaign=XCampaignRefModel(**campaign) if campaign else None, - barfly=XBarflyRefModel(**barfly) if barfly else None, - reject_reason=markers.get_x_reject_reason(task), - project_slug=project_slug, - project_name=project_name, - ) - - @router.get("/posts", response_model=list[XPostResponse]) async def list_x_posts( db: DbSession, agent: CurrentAgentContext ) -> list[XPostResponse]: """Every held X draft (release posts + mention replies) awaiting the CEO.""" - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on the X engine queue") tasks = await get_x_post_service(db).list_open_posts() - return [_to_response(t) for t in tasks] - - -def _to_history_response(task: "TaskTable") -> XPostHistoryResponse: - body = markers.get_x_draft_body(task) or task.description or "" - mention = markers.get_x_mention_ref(task) - feature = markers.get_x_feature_ref(task) - campaign = markers.get_x_campaign_ref(task) - barfly = markers.get_barfly_reply_ref(task) - project_slug, project_name = task_project_fields(task) - return XPostHistoryResponse( - task_id=str(task.id), - source=task.source, - title=task.title, - status=_status_value(task), - body=body, - char_count=len(body), - release_version=markers.get_x_release_version(task), - mention=XMentionRefModel(**mention) if mention else None, - feature=XFeatureRefModel(**feature) if feature else None, - campaign=XCampaignRefModel(**campaign) if campaign else None, - barfly=XBarflyRefModel(**barfly) if barfly else None, - tweet_id=markers.get_x_posted_tweet_id(task), - reject_reason=markers.get_x_reject_reason(task), - acted_at=task.updated_at or task.created_at, - project_slug=project_slug, - project_name=project_name, - ) + return [task_to_post_response(t) for t in tasks] @router.get("/posts/history", response_model=list[XPostHistoryResponse]) @@ -114,9 +45,9 @@ async def list_x_post_history( limit: int = Query(default=50, ge=1, le=200), ) -> list[XPostHistoryResponse]: """Posted or rejected X drafts, newest-acted-first, bounded by `limit`.""" - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on the X engine queue") tasks = await get_x_post_service(db).list_post_history(limit=limit) - return [_to_history_response(t) for t in tasks] + return [task_to_post_history_response(t) for t in tasks] @router.post("/posts/{task_id}/approve", response_model=XPostExecuteResponse) @@ -135,7 +66,7 @@ async def approve_x_post( Idempotent: approving an already-posted draft returns ``already_posted`` without calling the X API again. """ - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on the X engine queue") svc = get_x_post_service(db) try: result = await svc.approve(task_id, data.edited_body) @@ -165,7 +96,7 @@ async def reject_x_post( agent: CurrentAgentContext, ) -> XPostResponse: """Decline the draft with a reason; it is cancelled (never posted).""" - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on the X engine queue") svc = get_x_post_service(db) task = await svc.reject(task_id, data.reason) if task is None: @@ -173,7 +104,7 @@ async def reject_x_post( status_code=status.HTTP_404_NOT_FOUND, detail="No such open X draft" ) await db.commit() - return _to_response(task) + return task_to_post_response(task) @router.get("/credentials", response_model=XCredentialsStatus) @@ -181,7 +112,7 @@ async def get_x_credentials( db: DbSession, agent: CurrentAgentContext ) -> XCredentialsStatus: """Whether the four OAuth 1.0a secrets are stored. Never the secrets.""" - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on the X engine queue") has_creds = await get_x_credentials_service(db).has_credentials() return XCredentialsStatus(has_credentials=has_creds) @@ -197,7 +128,7 @@ async def set_x_credentials( data: XCredentialsSetRequest, db: DbSession, agent: CurrentAgentContext ) -> XCredentialsStatus: """Set (or, passing all four empty, clear) the four OAuth 1.0a secrets.""" - _require_ceo(agent) + require_ceo_role(agent.role, action="view or act on the X engine queue") svc = get_x_credentials_service(db) try: has_creds = await svc.set_credentials( diff --git a/roboco/api/schemas/pitch.py b/roboco/api/schemas/pitch.py index 050db986..f890648f 100644 --- a/roboco/api/schemas/pitch.py +++ b/roboco/api/schemas/pitch.py @@ -1,7 +1,12 @@ """Pitch API schemas — Board proposals and CEO decisions.""" +from typing import TYPE_CHECKING + from pydantic import BaseModel, Field +if TYPE_CHECKING: + from roboco.db.tables import PitchTable + class PitchCreateRequest(BaseModel): """Board authors a pitch.""" @@ -36,3 +41,25 @@ class PitchResponse(BaseModel): provisioned_project_ids: list[str] seed_task_id: str | None = None created_at: str | None = None + + +def pitch_to_response(pitch: "PitchTable") -> PitchResponse: + """Convert a PitchTable to PitchResponse.""" + return PitchResponse( + id=str(pitch.id), + title=pitch.title, + slug=pitch.slug, + problem=pitch.problem, + proposed_solution=pitch.proposed_solution, + target_cells=list(pitch.target_cells or []), + status=pitch.status, + created_by=str(pitch.created_by), + decided_by=str(pitch.decided_by) if pitch.decided_by else None, + decision_notes=pitch.decision_notes, + provisioned_product_id=( + str(pitch.provisioned_product_id) if pitch.provisioned_product_id else None + ), + provisioned_project_ids=list(pitch.provisioned_project_ids or []), + seed_task_id=str(pitch.seed_task_id) if pitch.seed_task_id else None, + created_at=pitch.created_at.isoformat() if pitch.created_at else None, + ) diff --git a/roboco/api/schemas/product.py b/roboco/api/schemas/product.py index 1b5d68b9..17307977 100644 --- a/roboco/api/schemas/product.py +++ b/roboco/api/schemas/product.py @@ -8,6 +8,7 @@ from uuid import UUID from pydantic import BaseModel, ConfigDict, Field from roboco.foundation.identity import Team +from roboco.models.product import ProductCellMapping if TYPE_CHECKING: from roboco.db.tables import ProductTable @@ -84,6 +85,11 @@ class ProductUpdateRequest(BaseModel): cells: list[CellMapping] | None = None +def cell_mappings_from_request(cells: list[CellMapping]) -> list[ProductCellMapping]: + """Convert request-body cell mappings into the service-layer model.""" + return [ProductCellMapping(team=c.team, project_id=c.project_id) for c in cells] + + def product_to_response(product: "ProductTable") -> ProductResponse: return ProductResponse( id=typing_cast("UUID", product.id), diff --git a/roboco/api/schemas/project.py b/roboco/api/schemas/project.py index 86df25ca..befc7b9c 100644 --- a/roboco/api/schemas/project.py +++ b/roboco/api/schemas/project.py @@ -15,6 +15,7 @@ from roboco.models.base import Team if TYPE_CHECKING: from roboco.db.tables import ProjectTable + from roboco.services.conventions import ScaffoldResult # ============================================================================= @@ -327,6 +328,15 @@ def project_to_response(project: "ProjectTable") -> ProjectResponse: ) +def conventions_action_to_response( + result: "ScaffoldResult", +) -> ConventionsActionResponse: + """Convert a conventions scaffold/restore/save ``ScaffoldResult``.""" + return ConventionsActionResponse( + pr_number=result.pr_number, branch=result.branch, created=result.created + ) + + def project_to_summary( project: "ProjectTable", task_counts: "ProjectTaskCounts | None" = None, diff --git a/roboco/api/schemas/provider.py b/roboco/api/schemas/provider.py index e2dad1c2..0700e1cd 100644 --- a/roboco/api/schemas/provider.py +++ b/roboco/api/schemas/provider.py @@ -18,13 +18,51 @@ from uuid import UUID # noqa: TC003 (pydantic needs the type at runtime) from pydantic import BaseModel, Field -from roboco.models.base import AssignmentScope, ModelProvider # noqa: TC001 +from roboco.models.base import AssignmentScope, ModelProvider from roboco.utils.converters import require_uuid if TYPE_CHECKING: from roboco.db.tables import ModelAssignmentTable, RoutingPresetTable +# Human remediation hint per provider type, for a complexity override that +# resolves to a not-ready (disabled / unconfigured) provider. +_PROVIDER_REMEDIATION: dict[ModelProvider, str] = { + ModelProvider.GROK: "Save the Grok (xAI) API key first (PUT /providers/grok-key).", + ModelProvider.OLLAMA_CLOUD: ( + "Save an Ollama Cloud API key first (PUT /providers/ollama-key)." + ), + ModelProvider.LOCAL: ( + "Configure + test the self-hosted server first (PUT /providers/self-hosted)." + ), + ModelProvider.ANTHROPIC: "The Anthropic provider is disabled — re-enable it first.", + ModelProvider.OPENAI: ( + "Codex authenticates via a mounted ChatGPT-subscription ~/.codex " + "directory, not a key — enable it via the Codex mode button, or " + "assign a Codex model to an agent in Mix mode (both force-enable " + "the row)." + ), + ModelProvider.GEMINI: ( + "Gemini authenticates via a mounted OAuth ~/.gemini credential, not " + "a key — enable it via the Gemini mode button, or assign a Gemini " + "model to an agent in Mix mode (both force-enable the row)." + ), + ModelProvider.KIMI: ( + "Kimi authenticates via a shared, symlinked-in ~/.kimi-code " + "subscription credential, not a key — enable it via the Kimi mode " + "button, or assign a Kimi model to an agent in Mix mode (both " + "force-enable the row)." + ), +} + + +def provider_remediation(provider_type: ModelProvider) -> str: + """Human remediation hint for a not-ready (disabled/unconfigured) provider.""" + return _PROVIDER_REMEDIATION.get( + provider_type, f"The {provider_type.value} provider is not configured." + ) + + # ============================================================================= # CATALOG # ============================================================================= @@ -276,6 +314,26 @@ class ComplexityOverrideResponse(BaseModel): warning: str | None = None +def parse_complexity_override( + scope_value: str, model_name: str +) -> ComplexityOverrideResponse | None: + """Parse a ROLE scope_value into a response row, or None if not a + well-formed "role:low"/"role:high" compound key (a plain role row, or a + malformed compound value, are both silently skipped).""" + role, sep, complexity = scope_value.partition(":") + if not sep or not role: + return None + if complexity == "low": + return ComplexityOverrideResponse( + role=role, complexity="low", model_name=model_name + ) + if complexity == "high": + return ComplexityOverrideResponse( + role=role, complexity="high", model_name=model_name + ) + return None + + # ============================================================================= # ROUTING PRESETS (named, full snapshots of the routing state) # ============================================================================= diff --git a/roboco/api/schemas/x.py b/roboco/api/schemas/x.py index 8d43f5fc..9e26f220 100644 --- a/roboco/api/schemas/x.py +++ b/roboco/api/schemas/x.py @@ -1,11 +1,17 @@ """Schemas for the X (Twitter) engine's CEO surface.""" from datetime import datetime +from typing import TYPE_CHECKING from pydantic import BaseModel, Field +from roboco.api.schemas.project_fields import task_project_fields +from roboco.foundation.policy.content import markers from roboco.services.x_client import MAX_TWEET_CHARS +if TYPE_CHECKING: + from roboco.db.tables import TaskTable + class XMentionRefModel(BaseModel): """The mention a held reply answers.""" @@ -61,6 +67,38 @@ class XPostResponse(BaseModel): project_name: str | None = None +def _task_status_value(task: "TaskTable") -> str: + """Render a task's status as a plain string, enum or raw value alike.""" + raw = task.status + return raw.value if hasattr(raw, "value") else str(raw) + + +def task_to_post_response(task: "TaskTable") -> XPostResponse: + """Render a held/open X-draft task as the CEO-facing queue entry.""" + body = markers.get_x_draft_body(task) or task.description or "" + mention = markers.get_x_mention_ref(task) + feature = markers.get_x_feature_ref(task) + campaign = markers.get_x_campaign_ref(task) + barfly = markers.get_barfly_reply_ref(task) + project_slug, project_name = task_project_fields(task) + return XPostResponse( + task_id=str(task.id), + source=task.source, + title=task.title, + status=_task_status_value(task), + body=body, + char_count=len(body), + release_version=markers.get_x_release_version(task), + mention=XMentionRefModel(**mention) if mention else None, + feature=XFeatureRefModel(**feature) if feature else None, + campaign=XCampaignRefModel(**campaign) if campaign else None, + barfly=XBarflyRefModel(**barfly) if barfly else None, + reject_reason=markers.get_x_reject_reason(task), + project_slug=project_slug, + project_name=project_name, + ) + + class XPostApproveRequest(BaseModel): """Approve a draft, optionally overwriting the body first.""" @@ -102,6 +140,34 @@ class XPostHistoryResponse(BaseModel): project_name: str | None = None +def task_to_post_history_response(task: "TaskTable") -> XPostHistoryResponse: + """Render a posted/rejected X-draft task as the CEO-facing history entry.""" + body = markers.get_x_draft_body(task) or task.description or "" + mention = markers.get_x_mention_ref(task) + feature = markers.get_x_feature_ref(task) + campaign = markers.get_x_campaign_ref(task) + barfly = markers.get_barfly_reply_ref(task) + project_slug, project_name = task_project_fields(task) + return XPostHistoryResponse( + task_id=str(task.id), + source=task.source, + title=task.title, + status=_task_status_value(task), + body=body, + char_count=len(body), + release_version=markers.get_x_release_version(task), + mention=XMentionRefModel(**mention) if mention else None, + feature=XFeatureRefModel(**feature) if feature else None, + campaign=XCampaignRefModel(**campaign) if campaign else None, + barfly=XBarflyRefModel(**barfly) if barfly else None, + tweet_id=markers.get_x_posted_tweet_id(task), + reject_reason=markers.get_x_reject_reason(task), + acted_at=task.updated_at or task.created_at, + project_slug=project_slug, + project_name=project_name, + ) + + class XCredentialsStatus(BaseModel): """Whether the four OAuth 1.0a secrets are stored. Never the secrets themselves.""" diff --git a/roboco/services/gateway/kb_authz.py b/roboco/services/gateway/kb_authz.py index bf486b92..5f4ef723 100644 --- a/roboco/services/gateway/kb_authz.py +++ b/roboco/services/gateway/kb_authz.py @@ -17,11 +17,15 @@ from __future__ import annotations from typing import TYPE_CHECKING +from fastapi import status +from fastapi.responses import JSONResponse + from roboco.models.permissions import KB_PERMISSIONS from roboco.services.gateway.envelope import Envelope if TYPE_CHECKING: from roboco.models.permissions import AgentContext + from roboco.services.base import UnauthorizedError from roboco.services.permissions import PermissionService @@ -62,6 +66,28 @@ def authorize_kb_action( ) +def kb_denial_response( + permissions: PermissionService, + agent: AgentContext, + action: str, +) -> JSONResponse | None: + """Gateway Envelope (HTTP 403) when the KB action is denied, else None. + + The authorization decision itself lives in ``authorize_kb_action``; this + only renders a denial verdict at the HTTP boundary. The body is the + Envelope wire-dict at top level — not nested under ``detail`` — so the + agent receives a non-null ``remediate`` it can act on, matching the + gateway Envelope contract. + """ + denial = authorize_kb_action(permissions, agent, action) + if denial is None: + return None + return JSONResponse( + status_code=status.HTTP_403_FORBIDDEN, + content=denial.as_dict(), + ) + + _DOCS_WRITE_ACTIONS = frozenset({"write_doc", "delete_doc"}) @@ -88,3 +114,18 @@ def docs_denial_envelope(action: str, reason: str | None) -> Envelope: message=reason or f"not authorized: {action}", remediate=remediate, ) + + +def docs_unauthorized_response(err: UnauthorizedError) -> JSONResponse: + """Render a docs-service denial as the gateway Envelope (HTTP 403). + + The RBAC decision is made in ``DocsService`` (it raises + ``UnauthorizedError``); this only renders that denial at the HTTP + boundary. The body is the Envelope wire-dict at top level so the agent + receives a non-null ``remediate`` instead of a bare ``detail`` string. + """ + envelope = docs_denial_envelope(err.action, err.reason) + return JSONResponse( + status_code=status.HTTP_403_FORBIDDEN, + content=envelope.as_dict(), + ) diff --git a/roboco/services/gateway/rate_limit_tracker.py b/roboco/services/gateway/rate_limit_tracker.py index c94ad435..878a4a66 100644 --- a/roboco/services/gateway/rate_limit_tracker.py +++ b/roboco/services/gateway/rate_limit_tracker.py @@ -10,7 +10,7 @@ cross-reconnection persistence requirement. from __future__ import annotations import json -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import Any import redis.asyncio as redis @@ -85,6 +85,17 @@ return 0 """ +def resume_at(hit_at: str | None, retry_after: float | None) -> str | None: + """Estimated lift time = hit_at + retry_after, ISO; falls back to hit_at.""" + if not hit_at or retry_after is None: + return hit_at + try: + lifted = datetime.fromisoformat(hit_at) + timedelta(seconds=retry_after) + except (ValueError, TypeError): + return hit_at + return lifted.isoformat() + + class RateLimitStateTracker: """Track rate-limit state for a single AI provider in Redis. diff --git a/roboco/services/git.py b/roboco/services/git.py index e194bb4c..ec7bf2bd 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -24,6 +24,7 @@ from typing import TYPE_CHECKING, Any, ClassVar, cast from uuid import UUID import httpx +from fastapi import HTTPException, status if TYPE_CHECKING: from collections.abc import Coroutine @@ -376,6 +377,27 @@ def _select_ci_head_run(runs: list[dict[str, Any]]) -> dict[str, Any]: return max(same_head, key=lambda r: int(r.get("run_attempt") or 0)) +def translate_git_error(e: ServiceError | GitError) -> HTTPException: + """Translate a git-route service/git error into an HTTPException.""" + if isinstance(e, NotFoundError): + return HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.message) + if isinstance(e, UnauthorizedError): + return HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=e.message) + if isinstance(e, ValidationError): + return HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=e.message) + if isinstance(e, GitTimeoutError): + return HTTPException( + status_code=status.HTTP_504_GATEWAY_TIMEOUT, detail=e.message + ) + if isinstance(e, GitCommandError): + return HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.message + ) + return HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=e.message + ) + + @dataclass(frozen=True) class _CiRunQuery: """Bundle of per-project inputs to a CI-run fetch (repo ref, branch, token, diff --git a/roboco/services/pitch.py b/roboco/services/pitch.py index 62d5a23d..9fa01153 100644 --- a/roboco/services/pitch.py +++ b/roboco/services/pitch.py @@ -21,11 +21,12 @@ from __future__ import annotations from typing import TYPE_CHECKING +from fastapi import HTTPException, status from sqlalchemy import select from roboco.config import settings from roboco.db.tables import PitchTable -from roboco.foundation.identity import Team +from roboco.foundation.identity import CELL_TEAMS, Team from roboco.models.base import Complexity, TaskNature, TaskStatus, TaskType from roboco.models.pitch import PitchCreate, PitchStatus from roboco.models.product import ProductCellMapping, ProductCreate, ProductUpdate @@ -40,6 +41,7 @@ from roboco.services.base import ( ) from roboco.services.github_provisioning import ( ProvisioningDisabledError, + ProvisioningError, get_github_provisioning_service, ) from roboco.services.product import get_product_service @@ -56,6 +58,47 @@ if TYPE_CHECKING: _DESCRIPTION_CAP = 500 +# Known pitch-flow exceptions, in priority order (ProvisioningDisabledError +# before its parent ProvisioningError so the more specific 400 wins). +_SERVICE_ERROR_HTTP: tuple[tuple[type[Exception], int], ...] = ( + (NotFoundError, status.HTTP_404_NOT_FOUND), + (ProvisioningDisabledError, status.HTTP_400_BAD_REQUEST), + (ProvisioningError, status.HTTP_502_BAD_GATEWAY), + (ConflictError, status.HTTP_409_CONFLICT), + (ValidationError, status.HTTP_400_BAD_REQUEST), +) + + +def pitch_error_to_http_exc(exc: Exception) -> HTTPException: + """Translate a known pitch service/provisioning error into an HTTPException.""" + detail = getattr(exc, "message", None) or str(exc) + for exc_type, code in _SERVICE_ERROR_HTTP: + if isinstance(exc, exc_type): + return HTTPException(status_code=code, detail=detail) + return HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=detail + ) + + +def parse_cell_teams(raw: list[str]) -> list[Team]: + """Validate + convert pitch ``target_cells`` strings into ``Team`` members.""" + cells: list[Team] = [] + for c in raw: + try: + team = Team(c) + except ValueError as exc: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + detail=f"unknown cell '{c}'", + ) from exc + if team not in CELL_TEAMS: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + detail=f"'{c}' is not a cell team", + ) + cells.append(team) + return cells + class PitchService(BaseService): """CRUD + approve/reject for Board pitches.""" diff --git a/roboco/services/project.py b/roboco/services/project.py index 71559011..fa60f506 100644 --- a/roboco/services/project.py +++ b/roboco/services/project.py @@ -9,6 +9,7 @@ from typing import ClassVar from typing import cast as typing_cast from uuid import UUID +from fastapi import HTTPException, status from sqlalchemy import case, func, select from sqlalchemy.ext.asyncio import AsyncSession @@ -210,6 +211,33 @@ class ProjectService(BaseService): raise NotFoundError("Project", str(project_id)) return project + async def get_by_id_or_slug_or_404(self, project_id: str) -> ProjectTable: + """Resolve a project by UUID or slug, raising HTTP 404 when absent. + + Used by the conventions endpoints, which accept either form in the + ``project_id`` path parameter. + """ + try: + project = await self.get(UUID(project_id)) + except ValueError: + project = await self.get_by_slug(project_id) + if project is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Project not found: {project_id}", + ) + return project + + async def resolve_slug_or_404(self, identifier: str) -> str: + """Resolve a project identifier (UUID string or slug) to its slug. + + Callers pass whatever string they have — a human-readable slug like + "roboco" or a UUID. Verifies the project exists and returns the + canonical slug so downstream git-service calls work. + """ + project = await self.get_by_id_or_slug_or_404(identifier) + return str(project.slug) + async def update( self, project_id: UUID, diff --git a/roboco/services/release_proposal.py b/roboco/services/release_proposal.py index 551d8c8a..cbd287df 100644 --- a/roboco/services/release_proposal.py +++ b/roboco/services/release_proposal.py @@ -18,6 +18,11 @@ from uuid import uuid4 import redis.asyncio as redis +from roboco.api.schemas.release import ( + ReleaseGapModel, + ReleaseProposalResponse, + ReleaseReportModel, +) from roboco.config import settings from roboco.foundation.policy.content import markers from roboco.models.base import TaskStatus @@ -635,6 +640,37 @@ def is_approve_in_flight(task_id: UUID) -> bool: return task_id in _INFLIGHT_APPROVES +def _task_status_value(task: TaskTable) -> str: + """Render a task's status as a plain string, enum or raw value alike.""" + raw = task.status + return raw.value if hasattr(raw, "value") else str(raw) + + +def task_to_proposal_response(task: TaskTable) -> ReleaseProposalResponse: + """Render a held release-proposal task as the CEO-facing response shape.""" + report = markers.get_release_report(task) or {} + outcome = markers.get_release_execute_outcome(task) + return ReleaseProposalResponse( + task_id=str(task.id), + title=task.title, + status=_task_status_value(task), + required_changes=markers.get_release_required_changes(task), + execute_status=outcome[0] if outcome else None, + execute_detail=outcome[1] if outcome else None, + execute_in_flight=is_approve_in_flight(cast("UUID", task.id)), + report=ReleaseReportModel( + proposed_version=report.get("proposed_version", ""), + bump_kind=report.get("bump_kind", ""), + change_summary=report.get("change_summary", []), + drafted_changelog=report.get("drafted_changelog", ""), + version_bump_plan=report.get("version_bump_plan", []), + gaps=[ReleaseGapModel(**gap) for gap in report.get("gaps", [])], + migration_notes=report.get("migration_notes", []), + gate_state=report.get("gate_state", "unknown"), + ), + ) + + def dispatch_approve( task_id: UUID, session_factory: async_sessionmaker[AsyncSession] ) -> asyncio.Task[None]: diff --git a/roboco/services/research_quota.py b/roboco/services/research_quota.py index b238faaa..80485f92 100644 --- a/roboco/services/research_quota.py +++ b/roboco/services/research_quota.py @@ -13,6 +13,7 @@ from dataclasses import dataclass from datetime import UTC, datetime import redis.asyncio as redis +from fastapi import HTTPException, status from roboco.config import settings @@ -76,3 +77,18 @@ class ResearchQuotaTracker: if self._redis is not None: await self._redis.aclose() self._redis = None + + +async def enforce_research_quota( + tracker: ResearchQuotaTracker, agent_id: str, daily_quota: int +) -> None: + """Consume one unit of ``agent_id``'s daily research quota, or raise 429.""" + result = await tracker.check_and_consume(agent_id, daily_quota) + if not result.allowed: + raise HTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + detail=( + f"daily research quota exhausted " + f"({result.limit}/day, resets {result.day} 24:00 UTC)" + ), + ) diff --git a/roboco/services/work_session.py b/roboco/services/work_session.py index 7d0f283e..5353ed2a 100644 --- a/roboco/services/work_session.py +++ b/roboco/services/work_session.py @@ -6,15 +6,16 @@ WorkSessions track branch management, commits, and PR lifecycle. """ from datetime import UTC, datetime -from typing import Any, ClassVar, cast +from typing import TYPE_CHECKING, Any, ClassVar, cast from uuid import UUID +from fastapi import HTTPException, status from sqlalchemy import and_, select from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession from roboco.db.tables import ProjectTable, TaskTable, WorkSessionTable -from roboco.models import Team +from roboco.models import AgentRole, Team from roboco.models.work_session import ( WorkSessionCreate, WorkSessionStatus, @@ -27,6 +28,9 @@ from roboco.services.base import ( ValidationError, ) +if TYPE_CHECKING: + from roboco.models.permissions import AgentContext + class WorkSessionService(BaseService): """ @@ -711,6 +715,52 @@ class WorkSessionService(BaseService): return work_session.pr_number is None +# ============================================================================= +# OWNERSHIP GUARD +# +# Every mutating route keys off session_id alone, so without a re-check any +# developer could mutate a peer's session and any PM could merge any cell's PR +# — bypassing the verb layer's active-claimant gate. Re-assert the caller owns +# the session (dev ops) or owns the session's task cell (PM ops) before the +# service call (#158). +# ============================================================================= + + +async def assert_session_ownership( + service: WorkSessionService, + session_id: UUID, + agent: "AgentContext", + *, + pm_op: bool, +) -> None: + """Fetch the session and verify the caller may mutate it. + + Raises 404 for a missing session, 403 for a wrong-owner / wrong-cell caller. + Dev ops require the caller to BE the session's agent. PM ops (merge_pr) + require a cell PM to own the session's task cell; main PM / CEO / board + coordinate every cell and are admitted by the role gate alone. + """ + session = await service.get(session_id) + if not session: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Work session not found: {session_id}", + ) + if pm_op: + if agent.role == AgentRole.CELL_PM: + team = await service.task_team_for_session(session_id) + if agent.team is None or team is None or team != agent.team: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="cell PM does not own this session's task cell", + ) + elif session.agent_id != agent.agent_id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="not the owner of this work session", + ) + + # ============================================================================= # SERVICE FACTORY # ============================================================================= diff --git a/roboco/utils/converters.py b/roboco/utils/converters.py index 81adc629..423054f4 100644 --- a/roboco/utils/converters.py +++ b/roboco/utils/converters.py @@ -78,6 +78,61 @@ def to_python_uuid(value: Any) -> PythonUUID | None: return PythonUUID(str(value)) +def compute_file_range( + *, + total: int, + line: int | None, + context: int, + explicit_range: tuple[int, int] | None, + max_lines: int, +) -> tuple[int, int, bool]: + """Resolve the (start, end, truncated) slice for a file-content read. + + An explicit ``explicit_range`` (start, end) wins; else ``line`` centers a + context window; else the whole file. Whichever branch resolves the + window, it is capped at ``max_lines`` lines afterward. Returns 1-based + inclusive [start, end] and whether the slice is shorter than the file. + """ + if explicit_range is not None: + s, e_ = explicit_range + elif line is not None: + s = max(1, line - context) + e_ = min(total, line + context) + else: + s, e_ = 1, total + + s = max(1, min(s, total)) + e_ = max(s, min(e_, total)) + + truncated = e_ < total + if e_ - s + 1 > max_lines: + e_ = s + max_lines - 1 + truncated = True + return s, e_, truncated + + +def parse_branch_line(line: str) -> tuple[str, bool, str | None] | None: + """Classify one `%(refname)|%(objectname:short)` line as (name, is_remote, + last_commit), or None for skippable entries (blank, origin/HEAD, other ref + namespaces). Full refname, not `:short` — a remote-tracking ref shortens to + `origin/`, indistinguishable from a local branch literally named + that; classify on the `refs/heads/` vs `refs/remotes/` prefix instead. + """ + if not line: + return None + parts = line.split("|") + ref = parts[0] + last_commit = parts[1] if len(parts) > 1 else None + if ref.startswith("refs/heads/"): + return ref.removeprefix("refs/heads/"), False, last_commit + if ref.startswith("refs/remotes/"): + _remote_name, _, name = ref.removeprefix("refs/remotes/").partition("/") + if not name or name == "HEAD": + return None # origin/HEAD is a symbolic pointer, not a branch + return name, True, last_commit + return None + + def to_python_uuid_list(values: list[Any] | None) -> list[PythonUUID]: """ Convert list of SQLAlchemy UUIDs to Python UUIDs. diff --git a/tests/integration/test_git_routes.py b/tests/integration/test_git_routes.py index 6448e295..f31d7180 100644 --- a/tests/integration/test_git_routes.py +++ b/tests/integration/test_git_routes.py @@ -14,7 +14,6 @@ import pytest_asyncio from fastapi import FastAPI from httpx import ASGITransport, AsyncClient from roboco.api.deps import get_agent_context, get_db -from roboco.api.routes.git import _translate_error from roboco.api.routes.git import router as git_router from roboco.db.tables import AgentTable, ProjectTable from roboco.exceptions import GitCommandError, GitTimeoutError @@ -26,6 +25,7 @@ from roboco.services.base import ( UnauthorizedError, ValidationError, ) +from roboco.services.git import translate_git_error if TYPE_CHECKING: from collections.abc import AsyncGenerator, AsyncIterator @@ -215,23 +215,23 @@ async def test_status_not_found(git_client: dict) -> None: @pytest.mark.asyncio async def test_status_git_timeout_directly() -> None: - """Exercise _translate_error's GitTimeoutError branch directly. + """Exercise translate_git_error's GitTimeoutError branch directly. The route uses `except ServiceError as e` from services.base, but GitTimeoutError extends roboco.exceptions.ServiceError (different - class), so it never enters _translate_error in practice. We invoke + class), so it never enters translate_git_error in practice. We invoke the helper directly to cover the branch. """ err = GitTimeoutError("git status", 10) - http_exc = _translate_error(err) + http_exc = translate_git_error(err) assert http_exc.status_code == HTTPStatus.GATEWAY_TIMEOUT @pytest.mark.asyncio async def test_status_git_command_error_directly() -> None: - """Direct invocation of _translate_error's GitCommandError branch.""" + """Direct invocation of translate_git_error's GitCommandError branch.""" err = GitCommandError("git status", "stderr") - http_exc = _translate_error(err) + http_exc = translate_git_error(err) assert http_exc.status_code == HTTPStatus.INTERNAL_SERVER_ERROR diff --git a/tests/unit/api/routes/test_git_project_lookup.py b/tests/unit/api/routes/test_git_project_lookup.py index 54730890..fa376795 100644 --- a/tests/unit/api/routes/test_git_project_lookup.py +++ b/tests/unit/api/routes/test_git_project_lookup.py @@ -1,13 +1,13 @@ -"""Unit tests: _resolve_project_slug accepts slug or UUID.""" +"""Unit tests: ProjectService.resolve_slug_or_404 accepts slug or UUID.""" from __future__ import annotations -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock from uuid import UUID, uuid4 import pytest from fastapi import HTTPException -from roboco.api.routes.git import _resolve_project_slug +from roboco.services.project import ProjectService _HTTP_404 = 404 @@ -24,15 +24,15 @@ def _make_project(slug: str, uid: UUID) -> MagicMock: async def test_resolve_project_slug_accepts_slug() -> None: """A plain slug string resolves to the project's slug.""" project = _make_project("roboco", uuid4()) - mock_service = MagicMock() - mock_service.get_by_slug = AsyncMock(return_value=project) + service = ProjectService(MagicMock()) + service.get_by_slug = AsyncMock(return_value=project) # type: ignore[method-assign] + service.get = AsyncMock() # type: ignore[method-assign] - with patch("roboco.api.routes.git.get_project_service", return_value=mock_service): - result = await _resolve_project_slug("roboco", MagicMock()) + result = await service.resolve_slug_or_404("roboco") assert result == "roboco" - mock_service.get_by_slug.assert_awaited_once_with("roboco") - mock_service.get.assert_not_called() + service.get_by_slug.assert_awaited_once_with("roboco") + service.get.assert_not_called() @pytest.mark.asyncio @@ -40,28 +40,25 @@ async def test_resolve_project_slug_accepts_uuid() -> None: """A UUID string resolves to the project's slug.""" uid = uuid4() project = _make_project("roboco", uid) - mock_service = MagicMock() - mock_service.get = AsyncMock(return_value=project) + service = ProjectService(MagicMock()) + service.get = AsyncMock(return_value=project) # type: ignore[method-assign] + service.get_by_slug = AsyncMock() # type: ignore[method-assign] - with patch("roboco.api.routes.git.get_project_service", return_value=mock_service): - result = await _resolve_project_slug(str(uid), MagicMock()) + result = await service.resolve_slug_or_404(str(uid)) assert result == "roboco" - mock_service.get.assert_awaited_once_with(UUID(str(uid))) - mock_service.get_by_slug.assert_not_called() + service.get.assert_awaited_once_with(UUID(str(uid))) + service.get_by_slug.assert_not_called() @pytest.mark.asyncio async def test_resolve_project_slug_raises_404_for_missing_slug() -> None: """Unknown slug raises HTTPException 404.""" - mock_service = MagicMock() - mock_service.get_by_slug = AsyncMock(return_value=None) + service = ProjectService(MagicMock()) + service.get_by_slug = AsyncMock(return_value=None) # type: ignore[method-assign] - with ( - patch("roboco.api.routes.git.get_project_service", return_value=mock_service), - pytest.raises(HTTPException) as exc_info, - ): - await _resolve_project_slug("nonexistent", MagicMock()) + with pytest.raises(HTTPException) as exc_info: + await service.resolve_slug_or_404("nonexistent") assert exc_info.value.status_code == _HTTP_404 assert "nonexistent" in exc_info.value.detail @@ -71,14 +68,11 @@ async def test_resolve_project_slug_raises_404_for_missing_slug() -> None: async def test_resolve_project_slug_raises_404_for_missing_uuid() -> None: """UUID that matches no project raises HTTPException 404.""" uid = uuid4() - mock_service = MagicMock() - mock_service.get = AsyncMock(return_value=None) + service = ProjectService(MagicMock()) + service.get = AsyncMock(return_value=None) # type: ignore[method-assign] - with ( - patch("roboco.api.routes.git.get_project_service", return_value=mock_service), - pytest.raises(HTTPException) as exc_info, - ): - await _resolve_project_slug(str(uid), MagicMock()) + with pytest.raises(HTTPException) as exc_info: + await service.resolve_slug_or_404(str(uid)) assert exc_info.value.status_code == _HTTP_404 assert str(uid) in exc_info.value.detail diff --git a/tests/unit/api/routes/test_x_project_fields.py b/tests/unit/api/routes/test_x_project_fields.py index 712c8119..52359470 100644 --- a/tests/unit/api/routes/test_x_project_fields.py +++ b/tests/unit/api/routes/test_x_project_fields.py @@ -1,9 +1,9 @@ -"""``roboco/api/routes/x.py`` response-builder wiring for project_slug/ +"""``roboco/api/schemas/x.py`` response-builder wiring for project_slug/ project_name. The sa_inspect(task).unloaded guard branches themselves are covered once on the shared helper in tests/unit/api/schemas/test_project_fields.py -— this only asserts _to_response/_to_history_response actually populate -the response from it (loaded case; a real ORM task always resolves the -"loaded" branch since ``project`` is lazy="joined").""" +— this only asserts task_to_post_response/task_to_post_history_response +actually populate the response from it (loaded case; a real ORM task always +resolves the "loaded" branch since ``project`` is lazy="joined").""" from __future__ import annotations @@ -11,11 +11,11 @@ from types import SimpleNamespace from typing import Any from unittest.mock import MagicMock, patch -from roboco.api.routes.x import _to_history_response, _to_response +from roboco.api.schemas.x import task_to_post_history_response, task_to_post_response def _stub_task(*, with_project: bool = False) -> Any: - """A TaskTable stand-in matching _to_response/_to_history_response's reads.""" + """A TaskTable stand-in matching the response builders' reads.""" return SimpleNamespace( id="task-1", source="x_post", @@ -44,7 +44,7 @@ def test_to_response_includes_project_fields_when_loaded() -> None: "roboco.api.schemas.project_fields.sa_inspect", return_value=_loaded_inspector(), ): - resp = _to_response(_stub_task(with_project=True)) + resp = task_to_post_response(_stub_task(with_project=True)) assert resp.project_slug == "acme-robotics" assert resp.project_name == "Acme Robotics" @@ -54,7 +54,7 @@ def test_to_response_omits_project_fields_when_project_unset() -> None: "roboco.api.schemas.project_fields.sa_inspect", return_value=_loaded_inspector(), ): - resp = _to_response(_stub_task(with_project=False)) + resp = task_to_post_response(_stub_task(with_project=False)) assert resp.project_slug is None assert resp.project_name is None @@ -64,7 +64,7 @@ def test_to_history_response_includes_project_fields_when_loaded() -> None: "roboco.api.schemas.project_fields.sa_inspect", return_value=_loaded_inspector(), ): - resp = _to_history_response(_stub_task(with_project=True)) + resp = task_to_post_history_response(_stub_task(with_project=True)) assert resp.project_slug == "acme-robotics" assert resp.project_name == "Acme Robotics" @@ -74,6 +74,6 @@ def test_to_history_response_omits_project_fields_when_project_unset() -> None: "roboco.api.schemas.project_fields.sa_inspect", return_value=_loaded_inspector(), ): - resp = _to_history_response(_stub_task(with_project=False)) + resp = task_to_post_history_response(_stub_task(with_project=False)) assert resp.project_slug is None assert resp.project_name is None diff --git a/tests/unit/api/test_dashboard_auditor_auth.py b/tests/unit/api/test_dashboard_auditor_auth.py index 641c15f0..224f700f 100644 --- a/tests/unit/api/test_dashboard_auditor_auth.py +++ b/tests/unit/api/test_dashboard_auditor_auth.py @@ -1,7 +1,7 @@ """Dashboard auditor flag/report mutating routes (``create_auditor_flag``, ``resolve_auditor_flag``, ``create_auditor_report``, ``send_auditor_report``) -are gated to AUDITOR or CEO via a ``CurrentAgentContext`` dependency plus a -coarse role gate, mirroring ``roboco/api/routes/playbooks.py::_require_curator``. +are gated to AUDITOR or CEO via a ``CurrentAgentContext`` dependency plus +``roboco.api.deps.require_auditor_or_ceo`` — the same check playbooks.py uses. """ from __future__ import annotations diff --git a/tests/unit/api/test_git_file_range.py b/tests/unit/api/test_git_file_range.py index bd74baad..ce21c4d9 100644 --- a/tests/unit/api/test_git_file_range.py +++ b/tests/unit/api/test_git_file_range.py @@ -1,4 +1,4 @@ -"""Unit tests for the /git/file range computation (roboco.api.routes.git). +"""Unit tests for the /git/file range computation (roboco.utils.converters). Pure logic — no DB, no git. Covers the line/context windowing, explicit range, whole-file cap, and truncation flag. @@ -6,50 +6,79 @@ range, whole-file cap, and truncation flag. from __future__ import annotations -from roboco.api.routes.git import _FILE_MAX_LINES, _compute_file_range +from roboco.api.routes.git import _FILE_MAX_LINES +from roboco.utils.converters import compute_file_range class TestComputeFileRange: def test_line_centers_context_window(self) -> None: - s, e_, trunc = _compute_file_range( - total=100, line=50, context=10, start=None, end=None + s, e_, trunc = compute_file_range( + total=100, + line=50, + context=10, + explicit_range=None, + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (40, 60, True) def test_line_window_clamps_to_file_start(self) -> None: - s, e_, trunc = _compute_file_range( - total=100, line=3, context=10, start=None, end=None + s, e_, trunc = compute_file_range( + total=100, + line=3, + context=10, + explicit_range=None, + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (1, 13, True) def test_line_window_clamps_to_file_end(self) -> None: - s, e_, trunc = _compute_file_range( - total=100, line=98, context=10, start=None, end=None + s, e_, trunc = compute_file_range( + total=100, + line=98, + context=10, + explicit_range=None, + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (88, 100, False) def test_explicit_start_end_override_line(self) -> None: - s, e_, trunc = _compute_file_range( - total=100, line=50, context=10, start=5, end=8 + s, e_, trunc = compute_file_range( + total=100, + line=50, + context=10, + explicit_range=(5, 8), + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (5, 8, True) def test_whole_file_when_no_range_args(self) -> None: - s, e_, trunc = _compute_file_range( - total=50, line=None, context=10, start=None, end=None + s, e_, trunc = compute_file_range( + total=50, + line=None, + context=10, + explicit_range=None, + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (1, 50, False) def test_whole_file_capped_when_huge(self) -> None: total = _FILE_MAX_LINES + 500 - s, e_, trunc = _compute_file_range( - total=total, line=None, context=10, start=None, end=None + s, e_, trunc = compute_file_range( + total=total, + line=None, + context=10, + explicit_range=None, + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (1, _FILE_MAX_LINES, True) def test_empty_file(self) -> None: - s, e_, trunc = _compute_file_range( - total=0, line=None, context=10, start=None, end=None + s, e_, trunc = compute_file_range( + total=0, + line=None, + context=10, + explicit_range=None, + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (1, 1, False) @@ -57,14 +86,22 @@ class TestComputeFileRange: # start=1, end=total-1 is not the exact-whole-file shape, but the # resolved window is still oversized and must be capped. total = 50000 - s, e_, trunc = _compute_file_range( - total=total, line=None, context=10, start=1, end=total - 1 + s, e_, trunc = compute_file_range( + total=total, + line=None, + context=10, + explicit_range=(1, total - 1), + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (1, _FILE_MAX_LINES, True) def test_oversized_line_context_window_is_capped(self) -> None: total = 10000 - s, e_, trunc = _compute_file_range( - total=total, line=5000, context=3000, start=None, end=None + s, e_, trunc = compute_file_range( + total=total, + line=5000, + context=3000, + explicit_range=None, + max_lines=_FILE_MAX_LINES, ) assert (s, e_, trunc) == (2000, 2000 + _FILE_MAX_LINES - 1, True) diff --git a/tests/unit/services/test_git_rebase.py b/tests/unit/services/test_git_rebase.py index b39b87a6..1b7ec401 100644 --- a/tests/unit/services/test_git_rebase.py +++ b/tests/unit/services/test_git_rebase.py @@ -816,6 +816,7 @@ async def test_rebase_endpoint_pm_gets_200() -> None: mock_project.slug = "roboco" mock_project_svc = MagicMock() mock_project_svc.get_by_slug = AsyncMock(return_value=mock_project) + mock_project_svc.resolve_slug_or_404 = AsyncMock(return_value="roboco") # Mock git service → workspace + rebase succeed without conflict mock_git_svc = MagicMock()