Code quality

This commit is contained in:
Renn F
2026-05-06 22:25:39 +02:00
parent 9aa30fb945
commit f0eec854d1
11 changed files with 283 additions and 415 deletions
+15 -10
View File
@@ -314,6 +314,21 @@ async def get_ceo_team_details(
# ============================================================================= # =============================================================================
@router.get("/kanban/main-pm")
async def get_main_pm_kanban(
db: DbSession,
) -> dict[str, Any]:
"""Get the Main PM cross-cell kanban board.
Declared BEFORE `/kanban/{team}` so FastAPI matches the literal
`main-pm` segment instead of treating it as a `Team` enum value
(which would 422 since "main-pm" isn't a Team member).
"""
kanban_service = get_kanban_service(db)
board = await kanban_service.get_main_pm_board_flat()
return board.model_dump()
@router.get("/kanban/{team}") @router.get("/kanban/{team}")
async def get_team_kanban( async def get_team_kanban(
team: Team, team: Team,
@@ -359,16 +374,6 @@ async def get_team_kanban(
) )
@router.get("/kanban/main-pm")
async def get_main_pm_kanban(
db: DbSession,
) -> dict[str, Any]:
"""Get the Main PM cross-cell kanban board."""
kanban_service = get_kanban_service(db)
board = await kanban_service.get_main_pm_board_flat()
return board.model_dump()
# ============================================================================= # =============================================================================
# AGENT STATUS ENDPOINTS # AGENT STATUS ENDPOINTS
# ============================================================================= # =============================================================================
+19 -12
View File
@@ -48,7 +48,7 @@ from roboco.api.schemas.git import (
GitPushResponse, GitPushResponse,
GitStatusResponse, GitStatusResponse,
) )
from roboco.exceptions import GitCommandError, GitTimeoutError from roboco.exceptions import GitCommandError, GitError, GitTimeoutError
from roboco.logging import get_logger from roboco.logging import get_logger
from roboco.services.base import ( from roboco.services.base import (
NotFoundError, NotFoundError,
@@ -66,8 +66,15 @@ router = APIRouter()
# Expected number of parts in log format output # Expected number of parts in log format output
_LOG_FORMAT_PARTS = 5 _LOG_FORMAT_PARTS = 5
# Catch tuple for service-layer errors. `roboco.exceptions.GitError` is a
# distinct class from `roboco.services.base.ServiceError` (it extends the
# `roboco.exceptions.ServiceError` class), so listing both is required for
# git timeouts/command failures to be translated to 504/500 instead of
# bubbling as 500 Internal Server Errors with no `detail`.
_TranslatableError = (ServiceError, GitError)
def _translate_error(e: ServiceError) -> HTTPException:
def _translate_error(e: ServiceError | GitError) -> HTTPException:
"""Translate service errors to HTTP exceptions.""" """Translate service errors to HTTP exceptions."""
if isinstance(e, NotFoundError): if isinstance(e, NotFoundError):
return HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.message) return HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=e.message)
@@ -139,7 +146,7 @@ async def get_git_status(
ahead, ahead,
behind, behind,
) = await git_service.get_status(workspace) ) = await git_service.get_status(workspace)
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
return GitStatusResponse( return GitStatusResponse(
@@ -190,7 +197,7 @@ async def get_git_log(
stderr=log_result.stderr[:200] if log_result.stderr else "", stderr=log_result.stderr[:200] if log_result.stderr else "",
) )
return GitLogResponse(project_slug=project_slug, branch=branch, commits=[]) return GitLogResponse(project_slug=project_slug, branch=branch, commits=[])
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
commits = [] commits = []
@@ -237,7 +244,7 @@ async def list_branches(
args.append("-a") args.append("-a")
branch_result = await git_service._run_git(workspace, args) branch_result = await git_service._run_git(workspace, args)
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
branches = [] branches = []
@@ -296,7 +303,7 @@ async def get_git_diff(
if staged: if staged:
stat_args.append("--staged") stat_args.append("--staged")
stat_result = await git_service._run_git(workspace, stat_args) stat_result = await git_service._run_git(workspace, stat_args)
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
files_changed = stat_result.stdout.count("\n") - 1 if stat_result.stdout else 0 files_changed = stat_result.stdout.count("\n") - 1 if stat_result.stdout else 0
@@ -331,7 +338,7 @@ async def create_commit(
insertions, insertions,
deletions, deletions,
) = await git_service.commit_for_task(agent.agent_id, data) ) = await git_service.commit_for_task(agent.agent_id, data)
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
return GitCommitResponse( return GitCommitResponse(
@@ -355,7 +362,7 @@ async def push_commits(
branch, commits_pushed = await git_service.push_for_task( branch, commits_pushed = await git_service.push_for_task(
agent.agent_id, agent.role, data agent.agent_id, agent.role, data
) )
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
return GitPushResponse( return GitPushResponse(
@@ -381,7 +388,7 @@ async def create_branch(
branch_name, created_from = await git_service.create_branch_for_task( branch_name, created_from = await git_service.create_branch_for_task(
agent.agent_id, data agent.agent_id, data
) )
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
return GitCreateBranchResponse( return GitCreateBranchResponse(
@@ -407,7 +414,7 @@ async def checkout_branch(
git_service = get_git_service(db) git_service = get_git_service(db)
try: try:
await git_service.checkout_branch_for_agent(agent.agent_id, data) await git_service.checkout_branch_for_agent(agent.agent_id, data)
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
return GitCheckoutResponse( return GitCheckoutResponse(
@@ -432,7 +439,7 @@ async def create_pull_request(
source_branch, source_branch,
target_branch, target_branch,
) = await git_service.create_pr_for_task(agent.agent_id, data) ) = await git_service.create_pr_for_task(agent.agent_id, data)
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
return GitCreatePRResponse( return GitCreatePRResponse(
@@ -456,7 +463,7 @@ async def merge_pull_request(
target_branch, merge_commit = await git_service.merge_pr_for_task( target_branch, merge_commit = await git_service.merge_pr_for_task(
agent.agent_id, agent.role, data agent.agent_id, agent.role, data
) )
except ServiceError as e: except _TranslatableError as e:
raise _translate_error(e) from e raise _translate_error(e) from e
return GitMergePRResponse( return GitMergePRResponse(
+23 -20
View File
@@ -432,6 +432,29 @@ async def get_stats(
) )
@router.get("/stats/staleness")
async def check_staleness(
agent: CurrentAgentContext,
permissions: PermissionServiceDep,
) -> dict[str, Any]:
"""
Check if indexes are stale (source files modified after last indexing).
Returns staleness info for file-based indexes (CODE, DOCUMENTATION).
Declared BEFORE `/stats/{index_type}` so FastAPI matches the literal
`staleness` segment instead of treating it as an `index_type` param.
"""
if not permissions.can_perform_kb_action(agent, KBAction.VIEW_STATS):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Not authorized to view index staleness",
)
service = await get_optimal_service()
return await service.check_index_staleness()
@router.get("/stats/{index_type}", response_model=SingleIndexStatsResponse) @router.get("/stats/{index_type}", response_model=SingleIndexStatsResponse)
async def get_single_index_stats( async def get_single_index_stats(
index_type: str, index_type: str,
@@ -465,26 +488,6 @@ async def get_single_index_stats(
) )
@router.get("/stats/staleness")
async def check_staleness(
agent: CurrentAgentContext,
permissions: PermissionServiceDep,
) -> dict[str, Any]:
"""
Check if indexes are stale (source files modified after last indexing).
Returns staleness info for file-based indexes (CODE, DOCUMENTATION).
"""
if not permissions.can_perform_kb_action(agent, KBAction.VIEW_STATS):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Not authorized to view index staleness",
)
service = await get_optimal_service()
return await service.check_index_staleness()
@router.get("/health", response_model=RAGHealthResponse) @router.get("/health", response_model=RAGHealthResponse)
async def rag_health_check() -> RAGHealthResponse: async def rag_health_check() -> RAGHealthResponse:
"""Check RAG system health (embedding, LLM, vector store).""" """Check RAG system health (embedding, LLM, vector store)."""
+52 -57
View File
@@ -115,18 +115,8 @@ async def create_task(
detail="Not authorized to create tasks", detail="Not authorized to create tasks",
) )
# Validate: all tasks require project_id # `data.project_id` is `UUID` (required) on TaskCreate, so pydantic
if not data.project_id: # rejects missing/null values with 422 before this handler runs.
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"error": {
"code": "PROJECT_REQUIRED",
"message": "All tasks require project_id",
"hint": "Specify project_id for the git repository",
}
},
)
# Acceptance criteria required — without them, QA has nothing to # Acceptance criteria required — without them, QA has nothing to
# verify and the task is structurally unclosable. # verify and the task is structurally unclosable.
@@ -389,6 +379,56 @@ async def get_task_stats_by_team(
return TaskCountResponse(counts=counts) return TaskCountResponse(counts=counts)
# Static-segment routes must be declared BEFORE `/{task_id}` so FastAPI
# matches the literal path instead of treating the segment as a UUID
# (which would 422 on these names).
@router.get("/awaiting-pm-review", response_model=list[TaskResponse])
async def get_awaiting_pm_review_tasks(
db: DbSession,
agent: CurrentAgentContext,
permissions: PermissionServiceDep,
team: Team | None = None,
) -> list[TaskResponse]:
"""Get tasks awaiting PM review."""
service = get_task_service(db)
# Apply team filter based on permissions
can_view_all = permissions.can_perform_task_action(agent, TaskAction.VIEW_ALL)
effective_team = team if can_view_all else agent.team
tasks = await service.list_awaiting_pm_review(effective_team)
return task_list_to_response(tasks)
@router.get("/awaiting-ceo-approval", response_model=list[TaskResponse])
async def get_awaiting_ceo_approval_tasks(
db: DbSession,
agent: CurrentAgentContext,
permissions: PermissionServiceDep,
) -> list[TaskResponse]:
"""Get tasks awaiting CEO approval.
CEO approval queue is org-wide (no team filter).
Only visible to PMs and above.
"""
# Only PMs and above can view the CEO approval queue
can_view_all = permissions.can_perform_task_action(agent, TaskAction.VIEW_ALL)
is_pm = agent.role in (AgentRole.CELL_PM, AgentRole.MAIN_PM)
is_ceo = agent.role == AgentRole.CEO
if not (can_view_all or is_pm or is_ceo):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Only PMs and management can view CEO approval queue",
)
service = get_task_service(db)
tasks = await service.list_awaiting_ceo_approval()
return task_list_to_response(tasks)
@router.get("/{task_id}", response_model=TaskResponse) @router.get("/{task_id}", response_model=TaskResponse)
async def get_task( async def get_task(
task_id: UUID, task_id: UUID,
@@ -1173,51 +1213,6 @@ async def cancel_task(
# ============================================================================= # =============================================================================
@router.get("/awaiting-pm-review", response_model=list[TaskResponse])
async def get_awaiting_pm_review_tasks(
db: DbSession,
agent: CurrentAgentContext,
permissions: PermissionServiceDep,
team: Team | None = None,
) -> list[TaskResponse]:
"""Get tasks awaiting PM review."""
service = get_task_service(db)
# Apply team filter based on permissions
can_view_all = permissions.can_perform_task_action(agent, TaskAction.VIEW_ALL)
effective_team = team if can_view_all else agent.team
tasks = await service.list_awaiting_pm_review(effective_team)
return task_list_to_response(tasks)
@router.get("/awaiting-ceo-approval", response_model=list[TaskResponse])
async def get_awaiting_ceo_approval_tasks(
db: DbSession,
agent: CurrentAgentContext,
permissions: PermissionServiceDep,
) -> list[TaskResponse]:
"""Get tasks awaiting CEO approval.
CEO approval queue is org-wide (no team filter).
Only visible to PMs and above.
"""
# Only PMs and above can view the CEO approval queue
can_view_all = permissions.can_perform_task_action(agent, TaskAction.VIEW_ALL)
is_pm = agent.role in (AgentRole.CELL_PM, AgentRole.MAIN_PM)
is_ceo = agent.role == AgentRole.CEO
if not (can_view_all or is_pm or is_ceo):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Only PMs and management can view CEO approval queue",
)
service = get_task_service(db)
tasks = await service.list_awaiting_ceo_approval()
return task_list_to_response(tasks)
@router.post("/{task_id}/escalate-to-ceo", response_model=TaskResponse) @router.post("/{task_id}/escalate-to-ceo", response_model=TaskResponse)
async def escalate_to_ceo( async def escalate_to_ceo(
task_id: UUID, task_id: UUID,
@@ -1293,15 +1293,13 @@ class Choreographer:
parent: Any, parent: Any,
inputs: DelegateInputs, inputs: DelegateInputs,
) -> Envelope | None: ) -> Envelope | None:
"""Slug / project_id / enum guards. Pure data-shape checks.""" """project_id / enum guards. Pure data-shape checks.
from roboco.seeds.initial_data import AGENT_UUIDS
if inputs.assigned_to not in AGENT_UUIDS: The slug-validity check used to live here, but `_delegate_role_guards`
return Envelope.invalid_state( runs first and `_validate_delegation_chain` rejects any slug outside
message=f"unknown agent slug: {inputs.assigned_to!r}", the allowed delegation targets which is a strict subset of
remediate=f"valid slugs: {sorted(AGENT_UUIDS)}", `AGENT_UUIDS` so any AGENT_UUIDS check here was unreachable.
context_briefing=await self._briefing_for(pm_agent_id, parent_task_id), """
)
if parent.project_id is None: if parent.project_id is None:
return Envelope.invalid_state( return Envelope.invalid_state(
message="parent task has no project_id", message="parent task has no project_id",
+127 -191
View File
@@ -41,10 +41,13 @@ if TYPE_CHECKING:
async def a2a_setup( async def a2a_setup(
db_session: AsyncSession, db_session: AsyncSession,
) -> AsyncIterator[dict]: ) -> AsyncIterator[dict]:
# Use the canonical seed slugs so the A2A policy matrix recognizes
# role + team and lets same-cell pairs talk. Random suffixes would
# leave them with role="unknown" → policy denies everything.
dev = AgentTable( dev = AgentTable(
id=uuid4(), id=uuid4(),
name="Dev", name="Dev",
slug=f"be-dev-{uuid4().hex[:8]}", slug="be-dev-1",
role=AgentRole.DEVELOPER, role=AgentRole.DEVELOPER,
team=Team.BACKEND, team=Team.BACKEND,
status=AgentStatus.ACTIVE, status=AgentStatus.ACTIVE,
@@ -57,7 +60,7 @@ async def a2a_setup(
qa = AgentTable( qa = AgentTable(
id=uuid4(), id=uuid4(),
name="QA", name="QA",
slug=f"be-qa-{uuid4().hex[:8]}", slug="be-qa",
role=AgentRole.QA, role=AgentRole.QA,
team=Team.BACKEND, team=Team.BACKEND,
status=AgentStatus.ACTIVE, status=AgentStatus.ACTIVE,
@@ -299,14 +302,10 @@ async def test_get_or_create_conversation_self_a2a_denied(a2a_setup: dict) -> No
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_get_or_create_conversation_creates(a2a_setup: dict) -> None: async def test_get_or_create_conversation_creates(a2a_setup: dict) -> None:
"""A2A between dev pairs is allowed by default; just exercise the create path.""" """A2A between two same-cell devs is allowed by the policy."""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-dev-2")
conv = await svc.get_or_create_conversation("be-dev-1", "be-dev-2") assert conv is not None
assert conv is not None
except Exception:
# If the policy blocks this pair, skip — we're focused on the call path.
pytest.skip("A2A policy denies this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -442,14 +441,11 @@ async def test_create_conversation_between_dev_and_qa_in_same_cell(
) -> None: ) -> None:
"""Cell members can A2A within their own cell.""" """Cell members can A2A within their own cell."""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") assert conv is not None
assert conv is not None # Idempotent — same agents, same conversation.
# Idempotent — same agents, same conversation. again = await svc.get_or_create_conversation("be-dev-1", "be-qa")
again = await svc.get_or_create_conversation("be-dev-1", "be-qa") assert again.id == conv.id
assert again.id == conv.id
except Exception:
pytest.skip("Policy denied this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -457,47 +453,35 @@ async def test_send_chat_message_in_existing_conversation(
a2a_setup: dict, a2a_setup: dict,
) -> None: ) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") msg = await svc.send_chat_message(UUID(conv.id), "be-dev-1", "hello")
msg = await svc.send_chat_message(UUID(conv.id), "be-dev-1", "hello") assert msg.content == "hello"
assert msg.content == "hello"
except Exception:
pytest.skip("Policy denied this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_get_messages_returns_chronological(a2a_setup: dict) -> None: async def test_get_messages_returns_chronological(a2a_setup: dict) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") cid = UUID(conv.id)
cid = UUID(conv.id) await svc.send_chat_message(cid, "be-dev-1", "first")
await svc.send_chat_message(cid, "be-dev-1", "first") await svc.send_chat_message(cid, "be-dev-1", "second")
await svc.send_chat_message(cid, "be-dev-1", "second") msgs = await svc.get_messages(cid, "be-dev-1")
msgs = await svc.get_messages(cid, "be-dev-1") _SENT_COUNT = 2
_SENT_COUNT = 2 assert len(msgs) == _SENT_COUNT
assert len(msgs) == _SENT_COUNT
except Exception:
pytest.skip("Policy denied this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_close_conversation_with_resolution(a2a_setup: dict) -> None: async def test_close_conversation_with_resolution(a2a_setup: dict) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") await svc.close_conversation(UUID(conv.id), "be-dev-1", resolution="done")
await svc.close_conversation(UUID(conv.id), "be-dev-1", resolution="done")
except Exception:
pytest.skip("Policy denied this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_mark_read_clears_unread(a2a_setup: dict) -> None: async def test_mark_read_clears_unread(a2a_setup: dict) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") await svc.mark_read(UUID(conv.id), "be-dev-1")
await svc.mark_read(UUID(conv.id), "be-dev-1")
except Exception:
pytest.skip("Policy denied this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -505,12 +489,9 @@ async def test_close_conversation_non_participant_raises(
a2a_setup: dict, a2a_setup: dict,
) -> None: ) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") with pytest.raises(ValueError, match="Not a participant"):
with pytest.raises(ValueError, match="Not a participant"): await svc.close_conversation(UUID(conv.id), "ghost-agent")
await svc.close_conversation(UUID(conv.id), "ghost-agent")
except Exception:
pytest.skip("Policy denied this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -518,12 +499,9 @@ async def test_send_chat_message_non_participant_raises(
a2a_setup: dict, a2a_setup: dict,
) -> None: ) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") with pytest.raises(ValueError, match="Not a participant"):
with pytest.raises(ValueError, match="Not a participant"): await svc.send_chat_message(UUID(conv.id), "ghost", "hi")
await svc.send_chat_message(UUID(conv.id), "ghost", "hi")
except Exception:
pytest.skip("Policy denied this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -580,9 +558,7 @@ async def test_update_task_with_message_appends_to_notes(
) -> None: ) -> None:
"""Use a real DB-backed task instance to avoid SA private state issues.""" """Use a real DB-backed task instance to avoid SA private state issues."""
db = a2a_setup["db"] db = a2a_setup["db"]
task = (await db.execute(select(TaskTable).limit(1))).scalar_one_or_none() task = (await db.execute(select(TaskTable).limit(1))).scalar_one()
if task is None:
pytest.skip("no task in DB")
original_notes = task.dev_notes original_notes = task.dev_notes
task.dev_notes = "existing notes" task.dev_notes = "existing notes"
msg = A2AMessage(role="user", parts=[TextPart(text="new message")]) msg = A2AMessage(role="user", parts=[TextPart(text="new message")])
@@ -597,9 +573,7 @@ async def test_update_task_with_message_no_text_parts_noop(
a2a_setup: dict, a2a_setup: dict,
) -> None: ) -> None:
db = a2a_setup["db"] db = a2a_setup["db"]
task = (await db.execute(select(TaskTable).limit(1))).scalar_one_or_none() task = (await db.execute(select(TaskTable).limit(1))).scalar_one()
if task is None:
pytest.skip("no task in DB")
original = task.dev_notes original = task.dev_notes
task.dev_notes = "existing" task.dev_notes = "existing"
msg = A2AMessage(role="user", parts=[]) msg = A2AMessage(role="user", parts=[])
@@ -1198,11 +1172,8 @@ async def test_list_conversations_with_status_and_task_filter(
async def test_list_conversations_with_messages(a2a_setup: dict) -> None: async def test_list_conversations_with_messages(a2a_setup: dict) -> None:
"""Seed a conversation + a message so the last_message preview path runs.""" """Seed a conversation + a message so the last_message preview path runs."""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") await svc.send_chat_message(UUID(conv.id), "be-dev-1", "preview text")
await svc.send_chat_message(UUID(conv.id), "be-dev-1", "preview text")
except Exception:
pytest.skip("Policy denies this pair")
convs = await svc.list_conversations("be-dev-1") convs = await svc.list_conversations("be-dev-1")
# last_message_preview should be truthy for this conv. # last_message_preview should be truthy for this conv.
@@ -1218,19 +1189,23 @@ async def test_list_conversations_with_messages(a2a_setup: dict) -> None:
async def test_send_chat_message_options_response_to( async def test_send_chat_message_options_response_to(
a2a_setup: dict, a2a_setup: dict,
) -> None: ) -> None:
"""response_to_id and requires_response options surface on the message.""" """response_to_id and requires_response options surface on the message.
`response_to_id` has a FK to `a2a_messages.id`, so we send a real
"first" message to thread off passing a random UUID would hit FK
violation.
"""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") first = await svc.send_chat_message(UUID(conv.id), "be-qa", "first message")
msg = await svc.send_chat_message( msg = await svc.send_chat_message(
UUID(conv.id), UUID(conv.id),
"be-dev-1", "be-dev-1",
"needs answer", "needs answer",
options={"requires_response": True, "response_to_id": _u()}, options={"requires_response": True, "response_to_id": UUID(first.id)},
) )
assert msg.requires_response is True assert msg.requires_response is True
except Exception: assert msg.response_to_id == first.id
pytest.skip("Policy denies this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -1238,18 +1213,15 @@ async def test_send_chat_message_from_agent_b_increments_unread_a(
a2a_setup: dict, a2a_setup: dict,
) -> None: ) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: # First exchange establishes canonical pair (a < b lexicographically).
# First exchange establishes canonical pair (a < b lexicographically). conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") # Send from whichever agent is conv.agent_b (the "other" side).
# Send from whichever agent is conv.agent_b (the "other" side). result = await svc.session.execute(
result = await svc.session.execute( _sel(A2AConversationTable).where(A2AConversationTable.id == UUID(conv.id))
_sel(A2AConversationTable).where(A2AConversationTable.id == UUID(conv.id)) )
) row = result.scalar_one()
row = result.scalar_one() msg = await svc.send_chat_message(UUID(conv.id), row.agent_b, "hi from b")
msg = await svc.send_chat_message(UUID(conv.id), row.agent_b, "hi from b") assert msg.from_agent == row.agent_b
assert msg.from_agent == row.agent_b
except Exception:
pytest.skip("Policy denies this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1262,26 +1234,20 @@ async def test_get_messages_non_participant_returns_empty(
a2a_setup: dict, a2a_setup: dict,
) -> None: ) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") msgs = await svc.get_messages(UUID(conv.id), "ghost-agent")
msgs = await svc.get_messages(UUID(conv.id), "ghost-agent") assert msgs == []
assert msgs == []
except Exception:
pytest.skip("Policy denies this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_get_messages_with_before_filter(a2a_setup: dict) -> None: async def test_get_messages_with_before_filter(a2a_setup: dict) -> None:
"""Pass a `before` datetime — exercises the filter branch.""" """Pass a `before` datetime — exercises the filter branch."""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") await svc.send_chat_message(UUID(conv.id), "be-dev-1", "first")
await svc.send_chat_message(UUID(conv.id), "be-dev-1", "first") future = datetime.now(UTC).replace(year=2099)
future = datetime.now(UTC).replace(year=2099) msgs = await svc.get_messages(UUID(conv.id), "be-dev-1", before=future)
msgs = await svc.get_messages(UUID(conv.id), "be-dev-1", before=future) assert isinstance(msgs, list)
assert isinstance(msgs, list)
except Exception:
pytest.skip("Policy denies this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1292,26 +1258,20 @@ async def test_get_messages_with_before_filter(a2a_setup: dict) -> None:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_mark_read_non_participant(a2a_setup: dict) -> None: async def test_mark_read_non_participant(a2a_setup: dict) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") # Non-participant → silent return.
# Non-participant → silent return. await svc.mark_read(UUID(conv.id), "ghost")
await svc.mark_read(UUID(conv.id), "ghost")
except Exception:
pytest.skip("Policy denies this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_mark_read_as_agent_b(a2a_setup: dict) -> None: async def test_mark_read_as_agent_b(a2a_setup: dict) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") result = await svc.session.execute(
result = await svc.session.execute( _sel(A2AConversationTable).where(A2AConversationTable.id == UUID(conv.id))
_sel(A2AConversationTable).where(A2AConversationTable.id == UUID(conv.id)) )
) row = result.scalar_one()
row = result.scalar_one() await svc.mark_read(UUID(conv.id), row.agent_b)
await svc.mark_read(UUID(conv.id), row.agent_b)
except Exception:
pytest.skip("Policy denies this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1322,16 +1282,13 @@ async def test_mark_read_as_agent_b(a2a_setup: dict) -> None:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_list_pairs_with_conversations(a2a_setup: dict) -> None: async def test_list_pairs_with_conversations(a2a_setup: dict) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: await svc.get_or_create_conversation("be-dev-1", "be-qa")
await svc.get_or_create_conversation("be-dev-1", "be-qa") pairs = await svc.list_pairs("be-dev-1")
pairs = await svc.list_pairs("be-dev-1") assert any(
assert any( (p.agent_a, p.agent_b) == ("be-dev-1", "be-qa")
(p.agent_a, p.agent_b) == ("be-dev-1", "be-qa") or (p.agent_a, p.agent_b) == ("be-qa", "be-dev-1")
or (p.agent_a, p.agent_b) == ("be-qa", "be-dev-1") for p in pairs
for p in pairs )
)
except Exception:
pytest.skip("Policy denies this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1367,17 +1324,14 @@ async def test_send_gateway_adapter_uuid_to_uuid(
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
dev = a2a_setup["dev"] dev = a2a_setup["dev"]
qa = a2a_setup["qa"] qa = a2a_setup["qa"]
try: msg = await svc.send(
msg = await svc.send( from_agent=dev.id,
from_agent=dev.id, to_agent=qa.id,
to_agent=qa.id, task_id=a2a_setup["task_id"],
task_id=a2a_setup["task_id"], body="hello",
body="hello", skill="general",
skill="general", )
) assert msg.content == "hello"
assert msg.content == "hello"
except Exception:
pytest.skip("Policy denies this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -1387,16 +1341,13 @@ async def test_send_gateway_adapter_with_string_recipient(
"""Recipient as slug string → no DB lookup for it.""" """Recipient as slug string → no DB lookup for it."""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
dev = a2a_setup["dev"] dev = a2a_setup["dev"]
try: msg = await svc.send(
msg = await svc.send( from_agent=dev.id,
from_agent=dev.id, to_agent="be-qa",
to_agent="be-qa", task_id=a2a_setup["task_id"],
task_id=a2a_setup["task_id"], body="hello",
body="hello", )
) assert msg.content == "hello"
assert msg.content == "hello"
except Exception:
pytest.skip("Policy denies this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1458,12 +1409,9 @@ async def test_get_or_create_conversation_with_topic(
a2a_setup: dict, a2a_setup: dict,
) -> None: ) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: a = await svc.get_or_create_conversation("be-dev-1", "be-qa", topic="Bug X")
a = await svc.get_or_create_conversation("be-dev-1", "be-qa", topic="Bug X") b = await svc.get_or_create_conversation("be-dev-1", "be-qa", topic="Bug X")
b = await svc.get_or_create_conversation("be-dev-1", "be-qa", topic="Bug X") assert a.id == b.id
assert a.id == b.id
except Exception:
pytest.skip("Policy denies this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1474,12 +1422,9 @@ async def test_get_or_create_conversation_with_topic(
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_get_conversation_non_participant(a2a_setup: dict) -> None: async def test_get_conversation_non_participant(a2a_setup: dict) -> None:
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") result = await svc.get_conversation(UUID(conv.id), "ghost")
result = await svc.get_conversation(UUID(conv.id), "ghost") assert result is None
assert result is None
except Exception:
pytest.skip("Policy denies this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -1488,13 +1433,10 @@ async def test_get_conversation_returns_model_when_participant(
) -> None: ) -> None:
"""Participant access → returns the conversation model.""" """Participant access → returns the conversation model."""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") result = await svc.get_conversation(UUID(conv.id), "be-dev-1")
result = await svc.get_conversation(UUID(conv.id), "be-dev-1") assert result is not None
assert result is not None assert result.id == conv.id
assert result.id == conv.id
except Exception:
pytest.skip("Policy denies this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1506,18 +1448,15 @@ async def test_get_conversation_returns_model_when_participant(
async def test_get_inbox_summary_with_unread(a2a_setup: dict) -> None: async def test_get_inbox_summary_with_unread(a2a_setup: dict) -> None:
"""Send a message from a2 → a1 has unread.""" """Send a message from a2 → a1 has unread."""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
try: conv = await svc.get_or_create_conversation("be-dev-1", "be-qa")
conv = await svc.get_or_create_conversation("be-dev-1", "be-qa") result = await svc.session.execute(
result = await svc.session.execute( _sel(A2AConversationTable).where(A2AConversationTable.id == UUID(conv.id))
_sel(A2AConversationTable).where(A2AConversationTable.id == UUID(conv.id)) )
) row = result.scalar_one()
row = result.scalar_one() # Send from agent_b → agent_a unread increments.
# Send from agent_b → agent_a unread increments. await svc.send_chat_message(UUID(conv.id), row.agent_b, "hi")
await svc.send_chat_message(UUID(conv.id), row.agent_b, "hi") inbox = await svc.get_inbox_summary(row.agent_a)
inbox = await svc.get_inbox_summary(row.agent_a) assert inbox.total_unread >= 1
assert inbox.total_unread >= 1
except Exception:
pytest.skip("Policy denies this pair")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1532,16 +1471,13 @@ async def test_send_gateway_adapter_skill_none(
"""skill=None branch → options dict stays empty.""" """skill=None branch → options dict stays empty."""
svc = a2a_setup["svc"] svc = a2a_setup["svc"]
dev = a2a_setup["dev"] dev = a2a_setup["dev"]
try: msg = await svc.send(
msg = await svc.send( from_agent=dev.id,
from_agent=dev.id, to_agent="be-qa",
to_agent="be-qa", task_id=a2a_setup["task_id"],
task_id=a2a_setup["task_id"], body="no skill",
body="no skill", )
) assert msg.content == "no skill"
assert msg.content == "no skill"
except Exception:
pytest.skip("Policy denies this pair")
@pytest.mark.asyncio @pytest.mark.asyncio
+8 -6
View File
@@ -236,13 +236,15 @@ async def test_get_ceo_velocity(dashboard_client: AsyncClient) -> None:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_get_main_pm_kanban_unreachable( async def test_get_main_pm_kanban_via_http(dashboard_client: AsyncClient) -> None:
dashboard_client: AsyncClient, """`/kanban/main-pm` is now declared before `/kanban/{team}`, so it routes
) -> None: correctly to `get_main_pm_kanban` instead of being matched as
"""Route order quirk: /kanban/{team} matches first; main-pm is unreachable.""" `team=main-pm` (which would 422)."""
response = await dashboard_client.get("/api/dashboard/kanban/main-pm", headers=_HDR) response = await dashboard_client.get("/api/dashboard/kanban/main-pm", headers=_HDR)
# Matched as /kanban/{team} with team=main-pm → invalid Team enum assert response.status_code == HTTPStatus.OK
assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY body = response.json()
# main_pm board has columns; shape is from KanbanBoard.model_dump().
assert "columns" in body
@pytest.mark.asyncio @pytest.mark.asyncio
+3 -37
View File
@@ -2355,46 +2355,12 @@ async def test_get_or_create_active_session_returns_existing_active(
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# _walk_task_ancestors — cycle-safety + parent missing # (orphan-parent walk is covered by test_walk_task_ancestors_orphan_parent_breaks
# elsewhere in this file, which patches session.execute to bypass the FK
# constraint that prevents inserting a real orphan row.)
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_walk_task_ancestors_orphan_parent(
msg_setup: dict, db_session: AsyncSession
) -> None:
"""parent_task_id points at non-existent task → walk stops."""
aid = msg_setup["agent_id"]
base_task_result = await db_session.execute(
__import__("sqlalchemy")
.select(TaskTable)
.where(TaskTable.id == msg_setup["task_id"])
)
base_task = base_task_result.scalar_one()
ghost_parent = uuid4()
child = TaskTable(
id=uuid4(),
title="child-orphan",
description="d",
acceptance_criteria=["ac"],
status=TaskStatus.PENDING,
priority=2,
task_type=TaskType.CODE,
nature=TaskNature.TECHNICAL,
project_id=base_task.project_id,
created_by=aid,
team=base_task.team,
parent_task_id=ghost_parent,
)
db_session.add(child)
# parent_task_id has FK to tasks; using a non-existent id will fail FK.
# Skip rather than committing — exercise the seen-set/path differently.
try:
await db_session.flush()
except Exception:
pytest.skip("FK enforces parent existence — orphan branch unreachable here")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# _resolve_group_for_session — auto-create default group when channel empty # _resolve_group_for_session — auto-create default group when channel empty
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
+15 -7
View File
@@ -330,13 +330,21 @@ async def test_get_single_stats_success(optimal_client: AsyncClient) -> None:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_check_staleness_unreachable_via_get( async def test_check_staleness_via_http(optimal_client: AsyncClient) -> None:
optimal_client: AsyncClient, """`/stats/staleness` is now declared before `/stats/{index_type}`, so it
) -> None: routes correctly to `check_staleness` instead of being matched as
"""Route order quirk: /stats/{index_type} matches first → 400 invalid type.""" `index_type=staleness` (which would 400 as invalid IndexType)."""
response = await optimal_client.get("/api/optimal/stats/staleness", headers=_HDR) with patch("roboco.api.routes.optimal.get_optimal_service") as mock_get:
# Matched as stats/{index_type} with type=staleness → invalid mock_service = AsyncMock()
assert response.status_code == HTTPStatus.BAD_REQUEST mock_service.check_index_staleness = AsyncMock(
return_value={"stale": False, "indexes": {}}
)
mock_get.return_value = mock_service
response = await optimal_client.get(
"/api/optimal/stats/staleness", headers=_HDR
)
assert response.status_code == HTTPStatus.OK
assert response.json() == {"stale": False, "indexes": {}}
@pytest.mark.asyncio @pytest.mark.asyncio
+15 -31
View File
@@ -15,7 +15,6 @@ from httpx import ASGITransport, AsyncClient
from roboco.api.deps import get_agent_context, get_db from roboco.api.deps import get_agent_context, get_db
from roboco.api.routes.tasks import ( from roboco.api.routes.tasks import (
_translate_error, _translate_error,
create_task,
get_awaiting_ceo_approval_tasks, get_awaiting_ceo_approval_tasks,
get_awaiting_pm_review_tasks, get_awaiting_pm_review_tasks,
) )
@@ -31,7 +30,6 @@ from roboco.models.base import (
TaskType, TaskType,
) )
from roboco.models.permissions import AgentContext from roboco.models.permissions import AgentContext
from roboco.models.task import TaskCreate
from roboco.services.base import ( from roboco.services.base import (
NotFoundError, NotFoundError,
ServiceError, ServiceError,
@@ -1385,36 +1383,22 @@ async def test_list_tasks_developer_with_team_filters_to_own(
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_create_task_project_required_branch(task_client: dict) -> None: async def test_create_task_missing_project_id_returns_422(task_client: dict) -> None:
"""create_task PROJECT_REQUIRED branch — pydantic normally enforces UUID, """`TaskCreate.project_id` is `UUID` (required); pydantic rejects missing
but invoke the helper directly with `model_construct` to cover the inline value with 422 before the route runs. (The previously-dead inline runtime
`if not data.project_id` 400-raising branch. `if not data.project_id` branch was removed.)"""
""" response = await task_client["client"].post(
bypassed = TaskCreate.model_construct( "/api/tasks",
title="T", json={
description="d", "title": "T",
acceptance_criteria=["a"], "description": "d",
team=Team.BACKEND, "acceptance_criteria": ["a"],
project_id=None, "team": "backend",
priority=2, # project_id intentionally missing
nature=TaskNature.TECHNICAL, },
task_type=TaskType.CODE, headers=_HDR,
sequence=0,
dependency_ids=[],
) )
agent_ctx = AgentContext( assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY
agent_id=task_client["agent"].id, role=AgentRole.MAIN_PM, team=None
)
permissions = PermissionService()
with pytest.raises(HTTPException) as exc_info:
await create_task(
data=bypassed,
db=task_client["db"],
agent=agent_ctx,
permissions=permissions,
)
assert exc_info.value.status_code == HTTPStatus.BAD_REQUEST
assert "PROJECT_REQUIRED" in str(exc_info.value.detail)
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -344,42 +344,6 @@ async def test_delegate_unknown_role_rejected() -> None:
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_delegate_static_guard_unknown_slug_via_helper() -> None:
"""Lines 1299-1304: unknown slug rejection in _delegate_static_guards.
Reached by calling the private helper directly the public ``delegate``
path is shielded by the chain validator which catches all slugs that
are not in the explicit cell_pm/main_pm target sets first.
"""
pm_id = uuid4()
parent_id = uuid4()
parent = MagicMock(
status="in_progress",
assigned_to=pm_id,
project_id=uuid4(),
title="p",
)
task_svc = AsyncMock()
task_svc.get.return_value = parent
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c._delegate_static_guards(
pm_id,
parent_id,
parent,
DelegateInputs(
title="x",
description="y",
assigned_to="ghost-agent",
team="backend",
),
)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "unknown agent slug" in body["message"]
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_delegate_parent_no_project_rejected() -> None: async def test_delegate_parent_no_project_rejected() -> None:
"""Line 1306: parent.project_id is None → invalid_state.""" """Line 1306: parent.project_id is None → invalid_state."""